From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mx.groups.io with SMTP id smtpd.web08.2565.1646183164505484604 for ; Tue, 01 Mar 2022 17:06:04 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=mgYqPBh+; spf=pass (domain: intel.com, ip: 192.55.52.136, mailfrom: min.m.xu@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1646183164; x=1677719164; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=Nb/AQ0xn7kx5MlD77w4KgNt5Ly0zebjbCB5Q5LL9wuM=; b=mgYqPBh+8wiz4/cCMKDbr4P1X7NtxWtvY3K/RcDrYY2hDce2AcEyspbG hRULDUBVIMHRBYq4OOzIzXaEH2biPEV5S7sVMk55L2h7pmIuocy2+PxRJ cfyInwOvGZul0Ql1Wsb5KCmeT8wIxAXcECYnNC3MeAQ8Ca8dufWT/wrVI il5ewzi6K5Ye5KG70dxjBoZX41rpH/NWFJuZdNNMHcCpiH9aAfhJsYeF+ jap3oZIoXjuAn9hY/D0QTE7GzoHa3ldK8Yxs/OHAdBRvhgc4uv8QrACsK 7rKd61SCthsWvCi9l6Jom3hNftralmDT9l2qKia7kW3cFpbYaPef/s/pk Q==; X-IronPort-AV: E=McAfee;i="6200,9189,10273"; a="233247954" X-IronPort-AV: E=Sophos;i="5.90,146,1643702400"; d="scan'208";a="233247954" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Mar 2022 17:06:03 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.90,146,1643702400"; d="scan'208";a="593821514" Received: from fmsmsx604.amr.corp.intel.com ([10.18.126.84]) by fmsmga008.fm.intel.com with ESMTP; 01 Mar 2022 17:06:03 -0800 Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.21; Tue, 1 Mar 2022 17:06:03 -0800 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.21 via Frontend Transport; Tue, 1 Mar 2022 17:06:03 -0800 Received: from NAM02-BN1-obe.outbound.protection.outlook.com (104.47.51.42) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2308.20; Tue, 1 Mar 2022 17:06:02 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=L96G1Z/NHaWJPNQvH8/+/t3FtD0qmRSLo1iHBNUa0MQP9y8nJA/Tb/iLm7048RLEiGVIg72V2hjJzHX/xTc+kDpAhRwJ65vuU7BLZuSB1kNhfbi1Dmw8l4SidUV/YNw8Zatjftv4cwcg113uyNWAcderkITOQYLg8sfI9GX8+rCayyBbwIcxBjB2Igpi5ZZtdy6O8YuJShDpMfoFXX94Jdytw4nzfDF1sG/0IhRla8EajyIxNHhAjwnXq+FU7sr+HwFF3kMQfGowwBwQocSDsKypZv/HJuzbYeye8cua7NasJBEJl7ffbHR9YhEjx/9m6LeB263ANcLt4ZIKo01rjA== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=lgZcLA6le6GQKZ0DhbQlqbmv4thgA2tjhpOZkoDUySM=; b=ftCuWzfE0sgVkppMkJ+xlwNT8m67hoQcJ5wIiLKeuNlsjL8i6VRYRTIOZrQ4FgNrmTmwT6ZbP49tdSq6C39ZFGeG585nA6cxaQwuawZ5VPfWjAJ9ji4Hka6o3lFTmvstV8Z+TAsgQXN2oQvknt74vIMOIh+kVRmQv5/xuGofxOnROKjN8oAWm8nDQ9Fonf9SRc8FSCmsMMFRespL4fLJo0liY6f5443+22xElMJERRlUYlyCnD3U2H8n2ExKY7rXaU0mAS8Wy6VRvi0jCwVNj8K9CDNXvYhy8g9re5J6nH6/rmQKJKnXBZtPFGk05XMwIB9Dw6ygMj2yg0l/nJEvzQ== 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 Received: from PH0PR11MB5064.namprd11.prod.outlook.com (2603:10b6:510:3b::15) by CY4PR11MB2007.namprd11.prod.outlook.com (2603:10b6:903:30::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5017.26; Wed, 2 Mar 2022 01:05:58 +0000 Received: from PH0PR11MB5064.namprd11.prod.outlook.com ([fe80::98f5:edb6:aee6:6886]) by PH0PR11MB5064.namprd11.prod.outlook.com ([fe80::98f5:edb6:aee6:6886%7]) with mapi id 15.20.5038.014; Wed, 2 Mar 2022 01:05:58 +0000 From: "Min Xu" To: Gerd Hoffmann CC: "devel@edk2.groups.io" , Ard Biesheuvel , "Justen, Jordan L" , Brijesh Singh , "Aktas, Erdem" , James Bottomley , "Yao, Jiewen" , Tom Lendacky Subject: Re: [PATCH V7 19/37] OvmfPkg/PlatformInitLib: Add memory functions Thread-Topic: [PATCH V7 19/37] OvmfPkg/PlatformInitLib: Add memory functions Thread-Index: AQHYLHPwm2Ef7wesBUuvgjNVu4Z5SayqglGAgADDA/A= Date: Wed, 2 Mar 2022 01:05:58 +0000 Message-ID: References: <46e0f662050779ec3ce3caf17196403266e73269.1646031164.git.min.m.xu@intel.com> <20220301130941.dw6cdc5nvh7c63u2@sirius.home.kraxel.org> In-Reply-To: <20220301130941.dw6cdc5nvh7c63u2@sirius.home.kraxel.org> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.6.401.20 authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: e37a194f-98f7-452c-822e-08d9fbe8cfaf x-ms-traffictypediagnostic: CY4PR11MB2007:EE_ x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-microsoft-antispam-prvs: x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 8XVN82XKK2AIjWypwCzTc26RvXIjCGW9Nnp9lFA01sKzRYX5abxuFlDQe0IW/AhCPzTDvBxKw2oSUT1vVYryQN1HZSaLL2c6UENIl/ZQvi7OwNT51j2PFN9hTtTYixMEsiElYoIeO9aGtV1pVIcwCOxypIJfq533Bkoly+pkgswraZG7VHLIRUXyeajebaq6kdGbiVAMyEyLBsrdP9l0aoFhO8LXDxyGoyng9FcMUMg2Uv9fr6eYIjI6QlCA0RYIwaBEdVjfPwZ9oZY5lHxxT8MqYSunlFKGqDT0NsE0Ll1o7c76AgwRPWpW3zRkXhRG2MekPms1Fzqnle6AQClG6a+1qpVtomBBzn1s6Li25QCv+tPemMoJJdfEyUiSrTm/9jVKGFZYShnwzbmvqPigbyGC0b1OfanMinzhvptbZnRr+T8MRZ/3QfeSyVkdTEWQdQWVyNxCpmIYTqYGhGSCFhQsJsbQkhhxn0n7lhVJzLSWtAFZwaEHnMyC+Nk4tFlfoSwYr9US4NyOhpAbypXtJxHQKpBelufkE+GxWgOAA1/aRNvOZWIwbz1edOQxjIxUkH20kzcfSpDL28u/nu7sRFqTT7K+DrzyYALWgKELVNctDHVOUUqhQiv685eoBLM1SST8PdQpNUr39t+MQrE2pomZCAUFiukPI/jdiNMG5SNB0KCXbRQh+xSsBVmxCy+989hltCsBwnVqfCLMJBvW/uqhLq9NXVVNdAqzl4uGxEfDvvxm3DVwtmQ9RrjxNAAzmdMD7NaWeiP/0a+ffVAWARsB4THwzJHdjaUwkYKvg1LBsE+2mAY+TNIyNHkxmseNWKdFAEJ5iiEWIq2h8gjO6g== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:PH0PR11MB5064.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230001)(366004)(38100700002)(71200400001)(122000001)(83380400001)(76116006)(26005)(82960400001)(66556008)(186003)(52536014)(2906002)(8936002)(8676002)(4326008)(33656002)(55016003)(5660300002)(966005)(7696005)(6916009)(316002)(66946007)(86362001)(19627235002)(6506007)(66446008)(64756008)(54906003)(508600001)(38070700005)(66476007)(9686003);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?1MTy8Vb3x8T5hggqK29A6wl7aHrh9g823amrghSk8NpRVbtG4DfEx+pl7q7v?= =?us-ascii?Q?9lb4Jdkq5jtflZBfPgxkZ5+INRtxUnDr4akXeeGUgUjnB5TCWUkk5w9QuNxX?= =?us-ascii?Q?NXU/VIbJy31T4v9bKF5Rdoe07XangoivkU0bLRi7R7uAHmzhlSijWTjZzePa?= =?us-ascii?Q?FIEQhqWclcYLujJXK50qA2r8vA8Yc5Z2ck5fZ1ZpXpArKpatsuUiD/r7h2Ty?= =?us-ascii?Q?9C57L+olApmNylO9QaBaUdAco72CaNlsSz98TRwlmPKQrql37Z5PR4GrK9Br?= =?us-ascii?Q?FwHRoO2EmWKMGuJrp/3Rl8/T+fuLq5qayuP3r00JPBwtBXA8SVHpcNKlRYD2?= =?us-ascii?Q?FIK0NOOWBPiktpCBvzwq385AJWdOh90sv0kiIOcgddvwO1i/9Ul/XKXv9vzG?= =?us-ascii?Q?2vsRh2jYwy9DRZV57oQBFyAHlHAy+PQsv+ZZBy5PX4N0gKIdsT76d51Rzd1b?= =?us-ascii?Q?KKf9nN6QBMjsrdP+QAuqEnZn5Wo5xJs1el05c39O5SOzvnQKT7PaUBT6fG1E?= =?us-ascii?Q?T37ZFK1Rkd4SIEij87KMHGmD07ofonoAM2tM2FCuVmXFPC0HfdKtqrlJOcid?= =?us-ascii?Q?+/Sac/Su3w12qHCNljWT8goMZGjJ7vTQZxPA7tTceBKcjmIZ69F5bF/w+++a?= =?us-ascii?Q?QS+6Pw/UXjENQu11NjZ3BJQL8p1SOgbSKNszFj31r7tDNwO4n4+L9sPXW8ut?= =?us-ascii?Q?/WMSx8Jtj5vEpPKue2f6Z5EvzG52AbJNHuMVILkYnpbTE+E9bPS1bGJcFWc9?= =?us-ascii?Q?1JO5pIRZi90pjQK7BZxyhv6mqAhszc6qocTi06J2PmQsotmPR2kKEBTdjiRH?= =?us-ascii?Q?0wnUjr1g6kMkjYh+piD5uFKf9CoIzaXfzQ1kDnmA9U7DG+TZRfTE7tpgLhqV?= =?us-ascii?Q?Yt7dXKyBmSYloEmjZs+4u/hvPQOy9TCZ63UhLg61g/L1TPD+8dsJxfiQqXOZ?= =?us-ascii?Q?RN+yV0EA0AAtS62WfecD1BIRkI8uFITQgreeOKI07uVpL6YcDhNWAHjlJEkA?= =?us-ascii?Q?0JKlSgo1KL9FeuWcF58a8fXLpPM2kaWK5fooFAfID9xleAvbSHxG2bWeZHD7?= =?us-ascii?Q?dLBAT3urWCbTqQHQCrpYEZV/w0eIKILXQOLq6VrhZPPQ0BW3hVXMa/J3WO1D?= =?us-ascii?Q?JKQ0jbLRWyzc+rkwpOcJ3VaqkjHfmoufQUHZKvUCcOpeJXw7ZFfB9Upf1dFQ?= =?us-ascii?Q?U5GDFq7CorEoEMxFsnj7egRkF2kqxFefes4RhX/pFfjt3F9XzPgdJ2qzmW23?= =?us-ascii?Q?VkoaGaxxgJSBa6g9Oy3lQPBKsvh0Vamj/TN3C8G4iI5WmoeSLgsBMgIfNR1T?= =?us-ascii?Q?aVlu4SfMjw2Ss3M8aTBfaaTGZfWzzcniLtX8FizCScieiuf+JTIuikrQsXIT?= =?us-ascii?Q?RKw506ZAJNiR7MjlV2SLRDhVWUq2gI1TZppByv5/geDceqZBlVIl40tjIjav?= =?us-ascii?Q?Mz12NuH9p5SfO2DNRMyTDp3mUVsvHrTUf/f1eqZE/wNtF00SyTVqcTnKcZl0?= =?us-ascii?Q?yHdxoLPErJ0pSoWoxbVg0e96/cbdvZgkk6NSxa2jLNqvlAFxLycWad8+2KjE?= =?us-ascii?Q?7W5nMfXg5Tda+SRIzyqNl1+avXfpz6eU6k1L8Ng4ObP6pjHN2Xs+7S9jZVwP?= =?us-ascii?Q?aWZBpoPjCeIoACzQiYBgBuM=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5064.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: e37a194f-98f7-452c-822e-08d9fbe8cfaf X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Mar 2022 01:05:58.4380 (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: UuVnzu0CSrVgDhI1vhCfSBdCurAzS5VS2gTEH+V1KZISI7TTgjxXMRvv331YzNu8fOrEZPTqn2yVHIypBcw1Mw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR11MB2007 Return-Path: min.m.xu@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable On March 1, 2022 9:10 PM, Gerd Hoffmann wrote: > On Mon, Feb 28, 2022 at 03:20:51PM +0800, Min Xu wrote: > > Below functions are introduced in PlatformInitLib: > > - PlatformGetFirstNonAddress > > - PlatformAddressWidthInitialization > > - PlatformGetSystemMemorySizeBelow4gb > > - PlatformQemuUc32BaseInitialization > > - PlatformInitializeRamRegions > > > > They correspond to the below functions in OvmfPkg/PlatformPei: > > - GetFirstNonAddress > > - AddressWidthInitialization > > - GetSystemMemorySizeBelow4gb > > - QemuUc32BaseInitialization > > - InitializeRamRegions > > > > After that OvmfPkg/PlatformPei is refactored with this library. > > > > Note: PlatformInitLib will not determine whether SMM or S3 is > > supported or not. Instead the caller of these functions should input > > SMM / S3 support as the IN parameter by themselves. This is to reduce > > the complexity of PlatformInitLib. Another reason is that some PCDs > > cannot be declared as FixedAtBuild while PlatformInitLib is designed > > to be used in both SEC and PEI phase. >=20 > Hmm. Unlike patches 17+18 which are pure code motion (except the > function renaming but that doesn't change the workflow) this patch mixes > code changes and code moving which makes it hard to review. >=20 > It should be splitted into one (or more) patches changing the functions a= s > needed (and keeping the code in PlatformPei), and one patch moving things > over to PlatformInitLib without functional changes. Ok. Looks like #21 & #22 in tdvf_wave2.v6? https://github.com/mxu9/edk2/commit/ef0615ca5665b2058e4352a322dfa74d258f9f3= 1 https://github.com/mxu9/edk2/commit/25f356a0bf7ee347be30e270aeffe6cbd8e0b46= 4 >=20 > > +PlatformGetFirstNonAddress ( > > + OUT UINT64 *Pci64Base, > > + OUT UINT64 *Pci64Size, > > + IN UINT64 DefaultPciMmio64Size > > + ) >=20 > > +VOID > > +QemuInitializeRam ( > > + UINT32 Uc32Base, > > + UINT16 HostBridgeDevId, > > + EFI_BOOT_MODE BootMode, > > + BOOLEAN SmmSmramRequire, > > + UINT32 LowerMemorySize, > > + UINT16 Q35TsegMbytes > > + ) >=20 > > +VOID > > +EFIAPI > > +PlatformInitializeRamRegions ( > > + IN UINT32 Uc32Base, > > + IN UINT16 HostBridgeDevId, > > + IN BOOLEAN SmmSmramRequire, > > + IN EFI_BOOT_MODE BootMode, > > + IN BOOLEAN S3Supported, > > + IN UINT32 LowerMemorySize, > > + IN UINT16 Q35TsegMbytes > > + ) >=20 > I think we should add all those parameters to the PLATFORM_INFO struct, > then simply pass around a pointer to that struct instead of having tons o= f > parameters for each function. Ok. It will be updated in the next version. >=20 > Due to this patch needing an update anyway it might be easier to do it ri= ght > away instead of an incremental cleanup after merging this series. >=20 > > @@ -85,7 +87,7 @@ MemMapInitialization ( > > return; > > } > > > > - TopOfLowRam =3D GetSystemMemorySizeBelow4gb (); > > + TopOfLowRam =3D PlatformGetSystemMemorySizeBelow4gb (); This is in function MemMapInitialization(). > > PciExBarBase =3D 0; > > if (mHostBridgeDevId =3D=3D INTEL_Q35_MCH_DEVICE_ID) { > > // > > @@ -736,6 +738,11 @@ InitializePlatform ( > > Q35SmramAtDefaultSmbaseInitialization (); > > } > > > > + // > > + // Fetch the lower memory size (Below 4G) // mLowerMemorySize =3D > > + PlatformGetSystemMemorySizeBelow4gb (); This is in function InitializePlatform(). >=20 > Can't you just use TopOfLowRam here? TopOfLowRam is a local variable in function MemMapInitialization(). It cann= ot be used in function InitialziePlatform(). Thanks Min