Please see my comments below inline.
Thanks, Chasel > -----Original Message----- > From: S, Ashraf Ali <ashraf.al...@intel.com> > Sent: Wednesday, September 22, 2021 10:23 PM > To: devel@edk2.groups.io > Cc: S, Ashraf Ali <ashraf.al...@intel.com>; Chiu, Chasel > <chasel.c...@intel.com>; > Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Zeng, Star > <star.z...@intel.com>; Kuo, Ted <ted....@intel.com>; Duggapu, Chinni B > <chinni.b.dugg...@intel.com>; Chaganty, Rangasai V > <rangasai.v.chaga...@intel.com>; Solanki, Digant H > <digant.h.sola...@intel.com>; V, Sangeetha <sangeeth...@intel.com>; Ni, Ray > <ray...@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.c...@intel.com> > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> > Cc: Star Zeng <star.z...@intel.com> > Cc: Kuo Ted <ted....@intel.com> > Cc: Duggapu Chinni B <chinni.b.dugg...@intel.com> > Cc: Rangasai V Chaganty <rangasai.v.chaga...@intel.com> > Cc: Digant H Solanki <digant.h.sola...@intel.com> > Cc: Sangeetha V <sangeeth...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Signed-off-by: Ashraf Ali S <ashraf.al...@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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#81035): https://edk2.groups.io/g/devel/message/81035 Mute This Topic: https://groups.io/mt/85791026/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-