Please see my comments below inline.
Thanks, Chasel
toggle quoted messageShow quoted text
-----Original Message----- From: S, Ashraf Ali <ashraf.ali.s@intel.com> Sent: Wednesday, September 22, 2021 10:23 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 v5] 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 | 8 +++--- .../FspmWrapperPeim/FspmWrapperPeim.inf | 16 ++++++++++-- .../FspmWrapperPeim/IA32/FspmHelper.c | 26 +++++++++++++++++++ .../FspmWrapperPeim/X64/FspmHelper.c | 26 +++++++++++++++++++ .../FspsWrapperPeim/FspsWrapperPeim.c | 6 ++--- .../FspsWrapperPeim/FspsWrapperPeim.inf | 14 +++++++++- .../FspsWrapperPeim/IA32/FspsHelper.c | 26 +++++++++++++++++++ .../FspsWrapperPeim/X64/FspsHelper.c | 26 +++++++++++++++++++ .../Include/Library/FspWrapperPlatformLib.h | 24 ++++++++++++++++- IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec | 2 ++ 10 files changed, 163 insertions(+), 11 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..6f2f0018ac 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
**/ @@ -59,7 +59,7 @@ PeiFspMemoryInit (
DEBUG ((DEBUG_INFO, "PeiFspMemoryInit enter\n"));
- FspHobListPtr = NULL; + FspHobListPtr = NULL; FspmUpdDataPtr = NULL;
FspmHeaderPtr = (FSP_INFO_HEADER *) FspFindFspHeader (PcdGet32 (PcdFspmBaseAddress)); @@ -68,7 +68,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 +80,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..5875cc0fdc 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
**/ @@ -283,7 +283,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 +292,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/Include/Library/FspWrapperPlatformLib.h b/IntelFsp2WrapperPkg/Include/Library/FspWrapperPlatformLib.h index 2aa14c92fd..4a06505531 100644 --- a/IntelFsp2WrapperPkg/Include/Library/FspWrapperPlatformLib.h +++ b/IntelFsp2WrapperPkg/Include/Library/FspWrapperPlatformLib.h @@ -1,7 +1,7 @@ /** @file Provide FSP wrapper platform related function.
- Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR> + Copyright (c) 2014 - 2021, Intel Corporation. All rights + reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent
**/ @@ -77,4 +77,26 @@ CallFspWrapperResetSystem ( IN UINT32 FspStatusResetType );
+/** + Get the Fspm Upd Data Address from the PCD + + @return FSPM UPD Data Address +**/ +UINTN +EFIAPI +GetFspmUpdDataAddress ( + VOID + ); + +/** + Get the Fsps Upd Data Address from the PCD + + @return FSPS UPD Data Address +**/ +UINTN +EFIAPI +GetFspsUpdDataAddress ( + VOID + ); + Include/Library/FspWrapperPlatformLib.h should only include those functions provided by FspWrapperPlatformLib, however the 2 new adding functions were local/private functions of FspmWrapperPeim/FspsWrapperPeim modules. I would recommend that we move definition of local functions to local modules FspmWrapperPeim and FspsWrapperPeim. #endif 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
|