[edk2-devel] [RFC PATCH] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization

[Ard Biesheuvel]

On Thu, 8 Jul 2021 at 06:05, Gary Lin <glin at suse.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.
>
> 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 at redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Anthony Perard <anthony.perard at citrix.com>
> Cc: Julien Grall <julien at xen.org>
> Cc: Jim Fehlig <jfehlig at suse.com>
> Signed-off-by: Gary Lin <glin at suse.com>

This needs an ack from the Xen folks.

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77873): https://edk2.groups.io/g/devel/message/77873
Mute This Topic: https://groups.io/mt/84061313/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-