public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gary Lin" <glin@suse.com>
To: devel@edk2.groups.io
Cc: Laszlo Ersek <lersek@redhat.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Jordan Justen <jordan.l.justen@intel.com>,
	Anthony Perard <anthony.perard@citrix.com>,
	Julien Grall <julien@xen.org>, Jim Fehlig <jfehlig@suse.com>
Subject: [RFC PATCH] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization
Date: Thu,  8 Jul 2021 12:05:49 +0800	[thread overview]
Message-ID: <20210708040549.8364-1-glin@suse.com> (raw)

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.

Setting PcdAcpiS3Enable at InitializeXenPlatform() "fixes" the crash
reported by my colleague. The other possible direction is to replace
QemuFwCfgS3Enabled() with PcdAcpiS3Enable. I'm not sure which one is
the right fix.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
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>
Signed-off-by: Gary Lin <glin@suse.com>
---
 OvmfPkg/XenPlatformPei/Platform.c         | 10 ++++++++++
 OvmfPkg/XenPlatformPei/XenPlatformPei.inf |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/OvmfPkg/XenPlatformPei/Platform.c b/OvmfPkg/XenPlatformPei/Platform.c
index a811e72ee301..f7edc979486e 100644
--- a/OvmfPkg/XenPlatformPei/Platform.c
+++ b/OvmfPkg/XenPlatformPei/Platform.c
@@ -26,6 +26,8 @@
 #include <Library/PciLib.h>
 #include <Library/PeimEntryPoint.h>
 #include <Library/PeiServicesLib.h>
+#include <Library/QemuFwCfgLib.h>
+#include <Library/QemuFwCfgS3Lib.h>
 #include <Library/ResourcePublicationLib.h>
 #include <Guid/MemoryTypeInformation.h>
 #include <Ppi/MasterBootMode.h>
@@ -423,6 +425,8 @@ InitializeXenPlatform (
   IN CONST EFI_PEI_SERVICES     **PeiServices
   )
 {
+  EFI_STATUS    Status;
+
   DEBUG ((DEBUG_INFO, "Platform PEIM Loaded\n"));
 
   DebugDumpCmos ();
@@ -433,6 +437,12 @@ InitializeXenPlatform (
     CpuDeadLoop ();
   }
 
+  if (QemuFwCfgS3Enabled ()) {
+    DEBUG ((DEBUG_INFO, "S3 support was detected on QEMU\n"));
+    Status = PcdSetBoolS (PcdAcpiS3Enable, TRUE);
+    ASSERT_EFI_ERROR (Status);
+  }
+
   XenConnect ();
 
   BootModeInitialization ();
diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
index 597cb6fcd7ff..1e22c0b2e2aa 100644
--- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
+++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
@@ -57,6 +57,8 @@ [LibraryClasses]
   ResourcePublicationLib
   PeiServicesLib
   PeimEntryPoint
+  QemuFwCfgLib
+  QemuFwCfgS3Lib
   MtrrLib
   MemEncryptSevLib
   PcdLib
@@ -79,6 +81,7 @@ [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode
-- 
2.31.1


             reply	other threads:[~2021-07-08  4:05 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-08  4:05 Gary Lin [this message]
2021-07-19  7:34 ` [RFC PATCH] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization Ard Biesheuvel
2021-07-19 10:11 ` Anthony PERARD
2021-07-19 16:07 ` Anthony PERARD
2021-07-19 16:48   ` Ard Biesheuvel
2021-07-20  6:52   ` Gary Lin
     [not found]   ` <16936D3065173CA9.5055@groups.io>
2021-07-21  6:56     ` [edk2-devel] " Gary Lin
2021-07-27 10:57       ` Anthony PERARD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210708040549.8364-1-glin@suse.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox