How about adding the minor comments like below? 😊 # # Non-0 means PcdFspmUpdDataAddress will be ignored, otherwise PcdFspmUpdDataAddress will be used. # gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|UINT64|0x50000002 # # Non-0 means PcdFspsUpdDataAddress will be ignored, otherwise PcdFspsUpdDataAddress will be used. # gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UINT64|0x50000003
Anyway, Reviewed-by: Star Zeng <star.z...@intel.com> Thanks, Star -----Original Message----- From: Chiu, Chasel <chasel.c...@intel.com> Sent: 2021年12月17日 11:48 To: S, Ashraf Ali <ashraf.al...@intel.com>; devel@edk2.groups.io Cc: 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: RE: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type Thanks Ashraf! Reviewed-by: Chasel Chiu <chasel.c...@intel.com> > -----Original Message----- > From: S, Ashraf Ali <ashraf.al...@intel.com> > Sent: Thursday, December 16, 2021 4:10 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 v8] 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. based on the which pcd platform set, use that. > > 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 | 25 ++++++++++++++++--- > .../FspmWrapperPeim/FspmWrapperPeim.inf | 3 ++- > .../FspsWrapperPeim/FspsWrapperPeim.c | 25 ++++++++++++++++--- > .../FspsWrapperPeim/FspsWrapperPeim.inf | 3 ++- > IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec | 2 ++ > 5 files changed, 50 insertions(+), 8 deletions(-) > > diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c > b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c > index 287e7f9159..49fbb27eca 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 > > **/ > @@ -38,6 +38,25 @@ > > extern EFI_GUID gFspHobGuid; > > +/** > + Get the FSP M UPD Data address > + > + @return FSP-M UPD Data Address > +**/ > + > +UINTN > +EFIAPI > +GetFspmUpdDataAddress ( > + VOID > + ) > +{ > + if (PcdGet64 (PcdFspmUpdDataAddress64) != 0) { > + return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64); > + } else { > + return (UINTN) PcdGet32 (PcdFspmUpdDataAddress); > + } > +} > + > /** > Call FspMemoryInit API. > > @@ -67,7 +86,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 > // > @@ -79,7 +98,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..5d0e021401 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 # @@ -60,6 +60,7 @@ > gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection ## CONSUMES > gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress ## CONSUMES > gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig ## > CONSUMES > + gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64 ## > CONSUMES > > [Sources] > FspmWrapperPeim.c > diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c > b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c > index f7459a90b5..ddee9cd029 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 > > **/ > @@ -181,6 +181,25 @@ FspSiliconInitDoneGetFspHobList ( > } > } > > +/** > + Get the FSP S UPD Data address > + > + @return FSP-S UPD Data Address > +**/ > + > +UINTN > +EFIAPI > +GetFspsUpdDataAddress ( > + VOID > + ) > +{ > + if (PcdGet64 (PcdFspsUpdDataAddress64) != 0) { > + return (UINTN) PcdGet64 (PcdFspsUpdDataAddress64); > + } else { > + return (UINTN) PcdGet32 (PcdFspsUpdDataAddress); > + } > +} > + > /** > This function is for FSP dispatch mode to perform post FSP-S process. > > @@ -283,7 +302,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 +311,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..da0049a654 100644 > --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf > +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.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 # @@ -68,6 +68,7 @@ > gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress ## CONSUMES > gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection ## CONSUMES > gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig ## > CONSUMES > + gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64 ## > CONSUMES > > [Guids] > gFspHobGuid ## CONSUMES ## HOB > diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec > b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec > index b8dac1b574..a5a8b8a19e 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 (#85042): https://edk2.groups.io/g/devel/message/85042 Mute This Topic: https://groups.io/mt/87763051/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
