Thanks for the feedback. I will send V3 patch. I also make that correction in closed source.
-----Original Message----- From: Chiu, Chasel <chasel.c...@intel.com> Sent: Monday, March 2, 2020 7:59 PM To: Shindo, Miki <miki.shi...@intel.com>; devel@edk2.groups.io Cc: Chaganty, Rangasai V <rangasai.v.chaga...@intel.com>; Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Agyeman, Prince <prince.agye...@intel.com> Subject: RE: [edk2-platform:PATCH v2] MinPlatformPkg/PeiReportFvLib: Remove redundant Fsp Fv installation Hi Shindo, Please see my comments below inline. With that updated: Reviewed-by: Chasel Chiu <chasel.c...@intel.com> Thanks, Chasel > -----Original Message----- > From: Shindo, Miki <miki.shi...@intel.com> > Sent: Tuesday, March 3, 2020 9:50 AM > To: devel@edk2.groups.io > Cc: Chaganty, Rangasai V <rangasai.v.chaga...@intel.com>; Chiu, Chasel > <chasel.c...@intel.com>; Desimone, Nathaniel L > <nathaniel.l.desim...@intel.com>; Agyeman, Prince > <prince.agye...@intel.com> > Subject: [edk2-platform:PATCH v2] MinPlatformPkg/PeiReportFvLib: > Remove redundant Fsp Fv installation > > REF : https://bugzilla.tianocore.org/show_bug.cgi?id=2542 > > ReportPreMemFv () has redundant calls to install Fsp FVs. > FSP-M, S, U FVs do not need to be installed when Fsp Wrapper Boot Mode > is disabled. > > Signed-off-by: Miki Shindo <miki.shi...@intel.com> > Cc: Sai Chaganty <rangasai.v.chaga...@intel.com> > Cc: Chasel Chiu <chasel.c...@intel.com> > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> > Cc: Prince Agyeman <prince.agye...@intel.com> > --- > > Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/PeiReport > FvLib.c | 38 +++++++------------------------------- > > Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/PeiR > eport > FvLib.inf | 6 ------ > 2 files changed, 7 insertions(+), 37 deletions(-) > > diff --git > a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/Pe > iRep > ortFvLib.c > b/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/Pe > iRep > ortFvLib.c > index 6158fc9412..1fad384cd6 100644 > --- > a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/Pe > iRep > ortFvLib.c > +++ b/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLi > +++ b/ > +++ PeiReportFvLib.c > @@ -20,17 +20,9 @@ ReportPreMemFv ( > VOID > ) > { > - if (!PcdGetBool(PcdFspWrapperBootMode)) { > - DEBUG ((DEBUG_INFO, "Install FlashFvFspM - 0x%x, 0x%x\n", PcdGet32 > (PcdFlashFvFspMBase), PcdGet32 (PcdFlashFvFspMSize))); > - PeiServicesInstallFvInfo2Ppi ( > - &(((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PcdGet32 > (PcdFlashFvFspMBase))->FileSystemGuid), > - (VOID *) (UINTN) PcdGet32 (PcdFlashFvFspMBase), > - PcdGet32 (PcdFlashFvFspMSize), > - NULL, > - NULL, > - 0 > - ); > - } > + /// > + /// Note : FSP FVs except FSP-T FV are installed in > + IntelFsp2Wrapper Pkg or > FspPkg in Dispatch mode. > + /// In Dispatch mode FSP FVs will be installed by IntelFsp2WrapperPkg, not FspPkg, please correct comments for PostMem phase too. > if (PcdGetBool(PcdFspWrapperBootMode)) { > DEBUG ((DEBUG_INFO, "Install FlashFvFspT - 0x%x, 0x%x\n", > PcdGet32 (PcdFlashFvFspTBase), PcdGet32 (PcdFlashFvFspTSize))); > PeiServicesInstallFvInfo2Ppi ( > @@ -80,6 +72,10 @@ ReportPostMemFv ( > Status = PeiServicesGetBootMode (&BootMode); > ASSERT_EFI_ERROR (Status); > > + /// > + /// Note : FSP FVs except FSP-T FV are installed in > + IntelFsp2Wrapper Pkg or > FspPkg in Dispatch mode. > + /// > + > /// > /// Build HOB for DXE > /// > @@ -97,26 +93,6 @@ ReportPostMemFv ( > NULL, > 0 > ); > - if (!PcdGetBool(PcdFspWrapperBootMode)) { > - DEBUG ((DEBUG_INFO, "Install FlashFvFspS - 0x%x, 0x%x\n", > PcdGet32 (PcdFlashFvFspSBase), PcdGet32 (PcdFlashFvFspSSize))); > - PeiServicesInstallFvInfo2Ppi ( > - &(((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PcdGet32 > (PcdFlashFvFspSBase))->FileSystemGuid), > - (VOID *) (UINTN) PcdGet32 (PcdFlashFvFspSBase), > - PcdGet32 (PcdFlashFvFspSSize), > - NULL, > - NULL, > - 0 > - ); > - DEBUG ((DEBUG_INFO, "Install FlashFvFspU - 0x%x, 0x%x\n", > PcdGet32 (PcdFlashFvFspUBase), PcdGet32 (PcdFlashFvFspUSize))); > - PeiServicesInstallFvInfo2Ppi ( > - &(((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PcdGet32 > (PcdFlashFvFspUBase))->FileSystemGuid), > - (VOID *) (UINTN) PcdGet32 (PcdFlashFvFspUBase), > - PcdGet32 (PcdFlashFvFspUSize), > - NULL, > - NULL, > - 0 > - ); > - } > DEBUG ((DEBUG_INFO, "Install FlashFvUefiBoot - 0x%x, 0x%x\n", > PcdGet32 (PcdFlashFvUefiBootBase), PcdGet32 (PcdFlashFvUefiBootSize))); > PeiServicesInstallFvInfo2Ppi ( > &(((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PcdGet32 > (PcdFlashFvUefiBootBase))->FileSystemGuid), > diff --git > a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/Pe > iRep > ortFvLib.inf > b/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/Pe > iRep > ortFvLib.inf > index 79cd5ee1f7..4258d0f2e7 100644 > --- > a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/Pe > iRep > ortFvLib.inf > +++ b/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLi > +++ b/ > +++ PeiReportFvLib.inf > @@ -34,14 +34,8 @@ > gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperBootMode > ## CONSUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress > ## CONSUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize > ## CONSUMES > - gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspMBase > ## CONSUMES > - gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspMSize > ## CONSUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspTBase > ## CONSUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspTSize > ## CONSUMES > - gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspSBase > ## CONSUMES > - gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspSSize > ## CONSUMES > - gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspUBase > ## CONSUMES > - gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspUSize > ## CONSUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPostMemoryBase > ## CONSUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPostMemorySize > ## CONSUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashFvUefiBootBase > ## CONSUMES > -- > 2.16.2.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55286): https://edk2.groups.io/g/devel/message/55286 Mute This Topic: https://groups.io/mt/71690962/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
