Currently we have the following call chain in OVMF:
BdsLibBootViaBootOption()
[IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c]
//
// calls, via AcpiS3Save->S3Save():
//
S3Ready() [OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c]
//
// 1. saves S3 state, then calls:
//
SaveS3BootScript() [OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c]
//
// 2. saves INFO opcode in S3 boot script
// 3. installs DxeSmmReadyToLockProtocol
//
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.
This GenericBdsLib bug is going to be fixed in the next patch -- the call
in BdsLibBootViaBootOption() will be eliminated. Also, following Yao
Jiewen's idea, AcpiS3SaveDxe now saves S3 system state on End-of-Dxe as
well (which OVMF's BDS doesn't signal yet).
Therefore, reorganize the call tree as follows:
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.
Plus, after the patch OVMF's BDS signals End-of-Dxe (in the right place),
which has additional benefits: variable reclaim, and the splitting of the
memory regions that is part of the recently added UEFI 2.5 properties
table feature.
Note that after the patch, both the new and the old calls to S3Ready()
occur. (The old call will be removed in the next patch.) Luckily, that
function has always been protected against second and later calls with a
static variable.
This patch has been regression tested for S3 suspend and resume, with
Fedora and Windows Server 2012 R2 guests.
Cc: Jordan Justen <jordan.l.jus...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
---
OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf | 4 +-
OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf | 4 +
OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h | 4 +
OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c | 49 -----------
OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 86 ++++++++++++++++++++
5 files changed, 95 insertions(+), 52 deletions(-)
diff --git a/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
b/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
index 81da2fb..e657bbe 100644
--- a/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
+++ b/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
@@ -66,8 +66,6 @@ [Protocols]
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 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable
[Depex]
- gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid AND
gEfiS3SaveStateProtocolGuid
+ gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid
diff --git a/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
b/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
index 5a28d78..f66f51c 100644
--- a/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
+++ b/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
@@ -64,4 +64,8 @@ [Pcd.IA32, Pcd.X64]
[Protocols]
gEfiDecompressProtocolGuid
+ gEfiS3SaveStateProtocolGuid # PROTOCOL SOMETIMES_CONSUMED
+ gEfiDxeSmmReadyToLockProtocolGuid # PROTOCOL SOMETIMES_PRODUCED
+[Guids]
+ gEfiEndOfDxeEventGroupGuid
diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h
b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h
index 7006fb3..f451e7e 100644
--- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h
+++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h
@@ -47,17 +47,21 @@ Abstract:
#include <Library/DevicePathLib.h>
#include <Library/IoLib.h>
#include <Library/NvVarsFileLib.h>
+#include <Library/QemuFwCfgLib.h>
#include <Protocol/Decompress.h>
#include <Protocol/PciIo.h>
#include <Protocol/FirmwareVolume2.h>
#include <Protocol/SimpleFileSystem.h>
+#include <Protocol/S3SaveState.h>
+#include <Protocol/DxeSmmReadyToLock.h>
#include <Guid/Acpi.h>
#include <Guid/SmBios.h>
#include <Guid/Mps.h>
#include <Guid/HobList.h>
#include <Guid/GlobalVariable.h>
+#include <Guid/EventGroup.h>
#include <OvmfPlatforms.h>
diff --git a/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c
b/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c
index 5eb33e0..8372db8 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 <Guid/Acpi.h>
#include <Guid/EventGroup.h>
#include <Protocol/AcpiS3Save.h>
-#include <Protocol/S3SaveState.h>
-#include <Protocol/DxeSmmReadyToLock.h>
#include <Protocol/LockBox.h>
#include <IndustryStandard/Acpi.h>
@@ -415,48 +413,6 @@ LegacyGetS3MemorySize (
}
/**
- 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.
Allocate the resources or prepare informations and save in ACPI variable set
for 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/Library/PlatformBdsLib/BdsPlatform.c
b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
index 1eb2a8b..7440380 100644
--- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
@@ -1152,6 +1152,68 @@ Returns:
}
+/**
+ Empty callback function executed when the EndOfDxe event group is signaled.
+
+ We only need this function because we'd like to signal EndOfDxe, and for that
+ we need to create an event, with a callback function.
+
+ @param[in] Event Event whose notification function is being invoked.
+ @param[in] Context The pointer to the notification function's context, which
+ is implementation-dependent.
+**/
+STATIC
+VOID
+EFIAPI
+OnEndOfDxe (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+}
+
+
+/**
+ 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 (
@@ -1186,11 +1248,35 @@ Returns:
{
EFI_STATUS Status;
EFI_BOOT_MODE BootMode;
+ EFI_EVENT EndOfDxeEvent;
DEBUG ((EFI_D_INFO, "PlatformBdsPolicyBehavior\n"));
ConnectRootBridge ();
+ //
+ // We can't signal End-of-Dxe earlier than this. Namely, End-of-Dxe triggers
+ // the preparation of S3 system information. That logic has a hard dependency
+ // on the presence of the FACS ACPI table. Since our ACPI tables are only
+ // installed after PCI enumeration completes, we must not trigger the S3 save
+ // earlier, hence we can't signal End-of-Dxe earlier.
+ //
+ Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK, OnEndOfDxe,
+ NULL /* NotifyContext */, &gEfiEndOfDxeEventGroupGuid,
+ &EndOfDxeEvent);
+ if (!EFI_ERROR (Status)) {
+ gBS->SignalEvent (EndOfDxeEvent);
+ 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"));
--
1.8.3.1
------------------------------------------------------------------------------
Monitor 25 network devices or servers for free with OpManager!
OpManager is web-based network management software that monitors
network devices and physical & virtual servers, alerts via email & sms
for fault. Monitor 25 devices for free with no restriction. Download now
http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
