1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v2 0/5] Fix OvmfXen HVM Direct kernel boot failure
  4. View patch

[edk2-devel] [PATCH v2 1/5] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization

Lin, Gary (HPS OE-Linux) posted 5 patches 4 years, 6 months ago
[edk2-devel] [PATCH v2 1/5] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization
Posted by Lin, Gary (HPS OE-Linux) 4 years, 6 months ago 
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". If "acpi_s3" is
set as "True" in xl.cfg, then the S3 Support bit is set and passed
with fw_cfg.

v2:
  - Amend the description and address "HVM Direct Kernel Boot"
  - Add the comment for the conditional test of QemuFwCfgS3Enabled()
  - Remove unused QemuFwCfgLib

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>
---
 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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79247): https://edk2.groups.io/g/devel/message/79247
Mute This Topic: https://groups.io/mt/84857773/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 1/5] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization
Posted by Ard Biesheuvel 4 years, 5 months ago 
Does this series have a cover letter?

On Fri, 13 Aug 2021 at 08:13, Lin, Gary (HPS OE-Linux) <gary.lin@hpe.com> wrote:
>
> 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". If "acpi_s3" is
> set as "True" in xl.cfg, then the S3 Support bit is set and passed
> with fw_cfg.
>
> v2:
>   - Amend the description and address "HVM Direct Kernel Boot"
>   - Add the comment for the conditional test of QemuFwCfgS3Enabled()
>   - Remove unused QemuFwCfgLib
>
> 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>
> ---
>  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
>
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79337): https://edk2.groups.io/g/devel/message/79337
Mute This Topic: https://groups.io/mt/84857773/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 1/5] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization
Posted by Lin, Gary (HPS OE-Linux) 4 years, 5 months ago 
On Mon, Aug 16, 2021 at 09:07:01AM +0200, Ard Biesheuvel wrote:
> Does this series have a cover letter?
> 
Sorry, I forgot to add Cc tags in the cover letter.
https://edk2.groups.io/g/devel/topic/84857762#79245

Gary Lin

> On Fri, 13 Aug 2021 at 08:13, Lin, Gary (HPS OE-Linux) <gary.lin@hpe.com> wrote:
> >
> > 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". If "acpi_s3" is
> > set as "True" in xl.cfg, then the S3 Support bit is set and passed
> > with fw_cfg.
> >
> > v2:
> >   - Amend the description and address "HVM Direct Kernel Boot"
> >   - Add the comment for the conditional test of QemuFwCfgS3Enabled()
> >   - Remove unused QemuFwCfgLib
> >
> > 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>
> > ---
> >  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
> >
> >
> >
> > 
> >
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79339): https://edk2.groups.io/g/devel/message/79339
Mute This Topic: https://groups.io/mt/84857773/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.