UefiCpuPkg: Simplify the code to set smm page table as RO

Simplify the code to set memory used by smm page table as RO.
Since memory used by smm page table are in PageTablePool list,
we only need to set all PageTablePool as ReadOnly in smm page
table itself. Also, we only need to flush tlb once after
setting all page table pool as Read Only.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index bbc536a56763941e9880fd623e72b89a7879cfef..34bf6e1a254605408a905ad0797a744db9124316 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -202,111 +202,6 @@ Exit:
   ReleaseSpinLock (mPFLock);\r
 }\r
 \r
-/**\r
-  This function sets memory attribute for page table.\r
-**/\r
-VOID\r
-SetPageTableAttributes (\r
-  VOID\r
-  )\r
-{\r
-  UINTN    Index2;\r
-  UINTN    Index3;\r
-  UINT64   *L1PageTable;\r
-  UINT64   *L2PageTable;\r
-  UINT64   *L3PageTable;\r
-  UINTN    PageTableBase;\r
-  BOOLEAN  IsSplitted;\r
-  BOOLEAN  PageTableSplitted;\r
-  BOOLEAN  CetEnabled;\r
-\r
-  //\r
-  // Don't mark page table to read-only if heap guard is enabled.\r
-  //\r
-  //      BIT2: SMM page guard enabled\r
-  //      BIT3: SMM pool guard enabled\r
-  //\r
-  if ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {\r
-    DEBUG ((DEBUG_INFO, "Don't mark page table to read-only as heap guard is enabled\n"));\r
-    return;\r
-  }\r
-\r
-  //\r
-  // Don't mark page table to read-only if SMM profile is enabled.\r
-  //\r
-  if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {\r
-    DEBUG ((DEBUG_INFO, "Don't mark page table to read-only as SMM profile is enabled\n"));\r
-    return;\r
-  }\r
-\r
-  DEBUG ((DEBUG_INFO, "SetPageTableAttributes\n"));\r
-\r
-  //\r
-  // Disable write protection, because we need mark page table to be write protected.\r
-  // We need *write* page table memory, to mark itself to be *read only*.\r
-  //\r
-  CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;\r
-  if (CetEnabled) {\r
-    //\r
-    // CET must be disabled if WP is disabled.\r
-    //\r
-    DisableCet ();\r
-  }\r
-\r
-  AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);\r
-\r
-  do {\r
-    DEBUG ((DEBUG_INFO, "Start...\n"));\r
-    PageTableSplitted = FALSE;\r
-\r
-    PageTableBase = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;\r
-    L3PageTable   = (UINT64 *)PageTableBase;\r
-\r
-    SmmSetMemoryAttributesEx (PageTableBase, FALSE, (EFI_PHYSICAL_ADDRESS)PageTableBase, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted);\r
-    PageTableSplitted = (PageTableSplitted || IsSplitted);\r
-\r
-    for (Index3 = 0; Index3 < 4; Index3++) {\r
-      L2PageTable = (UINT64 *)(UINTN)(L3PageTable[Index3] & ~mAddressEncMask & PAGING_4K_ADDRESS_MASK_64);\r
-      if (L2PageTable == NULL) {\r
-        continue;\r
-      }\r
-\r
-      SmmSetMemoryAttributesEx (PageTableBase, FALSE, (EFI_PHYSICAL_ADDRESS)(UINTN)L2PageTable, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted);\r
-      PageTableSplitted = (PageTableSplitted || IsSplitted);\r
-\r
-      for (Index2 = 0; Index2 < SIZE_4KB/sizeof (UINT64); Index2++) {\r
-        if ((L2PageTable[Index2] & IA32_PG_PS) != 0) {\r
-          // 2M\r
-          continue;\r
-        }\r
-\r
-        L1PageTable = (UINT64 *)(UINTN)(L2PageTable[Index2] & ~mAddressEncMask & PAGING_4K_ADDRESS_MASK_64);\r
-        if (L1PageTable == NULL) {\r
-          continue;\r
-        }\r
-\r
-        SmmSetMemoryAttributesEx (PageTableBase, FALSE, (EFI_PHYSICAL_ADDRESS)(UINTN)L1PageTable, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted);\r
-        PageTableSplitted = (PageTableSplitted || IsSplitted);\r
-      }\r
-    }\r
-  } while (PageTableSplitted);\r
-\r
-  //\r
-  // Enable write protection, after page table updated.\r
-  //\r
-  AsmWriteCr0 (AsmReadCr0 () | CR0_WP);\r
-  if (CetEnabled) {\r
-    //\r
-    // re-enable CET.\r
-    //\r
-    EnableCet ();\r
-  }\r
-\r
-  mIsReadOnlyPageTable = TRUE;\r
-\r
-  return;\r
-}\r
-\r
 /**\r
   This function returns with no action for 32 bit.\r
 \r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 3e69e043cac14a8ad9883534229eb904240fcca7..5f0a38e4002d33e46c7712f05633600c66e090a8 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -260,7 +260,6 @@ extern UINTN                 mNumberOfCpus;
 extern EFI_SMM_CPU_PROTOCOL  mSmmCpu;\r
 extern EFI_MM_MP_PROTOCOL    mSmmMp;\r
 extern BOOLEAN               m5LevelPagingNeeded;\r
-extern BOOLEAN               mIsReadOnlyPageTable;\r
 \r
 ///\r
 /// The mode of the CPU at the time an SMI occurs\r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 11df7af016d458c1cdbb2ee59de5a67241b6e215..4bb23f692096962e8595db7ff218ab8ce4caad7b 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -1753,3 +1753,139 @@ EdkiiSmmGetMemoryAttributes (
 \r
   return EFI_SUCCESS;\r
 }\r
+\r
+/**\r
+  Prevent the memory pages used for SMM page table from been overwritten.\r
+**/\r
+VOID\r
+EnablePageTableProtection (\r
+  VOID\r
+  )\r
+{\r
+  PAGE_TABLE_POOL       *HeadPool;\r
+  PAGE_TABLE_POOL       *Pool;\r
+  UINT64                PoolSize;\r
+  EFI_PHYSICAL_ADDRESS  Address;\r
+  UINTN                 PageTableBase;\r
+\r
+  if (mPageTablePool == NULL) {\r
+    return;\r
+  }\r
+\r
+  PageTableBase = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;\r
+\r
+  //\r
+  // ConvertMemoryPageAttributes might update mPageTablePool. It's safer to\r
+  // remember original one in advance.\r
+  //\r
+  HeadPool = mPageTablePool;\r
+  Pool     = HeadPool;\r
+  do {\r
+    Address  = (EFI_PHYSICAL_ADDRESS)(UINTN)Pool;\r
+    PoolSize = Pool->Offset + EFI_PAGES_TO_SIZE (Pool->FreePages);\r
+    //\r
+    // Set entire pool including header, used-memory and left free-memory as ReadOnly in SMM page table.\r
+    //\r
+    ConvertMemoryPageAttributes (PageTableBase, m5LevelPagingNeeded, Address, PoolSize, EFI_MEMORY_RO, TRUE, NULL, NULL);\r
+    Pool = Pool->NextPool;\r
+  } while (Pool != HeadPool);\r
+}\r
+\r
+/**\r
+  Return whether memory used by SMM page table need to be set as Read Only.\r
+\r
+  @retval TRUE  Need to set SMM page table as Read Only.\r
+  @retval FALSE Do not set SMM page table as Read Only.\r
+**/\r
+BOOLEAN\r
+IfReadOnlyPageTableNeeded (\r
+  VOID\r
+  )\r
+{\r
+  //\r
+  // Don't mark page table memory as read-only if\r
+  //  - no restriction on access to non-SMRAM memory; or\r
+  //  - SMM heap guard feature enabled; or\r
+  //      BIT2: SMM page guard enabled\r
+  //      BIT3: SMM pool guard enabled\r
+  //  - SMM profile feature enabled\r
+  //\r
+  if (!IsRestrictedMemoryAccess () ||\r
+      ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) ||\r
+      FeaturePcdGet (PcdCpuSmmProfileEnable))\r
+  {\r
+    if (sizeof (UINTN) == sizeof (UINT64)) {\r
+      //\r
+      // Restriction on access to non-SMRAM memory and heap guard could not be enabled at the same time.\r
+      //\r
+      ASSERT (\r
+        !(IsRestrictedMemoryAccess () &&\r
+          (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0)\r
+        );\r
+\r
+      //\r
+      // Restriction on access to non-SMRAM memory and SMM profile could not be enabled at the same time.\r
+      //\r
+      ASSERT (!(IsRestrictedMemoryAccess () && FeaturePcdGet (PcdCpuSmmProfileEnable)));\r
+    }\r
+\r
+    return FALSE;\r
+  }\r
+\r
+  return TRUE;\r
+}\r
+\r
+/**\r
+  This function sets memory attribute for page table.\r
+**/\r
+VOID\r
+SetPageTableAttributes (\r
+  VOID\r
+  )\r
+{\r
+  BOOLEAN  CetEnabled;\r
+\r
+  if (!IfReadOnlyPageTableNeeded ()) {\r
+    return;\r
+  }\r
+\r
+  DEBUG ((DEBUG_INFO, "SetPageTableAttributes\n"));\r
+\r
+  //\r
+  // Disable write protection, because we need mark page table to be write protected.\r
+  // We need *write* page table memory, to mark itself to be *read only*.\r
+  //\r
+  CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;\r
+  if (CetEnabled) {\r
+    //\r
+    // CET must be disabled if WP is disabled.\r
+    //\r
+    DisableCet ();\r
+  }\r
+\r
+  AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);\r
+\r
+  // Set memory used by page table as Read Only.\r
+  DEBUG ((DEBUG_INFO, "Start...\n"));\r
+  EnablePageTableProtection ();\r
+\r
+  //\r
+  // Enable write protection, after page table attribute updated.\r
+  //\r
+  AsmWriteCr0 (AsmReadCr0 () | CR0_WP);\r
+  mIsReadOnlyPageTable = TRUE;\r
+\r
+  //\r
+  // Flush TLB after mark all page table pool as read only.\r
+  //\r
+  FlushTlbForAll ();\r
+\r
+  if (CetEnabled) {\r
+    //\r
+    // re-enable CET.\r
+    //\r
+    EnableCet ();\r
+  }\r
+\r
+  return;\r
+}\r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 8d42d89801a3de05a961768c88c0bd5302fa6d35..3deb1ffd6755920875025d3c604bd1f696983314 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -1139,159 +1139,6 @@ Exit:
   ReleaseSpinLock (mPFLock);\r
 }\r
 \r
-/**\r
-  This function sets memory attribute for page table.\r
-**/\r
-VOID\r
-SetPageTableAttributes (\r
-  VOID\r
-  )\r
-{\r
-  UINTN     Index2;\r
-  UINTN     Index3;\r
-  UINTN     Index4;\r
-  UINTN     Index5;\r
-  UINT64    *L1PageTable;\r
-  UINT64    *L2PageTable;\r
-  UINT64    *L3PageTable;\r
-  UINT64    *L4PageTable;\r
-  UINT64    *L5PageTable;\r
-  UINTN     PageTableBase;\r
-  BOOLEAN   IsSplitted;\r
-  BOOLEAN   PageTableSplitted;\r
-  BOOLEAN   CetEnabled;\r
-  BOOLEAN   Enable5LevelPaging;\r
-  IA32_CR4  Cr4;\r
-\r
-  //\r
-  // Don't mark page table memory as read-only if\r
-  //  - no restriction on access to non-SMRAM memory; or\r
-  //  - SMM heap guard feature enabled; or\r
-  //      BIT2: SMM page guard enabled\r
-  //      BIT3: SMM pool guard enabled\r
-  //  - SMM profile feature enabled\r
-  //\r
-  if (!mCpuSmmRestrictedMemoryAccess ||\r
-      ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) ||\r
-      FeaturePcdGet (PcdCpuSmmProfileEnable))\r
-  {\r
-    //\r
-    // Restriction on access to non-SMRAM memory and heap guard could not be enabled at the same time.\r
-    //\r
-    ASSERT (\r
-      !(mCpuSmmRestrictedMemoryAccess &&\r
-        (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0)\r
-      );\r
-\r
-    //\r
-    // Restriction on access to non-SMRAM memory and SMM profile could not be enabled at the same time.\r
-    //\r
-    ASSERT (!(mCpuSmmRestrictedMemoryAccess && FeaturePcdGet (PcdCpuSmmProfileEnable)));\r
-    return;\r
-  }\r
-\r
-  DEBUG ((DEBUG_INFO, "SetPageTableAttributes\n"));\r
-\r
-  //\r
-  // Disable write protection, because we need mark page table to be write protected.\r
-  // We need *write* page table memory, to mark itself to be *read only*.\r
-  //\r
-  CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;\r
-  if (CetEnabled) {\r
-    //\r
-    // CET must be disabled if WP is disabled.\r
-    //\r
-    DisableCet ();\r
-  }\r
-\r
-  AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);\r
-\r
-  do {\r
-    DEBUG ((DEBUG_INFO, "Start...\n"));\r
-    PageTableSplitted = FALSE;\r
-    L5PageTable       = NULL;\r
-\r
-    PageTableBase      = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;\r
-    Cr4.UintN          = AsmReadCr4 ();\r
-    Enable5LevelPaging = (BOOLEAN)(Cr4.Bits.LA57 == 1);\r
-\r
-    if (Enable5LevelPaging) {\r
-      L5PageTable = (UINT64 *)PageTableBase;\r
-      SmmSetMemoryAttributesEx (PageTableBase, Enable5LevelPaging, (EFI_PHYSICAL_ADDRESS)PageTableBase, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted);\r
-      PageTableSplitted = (PageTableSplitted || IsSplitted);\r
-    }\r
-\r
-    for (Index5 = 0; Index5 < (Enable5LevelPaging ? SIZE_4KB/sizeof (UINT64) : 1); Index5++) {\r
-      if (Enable5LevelPaging) {\r
-        L4PageTable = (UINT64 *)(UINTN)(L5PageTable[Index5] & ~mAddressEncMask & PAGING_4K_ADDRESS_MASK_64);\r
-        if (L4PageTable == NULL) {\r
-          continue;\r
-        }\r
-      } else {\r
-        L4PageTable = (UINT64 *)PageTableBase;\r
-      }\r
-\r
-      SmmSetMemoryAttributesEx (PageTableBase, Enable5LevelPaging, (EFI_PHYSICAL_ADDRESS)(UINTN)L4PageTable, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted);\r
-      PageTableSplitted = (PageTableSplitted || IsSplitted);\r
-\r
-      for (Index4 = 0; Index4 < SIZE_4KB/sizeof (UINT64); Index4++) {\r
-        L3PageTable = (UINT64 *)(UINTN)(L4PageTable[Index4] & ~mAddressEncMask & PAGING_4K_ADDRESS_MASK_64);\r
-        if (L3PageTable == NULL) {\r
-          continue;\r
-        }\r
-\r
-        SmmSetMemoryAttributesEx (PageTableBase, Enable5LevelPaging, (EFI_PHYSICAL_ADDRESS)(UINTN)L3PageTable, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted);\r
-        PageTableSplitted = (PageTableSplitted || IsSplitted);\r
-\r
-        for (Index3 = 0; Index3 < SIZE_4KB/sizeof (UINT64); Index3++) {\r
-          if ((L3PageTable[Index3] & IA32_PG_PS) != 0) {\r
-            // 1G\r
-            continue;\r
-          }\r
-\r
-          L2PageTable = (UINT64 *)(UINTN)(L3PageTable[Index3] & ~mAddressEncMask & PAGING_4K_ADDRESS_MASK_64);\r
-          if (L2PageTable == NULL) {\r
-            continue;\r
-          }\r
-\r
-          SmmSetMemoryAttributesEx (PageTableBase, Enable5LevelPaging, (EFI_PHYSICAL_ADDRESS)(UINTN)L2PageTable, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted);\r
-          PageTableSplitted = (PageTableSplitted || IsSplitted);\r
-\r
-          for (Index2 = 0; Index2 < SIZE_4KB/sizeof (UINT64); Index2++) {\r
-            if ((L2PageTable[Index2] & IA32_PG_PS) != 0) {\r
-              // 2M\r
-              continue;\r
-            }\r
-\r
-            L1PageTable = (UINT64 *)(UINTN)(L2PageTable[Index2] & ~mAddressEncMask & PAGING_4K_ADDRESS_MASK_64);\r
-            if (L1PageTable == NULL) {\r
-              continue;\r
-            }\r
-\r
-            SmmSetMemoryAttributesEx (PageTableBase, Enable5LevelPaging, (EFI_PHYSICAL_ADDRESS)(UINTN)L1PageTable, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted);\r
-            PageTableSplitted = (PageTableSplitted || IsSplitted);\r
-          }\r
-        }\r
-      }\r
-    }\r
-  } while (PageTableSplitted);\r
-\r
-  //\r
-  // Enable write protection, after page table updated.\r
-  //\r
-  AsmWriteCr0 (AsmReadCr0 () | CR0_WP);\r
-  if (CetEnabled) {\r
-    //\r
-    // re-enable CET.\r
-    //\r
-    EnableCet ();\r
-  }\r
-\r
-  mIsReadOnlyPageTable = TRUE;\r
-\r
-  return;\r
-}\r
-\r
 /**\r
   This function reads CR2 register when on-demand paging is enabled.\r
 \r