From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web09.24753.1626680097730767215 for ; Mon, 19 Jul 2021 00:34:57 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=lx2SBjuN; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id B2C5E6109E for ; Mon, 19 Jul 2021 07:34:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1626680096; bh=Zw2zPmFiMSKXR12jhNKAEJF24Ov+S2n1IL7HKpoDyEQ=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=lx2SBjuNAxElffgjogSUTPU5oGubigxTxHrNdhzlsZzLh07Wy/3afIA/kiJVxPLgl yZSfI1DJY4SnNxKCmVL2yx5d7mrS+CZk7Sjbjg9HRYXiT/aqWfX4vGYCQ/upe7EHdl ARdNorW7xRXpKC9iwFeoUSvUJUOYVmT6gdLScuYpHNEocUSiysh0fQCHE2Joeedv3a l0038DEeZk1np//D6EQ+7jfXgWegYcR2CO2YfWEQryrwNtf5xZMpjwKnPsr/ycVHEX yu7dSE8ugxI/vRnnrvUkSnS7VzzQgqXOBo6HCgUZe6BegK67V2lqymPAnqU44YQJGo D7Bf3wm090ZxA== Received: by mail-ot1-f41.google.com with SMTP id o72-20020a9d224e0000b02904bb9756274cso17355275ota.6 for ; Mon, 19 Jul 2021 00:34:56 -0700 (PDT) X-Gm-Message-State: AOAM533SeZJ45xQaLcSuPty2RnIW2EYIRoHb/vvtum0SaS3m+srDpK1h fqwxuNm9+nlvPssgjHGQdzMk2rXDwdHbCTWW+Rc= X-Google-Smtp-Source: ABdhPJwcnwFGQziGfu9AR6nr4Hvnl3GZhBhnlihN0DfebNqoxN0ro2AdOksClrsOGJ+weGPRgsqOCX6rc6mKtUxkU5E= X-Received: by 2002:a05:6830:34a6:: with SMTP id c38mr5139631otu.108.1626680096118; Mon, 19 Jul 2021 00:34:56 -0700 (PDT) MIME-Version: 1.0 References: <20210708040549.8364-1-glin@suse.com> In-Reply-To: <20210708040549.8364-1-glin@suse.com> From: "Ard Biesheuvel" Date: Mon, 19 Jul 2021 09:34:45 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC PATCH] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization To: Gary Lin Cc: edk2-devel-groups-io , Laszlo Ersek , Ard Biesheuvel , Jordan Justen , Anthony Perard , Julien Grall , Jim Fehlig Content-Type: text/plain; charset="UTF-8" On Thu, 8 Jul 2021 at 06:05, Gary Lin 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 > Cc: Ard Biesheuvel > Cc: Jordan Justen > Cc: Anthony Perard > Cc: Julien Grall > Cc: Jim Fehlig > Signed-off-by: Gary Lin 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 > #include > #include > +#include > +#include > #include > #include > #include > @@ -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 >