From 9d7051b7a09281f86533bc3ce65cfe69638e14a9 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Sun, 26 Jul 2015 08:02:24 +0000 Subject: [PATCH] OvmfPkg: install DxeSmmReadyToLock in PlatformBdsLib Currently we have the following call chain in OVMF: PlatformBdsPolicyBehavior() [OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c] // // signals End-of-Dxe // OnEndOfDxe() [OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c] S3Ready() [OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c] // // 1. saves S3 state // SaveS3BootScript() [OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c] // // 2. saves INFO opcode in S3 boot script // 3. installs DxeSmmReadyToLockProtocol // The bottom of this call chain was introduced in git commit 5a217a06 (SVN r15305, "OvmfPkg: S3 Suspend: save boot script after ACPI context"). That patch was necessary because there was no other way, due to GenericBdsLib calling S3Save() from BdsLibBootViaBootOption(), to perform the necessary steps in the right order: - save S3 system information, - save a final (well, only) boot script opcode, - signal DxeSmmReadyToLock, closing the boot script, and locking down LockBox and SMM. The GenericBdsLib bug has been fixed in the previous patch -- the call in BdsLibBootViaBootOption() has been eliminated. Therefore, hoist the SaveS3BootScript() code, and call, from OvmfPkg/AcpiS3SaveDxe, to PlatformBdsLib: PlatformBdsPolicyBehavior() [OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c] // // signals End-of-Dxe // OnEndOfDxe() [OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c] S3Ready() [OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c] // // 1. saves S3 state // <--- SaveS3BootScript() [OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c] // // 2. saves INFO opcode in S3 boot script // 3. installs DxeSmmReadyToLockProtocol // The installation of DxeSmmReadyToLockProtocol belongs with Platform BDS, not AcpiS3SaveDxe, and we can now undo the hack in SVN r15305, without upsetting the relative order of the steps. Cc: Jordan Justen Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek Reviewed-by: Jordan Justen git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18037 6f19259b-4bc3-4df7-8a09-765794883524 --- OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c | 49 ------------------- OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf | 4 +- OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 49 +++++++++++++++++++ OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h | 3 ++ .../Library/PlatformBdsLib/PlatformBdsLib.inf | 2 + 5 files changed, 55 insertions(+), 52 deletions(-) diff --git a/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c b/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c index 5eb33e0587..8372db85bd 100644 --- a/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c +++ b/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c @@ -31,8 +31,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include #include -#include -#include #include #include @@ -414,48 +412,6 @@ LegacyGetS3MemorySize ( return EFI_SUCCESS; } -/** - Save the S3 boot script. - - Note that we trigger DxeSmmReadyToLock here -- otherwise the script wouldn't - be saved actually. Triggering this protocol installation event in turn locks - down SMM, so no further changes to LockBoxes or SMRAM are possible - afterwards. -**/ -STATIC -VOID -EFIAPI -SaveS3BootScript ( - VOID - ) -{ - EFI_STATUS Status; - EFI_S3_SAVE_STATE_PROTOCOL *BootScript; - EFI_HANDLE Handle; - STATIC CONST UINT8 Info[] = { 0xDE, 0xAD, 0xBE, 0xEF }; - - Status = gBS->LocateProtocol (&gEfiS3SaveStateProtocolGuid, NULL, - (VOID **) &BootScript); - ASSERT_EFI_ERROR (Status); - - // - // Despite the opcode documentation in the PI spec, the protocol - // implementation embeds a deep copy of the info in the boot script, rather - // than storing just a pointer to runtime or NVS storage. - // - Status = BootScript->Write(BootScript, EFI_BOOT_SCRIPT_INFORMATION_OPCODE, - (UINT32) sizeof Info, - (EFI_PHYSICAL_ADDRESS)(UINTN) &Info); - ASSERT_EFI_ERROR (Status); - - Handle = NULL; - Status = gBS->InstallProtocolInterface (&Handle, - &gEfiDxeSmmReadyToLockProtocolGuid, EFI_NATIVE_INTERFACE, - NULL); - ASSERT_EFI_ERROR (Status); -} - - /** Prepares all information that is needed in the S3 resume boot path. @@ -563,11 +519,6 @@ S3Ready ( Status = SetLockBoxAttributes (&gEfiAcpiS3ContextGuid, LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE); ASSERT_EFI_ERROR (Status); - // - // Save the boot script too. Note that this requires/includes emitting the - // DxeSmmReadyToLock event, which in turn locks down SMM. - // - SaveS3BootScript (); return EFI_SUCCESS; } diff --git a/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf b/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf index 81da2fb80c..e657bbe8c5 100644 --- a/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf +++ b/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf @@ -66,8 +66,6 @@ gEfiLegacyBiosProtocolGuid # PROTOCOL ALWAYS_CONSUMED gEfiLegacyRegion2ProtocolGuid # PROTOCOL SOMETIMES_CONSUMED gFrameworkEfiMpServiceProtocolGuid # PROTOCOL SOMETIMES_CONSUMED - gEfiS3SaveStateProtocolGuid # PROTOCOL ALWAYS_CONSUMED - gEfiDxeSmmReadyToLockProtocolGuid # PROTOCOL ALWAYS_PRODUCED [FeaturePcd] gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPlatformCsmSupport ## CONSUMES @@ -79,4 +77,4 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable [Depex] - gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid AND gEfiS3SaveStateProtocolGuid + gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c index ce299875cd..0abba98dfe 100644 --- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c +++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c @@ -1184,6 +1184,47 @@ OnEndOfDxe ( } +/** + Save the S3 boot script. + + Note that we trigger DxeSmmReadyToLock here -- otherwise the script wouldn't + be saved actually. Triggering this protocol installation event in turn locks + down SMM, so no further changes to LockBoxes or SMRAM are possible + afterwards. +**/ +STATIC +VOID +SaveS3BootScript ( + VOID + ) +{ + EFI_STATUS Status; + EFI_S3_SAVE_STATE_PROTOCOL *BootScript; + EFI_HANDLE Handle; + STATIC CONST UINT8 Info[] = { 0xDE, 0xAD, 0xBE, 0xEF }; + + Status = gBS->LocateProtocol (&gEfiS3SaveStateProtocolGuid, NULL, + (VOID **) &BootScript); + ASSERT_EFI_ERROR (Status); + + // + // Despite the opcode documentation in the PI spec, the protocol + // implementation embeds a deep copy of the info in the boot script, rather + // than storing just a pointer to runtime or NVS storage. + // + Status = BootScript->Write(BootScript, EFI_BOOT_SCRIPT_INFORMATION_OPCODE, + (UINT32) sizeof Info, + (EFI_PHYSICAL_ADDRESS)(UINTN) &Info); + ASSERT_EFI_ERROR (Status); + + Handle = NULL; + Status = gBS->InstallProtocolInterface (&Handle, + &gEfiDxeSmmReadyToLockProtocolGuid, EFI_NATIVE_INTERFACE, + NULL); + ASSERT_EFI_ERROR (Status); +} + + VOID EFIAPI PlatformBdsPolicyBehavior ( @@ -1240,6 +1281,14 @@ Returns: gBS->CloseEvent (EndOfDxeEvent); } + if (QemuFwCfgS3Enabled ()) { + // + // Save the boot script too. Note that this requires/includes emitting the + // DxeSmmReadyToLock event, which in turn locks down SMM. + // + SaveS3BootScript (); + } + if (PcdGetBool (PcdOvmfFlashVariablesEnable)) { DEBUG ((EFI_D_INFO, "PlatformBdsPolicyBehavior: not restoring NvVars " "from disk since flash variables appear to be supported.\n")); diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h index b510178668..6ba0d48e80 100644 --- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h +++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h @@ -47,12 +47,15 @@ Abstract: #include #include #include +#include #include #include #include #include #include +#include +#include #include #include diff --git a/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf b/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf index c40871b673..ab5468368d 100644 --- a/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf +++ b/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf @@ -65,6 +65,8 @@ [Protocols] gEfiDecompressProtocolGuid gEfiPciRootBridgeIoProtocolGuid + gEfiS3SaveStateProtocolGuid # PROTOCOL SOMETIMES_CONSUMED + gEfiDxeSmmReadyToLockProtocolGuid # PROTOCOL SOMETIMES_PRODUCED [Guids] gEfiEndOfDxeEventGroupGuid