From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mx.groups.io with SMTP id smtpd.web10.16532.1629904507925651555 for ; Wed, 25 Aug 2021 08:15:09 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=nqEkWUYl; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: jiewen.yao@intel.com) X-IronPort-AV: E=McAfee;i="6200,9189,10087"; a="204676780" X-IronPort-AV: E=Sophos;i="5.84,351,1620716400"; d="scan'208";a="204676780" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Aug 2021 08:15:06 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.84,351,1620716400"; d="scan'208";a="684547343" Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by fmsmga006.fm.intel.com with ESMTP; 25 Aug 2021 08:15:06 -0700 Received: from orsmsx609.amr.corp.intel.com (10.22.229.22) by ORSMSX603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10; Wed, 25 Aug 2021 08:15:05 -0700 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx609.amr.corp.intel.com (10.22.229.22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10 via Frontend Transport; Wed, 25 Aug 2021 08:15:05 -0700 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.177) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2242.10; Wed, 25 Aug 2021 08:15:05 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gbpc7rryfUhxNQpDNyKUjY6rRhIzJAtnIrwEQQVEC3RAQQW056EHLotZdkODOzd0YaiT+ljDV4I/YZEfAyGl5O9HyTXT9EKiYPjAweiD9ktjaUarbycbcAAcys3Td3rkWizzjRMEqS2YSmGaGmb1kU1s4AGsHz0PQwenhTddasOZWgPHcsHqYEHofZxmts2oiZPCouOcw8cah8o/Dx1r/mYw5hvqOTSGRkQToHflGWavle4IKsaPo6/U4QkYVreyYA20z3O+7IXwa6oOFhquP+AQ3dDjEzxromZrbaDNpa3IWfkwrZ83pVNz1T3pWKcaUjYOrFiZ3EeM1FS7m59xLQ== 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=YSOqfSQesiUVHL+ELl44vTqgHAylO8Qh6i9FAFAHXyA=; b=ZSx8O9sBZlq3kDZ60GSXskoQex9WqS2rbPJVL6JEJ8dbR6lbfg0AYqEfwJ7lmHgt/i1ODaRHS4jngf7jg5eHSSbWgRpUmTOidI5RiPixtjGLjfWHT+4ZpopdwhDGfwjjZX0isOv15jgTlCdYCiJZiEp22ra/BYOpqtJYVy9DkBDZkZkHQ7ekCApzAHdiZfy1nnyXaW1it1LZTcfyhkTpSCNCiIj4lGZLGpDKtd86OSBTY9HfELQppVN9xKwT55m/D1O0N0W6v8cR6OTloItHX9avzKY2MbAzmQfVGLf/F0o45Iys2IAHI0W1lodgAegqNEVwq5eUNlLCmVhGtbKXTA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=YSOqfSQesiUVHL+ELl44vTqgHAylO8Qh6i9FAFAHXyA=; b=nqEkWUYlFUgC28BKN9pD13q4GNb3Z7ZIVunxZZni0oswRqpMgrRdrnH6QNiW2Y1KBPpzBzpITaLngNFmBSRmeEBQR82N6MeXqqL6A0zoJFHwOrw/e4O81EX6DQUoKe7a2dmrJCKRARh6iWXXCjReapgjT2L+sTQm+hPqRYNvLSM= Received: from PH0PR11MB4885.namprd11.prod.outlook.com (2603:10b6:510:35::14) by PH0PR11MB5173.namprd11.prod.outlook.com (2603:10b6:510:39::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4436.19; Wed, 25 Aug 2021 15:15:04 +0000 Received: from PH0PR11MB4885.namprd11.prod.outlook.com ([fe80::e97b:e466:268f:fb79]) by PH0PR11MB4885.namprd11.prod.outlook.com ([fe80::e97b:e466:268f:fb79%6]) with mapi id 15.20.4436.024; Wed, 25 Aug 2021 15:15:04 +0000 From: "Yao, Jiewen" To: "kraxel@redhat.com" CC: "devel@edk2.groups.io" Subject: Re: [edk2-devel] [PATCH 2/2] OvmfPkg/PlatformPei: prefer etc/e820 for memory detection Thread-Topic: [edk2-devel] [PATCH 2/2] OvmfPkg/PlatformPei: prefer etc/e820 for memory detection Thread-Index: AQHXlNHd+ct4uWoBFEmrpZV2UtccPauD+vOQgABBaYCAABIYcA== Date: Wed, 25 Aug 2021 15:15:04 +0000 Message-ID: References: <20210819081110.1612205-1-kraxel@redhat.com> <20210819081110.1612205-3-kraxel@redhat.com> <20210825131331.5mh76tk5vntapwbj@sirius.home.kraxel.org> In-Reply-To: <20210825131331.5mh76tk5vntapwbj@sirius.home.kraxel.org> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-version: 11.5.1.3 dlp-product: dlpe-windows dlp-reaction: no-action authentication-results: redhat.com; dkim=none (message not signed) header.d=none;redhat.com; dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 3358a857-bf29-45d7-5cfe-08d967db1dd2 x-ms-traffictypediagnostic: PH0PR11MB5173: 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: iSXzJVVisumjRhRDvCy7vkoXx3HVowPp6lS6XfReyKKCDETzbD8sxxoUPCa0z1krRqyZ/bc+NH5u1l+5Ctphd28qaGxyBCutpKJwu6g5dvVpsWI8vrDYw4S24CPC7a0lYJ0z75AI4P38os0obx9NhiZFelcAQl4p3Es1TO170fJoO7JbPdWc/7u8GJB/KYn63bF5aNCkaU0R9oogUXmVJLlgMwkoyra/ysHQZuwcx8dGpsC9PoFDU6idiUJzWuCNDvLs3IRduoG8rL+UbrZgsYQgEEpqCQq4Tnv0vBMyDLGc5bjlZALFBkMa+ENULDMwj/IARg917QzInJUtsYZvoBSi72ZxqL5MmhLkSPkRxDZ5m2gku8MQVTyC510VoXNlsaFpj/sg81qSng7Z5cd8aZdeoyJ+4L62jRMtdhoUxINqHOIaiw4b1ru8kB5wOTg1a96quVzYJI58RH/aLX+HYbmW7YR712yu6l0IHMiZRSPwcPaEObEfaDMF3+G/7bThfOs6xKiU4IhUZyPOfKWFF6iHxleP8Xq64TCR/JKs0C1GGt+Jkg1MAoBQNEeLZk9A6qyrppVj6ePo3dTfOtuWAQCIyi1HytKEEtdth5e7ViHuac200sVDGXhciw/LuRTT6IzzOxON2XO07H0c4pRqN6lmzpO9cvNMzNpttiT9u2KX3RFaMv2XeiGrlWcOeb/I3JzwNdYTrC2KNsQhqFlBsg== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:PH0PR11MB4885.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(39860400002)(366004)(376002)(346002)(396003)(136003)(316002)(6916009)(83380400001)(4326008)(38070700005)(5660300002)(66476007)(66446008)(8676002)(26005)(76116006)(186003)(52536014)(66556008)(86362001)(55016002)(71200400001)(7696005)(2906002)(19627235002)(33656002)(478600001)(6506007)(64756008)(8936002)(66946007)(38100700002)(9686003)(122000001)(53546011);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?BOu03G+6+TbgRorGLwxtPcQT5EU2BV1zygoyNLFm3IYys+Cs14dfS3tTAU9r?= =?us-ascii?Q?aZcEMXUSeAA8YNL3NFhd9y3MOFYQVLi/UB5B1XnYGp5flXc2l1nztyXYXUL2?= =?us-ascii?Q?WZhOlFmN9cCaS9B87hc1ud9xJ2+LKVqW+dOJiRA//mgxvFi6paZyPmJXlRu5?= =?us-ascii?Q?3FPX2gzMpkj7r7mgdQXSRRk8gaKWQScDVJMK5qOXy+Q70Kw+fHCIgJPSul9b?= =?us-ascii?Q?VCxblA79xUzo6Nxfgwoar4fKVt1+GCOITFcmVfzlpT5rHYvtgj8t+FIyDABv?= =?us-ascii?Q?u/k59Ndth/pux8qD8cH+ATM3ccL1tpVQvdujLSenvvZDmpTszwx7782QfbF8?= =?us-ascii?Q?5ap8DMUJnQIDlDAP0NU1HxjgZyzwVYJXN0lwkqvSCrGBhkgC+qf21lvGjk20?= =?us-ascii?Q?yzESy2wvjHmaxmr5XLkFCgTAUvPF8ItTIPRtkmx/lbyg3+7NKklWyARxkbDn?= =?us-ascii?Q?HyeRT0YDv8n/OopsQitsHLqKdZlW9Rej59inzjhHn5P8TPVzFUCeZz4AovvS?= =?us-ascii?Q?dbypKrtOeUG6W0FDvINfH1dfTGwilvrcSMTPfi2FvaY7EqWKLh5HG8dFXCv8?= =?us-ascii?Q?PuJ/ayyTe9jKX1Mj+KDTQuQN2KzUanP86FRJOrtNvHAmvsw/ixLqquXFAL2I?= =?us-ascii?Q?z1lgfKeHqdeSCtcm5/3oBx5c2viGSHLHyBTeyBIjjyYf0EWyWV6kNE0d0OfA?= =?us-ascii?Q?8BxrTeold+QNyo3+G9+9qbPtk4x/tfowQGLeRoBVHhDdbjmeSEUZ1pfmWKfh?= =?us-ascii?Q?LpHetAanzlwfeupIcdIqBlxiDRoWyO6K6oMR6PplfEvIsxkOOgk4D+18xHt6?= =?us-ascii?Q?8lOLDji+p4hnm4otOwITNstLJzdYNgnwem26pkSVOhDRonR9ll6CIwpWLCWz?= =?us-ascii?Q?i/EparaCHlJsIz5QZCMUQ2oRfAX6J1oRXPGc4drj9F4cHQaGhNR01H0KdoaV?= =?us-ascii?Q?hv1/M2gMoC8KtA4J8R9LCLAYNTLw4AlQn+4VUo//qozCc/JlOCh7hoYuJIKE?= =?us-ascii?Q?PzVdZvo7RafYztY79siGZisYtsE5u34yCZXAOd3dMjoZsrURyeeIZU5B3Yqq?= =?us-ascii?Q?RxYf+COFPkOAjTXLwNz44jDrHscy5JTZ62WULQVBkVzNYXUQV/DPkFRmINBi?= =?us-ascii?Q?ntKuQimDtIXEWgQVEtRERknbnrZkddqD38Enr1L7GPPq/mwd6382F0WrUt/x?= =?us-ascii?Q?YsvHjSJEGk45KOiVWEFXEIbfIKsOHM3y3DpV8gWz2Rr2QYwrddxNduPOvaeZ?= =?us-ascii?Q?LcvAvoifWJpCYjBP4Hn6J0+lqzFxxMaGjdAtuVUG+XKXlBd5Z/fET2jivh2a?= =?us-ascii?Q?m8ENATelueWJLMp3i24NlCsk?= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4885.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 3358a857-bf29-45d7-5cfe-08d967db1dd2 X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Aug 2021 15:15:04.4922 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: elQ8k/f1xqEPYjkuqKSepjPMBGjwpET9/5jlq6h04TMSasVGJSFEbzroaMNN3oY4v5wUP4znEgm6tfDPjNDReA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5173 Return-Path: jiewen.yao@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks for the detail info. We do have process for handle compatibility. My recommendation is: 1) Please send out RFC email about removing CMOS support in QEMU. To see if someone still need support old version before qemu version 1.7 (r= eleased Nov 2013). 2) Let's wait for 1 WW. 3) If no people need this, we can file a bugzilla to remove CMOS. Then we c= an follow up to submit patch to remove. Please help me understand another thing: Is that any plan in QEMU to *remov= e* CMOS interface ? That could be a good indicator to remove the problem - we have two ways to = get one data. I am worried the logic below to add "LowerMemorySize > 0". =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D GetSystemMemorySizeBelow4gb() { EFI_STATUS Status; UINT64 LowerMemorySize; UINT8 Cmos0x34; UINT8 Cmos0x35; Status =3D ScanOrAdd64BitE820Ram (FALSE, &LowerMemorySize, NULL); if (Status =3D=3D EFI_SUCCESS || LowerMemorySize > 0) { return LowerMemorySize; } ... } EFI_STATUS ScanOrAdd64BitE820Ram ( IN BOOLEAN AddHighHob, OUT UINT64 *LowMemory OPTIONAL, OUT UINT64 *MaxAddress OPTIONAL ) { EFI_STATUS Status; FIRMWARE_CONFIG_ITEM FwCfgItem; UINTN FwCfgSize; EFI_E820_ENTRY64 E820Entry; UINTN Processed; Status =3D QemuFwCfgFindFile ("etc/e820", &FwCfgItem, &FwCfgSize); if (EFI_ERROR (Status)) { return Status; } if (FwCfgSize % sizeof E820Entry !=3D 0) { return EFI_PROTOCOL_ERROR; } if (LowMemory !=3D NULL) { *LowMemory =3D 0; } ... } =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D In GetSystemMemorySizeBelow4gb (), we return LowerMemorySize, if Status is = SUCCESS, or (Status =3D=3D ERROR && LowerMemorySize > 0) However, the LowerMemorySize is unitialized data in stack. It may be a garb= age. In ScanOrAdd64BitE820Ram(), the LowerMemorySize is initialized after the ER= ROR is returned. That means, if the Status is ERROR, we have still a big chance to return Lo= werMemorySize > 0 case, if the LowerMemorySize is garbage. Would you please also share the info, what kind of test you have run for th= at code? Have you tried the E820 fail case? Thank you Yao Jiewen > -----Original Message----- > From: kraxel@redhat.com > Sent: Wednesday, August 25, 2021 9:14 PM > To: Yao, Jiewen > Cc: devel@edk2.groups.io > Subject: Re: [edk2-devel] [PATCH 2/2] OvmfPkg/PlatformPei: prefer etc/e82= 0 > for memory detection >=20 > On Wed, Aug 25, 2021 at 09:24:43AM +0000, Yao, Jiewen wrote: > > Hi > > Would you please follow EDKII process? > > 1) File an EDKII Bugzilla. >=20 > Ok, will do. >=20 > > 2) CC all reviewers in OVMF package. >=20 > Is there some way to automate this? >=20 > I see there is BaseTools/Scripts/GetMaintainer.py, but the script wants > a commit hash not a patch file as argument, so I can't hook it into 'git > send-email'. >=20 > > Please also describe what the reason of change, what is the benefit we > > can get from the change? > > > > I just feel it is confusing. Previously, the data is consistent from > > CMOS. Now, we have two ways to get one data from different sources. >=20 > It is *not* consistent from CMOS. CMOS is only used for memory below 4G > whereas the etc/e820 fw_cfg file is used for memory above 4G. So this > patch actually makes things more consistent. >=20 > > Please help me understand: > > > > A) What if the data are different from different source? > > > > B) Why we choose to trust E820 at first, the CMOS? Not verse versa. >=20 > e820 is the newer and more capable interface, specifically cmos can > handle at most 1TB of memory (which is the reason why e820 is already > used to detect high memory). >=20 > > C) If we trust E820 (in B), then why we need go back to CMOS, if > LowMemorySize is 0? >=20 > Backward compatibility with old qemu versions. etc/e820 is available > in qemu version 1.7 (released Nov 2013) and newer. >=20 > Does OVMF have any policy for backward compatibility? If breaking > compatibility with qemu versions that old is acceptable I happily > delete any CMOS access from qemu PlatformPei and throw an assert() > instead. >=20 > take care, > Gerd