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.web08.27105.1658150328221476289 for ; Mon, 18 Jul 2022 06:18:51 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=Sy4nw0pr; 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=1658150331; x=1689686331; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=n9McP3XmMqjTbxdqAqJh3hwjrpipiuGj0Ru3SPE9PxA=; b=Sy4nw0prtCxWXKeWU0edoJ3gZtRiTymcQBSiA+LuUKHwS2bFP7J481AA 5F57ZXVA+4dvMfG1fCg+v1v29oL0dagyPb5uo8NSETkJ5K3K/5RnMbQjf R4P6BLQiORN4KNxogufgGYQ9wpKIuUVxHPjSDaaCujvMf2LSG3bEM5qT/ G8Os95UvroefbR2k4brG4U4YM/2tetpsydBLTRiJyO+/f5OsdwAm0s4RG wlq2ZbIUhABrIMYeyODBJ/L940WIh0YMEQqDKhpm83zG/Qrle76VNoaVW LqaASOVBMseg2FCUSvov4nGSSYoWpi5QaEHPX7Y7AHNHTphtT0vVD6Eov Q==; X-IronPort-AV: E=McAfee;i="6400,9594,10411"; a="287363967" X-IronPort-AV: E=Sophos;i="5.92,281,1650956400"; d="scan'208";a="287363967" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jul 2022 06:18:50 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.92,281,1650956400"; d="scan'208";a="624725005" Received: from shwdeopenlab706.ccr.corp.intel.com ([10.239.183.102]) by orsmga008.jf.intel.com with ESMTP; 18 Jul 2022 06:18:49 -0700 From: "Ni, Ray" To: devel@edk2.groups.io Cc: Zhiguang Liu , Eric Dong Subject: [PATCH 08/10] CpuPageTableLib: Fix a bug to avoid unnecessary changing to page table Date: Mon, 18 Jul 2022 21:18:29 +0800 Message-Id: <20220718131831.660-9-ray.ni@intel.com> X-Mailer: git-send-email 2.35.1.windows.2 In-Reply-To: <20220718131831.660-1-ray.ni@intel.com> References: <20220718131831.660-1-ray.ni@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable With the following paging structure that maps [0, 2G] with ReadWrite bit set. PML4[0] --> PDPTE[0] --> PDE[0-255] \-> PDPTE[1] --> PDE[0-255] If ReadWrite bit is cleared in PML4[0] and PageTableMap() is called to change [0, 2M] as read-only, today's logic unnecessarily changes the paging structure in 2 aspects: 1. When setting PageTableBaseAddress in the entry, the code clears all attributes. 2. Even the ReadWrite bit in parent entry is not set, the code clears the ReadWrite bit in the leaf entry. First change is wrong. It should not change other attributes when setting the PA. Second change is unnecessary. Because the parent entry already declares the whole region as read-only, there is no need to clear ReadWrite bit in the leaf entry again. Signed-off-by: Zhiguang Liu Signed-off-by: Ray Ni Cc: Eric Dong --- .../Library/CpuPageTableLib/CpuPageTableMap.c | 41 +++++++++++++++---- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c b/UefiCpu= Pkg/Library/CpuPageTableLib/CpuPageTableMap.c index ac5d1c79f4..1205119fc8 100644 --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c @@ -27,10 +27,7 @@ PageTableLibSetPte4K ( )=0D {=0D if (Mask->Bits.PageTableBaseAddress) {=0D - //=0D - // Reset all attributes when the physical address is changed.=0D - //=0D - Pte4K->Uint64 =3D IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribut= e) + Offset;=0D + Pte4K->Uint64 =3D (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribu= te) + Offset) | (Pte4K->Uint64 & ~IA32_PE_BASE_ADDRESS_MASK_40);=0D }=0D =0D if (Mask->Bits.Present) {=0D @@ -97,10 +94,7 @@ PageTableLibSetPleB ( )=0D {=0D if (Mask->Bits.PageTableBaseAddress) {=0D - //=0D - // Reset all attributes when the physical address is changed.=0D - //=0D - PleB->Uint64 =3D IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribute= ) + Offset;=0D + PleB->Uint64 =3D (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribut= e) + Offset) | (PleB->Uint64 & ~IA32_PE_BASE_ADDRESS_MASK_39);=0D }=0D =0D PleB->Bits.MustBeOne =3D 1;=0D @@ -277,6 +271,7 @@ PageTableLibMapInLevel ( IA32_PAGING_ENTRY OneOfPagingEntry;=0D IA32_MAP_ATTRIBUTE ChildAttribute;=0D IA32_MAP_ATTRIBUTE ChildMask;=0D + IA32_MAP_ATTRIBUTE CurrentMask;=0D =0D ASSERT (Level !=3D 0);=0D ASSERT ((Attribute !=3D NULL) && (Mask !=3D NULL));=0D @@ -464,7 +459,35 @@ PageTableLibMapInLevel ( // Create one entry mapping the entire region (1G, 2M or 4K).=0D //=0D if (Modify) {=0D - PageTableLibSetPle (Level, CurrentPagingEntry, Offset, Attribute, = Mask);=0D + //=0D + // When the inheritable attributes in parent entry could override = the child attributes,=0D + // e.g.: Present/ReadWrite/UserSupervisor is 0 in parent entry, or= =0D + // Nx is 1 in parent entry,=0D + // we just skip setting any value to these attributes in child.=0D + // We add assertion to make sure the requested settings don't conf= lict with parent attributes in this case.=0D + //=0D + CurrentMask.Uint64 =3D Mask->Uint64;=0D + if (ParentAttribute->Bits.Present =3D=3D 0) {=0D + CurrentMask.Bits.Present =3D 0;=0D + ASSERT (CreateNew || (Mask->Bits.Present =3D=3D 0) || (Attribute= ->Bits.Present =3D=3D 0));=0D + }=0D +=0D + if (ParentAttribute->Bits.ReadWrite =3D=3D 0) {=0D + CurrentMask.Bits.ReadWrite =3D 0;=0D + ASSERT (CreateNew || (Mask->Bits.ReadWrite =3D=3D 0) || (Attribu= te->Bits.ReadWrite =3D=3D 0));=0D + }=0D +=0D + if (ParentAttribute->Bits.UserSupervisor =3D=3D 0) {=0D + CurrentMask.Bits.UserSupervisor =3D 0;=0D + ASSERT (CreateNew || (Mask->Bits.UserSupervisor =3D=3D 0) || (At= tribute->Bits.UserSupervisor =3D=3D 0));=0D + }=0D +=0D + if (ParentAttribute->Bits.Nx =3D=3D 1) {=0D + CurrentMask.Bits.Nx =3D 0;=0D + ASSERT (CreateNew || (Mask->Bits.Nx =3D=3D 0) || (Attribute->Bit= s.Nx =3D=3D 1));=0D + }=0D +=0D + PageTableLibSetPle (Level, CurrentPagingEntry, Offset, Attribute, = &CurrentMask);=0D }=0D } else {=0D //=0D --=20 2.35.1.windows.2