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 <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18037 6f19259b-4bc3-4df7-8a09-765794883524

diff --git a/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c b/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c
index 5eb33e0587012be8fff50d69689d1d29e47d797a..8372db85bdb5a899b75fbc71c501efabcbc186ff 100644 (file)
--- 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 <Guid/Acpi.h>\r
 #include <Guid/EventGroup.h>\r
 #include <Protocol/AcpiS3Save.h>\r
-#include <Protocol/S3SaveState.h>\r
-#include <Protocol/DxeSmmReadyToLock.h>\r
 #include <Protocol/LockBox.h>\r
 #include <IndustryStandard/Acpi.h>\r
 \r
@@ -414,48 +412,6 @@ LegacyGetS3MemorySize (
   return EFI_SUCCESS;\r
 }\r
 \r
-/**\r
-  Save the S3 boot script.\r
-\r
-  Note that we trigger DxeSmmReadyToLock here -- otherwise the script wouldn't\r
-  be saved actually. Triggering this protocol installation event in turn locks\r
-  down SMM, so no further changes to LockBoxes or SMRAM are possible\r
-  afterwards.\r
-**/\r
-STATIC\r
-VOID\r
-EFIAPI\r
-SaveS3BootScript (\r
-  VOID\r
-  )\r
-{\r
-  EFI_STATUS                 Status;\r
-  EFI_S3_SAVE_STATE_PROTOCOL *BootScript;\r
-  EFI_HANDLE                 Handle;\r
-  STATIC CONST UINT8         Info[] = { 0xDE, 0xAD, 0xBE, 0xEF };\r
-\r
-  Status = gBS->LocateProtocol (&gEfiS3SaveStateProtocolGuid, NULL,\r
-                  (VOID **) &BootScript);\r
-  ASSERT_EFI_ERROR (Status);\r
-\r
-  //\r
-  // Despite the opcode documentation in the PI spec, the protocol\r
-  // implementation embeds a deep copy of the info in the boot script, rather\r
-  // than storing just a pointer to runtime or NVS storage.\r
-  //\r
-  Status = BootScript->Write(BootScript, EFI_BOOT_SCRIPT_INFORMATION_OPCODE,\r
-                         (UINT32) sizeof Info,\r
-                         (EFI_PHYSICAL_ADDRESS)(UINTN) &Info);\r
-  ASSERT_EFI_ERROR (Status);\r
-\r
-  Handle = NULL;\r
-  Status = gBS->InstallProtocolInterface (&Handle,\r
-                  &gEfiDxeSmmReadyToLockProtocolGuid, EFI_NATIVE_INTERFACE,\r
-                  NULL);\r
-  ASSERT_EFI_ERROR (Status);\r
-}\r
-\r
-\r
 /**\r
   Prepares all information that is needed in the S3 resume boot path.\r
   \r
@@ -563,11 +519,6 @@ S3Ready (
   Status = SetLockBoxAttributes (&gEfiAcpiS3ContextGuid, LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE);\r
   ASSERT_EFI_ERROR (Status);\r
 \r
-  //\r
-  // Save the boot script too. Note that this requires/includes emitting the\r
-  // DxeSmmReadyToLock event, which in turn locks down SMM.\r
-  //\r
-  SaveS3BootScript ();\r
   return EFI_SUCCESS;\r
 }\r
 \r

diff --git a/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf b/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
index 81da2fb80c9c0ae55ff754b92b81171e1ec4bfed..e657bbe8c56422bf642c8174b546d355131e3d75 100644 (file)
--- a/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
+++ b/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
@@ -66,8 +66,6 @@
   gEfiLegacyBiosProtocolGuid                    # PROTOCOL ALWAYS_CONSUMED\r
   gEfiLegacyRegion2ProtocolGuid                 # PROTOCOL SOMETIMES_CONSUMED\r
   gFrameworkEfiMpServiceProtocolGuid            # PROTOCOL SOMETIMES_CONSUMED\r
-  gEfiS3SaveStateProtocolGuid                   # PROTOCOL ALWAYS_CONSUMED\r
-  gEfiDxeSmmReadyToLockProtocolGuid             # PROTOCOL ALWAYS_PRODUCED\r
 \r
 [FeaturePcd]\r
   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPlatformCsmSupport          ## CONSUMES\r
@@ -79,4 +77,4 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable\r
 \r
 [Depex]\r
-  gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid AND gEfiS3SaveStateProtocolGuid\r
+  gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid\r

diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
index ce299875cdd21f5a8642717746766a126bf96a7c..0abba98dfe0307e3629f18fb667cee87f365a1c0 100644 (file)
--- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
@@ -1184,6 +1184,47 @@ OnEndOfDxe (
 }\r
 \r
 \r
+/**\r
+  Save the S3 boot script.\r
+\r
+  Note that we trigger DxeSmmReadyToLock here -- otherwise the script wouldn't\r
+  be saved actually. Triggering this protocol installation event in turn locks\r
+  down SMM, so no further changes to LockBoxes or SMRAM are possible\r
+  afterwards.\r
+**/\r
+STATIC\r
+VOID\r
+SaveS3BootScript (\r
+  VOID\r
+  )\r
+{\r
+  EFI_STATUS                 Status;\r
+  EFI_S3_SAVE_STATE_PROTOCOL *BootScript;\r
+  EFI_HANDLE                 Handle;\r
+  STATIC CONST UINT8         Info[] = { 0xDE, 0xAD, 0xBE, 0xEF };\r
+\r
+  Status = gBS->LocateProtocol (&gEfiS3SaveStateProtocolGuid, NULL,\r
+                  (VOID **) &BootScript);\r
+  ASSERT_EFI_ERROR (Status);\r
+\r
+  //\r
+  // Despite the opcode documentation in the PI spec, the protocol\r
+  // implementation embeds a deep copy of the info in the boot script, rather\r
+  // than storing just a pointer to runtime or NVS storage.\r
+  //\r
+  Status = BootScript->Write(BootScript, EFI_BOOT_SCRIPT_INFORMATION_OPCODE,\r
+                         (UINT32) sizeof Info,\r
+                         (EFI_PHYSICAL_ADDRESS)(UINTN) &Info);\r
+  ASSERT_EFI_ERROR (Status);\r
+\r
+  Handle = NULL;\r
+  Status = gBS->InstallProtocolInterface (&Handle,\r
+                  &gEfiDxeSmmReadyToLockProtocolGuid, EFI_NATIVE_INTERFACE,\r
+                  NULL);\r
+  ASSERT_EFI_ERROR (Status);\r
+}\r
+\r
+\r
 VOID\r
 EFIAPI\r
 PlatformBdsPolicyBehavior (\r
@@ -1240,6 +1281,14 @@ Returns:
     gBS->CloseEvent (EndOfDxeEvent);\r
   }\r
 \r
+  if (QemuFwCfgS3Enabled ()) {\r
+    //\r
+    // Save the boot script too. Note that this requires/includes emitting the\r
+    // DxeSmmReadyToLock event, which in turn locks down SMM.\r
+    //\r
+    SaveS3BootScript ();\r
+  }\r
+\r
   if (PcdGetBool (PcdOvmfFlashVariablesEnable)) {\r
     DEBUG ((EFI_D_INFO, "PlatformBdsPolicyBehavior: not restoring NvVars "\r
       "from disk since flash variables appear to be supported.\n"));\r

diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h
index b510178668d57c3ff128e586517a0e40d22dac46..6ba0d48e80ee3d95274b50dacda88ede2bdadc67 100644 (file)
--- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h
+++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h
@@ -47,12 +47,15 @@ Abstract:
 #include <Library/DevicePathLib.h>\r
 #include <Library/IoLib.h>\r
 #include <Library/NvVarsFileLib.h>\r
+#include <Library/QemuFwCfgLib.h>\r
 \r
 #include <Protocol/Decompress.h>\r
 #include <Protocol/PciIo.h>\r
 #include <Protocol/FirmwareVolume2.h>\r
 #include <Protocol/SimpleFileSystem.h>\r
 #include <Protocol/PciRootBridgeIo.h>\r
+#include <Protocol/S3SaveState.h>\r
+#include <Protocol/DxeSmmReadyToLock.h>\r
 \r
 #include <Guid/Acpi.h>\r
 #include <Guid/SmBios.h>\r

diff --git a/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf b/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
index c40871b673db469dc8913e1b0e148682f61e762d..ab5468368da57d92baf9e5cbc451a5fb43da9d50 100644 (file)
--- a/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
+++ b/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
@@ -65,6 +65,8 @@
 [Protocols]\r
   gEfiDecompressProtocolGuid\r
   gEfiPciRootBridgeIoProtocolGuid\r
+  gEfiS3SaveStateProtocolGuid                   # PROTOCOL SOMETIMES_CONSUMED\r
+  gEfiDxeSmmReadyToLockProtocolGuid             # PROTOCOL SOMETIMES_PRODUCED\r
 \r
 [Guids]\r
   gEfiEndOfDxeEventGroupGuid\r