[edk2-devel] [edk2-platforms] [PATCH V2 14/14] MinPlatformPkg: Remove BoardInitLib dependency from PlatformSecLib
Kubacki, Michael A
michael.a.kubacki at intel.com
Fri Nov 22 06:33:04 UTC 2019
Reviewed-by: Michael Kubacki <michael.a.kubacki at intel.com>
> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>
> Sent: Thursday, November 21, 2019 12:59 AM
> To: devel at edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki at intel.com>; Chiu, Chasel
> <chasel.chiu at intel.com>; Gao, Liming <liming.gao at intel.com>
> Subject: [edk2-platforms] [PATCH V2 14/14] MinPlatformPkg: Remove
> BoardInitLib dependency from PlatformSecLib
>
> 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().
>
> 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.
>
> 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 are not actually
> required.
>
> 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.
>
> Cc: Michael Kubacki <michael.a.kubacki at intel.com>
> Cc: Chasel Chiu <chasel.chiu at intel.com>
> Cc: Liming Gao <liming.gao at intel.com>
> Signed-off-by: Nate DeSimone <nathaniel.l.desimone at intel.com>
> ---
> .../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
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
> ormSecLib/SecFspWrapperPlatformSecLib.inf
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
> ormSecLib/SecFspWrapperPlatformSecLib.inf
> index 02c720c73d..4f3fa9fa34 100644
> ---
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
> ormSecLib/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/SecFspWrapperPlatf
> ormSecLib/SecTempRamDone.c
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
> ormSecLib/SecTempRamDone.c
> index 922e4ec204..b22cf57d6c 100644
> ---
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
> ormSecLib/SecTempRamDone.c
> +++
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
> +++ formSecLib/SecTempRamDone.c
> @@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Ppi/TemporaryRamDone.h> #include
> <Ppi/TempRamExitPpi.h>+#include <Ppi/PlatformInitTempRamExitPpi.h>
> #include <Library/BaseMemoryLib.h> #include <Library/DebugLib.h>@@ -
> 17,7 +18,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include
> <Library/DebugAgentLib.h> #include <Library/FspWrapperPlatformLib.h>
> #include <Library/FspWrapperApiLib.h>-#include <Library/BoardInitLib.h>
> #include <Library/PeiServicesTablePointerLib.h> /**@@ -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 =
> GetPeiServicesTablePointer ();+ ASSERT (PeiServices != NULL);+ if
> (PeiServices == NULL) {+ return;+ }+ ASSERT ((*PeiServices) != NULL);+ if
> ((*PeiServices) == NULL) {+ return;+ }+ Status = (*PeiServices)->LocatePpi
> (+ PeiServices,+
> &gPlatformInitTempRamExitPpiGuid,+ 0,+ NULL,+
> (VOID **) &PlatformInitTempRamExitPpi+ );+
> ASSERT_EFI_ERROR (Status);+ if (EFI_ERROR (Status)) {+ return;+ } - Status
> = BoardInitBeforeTempRamExit ();+ Status = PlatformInitTempRamExitPpi-
> >PlatformInitBeforeTempRamExit (); ASSERT_EFI_ERROR (Status); if
> (PcdGet8 (PcdFspModeSelection) == 1) {@@ -51,7 +72,6 @@
> SecPlatformDisableTemporaryMemory (
> // // FSP Dispatch mode //- PeiServices = GetPeiServicesTablePointer
> (); Status = (*PeiServices)->LocatePpi ( PeiServices,
> &gFspTempRamExitPpiGuid,@@ -66,7 +86,7 @@
> SecPlatformDisableTemporaryMemory (
> TempRamExitPpi->TempRamExit (NULL); } - Status =
> BoardInitAfterTempRamExit ();+ Status = 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.<BR>+SPDX-License-Identifier: BSD-2-Clause-
> Patent++**/++#ifndef
> _PLATFORM_INIT_TEMP_RAM_EXIT_PPI_H_+#define
> _PLATFORM_INIT_TEMP_RAM_EXIT_PPI_H_++#include <PiPei.h>++//+//
> 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 = {0xb6ec423c, 0x21d2, 0x490d, {0x85,
> 0xc6, 0xdd, 0x58, 0x64, 0xea, 0xa6, 0x74}} gPeiPlatformMemorySizePpiGuid
> = {0x9a7ef41e, 0xc140, 0x4bd1, {0xb8, 0x84, 0x1e, 0x11, 0x24, 0x0b, 0x4c,
> 0xe6}} + gPlatformInitTempRamExitPpiGuid = {0xbae23646, 0xbd60, 0x4f8b,
> {0xb3, 0xf9, 0xf3, 0x91, 0xee, 0x7e, 0xe6, 0xc8}}+ [Guids]
> gMinPlatformPkgTokenSpaceGuid = {0x69d13bf0, 0xaf91, 0x4d96, {0xaa,
> 0x9f, 0x21, 0x84, 0xc5, 0xce, 0x3b, 0xc0}} diff --git
> a/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitP
> reMem.c
> b/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitP
> reMem.c
> index c579ff008e..efdeb6a91c 100644
> ---
> a/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitP
> reMem.c
> +++ b/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/Platfor
> +++ mInitPreMem.c
> @@ -29,6 +29,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include
> <Guid/MemoryTypeInformation.h> #include <Ppi/PlatformMemorySize.h>
> #include <Ppi/BaseMemoryTest.h>+#include
> <Ppi/PlatformInitTempRamExitPpi.h> EFI_STATUS EFIAPI@@ -72,7 +73,31
> @@ BaseMemoryTest (
> OUT EFI_PHYSICAL_ADDRESS *ErrorAddress ); -static
> EFI_PEI_NOTIFY_DESCRIPTOR mMemDiscoveredNotifyList = {+/**+ 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 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+PlatformInitAfterTempRamExit (+ VOID+
> );++GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_NOTIFY_DESCRIPTOR
> mMemDiscoveredNotifyList = {
> (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 = {
> NULL }; -static PEI_BASE_MEMORY_TEST_PPI mPeiBaseMemoryTestPpi =
> { BaseMemoryTest };+GLOBAL_REMOVE_IF_UNREFERENCED
> PEI_BASE_MEMORY_TEST_PPI mPeiBaseMemoryTestPpi = {
> BaseMemoryTest }; -static PEI_PLATFORM_MEMORY_SIZE_PPI
> mMemoryMemorySizePpi = { GetPlatformMemorySize
> };+GLOBAL_REMOVE_IF_UNREFERENCED
> PEI_PLATFORM_MEMORY_SIZE_PPI mMemoryMemorySizePpi = {
> GetPlatformMemorySize }; -static EFI_PEI_PPI_DESCRIPTOR
> mMemPpiList[] = {+GLOBAL_REMOVE_IF_UNREFERENCED
> EFI_PEI_PPI_DESCRIPTOR mMemPpiList[] = { {
> EFI_PEI_PPI_DESCRIPTOR_PPI, &gPeiBaseMemoryTestPpiGuid,@@ -107,6
> +132,17 @@ static EFI_PEI_PPI_DESCRIPTOR mMemPpiList[] = {
> }, }; +GLOBAL_REMOVE_IF_UNREFERENCED
> PLATFORM_INIT_TEMP_RAM_EXIT_PPI mPlatformInitTempRamExitPpi = {+
> PlatformInitBeforeTempRamExit,+
> PlatformInitAfterTempRamExit+};++GLOBAL_REMOVE_IF_UNREFERENCED
> EFI_PEI_PPI_DESCRIPTOR mPlatformInitTempRamExitPpiDesc = {+
> (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 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+
> )+{+ return BoardInitBeforeTempRamExit ();+}++/**+ 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.+**/+EFI_STATUS+EFIAPI+PlatformInitAfterTempRamExit (+ VOID+
> )+{+ return BoardInitAfterTempRamExit ();+} /** This function handles
> PlatformInit task after PeiReadOnlyVariable2 PPI produced@@ -446,6 +511,9
> @@ PlatformInitPreMem (
> Status = BoardInitBeforeMemoryInit (); ASSERT_EFI_ERROR (Status); +
> Status = PeiServicesInstallPpi (&mPlatformInitTempRamExitPpiDesc);+
> ASSERT_EFI_ERROR (Status);+ return Status; } diff --git
> a/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitP
> reMem.inf
> b/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitP
> reMem.inf
> index af5dbe8772..7ee18eb6d5 100644
> ---
> a/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitP
> reMem.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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#51158): https://edk2.groups.io/g/devel/message/51158
Mute This Topic: https://groups.io/mt/61073545/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list