From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: eric.dong@intel.com) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by groups.io with SMTP; Sun, 08 Sep 2019 17:51:27 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Sep 2019 17:51:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,483,1559545200"; d="scan'208";a="185052415" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga007.fm.intel.com with ESMTP; 08 Sep 2019 17:51:27 -0700 Received: from shsmsx106.ccr.corp.intel.com (10.239.4.159) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 8 Sep 2019 17:51:26 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.113]) by SHSMSX106.ccr.corp.intel.com ([169.254.10.86]) with mapi id 14.03.0439.000; Mon, 9 Sep 2019 08:51:25 +0800 From: "Dong, Eric" To: "Ni, Ray" , "devel@edk2.groups.io" CC: "Gao, Liming" , Laszlo Ersek Subject: Re: [PATCH] UefiCpuPkg/PiSmmCpu: Enable 5L paging only when phy addr line > 48 Thread-Topic: [PATCH] UefiCpuPkg/PiSmmCpu: Enable 5L paging only when phy addr line > 48 Thread-Index: AQHVZDgYjaO+lLoYfU22LRriXUZBbaciiOVA Date: Mon, 9 Sep 2019 00:51:24 +0000 Message-ID: References: <20190905221911.197356-1-ray.ni@intel.com> In-Reply-To: <20190905221911.197356-1-ray.ni@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: eric.dong@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Eric Dong > -----Original Message----- > From: Ni, Ray > Sent: Friday, September 6, 2019 6:19 AM > To: devel@edk2.groups.io > Cc: Gao, Liming ; Dong, Eric ; > Laszlo Ersek > Subject: [PATCH] UefiCpuPkg/PiSmmCpu: Enable 5L paging only when phy addr > line > 48 >=20 > Today's behavior is to enable 5l paging when CPU supports it > (CPUID[7,0].ECX.BIT[16] is set). >=20 > The patch changes the behavior to enable 5l paging when two conditions ar= e > both met: > 1. CPU supports it; > 2. The max physical address bits is bigger than 48. >=20 > Because 4-level paging can support to address physical address up to > 2^48 - 1, there is no need to enable 5-level paging with max physical add= ress > bits <=3D 48. >=20 > Signed-off-by: Ray Ni > Cc: Liming Gao > Cc: Eric Dong > Cc: Laszlo Ersek > --- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 57 +++++++++++++-------- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 4 +- > 2 files changed, 39 insertions(+), 22 deletions(-) >=20 > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index 733d107efd..e5c4788c13 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -16,8 +16,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > LIST_ENTRY mPagePool =3D INITIALIZE_LIST_HEAD_V= ARIABLE > (mPagePool); > BOOLEAN m1GPageTableSupport =3D FALSE; > BOOLEAN mCpuSmmRestrictedMemoryAccess; > -BOOLEAN m5LevelPagingSupport; > -X86_ASSEMBLY_PATCH_LABEL gPatch5LevelPagingSupport; > +BOOLEAN m5LevelPagingNeeded; > +X86_ASSEMBLY_PATCH_LABEL gPatch5LevelPagingNeeded; >=20 > /** > Disable CET. > @@ -63,28 +63,45 @@ Is1GPageSupport ( > } >=20 > /** > - Check if 5-level paging is supported by processor or not. > - > - @retval TRUE 5-level paging is supported. > - @retval FALSE 5-level paging is not supported. > + The routine returns TRUE when CPU supports it (CPUID[7,0].ECX.BIT[16] > + is set) and the max physical address bits is bigger than 48. Because > + 4-level paging can support to address physical address up to 2^48 - > + 1, there is no need to enable 5-level paging with max physical address= bits > <=3D 48. >=20 > + @retval TRUE 5-level paging enabling is needed. > + @retval FALSE 5-level paging enabling is not needed. > **/ > BOOLEAN > -Is5LevelPagingSupport ( > +Is5LevelPagingNeeded ( > VOID > ) > { > - CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_ECX EcxFlags; > + CPUID_VIR_PHY_ADDRESS_SIZE_EAX VirPhyAddressSize; > + CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_ECX ExtFeatureEcx; > + UINT32 MaxExtendedFunctionId; >=20 > + AsmCpuid (CPUID_EXTENDED_FUNCTION, &MaxExtendedFunctionId, NULL, > + NULL, NULL); if (MaxExtendedFunctionId >=3D > CPUID_VIR_PHY_ADDRESS_SIZE) { > + AsmCpuid (CPUID_VIR_PHY_ADDRESS_SIZE, &VirPhyAddressSize.Uint32, > + NULL, NULL, NULL); } else { > + VirPhyAddressSize.Bits.PhysicalAddressBits =3D 36; } > AsmCpuidEx ( > CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, > CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_SUB_LEAF_INFO, > - NULL, > - NULL, > - &EcxFlags.Uint32, > - NULL > + NULL, NULL, &ExtFeatureEcx.Uint32, NULL > ); > - return (BOOLEAN) (EcxFlags.Bits.FiveLevelPage !=3D 0); > + DEBUG (( > + DEBUG_INFO, "PhysicalAddressBits =3D %d, 5LPageTable =3D %d.\n", > + VirPhyAddressSize.Bits.PhysicalAddressBits, > ExtFeatureEcx.Bits.FiveLevelPage > + )); > + > + if (VirPhyAddressSize.Bits.PhysicalAddressBits > 4 * 9 + 12) { > + ASSERT (ExtFeatureEcx.Bits.FiveLevelPage =3D=3D 1); > + return TRUE; > + } else { > + return FALSE; > + } > } >=20 > /** > @@ -190,7 +207,7 @@ SetStaticPageTable ( > // when 5-Level Paging is disabled. > // > ASSERT (mPhysicalAddressBits <=3D 52); > - if (!m5LevelPagingSupport && mPhysicalAddressBits > 48) { > + if (!m5LevelPagingNeeded && mPhysicalAddressBits > 48) { > mPhysicalAddressBits =3D 48; > } >=20 > @@ -217,7 +234,7 @@ SetStaticPageTable ( >=20 > PageMapLevel4Entry =3D PageMap; > PageMapLevel5Entry =3D NULL; > - if (m5LevelPagingSupport) { > + if (m5LevelPagingNeeded) { > // > // By architecture only one PageMapLevel5 exists - so lets allocate = storage > for it. > // > @@ -233,7 +250,7 @@ SetStaticPageTable ( > // So lets allocate space for them and fill them in in the IndexOfPm= l4Entries > loop. > // When 5-Level Paging is disabled, below allocation happens only on= ce. > // > - if (m5LevelPagingSupport) { > + if (m5LevelPagingNeeded) { > PageMapLevel4Entry =3D (UINT64 *) ((*PageMapLevel5Entry) & > ~mAddressEncMask & gPhyMask); > if (PageMapLevel4Entry =3D=3D NULL) { > PageMapLevel4Entry =3D AllocatePageTableMemory (1); @@ -336,10 > +353,10 @@ SmmInitPageTable ( >=20 > mCpuSmmRestrictedMemoryAccess =3D PcdGetBool > (PcdCpuSmmRestrictedMemoryAccess); > m1GPageTableSupport =3D Is1GPageSupport (); > - m5LevelPagingSupport =3D Is5LevelPagingSupport (); > + m5LevelPagingNeeded =3D Is5LevelPagingNeeded (); > mPhysicalAddressBits =3D CalculateMaximumSupportAddress (); > - PatchInstructionX86 (gPatch5LevelPagingSupport, m5LevelPagingSupport, = 1); > - DEBUG ((DEBUG_INFO, "5LevelPaging Support - %d\n", > m5LevelPagingSupport)); > + PatchInstructionX86 (gPatch5LevelPagingNeeded, m5LevelPagingNeeded, 1)= ; > + DEBUG ((DEBUG_INFO, "5LevelPaging Needed - %d\n", > m5LevelPagingNeeded)); > DEBUG ((DEBUG_INFO, "1GPageTable Support - %d\n", > m1GPageTableSupport)); > DEBUG ((DEBUG_INFO, "PcdCpuSmmRestrictedMemoryAccess - %d\n", > mCpuSmmRestrictedMemoryAccess)); > DEBUG ((DEBUG_INFO, "PhysicalAddressBits - %d\n", > mPhysicalAddressBits)); > @@ -370,7 +387,7 @@ SmmInitPageTable ( > SetSubEntriesNum (Pml4Entry, 3); > PTEntry =3D Pml4Entry; >=20 > - if (m5LevelPagingSupport) { > + if (m5LevelPagingNeeded) { > // > // Fill PML5 entry > // > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > index 271492a9d7..db06d22d51 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > @@ -69,7 +69,7 @@ extern ASM_PFX(mXdSupported) global > ASM_PFX(gPatchXdSupported) global ASM_PFX(gPatchSmiStack) global > ASM_PFX(gPatchSmiCr3) -global ASM_PFX(gPatch5LevelPagingSupport) > +global ASM_PFX(gPatch5LevelPagingNeeded) > global ASM_PFX(gcSmiHandlerTemplate) > global ASM_PFX(gcSmiHandlerSize) >=20 > @@ -127,7 +127,7 @@ ASM_PFX(gPatchSmiCr3): > mov eax, 0x668 ; as cr4.PGE is not set here, r= efresh cr3 >=20 > mov cl, strict byte 0 ; source operand will be patche= d > -ASM_PFX(gPatch5LevelPagingSupport): > +ASM_PFX(gPatch5LevelPagingNeeded): > cmp cl, 0 > je SkipEnable5LevelPaging > ; > -- > 2.21.0.windows.1