* [RESEND PATCH v3 1/4] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization
2021-08-31 1:31 [RESEND PATCH v3 0/4] Fix OvmfXen boot failure due to s3 support state Lin, Gary (HPS OE-Linux)
@ 2021-08-31 1:31 ` Lin, Gary (HPS OE-Linux)
2021-08-31 1:31 ` [RESEND PATCH v3 2/4] OvmfPkg/LockBoxLib: use PcdAcpiS3Enable to detect S3 support Lin, Gary (HPS OE-Linux)
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Lin, Gary (HPS OE-Linux) @ 2021-08-31 1:31 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Anthony Perard,
Julien Grall, Jim Fehlig, Joey Li, Gerd Hoffmann, Jiewen Yao
There are several functions in OvmfPkg/Library using
QemuFwCfgS3Enabled() to detect the S3 support status. However, in
MdeModulePkg, PcdAcpiS3Enable is used to check S3 support. Since
InitializeXenPlatform() didn't set PcdAcpiS3Enable as
InitializePlatform() did, this made the inconsistency between
drivers/functions.
For example, S3SaveStateDxe checked PcdAcpiS3Enable and skipped
S3BootScript because the default value is FALSE. On the other hand,
PlatformBootManagerBeforeConsole() from OvmfPkg/Library called
QemuFwCfgS3Enabled() and found it returned TRUE, so it invoked
SaveS3BootScript(). However, S3SaveStateDxe skipped S3BootScript, so
SaveS3BootScript() asserted due to EFI_NOT_FOUND.
This issue mainly affects "HVM Direct Kernel Boot". When used,
"fw_cfg" is enabled in QEMU and QemuFwCfgS3Enabled() returns true in
that case.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3573
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien@xen.org>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Joey Li <jlee@suse.com>
Signed-off-by: Gary Lin <gary.lin@hpe.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
---
v3:
- Update the description per Anthony's suggestion
- Add the bugzilla link
v2:
- Amend the description and address "HVM Direct Kernel Boot"
- Add the comment for the conditional test of QemuFwCfgS3Enabled()
- Remove unused QemuFwCfgLib
---
OvmfPkg/XenPlatformPei/XenPlatformPei.inf | 2 ++
OvmfPkg/XenPlatformPei/Platform.c | 13 +++++++++++++
2 files changed, 15 insertions(+)
diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
index 597cb6fcd7ff..20c27ff34b6c 100644
--- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
+++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
@@ -57,6 +57,7 @@ [LibraryClasses]
ResourcePublicationLib
PeiServicesLib
PeimEntryPoint
+ QemuFwCfgS3Lib
MtrrLib
MemEncryptSevLib
PcdLib
@@ -79,6 +80,7 @@ [Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base
gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode
diff --git a/OvmfPkg/XenPlatformPei/Platform.c b/OvmfPkg/XenPlatformPei/Platform.c
index a811e72ee301..e60478fdb493 100644
--- a/OvmfPkg/XenPlatformPei/Platform.c
+++ b/OvmfPkg/XenPlatformPei/Platform.c
@@ -26,6 +26,7 @@
#include <Library/PciLib.h>
#include <Library/PeimEntryPoint.h>
#include <Library/PeiServicesLib.h>
+#include <Library/QemuFwCfgS3Lib.h>
#include <Library/ResourcePublicationLib.h>
#include <Guid/MemoryTypeInformation.h>
#include <Ppi/MasterBootMode.h>
@@ -423,6 +424,8 @@ InitializeXenPlatform (
IN CONST EFI_PEI_SERVICES **PeiServices
)
{
+ EFI_STATUS Status;
+
DEBUG ((DEBUG_INFO, "Platform PEIM Loaded\n"));
DebugDumpCmos ();
@@ -433,6 +436,16 @@ InitializeXenPlatform (
CpuDeadLoop ();
}
+ //
+ // This S3 conditional test is mainly for HVM Direct Kernel Boot since
+ // QEMU fwcfg isn't really supported other than that.
+ //
+ if (QemuFwCfgS3Enabled ()) {
+ DEBUG ((DEBUG_INFO, "S3 support was detected on QEMU\n"));
+ Status = PcdSetBoolS (PcdAcpiS3Enable, TRUE);
+ ASSERT_EFI_ERROR (Status);
+ }
+
XenConnect ();
BootModeInitialization ();
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [RESEND PATCH v3 2/4] OvmfPkg/LockBoxLib: use PcdAcpiS3Enable to detect S3 support
2021-08-31 1:31 [RESEND PATCH v3 0/4] Fix OvmfXen boot failure due to s3 support state Lin, Gary (HPS OE-Linux)
2021-08-31 1:31 ` [RESEND PATCH v3 1/4] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization Lin, Gary (HPS OE-Linux)
@ 2021-08-31 1:31 ` Lin, Gary (HPS OE-Linux)
2021-08-31 1:31 ` [RESEND PATCH v3 3/4] OvmfPkg/PlatformBootManagerLib: " Lin, Gary (HPS OE-Linux)
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Lin, Gary (HPS OE-Linux) @ 2021-08-31 1:31 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Jim Fehlig, Joey Li,
Philippe Mathieu-Daude, Gerd Hoffmann, Jiewen Yao
To avoid the potential inconsistency between PcdAcpiS3Enable and
QemuFwCfgS3Enabled(), this commit modifies LockBoxLib to detect
S3 support by PcdAcpiS3Enable as modules in MdeModulePkg do.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3573
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Joey Li <jlee@suse.com>
Signed-off-by: Gary Lin <gary.lin@hpe.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
---
v3:
- Add the bugzilla link
---
OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf | 3 +--
OvmfPkg/Library/LockBoxLib/LockBoxDxe.c | 4 +---
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
index 38bcc577084a..9140b1ba9de9 100644
--- a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
+++ b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
@@ -33,8 +33,6 @@ [LibraryClasses]
BaseMemoryLib
DebugLib
UefiBootServicesTableLib
- QemuFwCfgLib
- QemuFwCfgS3Lib
[Protocols]
gEfiLockBoxProtocolGuid ## SOMETIMES_PRODUCES
@@ -42,6 +40,7 @@ [Protocols]
[Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
[FeaturePcd]
gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxDxe.c b/OvmfPkg/Library/LockBoxLib/LockBoxDxe.c
index b28ad4d2dba7..7dc2eea2395a 100644
--- a/OvmfPkg/Library/LockBoxLib/LockBoxDxe.c
+++ b/OvmfPkg/Library/LockBoxLib/LockBoxDxe.c
@@ -12,8 +12,6 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
-#include <Library/QemuFwCfgLib.h>
-#include <Library/QemuFwCfgS3Lib.h>
#include <Protocol/LockBox.h>
#include <LockBoxLib.h>
@@ -117,7 +115,7 @@ LockBoxDxeLibInitialize (
Status = LockBoxLibInitialize ();
if (!EFI_ERROR (Status)) {
- if (QemuFwCfgS3Enabled ()) {
+ if (PcdGetBool (PcdAcpiS3Enable)) {
//
// When S3 enabled, the first driver run with this library linked will
// have this library constructor to install LockBox protocol on the
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [RESEND PATCH v3 3/4] OvmfPkg/PlatformBootManagerLib: use PcdAcpiS3Enable to detect S3 support
2021-08-31 1:31 [RESEND PATCH v3 0/4] Fix OvmfXen boot failure due to s3 support state Lin, Gary (HPS OE-Linux)
2021-08-31 1:31 ` [RESEND PATCH v3 1/4] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization Lin, Gary (HPS OE-Linux)
2021-08-31 1:31 ` [RESEND PATCH v3 2/4] OvmfPkg/LockBoxLib: use PcdAcpiS3Enable to detect S3 support Lin, Gary (HPS OE-Linux)
@ 2021-08-31 1:31 ` Lin, Gary (HPS OE-Linux)
2021-08-31 1:31 ` [RESEND PATCH v3 4/4] OvmfPkg/SmmControl2Dxe: " Lin, Gary (HPS OE-Linux)
2021-08-31 11:08 ` [edk2-devel] [RESEND PATCH v3 0/4] Fix OvmfXen boot failure due to s3 support state Ard Biesheuvel
4 siblings, 0 replies; 6+ messages in thread
From: Lin, Gary (HPS OE-Linux) @ 2021-08-31 1:31 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Jim Fehlig, Joey Li,
Gerd Hoffmann, Jiewen Yao
To avoid the potential inconsistency between PcdAcpiS3Enable and
QemuFwCfgS3Enabled(), this commit modifies PlatformBootManagerLib to
detect S3 support by PcdAcpiS3Enable as modules in MdeModulePkg do.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3573
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Joey Li <jlee@suse.com>
Signed-off-by: Gary Lin <gary.lin@hpe.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
---
v3:
- Add the bugzilla link
---
.../Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index e470b9a6a3e5..c249a3cf1e35 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -61,6 +61,7 @@ [Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index b0e97429372b..71f63b244828 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -379,7 +379,7 @@ PlatformBootManagerBeforeConsole (
//
EfiEventGroupSignal (&gEfiEndOfDxeEventGroupGuid);
- if (QemuFwCfgS3Enabled ()) {
+ if (PcdGetBool (PcdAcpiS3Enable)) {
//
// Save the boot script too. Note that this will require us to emit the
// DxeSmmReadyToLock event just below, which in turn locks down SMM.
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [RESEND PATCH v3 4/4] OvmfPkg/SmmControl2Dxe: use PcdAcpiS3Enable to detect S3 support
2021-08-31 1:31 [RESEND PATCH v3 0/4] Fix OvmfXen boot failure due to s3 support state Lin, Gary (HPS OE-Linux)
` (2 preceding siblings ...)
2021-08-31 1:31 ` [RESEND PATCH v3 3/4] OvmfPkg/PlatformBootManagerLib: " Lin, Gary (HPS OE-Linux)
@ 2021-08-31 1:31 ` Lin, Gary (HPS OE-Linux)
2021-08-31 11:08 ` [edk2-devel] [RESEND PATCH v3 0/4] Fix OvmfXen boot failure due to s3 support state Ard Biesheuvel
4 siblings, 0 replies; 6+ messages in thread
From: Lin, Gary (HPS OE-Linux) @ 2021-08-31 1:31 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Jim Fehlig, Joey Li,
Gerd Hoffmann, Jiewen Yao
To avoid the potential inconsistency between PcdAcpiS3Enable and
QemuFwCfgS3Enabled(), this commit modifies SmmControl2Dxe to detect
S3 support by PcdAcpiS3Enable as modules in MdeModulePkg do.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3573
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Joey Li <jlee@suse.com>
Signed-off-by: Gary Lin <gary.lin@hpe.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
---
v3:
- Add the bugzilla link
---
OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf | 2 ++
OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c | 4 +---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf
index b8fdea8deb84..4cad56516f49 100644
--- a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf
+++ b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf
@@ -39,6 +39,7 @@ [Sources]
[Packages]
MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
OvmfPkg/OvmfPkg.dec
UefiCpuPkg/UefiCpuPkg.dec
@@ -62,6 +63,7 @@ [Protocols]
[Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout ## SOMETIMES_PRODUCES
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode ## SOMETIMES_PRODUCES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
[FeaturePcd]
gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c
index 9547c202880f..be04baf7b288 100644
--- a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c
+++ b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c
@@ -25,8 +25,6 @@
#include <Library/IoLib.h>
#include <Library/PcdLib.h>
#include <Library/PciLib.h>
-#include <Library/QemuFwCfgLib.h>
-#include <Library/QemuFwCfgS3Lib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Protocol/S3SaveState.h>
#include <Protocol/SmmControl2.h>
@@ -238,7 +236,7 @@ SmmControl2DxeEntryPoint (
//
mSmiFeatureNegotiation = NegotiateSmiFeatures ();
- if (QemuFwCfgS3Enabled ()) {
+ if (PcdGetBool (PcdAcpiS3Enable)) {
VOID *Registration;
//
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [edk2-devel] [RESEND PATCH v3 0/4] Fix OvmfXen boot failure due to s3 support state
2021-08-31 1:31 [RESEND PATCH v3 0/4] Fix OvmfXen boot failure due to s3 support state Lin, Gary (HPS OE-Linux)
` (3 preceding siblings ...)
2021-08-31 1:31 ` [RESEND PATCH v3 4/4] OvmfPkg/SmmControl2Dxe: " Lin, Gary (HPS OE-Linux)
@ 2021-08-31 11:08 ` Ard Biesheuvel
4 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2021-08-31 11:08 UTC (permalink / raw)
To: devel, gary.lin
Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann,
Anthony Perard, Julien Grall, Jim Fehlig, Joey Li
On Tue, 31 Aug 2021 at 03:31, Lin, Gary (HPS OE-Linux) <gary.lin@hpe.com> wrote:
>
> When using HVM Direct kernel boot with OvmfXen, it could fail at the
> S3BootScript due to the inconsistency between QemuFwCfgS3Enabled()
> and PcdAcpiS3Enable.
>
> This patch series initializes PcdAcpiS3Enable in
> . Besides, QemuFwCfgS3Enabled() is
> replaced with PcdAcpiS3Enable in several OVMF libraries to avoid the
> potential inconsistency.
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3573
>
> v3:
> - Update the description per Anthony's suggestion
> - Add the bugzilla links
> - Move the QemuKernelLoaderFsDxe patch out of this patch series
> and make it an independent patch
> v2:
> - Amend the description and address "HVM Direct Kernel Boot"
> - Add the comment for the conditional test of QemuFwCfgS3Enabled()
> - Remove unused QemuFwCfgLib
> - Update my email address
>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Jim Fehlig <jfehlig@suse.com>
> Cc: Joey Li <jlee@suse.com>
>
> Gary Lin (4):
> OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization
> OvmfPkg/LockBoxLib: use PcdAcpiS3Enable to detect S3 support
> OvmfPkg/PlatformBootManagerLib: use PcdAcpiS3Enable to detect S3
> support
> OvmfPkg/SmmControl2Dxe: use PcdAcpiS3Enable to detect S3 support
>
Merged as #1933
Thanks,
> OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf | 3 +--
> .../PlatformBootManagerLib.inf | 1 +
> OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf | 2 ++
> OvmfPkg/XenPlatformPei/XenPlatformPei.inf | 2 ++
> OvmfPkg/Library/LockBoxLib/LockBoxDxe.c | 4 +---
> .../Library/PlatformBootManagerLib/BdsPlatform.c | 2 +-
> OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c | 4 +---
> OvmfPkg/XenPlatformPei/Platform.c | 13 +++++++++++++
> 8 files changed, 22 insertions(+), 9 deletions(-)
>
> --
> 2.31.1
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread