From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from de-smtp-delivery-102.mimecast.com (de-smtp-delivery-102.mimecast.com [194.104.109.102]) by mx.groups.io with SMTP id smtpd.web10.1033.1626850618037375881 for ; Tue, 20 Jul 2021 23:56:58 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@suse.com header.s=mimecast20200619 header.b=h4zpRavY; spf=pass (domain: suse.com, ip: 194.104.109.102, mailfrom: glin@suse.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=mimecast20200619; t=1626850615; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=YbyDDk0bQ55sNzcAJWZS1TNOLYrIZyhG/RP9glJXXBk=; b=h4zpRavYNP57WCGvBnIHzoMrxT5aIk6yfVRjXWQLz1jW6JCl5sfzKBL3oV6YS15sw9sCKl amxjsIFfX7t5H44WFe8NchTpTwbfIxQx+xCtmLqLo7QJvgQ7vVzw+wrxFAiwcS3EJfWoMq Ssn7BEF4XjQeQox5AXSgRM61nkMzfBc= Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-ve1eur03lp2053.outbound.protection.outlook.com [104.47.9.53]) (Using TLS) by relay.mimecast.com with ESMTP id de-mta-23-U6Pl0_9GN-uWJhyFO5IZrA-1; Wed, 21 Jul 2021 08:56:53 +0200 X-MC-Unique: U6Pl0_9GN-uWJhyFO5IZrA-1 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=X0uGTZhgPYAUbPDPi1Gs+SU/TNlL0GdF5mtKqFfRc5FETsJReOWx9DpIkogNjIEElRUkjDDCLdKn2V/mOLqQfUbEOtpVepT0xmwsyT2NKwflmkO0I3uEZxq4YMoLeH4palOhQ+1P6GReYVncj+AVCYmqrAJgSxE6tyFYfRr0vzTAOrKV6J4q/SKNo7QAhHaC68R4nMm+s7m/FG71RVu8h8za87kt2FOez0p49NOgbcRQXSjVcjKht/eETyk/mkHG0YVS/3tPd4BYxb1wgLs4I6dHCkm8FEX/Q17TIPfJn/LvLXK6iUdHeIMQvcqNrGq4gDxbN/3kwsyXmZw6kKcIng== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=YbyDDk0bQ55sNzcAJWZS1TNOLYrIZyhG/RP9glJXXBk=; b=hETH0U30NOaE8isDJlbPZuAc/rLGdZ1NxwhfJvI4cZHX8LJvvj4eKuJcLdjTeO1PSwr37z1rgUc0Fde0Sb6TvzdW9+2xLHAV/X4BnKvDSl2p6ypwcAXccOF2RjjlzXDFzLHM7ygslnHpSukEK27kp5hI/DtyEfij1t+OjFBjcqSfsvWsiNT30nCVpyjJRXBFOihd6otNMS/bW2otQCXgR+gl2+RoMx5qokPIHxfIhIJ6kqGqGSfYSY5yltA6MOu9tM6OPTBl5pUAnOni56lH+ncOuCboA8mgszpXADbu4UvLBKUD2Sq4RDp1t7jAj1jPV+ZTYrQDbmSkZpDTw85ojA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none Authentication-Results: citrix.com; dkim=none (message not signed) header.d=none;citrix.com; dmarc=none action=none header.from=suse.com; Received: from AM7PR04MB7013.eurprd04.prod.outlook.com (2603:10a6:20b:116::18) by AM6PR0402MB3783.eurprd04.prod.outlook.com (2603:10a6:209:1b::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4331.29; Wed, 21 Jul 2021 06:56:51 +0000 Received: from AM7PR04MB7013.eurprd04.prod.outlook.com ([fe80::a4b6:d6ff:1fc7:1f98]) by AM7PR04MB7013.eurprd04.prod.outlook.com ([fe80::a4b6:d6ff:1fc7:1f98%9]) with mapi id 15.20.4331.034; Wed, 21 Jul 2021 06:56:51 +0000 Date: Wed, 21 Jul 2021 14:56:46 +0800 From: "Gary Lin" To: Anthony PERARD Cc: devel@edk2.groups.io, Ard Biesheuvel , Jordan Justen , Julien Grall , Jim Fehlig Subject: Re: [edk2-devel] [RFC PATCH] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization Message-ID: References: <20210708040549.8364-1-glin@suse.com> <16936D3065173CA9.5055@groups.io> In-Reply-To: <16936D3065173CA9.5055@groups.io> X-ClientProxiedBy: PR0P264CA0194.FRAP264.PROD.OUTLOOK.COM (2603:10a6:100:1f::14) To AM7PR04MB7013.eurprd04.prod.outlook.com (2603:10a6:20b:116::18) Return-Path: glin@suse.com MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from GaryWorkstation (60.251.47.115) by PR0P264CA0194.FRAP264.PROD.OUTLOOK.COM (2603:10a6:100:1f::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4331.21 via Frontend Transport; Wed, 21 Jul 2021 06:56:49 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: b3ef3e96-bdad-4705-d17c-08d94c14b7c1 X-MS-TrafficTypeDiagnostic: AM6PR0402MB3783: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: T1iRG7awEUgG02vQvpx/4GgepYIg7zN1d4trvybQF5K/wlREaJQwOb0B/OyUHo0O8yF9iN3KB3f+AUdKdngJPvkFn9YMo5stX4dC5WG6sy7dvvz3rQqHTO1Ggc8h+I5d/MICoRIZXo8oWA2aE/zLEMzVChOej0UazFfPpKRW+RobzaHn49V1oUNEuB9OQ1zDpHmET7fNmu7/KDaCI/3f9GbuIQsGabC5xQSu34wDA0p+qvdLWafyFBK4U6YYnTIiqFcB1npd4rhWctP7oRnXnpLCL2iwOIVuk+CZneT0W+kPBVStyevOY6MR/Hri67pPq4FGgSJxqMzb814gEIEqjHWIYDOEWkTtF3eurzGgtIQ9cqJqvTGoKnsrhCvurXJHUovC622K3s9TWPaZ1uKxAZwC0/ElLsrugFPtb78tfLiP3BYYA59xYuYOrliQia6tZmTCgCENU524blukix1R20SL19oIRkwnKNgh7EzvaDEzlDMZYLo/JDbavMWTDDucwQscXlKW6kmJAT8GWjUWojYcqIO+IRp8LNB+DvcpJ/Qug0PTtkq0k4Q8igzWasfKV5ZW2iGSDLXp+iQg5M52kqSy9md55iuTgb+JhErIaHsdoX+KyF4BO3V78Tpub9iODdhfwocXZMul6Vt3RL7RK5K8OwunTnpvMJuf/dWFlEsF05o2Ovk5Yh0iqST6zHNCxTn7R3ME9iloBHsMeyFF8LHuVpwIURkn+W9ptUGSlYYlRts5rky4VMXYMvRMlBM8MeHGGbDZ2xXSTsa07X+7YA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AM7PR04MB7013.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(376002)(346002)(396003)(39850400004)(136003)(366004)(26005)(6496006)(956004)(186003)(8676002)(4326008)(316002)(9686003)(38100700002)(6666004)(8936002)(5660300002)(54906003)(107886003)(6916009)(2906002)(83380400001)(55236004)(966005)(478600001)(86362001)(55016002)(66476007)(66556008)(33716001)(66946007);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?1wnNyVgiqXLYK/2aeDZpjliUO4fH0yOcDXYZjb5pq1E2GLFbLxO5jb+NMfg2?= =?us-ascii?Q?KuzwSDYMVD2QW7RU2YlkrSHWBZgL7ZbOAVUIEdJgJkKhpMxJQuPNbC4A9zxa?= =?us-ascii?Q?mpSMh2ES1eG1ZZMJqE8023hBWgqY0KrYK8xPqOr96dWYD+xjvlVQzZ5SK7xZ?= =?us-ascii?Q?kRJ1zeDAuONL1cS9wia90SrudZyVbUCoa/uS2dDzioWQoZeuFUtSObVNsUJd?= =?us-ascii?Q?92MkoyUC9/mDyvL/dvcr5DAw94vWnFMF1QbAlNwbwCpUk31OzIU7s+LQcy0f?= =?us-ascii?Q?Yl1e8rLEoj7x2ssfwJVy0zQHkxvXSdIOPBvx4+WYcqiG6yKnBANA8ojEGV4i?= =?us-ascii?Q?g7kZJNiKNKd4mBYLDAuFcnTiTiiYumfDvY8hAn5iYUGNcJPLNSBg+QImT5g9?= =?us-ascii?Q?dKOIjVW0x2ECYzAvt85RFZCAYs7PGux6XJCj2i97l4EVfEb3YFHQGgH0Hwin?= =?us-ascii?Q?ZYJyWvVcREnyKp/iK07V0p9CmIpibtIsAC8vreoNhXrCdKunCgu0jpgYwy3F?= =?us-ascii?Q?s54EUG8YVEfSxXp5OzowkhVE4f/Z/wKFVJeHTx23sPzCj6nNHx0jCJBQ8B4M?= =?us-ascii?Q?pFgb1FaPwtLCbdS5nfoGfPYE9ZLJ2HFWN5j9vw6rVN8fcNc9Vs/Ol8p6PlN+?= =?us-ascii?Q?+8IzPeOpsgIZ6GA9gG7qTEQ5HxXgtu3y47U7APiSDYyqzVEp/ItPUnc8zFqJ?= =?us-ascii?Q?nVTR6Bme5oMdULjiKUcK6SBrsDIKR81WyGPJ8PYKO2BrNRVNm9qtPF9LHnKC?= =?us-ascii?Q?C1fALYAv5Ssx9AQJPiDKA8MTukFPaY0w2xTjXpnOjTSqPvlkrt8/TCd9ubmb?= =?us-ascii?Q?CNkP4Qne0gODuu2Gb/Z18+79SGU7r3QG0Gg+EN9jx8ElNoe5XSgPaI/koOJe?= =?us-ascii?Q?HMRGU//G2nxswW1s7e8+U5BTrbMHh2LF4FKgtJrkQGT2Vc5pgbAjge7sGgra?= =?us-ascii?Q?eqQAyhMtSelwMaxFgph4APWdOhflhY2hc4sUC9fwW+XSzCPqiORDBpwsjChD?= =?us-ascii?Q?0w7OSzn73WbK8A4waXVOPZQlxyNyrVUW1kxf2N00d7eP4uwHig1O/zB3rAX6?= =?us-ascii?Q?ylTtMuj5OiXsUqs6IqbEJRp2iCSSaxAlicB2OHgc0hc2cZEOfeiiLjMF/A12?= =?us-ascii?Q?gkTwNtyC7UAMFQhlgxj6DwZmkfwRM4AbapunMayL+0ds7Ou5bIUVkt/C/+QD?= =?us-ascii?Q?+J+4Xg72drmt05FMh0awjB9mvRZzzRnn0zELTCD9Cd1aDGUq8QDmzZKY6yhK?= =?us-ascii?Q?2paMZN7sIXXm7ae8IO3hUuDqVxErTUB5wVpaB4rtoED3rLeY7nC40FXVwPIq?= =?us-ascii?Q?BhlgEhL63ctUzT21Ekeap3YD?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: b3ef3e96-bdad-4705-d17c-08d94c14b7c1 X-MS-Exchange-CrossTenant-AuthSource: AM7PR04MB7013.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Jul 2021 06:56:51.7128 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: wSh76GKa8Wd+QEjEvS3LEdID1pxs0BFH/+35tDqGNPsB/KZa2yjh6nQkMjuxQHjq X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR0402MB3783 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Jul 20, 2021 at 02:52:12PM +0800, Gary Lin via groups.io wrote: > On Mon, Jul 19, 2021 at 05:07:21PM +0100, Anthony PERARD wrote: > > It would have been nice to have this patch in a patch series with > > "OvmfPkg/OvmfXen: add QemuKernelLoaderFsDxe", mostly to make it simpler > > to understand the problem needed to be fixed. > > > To be honest, I don't have Xen environment and didn't realize that it's > about direct kernel boot until looking into another bug report. I just > compared InitializeXenPlatform() with InitializePlatform() and my > colleague told me OvmfXen works again after setting PcdAcpiS3Enable. > > > On Thu, Jul 08, 2021 at 12:05:49PM +0800, 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. > > > > This sounds like OvmfPkg would need to be fixed to use PcdAcpiS3Enable > > instead of QemuFwCfgS3Enabled() in most placed and have a single place > > where QemuFwCfgS3Enabled() is used to set PcdAcpiS3Enable. If you feel > > like trying to fix that, that would be nice, and then we could probably > > set PcdAcpiS3Enable unconditionally on OvmfXen (and maybe hope that S3 > > support actually works with Xen). > > > That's why I marked this patch as RFC since the inconsistency could > exist in OVMF for KVM, not just Xen, so I would like to have feedbacks > from OvmfPkg maintainers. I'll amend the patch set to cover other > drivers/libraries in OvmfPkg. > > > In the mean time, this patch is fine but wants better comments. First > > two paragraphs are good, but the rest needs explanation on what we are > > trying to fix/workaround, that is "Direct Kernel Boot" as it is called > > in "man xl.cfg". > > > Thanks for the suggestion. Will amend the comment in v2. > BTW, it seems to me that QEMU fwcfg is only used for Xen Direct Kernel Boot. However, per xl.cfg manpage, it's possible to turn on or off S3 support by setting "acpi_s3" in xl.cfg, but PcdAcpiS3Enable wasn't set in the current OvmfXen implementation. Just wonder how xl passes the S3 support bit to OvmfXen. Thanks, Gary Lin > > > 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. > > > > > > Signed-off-by: Gary Lin > > > --- > > > 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 > > > > I don't think QemuFwCfgLib.h is needed, can you remove it? > > > Sure, will remove it from v2. > > > > +#include > > > #include > > > #include > > > #include > > > @@ -433,6 +437,12 @@ InitializeXenPlatform ( > > > CpuDeadLoop (); > > > } > > > > > > + if (QemuFwCfgS3Enabled ()) { > > > > This test needs a comment. QEMU's fwcfg isn't supposed to be available, > > unless one try to use the Direct Kernel Boot functionality. > > > > > + 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 > > > > Same here, QemuFwCfgLib doesn't seems to be needed or used. > > > Will remove it from v2. > > Thanks, > > Gary Lin > > > > > >