From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mx.groups.io with SMTP id smtpd.web09.2654.1574399929848574076 for ; Thu, 21 Nov 2019 21:18:50 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.31, mailfrom: chasel.chiu@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Nov 2019 21:18:49 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,228,1571727600"; d="scan'208";a="205300272" Received: from kmsmsx153.gar.corp.intel.com ([172.21.73.88]) by fmsmga008.fm.intel.com with ESMTP; 21 Nov 2019 21:18:48 -0800 Received: from pgsmsx111.gar.corp.intel.com ([169.254.2.24]) by KMSMSX153.gar.corp.intel.com ([169.254.5.18]) with mapi id 14.03.0439.000; Fri, 22 Nov 2019 13:18:47 +0800 From: "Chiu, Chasel" To: "Desimone, Nathaniel L" , "devel@edk2.groups.io" CC: "Kubacki, Michael A" , "Gao, Liming" Subject: Re: [edk2-platforms] [PATCH V2 14/14] MinPlatformPkg: Remove BoardInitLib dependency from PlatformSecLib Thread-Topic: [edk2-platforms] [PATCH V2 14/14] MinPlatformPkg: Remove BoardInitLib dependency from PlatformSecLib Thread-Index: AQHVoEoAXtwK7oNuQkqvaQo417tURqeWqALQ Date: Fri, 22 Nov 2019 05:18:46 +0000 Message-ID: <3C3EFB470A303B4AB093197B6777CCEC505AB912@PGSMSX111.gar.corp.intel.com> References: <20191121085853.2626-1-nathaniel.l.desimone@intel.com> <20191121085853.2626-15-nathaniel.l.desimone@intel.com> In-Reply-To: <20191121085853.2626-15-nathaniel.l.desimone@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiODU2MDJkY2QtOTgxYi00NjRmLTljYjAtYTJkYTliMzA0Nzg0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiYUxqdFFZaklDdFd0SllEaWZJSWV5WXd4Yjg0djdxWmZId0NSYUNxaFNmTld0cERqMUJVbHBcL3J5NVFKVFlvT3UifQ== x-ctpclassification: CTP_NT x-originating-ip: [172.30.20.205] MIME-Version: 1.0 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Chasel Chiu > -----Original Message----- > From: Desimone, Nathaniel L > Sent: Thursday, November 21, 2019 4:59 PM > To: devel@edk2.groups.io > Cc: Kubacki, Michael A ; Chiu, Chasel > ; Gao, Liming > Subject: [edk2-platforms] [PATCH V2 14/14] MinPlatformPkg: Remove > BoardInitLib dependency from PlatformSecLib >=20 > SecFspWrapperPlatformSecLib contains the implementation of > SecPlatformDisableTemporaryMemory(), which SecMain in UefiCpuPkg will > call as part of its implementation of EFI_PEI_TEMPORARY_RAM_DONE_PPI. > For platforms that use FSP, the implementation of > SecPlatformDisableTemporaryMemory() can be made generic since the > chipset specifics will be contained in FspTempRamExit(). >=20 > The Minimum Platform Specification provides the BoardPkg two interface > hook points, BoardInitBeforeTempRamExit() and > BoardInitAfterTempRamExit() which must be called during > SecPlatformDisableTemporaryMemory(). Due to > EFI_PEI_TEMPORARY_RAM_DONE_PPI being a special case of a PPI that is > implemented in SEC, these two functions are the only ones in BoardInitLib > that need to be called by SEC. >=20 > Linking BoardInitLib with SEC places many restrictions on the > implementation of that library. The features available to SEC phase code = are > very minimal. Since this code runs during PEI phase, these restrictions a= re > not actually required. >=20 > Instead of directly linking with BoardInitLib, > SecPlatformDisableTemporaryMemory() shall call BoardInitLib indirectly > through a PPI (PLATFORM_INIT_TEMP_RAM_EXIT_PPI.) This PPI is produced > by PlatformInitPreMem, which implements the other BoardInitLib calls > already, so this change should also slightly reduce the size of the final= binary > image since less PE/COFF images will need to link with BoardInitLib. >=20 > Cc: Michael Kubacki > Cc: Chasel Chiu > Cc: Liming Gao > Signed-off-by: Nate DeSimone > --- > .../SecFspWrapperPlatformSecLib.inf | 2 +- > .../SecTempRamDone.c | 36 +++++++-- > .../Include/Ppi/PlatformInitTempRamExitPpi.h | 55 ++++++++++++++ > .../Intel/MinPlatformPkg/MinPlatformPkg.dec | 2 + > .../PlatformInitPei/PlatformInitPreMem.c | 76 ++++++++++++++++++- > .../PlatformInitPei/PlatformInitPreMem.inf | 1 + > 6 files changed, 159 insertions(+), 13 deletions(-) create mode 100644 > Platform/Intel/MinPlatformPkg/Include/Ppi/PlatformInitTempRamExitPpi.h >=20 > diff --git > a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor > mSecLib/SecFspWrapperPlatformSecLib.inf > b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor > mSecLib/SecFspWrapperPlatformSecLib.inf > index 02c720c73d..4f3fa9fa34 100644 > --- > a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor > mSecLib/SecFspWrapperPlatformSecLib.inf > +++ > b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat > +++ formSecLib/SecFspWrapperPlatformSecLib.inf > @@ -69,7 +69,6 @@ > SerialPortLib FspWrapperPlatformLib FspWrapperApiLib- > BoardInitLib SecBoardInitLib TestPointCheckLib > PeiServicesTablePointerLib@@ -80,6 +79,7 @@ > gTopOfTemporaryRamPpiGuid ## PRODUCES > gEfiPeiFirmwareVolumeInfoPpiGuid ## PRODUCES > gFspTempRamExitPpiGuid ## CONSUMES+ > gPlatformInitTempRamExitPpiGuid ## CONSUMES [Pcd] > gUefiCpuPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize > ## CONSUMESdiff --git > a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor > mSecLib/SecTempRamDone.c > b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor > mSecLib/SecTempRamDone.c > index 922e4ec204..b22cf57d6c 100644 > --- > a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor > mSecLib/SecTempRamDone.c > +++ > b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat > +++ formSecLib/SecTempRamDone.c > @@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include #include > +#include > #include #include @@ > -17,7 +18,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include > #include > #include -#include > #include /**@@ -29,14 +29,35 @@ > SecPlatformDisableTemporaryMemory ( > VOID ) {- EFI_STATUS Status;- VOID > *TempRamExitParam;- CONST EFI_PEI_SERVICES **PeiServices;- > FSP_TEMP_RAM_EXIT_PPI *TempRamExitPpi;+ EFI_STATUS > Status;+ VOID *TempRamExitParam;+ > CONST EFI_PEI_SERVICES **PeiServices;+ > FSP_TEMP_RAM_EXIT_PPI *TempRamExitPpi;+ > PLATFORM_INIT_TEMP_RAM_EXIT_PPI *PlatformInitTempRamExitPpi; > DEBUG ((DEBUG_INFO, "SecPlatformDisableTemporaryMemory enter\n"));+ > PeiServices =3D GetPeiServicesTablePointer ();+ ASSERT (PeiServices !=3D= NULL);+ > if (PeiServices =3D=3D NULL) {+ return;+ }+ ASSERT ((*PeiServices) != =3D > NULL);+ if ((*PeiServices) =3D=3D NULL) {+ return;+ }+ Status =3D > (*PeiServices)->LocatePpi (+ PeiServices,+ > &gPlatformInitTempRamExitPpiGuid,+ 0,+ > NULL,+ (VOID **) > &PlatformInitTempRamExitPpi+ );+ > ASSERT_EFI_ERROR (Status);+ if (EFI_ERROR (Status)) {+ return;+ } - > Status =3D BoardInitBeforeTempRamExit ();+ Status =3D > PlatformInitTempRamExitPpi->PlatformInitBeforeTempRamExit (); > ASSERT_EFI_ERROR (Status); if (PcdGet8 (PcdFspModeSelection) =3D=3D 1) > {@@ -51,7 +72,6 @@ SecPlatformDisableTemporaryMemory ( > // // FSP Dispatch mode //- PeiServices =3D > GetPeiServicesTablePointer (); Status =3D (*PeiServices)->LocatePpi > ( PeiServices, > &gFspTempRamExitPpiGuid,@@ -66,7 +86,7 @@ > SecPlatformDisableTemporaryMemory ( > TempRamExitPpi->TempRamExit (NULL); } - Status =3D > BoardInitAfterTempRamExit ();+ Status =3D > PlatformInitTempRamExitPpi->PlatformInitAfterTempRamExit (); > ASSERT_EFI_ERROR (Status); return ;diff --git > a/Platform/Intel/MinPlatformPkg/Include/Ppi/PlatformInitTempRamExitPpi.h > b/Platform/Intel/MinPlatformPkg/Include/Ppi/PlatformInitTempRamExitPpi.h > new file mode 100644 > index 0000000000..590647738c > --- /dev/null > +++ > b/Platform/Intel/MinPlatformPkg/Include/Ppi/PlatformInitTempRamExitP > +++ pi.h > @@ -0,0 +1,55 @@ > +/** @file+ This file defines the PPI for notifying PlatformInitPreMem+ = of > temporary memory being disabled.++Copyright (c) 2019, Intel Corporation. > All rights reserved.
+SPDX-License-Identifier: > BSD-2-Clause-Patent++**/++#ifndef > _PLATFORM_INIT_TEMP_RAM_EXIT_PPI_H_+#define > _PLATFORM_INIT_TEMP_RAM_EXIT_PPI_H_++#include ++//+// > Forward declaration for the > PLATFORM_INIT_TEMP_RAM_EXIT_PPI.+//+typedef struct > _PLATFORM_INIT_TEMP_RAM_EXIT_PPI > PLATFORM_INIT_TEMP_RAM_EXIT_PPI;++/**+ A hook for platform-specific > initialization prior to disabling temporary RAM.++ @retval EFI_SUCCESS > The platform initialization was successful.+ @retval EFI_NOT_READY The > platform has not been detected yet.+**/+typedef+EFI_STATUS+(EFIAPI > *PLATFORM_INIT_BEFORE_TEMP_RAM_EXIT) (+ VOID+ );++/**+ A hook > for platform-specific initialization after disabling temporary RAM.++ > @retval EFI_SUCCESS The platform initialization was successful.+ > @retval EFI_NOT_READY The platform has not been detected > yet.+**/+typedef+EFI_STATUS+(EFIAPI > *PLATFORM_INIT_AFTER_TEMP_RAM_EXIT) (+ VOID+ );++///+/// This PPI > provides functions for notifying PlatformInitPreMem+/// of temporary > memory being disabled.+///+struct _PLATFORM_INIT_TEMP_RAM_EXIT_PPI > {+ PLATFORM_INIT_BEFORE_TEMP_RAM_EXIT > PlatformInitBeforeTempRamExit;+ > PLATFORM_INIT_AFTER_TEMP_RAM_EXIT > PlatformInitAfterTempRamExit;+};++extern EFI_GUID > gPlatformInitTempRamExitPpiGuid;++#endif // > _PLATFORM_INIT_TEMP_RAM_EXIT_PPI_H_diff --git > a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > index 6ef0219129..6a765d689d 100644 > --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > @@ -28,6 +28,8 @@ > gPeiBaseMemoryTestPpiGuid =3D {0xb6ec423c, 0x21d2, 0x490d, > {0x85, 0xc6, 0xdd, 0x58, 0x64, 0xea, 0xa6, 0x74}} > gPeiPlatformMemorySizePpiGuid =3D {0x9a7ef41e, 0xc140, 0x4bd1, {0xb8, > 0x84, 0x1e, 0x11, 0x24, 0x0b, 0x4c, 0xe6}} + > gPlatformInitTempRamExitPpiGuid =3D {0xbae23646, 0xbd60, 0x4f8b, {0xb3, > 0xf9, 0xf3, 0x91, 0xee, 0x7e, 0xe6, 0xc8}}+ [Guids] > gMinPlatformPkgTokenSpaceGuid =3D {0x69d13bf0, 0xaf91, 0x4d96, {0xaa, > 0x9f, 0x21, 0x84, 0xc5, 0xce, 0x3b, 0xc0}} diff --git > a/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInit= Pr > eMem.c > b/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInit= Pr > eMem.c > index c579ff008e..efdeb6a91c 100644 > --- > a/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInit= Pr > eMem.c > +++ b/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/Platfor > +++ mInitPreMem.c > @@ -29,6 +29,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include > #include > #include +#include > EFI_STATUS EFIAPI@@ -72,7 +73,31 > @@ BaseMemoryTest ( > OUT EFI_PHYSICAL_ADDRESS *ErrorAddress ); -static > EFI_PEI_NOTIFY_DESCRIPTOR mMemDiscoveredNotifyList =3D {+/**+ A hook > for platform-specific initialization prior to disabling temporary RAM.++ > @retval EFI_SUCCESS The platform initialization was successful.+ > @retval EFI_NOT_READY The platform has not been detected > yet.+**/+EFI_STATUS+EFIAPI+PlatformInitBeforeTempRamExit (+ > VOID+ );++/**+ A hook for platform-specific initialization after disabl= ing > temporary RAM.++ @retval EFI_SUCCESS The platform initialization was > successful.+ @retval EFI_NOT_READY The platform has not been detected > yet.+**/+EFI_STATUS+EFIAPI+PlatformInitAfterTempRamExit (+ > VOID+ );++GLOBAL_REMOVE_IF_UNREFERENCED > EFI_PEI_NOTIFY_DESCRIPTOR mMemDiscoveredNotifyList =3D > { (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), > &gEfiPeiMemoryDiscoveredPpiGuid, (EFI_PEIM_NOTIFY_ENTRY_POINT) > MemoryDiscoveredPpiNotifyCallback@@ -90,11 +115,11 @@ > GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_PPI_DESCRIPTOR > mPpiBootMode =3D { > NULL }; -static PEI_BASE_MEMORY_TEST_PPI > mPeiBaseMemoryTestPpi =3D > { BaseMemoryTest };+GLOBAL_REMOVE_IF_UNREFERENCED > PEI_BASE_MEMORY_TEST_PPI mPeiBaseMemoryTestPpi =3D > { BaseMemoryTest }; -static PEI_PLATFORM_MEMORY_SIZE_PPI > mMemoryMemorySizePpi =3D > { GetPlatformMemorySize };+GLOBAL_REMOVE_IF_UNREFERENCED > PEI_PLATFORM_MEMORY_SIZE_PPI mMemoryMemorySizePpi =3D > { GetPlatformMemorySize }; -static EFI_PEI_PPI_DESCRIPTOR > mMemPpiList[] =3D {+GLOBAL_REMOVE_IF_UNREFERENCED > EFI_PEI_PPI_DESCRIPTOR mMemPpiList[] =3D > { { EFI_PEI_PPI_DESCRIPTOR_PPI, > &gPeiBaseMemoryTestPpiGuid,@@ -107,6 +132,17 @@ static > EFI_PEI_PPI_DESCRIPTOR mMemPpiList[] =3D { > }, }; +GLOBAL_REMOVE_IF_UNREFERENCED > PLATFORM_INIT_TEMP_RAM_EXIT_PPI mPlatformInitTempRamExitPpi =3D {+ > PlatformInitBeforeTempRamExit,+ > PlatformInitAfterTempRamExit+};++GLOBAL_REMOVE_IF_UNREFERENCED > EFI_PEI_PPI_DESCRIPTOR mPlatformInitTempRamExitPpiDesc =3D {+ > (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),+ > &gPlatformInitTempRamExitPpiGuid,+ &mPlatformInitTempRamExitPpi+};+ > /// /// Memory Reserved should be between 125% to 150% of the Current > required memory /// otherwise BdsMisc.c would do a reset to make it 125% > to avoid s4 resume issues.@@ -391,6 +427,35 @@ > MemoryDiscoveredPpiNotifyCallback ( > return Status; } +/**+ A hook for platform-specific initialization pr= ior to > disabling temporary RAM.++ @retval EFI_SUCCESS The platform > initialization was successful.+ @retval EFI_NOT_READY The platform has > not been detected > yet.+**/+EFI_STATUS+EFIAPI+PlatformInitBeforeTempRamExit (+ > VOID+ )+{+ return BoardInitBeforeTempRamExit ();+}++/**+ A hook for > platform-specific initialization after disabling temporary RAM.++ @retva= l > EFI_SUCCESS The platform initialization was successful.+ @retval > EFI_NOT_READY The platform has not been detected > yet.+**/+EFI_STATUS+EFIAPI+PlatformInitAfterTempRamExit (+ VOID+ )+{+ > return BoardInitAfterTempRamExit ();+} /** This function handles > PlatformInit task after PeiReadOnlyVariable2 PPI produced@@ -446,6 +511,9 > @@ PlatformInitPreMem ( > Status =3D BoardInitBeforeMemoryInit (); ASSERT_EFI_ERROR (Status); = + > Status =3D PeiServicesInstallPpi (&mPlatformInitTempRamExitPpiDesc);+ > ASSERT_EFI_ERROR (Status);+ return Status; } diff --git > a/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInit= Pr > eMem.inf > b/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInit= Pr > eMem.inf > index af5dbe8772..7ee18eb6d5 100644 > --- > a/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInit= Pr > eMem.inf > +++ b/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/Platfor > +++ mInitPreMem.inf > @@ -53,6 +53,7 @@ > gEfiPeiMemoryDiscoveredPpiGuid gEfiPeiMasterBootModePpiGuid > ## PRODUCES gEfiPeiBootInRecoveryModePpiGuid ## > PRODUCES+ gPlatformInitTempRamExitPpiGuid ## > PRODUCES gEfiPeiReadOnlyVariable2PpiGuid > gPeiBaseMemoryTestPpiGuid gPeiPlatformMemorySizePpiGuid-- > 2.24.0.windows.2