From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mx.groups.io with SMTP id smtpd.web12.549.1583189966800331654 for ; Mon, 02 Mar 2020 14:59:26 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: nathaniel.l.desimone@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Mar 2020 14:59:24 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,508,1574150400"; d="scan'208";a="233356817" Received: from orsmsx109.amr.corp.intel.com ([10.22.240.7]) by orsmga008.jf.intel.com with ESMTP; 02 Mar 2020 14:59:24 -0800 Received: from orsmsx156.amr.corp.intel.com (10.22.240.22) by ORSMSX109.amr.corp.intel.com (10.22.240.7) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 2 Mar 2020 14:59:24 -0800 Received: from orsmsx114.amr.corp.intel.com ([169.254.8.140]) by ORSMSX156.amr.corp.intel.com ([169.254.8.240]) with mapi id 14.03.0439.000; Mon, 2 Mar 2020 14:59:23 -0800 From: "Nate DeSimone" To: "Shindo, Miki" CC: "devel@edk2.groups.io" , "Chaganty, Rangasai V" , "Chiu, Chasel" , "Agyeman, Prince" Subject: Re: [edk2-platforms:PATCH] MinPlatformPkg/PeiReportFvLib: Remove redundant Fsp Fv installation Thread-Topic: [edk2-platforms:PATCH] MinPlatformPkg/PeiReportFvLib: Remove redundant Fsp Fv installation Thread-Index: AQHV8G5T4F3G+orp40al9aFAnXUOqag2caaA Date: Mon, 2 Mar 2020 22:59:23 +0000 Message-ID: <20200302225917.GA3977@nate-virtualbox> References: <20200302084042.27048-1-miki.shindo@intel.com> In-Reply-To: <20200302084042.27048-1-miki.shindo@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.7.159.63] MIME-Version: 1.0 Return-Path: nathaniel.l.desimone@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-ID: <63585F0E2E94F04380FAECE298EBB7CB@intel.com> Content-Transfer-Encoding: quoted-printable Hi Shindo-san, Please see my feedback inline. Thanks, Nate On Mon, Mar 02, 2020 at 08:40:42AM +0000, Shindo, Miki wrote: > REF : https://bugzilla.tianocore.org/show_bug.cgi?id=3D2542 >=20 > 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. >=20 > Signed-off-by: Miki Shindo > Cc: Sai Chaganty > Cc: Chasel Chiu > Cc: Nate DeSimone > Cc: Prince Agyeman > --- > Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/PeiRep= ortFvLib.c | 38 +++++++------------------------------- > Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/PeiRep= ortFvLib.inf | 6 ------ > 2 files changed, 7 insertions(+), 37 deletions(-) >=20 > diff --git a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReport= FvLib/PeiReportFvLib.c b/Platform/Intel/MinPlatformPkg/PlatformInit/Library= /PeiReportFvLib/PeiReportFvLib.c > index 6158fc9412..95e3c88275 100644 > --- a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/P= eiReportFvLib.c > +++ b/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/P= eiReportFvLib.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 (PcdFlashFvFspM= Base))->FileSystemGuid), > - (VOID *) (UINTN) PcdGet32 (PcdFlashFvFspMBase), > - PcdGet32 (PcdFlashFvFspMSize), > - NULL, > - NULL, > - 0 > - ); > - } > + /* > + Note : FSP FVs are installed in IntelFsp2Wrapper Pkg or FspPkg in Di= spatch mode. > + */ > if (PcdGetBool(PcdFspWrapperBootMode)) { > DEBUG ((DEBUG_INFO, "Install FlashFvFspT - 0x%x, 0x%x\n", PcdGet32 (= PcdFlashFvFspTBase), PcdGet32 (PcdFlashFvFspTSize))); > PeiServicesInstallFvInfo2Ppi ( > @@ -80,6 +72,10 @@ ReportPostMemFv ( > Status =3D PeiServicesGetBootMode (&BootMode); > ASSERT_EFI_ERROR (Status); > =20 Please use the "///" style comments to maintain consistency with the other code in this file. > + /* > + Note : FSP FVs are installed in IntelFsp2Wrapper Pkg or FspPkg in Di= spatch 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 (PcdFlashFvFs= pSBase))->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 (PcdFlashFvFs= pUBase))->FileSystemGuid), > - (VOID *) (UINTN) PcdGet32 (PcdFlashFvFspUBase), > - PcdGet32 (PcdFlashFvFspUSize), > - NULL, > - NULL, > - 0 > - ); > - } > DEBUG ((DEBUG_INFO, "Install FlashFvUefiBoot - 0x%x, 0x%x\n", PcdGet= 32 (PcdFlashFvUefiBootBase), PcdGet32 (PcdFlashFvUefiBootSize))); > PeiServicesInstallFvInfo2Ppi ( > &(((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PcdGet32 (PcdFlashFvUefi= BootBase))->FileSystemGuid), > diff --git a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReport= FvLib/PeiReportFvLib.inf b/Platform/Intel/MinPlatformPkg/PlatformInit/Libra= ry/PeiReportFvLib/PeiReportFvLib.inf > index 79cd5ee1f7..4258d0f2e7 100644 > --- a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/P= eiReportFvLib.inf > +++ b/Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/P= eiReportFvLib.inf > @@ -34,14 +34,8 @@ > gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperBootMode ## CON= SUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress ## CON= SUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize ## CON= SUMES > - gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspMBase ## CON= SUMES > - gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspMSize ## CON= SUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspTBase ## CON= SUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspTSize ## CON= SUMES > - gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspSBase ## CON= SUMES > - gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspSSize ## CON= SUMES > - gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspUBase ## CON= SUMES > - gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspUSize ## CON= SUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPostMemoryBase ## CON= SUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPostMemorySize ## CON= SUMES > gMinPlatformPkgTokenSpaceGuid.PcdFlashFvUefiBootBase ## CON= SUMES > --=20 > 2.16.2.windows.1 > =