From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web11.3466.1685581791022549933 for ; Wed, 31 May 2023 18:09:51 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=IwZ0UTLg; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: ray.ni@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1685581790; x=1717117790; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=44CvJI0XTChQn7rKLtPG08dmbDUdrfLbOGceYPbkCAg=; b=IwZ0UTLg2Yd834fb2jo1A12b0ZAIMBTq9hq4Rjx2hzZygPYNmLCd44i7 8HediG79ycQoqRNxDUGN1/02gdOvvr2qklV3rCL2Wjl5Mq7Y6xqwEpCiX ki3bEnwDi0u+gzshsKMLvKMbecFChlzn7hspg4v5NogkCLScABwtvVBOJ vW6iENCX0oBFrvsqdJ8ob2pbf9hujaxOirKtlpUGky2ybcvdYhtSddkOa MoeqedDNZ6vTzj5LzcL/tQVLr7KygkS8TThmKMHc5hQ+uHBRhVz4L+mLn 0g6opJ5mS6o/pNtzZijOtdTimDiWm28b0SUXGoR2/Lme3Y9z1rnmfSbnK A==; X-IronPort-AV: E=McAfee;i="6600,9927,10727"; a="355422720" X-IronPort-AV: E=Sophos;i="6.00,207,1681196400"; d="scan'208";a="355422720" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 May 2023 18:09:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10727"; a="701335680" X-IronPort-AV: E=Sophos;i="6.00,207,1681196400"; d="scan'208";a="701335680" Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by orsmga007.jf.intel.com with ESMTP; 31 May 2023 18:09:42 -0700 Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) 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.2507.23; Wed, 31 May 2023 18:09:42 -0700 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX611.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Wed, 31 May 2023 18:09:42 -0700 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23 via Frontend Transport; Wed, 31 May 2023 18:09:41 -0700 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.168) 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.2507.23; Wed, 31 May 2023 18:09:41 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=eIDpMCinnMV9VITM2eusBulKugXKxFB5r3Aco1dvfgqpSONqqdOWUE2Ormm1Uqm6jzgVZvuhA7Axgk8TfVffY9wJ3Whkv2uwJUFtMAhZHDSAAFfN87S/aYlVzUz1RCvAz/ccXlUpelwN7AllK3Ij/jOQ4LDwQ6/4OXpReb3qqP6ZOIuki8uV/QNtLbvFSbq2WkZMW+ZaxPz3MDF/twaIWCHS4Qlx/QlBLiOjr8wqCrodSLrqPeBazX3RbS2lFssRUQZnA+7CIQYlYDuDTZL4kLVoUs2Lb0thlHo0t25s/PZ4qSicQjxE5xGgXk1H6m8oalCczLh3pxrmOJAg+lo56g== 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=mvHwGb/qpSaox2EM4wKPzCbamLu3sJp/DFFplUnroKw=; b=kiEiJs/gG7Goy4F3/AwBWaCfOHkW9IzWb15HumNAzL4D2V5fhoAZpo8FoI9C8ujkNiUk+Qq4owvvbSKwvyCqisyT0Av19xLJUd/Rrw0JTph5Ip9xiFLQjpxwGI8SOAzFkOGZ6ghNqtMaRs5nL5sgfgFlCZJpgJ+PpNLoAzfF49hXtpYnBmhDIvo2ErD7rCZCxHBwoWg7ACtxqgSBAnN4dc82MY1gANEtP6g2n5EYnqN+0FAHGVarMcJJ2M/Zjyyzhw+zbcOtcZgXPNOgtlxezXs3hvLcboy8Cw72WjgkSSauckFos7Crvl6AdbGUS7dxbo+k+MGz6ZxWKkpHjH//Qg== 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 MN6PR11MB8244.namprd11.prod.outlook.com (2603:10b6:208:470::14) by SA3PR11MB7525.namprd11.prod.outlook.com (2603:10b6:806:31a::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6455.22; Thu, 1 Jun 2023 01:09:39 +0000 Received: from MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::892b:b8e6:bab7:635d]) by MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::892b:b8e6:bab7:635d%5]) with mapi id 15.20.6433.018; Thu, 1 Jun 2023 01:09:39 +0000 From: "Ni, Ray" To: "Tan, Dun" , "devel@edk2.groups.io" CC: "Dong, Eric" , "Kumar, Rahul R" , Gerd Hoffmann Subject: Re: [Patch V4 05/15] UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute. Thread-Topic: [Patch V4 05/15] UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute. Thread-Index: AQHZh91EyCdTUV/K+0ScbejoF+N4Va90X5Ng Date: Thu, 1 Jun 2023 01:09:39 +0000 Message-ID: References: <20230516095932.1525-1-dun.tan@intel.com> <20230516095932.1525-6-dun.tan@intel.com> In-Reply-To: <20230516095932.1525-6-dun.tan@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: MN6PR11MB8244:EE_|SA3PR11MB7525:EE_ x-ms-office365-filtering-correlation-id: 80d8afdf-c052-4218-3e5b-08db623cdfcb x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Os6UEb+/u63xc4flc3/DZXyk+DiRugj8TUkLRZZgH1xntBqT2i3zS687fDavzs33/bohnfUXtOX1U+VBYY3+bD80nNj35cBC/2nhiegtFxnibMEUSzkzXqfosaVFmrSyHdyV0AR7dN959W68f49VhqN8gFxkZhnE0c1pt+2zG7HrJ0f2r3N6bDLGCWpvgUcdKI2zkNcsoG1WNm3iq+HCBw/K/RWVOzrtGUZ5YQSRx9Cv0+U/5OHv+Sn3uwhGHSobFqwJ7B27hnzZdCqrLgw4dD9PDRnmT7u+9+VzJuCR0PLaXApKXHf53YAe7wgMzZdZlvNEYEwbE1g7uTigjPKK0+7MFPO0EZbf3iNNexX6i7hLhtTQneussaF7rYKdDTtY0txP4j5U2fvBFdImY2XC2KsN/2NVG4UeNw8EXGtxM5N/uJ0u1gIiRrkp8Qp+4m7/mJ8samRvDcIaL7OQGc9TrkZ1rUZNk9RrFulndLzq8/Ek7VaBOPOwGQpMu1ARzTI6XmWbJ02mFI3NUp2cZYZAwlTxiv6g2RQodyZ8BipFyEZTHxC9yrRSc4zhlcBMQQrbg6HqllMky4oHpwj6EF5ImxnQm1N6auTPZs+vo/zjKbTmPHChA0icVsC4i7KoKtFa x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MN6PR11MB8244.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(346002)(136003)(366004)(396003)(376002)(39860400002)(451199021)(110136005)(54906003)(19627235002)(8676002)(8936002)(66556008)(76116006)(2906002)(4326008)(66946007)(64756008)(66476007)(66446008)(316002)(41300700001)(5660300002)(52536014)(478600001)(71200400001)(7696005)(33656002)(122000001)(6506007)(9686003)(186003)(55016003)(26005)(83380400001)(86362001)(82960400001)(38070700005)(38100700002);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?aD5tK4fLWMTB1hNxkyORUWlIWQmPCCgrHf7t/CNPTY+wJxHifELCFF3fCnSg?= =?us-ascii?Q?eLs8JVi+1EJ+792QNo1z7AO/RvUvMSO5naOwukhhnjmDyCjrekQR/6BbN7YW?= =?us-ascii?Q?WwEFfvw+qhk2pLOuJszVpFNTpdn8/TRJ30obohqWwybXDCSST8AWf34OqRV/?= =?us-ascii?Q?xUcppIex7bsGSC98rbBnzvCvR6wFJmyIM4P6LVJTHY5UUSGxAj6DqyGe8ygz?= =?us-ascii?Q?HOq1itoLFsWnmDwqDoWQOuObRmIf0WRHuSuC4zx6lS0xhkKq8Wjkmab1W1Cf?= =?us-ascii?Q?WxpXmPHvtzlGrMM5+sGO+rgFnFa/V/qiqR63xoiQOapk/Nharu+KrYbHh07C?= =?us-ascii?Q?qGyVJI7U6B8Ph+MlLBgji48kdjhLmBYHIjGegLSWE3u/YriWBTslm0OAJ5Ym?= =?us-ascii?Q?jg2mjcKPtqfJbY/R4mdW+TufjdeJqW579b7TeVrTTJCVbMEni+BpMQanyThz?= =?us-ascii?Q?P7yvFMKC+pXk8YIqVUqz0u6YHfkkxRCDa0/qXlZoeJZJcd7AzWkObY/TAa5P?= =?us-ascii?Q?l/4vJk1ZQTCPfLraYioYnpJGkLWUHstXqjlFITzxFQPPpJ0+HKegZy8y5B2n?= =?us-ascii?Q?3njEBDobIumKt7a4rgQNIalKgcks2CxEdMfC8aIZ0Zf1wFHuubGN965s6d5X?= =?us-ascii?Q?Gh5KWvReysjxzUHJtWC3gTdyBg6OMIbXTdf2Qw5pGhq7jJhWM4evjZ6YFLB/?= =?us-ascii?Q?DqMMlluN9fy1SJmIy+qWudBPrIrL+j3aj2toZY6lFcX/ZWw4hZYHhSbPbnOw?= =?us-ascii?Q?8JKgvd79tlfNuEmn6cC7RWzVRt/onCueQQnQR6mAjDUqd4Z51XWu2nt7Dbhf?= =?us-ascii?Q?FixHxh2f2TKj8l+rXz7OxNNSkf1Tx3HosonJMtwMPkycUNlAe3FrXYHI6KIE?= =?us-ascii?Q?mn/r6gwETlh9PfXMNZdVQhTR0HCHdlGUuJB8yVfWkQwVhr/UdiIEP84fgGe5?= =?us-ascii?Q?QoeAVXgsA3dPu3vAwbAqr24H+tb9j7MXwin+J4jQQOX8yN3c5oOV1cYB6gQ0?= =?us-ascii?Q?4K9SIUkTffnlrfvsXr3u8kSYgrACnwDvI3gY4L4VsZrsBQu6/CHkcJ+pPs2G?= =?us-ascii?Q?PdD28196tY+SO5XEUNybP58lgNDIXbUtnVfaTLnQpLHLkjreR/Dk+OIioV2E?= =?us-ascii?Q?2sWe/UlHy2M7x4TDk4TVLPon1nDPrnrvE38UDS/0l1mfQ+IbF8vvRWb+e1f+?= =?us-ascii?Q?yISRlPVJxvDpAg4hBHEwasByQ3P0XAIpZgRpNrEymJSugxRdUqCd3ZyPbAcr?= =?us-ascii?Q?yIooaxLEG5ZNDnYNY2UkKzrQA7acRv7ffsLp0sLWHD0EOLIZzLYPVZ7mtzZH?= =?us-ascii?Q?fUWslyVTfTU5wPpXzPmJYjl50nQVG+QEcN2GbEJ8kZL3lxyzALNwRLspwisW?= =?us-ascii?Q?C9fI8i3mJ6n1CeYvaojFdc5Gukx6MsBZ9pBb4rxm97iNQ/ORGHcfxW9oVkd/?= =?us-ascii?Q?AmIGP+5fee7krv1mn9akwDrNcaOIePUFQ+WOZxId6cB4TAOXa+5W965Wso7p?= =?us-ascii?Q?Kl/tIReOBoEc02dPDQXdN2rIABxc68RT6qblgAOkIzSuu7+hKNlZJh994/i0?= =?us-ascii?Q?T1vvHzL7jO/A7MXgksk=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MN6PR11MB8244.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 80d8afdf-c052-4218-3e5b-08db623cdfcb X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Jun 2023 01:09:39.4915 (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: dEbKFBIAJwKT3mbuHPtW2dzkFSREWVwOZUOBYTXq3658c0tO4XDACQWJZfsmtQW9gwQsOOpe7oJshqQssWX5xQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR11MB7525 Return-Path: ray.ni@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index 3deb1ffd67..a25a96f68c 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -1,7 +1,7 @@ > /** @file > Page Fault (#PF) handler for X64 processors >=20 > -Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.
> +Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.
> Copyright (c) 2017, AMD Incorporated. All rights reserved.
>=20 > SPDX-License-Identifier: BSD-2-Clause-Patent > @@ -353,7 +353,12 @@ SmmInitPageTable ( > m1GPageTableSupport =3D Is1GPageSupport (); > m5LevelPagingNeeded =3D Is5LevelPagingNeeded (); > mPhysicalAddressBits =3D CalculateMaximumSupportAddress (); > - PatchInstructionX86 (gPatch5LevelPagingNeeded, m5LevelPagingNeeded, > 1); > + if (m5LevelPagingNeeded) { > + mPagingMode =3D m1GPageTableSupport ? Paging5Level1GB : Paging5Level= ; > + PatchInstructionX86 (gPatch5LevelPagingNeeded, TRUE, 1); 1. this change assumes the default value in assembly is 0 while old logic d= oesn't have such assumption. Can you patch the instruction no matter m5LevelPaging= Needed? > + DEBUG_CODE ( > + if (((Attributes & EFI_MEMORY_RO) =3D=3D 0) || (((Attributes & > EFI_MEMORY_XP) =3D=3D 0) && (mXdSupported))) { > + // > + // When mapping a range to present and EFI_MEMORY_RO or > EFI_MEMORY_XP is not specificed, > + // check if [BaseAddress, BaseAddress + Length] contains present= range. > + // Existing Present range in [BaseAddress, BaseAddress + Length]= is set to > NX disable and ReadOnly. > + // > + Count =3D 0; > + Map =3D NULL; > + Status =3D PageTableParse (PageTableBase, mPagingMode, NULL, &Co= unt); > + while (Status =3D=3D RETURN_BUFFER_TOO_SMALL) { > + if (Map !=3D NULL) { > + FreePool (Map); > + } >=20 > - if (IsModified !=3D NULL) { > - *IsModified =3D TRUE; > + Map =3D AllocatePool (Count * sizeof (IA32_MAP_ENTRY)); > + ASSERT (Map !=3D NULL); > + Status =3D PageTableParse (PageTableBase, mPagingMode, Map, &C= ount); > + } > + > + ASSERT_RETURN_ERROR (Status); > + > + for (Index =3D 0; Index < Count; Index++) { > + if ((BaseAddress < Map[Index].LinearAddress + > + Map[Index].Length) && (BaseAddress + Length > > Map[Index].LinearAddress)) > + { > + DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes: > Existing Present range in [0x%lx, 0x%lx] is set to NX disable and ReadOnl= y\n", > BaseAddress, BaseAddress + Length)); > + break; > + } > + } > + > + FreePool (Map); > } 2. What's the purpose of the above DEBUG_CODE()? Because when mapping a range of memory from not-present to present, the function clears all other attributes but only set the "present" bit. If part of the range is "present" already, the function might reset its other attributes. This is not expected by caller. So, you want to notify caller? Can you split this logic to a separate commit? If the sub-range's attributes match to what you are going to set for the entire range, caller can ignore such error message, right? >=20 > - // > - // Just split current page > - // Convert success in next around > - // > + ); > } > } >=20 > + if (PagingAttrMask.Uint64 =3D=3D 0) { > + return RETURN_SUCCESS; > + } > + > + PageTableBufferSize =3D 0; > + Status =3D PageTableMap (&PageTableBase, PagingMode, NULL= , > &PageTableBufferSize, BaseAddress, Length, &PagingAttribute, > &PagingAttrMask, IsModified); > + > + if (Status =3D=3D RETURN_BUFFER_TOO_SMALL) { > + PageTableBuffer =3D AllocatePageTableMemory (EFI_SIZE_TO_PAGES > (PageTableBufferSize)); > + ASSERT (PageTableBuffer !=3D NULL); > + Status =3D PageTableMap (&PageTableBase, PagingMode, PageTableBuffer= , > &PageTableBufferSize, BaseAddress, Length, &PagingAttribute, > &PagingAttrMask, IsModified); > + } > + > + if (Status =3D=3D RETURN_INVALID_PARAMETER) { > + // > + // The only reason that PageTableMap returns > RETURN_INVALID_PARAMETER here is to modify other attributes > + // of a non-present range but remains the non-present range still as= non- > present. > + // > + DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes: Non- > present range in [0x%lx, 0x%lx] needs to be removed\n", BaseAddress, > BaseAddress + Length)); 3. Don't quite understand. Can you describe in a clearer way?