From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mx.groups.io with SMTP id smtpd.web10.11170.1683705561855693700 for ; Wed, 10 May 2023 00:59:22 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=HURNwT1Q; spf=pass (domain: intel.com, ip: 134.134.136.65, 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=1683705561; x=1715241561; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=XDgcS/EA9jAT2ut36+TfSTHnfiQYvm5xlPv2jNsan60=; b=HURNwT1QmyZumJKb9PKduFI9I4kkD/YvLXKqAXD4S0nSUg1eDt9yTjMV 6fRK9lALX2gGMKhQdxC/+xFuHWUEAgIeNfroignzvox6IhOm2PYK0ax1s v2lhBi7ubd2yKfCfg21khgKqI0JBlZu9k6QVnTDUM/He2nHoTt3XvUTY5 dro6AMzPiWCkAzYwXE9HT+uVYCruJ9j3k9XOKs+UFN/p6/rvUQArcgKRM VcK0OLbG5NqhnLaakCgNK+s8v6m4JPXwMzQbn70juTdSeJLBV3LdSg+xj laEEke6iouIyTneQmj0ynTnVTRM+iufKgCgeKLtFt2t0Bm/xwhF0y3Y80 Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10705"; a="353222752" X-IronPort-AV: E=Sophos;i="5.99,264,1677571200"; d="scan'208";a="353222752" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 May 2023 00:59:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10705"; a="823453220" X-IronPort-AV: E=Sophos;i="5.99,264,1677571200"; d="scan'208";a="823453220" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by orsmga004.jf.intel.com with ESMTP; 10 May 2023 00:59:21 -0700 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) 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.2507.23; Wed, 10 May 2023 00:59:20 -0700 Received: from FMSEDG603.ED.cps.intel.com (10.1.192.133) by fmsmsx610.amr.corp.intel.com (10.18.126.90) 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, 10 May 2023 00:59:20 -0700 Received: from NAM04-BN8-obe.outbound.protection.outlook.com (104.47.74.45) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.23; Wed, 10 May 2023 00:59:20 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=oQla64J0M7E48H3iuIY8T0c+VE2plnpyAJlGJSVni1e9m+bmCr59N1mo5XAq5w2IJyBIR9RxsmfKVW8zYJUveVrmSUStwCtFmwdmQi2x/Up/gRlNEiTMQ6XaFB2bIMpMo+beofisnG6aGY8Y68UfMlC3nFIu3MxhRqHPXKd3+SNX6hMbuQ/9w9A+Y0ygOpIZQ6lsX4vfMH772KbvkB939W5mAKC58Id+BooB8kuONPsPxOXOdCDwLPKLx2gXJv45z9TaRvCO4VRX8avpGDbVmACqW1v6cgiEw2q8ZTXsmxU1XlvI+dDq7unVIBjrweeTDvDcnQZZ0jbHr7/Exv7tmw== 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=d42Vy8A0R836WUA+prKiPj9W/3R1RhRmyT3cXof3nKQ=; b=L3/Unz+8FYEtekCJ+yXGZNQXeFIZXSgp94PZBFPvqdflGJQuhV86a2fjimgGTEkon1DtG0q7XTpHu9cxzZ9P6jHnlSKINj7bQe0urdHdnfWC8iDS+RDnpch0SwvTzfhXk//UKhD4LClIsJW7TBBlTRg37B1xRctg7F4tSgvRazboVqPcsZGqOf2vv+REX47TqCPVaaX0bzTyQNBlqJlAbyF2SIxdlswpmucpsSvbqfMq3LRu5i/MVgVh/vEbqakT00hJ48X9QfuGDPU1YvF4drmlrdInAoZISsO8y87HnKy0HimtnC8pDX6kkOzCNXZLhByjWQytRPffBjRnGmPFvg== 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 PH7PR11MB6029.namprd11.prod.outlook.com (2603:10b6:510:1d0::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6363.32; Wed, 10 May 2023 07:59:17 +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.6363.032; Wed, 10 May 2023 07:59:17 +0000 From: "Ni, Ray" To: "Wu, Jiaxin" , "devel@edk2.groups.io" CC: "Dong, Eric" , "Zeng, Star" , Gerd Hoffmann , "Kumar, Rahul R" Subject: Re: [PATCH v1 2/3] UefiCpuPkg/CpuMpPei: Enable PAE page table if CR0.PG is not set Thread-Topic: [PATCH v1 2/3] UefiCpuPkg/CpuMpPei: Enable PAE page table if CR0.PG is not set Thread-Index: AQHZgmBCxJjjFrJjdkGddQNkth9Opq9TJAJQ Date: Wed, 10 May 2023 07:59:17 +0000 Message-ID: References: <20230509102253.16632-1-jiaxin.wu@intel.com> <20230509102253.16632-3-jiaxin.wu@intel.com> In-Reply-To: <20230509102253.16632-3-jiaxin.wu@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_|PH7PR11MB6029:EE_ x-ms-office365-filtering-correlation-id: 8284f4b8-54e8-46ce-966e-08db512c746a x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: eCYRuJffEGvEZjqF390utOJrin6diW2I8CpW3/uYcUlv7z47PqWQc/ilc2JzHtKXDhcT9Aevlaum/eMmOdEU9zdG3XhaWuWstwv0RM+ATsnluTdai713Cwhbd1kr1JyyUcmDqYFVDqVyjGu996Q/SB+ehd+9WzLL6e3khQVCSgq1R/0gL67W7IMp8QyWVAOBL4skm8zD8eVFp/fbFR+X9VTxYm07D2dVRneHhAaqUrstVW5zFV1QihKQtrEkK5NlEiY9cpQf9MmCQDHIPLzmNx0gTiY3cL9nk8uVpBe6QaDf4eWGf91Qw0NInFg0b/Scuh3NxyuVA87BAmLCKE73T90heTsNZrF9p1gusynWKDAnTfxq7yzz8VrGzY9MO1ZGMXMrvwGEkCpw6yvBYMf/4l6ukJq6KIDITqnBJDLVG+XWjJVL9tNldV3UT+/RMuWIMlWK6lFY1t6gvxEBYfqm/ThacQqjnte9IEJpqE+Hx+5p9hzYeDVAmoamilhFL0aVDW2CLkAXP8jYnMpU1ODPVD3O0sHni0+QVwGuSXMxAi4eT2NGKnOQW/FeS8tQlD2Qo/3Kopj/fVelNeR5z2WJG9A7QbuvS+K4vEx44GlLzXf0Yc7PKLJpm4W39S4jj7Bj 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)(6029001)(39860400002)(396003)(346002)(366004)(136003)(376002)(451199021)(53546011)(33656002)(66476007)(7696005)(4326008)(66446008)(19627235002)(110136005)(54906003)(316002)(76116006)(66946007)(86362001)(66556008)(478600001)(64756008)(55016003)(5660300002)(52536014)(30864003)(8936002)(41300700001)(71200400001)(8676002)(186003)(82960400001)(122000001)(38100700002)(38070700005)(2906002)(6506007)(107886003)(83380400001)(9686003)(26005);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?25Dj1DH/Mt2+4qszewuuR2M4CGllso/WerP6mfTpK00vockl3KcXycEfFlng?= =?us-ascii?Q?Wc1j21sWVstFtPlrjlumJp+dN1n7kacvtjUvNvqOQmjHOx1qeRtR6FWKYI7b?= =?us-ascii?Q?yTlLOVyAecxUT65XDzAoh4gID4Zde97AU2dYINXkMCRSfrh5SXSiWjFWQ3K5?= =?us-ascii?Q?0QZywfQf+Rt2TNTxb78rUOu3yXtYqm9jltrSvsT3WhiXKu0QreX+5pEsfH2t?= =?us-ascii?Q?IR1JkTJ/wTir+FXG80pnfHJ3w4uAkVO3YmD3SqtBFNcH9Eu+pVnw/GA/2iEg?= =?us-ascii?Q?B47fyfWO645UJhzdiPfxugoXMNYpNufbdZzJCPtsv9aJn6i9dxeoB/dtWJb9?= =?us-ascii?Q?vMqv8gbkWP2PXAenZcT9X0K75tit6+ou4wCjpGgGftGXjuPurBL37SgA1DMs?= =?us-ascii?Q?/VfFt8PmfGrOebxL5EQIXSw8vejdtbqmqx/LBKwJpZsbSEkwy6zirzxveAXq?= =?us-ascii?Q?AZ+g7qXm7G4/OegUoil64RM0Kka47BeZwSKD8PeZAG7RAXnnVe/TweslbaA0?= =?us-ascii?Q?d0QyfA2lvxEUwxqFrVkXbCwjwtjacj38sAQ9Yv3Wbr+WejtP+/4oRkn3Rfkk?= =?us-ascii?Q?vISxpBcvZOr7KgLqnJJaskSuoXkkIBEtxOFnF1gVqjxnqQy9+kl+zAxevT4U?= =?us-ascii?Q?k3CQX+ffiRturyb7adhwrAoB6nieYZ38+8pSUjU5s2S+6G3HgEGobJ4ssWYW?= =?us-ascii?Q?CVyLpyvlTCWj3MHqcrWF7VPwTLXwAMAamvhp2zqxDxwsAfWCIZGZdhmwtDF0?= =?us-ascii?Q?gpN4B50Ga96mW+Djo3wr1IKjMYK2oDIWxNvY6E2QaMQ9r0L0nUdoInfBtobt?= =?us-ascii?Q?IM2iyltfITj/wudRI+du3av+7/t91Cd7EHMPXC6UxCnCb1v1ZgfYj42xOcyy?= =?us-ascii?Q?9uAwYrvtdQ8BQb5RGY1QC7G9edliqyajTg+8NwYRwQ6w4ycpVrtr6iWa80O7?= =?us-ascii?Q?le9qnMTOxU5UeFUkYrnOWDhCT2LtRMpqqvNY/eBLVCT/uU6t9bvuNXa4FIP+?= =?us-ascii?Q?LO+vZOxC4bipsUABIk6yQLYoevB8gpdMN56KPj2N1q6/4padg1b504BXChN0?= =?us-ascii?Q?auyZHGVDM67UDeZlzLX0I1S8s3yKcR1oKUJKHB62CzpwU3FR8kof9gS99L+q?= =?us-ascii?Q?b4suN3PdJWlpq35BY9Yvpyg1ijyz37VW710A4XkNz+9tNbSfg/1tXrxCbYF8?= =?us-ascii?Q?18TOmpc3VXJYW9d/lEcHtyOPG/wlNYVBcZFKfJCf+aq068YhjsFfx8lnqfFq?= =?us-ascii?Q?faIc6z+D0xqzbgsN5IM6Q+OwnDWQUaaUYUseXQnviZCFga4SvUwMTxMB4Bow?= =?us-ascii?Q?NdmKQMpHW2KmtR6Yi2lu5CwdVjsoCiixBNxKAfKm4zM+3XfPDkheJLPtLH2Z?= =?us-ascii?Q?yXVVia/jwncuz0WTIuo6ARzfbDKSGbBcVD79YC+KOK6SPSaCDmLAiJqSX90i?= =?us-ascii?Q?vYRU8AZHjGinePyFhDaCYyWJo0b8e67mOWrWbpyWTEkEz7reTIWaGcltP0hW?= =?us-ascii?Q?k1tZn37Sg1Uh5XogoMVFg2QLXtt2BaBSRwIBfTj66JvuaGNBcB1C9OacjWBB?= =?us-ascii?Q?itg/GLcnSoVfD9c45Mw=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: 8284f4b8-54e8-46ce-966e-08db512c746a X-MS-Exchange-CrossTenant-originalarrivaltime: 10 May 2023 07:59:17.6500 (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: o4uz9g5L19T3dno4rknknKJeLwRvXr8w88ze76NpRVmJTimPVNI4DkfThyFscj/m/gmJ2q+m/RQZNY2sp4uPxw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6029 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 > -----Original Message----- > From: Wu, Jiaxin > Sent: Tuesday, May 9, 2023 6:23 PM > To: devel@edk2.groups.io > Cc: Dong, Eric ; Ni, Ray ; Zeng, S= tar > ; Gerd Hoffmann ; Kumar, Rahul R > > Subject: [PATCH v1 2/3] UefiCpuPkg/CpuMpPei: Enable PAE page table if CR0= .PG > is not set >=20 > Some security features depends on the page table enabling. So, This patch > is to enable the page table if page table has not been enabled during the > transition from Temporary RAM to Permanent RAM. >=20 > Note: If page table is not enabled before this point, which means the sys= tem > IA-32e Mode is not activated. Because on Intel 64 processors, IA-32e Mode > operation requires physical address extensions with 4 or 5 levels of enha= nced > paging structures (see Section 4.5, "4 - Level Paging and 5 -Level Paging= " > and Section 9.8, "Initializing IA-32e Mode"). So, just enable PAE page ta= ble > if CR0.PG is not set. >=20 > Change-Id: Ibfbfdace1fe7e29ab94463629d8d2f539a43f1b9 > Cc: Eric Dong > Cc: Ray Ni > Cc: Zeng Star > Cc: Gerd Hoffmann > Cc: Rahul Kumar > Signed-off-by: Jiaxin Wu > --- > UefiCpuPkg/CpuMpPei/CpuMpPei.h | 1 + > UefiCpuPkg/CpuMpPei/CpuMpPei.inf | 1 + > UefiCpuPkg/CpuMpPei/CpuPaging.c | 228 +++++++++++++++++----------------= --- > --- > 3 files changed, 102 insertions(+), 128 deletions(-) >=20 > diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h > b/UefiCpuPkg/CpuMpPei/CpuMpPei.h > index 0649c48d14..1b9a94e18f 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h > +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h > @@ -26,10 +26,11 @@ > #include > #include > #include > #include > #include > +#include >=20 > extern EFI_PEI_PPI_DESCRIPTOR mPeiCpuMpPpiDesc; >=20 > /** > This service retrieves the number of logical processor in the platform > diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf > b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf > index 7444bdb968..865be5627e 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf > +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf > @@ -44,10 +44,11 @@ > CpuExceptionHandlerLib > MpInitLib > BaseMemoryLib > CpuLib > MemoryAllocationLib > + CpuPageTableLib >=20 > [Guids] > gEdkiiMigratedFvInfoGuid #= # SOMETIMES_CONSUMES > ## HOB >=20 > [Ppis] > diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c > b/UefiCpuPkg/CpuMpPei/CpuPaging.c > index a471f089c8..6c113051fe 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c > +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c > @@ -115,42 +115,10 @@ AllocatePageTableMemory ( > } >=20 > return Address; > } >=20 > -/** > - Get the address width supported by current processor. > - > - @retval 32 If processor is in 32-bit mode. > - @retval 36-48 If processor is in 64-bit mode. > - > -**/ > -UINTN > -GetPhysicalAddressWidth ( > - VOID > - ) > -{ > - UINT32 RegEax; > - > - if (sizeof (UINTN) =3D=3D 4) { > - return 32; > - } > - > - AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL); > - if (RegEax >=3D CPUID_VIR_PHY_ADDRESS_SIZE) { > - AsmCpuid (CPUID_VIR_PHY_ADDRESS_SIZE, &RegEax, NULL, NULL, NULL); > - RegEax &=3D 0xFF; > - if (RegEax > 48) { > - return 48; > - } > - > - return (UINTN)RegEax; > - } > - > - return 36; > -} > - > /** > Get the type of top level page table. >=20 > @retval Page512G PML4 paging. > @retval Page1G PAE paging. > @@ -381,120 +349,93 @@ ConvertMemoryPageAttributes ( >=20 > return RETURN_SUCCESS; > } >=20 > /** > - Get maximum size of page memory supported by current processor. > - > - @param[in] TopLevelType The type of top level page entry. > + Enable PAE Page Table. >=20 > - @retval Page1G If processor supports 1G page and PML4. > - @retval Page2M For all other situations. > + @retval EFI_SUCCESS The PAE Page Table was enabled success= fully. > + @retval EFI_OUT_OF_RESOURCES The PAE Page Table could not be enable= d > due to lack of available memory. >=20 > **/ > -PAGE_ATTRIBUTE > -GetMaxMemoryPage ( > - IN PAGE_ATTRIBUTE TopLevelType > - ) > -{ > - UINT32 RegEax; > - UINT32 RegEdx; > - > - if (TopLevelType =3D=3D Page512G) { > - AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL); > - if (RegEax >=3D CPUID_EXTENDED_CPU_SIG) { > - AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx); > - if ((RegEdx & BIT26) !=3D 0) { > - return Page1G; > - } > - } > - } > - > - return Page2M; > -} > - > -/** > - Create PML4 or PAE page table. > - > - @return The address of page table. > - > -**/ > -UINTN > -CreatePageTable ( > +EFI_STATUS > +EnablePaePageTable ( > VOID > ) > { > - RETURN_STATUS Status; > - UINTN PhysicalAddressBits; > - UINTN NumberOfEntries; > - PAGE_ATTRIBUTE TopLevelPageAttr; > - UINTN PageTable; > - PAGE_ATTRIBUTE MaxMemoryPage; > - UINTN Index; > - UINT64 AddressEncMask; > - UINT64 *PageEntry; > - EFI_PHYSICAL_ADDRESS PhysicalAddress; > - > - TopLevelPageAttr =3D (PAGE_ATTRIBUTE)GetPageTableTopLevelType (); > - PhysicalAddressBits =3D GetPhysicalAddressWidth (); > - NumberOfEntries =3D (UINTN)1 << (PhysicalAddressBits - > - mPageAttributeTable[TopLevelPageAtt= r].AddressBitOffset); > + EFI_STATUS Status; > + PAGING_MODE PagingMode; > + > + UINTN PageTable; > + VOID *Buffer; > + UINTN BufferSize; > + IA32_MAP_ATTRIBUTE MapAttribute; > + IA32_MAP_ATTRIBUTE MapMask; > + > + PagingMode =3D PagingPae; > + PageTable =3D 0; > + Buffer =3D NULL; > + BufferSize =3D 0; > + MapAttribute.Uint64 =3D 0; > + MapMask.Uint64 =3D MAX_UINT64; > + MapAttribute.Bits.Present =3D 1; > + MapAttribute.Bits.ReadWrite =3D 1; >=20 > - PageTable =3D (UINTN)AllocatePageTableMemory (1); > - if (PageTable =3D=3D 0) { > - return 0; > + // > + // Get required buffer size for the pagetable that will be created. > + // The Max size of LinearAddress for PAE is 2^32. > + // > + Status =3D PageTableMap (&PageTable, PagingMode, 0, &BufferSize, 0, > LShiftU64 (1, 32), &MapAttribute, &MapMask, NULL); 1. how about directly use SIZE_4GB macro in above? > + ASSERT (Status =3D=3D EFI_BUFFER_TOO_SMALL); > + if (Status !=3D EFI_BUFFER_TOO_SMALL) { > + DEBUG ((DEBUG_ERROR, "EnablePaePageTable: Failed to get PageTable > required BufferSize, Status =3D %r\n", Status)); 2. no need for debug message. > + return Status; > } >=20 > - AddressEncMask =3D PcdGet64 (PcdPteMemoryEncryptionAddressOrMask); > - AddressEncMask &=3D mPageAttributeTable[TopLevelPageAttr].AddressMask; 3. Good to remove the address encryption mask for 32bit env. > - MaxMemoryPage =3D GetMaxMemoryPage (TopLevelPageAttr); > - PageEntry =3D (UINT64 *)PageTable; > + DEBUG ((DEBUG_INFO, "EnablePaePageTable: Get PageTable required > BufferSize =3D %x\n", BufferSize)); 4. Can you remove this debug log? > + > + // > + // Allocate required Buffer. > + // > + Buffer =3D AllocatePageTableMemory (EFI_SIZE_TO_PAGES (BufferSize)); > + ASSERT (Buffer !=3D NULL); > + if (Buffer =3D=3D NULL) { > + DEBUG ((DEBUG_ERROR, "EnablePaePageTable: Failed to allocate PageTab= le > required BufferSize!\n")); 5. can you please dump the failure message but also the BufferSize value? > + return EFI_OUT_OF_RESOURCES; > + } >=20 > - PhysicalAddress =3D 0; > - for (Index =3D 0; Index < NumberOfEntries; ++Index) { > - *PageEntry =3D PhysicalAddress | AddressEncMask | PAGE_ATTRIBUTE_BIT= S; > + // > + // Create PageTable in permanent memory. > + // The Max size of LinearAddress for PAE is 2^32. > + // > + Status =3D PageTableMap (&PageTable, PagingMode, Buffer, &BufferSize, = 0, > LShiftU64 (1, 32), &MapAttribute, &MapMask, NULL); > + ASSERT (!EFI_ERROR (Status) && PageTable !=3D 0); 6. ASSERT_EFI_ERROR (Status) is enough. > + if (EFI_ERROR (Status) || PageTable =3D=3D 0) { > + DEBUG ((DEBUG_ERROR, "EnablePaePageTable: Failed to create PageTable= , > Status =3D %r, PageTable =3D 0x%lx\n", Status, PageTable)); 7. if error happens, no need to dump PageTable value. > + return EFI_OUT_OF_RESOURCES; > + } >=20 > - // > - // Split the top page table down to the maximum page size supported > - // > - if (MaxMemoryPage < TopLevelPageAttr) { > - Status =3D SplitPage (PageEntry, TopLevelPageAttr, MaxMemoryPage, = TRUE); > - ASSERT_EFI_ERROR (Status); > - } > + DEBUG ((DEBUG_INFO, "EnablePaePageTable: Create PageTable =3D 0x%x\n", > PageTable)); >=20 > - if (TopLevelPageAttr =3D=3D Page1G) { > - // > - // PDPTE[2:1] (PAE Paging) must be 0. SplitPage() might change the= m to 1. > - // > - *PageEntry &=3D ~(UINT64)(IA32_PG_RW | IA32_PG_U); > - } > + // > + // Write the Pagetable to CR3. > + // > + AsmWriteCr3 (PageTable); >=20 > - PageEntry +=3D 1; > - PhysicalAddress +=3D mPageAttributeTable[TopLevelPageAttr].Length; > - } > + // > + // Enable CR4.PAE > + // > + AsmWriteCr4 (AsmReadCr4 () | BIT5); >=20 > - return PageTable; > -} > + // > + // Enable CR0.PG > + // > + AsmWriteCr0 (AsmReadCr0 () | BIT31); >=20 > -/** > - Setup page tables and make them work. > + DEBUG ((DEBUG_INFO, "EnablePaePageTable: Enabled PAE PageTable > Sucessfully.\n")); >=20 > -**/ > -VOID > -EnablePaging ( > - VOID > - ) > -{ > - UINTN PageTable; > - > - PageTable =3D CreatePageTable (); > - ASSERT (PageTable !=3D 0); > - if (PageTable !=3D 0) { > - AsmWriteCr3 (PageTable); > - AsmWriteCr4 (AsmReadCr4 () | BIT5); // CR4.PAE > - AsmWriteCr0 (AsmReadCr0 () | BIT31); // CR0.PG > - } > + return Status; > } >=20 > /** > Get the base address of current AP's stack. >=20 > @@ -622,10 +563,11 @@ MemoryDiscoveredPpiNotifyCallback ( > { > EFI_STATUS Status; > BOOLEAN InitStackGuard; > EDKII_MIGRATED_FV_INFO *MigratedFvInfo; > EFI_PEI_HOB_POINTERS Hob; > + MSR_IA32_EFER_REGISTER MsrEfer; >=20 > // > // Paging must be setup first. Otherwise the exception TSS setup durin= g MP > // initialization later will not contain paging information and then f= ail > // the task switch (for the sake of stack switch). > @@ -635,12 +577,42 @@ MemoryDiscoveredPpiNotifyCallback ( > if (IsIa32PaeSupported ()) { > Hob.Raw =3D GetFirstGuidHob (&gEdkiiMigratedFvInfoGuid); > InitStackGuard =3D PcdGetBool (PcdCpuStackGuard); > } >=20 > - if (InitStackGuard || (Hob.Raw !=3D NULL)) { > - EnablePaging (); > + // > + // Some security features depends on the page table enabling.So, here > + // is to enable the page table if page table has not been enabled yet. > + // If page table is not enabled before this point, which means the sys= tem > + // IA-32e Mode is not activated.Because on Intel 64 processors, IA-32e= Mode > + // operation requires physical address extensions with 4 or 5 levels o= f > + // enhanced paging structures (see Section 4.5, "4 - Level Paging and = 5 - > + // Level Paging" and Section 9.8, "Initializing IA-32e Mode"). So, jus= t > + // enable PAE page table if CR0.PG is not set. > + // > + if (((AsmReadCr0 () & BIT31) =3D=3D 0) && (InitStackGuard || (Hob.Raw = !=3D NULL))) { 8. Can you use IA32_CR0 structure in if-check? > + // > + // Check CPU runs in 32bit mode. > + // > + MsrEfer.Uint64 =3D AsmReadMsr64 (MSR_CORE_IA32_EFER); > + if (MsrEfer.Bits.LMA =3D=3D 1) { > + // > + // On Intel 64 processors, IA-32e Mode operation requires physical= - > address extensions with > + // 4 or 5 levels of enhanced paging structures (see Section 4.5, "= 4 - Level > Paging and > + // 5 - Level Paging" and Section 9.8, "Initializing IA-32e Mode").= So, it must > something wrong > + // if MsrEfer.Bits.LMA =3D=3D 1 with no page table enbaled before. > + // > + DEBUG ((DEBUG_ERROR, "MemoryDiscoveredPpiNotifyCallback: No page > table with IA-32e Mode actived!\n")); > + ASSERT (MsrEfer.Bits.LMA =3D=3D 0); > + return EFI_DEVICE_ERROR; > + } 9. I don't think we need to check if CPU runs in 32bit mode. We should have= confidence that when paging is disabled, CPU should run in 32bit mode. I agree adding ASSERT (sizeof (UINTN) =3D=3D sizeof (UINT32)) can improve t= he readability. > + > + Status =3D EnablePaePageTable (); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_WARN, "MemoryDiscoveredPpiNotifyCallback: Failed to > enable PAE page table: %r.\n", Status)); > + ASSERT_EFI_ERROR (Status); > + } > } >=20 > Status =3D InitializeCpuMpWorker ((CONST EFI_PEI_SERVICES **)PeiServic= es); > ASSERT_EFI_ERROR (Status); >=20 > -- > 2.16.2.windows.1