[PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
@ 2021-09-24 11:42 Ashraf Ali S
  2021-09-27  1:10 ` Chiu, Chasel
  0 siblings, 1 reply; 6+ messages in thread
From: Ashraf Ali S @ 2021-09-24 11:42 UTC (permalink / raw)
  To: devel
  Cc: Ashraf Ali S, Chasel Chiu, Nate DeSimone, Star Zeng, Kuo Ted,
	Duggapu Chinni B, Rangasai V Chaganty, Digant H Solanki,
	Sangeetha V, Ray Ni

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642
when the module is not building in IA32 mode which will lead to
building error. when a module built-in X64 function pointer will be the
size of 64bit width which cannot be fit in 32bit address which will lead
to error. to overcome this issue introducing the 2 new PCD's
for the 64bit modules can consume it.
Creating the API's to support different architecture

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Kuo Ted <ted.kuo@intel.com>
Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Digant H Solanki <digant.h.solanki@intel.com>
Cc: Sangeetha V <sangeetha.v@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
---
 .../FspmWrapperPeim/FspmWrapperPeim.c         | 19 +++++++++++---
 .../FspmWrapperPeim/FspmWrapperPeim.inf       | 16 ++++++++++--
 .../FspmWrapperPeim/IA32/FspmHelper.c         | 26 +++++++++++++++++++
 .../FspmWrapperPeim/X64/FspmHelper.c          | 26 +++++++++++++++++++
 .../FspsWrapperPeim/FspsWrapperPeim.c         | 17 +++++++++---
 .../FspsWrapperPeim/FspsWrapperPeim.inf       | 14 +++++++++-
 .../FspsWrapperPeim/IA32/FspsHelper.c         | 26 +++++++++++++++++++
 .../FspsWrapperPeim/X64/FspsHelper.c          | 26 +++++++++++++++++++
 IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  2 ++
 9 files changed, 162 insertions(+), 10 deletions(-)
 create mode 100644 IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
 create mode 100644 IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
 create mode 100644 IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
 create mode 100644 IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c

diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
index 24ab534620..4a15136c39 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
@@ -3,7 +3,7 @@
   register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
   notify to call FspSiliconInit API.
 
-  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -39,6 +39,17 @@
 
 extern EFI_GUID gFspHobGuid;
 
+/**
+  Get the Fspm Upd Data Address from the PCD
+
+  @return FSPM UPD Data Address
+**/
+UINTN
+EFIAPI
+GetFspmUpdDataAddress (
+  VOID
+  );
+
 /**
   Call FspMemoryInit API.
 
@@ -59,7 +70,7 @@ PeiFspMemoryInit (
 
   DEBUG ((DEBUG_INFO, "PeiFspMemoryInit enter\n"));
 
-  FspHobListPtr = NULL;
+  FspHobListPtr  = NULL;
   FspmUpdDataPtr = NULL;
 
   FspmHeaderPtr = (FSP_INFO_HEADER *) FspFindFspHeader (PcdGet32 (PcdFspmBaseAddress));
@@ -68,7 +79,7 @@ PeiFspMemoryInit (
     return EFI_DEVICE_ERROR;
   }
 
-  if (PcdGet32 (PcdFspmUpdDataAddress) == 0 && (FspmHeaderPtr->CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
+  if (GetFspmUpdDataAddress () == 0 && (FspmHeaderPtr->CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
     //
     // Copy default FSP-M UPD data from Flash
     //
@@ -80,7 +91,7 @@ PeiFspMemoryInit (
     //
     // External UPD is ready, get the buffer from PCD pointer.
     //
-    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32 (PcdFspmUpdDataAddress);
+    FspmUpdDataPtr = (FSPM_UPD_COMMON *) GetFspmUpdDataAddress ();
     ASSERT (FspmUpdDataPtr != NULL);
   }
 
diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
index 00166e56a0..5b4ad531e7 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
@@ -6,7 +6,7 @@
 # register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
 # notify to call FspSiliconInit API.
 #
-#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -45,6 +45,7 @@
   FspWrapperApiLib
   FspWrapperApiTestLib
   FspMeasurementLib
+  PcdLib
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -56,14 +57,25 @@
 
 [Pcd]
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress       ## CONSUMES
-  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## CONSUMES
 
+[Pcd.IA32]
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ## CONSUMES
+
+[Pcd.X64]
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64  ## CONSUMES
+
 [Sources]
   FspmWrapperPeim.c
 
+[Sources.IA32]
+  IA32/FspmHelper.c
+
+[Sources.X64]
+  X64/FspmHelper.c
+
 [Guids]
   gFspHobGuid                           ## PRODUCES ## HOB
   gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c b/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
new file mode 100644
index 0000000000..cab11173cc
--- /dev/null
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
@@ -0,0 +1,26 @@
+/** @file
+  Sample to provide FSP wrapper related function.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#include <Library/PcdLib.h>
+#include <Library/FspWrapperPlatformLib.h>
+#include <Uefi/UefiBaseType.h>
+
+/**
+  Get the Fspm Upd Data Address from the PCD
+
+  @return FSPM UPD Data Address
+**/
+UINTN
+EFIAPI
+GetFspmUpdDataAddress (
+  VOID
+  )
+{
+  return PcdGet32 (PcdFspmUpdDataAddress);
+}
diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c b/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
new file mode 100644
index 0000000000..25b89ff2e1
--- /dev/null
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
@@ -0,0 +1,26 @@
+/** @file
+  Sample to provide FSP wrapper related function.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#include <Library/PcdLib.h>
+#include <Library/FspWrapperPlatformLib.h>
+#include <Uefi/UefiBaseType.h>
+
+/**
+  Get the Fspm Upd Data Address from the PCD
+
+  @return FSPM UPD Data Address
+**/
+UINTN
+EFIAPI
+GetFspmUpdDataAddress (
+  VOID
+  )
+{
+  return PcdGet64 (PcdFspmUpdDataAddress64);
+}
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
index 9d4f279e81..8bd510502f 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
@@ -3,7 +3,7 @@
   register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
   notify to call FspSiliconInit API.
 
-  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -58,6 +58,17 @@ S3EndOfPeiNotify(
   IN VOID                      *Ppi
   );
 
+/**
+  Get the Fsps Upd Data Address from the PCD
+
+  @return FSPS UPD Data Address
+**/
+UINTN
+EFIAPI
+GetFspsUpdDataAddress (
+  VOID
+  );
+
 EFI_PEI_NOTIFY_DESCRIPTOR mS3EndOfPeiNotifyDesc = {
   (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
   &gEfiEndOfPeiSignalPpiGuid,
@@ -283,7 +294,7 @@ PeiMemoryDiscoveredNotify (
     return EFI_DEVICE_ERROR;
   }
 
-  if (PcdGet32 (PcdFspsUpdDataAddress) == 0 && (FspsHeaderPtr->CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
+  if (GetFspsUpdDataAddress () == 0 && (FspsHeaderPtr->CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
     //
     // Copy default FSP-S UPD data from Flash
     //
@@ -292,7 +303,7 @@ PeiMemoryDiscoveredNotify (
     SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase + (UINTN)FspsHeaderPtr->CfgRegionOffset);
     CopyMem (FspsUpdDataPtr, SourceData, (UINTN)FspsHeaderPtr->CfgRegionSize);
   } else {
-    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32 (PcdFspsUpdDataAddress);
+    FspsUpdDataPtr = (FSPS_UPD_COMMON *) GetFspsUpdDataAddress ();
     ASSERT (FspsUpdDataPtr != NULL);
   }
 
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
index aeeca58d6d..e988ebab21 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
@@ -45,6 +45,7 @@
   FspWrapperApiLib
   FspWrapperApiTestLib
   FspMeasurementLib
+  PcdLib
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -65,10 +66,15 @@
 
 [Pcd]
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress       ## CONSUMES
-  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## CONSUMES
 
+[Pcd.IA32]
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
+
+[Pcd.X64]
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64  ## CONSUMES
+
 [Guids]
   gFspHobGuid                           ## CONSUMES ## HOB
   gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
@@ -76,5 +82,11 @@
 [Sources]
   FspsWrapperPeim.c
 
+[Sources.IA32]
+  IA32/FspsHelper.c
+
+[Sources.X64]
+  X64/FspsHelper.c
+
 [Depex]
   gEfiPeiMemoryDiscoveredPpiGuid
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c b/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
new file mode 100644
index 0000000000..c4ae292ffb
--- /dev/null
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
@@ -0,0 +1,26 @@
+/** @file
+  Sample to provide FSP wrapper related function.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#include <Library/PcdLib.h>
+#include <Library/FspWrapperPlatformLib.h>
+#include <Uefi/UefiBaseType.h>
+
+/**
+  Get the Fsps Upd Data Address from the PCD
+
+  @return FSPS UPD Data Address
+**/
+UINTN
+EFIAPI
+GetFspsUpdDataAddress (
+  VOID
+  )
+{
+  return PcdGet32 (PcdFspsUpdDataAddress);
+}
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c b/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
new file mode 100644
index 0000000000..a0d6adb281
--- /dev/null
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
@@ -0,0 +1,26 @@
+/** @file
+  Sample to provide FSP wrapper related function.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#include <Library/PcdLib.h>
+#include <Library/FspWrapperPlatformLib.h>
+#include <Uefi/UefiBaseType.h>
+
+/**
+  Get the Fsps Upd Data Address from the PCD
+
+  @return FSPS UPD Data Address
+**/
+UINTN
+EFIAPI
+GetFspsUpdDataAddress (
+  VOID
+  )
+{
+  return PcdGet64 (PcdFspsUpdDataAddress64);
+}
diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
index a3b9363779..8c98dbd55d 100644
--- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
+++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
@@ -121,3 +121,5 @@
   #
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UINT32|0x50000000
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UINT32|0x50000001
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|UINT64|0x50000002
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UINT64|0x50000003
-- 
2.30.2.windows.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
  2021-09-24 11:42 [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type Ashraf Ali S
@ 2021-09-27  1:10 ` Chiu, Chasel
  2021-09-27  8:44   ` Zeng, Star
  0 siblings, 1 reply; 6+ messages in thread
From: Chiu, Chasel @ 2021-09-27  1:10 UTC (permalink / raw)
  To: S, Ashraf Ali, devel@edk2.groups.io
  Cc: Desimone, Nathaniel L, Zeng, Star, Kuo, Ted, Duggapu, Chinni B,
	Chaganty, Rangasai V, Solanki, Digant H, V, Sangeetha, Ni, Ray


Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Friday, September 24, 2021 7:43 PM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B
> <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Solanki, Digant H
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on
> Build Type
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642
> when the module is not building in IA32 mode which will lead to building error.
> when a module built-in X64 function pointer will be the size of 64bit width which
> cannot be fit in 32bit address which will lead to error. to overcome this issue
> introducing the 2 new PCD's for the 64bit modules can consume it.
> Creating the API's to support different architecture
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Kuo Ted <ted.kuo@intel.com>
> Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Digant H Solanki <digant.h.solanki@intel.com>
> Cc: Sangeetha V <sangeetha.v@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
> ---
>  .../FspmWrapperPeim/FspmWrapperPeim.c         | 19 +++++++++++---
>  .../FspmWrapperPeim/FspmWrapperPeim.inf       | 16 ++++++++++--
>  .../FspmWrapperPeim/IA32/FspmHelper.c         | 26 +++++++++++++++++++
>  .../FspmWrapperPeim/X64/FspmHelper.c          | 26 +++++++++++++++++++
>  .../FspsWrapperPeim/FspsWrapperPeim.c         | 17 +++++++++---
>  .../FspsWrapperPeim/FspsWrapperPeim.inf       | 14 +++++++++-
>  .../FspsWrapperPeim/IA32/FspsHelper.c         | 26 +++++++++++++++++++
>  .../FspsWrapperPeim/X64/FspsHelper.c          | 26 +++++++++++++++++++
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  2 ++
>  9 files changed, 162 insertions(+), 10 deletions(-)  create mode 100644
> IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
>  create mode 100644
> IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
>  create mode 100644
> IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
>  create mode 100644 IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> index 24ab534620..4a15136c39 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -39,6 +39,17 @@
> 
>  extern EFI_GUID gFspHobGuid;
> 
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  );
> +
>  /**
>    Call FspMemoryInit API.
> 
> @@ -59,7 +70,7 @@ PeiFspMemoryInit (
> 
>    DEBUG ((DEBUG_INFO, "PeiFspMemoryInit enter\n"));
> 
> -  FspHobListPtr = NULL;
> +  FspHobListPtr  = NULL;
>    FspmUpdDataPtr = NULL;
> 
>    FspmHeaderPtr = (FSP_INFO_HEADER *) FspFindFspHeader (PcdGet32
> (PcdFspmBaseAddress)); @@ -68,7 +79,7 @@ PeiFspMemoryInit (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if (PcdGet32 (PcdFspmUpdDataAddress) == 0 && (FspmHeaderPtr-
> >CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
> +  if (GetFspmUpdDataAddress () == 0 && (FspmHeaderPtr->CfgRegionSize !=
> + 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-M UPD data from Flash
>      //
> @@ -80,7 +91,7 @@ PeiFspMemoryInit (
>      //
>      // External UPD is ready, get the buffer from PCD pointer.
>      //
> -    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32
> (PcdFspmUpdDataAddress);
> +    FspmUpdDataPtr = (FSPM_UPD_COMMON *) GetFspmUpdDataAddress ();
>      ASSERT (FspmUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> index 00166e56a0..5b4ad531e7 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -45,6 +45,7 @@
>    FspWrapperApiLib
>    FspWrapperApiTestLib
>    FspMeasurementLib
> +  PcdLib
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -56,14 +57,25 @@
> 
>  [Pcd]
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress       ## CONSUMES
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ##
> CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ##
> CONSUMES
> 
> +[Pcd.IA32]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ##
> CONSUMES
> +
> +[Pcd.X64]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64  ##
> CONSUMES
> +
>  [Sources]
>    FspmWrapperPeim.c
> 
> +[Sources.IA32]
> +  IA32/FspmHelper.c
> +
> +[Sources.X64]
> +  X64/FspmHelper.c
> +
>  [Guids]
>    gFspHobGuid                           ## PRODUCES ## HOB
>    gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> new file mode 100644
> index 0000000000..cab11173cc
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet32 (PcdFspmUpdDataAddress); }
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> new file mode 100644
> index 0000000000..25b89ff2e1
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet64 (PcdFspmUpdDataAddress64); }
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> index 9d4f279e81..8bd510502f 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -58,6 +58,17 @@ S3EndOfPeiNotify(
>    IN VOID                      *Ppi
>    );
> 
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  );
> +
>  EFI_PEI_NOTIFY_DESCRIPTOR mS3EndOfPeiNotifyDesc = {
>    (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK |
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
>    &gEfiEndOfPeiSignalPpiGuid,
> @@ -283,7 +294,7 @@ PeiMemoryDiscoveredNotify (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if (PcdGet32 (PcdFspsUpdDataAddress) == 0 && (FspsHeaderPtr-
> >CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
> +  if (GetFspsUpdDataAddress () == 0 && (FspsHeaderPtr->CfgRegionSize !=
> + 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-S UPD data from Flash
>      //
> @@ -292,7 +303,7 @@ PeiMemoryDiscoveredNotify (
>      SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase +
> (UINTN)FspsHeaderPtr->CfgRegionOffset);
>      CopyMem (FspsUpdDataPtr, SourceData, (UINTN)FspsHeaderPtr-
> >CfgRegionSize);
>    } else {
> -    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32
> (PcdFspsUpdDataAddress);
> +    FspsUpdDataPtr = (FSPS_UPD_COMMON *) GetFspsUpdDataAddress ();
>      ASSERT (FspsUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> index aeeca58d6d..e988ebab21 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> @@ -45,6 +45,7 @@
>    FspWrapperApiLib
>    FspWrapperApiTestLib
>    FspMeasurementLib
> +  PcdLib
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -65,10 +66,15 @@
> 
>  [Pcd]
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress       ## CONSUMES
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ##
> CONSUMES
> 
> +[Pcd.IA32]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
> +
> +[Pcd.X64]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64  ##
> CONSUMES
> +
>  [Guids]
>    gFspHobGuid                           ## CONSUMES ## HOB
>    gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
> @@ -76,5 +82,11 @@
>  [Sources]
>    FspsWrapperPeim.c
> 
> +[Sources.IA32]
> +  IA32/FspsHelper.c
> +
> +[Sources.X64]
> +  X64/FspsHelper.c
> +
>  [Depex]
>    gEfiPeiMemoryDiscoveredPpiGuid
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> new file mode 100644
> index 0000000000..c4ae292ffb
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet32 (PcdFspsUpdDataAddress); }
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> new file mode 100644
> index 0000000000..a0d6adb281
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet64 (PcdFspsUpdDataAddress64); }
> diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> index a3b9363779..8c98dbd55d 100644
> --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> @@ -121,3 +121,5 @@
>    #
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UIN
> T32|0x50000000
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UINT
> 32|0x50000001
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|U
> IN
> + T64|0x50000002
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UI
> N
> + T64|0x50000003
> --
> 2.30.2.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
  2021-09-27  1:10 ` Chiu, Chasel
@ 2021-09-27  8:44   ` Zeng, Star
  2021-09-27 10:15     ` Ashraf Ali S
  0 siblings, 1 reply; 6+ messages in thread
From: Zeng, Star @ 2021-09-27  8:44 UTC (permalink / raw)
  To: Chiu, Chasel, S, Ashraf Ali, devel@edk2.groups.io
  Cc: Desimone, Nathaniel L, Kuo, Ted, Duggapu, Chinni B,
	Chaganty, Rangasai V, Solanki, Digant H, V, Sangeetha, Ni, Ray,
	Zeng, Star

Curious: It does not work to have one function implementation like below?

UINTN
EFIAPI
GetFspmUpdDataAddress (
  VOID
  )
{
  if (PcdGet64 (PcdFspmUpdDataAddress) != 0) {
    return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64);
  } else {
    return (UINTN) PcdGet32 (PcdFspmUpdDataAddress);
  }
}

Thanks,
Star

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@intel.com> 
Sent: Monday, September 27, 2021 9:10 AM
To: S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type


Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Friday, September 24, 2021 7:43 PM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chiu, Chasel 
> <chasel.chiu@intel.com>; Desimone, Nathaniel L 
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; 
> Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B 
> <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V 
> <rangasai.v.chaganty@intel.com>; Solanki, Digant H 
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; 
> Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address 
> based on Build Type
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642
> when the module is not building in IA32 mode which will lead to building error.
> when a module built-in X64 function pointer will be the size of 64bit 
> width which cannot be fit in 32bit address which will lead to error. 
> to overcome this issue introducing the 2 new PCD's for the 64bit modules can consume it.
> Creating the API's to support different architecture
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Kuo Ted <ted.kuo@intel.com>
> Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Digant H Solanki <digant.h.solanki@intel.com>
> Cc: Sangeetha V <sangeetha.v@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
> ---
>  .../FspmWrapperPeim/FspmWrapperPeim.c         | 19 +++++++++++---
>  .../FspmWrapperPeim/FspmWrapperPeim.inf       | 16 ++++++++++--
>  .../FspmWrapperPeim/IA32/FspmHelper.c         | 26 +++++++++++++++++++
>  .../FspmWrapperPeim/X64/FspmHelper.c          | 26 +++++++++++++++++++
>  .../FspsWrapperPeim/FspsWrapperPeim.c         | 17 +++++++++---
>  .../FspsWrapperPeim/FspsWrapperPeim.inf       | 14 +++++++++-
>  .../FspsWrapperPeim/IA32/FspsHelper.c         | 26 +++++++++++++++++++
>  .../FspsWrapperPeim/X64/FspsHelper.c          | 26 +++++++++++++++++++
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  2 ++
>  9 files changed, 162 insertions(+), 10 deletions(-)  create mode 
> 100644 IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
>  create mode 100644
> IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
>  create mode 100644
> IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
>  create mode 100644 
> IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> index 24ab534620..4a15136c39 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -39,6 +39,17 @@
> 
>  extern EFI_GUID gFspHobGuid;
> 
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  );
> +
>  /**
>    Call FspMemoryInit API.
> 
> @@ -59,7 +70,7 @@ PeiFspMemoryInit (
> 
>    DEBUG ((DEBUG_INFO, "PeiFspMemoryInit enter\n"));
> 
> -  FspHobListPtr = NULL;
> +  FspHobListPtr  = NULL;
>    FspmUpdDataPtr = NULL;
> 
>    FspmHeaderPtr = (FSP_INFO_HEADER *) FspFindFspHeader (PcdGet32 
> (PcdFspmBaseAddress)); @@ -68,7 +79,7 @@ PeiFspMemoryInit (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if (PcdGet32 (PcdFspmUpdDataAddress) == 0 && (FspmHeaderPtr-
> >CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
> +  if (GetFspmUpdDataAddress () == 0 && (FspmHeaderPtr->CfgRegionSize 
> + !=
> + 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-M UPD data from Flash
>      //
> @@ -80,7 +91,7 @@ PeiFspMemoryInit (
>      //
>      // External UPD is ready, get the buffer from PCD pointer.
>      //
> -    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32
> (PcdFspmUpdDataAddress);
> +    FspmUpdDataPtr = (FSPM_UPD_COMMON *) GetFspmUpdDataAddress ();
>      ASSERT (FspmUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> index 00166e56a0..5b4ad531e7 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -45,6 +45,7 @@
>    FspWrapperApiLib
>    FspWrapperApiTestLib
>    FspMeasurementLib
> +  PcdLib
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -56,14 +57,25 @@
> 
>  [Pcd]
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress       ## CONSUMES
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ##
> CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## 
> CONSUMES
> 
> +[Pcd.IA32]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ##
> CONSUMES
> +
> +[Pcd.X64]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64  ##
> CONSUMES
> +
>  [Sources]
>    FspmWrapperPeim.c
> 
> +[Sources.IA32]
> +  IA32/FspmHelper.c
> +
> +[Sources.X64]
> +  X64/FspmHelper.c
> +
>  [Guids]
>    gFspHobGuid                           ## PRODUCES ## HOB
>    gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> new file mode 100644
> index 0000000000..cab11173cc
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet32 (PcdFspmUpdDataAddress); }
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> new file mode 100644
> index 0000000000..25b89ff2e1
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet64 (PcdFspmUpdDataAddress64); }
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> index 9d4f279e81..8bd510502f 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -58,6 +58,17 @@ S3EndOfPeiNotify(
>    IN VOID                      *Ppi
>    );
> 
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  );
> +
>  EFI_PEI_NOTIFY_DESCRIPTOR mS3EndOfPeiNotifyDesc = {
>    (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | 
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
>    &gEfiEndOfPeiSignalPpiGuid,
> @@ -283,7 +294,7 @@ PeiMemoryDiscoveredNotify (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if (PcdGet32 (PcdFspsUpdDataAddress) == 0 && (FspsHeaderPtr-
> >CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
> +  if (GetFspsUpdDataAddress () == 0 && (FspsHeaderPtr->CfgRegionSize 
> + !=
> + 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-S UPD data from Flash
>      //
> @@ -292,7 +303,7 @@ PeiMemoryDiscoveredNotify (
>      SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase + 
> (UINTN)FspsHeaderPtr->CfgRegionOffset);
>      CopyMem (FspsUpdDataPtr, SourceData, (UINTN)FspsHeaderPtr-
> >CfgRegionSize);
>    } else {
> -    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32
> (PcdFspsUpdDataAddress);
> +    FspsUpdDataPtr = (FSPS_UPD_COMMON *) GetFspsUpdDataAddress ();
>      ASSERT (FspsUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> index aeeca58d6d..e988ebab21 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> @@ -45,6 +45,7 @@
>    FspWrapperApiLib
>    FspWrapperApiTestLib
>    FspMeasurementLib
> +  PcdLib
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -65,10 +66,15 @@
> 
>  [Pcd]
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress       ## CONSUMES
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## 
> CONSUMES
> 
> +[Pcd.IA32]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
> +
> +[Pcd.X64]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64  ##
> CONSUMES
> +
>  [Guids]
>    gFspHobGuid                           ## CONSUMES ## HOB
>    gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
> @@ -76,5 +82,11 @@
>  [Sources]
>    FspsWrapperPeim.c
> 
> +[Sources.IA32]
> +  IA32/FspsHelper.c
> +
> +[Sources.X64]
> +  X64/FspsHelper.c
> +
>  [Depex]
>    gEfiPeiMemoryDiscoveredPpiGuid
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> new file mode 100644
> index 0000000000..c4ae292ffb
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet32 (PcdFspsUpdDataAddress); }
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> new file mode 100644
> index 0000000000..a0d6adb281
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet64 (PcdFspsUpdDataAddress64); }
> diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> index a3b9363779..8c98dbd55d 100644
> --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> @@ -121,3 +121,5 @@
>    #
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UIN
> T32|0x50000000
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UINT
> 32|0x50000001
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|U
> IN
> + T64|0x50000002
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UI
> N
> + T64|0x50000003
> --
> 2.30.2.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
  2021-09-27  8:44   ` Zeng, Star
@ 2021-09-27 10:15     ` Ashraf Ali S
  2021-09-27 10:36       ` Zeng, Star
       [not found]       ` <16A8A77591607F71.12372@groups.io>
  0 siblings, 2 replies; 6+ messages in thread
From: Ashraf Ali S @ 2021-09-27 10:15 UTC (permalink / raw)
  To: Zeng, Star, Chiu, Chasel, devel@edk2.groups.io
  Cc: Desimone, Nathaniel L, Kuo, Ted, Duggapu, Chinni B,
	Chaganty, Rangasai V, Solanki, Digant H, V, Sangeetha, Ni, Ray

Hi., @Zeng, Star

Creating a single function is doable, but the problem with that is 
If someone set the PCD  PcdFspmUpdDataAddress64 accidentally which should be applicable only in X64. Since the PCD has some junk value, it will return the false data in IA32 case. Which will break everything with respect to FSP-S/M.

To Avoid Such cases separating the IA32 vs X64 is more feasible. And easily differentiating with IA32 and X64. 

Regards,
Ashraf Ali S
Intel Technology India Pvt. Ltd. 

-----Original Message-----
From: Zeng, Star <star.zeng@intel.com> 
Sent: Monday, September 27, 2021 2:15 PM
To: Chiu, Chasel <chasel.chiu@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type

Curious: It does not work to have one function implementation like below?

UINTN
EFIAPI
GetFspmUpdDataAddress (
  VOID
  )
{
  if (PcdGet64 (PcdFspmUpdDataAddress) != 0) {
    return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64);
  } else {
    return (UINTN) PcdGet32 (PcdFspmUpdDataAddress);
  }
}

Thanks,
Star

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@intel.com> 
Sent: Monday, September 27, 2021 9:10 AM
To: S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type


Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Friday, September 24, 2021 7:43 PM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chiu, Chasel 
> <chasel.chiu@intel.com>; Desimone, Nathaniel L 
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; 
> Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B 
> <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V 
> <rangasai.v.chaganty@intel.com>; Solanki, Digant H 
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; 
> Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address 
> based on Build Type
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642
> when the module is not building in IA32 mode which will lead to building error.
> when a module built-in X64 function pointer will be the size of 64bit 
> width which cannot be fit in 32bit address which will lead to error. 
> to overcome this issue introducing the 2 new PCD's for the 64bit modules can consume it.
> Creating the API's to support different architecture
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Kuo Ted <ted.kuo@intel.com>
> Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Digant H Solanki <digant.h.solanki@intel.com>
> Cc: Sangeetha V <sangeetha.v@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
> ---
>  .../FspmWrapperPeim/FspmWrapperPeim.c         | 19 +++++++++++---
>  .../FspmWrapperPeim/FspmWrapperPeim.inf       | 16 ++++++++++--
>  .../FspmWrapperPeim/IA32/FspmHelper.c         | 26 +++++++++++++++++++
>  .../FspmWrapperPeim/X64/FspmHelper.c          | 26 +++++++++++++++++++
>  .../FspsWrapperPeim/FspsWrapperPeim.c         | 17 +++++++++---
>  .../FspsWrapperPeim/FspsWrapperPeim.inf       | 14 +++++++++-
>  .../FspsWrapperPeim/IA32/FspsHelper.c         | 26 +++++++++++++++++++
>  .../FspsWrapperPeim/X64/FspsHelper.c          | 26 +++++++++++++++++++
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  2 ++
>  9 files changed, 162 insertions(+), 10 deletions(-)  create mode 
> 100644 IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
>  create mode 100644
> IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
>  create mode 100644
> IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
>  create mode 100644 
> IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> index 24ab534620..4a15136c39 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -39,6 +39,17 @@
> 
>  extern EFI_GUID gFspHobGuid;
> 
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  );
> +
>  /**
>    Call FspMemoryInit API.
> 
> @@ -59,7 +70,7 @@ PeiFspMemoryInit (
> 
>    DEBUG ((DEBUG_INFO, "PeiFspMemoryInit enter\n"));
> 
> -  FspHobListPtr = NULL;
> +  FspHobListPtr  = NULL;
>    FspmUpdDataPtr = NULL;
> 
>    FspmHeaderPtr = (FSP_INFO_HEADER *) FspFindFspHeader (PcdGet32 
> (PcdFspmBaseAddress)); @@ -68,7 +79,7 @@ PeiFspMemoryInit (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if (PcdGet32 (PcdFspmUpdDataAddress) == 0 && (FspmHeaderPtr-
> >CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
> +  if (GetFspmUpdDataAddress () == 0 && (FspmHeaderPtr->CfgRegionSize 
> + !=
> + 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-M UPD data from Flash
>      //
> @@ -80,7 +91,7 @@ PeiFspMemoryInit (
>      //
>      // External UPD is ready, get the buffer from PCD pointer.
>      //
> -    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32
> (PcdFspmUpdDataAddress);
> +    FspmUpdDataPtr = (FSPM_UPD_COMMON *) GetFspmUpdDataAddress ();
>      ASSERT (FspmUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> index 00166e56a0..5b4ad531e7 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -45,6 +45,7 @@
>    FspWrapperApiLib
>    FspWrapperApiTestLib
>    FspMeasurementLib
> +  PcdLib
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -56,14 +57,25 @@
> 
>  [Pcd]
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress       ## CONSUMES
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ##
> CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## 
> CONSUMES
> 
> +[Pcd.IA32]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ##
> CONSUMES
> +
> +[Pcd.X64]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64  ##
> CONSUMES
> +
>  [Sources]
>    FspmWrapperPeim.c
> 
> +[Sources.IA32]
> +  IA32/FspmHelper.c
> +
> +[Sources.X64]
> +  X64/FspmHelper.c
> +
>  [Guids]
>    gFspHobGuid                           ## PRODUCES ## HOB
>    gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> new file mode 100644
> index 0000000000..cab11173cc
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet32 (PcdFspmUpdDataAddress); }
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> new file mode 100644
> index 0000000000..25b89ff2e1
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet64 (PcdFspmUpdDataAddress64); }
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> index 9d4f279e81..8bd510502f 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -58,6 +58,17 @@ S3EndOfPeiNotify(
>    IN VOID                      *Ppi
>    );
> 
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  );
> +
>  EFI_PEI_NOTIFY_DESCRIPTOR mS3EndOfPeiNotifyDesc = {
>    (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | 
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
>    &gEfiEndOfPeiSignalPpiGuid,
> @@ -283,7 +294,7 @@ PeiMemoryDiscoveredNotify (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if (PcdGet32 (PcdFspsUpdDataAddress) == 0 && (FspsHeaderPtr-
> >CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
> +  if (GetFspsUpdDataAddress () == 0 && (FspsHeaderPtr->CfgRegionSize 
> + !=
> + 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-S UPD data from Flash
>      //
> @@ -292,7 +303,7 @@ PeiMemoryDiscoveredNotify (
>      SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase + 
> (UINTN)FspsHeaderPtr->CfgRegionOffset);
>      CopyMem (FspsUpdDataPtr, SourceData, (UINTN)FspsHeaderPtr-
> >CfgRegionSize);
>    } else {
> -    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32
> (PcdFspsUpdDataAddress);
> +    FspsUpdDataPtr = (FSPS_UPD_COMMON *) GetFspsUpdDataAddress ();
>      ASSERT (FspsUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> index aeeca58d6d..e988ebab21 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> @@ -45,6 +45,7 @@
>    FspWrapperApiLib
>    FspWrapperApiTestLib
>    FspMeasurementLib
> +  PcdLib
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -65,10 +66,15 @@
> 
>  [Pcd]
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress       ## CONSUMES
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## 
> CONSUMES
> 
> +[Pcd.IA32]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
> +
> +[Pcd.X64]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64  ##
> CONSUMES
> +
>  [Guids]
>    gFspHobGuid                           ## CONSUMES ## HOB
>    gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
> @@ -76,5 +82,11 @@
>  [Sources]
>    FspsWrapperPeim.c
> 
> +[Sources.IA32]
> +  IA32/FspsHelper.c
> +
> +[Sources.X64]
> +  X64/FspsHelper.c
> +
>  [Depex]
>    gEfiPeiMemoryDiscoveredPpiGuid
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> new file mode 100644
> index 0000000000..c4ae292ffb
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet32 (PcdFspsUpdDataAddress); }
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> new file mode 100644
> index 0000000000..a0d6adb281
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet64 (PcdFspsUpdDataAddress64); }
> diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> index a3b9363779..8c98dbd55d 100644
> --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> @@ -121,3 +121,5 @@
>    #
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UIN
> T32|0x50000000
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UINT
> 32|0x50000001
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|U
> IN
> + T64|0x50000002
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UI
> N
> + T64|0x50000003
> --
> 2.30.2.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
  2021-09-27 10:15     ` Ashraf Ali S
@ 2021-09-27 10:36       ` Zeng, Star
       [not found]       ` <16A8A77591607F71.12372@groups.io>
  1 sibling, 0 replies; 6+ messages in thread
From: Zeng, Star @ 2021-09-27 10:36 UTC (permalink / raw)
  To: S, Ashraf Ali, Chiu, Chasel, devel@edk2.groups.io
  Cc: Desimone, Nathaniel L, Kuo, Ted, Duggapu, Chinni B,
	Chaganty, Rangasai V, Solanki, Digant H, V, Sangeetha, Ni, Ray,
	Zeng, Star

If someone sets the PCD PcdFspmUpdDataAddress64 accidentally, it is better to be caught by some way but ignored.

BTW, if the function GetFspmUpdDataAddress() is a internal function in the module, no EFIAPI is needed in the declaration.

And, @Chiu, Chasel, is PcdFspmUpdDataAddress supposed to be always configured to be Dynamic type by platform?

If yes, maybe we can have two help functions in FspWrapperPlatformLib like below and remove "PcdsFixedAtBuild, PcdsPatchableInModule" in dec. Platform can use SetFspmUpdDataAddress() without need to be aware of the PCDs.

UINTN
EFIAPI
GetFspmUpdDataAddress (
  VOID
  )
{
  if (PcdGet64 (PcdFspmUpdDataAddress64) != 0) {
    return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64);
  } else {
    return (UINTN) PcdGet32 (PcdFspmUpdDataAddress);
  }
}

VOID
EFIAPI
SetFspmUpdDataAddress (
  IN UINTN Address
  )
{
  if (Address <= MAX_UINT32) {
    PcdSet32S (PcdFspmUpdDataAddress, (UINT32) Address);
  } else {
    PcdSet64S (PcdFspmUpdDataAddress64, (UINT64) Address);
  }
}


-----Original Message-----
From: S, Ashraf Ali <ashraf.ali.s@intel.com> 
Sent: 2021年9月27日 18:16
To: Zeng, Star <star.zeng@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type

Hi., @Zeng, Star

Creating a single function is doable, but the problem with that is If someone set the PCD  PcdFspmUpdDataAddress64 accidentally which should be applicable only in X64. Since the PCD has some junk value, it will return the false data in IA32 case. Which will break everything with respect to FSP-S/M.

To Avoid Such cases separating the IA32 vs X64 is more feasible. And easily differentiating with IA32 and X64. 

Regards,
Ashraf Ali S
Intel Technology India Pvt. Ltd. 

-----Original Message-----
From: Zeng, Star <star.zeng@intel.com>
Sent: Monday, September 27, 2021 2:15 PM
To: Chiu, Chasel <chasel.chiu@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type

Curious: It does not work to have one function implementation like below?

UINTN
EFIAPI
GetFspmUpdDataAddress (
  VOID
  )
{
  if (PcdGet64 (PcdFspmUpdDataAddress64) != 0) {
    return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64);
  } else {
    return (UINTN) PcdGet32 (PcdFspmUpdDataAddress);
  }
}

Thanks,
Star

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@intel.com>
Sent: Monday, September 27, 2021 9:10 AM
To: S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type


Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Friday, September 24, 2021 7:43 PM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chiu, Chasel 
> <chasel.chiu@intel.com>; Desimone, Nathaniel L 
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; 
> Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B 
> <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V 
> <rangasai.v.chaganty@intel.com>; Solanki, Digant H 
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; 
> Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address 
> based on Build Type
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642
> when the module is not building in IA32 mode which will lead to building error.
> when a module built-in X64 function pointer will be the size of 64bit 
> width which cannot be fit in 32bit address which will lead to error. 
> to overcome this issue introducing the 2 new PCD's for the 64bit modules can consume it.
> Creating the API's to support different architecture
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Kuo Ted <ted.kuo@intel.com>
> Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Digant H Solanki <digant.h.solanki@intel.com>
> Cc: Sangeetha V <sangeetha.v@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
> ---
>  .../FspmWrapperPeim/FspmWrapperPeim.c         | 19 +++++++++++---
>  .../FspmWrapperPeim/FspmWrapperPeim.inf       | 16 ++++++++++--
>  .../FspmWrapperPeim/IA32/FspmHelper.c         | 26 +++++++++++++++++++
>  .../FspmWrapperPeim/X64/FspmHelper.c          | 26 +++++++++++++++++++
>  .../FspsWrapperPeim/FspsWrapperPeim.c         | 17 +++++++++---
>  .../FspsWrapperPeim/FspsWrapperPeim.inf       | 14 +++++++++-
>  .../FspsWrapperPeim/IA32/FspsHelper.c         | 26 +++++++++++++++++++
>  .../FspsWrapperPeim/X64/FspsHelper.c          | 26 +++++++++++++++++++
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  2 ++
>  9 files changed, 162 insertions(+), 10 deletions(-)  create mode 
> 100644 IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
>  create mode 100644
> IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
>  create mode 100644
> IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
>  create mode 100644 
> IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> index 24ab534620..4a15136c39 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -39,6 +39,17 @@
> 
>  extern EFI_GUID gFspHobGuid;
> 
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  );
> +
>  /**
>    Call FspMemoryInit API.
> 
> @@ -59,7 +70,7 @@ PeiFspMemoryInit (
> 
>    DEBUG ((DEBUG_INFO, "PeiFspMemoryInit enter\n"));
> 
> -  FspHobListPtr = NULL;
> +  FspHobListPtr  = NULL;
>    FspmUpdDataPtr = NULL;
> 
>    FspmHeaderPtr = (FSP_INFO_HEADER *) FspFindFspHeader (PcdGet32 
> (PcdFspmBaseAddress)); @@ -68,7 +79,7 @@ PeiFspMemoryInit (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if (PcdGet32 (PcdFspmUpdDataAddress) == 0 && (FspmHeaderPtr-
> >CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
> +  if (GetFspmUpdDataAddress () == 0 && (FspmHeaderPtr->CfgRegionSize 
> + !=
> + 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-M UPD data from Flash
>      //
> @@ -80,7 +91,7 @@ PeiFspMemoryInit (
>      //
>      // External UPD is ready, get the buffer from PCD pointer.
>      //
> -    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32
> (PcdFspmUpdDataAddress);
> +    FspmUpdDataPtr = (FSPM_UPD_COMMON *) GetFspmUpdDataAddress ();
>      ASSERT (FspmUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> index 00166e56a0..5b4ad531e7 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -45,6 +45,7 @@
>    FspWrapperApiLib
>    FspWrapperApiTestLib
>    FspMeasurementLib
> +  PcdLib
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -56,14 +57,25 @@
> 
>  [Pcd]
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress       ## CONSUMES
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ##
> CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## 
> CONSUMES
> 
> +[Pcd.IA32]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ##
> CONSUMES
> +
> +[Pcd.X64]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64  ##
> CONSUMES
> +
>  [Sources]
>    FspmWrapperPeim.c
> 
> +[Sources.IA32]
> +  IA32/FspmHelper.c
> +
> +[Sources.X64]
> +  X64/FspmHelper.c
> +
>  [Guids]
>    gFspHobGuid                           ## PRODUCES ## HOB
>    gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> new file mode 100644
> index 0000000000..cab11173cc
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet32 (PcdFspmUpdDataAddress); }
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> new file mode 100644
> index 0000000000..25b89ff2e1
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet64 (PcdFspmUpdDataAddress64); }
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> index 9d4f279e81..8bd510502f 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -58,6 +58,17 @@ S3EndOfPeiNotify(
>    IN VOID                      *Ppi
>    );
> 
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  );
> +
>  EFI_PEI_NOTIFY_DESCRIPTOR mS3EndOfPeiNotifyDesc = {
>    (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | 
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
>    &gEfiEndOfPeiSignalPpiGuid,
> @@ -283,7 +294,7 @@ PeiMemoryDiscoveredNotify (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if (PcdGet32 (PcdFspsUpdDataAddress) == 0 && (FspsHeaderPtr-
> >CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
> +  if (GetFspsUpdDataAddress () == 0 && (FspsHeaderPtr->CfgRegionSize 
> + !=
> + 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-S UPD data from Flash
>      //
> @@ -292,7 +303,7 @@ PeiMemoryDiscoveredNotify (
>      SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase + 
> (UINTN)FspsHeaderPtr->CfgRegionOffset);
>      CopyMem (FspsUpdDataPtr, SourceData, (UINTN)FspsHeaderPtr-
> >CfgRegionSize);
>    } else {
> -    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32
> (PcdFspsUpdDataAddress);
> +    FspsUpdDataPtr = (FSPS_UPD_COMMON *) GetFspsUpdDataAddress ();
>      ASSERT (FspsUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> index aeeca58d6d..e988ebab21 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> @@ -45,6 +45,7 @@
>    FspWrapperApiLib
>    FspWrapperApiTestLib
>    FspMeasurementLib
> +  PcdLib
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -65,10 +66,15 @@
> 
>  [Pcd]
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress       ## CONSUMES
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## 
> CONSUMES
> 
> +[Pcd.IA32]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
> +
> +[Pcd.X64]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64  ##
> CONSUMES
> +
>  [Guids]
>    gFspHobGuid                           ## CONSUMES ## HOB
>    gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
> @@ -76,5 +82,11 @@
>  [Sources]
>    FspsWrapperPeim.c
> 
> +[Sources.IA32]
> +  IA32/FspsHelper.c
> +
> +[Sources.X64]
> +  X64/FspsHelper.c
> +
>  [Depex]
>    gEfiPeiMemoryDiscoveredPpiGuid
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> new file mode 100644
> index 0000000000..c4ae292ffb
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet32 (PcdFspsUpdDataAddress); }
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> new file mode 100644
> index 0000000000..a0d6adb281
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet64 (PcdFspsUpdDataAddress64); }
> diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> index a3b9363779..8c98dbd55d 100644
> --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> @@ -121,3 +121,5 @@
>    #
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UIN
> T32|0x50000000
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UINT
> 32|0x50000001
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|U
> IN
> + T64|0x50000002
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UI
> N
> + T64|0x50000003
> --
> 2.30.2.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
       [not found]       ` <16A8A77591607F71.12372@groups.io>
@ 2021-09-27 10:37         ` Zeng, Star
  0 siblings, 0 replies; 6+ messages in thread
From: Zeng, Star @ 2021-09-27 10:37 UTC (permalink / raw)
  To: devel@edk2.groups.io, Zeng, Star, S, Ashraf Ali, Chiu, Chasel
  Cc: Desimone, Nathaniel L, Kuo, Ted, Duggapu, Chinni B,
	Chaganty, Rangasai V, Solanki, Digant H, V, Sangeetha, Ni, Ray,
	Zeng, Star

Typo "ignored" to "not ignored".

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zeng, Star
Sent: 2021年9月27日 18:36
To: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2-devel] [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type

If someone sets the PCD PcdFspmUpdDataAddress64 accidentally, it is better to be caught by some way but ignored.

BTW, if the function GetFspmUpdDataAddress() is a internal function in the module, no EFIAPI is needed in the declaration.

And, @Chiu, Chasel, is PcdFspmUpdDataAddress supposed to be always configured to be Dynamic type by platform?

If yes, maybe we can have two help functions in FspWrapperPlatformLib like below and remove "PcdsFixedAtBuild, PcdsPatchableInModule" in dec. Platform can use SetFspmUpdDataAddress() without need to be aware of the PCDs.

UINTN
EFIAPI
GetFspmUpdDataAddress (
  VOID
  )
{
  if (PcdGet64 (PcdFspmUpdDataAddress64) != 0) {
    return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64);
  } else {
    return (UINTN) PcdGet32 (PcdFspmUpdDataAddress);
  }
}

VOID
EFIAPI
SetFspmUpdDataAddress (
  IN UINTN Address
  )
{
  if (Address <= MAX_UINT32) {
    PcdSet32S (PcdFspmUpdDataAddress, (UINT32) Address);
  } else {
    PcdSet64S (PcdFspmUpdDataAddress64, (UINT64) Address);
  }
}


-----Original Message-----
From: S, Ashraf Ali <ashraf.ali.s@intel.com>
Sent: 2021年9月27日 18:16
To: Zeng, Star <star.zeng@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type

Hi., @Zeng, Star

Creating a single function is doable, but the problem with that is If someone set the PCD  PcdFspmUpdDataAddress64 accidentally which should be applicable only in X64. Since the PCD has some junk value, it will return the false data in IA32 case. Which will break everything with respect to FSP-S/M.

To Avoid Such cases separating the IA32 vs X64 is more feasible. And easily differentiating with IA32 and X64. 

Regards,
Ashraf Ali S
Intel Technology India Pvt. Ltd. 

-----Original Message-----
From: Zeng, Star <star.zeng@intel.com>
Sent: Monday, September 27, 2021 2:15 PM
To: Chiu, Chasel <chasel.chiu@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type

Curious: It does not work to have one function implementation like below?

UINTN
EFIAPI
GetFspmUpdDataAddress (
  VOID
  )
{
  if (PcdGet64 (PcdFspmUpdDataAddress64) != 0) {
    return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64);
  } else {
    return (UINTN) PcdGet32 (PcdFspmUpdDataAddress);
  }
}

Thanks,
Star

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@intel.com>
Sent: Monday, September 27, 2021 9:10 AM
To: S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type


Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Friday, September 24, 2021 7:43 PM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chiu, Chasel 
> <chasel.chiu@intel.com>; Desimone, Nathaniel L 
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; 
> Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B 
> <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V 
> <rangasai.v.chaganty@intel.com>; Solanki, Digant H 
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; 
> Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address 
> based on Build Type
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642
> when the module is not building in IA32 mode which will lead to building error.
> when a module built-in X64 function pointer will be the size of 64bit 
> width which cannot be fit in 32bit address which will lead to error.
> to overcome this issue introducing the 2 new PCD's for the 64bit modules can consume it.
> Creating the API's to support different architecture
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Kuo Ted <ted.kuo@intel.com>
> Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Digant H Solanki <digant.h.solanki@intel.com>
> Cc: Sangeetha V <sangeetha.v@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
> ---
>  .../FspmWrapperPeim/FspmWrapperPeim.c         | 19 +++++++++++---
>  .../FspmWrapperPeim/FspmWrapperPeim.inf       | 16 ++++++++++--
>  .../FspmWrapperPeim/IA32/FspmHelper.c         | 26 +++++++++++++++++++
>  .../FspmWrapperPeim/X64/FspmHelper.c          | 26 +++++++++++++++++++
>  .../FspsWrapperPeim/FspsWrapperPeim.c         | 17 +++++++++---
>  .../FspsWrapperPeim/FspsWrapperPeim.inf       | 14 +++++++++-
>  .../FspsWrapperPeim/IA32/FspsHelper.c         | 26 +++++++++++++++++++
>  .../FspsWrapperPeim/X64/FspsHelper.c          | 26 +++++++++++++++++++
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  2 ++
>  9 files changed, 162 insertions(+), 10 deletions(-)  create mode
> 100644 IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
>  create mode 100644
> IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
>  create mode 100644
> IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
>  create mode 100644
> IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> index 24ab534620..4a15136c39 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -39,6 +39,17 @@
> 
>  extern EFI_GUID gFspHobGuid;
> 
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  );
> +
>  /**
>    Call FspMemoryInit API.
> 
> @@ -59,7 +70,7 @@ PeiFspMemoryInit (
> 
>    DEBUG ((DEBUG_INFO, "PeiFspMemoryInit enter\n"));
> 
> -  FspHobListPtr = NULL;
> +  FspHobListPtr  = NULL;
>    FspmUpdDataPtr = NULL;
> 
>    FspmHeaderPtr = (FSP_INFO_HEADER *) FspFindFspHeader (PcdGet32 
> (PcdFspmBaseAddress)); @@ -68,7 +79,7 @@ PeiFspMemoryInit (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if (PcdGet32 (PcdFspmUpdDataAddress) == 0 && (FspmHeaderPtr-
> >CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
> +  if (GetFspmUpdDataAddress () == 0 && (FspmHeaderPtr->CfgRegionSize 
> + !=
> + 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-M UPD data from Flash
>      //
> @@ -80,7 +91,7 @@ PeiFspMemoryInit (
>      //
>      // External UPD is ready, get the buffer from PCD pointer.
>      //
> -    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32
> (PcdFspmUpdDataAddress);
> +    FspmUpdDataPtr = (FSPM_UPD_COMMON *) GetFspmUpdDataAddress ();
>      ASSERT (FspmUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> index 00166e56a0..5b4ad531e7 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -45,6 +45,7 @@
>    FspWrapperApiLib
>    FspWrapperApiTestLib
>    FspMeasurementLib
> +  PcdLib
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -56,14 +57,25 @@
> 
>  [Pcd]
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress       ## CONSUMES
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ##
> CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## 
> CONSUMES
> 
> +[Pcd.IA32]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ##
> CONSUMES
> +
> +[Pcd.X64]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64  ##
> CONSUMES
> +
>  [Sources]
>    FspmWrapperPeim.c
> 
> +[Sources.IA32]
> +  IA32/FspmHelper.c
> +
> +[Sources.X64]
> +  X64/FspmHelper.c
> +
>  [Guids]
>    gFspHobGuid                           ## PRODUCES ## HOB
>    gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> new file mode 100644
> index 0000000000..cab11173cc
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/IA32/FspmHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet32 (PcdFspmUpdDataAddress); }
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> new file mode 100644
> index 0000000000..25b89ff2e1
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/X64/FspmHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fspm Upd Data Address from the PCD
> +
> +  @return FSPM UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet64 (PcdFspmUpdDataAddress64); }
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> index 9d4f279e81..8bd510502f 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -58,6 +58,17 @@ S3EndOfPeiNotify(
>    IN VOID                      *Ppi
>    );
> 
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  );
> +
>  EFI_PEI_NOTIFY_DESCRIPTOR mS3EndOfPeiNotifyDesc = {
>    (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | 
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
>    &gEfiEndOfPeiSignalPpiGuid,
> @@ -283,7 +294,7 @@ PeiMemoryDiscoveredNotify (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if (PcdGet32 (PcdFspsUpdDataAddress) == 0 && (FspsHeaderPtr-
> >CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
> +  if (GetFspsUpdDataAddress () == 0 && (FspsHeaderPtr->CfgRegionSize 
> + !=
> + 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-S UPD data from Flash
>      //
> @@ -292,7 +303,7 @@ PeiMemoryDiscoveredNotify (
>      SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase + 
> (UINTN)FspsHeaderPtr->CfgRegionOffset);
>      CopyMem (FspsUpdDataPtr, SourceData, (UINTN)FspsHeaderPtr-
> >CfgRegionSize);
>    } else {
> -    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32
> (PcdFspsUpdDataAddress);
> +    FspsUpdDataPtr = (FSPS_UPD_COMMON *) GetFspsUpdDataAddress ();
>      ASSERT (FspsUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> index aeeca58d6d..e988ebab21 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> @@ -45,6 +45,7 @@
>    FspWrapperApiLib
>    FspWrapperApiTestLib
>    FspMeasurementLib
> +  PcdLib
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -65,10 +66,15 @@
> 
>  [Pcd]
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress       ## CONSUMES
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## 
> CONSUMES
> 
> +[Pcd.IA32]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
> +
> +[Pcd.X64]
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64  ##
> CONSUMES
> +
>  [Guids]
>    gFspHobGuid                           ## CONSUMES ## HOB
>    gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
> @@ -76,5 +82,11 @@
>  [Sources]
>    FspsWrapperPeim.c
> 
> +[Sources.IA32]
> +  IA32/FspsHelper.c
> +
> +[Sources.X64]
> +  X64/FspsHelper.c
> +
>  [Depex]
>    gEfiPeiMemoryDiscoveredPpiGuid
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> new file mode 100644
> index 0000000000..c4ae292ffb
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/IA32/FspsHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet32 (PcdFspsUpdDataAddress); }
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> new file mode 100644
> index 0000000000..a0d6adb281
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/X64/FspsHelper.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  Sample to provide FSP wrapper related function.
> +
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#include <Library/PcdLib.h>
> +#include <Library/FspWrapperPlatformLib.h> #include 
> +<Uefi/UefiBaseType.h>
> +
> +/**
> +  Get the Fsps Upd Data Address from the PCD
> +
> +  @return FSPS UPD Data Address
> +**/
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  return PcdGet64 (PcdFspsUpdDataAddress64); }
> diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> index a3b9363779..8c98dbd55d 100644
> --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> @@ -121,3 +121,5 @@
>    #
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UIN
> T32|0x50000000
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UINT
> 32|0x50000001
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|U
> IN
> + T64|0x50000002
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UI
> N
> + T64|0x50000003
> --
> 2.30.2.windows.1







^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-27 10:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-24 11:42 [PATCH v7] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type Ashraf Ali S
2021-09-27  1:10 ` Chiu, Chasel
2021-09-27  8:44   ` Zeng, Star
2021-09-27 10:15     ` Ashraf Ali S
2021-09-27 10:36       ` Zeng, Star
     [not found]       ` <16A8A77591607F71.12372@groups.io>
2021-09-27 10:37         ` [edk2-devel] " Zeng, Star

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox