Discussion:
[edk2] [PATCH v2 0/6] OvmfPkg: save S3 state at EndOfDxe
Laszlo Ersek
2015-07-14 16:48:01 UTC
Permalink
Version 2 of
<http://thread.gmane.org/gmane.comp.bios.tianocore.devel/16304>.

Changes relative to v1:

- Since I received no feedback for Vlv2TbltDevicePkg, and no answer to
my explicit query about interest in Vlv2TbltDevicePkg, I dropped the
following v1 patches:

1/9 IntelFrameworkModulePkg: AcpiS3SaveDxe: prepare for End-of-Dxe
callback
2/9 IntelFrameworkModulePkg: AcpiS3SaveDxe: call S3Ready() at
End-of-Dxe
5/9 Vlv2TbltDevicePkg: replace AcpiS3Save->S3Save() with End-of-Dxe
signal
8/9 IntelFrameworkModulePkg: AcpiS3SaveDxe: drop
EFI_ACPI_S3_SAVE_PROTOCOL

The explanation is very simple: IntelFrameworkModulePkg/AcpiS3SaveDxe
and Vlv2TbltDevicePkg must be modified *together*. Since
Vlv2TbltDevicePkg is not being modified (see above), so can't
IntelFrameworkModulePkg/AcpiS3SaveDxe.

The resultant status is valid for the entire tree, and it is also
explained in v2 4/6.

- Incorporated v1 feedback; please see the notes section of each patch.

Tested with S3 enabled and disabled, with Fedora and Windows Server 2012
R2.

The series compiles and works at each stage (including S3 suspend+resume
when it is enabled).

Public branch: none ATM. The github repo is lagging behind SVN, and
without the most recent SVN commits, the series doesn't apply. It should
be no problem; we discussed repeatedly how to apply edk2 patches from
the mailing list.

Thanks
Laszlo

Ard Biesheuvel (1):
IntelFrameworkModulePkg/GenericBdsLib: remove AcpiS3->S3Save() call

Laszlo Ersek (5):
OvmfPkg: AcpiS3SaveDxe: prepare for End-of-Dxe callback
OvmfPkg: AcpiS3SaveDxe: call S3Ready() at End-of-Dxe
OvmfPkg: PlatformBdsLib: signal End-of-Dxe event group
OvmfPkg: install DxeSmmReadyToLock in PlatformBdsLib
OvmfPkg: AcpiS3SaveDxe: drop EFI_ACPI_S3_SAVE_PROTOCOL

IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf | 1 -
OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf | 13 +-
OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf | 5 +
IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h | 1 -
OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.h | 59 -------
OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h | 4 +
IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c | 10 --
OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c | 168 +++++++-------------
OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 86 ++++++++++
9 files changed, 154 insertions(+), 193 deletions(-)
delete mode 100644 OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.h
--
1.8.3.1
Laszlo Ersek
2015-07-14 16:48:02 UTC
Permalink
We are preparing for detaching the S3Ready() functionality from the
EFI_ACPI_S3_SAVE_PROTOCOL.S3Save() protocol member function. Instead, we
will hook the same logic to the End-of-Dxe event group.

The EFI_ACPI_S3_SAVE_PROTOCOL has another member: GetLegacyMemorySize().
According to the documenation,

This function returns the size of the legacy memory (meaning below 1 MB)
that is required during an S3 resume. Before the Framework-based
firmware transfers control to the OS, it has to transition from flat
mode into real mode in case the OS supplies only a real-mode waking
vector. This transition requires a certain amount of legacy memory.
After getting the size of legacy memory below, the caller is responsible
for allocating the legacy memory below 1 MB according to the size that
is returned. The specific implementation of allocating the legacy memory
is out of the scope of this specification.

When EFI_ACPI_S3_SAVE_PROTOCOL.S3Save() is called, the address of the
legacy memory allocated above must be passed to it, in the
LegacyMemoryAddress parameter.

In practice however:

- The S3Ready() function ignores the LegacyMemoryAddress completely.

- No code in the edk2 tree calls
EFI_ACPI_S3_SAVE_PROTOCOL.GetLegacyMemorySize(), ever.

- All callers of this specific implementation of
EFI_ACPI_S3_SAVE_PROTOCOL.S3Save() in the edk2 tree pass a NULL
LegacyMemoryAddress:

BdsLibBootViaBootOption()
[IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c]

For this reason, ASSERT() explicitly that LegacyGetS3MemorySize() is never
called, and that the LegacyMemoryAddress parameter is always NULL.

This fact is important to capture in the code, because in the End-of-Dxe
callback, no LegacyMemoryAddress parameter can be taken. So let's make it
clear that we actually don't even have any use for that parameter.

This patch ports the identical change from IntelFrameworkModulePkg to
OvmfPkg.

Cc: Jordan Justen <***@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <***@redhat.com>
Reviewed-by: Jordan Justen <***@intel.com>
---

Notes:
v2:
- unchanged

OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c b/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c
index f322981..f05764a 100644
--- a/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c
+++ b/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c
@@ -2,6 +2,7 @@
This is an implementation of the ACPI S3 Save protocol. This is defined in
S3 boot path specification 0.9.

+Copyright (c) 2014-2015, Red Hat, Inc.<BR>
Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>

This program and the accompanying materials
@@ -402,6 +403,8 @@ LegacyGetS3MemorySize (
OUT UINTN *Size
)
{
+ ASSERT (FALSE);
+
if (Size == NULL) {
return EFI_INVALID_PARAMETER;
}
@@ -491,6 +494,8 @@ S3Ready (
}
AlreadyEntered = TRUE;

+ ASSERT (LegacyMemoryAddress == NULL);
+
AcpiS3Context = AllocateMemoryBelow4G (EfiReservedMemoryType, sizeof(*AcpiS3Context));
ASSERT (AcpiS3Context != NULL);
AcpiS3ContextBuffer = (EFI_PHYSICAL_ADDRESS)(UINTN)AcpiS3Context;
--
1.8.3.1
Laszlo Ersek
2015-07-14 16:48:03 UTC
Permalink
Call S3Ready() whenever the first of the following occurs:
- a driver signals End-of-Dxe,
- a driver calls EFI_ACPI_S3_SAVE_PROTOCOL.S3Save().

S3Ready() already contains a static, function scope "latch" that causes it
to exit early when called for the second time or later.

(At the moment, the only platform in the edk2 tree that includes this
driver is OvmfPkg. That platform does not signal End-of-Dxe (yet).)

http://thread.gmane.org/gmane.comp.bios.tianocore.devel/16088/focus=16146
Suggested-by: Yao Jiewen <***@intel.com>
Cc: Jordan Justen <***@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <***@redhat.com>
Reviewed-by: Jordan Justen <***@intel.com>
---

Notes:
v2:
- unchanged

OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf | 1 +
OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c | 46 ++++++++++++++++++++
2 files changed, 47 insertions(+)

diff --git a/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf b/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
index ae56a20..81da2fb 100644
--- a/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
+++ b/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
@@ -58,6 +58,7 @@ [Guids]
gEfiAcpiS3ContextGuid # ALWAYS_CONSUMED
gEfiAcpi20TableGuid # ALWAYS_CONSUMED System Table
gEfiAcpi10TableGuid # ALWAYS_CONSUMED System Table
+ gEfiEndOfDxeEventGroupGuid ## CONSUMES ## Event

[Protocols]
gEfiAcpiS3SaveProtocolGuid # PROTOCOL ALWAYS_PRODUCED
diff --git a/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c b/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c
index f05764a..5eb33e0 100644
--- a/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c
+++ b/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c
@@ -29,6 +29,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Guid/AcpiVariableCompatibility.h>
#include <Guid/AcpiS3Context.h>
#include <Guid/Acpi.h>
+#include <Guid/EventGroup.h>
#include <Protocol/AcpiS3Save.h>
#include <Protocol/S3SaveState.h>
#include <Protocol/DxeSmmReadyToLock.h>
@@ -571,6 +572,40 @@ S3Ready (
}

/**
+ Callback function executed when the EndOfDxe event group is signaled.
+
+ @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.
+**/
+VOID
+EFIAPI
+OnEndOfDxe (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Our S3Ready() function ignores both of its parameters, and always
+ // succeeds.
+ //
+ Status = S3Ready (
+ NULL, // This
+ NULL // LegacyMemoryAddress
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ //
+ // Close the event, deregistering the callback and freeing resources.
+ //
+ Status = gBS->CloseEvent (Event);
+ ASSERT_EFI_ERROR (Status);
+}
+
+
+/**
The Driver Entry Point.

The function is the driver Entry point which will produce AcpiS3SaveProtocol.
@@ -591,6 +626,7 @@ InstallAcpiS3Save (
)
{
EFI_STATUS Status;
+ EFI_EVENT EndOfDxeEvent;

if (!QemuFwCfgS3Enabled()) {
return EFI_LOAD_ERROR;
@@ -612,5 +648,15 @@ InstallAcpiS3Save (
NULL
);
ASSERT_EFI_ERROR (Status);
+
+ Status = gBS->CreateEventEx (
+ EVT_NOTIFY_SIGNAL,
+ TPL_CALLBACK,
+ OnEndOfDxe,
+ NULL, /* NotifyContext */
+ &gEfiEndOfDxeEventGroupGuid,
+ &EndOfDxeEvent
+ );
+ ASSERT_EFI_ERROR (Status);
return Status;
}
--
1.8.3.1
Laszlo Ersek
2015-07-14 16:48:04 UTC
Permalink
(Paraphrasing git commit 9cd7d3c5 / SVN r17713:)

Currently, OvmfPkg fails to signal the End-of-Dxe event group when
entering the BDS phase, which results in some loss of functionality, eg.
variable reclaim in the variable driver, and the memory region splitting
in the DXE core that belongs to the properties table feature specified in
UEFI-2.5.

As discussed on the edk2-devel mailing list here:

http://thread.gmane.org/gmane.comp.bios.tianocore.devel/16088/focus=16109

it is up to the platform BDS to signal End-of-Dxe, since there may be
platform specific ordering constraints with respect to the signalling of
the event that are difficult to honor at the generic level.

(OvmfPkg specifics:)

(1) In OvmfPkg, we can't signal End-of-Dxe before PCI enumeration
completes. According to the previous patch, that would trigger
OvmfPkg/AcpiS3SaveDxe to save S3 state *before* the following chain of
action happened:

- PCI enumeration completes
- ACPI tables are installed by OvmfPkg/AcpiPlatformDxe
- the FACS table becomes available

Since OvmfPkg/AcpiS3SaveDxe can only save S3 state once the FACS table
is available, we must delay the End-of-Dxe signal until after PCI
enumeration completes (ie. root bridges are connected).

(2) Pre-patch, S3Ready() in OvmfPkg/AcpiS3SaveDxe is entered from
BdsLibBootViaBootOption()
[IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c].

After the patch, we enter S3Ready() earlier than that, by signaling
End-of-Dxe in PlatformBdsPolicyBehavior(). The timing / location of
this new call is correct as well, and the original call (that now
becomes the chronologically second call) becomes a no-op: S3Ready() is
protected against 2nd and later entries.

Cc: Jordan Justen <***@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <***@redhat.com>
---

Notes:
v2:
- split End-of-Dxe signalling into standalone patch [Jordan]

OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf | 3 ++
OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h | 1 +
OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 37 ++++++++++++++++++++
3 files changed, 41 insertions(+)

diff --git a/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf b/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
index ce29720..c40871b 100644
--- a/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
+++ b/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
@@ -65,3 +65,6 @@ [Pcd.IA32, Pcd.X64]
[Protocols]
gEfiDecompressProtocolGuid
gEfiPciRootBridgeIoProtocolGuid
+
+[Guids]
+ gEfiEndOfDxeEventGroupGuid
diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h
index e3e950e..b510178 100644
--- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h
+++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h
@@ -59,6 +59,7 @@ Abstract:
#include <Guid/Mps.h>
#include <Guid/HobList.h>
#include <Guid/GlobalVariable.h>
+#include <Guid/EventGroup.h>

#include <OvmfPlatforms.h>

diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
index de13c6b..ce29987 100644
--- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
@@ -1163,6 +1163,27 @@ 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
+ )
+{
+}
+
+
VOID
EFIAPI
PlatformBdsPolicyBehavior (
@@ -1197,12 +1218,28 @@ Returns:
{
EFI_STATUS Status;
EFI_BOOT_MODE BootMode;
+ EFI_EVENT EndOfDxeEvent;

DEBUG ((EFI_D_INFO, "PlatformBdsPolicyBehavior\n"));

VisitAllInstancesOfProtocol (&gEfiPciRootBridgeIoProtocolGuid,
ConnectRootBridge, NULL);

+ //
+ // 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 (PcdGetBool (PcdOvmfFlashVariablesEnable)) {
DEBUG ((EFI_D_INFO, "PlatformBdsPolicyBehavior: not restoring NvVars "
"from disk since flash variables appear to be supported.\n"));
--
1.8.3.1
Laszlo Ersek
2015-07-14 16:48:05 UTC
Permalink
From: Ard Biesheuvel <***@linaro.org>

The AcpiS3->S3Save() call needs to occur before the end-of-DXE event
is signalled. The end-of-DXE event needs to be signalled prior to
invoking any UEFI drivers, applications, or connecting consoles.

This means the call to S3Save() that occurs in BdsLibBootViaBootOption()
violates the ordering constraints, and should be removed. Since it is
the responsibility of the platform BDS to signal the end-of-DXE event,
it should also perform the AcpiS3->S3Save() call at an appropriate time.

Commit message update from Laszlo Ersek <***@redhat.com>:

Following Jiewen Yao's idea in

http://thread.gmane.org/gmane.comp.bios.tianocore.devel/16088/focus=16146

platforms that
(1) use this exact instance of GenericBdsLib, *and*
(2) support S3

should now collect the S3 state
(3) in an End-of-Dxe callback in their AcpiS3SaveDxe drivers, *or*
(4) with an explicit AcpiS3->S3Save() call made to their AcpiS3SaveDxe
drivers from their PlatformBdsLib instances.

OvmfPkg, which uses this GenericBdsLib instance, and has its own
AcpiS3SaveDxe fork, follows (3).

Vlv2TbltDevicePkg, which has a GenericBdsLib fork, and uses
IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe, follows (4).

There are no other platforms in the public edk2 repository that support
S3.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <***@linaro.org>
Reviewed-by: Laszlo Ersek <***@redhat.com>
Reviewed-by: Yao, Jiewen <***@intel.com>
[***@redhat.com: updated commit message]
Cc: Ard Biesheuvel <***@linaro.org>
Cc: Jeff Fan <***@intel.com>
Cc: Yao, Jiewen <***@intel.com>
Signed-off-by: Laszlo Ersek <***@redhat.com>
---

Notes:
v2:
- update commit message again

IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf | 1 -
IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h | 1 -
IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c | 10 ----------
3 files changed, 12 deletions(-)

diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf b/IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
index 5381e33..5a138a9 100644
--- a/IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
+++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
@@ -119,7 +119,6 @@ [Protocols]
gEfiLegacyBiosProtocolGuid ## SOMETIMES_CONSUMES
gEfiCpuArchProtocolGuid ## CONSUMES
gEfiDevicePathProtocolGuid ## CONSUMES
- gEfiAcpiS3SaveProtocolGuid ## SOMETIMES_CONSUMES
gEfiGraphicsOutputProtocolGuid ## SOMETIMES_CONSUMES
gEfiUgaDrawProtocolGuid |gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport ## SOMETIMES_CONSUMES
gEfiOEMBadgingProtocolGuid ## SOMETIMES_CONSUMES
diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h b/IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h
index c32579b..7201d8a 100644
--- a/IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h
+++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h
@@ -33,7 +33,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Protocol/SimpleNetwork.h>
#include <Protocol/FirmwareVolume2.h>
#include <Protocol/PciIo.h>
-#include <Protocol/AcpiS3Save.h>
#include <Protocol/OEMBadging.h>
#include <Protocol/GraphicsOutput.h>
#include <Protocol/UgaDraw.h>
diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
index e02a710..4b7eca7 100644
--- a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
+++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
@@ -2233,7 +2233,6 @@ BdsLibBootViaBootOption (
EFI_DEVICE_PATH_PROTOCOL *FilePath;
EFI_LOADED_IMAGE_PROTOCOL *ImageInfo;
EFI_DEVICE_PATH_PROTOCOL *WorkingDevicePath;
- EFI_ACPI_S3_SAVE_PROTOCOL *AcpiS3Save;
LIST_ENTRY TempBootLists;
EFI_BOOT_LOGO_PROTOCOL *BootLogo;

@@ -2241,15 +2240,6 @@ BdsLibBootViaBootOption (
*ExitData = NULL;

//
- // Notes: this code can be remove after the s3 script table
- // hook on the event EVT_SIGNAL_READY_TO_BOOT or
- // EVT_SIGNAL_LEGACY_BOOT
- //
- Status = gBS->LocateProtocol (&gEfiAcpiS3SaveProtocolGuid, NULL, (VOID **) &AcpiS3Save);
- if (!EFI_ERROR (Status)) {
- AcpiS3Save->S3Save (AcpiS3Save, NULL);
- }
- //
// If it's Device Path that starts with a hard drive path, append it with the front part to compose a
// full device path
//
--
1.8.3.1
Laszlo Ersek
2015-07-14 16:48:06 UTC
Permalink
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 <***@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <***@redhat.com>
---

Notes:
v2:
- move SaveS3BootScript() function definition and call from
OvmfPkg/AcpiS3SaveDxe to OvmfPkg/Library/PlatformBdsLib in a separate
patch [Jordan]

OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf | 4 +-
OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf | 2 +
OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h | 3 ++
OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c | 49 --------------------
OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 49 ++++++++++++++++++++
5 files changed, 55 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 c40871b..ab54683 100644
--- a/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
+++ b/OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
@@ -65,6 +65,8 @@ [Pcd.IA32, Pcd.X64]
[Protocols]
gEfiDecompressProtocolGuid
gEfiPciRootBridgeIoProtocolGuid
+ 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 b510178..6ba0d48 100644
--- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h
+++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h
@@ -47,12 +47,15 @@ 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/PciRootBridgeIo.h>
+#include <Protocol/S3SaveState.h>
+#include <Protocol/DxeSmmReadyToLock.h>

#include <Guid/Acpi.h>
#include <Guid/SmBios.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 ce29987..0abba98 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"));
--
1.8.3.1
Laszlo Ersek
2015-07-14 16:48:07 UTC
Permalink
At this point, nothing in the OVMF build calls EFI_ACPI_S3_SAVE_PROTOCOL
member functions; simplify the code by dropping this protocol interface.

Cc: Jordan Justen <***@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <***@redhat.com>
---

Notes:
v2:
- replace / update "ACPI S3 Save" comments with End-of-Dxe callback
references [Hot Tian]
- remove AcpiS3Save.h, it is now superfluous [Hot Tian]

OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf | 8 +-
OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.h | 59 -------------
OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c | 88 ++++----------------
3 files changed, 16 insertions(+), 139 deletions(-)

diff --git a/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf b/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
index e657bbe..4cc0713 100644
--- a/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
+++ b/OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
@@ -1,7 +1,6 @@
## @file
-# Component description file for AcpiS3Save module.
+# AcpiS3Save module installs EndOfDxe callback to prepare S3 boot data.
#
-# This is an implementation of the ACPI S3 Save protocol.
# Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
#
# This program and the accompanying materials are
@@ -21,7 +20,7 @@ [Defines]
MODULE_TYPE = DXE_DRIVER
VERSION_STRING = 1.0

- ENTRY_POINT = InstallAcpiS3Save
+ ENTRY_POINT = InstallEndOfDxeCallback

#
# The following information is for reference only and not required by the build tools.
@@ -30,7 +29,6 @@ [Defines]
#

[Sources]
- AcpiS3Save.h
AcpiS3Save.c

[Packages]
@@ -61,14 +59,12 @@ [Guids]
gEfiEndOfDxeEventGroupGuid ## CONSUMES ## Event

[Protocols]
- gEfiAcpiS3SaveProtocolGuid # PROTOCOL ALWAYS_PRODUCED
gEfiLockBoxProtocolGuid # PROTOCOL ALWAYS_PRODUCED
gEfiLegacyBiosProtocolGuid # PROTOCOL ALWAYS_CONSUMED
gEfiLegacyRegion2ProtocolGuid # PROTOCOL SOMETIMES_CONSUMED
gFrameworkEfiMpServiceProtocolGuid # PROTOCOL SOMETIMES_CONSUMED

[FeaturePcd]
- gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPlatformCsmSupport ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode

[Pcd]
diff --git a/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.h b/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.h
deleted file mode 100644
index 65974a3..0000000
--- a/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/** @file
- This is an implementation of the ACPI S3 Save protocol. This is defined in
- S3 boot path specification 0.9.
-
-Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
-
-This program and the accompanying materials
-are licensed and made available under the terms and conditions
-of the BSD License which accompanies this distribution. The
-full text of the license may be found at
-http://opensource.org/licenses/bsd-license.php
-
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-#ifndef _ACPI_S3_SAVE_H_
-#define _ACPI_S3_SAVE_H_
-
-/**
- Gets the buffer of legacy memory below 1 MB
- This function is to get the buffer in legacy memory below 1MB that is required during S3 resume.
-
- @param This A pointer to the EFI_ACPI_S3_SAVE_PROTOCOL instance.
- @param Size The returned size of legacy memory below 1 MB.
-
- @retval EFI_SUCCESS Size is successfully returned.
- @retval EFI_INVALID_PARAMETER The pointer Size is NULL.
-
-**/
-EFI_STATUS
-EFIAPI
-LegacyGetS3MemorySize (
- IN EFI_ACPI_S3_SAVE_PROTOCOL * This,
- OUT UINTN * Size
- );
-
-/**
- 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
-
- @param This A pointer to the EFI_ACPI_S3_SAVE_PROTOCOL instance.
- @param LegacyMemoryAddress The base address of legacy memory.
-
- @retval EFI_NOT_FOUND Some necessary information cannot be found.
- @retval EFI_SUCCESS All information was saved successfully.
- @retval EFI_OUT_OF_RESOURCES Resources were insufficient to save all the information.
- @retval EFI_INVALID_PARAMETER The memory range is not located below 1 MB.
-
-**/
-EFI_STATUS
-EFIAPI
-S3Ready (
- IN EFI_ACPI_S3_SAVE_PROTOCOL *This,
- IN VOID *LegacyMemoryAddress
- );
-#endif
diff --git a/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c b/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c
index 8372db8..2e1040b 100644
--- a/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c
+++ b/OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c
@@ -1,6 +1,9 @@
/** @file
- This is an implementation of the ACPI S3 Save protocol. This is defined in
- S3 boot path specification 0.9.
+ This is a replacement for the ACPI S3 Save protocol.
+
+ The ACPI S3 Save protocol used to be defined in the S3 boot path
+ specification 0.9. Instead, the same functionality is now hooked to the
+ End-of-Dxe event.

Copyright (c) 2014-2015, Red Hat, Inc.<BR>
Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
@@ -30,19 +33,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Guid/AcpiS3Context.h>
#include <Guid/Acpi.h>
#include <Guid/EventGroup.h>
-#include <Protocol/AcpiS3Save.h>
#include <Protocol/LockBox.h>
#include <IndustryStandard/Acpi.h>

-#include "AcpiS3Save.h"
-
-UINTN mLegacyRegionSize;
-
-EFI_ACPI_S3_SAVE_PROTOCOL mS3Save = {
- LegacyGetS3MemorySize,
- S3Ready,
-};
-
EFI_GUID mAcpiS3IdtrProfileGuid = {
0xdea652b0, 0xd587, 0x4c54, { 0xb5, 0xb4, 0xc6, 0x82, 0xe7, 0xa0, 0xaa, 0x3d }
};
@@ -385,52 +378,17 @@ S3CreateIdentityMappingPageTables (
}

/**
- Gets the buffer of legacy memory below 1 MB
- This function is to get the buffer in legacy memory below 1MB that is required during S3 resume.
-
- @param This A pointer to the EFI_ACPI_S3_SAVE_PROTOCOL instance.
- @param Size The returned size of legacy memory below 1 MB.
-
- @retval EFI_SUCCESS Size is successfully returned.
- @retval EFI_INVALID_PARAMETER The pointer Size is NULL.
-
-**/
-EFI_STATUS
-EFIAPI
-LegacyGetS3MemorySize (
- IN EFI_ACPI_S3_SAVE_PROTOCOL *This,
- OUT UINTN *Size
- )
-{
- ASSERT (FALSE);
-
- if (Size == NULL) {
- return EFI_INVALID_PARAMETER;
- }
-
- *Size = mLegacyRegionSize;
- return EFI_SUCCESS;
-}
-
-/**
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

- @param This A pointer to the EFI_ACPI_S3_SAVE_PROTOCOL instance.
- @param LegacyMemoryAddress The base address of legacy memory.
-
- @retval EFI_NOT_FOUND Some necessary information cannot be found.
@retval EFI_SUCCESS All information was saved successfully.
- @retval EFI_OUT_OF_RESOURCES Resources were insufficient to save all the information.
- @retval EFI_INVALID_PARAMETER The memory range is not located below 1 MB.
-
**/
+STATIC
EFI_STATUS
EFIAPI
S3Ready (
- IN EFI_ACPI_S3_SAVE_PROTOCOL *This,
- IN VOID *LegacyMemoryAddress
+ VOID
)
{
EFI_STATUS Status;
@@ -442,17 +400,12 @@ S3Ready (

DEBUG ((EFI_D_INFO, "S3Ready!\n"));

- //
- // Platform may invoke AcpiS3Save->S3Save() before ExitPmAuth, because we need save S3 information there, while BDS ReadyToBoot may invoke it again.
- // So if 2nd S3Save() is triggered later, we need ignore it.
- //
+ ASSERT (!AlreadyEntered);
if (AlreadyEntered) {
return EFI_SUCCESS;
}
AlreadyEntered = TRUE;

- ASSERT (LegacyMemoryAddress == NULL);
-
AcpiS3Context = AllocateMemoryBelow4G (EfiReservedMemoryType, sizeof(*AcpiS3Context));
ASSERT (AcpiS3Context != NULL);
AcpiS3ContextBuffer = (EFI_PHYSICAL_ADDRESS)(UINTN)AcpiS3Context;
@@ -539,13 +492,9 @@ OnEndOfDxe (
EFI_STATUS Status;

//
- // Our S3Ready() function ignores both of its parameters, and always
- // succeeds.
+ // Our S3Ready() function always succeeds.
//
- Status = S3Ready (
- NULL, // This
- NULL // LegacyMemoryAddress
- );
+ Status = S3Ready ();
ASSERT_EFI_ERROR (Status);

//
@@ -559,8 +508,9 @@ OnEndOfDxe (
/**
The Driver Entry Point.

- The function is the driver Entry point which will produce AcpiS3SaveProtocol.
-
+ The function is the driver Entry point that will register the End-of-Dxe
+ callback.
+
@param ImageHandle A handle for the image that is initializing this driver
@param SystemTable A pointer to the EFI system table

@@ -571,7 +521,7 @@ OnEndOfDxe (
**/
EFI_STATUS
EFIAPI
-InstallAcpiS3Save (
+InstallEndOfDxeCallback (
IN EFI_HANDLE ImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
)
@@ -583,18 +533,8 @@ InstallAcpiS3Save (
return EFI_LOAD_ERROR;
}

- if (!FeaturePcdGet(PcdPlatformCsmSupport)) {
- //
- // More memory for no CSM tip, because GDT need relocation
- //
- mLegacyRegionSize = 0x250;
- } else {
- mLegacyRegionSize = 0x100;
- }
-
Status = gBS->InstallMultipleProtocolInterfaces (
&ImageHandle,
- &gEfiAcpiS3SaveProtocolGuid, &mS3Save,
&gEfiLockBoxProtocolGuid, NULL,
NULL
);
--
1.8.3.1
Laszlo Ersek
2015-07-23 17:17:32 UTC
Permalink
Post by Laszlo Ersek
Version 2 of
<http://thread.gmane.org/gmane.comp.bios.tianocore.devel/16304>.
- Since I received no feedback for Vlv2TbltDevicePkg, and no answer to
my explicit query about interest in Vlv2TbltDevicePkg, I dropped the
1/9 IntelFrameworkModulePkg: AcpiS3SaveDxe: prepare for End-of-Dxe
callback
2/9 IntelFrameworkModulePkg: AcpiS3SaveDxe: call S3Ready() at
End-of-Dxe
5/9 Vlv2TbltDevicePkg: replace AcpiS3Save->S3Save() with End-of-Dxe
signal
8/9 IntelFrameworkModulePkg: AcpiS3SaveDxe: drop
EFI_ACPI_S3_SAVE_PROTOCOL
The explanation is very simple: IntelFrameworkModulePkg/AcpiS3SaveDxe
and Vlv2TbltDevicePkg must be modified *together*. Since
Vlv2TbltDevicePkg is not being modified (see above), so can't
IntelFrameworkModulePkg/AcpiS3SaveDxe.
The resultant status is valid for the entire tree, and it is also
explained in v2 4/6.
- Incorporated v1 feedback; please see the notes section of each patch.
Tested with S3 enabled and disabled, with Fedora and Windows Server 2012
R2.
The series compiles and works at each stage (including S3 suspend+resume
when it is enabled).
Public branch: none ATM. The github repo is lagging behind SVN, and
without the most recent SVN commits, the series doesn't apply. It should
be no problem; we discussed repeatedly how to apply edk2 patches from
the mailing list.
Thanks
Laszlo
IntelFrameworkModulePkg/GenericBdsLib: remove AcpiS3->S3Save() call
OvmfPkg: AcpiS3SaveDxe: prepare for End-of-Dxe callback
OvmfPkg: AcpiS3SaveDxe: call S3Ready() at End-of-Dxe
OvmfPkg: PlatformBdsLib: signal End-of-Dxe event group
OvmfPkg: install DxeSmmReadyToLock in PlatformBdsLib
OvmfPkg: AcpiS3SaveDxe: drop EFI_ACPI_S3_SAVE_PROTOCOL
IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf | 1 -
OvmfPkg/AcpiS3SaveDxe/AcpiS3SaveDxe.inf | 13 +-
OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf | 5 +
IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h | 1 -
OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.h | 59 -------
OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h | 4 +
IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c | 10 --
OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.c | 168 +++++++-------------
OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 86 ++++++++++
9 files changed, 154 insertions(+), 193 deletions(-)
delete mode 100644 OvmfPkg/AcpiS3SaveDxe/AcpiS3Save.h
Until the SVN outage at sourceforge.net gets resolved, we plan to commit
patches that are ready for merging to the "master" branch of the git
repository at <https://github.com/tianocore/edk2-svn-offline.git>.
Please see the announcement:

http://thread.gmane.org/gmane.comp.bios.edk2.devel/227

This particular patch (or series) appears ready for merging, therefore
it is being picked up:

http://thread.gmane.org/gmane.comp.bios.edk2.devel/227/focus=251

Further development should be based on edk2-svn-offline/master, until
the SVN outage is fixed. At that point the accumulated patches will be
mass-committed from edk2-svn-offline/master to SVN.

Thanks
Laszlo


------------------------------------------------------------------------------
Loading...