[edk2-devel] [PATCH v6 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Reflect page table depth with page table address

Sheng Wei w.sheng at intel.com
Tue Nov 10 02:24:30 UTC 2020


When trying to get page table base, if mInternalCr3 is zero, it will use
 the page table from CR3, and reflect the page table depth by CR4 LA57 bit.
If mInternalCr3 is non zero, it will use the page table from mInternalCr3
 and reflect the page table depth of mInternalCr3 at same time.
In the case of X64, we use m5LevelPagingNeeded to reflect the depth of
 the page table. And in the case of IA32, it will not the page table depth
 information.

This patch is a bug fix when enable CET feature with 5 level paging.
The SMM page tables are allocated / initialized in PiCpuSmmEntry().
When CET is enabled, PiCpuSmmEntry() must further modify the attribute of
 shadow stack pages. This page table is not set to CR3 in PiCpuSmmEntry().
 So the page table base address is set to mInternalCr3 for modifty the
 page table attribute. It could not use CR4 LA57 bit to reflect the
 page table depth for mInternalCr3.
So we create a architecture-specific implementation GetPageTable() with
 2 output parameters. One parameter is used to output the page table
 address. Another parameter is used to reflect if it is 5 level paging
 or not.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3015

Signed-off-by: Sheng Wei <w.sheng at intel.com>
Cc: Eric Dong <eric.dong at intel.com>
Cc: Ray Ni <ray.ni at intel.com>
Cc: Laszlo Ersek <lersek at redhat.com>
Cc: Rahul Kumar <rahul1.kumar at intel.com>
Cc: Jiewen Yao <jiewen.yao at intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c           | 24 ++++++++++++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h         | 12 ++++---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 27 +++------------
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c            | 40 +++++++++++++++++++---
 4 files changed, 70 insertions(+), 33 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index 2483f2ea84..f5d64392bd 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -10,6 +10,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include "PiSmmCpuDxeSmm.h"
 
+extern UINTN mInternalCr3;
+
 /**
   Disable CET.
 **/
@@ -28,6 +30,26 @@ EnableCet (
   VOID
   );
 
+/**
+  Get page table base address and the depth of the page table.
+
+  @param[out] Base        Page table base address.
+  @param[out] FiveLevels  TRUE means 5 level paging. FALSE means 4 level paging.
+**/
+VOID
+GetPageTable (
+  OUT UINTN   *Base,
+  OUT BOOLEAN *FiveLevels OPTIONAL
+  )
+{
+  *Base = ((mInternalCr3 == 0) ?
+            (AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64) :
+            mInternalCr3);
+  if (FiveLevels != NULL) {
+    *FiveLevels = FALSE;
+  }
+}
+
 /**
   Create PageTable for SMM use.
 
@@ -268,7 +290,7 @@ SetPageTableAttributes (
     DEBUG ((DEBUG_INFO, "Start...\n"));
     PageTableSplitted = FALSE;
 
-    L3PageTable = (UINT64 *)GetPageTableBase ();
+    GetPageTable ((UINTN *)&L3PageTable, NULL);
 
     SmmSetMemoryAttributesEx ((EFI_PHYSICAL_ADDRESS)(UINTN)L3PageTable, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted);
     PageTableSplitted = (PageTableSplitted || IsSplitted);
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 7fb3a2d9e4..59bc764140 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -942,13 +942,15 @@ SetPageTableAttributes (
   );
 
 /**
-  Return page table base.
+  Get page table base address and the depth of the page table.
 
-  @return page table base.
+  @param[out] Base        Page table base address.
+  @param[out] FiveLevels  TRUE means 5 level paging. FALSE means 4 level paging.
 **/
-UINTN
-GetPageTableBase (
-  VOID
+VOID
+GetPageTable (
+  OUT UINTN   *Base,
+  OUT BOOLEAN *FiveLevels OPTIONAL
   );
 
 /**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index d67f036aea..fe71b77295 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -49,22 +49,6 @@ SetPageTableBase (
   mInternalCr3 = Cr3;
 }
 
-/**
-  Return page table base.
-
-  @return page table base.
-**/
-UINTN
-GetPageTableBase (
-  VOID
-  )
-{
-  if (mInternalCr3 != 0) {
-    return mInternalCr3;
-  }
-  return (AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64);
-}
-
 /**
   Return length according to page attributes.
 
@@ -131,8 +115,8 @@ GetPageTableEntry (
   UINT64                *L3PageTable;
   UINT64                *L4PageTable;
   UINT64                *L5PageTable;
-  IA32_CR4              Cr4;
   BOOLEAN               Enable5LevelPaging;
+  UINT64                *PageTableBase = NULL;
 
   Index5 = ((UINTN)RShiftU64 (Address, 48)) & PAGING_PAE_INDEX_MASK;
   Index4 = ((UINTN)RShiftU64 (Address, 39)) & PAGING_PAE_INDEX_MASK;
@@ -140,12 +124,11 @@ GetPageTableEntry (
   Index2 = ((UINTN)Address >> 21) & PAGING_PAE_INDEX_MASK;
   Index1 = ((UINTN)Address >> 12) & PAGING_PAE_INDEX_MASK;
 
-  Cr4.UintN = AsmReadCr4 ();
-  Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1);
+  GetPageTable ((UINTN *)&PageTableBase, &Enable5LevelPaging);
 
   if (sizeof(UINTN) == sizeof(UINT64)) {
     if (Enable5LevelPaging) {
-      L5PageTable = (UINT64 *)GetPageTableBase ();
+      L5PageTable = PageTableBase;
       if (L5PageTable[Index5] == 0) {
         *PageAttribute = PageNone;
         return NULL;
@@ -153,7 +136,7 @@ GetPageTableEntry (
 
       L4PageTable = (UINT64 *)(UINTN)(L5PageTable[Index5] & ~mAddressEncMask & PAGING_4K_ADDRESS_MASK_64);
     } else {
-      L4PageTable = (UINT64 *)GetPageTableBase ();
+      L4PageTable = PageTableBase;
     }
     if (L4PageTable[Index4] == 0) {
       *PageAttribute = PageNone;
@@ -162,7 +145,7 @@ GetPageTableEntry (
 
     L3PageTable = (UINT64 *)(UINTN)(L4PageTable[Index4] & ~mAddressEncMask & PAGING_4K_ADDRESS_MASK_64);
   } else {
-    L3PageTable = (UINT64 *)GetPageTableBase ();
+    L3PageTable = PageTableBase;
   }
   if (L3PageTable[Index3] == 0) {
     *PageAttribute = PageNone;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 810985df20..51469d3b88 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -19,6 +19,8 @@ BOOLEAN                             mCpuSmmRestrictedMemoryAccess;
 BOOLEAN                             m5LevelPagingNeeded;
 X86_ASSEMBLY_PATCH_LABEL            gPatch5LevelPagingNeeded;
 
+extern UINTN mInternalCr3;
+
 /**
   Disable CET.
 **/
@@ -104,6 +106,35 @@ Is5LevelPagingNeeded (
   }
 }
 
+/**
+  Get page table base address and the depth of the page table.
+
+  @param[out] Base        Page table base address.
+  @param[out] FiveLevels  TRUE means 5 level paging. FALSE means 4 level paging.
+**/
+VOID
+GetPageTable (
+  OUT UINTN   *Base,
+  OUT BOOLEAN *FiveLevels OPTIONAL
+  )
+{
+  IA32_CR4 Cr4;
+
+  if (mInternalCr3 == 0) {
+    *Base = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;
+    if (FiveLevels != NULL) {
+      Cr4.UintN = AsmReadCr4 ();
+      *FiveLevels = (BOOLEAN)(Cr4.Bits.LA57 == 1);
+    }
+    return;
+  }
+
+  *Base = mInternalCr3;
+  if (FiveLevels != NULL) {
+    *FiveLevels = m5LevelPagingNeeded;
+  }
+}
+
 /**
   Set sub-entries number in entry.
 
@@ -1114,11 +1145,10 @@ SetPageTableAttributes (
   BOOLEAN               IsSplitted;
   BOOLEAN               PageTableSplitted;
   BOOLEAN               CetEnabled;
-  IA32_CR4              Cr4;
   BOOLEAN               Enable5LevelPaging;
+  UINT64                *PageTableBase = NULL;
 
-  Cr4.UintN = AsmReadCr4 ();
-  Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1);
+  GetPageTable ((UINTN *)&PageTableBase, &Enable5LevelPaging);
 
   //
   // Don't mark page table memory as read-only if
@@ -1164,7 +1194,7 @@ SetPageTableAttributes (
     PageTableSplitted = FALSE;
     L5PageTable = NULL;
     if (Enable5LevelPaging) {
-      L5PageTable = (UINT64 *)GetPageTableBase ();
+      L5PageTable = PageTableBase;
       SmmSetMemoryAttributesEx ((EFI_PHYSICAL_ADDRESS)(UINTN)L5PageTable, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted);
       PageTableSplitted = (PageTableSplitted || IsSplitted);
     }
@@ -1176,7 +1206,7 @@ SetPageTableAttributes (
           continue;
         }
       } else {
-        L4PageTable = (UINT64 *)GetPageTableBase ();
+        L4PageTable = PageTableBase;
       }
       SmmSetMemoryAttributesEx ((EFI_PHYSICAL_ADDRESS)(UINTN)L4PageTable, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted);
       PageTableSplitted = (PageTableSplitted || IsSplitted);
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67212): https://edk2.groups.io/g/devel/message/67212
Mute This Topic: https://groups.io/mt/78152032/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list