public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "duntan" <dun.tan@intel.com>
To: devel@edk2.groups.io
Cc: Eric Dong <eric.dong@intel.com>, Ray Ni <ray.ni@intel.com>,
	Rahul Kumar <rahul1.kumar@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>
Subject: [Patch V2 3/8] UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute.
Date: Wed, 12 Apr 2023 16:53:38 +0800	[thread overview]
Message-ID: <20230412085343.1077-4-dun.tan@intel.com> (raw)
In-Reply-To: <20230412085343.1077-1-dun.tan@intel.com>

Simplify the ConvertMemoryPageAttributes API to convert paging
attribute by CpuPageTableLib. In the new API, it calls
PageTableMap() to update the page attributes of a memory range.
With the PageTableMap() API in CpuPageTableLib, we can remove
the complicated page table manipulating code.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c           |   3 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h         |  28 +++++++++++++---------------
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf       |   1 +
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 403 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c            |   9 +++++++--
 5 files changed, 114 insertions(+), 330 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index 34bf6e1a25..9c8107080a 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -1,7 +1,7 @@
 /** @file
 Page table manipulation functions for IA-32 processors
 
-Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -31,6 +31,7 @@ SmmInitPageTable (
   InitializeSpinLock (mPFLock);
 
   mPhysicalAddressBits = 32;
+  mPagingMode          = PagingPae;
 
   if (FeaturePcdGet (PcdCpuSmmProfileEnable) ||
       HEAP_GUARD_NONSTOP_MODE ||
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index a5c2bdd971..ba341cadc6 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -50,6 +50,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/SmmCpuFeaturesLib.h>
 #include <Library/PeCoffGetEntryPointLib.h>
 #include <Library/RegisterCpuFeaturesLib.h>
+#include <Library/CpuPageTableLib.h>
 
 #include <AcpiCpuData.h>
 #include <CpuHotPlugData.h>
@@ -260,6 +261,7 @@ extern UINTN                 mNumberOfCpus;
 extern EFI_SMM_CPU_PROTOCOL  mSmmCpu;
 extern EFI_MM_MP_PROTOCOL    mSmmMp;
 extern BOOLEAN               m5LevelPagingNeeded;
+extern PAGING_MODE           mPagingMode;
 
 ///
 /// The mode of the CPU at the time an SMI occurs
@@ -1008,11 +1010,10 @@ SetPageTableAttributes (
   Length from their current attributes to the attributes specified by Attributes.
 
   @param[in]   PageTableBase    The page table base.
-  @param[in]   EnablePML5Paging If PML5 paging is enabled.
+  @param[in]   PagingMode       The paging mode.
   @param[in]   BaseAddress      The physical address that is the start address of a memory region.
   @param[in]   Length           The size in bytes of the memory region.
   @param[in]   Attributes       The bit mask of attributes to set for the memory region.
-  @param[out]  IsSplitted       TRUE means page table splitted. FALSE means page table not splitted.
 
   @retval EFI_SUCCESS           The attributes were set for the memory region.
   @retval EFI_ACCESS_DENIED     The attributes for the memory resource range specified by
@@ -1030,12 +1031,11 @@ SetPageTableAttributes (
 **/
 EFI_STATUS
 SmmSetMemoryAttributesEx (
-  IN  UINTN                 PageTableBase,
-  IN  BOOLEAN               EnablePML5Paging,
-  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
-  IN  UINT64                Length,
-  IN  UINT64                Attributes,
-  OUT BOOLEAN               *IsSplitted  OPTIONAL
+  IN  UINTN             PageTableBase,
+  IN  PAGING_MODE       PagingMode,
+  IN  PHYSICAL_ADDRESS  BaseAddress,
+  IN  UINT64            Length,
+  IN  UINT64            Attributes
   );
 
 /**
@@ -1043,34 +1043,32 @@ SmmSetMemoryAttributesEx (
   Length from their current attributes to the attributes specified by Attributes.
 
   @param[in]   PageTableBase    The page table base.
-  @param[in]   EnablePML5Paging If PML5 paging is enabled.
+  @param[in]   PagingMode       The paging mode.
   @param[in]   BaseAddress      The physical address that is the start address of a memory region.
   @param[in]   Length           The size in bytes of the memory region.
   @param[in]   Attributes       The bit mask of attributes to clear for the memory region.
-  @param[out]  IsSplitted       TRUE means page table splitted. FALSE means page table not splitted.
 
   @retval EFI_SUCCESS           The attributes were cleared for the memory region.
   @retval EFI_ACCESS_DENIED     The attributes for the memory resource range specified by
                                 BaseAddress and Length cannot be modified.
   @retval EFI_INVALID_PARAMETER Length is zero.
                                 Attributes specified an illegal combination of attributes that
-                                cannot be set together.
+                                cannot be cleared together.
   @retval EFI_OUT_OF_RESOURCES  There are not enough system resources to modify the attributes of
                                 the memory resource range.
   @retval EFI_UNSUPPORTED       The processor does not support one or more bytes of the memory
                                 resource range specified by BaseAddress and Length.
-                                The bit mask of attributes is not support for the memory resource
+                                The bit mask of attributes is not supported for the memory resource
                                 range specified by BaseAddress and Length.
 
 **/
 EFI_STATUS
 SmmClearMemoryAttributesEx (
   IN  UINTN                 PageTableBase,
-  IN  BOOLEAN               EnablePML5Paging,
+  IN  PAGING_MODE           PagingMode,
   IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
   IN  UINT64                Length,
-  IN  UINT64                Attributes,
-  OUT BOOLEAN               *IsSplitted  OPTIONAL
+  IN  UINT64                Attributes
   );
 
 /**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 158e05e264..38d4e950a4 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -97,6 +97,7 @@
   ReportStatusCodeLib
   SmmCpuFeaturesLib
   PeCoffGetEntryPointLib
+  CpuPageTableLib
 
 [Protocols]
   gEfiSmmAccess2ProtocolGuid               ## CONSUMES
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 834a756061..deb5895d83 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2016 - 2023, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -26,14 +26,9 @@ UINTN                            mGcdMemNumberOfDesc = 0;
 
 EFI_MEMORY_ATTRIBUTES_TABLE  *mUefiMemoryAttributesTable = NULL;
 
-PAGE_ATTRIBUTE_TABLE  mPageAttributeTable[] = {
-  { Page4K, SIZE_4KB, PAGING_4K_ADDRESS_MASK_64 },
-  { Page2M, SIZE_2MB, PAGING_2M_ADDRESS_MASK_64 },
-  { Page1G, SIZE_1GB, PAGING_1G_ADDRESS_MASK_64 },
-};
-
-BOOLEAN  mIsShadowStack      = FALSE;
-BOOLEAN  m5LevelPagingNeeded = FALSE;
+BOOLEAN      mIsShadowStack      = FALSE;
+BOOLEAN      m5LevelPagingNeeded = FALSE;
+PAGING_MODE  mPagingMode         = PagingModeMax;
 
 //
 // Global variable to keep track current available memory used as page table.
@@ -185,52 +180,6 @@ AllocatePageTableMemory (
   return Buffer;
 }
 
-/**
-  Return length according to page attributes.
-
-  @param[in]  PageAttributes   The page attribute of the page entry.
-
-  @return The length of page entry.
-**/
-UINTN
-PageAttributeToLength (
-  IN PAGE_ATTRIBUTE  PageAttribute
-  )
-{
-  UINTN  Index;
-
-  for (Index = 0; Index < sizeof (mPageAttributeTable)/sizeof (mPageAttributeTable[0]); Index++) {
-    if (PageAttribute == mPageAttributeTable[Index].Attribute) {
-      return (UINTN)mPageAttributeTable[Index].Length;
-    }
-  }
-
-  return 0;
-}
-
-/**
-  Return address mask according to page attributes.
-
-  @param[in]  PageAttributes   The page attribute of the page entry.
-
-  @return The address mask of page entry.
-**/
-UINTN
-PageAttributeToMask (
-  IN PAGE_ATTRIBUTE  PageAttribute
-  )
-{
-  UINTN  Index;
-
-  for (Index = 0; Index < sizeof (mPageAttributeTable)/sizeof (mPageAttributeTable[0]); Index++) {
-    if (PageAttribute == mPageAttributeTable[Index].Attribute) {
-      return (UINTN)mPageAttributeTable[Index].AddressMask;
-    }
-  }
-
-  return 0;
-}
-
 /**
   Return page table entry to match the address.
 
@@ -353,181 +302,6 @@ GetAttributesFromPageEntry (
   return Attributes;
 }
 
-/**
-  Modify memory attributes of page entry.
-
-  @param[in]   PageEntry        The page entry.
-  @param[in]   Attributes       The bit mask of attributes to modify for the memory region.
-  @param[in]   IsSet            TRUE means to set attributes. FALSE means to clear attributes.
-  @param[out]  IsModified       TRUE means page table modified. FALSE means page table not modified.
-**/
-VOID
-ConvertPageEntryAttribute (
-  IN  UINT64   *PageEntry,
-  IN  UINT64   Attributes,
-  IN  BOOLEAN  IsSet,
-  OUT BOOLEAN  *IsModified
-  )
-{
-  UINT64  CurrentPageEntry;
-  UINT64  NewPageEntry;
-
-  CurrentPageEntry = *PageEntry;
-  NewPageEntry     = CurrentPageEntry;
-  if ((Attributes & EFI_MEMORY_RP) != 0) {
-    if (IsSet) {
-      NewPageEntry &= ~(UINT64)IA32_PG_P;
-    } else {
-      NewPageEntry |= IA32_PG_P;
-    }
-  }
-
-  if ((Attributes & EFI_MEMORY_RO) != 0) {
-    if (IsSet) {
-      NewPageEntry &= ~(UINT64)IA32_PG_RW;
-      if (mIsShadowStack) {
-        // Environment setup
-        // ReadOnly page need set Dirty bit for shadow stack
-        NewPageEntry |= IA32_PG_D;
-        // Clear user bit for supervisor shadow stack
-        NewPageEntry &= ~(UINT64)IA32_PG_U;
-      } else {
-        // Runtime update
-        // Clear dirty bit for non shadow stack, to protect RO page.
-        NewPageEntry &= ~(UINT64)IA32_PG_D;
-      }
-    } else {
-      NewPageEntry |= IA32_PG_RW;
-    }
-  }
-
-  if ((Attributes & EFI_MEMORY_XP) != 0) {
-    if (mXdSupported) {
-      if (IsSet) {
-        NewPageEntry |= IA32_PG_NX;
-      } else {
-        NewPageEntry &= ~IA32_PG_NX;
-      }
-    }
-  }
-
-  *PageEntry = NewPageEntry;
-  if (CurrentPageEntry != NewPageEntry) {
-    *IsModified = TRUE;
-    DEBUG ((DEBUG_VERBOSE, "ConvertPageEntryAttribute 0x%lx", CurrentPageEntry));
-    DEBUG ((DEBUG_VERBOSE, "->0x%lx\n", NewPageEntry));
-  } else {
-    *IsModified = FALSE;
-  }
-}
-
-/**
-  This function returns if there is need to split page entry.
-
-  @param[in]  BaseAddress      The base address to be checked.
-  @param[in]  Length           The length to be checked.
-  @param[in]  PageEntry        The page entry to be checked.
-  @param[in]  PageAttribute    The page attribute of the page entry.
-
-  @retval SplitAttributes on if there is need to split page entry.
-**/
-PAGE_ATTRIBUTE
-NeedSplitPage (
-  IN  PHYSICAL_ADDRESS  BaseAddress,
-  IN  UINT64            Length,
-  IN  UINT64            *PageEntry,
-  IN  PAGE_ATTRIBUTE    PageAttribute
-  )
-{
-  UINT64  PageEntryLength;
-
-  PageEntryLength = PageAttributeToLength (PageAttribute);
-
-  if (((BaseAddress & (PageEntryLength - 1)) == 0) && (Length >= PageEntryLength)) {
-    return PageNone;
-  }
-
-  if (((BaseAddress & PAGING_2M_MASK) != 0) || (Length < SIZE_2MB)) {
-    return Page4K;
-  }
-
-  return Page2M;
-}
-
-/**
-  This function splits one page entry to small page entries.
-
-  @param[in]  PageEntry        The page entry to be splitted.
-  @param[in]  PageAttribute    The page attribute of the page entry.
-  @param[in]  SplitAttribute   How to split the page entry.
-
-  @retval RETURN_SUCCESS            The page entry is splitted.
-  @retval RETURN_UNSUPPORTED        The page entry does not support to be splitted.
-  @retval RETURN_OUT_OF_RESOURCES   No resource to split page entry.
-**/
-RETURN_STATUS
-SplitPage (
-  IN  UINT64          *PageEntry,
-  IN  PAGE_ATTRIBUTE  PageAttribute,
-  IN  PAGE_ATTRIBUTE  SplitAttribute
-  )
-{
-  UINT64  BaseAddress;
-  UINT64  *NewPageEntry;
-  UINTN   Index;
-
-  ASSERT (PageAttribute == Page2M || PageAttribute == Page1G);
-
-  if (PageAttribute == Page2M) {
-    //
-    // Split 2M to 4K
-    //
-    ASSERT (SplitAttribute == Page4K);
-    if (SplitAttribute == Page4K) {
-      NewPageEntry = AllocatePageTableMemory (1);
-      DEBUG ((DEBUG_VERBOSE, "Split - 0x%x\n", NewPageEntry));
-      if (NewPageEntry == NULL) {
-        return RETURN_OUT_OF_RESOURCES;
-      }
-
-      BaseAddress = *PageEntry & PAGING_2M_ADDRESS_MASK_64;
-      for (Index = 0; Index < SIZE_4KB / sizeof (UINT64); Index++) {
-        NewPageEntry[Index] = (BaseAddress + SIZE_4KB * Index) | mAddressEncMask | ((*PageEntry) & PAGE_PROGATE_BITS);
-      }
-
-      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
-      return RETURN_SUCCESS;
-    } else {
-      return RETURN_UNSUPPORTED;
-    }
-  } else if (PageAttribute == Page1G) {
-    //
-    // Split 1G to 2M
-    // No need support 1G->4K directly, we should use 1G->2M, then 2M->4K to get more compact page table.
-    //
-    ASSERT (SplitAttribute == Page2M || SplitAttribute == Page4K);
-    if (((SplitAttribute == Page2M) || (SplitAttribute == Page4K))) {
-      NewPageEntry = AllocatePageTableMemory (1);
-      DEBUG ((DEBUG_VERBOSE, "Split - 0x%x\n", NewPageEntry));
-      if (NewPageEntry == NULL) {
-        return RETURN_OUT_OF_RESOURCES;
-      }
-
-      BaseAddress = *PageEntry & PAGING_1G_ADDRESS_MASK_64;
-      for (Index = 0; Index < SIZE_4KB / sizeof (UINT64); Index++) {
-        NewPageEntry[Index] = (BaseAddress + SIZE_2MB * Index) | mAddressEncMask | IA32_PG_PS | ((*PageEntry) & PAGE_PROGATE_BITS);
-      }
-
-      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
-      return RETURN_SUCCESS;
-    } else {
-      return RETURN_UNSUPPORTED;
-    }
-  } else {
-    return RETURN_UNSUPPORTED;
-  }
-}
-
 /**
   This function modifies the page attributes for the memory region specified by BaseAddress and
   Length from their current attributes to the attributes specified by Attributes.
@@ -535,12 +309,11 @@ SplitPage (
   Caller should make sure BaseAddress and Length is at page boundary.
 
   @param[in]   PageTableBase    The page table base.
-  @param[in]   EnablePML5Paging If PML5 paging is enabled.
+  @param[in]   PagingMode       The paging mode.
   @param[in]   BaseAddress      The physical address that is the start address of a memory region.
   @param[in]   Length           The size in bytes of the memory region.
   @param[in]   Attributes       The bit mask of attributes to modify for the memory region.
   @param[in]   IsSet            TRUE means to set attributes. FALSE means to clear attributes.
-  @param[out]  IsSplitted       TRUE means page table splitted. FALSE means page table not splitted.
   @param[out]  IsModified       TRUE means page table modified. FALSE means page table not modified.
 
   @retval RETURN_SUCCESS           The attributes were modified for the memory region.
@@ -559,21 +332,19 @@ SplitPage (
 RETURN_STATUS
 ConvertMemoryPageAttributes (
   IN  UINTN             PageTableBase,
-  IN  BOOLEAN           EnablePML5Paging,
+  IN  PAGING_MODE       PagingMode,
   IN  PHYSICAL_ADDRESS  BaseAddress,
   IN  UINT64            Length,
   IN  UINT64            Attributes,
   IN  BOOLEAN           IsSet,
-  OUT BOOLEAN           *IsSplitted   OPTIONAL,
   OUT BOOLEAN           *IsModified   OPTIONAL
   )
 {
-  UINT64                *PageEntry;
-  PAGE_ATTRIBUTE        PageAttribute;
-  UINTN                 PageEntryLength;
-  PAGE_ATTRIBUTE        SplitAttribute;
   RETURN_STATUS         Status;
-  BOOLEAN               IsEntryModified;
+  IA32_MAP_ATTRIBUTE    PagingAttribute;
+  IA32_MAP_ATTRIBUTE    PagingAttrMask;
+  UINTN                 PageTableBufferSize;
+  VOID                  *PageTableBuffer;
   EFI_PHYSICAL_ADDRESS  MaximumSupportMemAddress;
 
   ASSERT (Attributes != 0);
@@ -581,6 +352,7 @@ ConvertMemoryPageAttributes (
 
   ASSERT ((BaseAddress & (SIZE_4KB - 1)) == 0);
   ASSERT ((Length & (SIZE_4KB - 1)) == 0);
+  ASSERT (PageTableBase != 0);
 
   if (Length == 0) {
     return RETURN_INVALID_PARAMETER;
@@ -599,61 +371,80 @@ ConvertMemoryPageAttributes (
     return RETURN_UNSUPPORTED;
   }
 
-  //  DEBUG ((DEBUG_ERROR, "ConvertMemoryPageAttributes(%x) - %016lx, %016lx, %02lx\n", IsSet, BaseAddress, Length, Attributes));
-
-  if (IsSplitted != NULL) {
-    *IsSplitted = FALSE;
-  }
+  PagingAttribute.Uint64 = 0;
+  PagingAttribute.Uint64 = mAddressEncMask | BaseAddress;
+  PagingAttrMask.Uint64  = 0;
 
-  if (IsModified != NULL) {
-    *IsModified = FALSE;
+  if ((Attributes & EFI_MEMORY_RO) != 0) {
+    PagingAttrMask.Bits.ReadWrite = 1;
+    if (IsSet) {
+      PagingAttribute.Bits.ReadWrite = 0;
+      PagingAttrMask.Bits.Dirty      = 1;
+      if (mIsShadowStack) {
+        // Environment setup
+        // ReadOnly page need set Dirty bit for shadow stack
+        PagingAttribute.Bits.Dirty = 1;
+        // Clear user bit for supervisor shadow stack
+        PagingAttribute.Bits.UserSupervisor = 0;
+        PagingAttrMask.Bits.UserSupervisor  = 1;
+      } else {
+        // Runtime update
+        // Clear dirty bit for non shadow stack, to protect RO page.
+        PagingAttribute.Bits.Dirty = 0;
+      }
+    } else {
+      PagingAttribute.Bits.ReadWrite = 1;
+    }
   }
 
-  //
-  // Below logic is to check 2M/4K page to make sure we do not waste memory.
-  //
-  while (Length != 0) {
-    PageEntry = GetPageTableEntry (PageTableBase, EnablePML5Paging, BaseAddress, &PageAttribute);
-    if (PageEntry == NULL) {
-      return RETURN_UNSUPPORTED;
+  if ((Attributes & EFI_MEMORY_XP) != 0) {
+    if (mXdSupported) {
+      PagingAttribute.Bits.Nx = IsSet ? 1 : 0;
+      PagingAttrMask.Bits.Nx  = 1;
     }
+  }
 
-    PageEntryLength = PageAttributeToLength (PageAttribute);
-    SplitAttribute  = NeedSplitPage (BaseAddress, Length, PageEntry, PageAttribute);
-    if (SplitAttribute == PageNone) {
-      ConvertPageEntryAttribute (PageEntry, Attributes, IsSet, &IsEntryModified);
-      if (IsEntryModified) {
-        if (IsModified != NULL) {
-          *IsModified = TRUE;
-        }
-      }
-
+  if ((Attributes & EFI_MEMORY_RP) != 0) {
+    if (IsSet) {
+      PagingAttribute.Bits.Present = 0;
       //
-      // Convert success, move to next
+      // When map a range to non-present, all attributes except Present should not be provided.
       //
-      BaseAddress += PageEntryLength;
-      Length      -= PageEntryLength;
+      PagingAttrMask.Uint64       = 0;
+      PagingAttrMask.Bits.Present = 1;
     } else {
-      Status = SplitPage (PageEntry, PageAttribute, SplitAttribute);
-      if (RETURN_ERROR (Status)) {
-        return RETURN_UNSUPPORTED;
-      }
-
-      if (IsSplitted != NULL) {
-        *IsSplitted = TRUE;
-      }
-
-      if (IsModified != NULL) {
-        *IsModified = TRUE;
-      }
-
       //
-      // Just split current page
-      // Convert success in next around
+      // When map range to present range, provide all attributes.
       //
+      PagingAttribute.Bits.Present = 1;
+      PagingAttrMask.Uint64        = MAX_UINT64;
     }
   }
 
+  if (PagingAttrMask.Uint64 == 0) {
+    return RETURN_SUCCESS;
+  }
+
+  PageTableBufferSize = 0;
+  Status              = PageTableMap (&PageTableBase, PagingMode, NULL, &PageTableBufferSize, BaseAddress, Length, &PagingAttribute, &PagingAttrMask, IsModified);
+
+  if (Status == RETURN_INVALID_PARAMETER) {
+    //
+    // The only reason that PageTableMap returns RETURN_INVALID_PARAMETER here is to modify other attributes
+    // of non-present range but remains the non-present range still as non-present.
+    //
+    DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes: Non-present range in [0x%lx, 0x%lx] needs to be removed\n", BaseAddress, BaseAddress + Length));
+  }
+
+  if (Status == RETURN_BUFFER_TOO_SMALL) {
+    PageTableBuffer = AllocatePageTableMemory (EFI_SIZE_TO_PAGES (PageTableBufferSize));
+    ASSERT (PageTableBuffer != NULL);
+    Status = PageTableMap (&PageTableBase, PagingMode, PageTableBuffer, &PageTableBufferSize, BaseAddress, Length, &PagingAttribute, &PagingAttrMask, IsModified);
+  }
+
+  ASSERT_RETURN_ERROR (Status);
+  ASSERT (PageTableBufferSize == 0);
+
   return RETURN_SUCCESS;
 }
 
@@ -697,11 +488,10 @@ FlushTlbForAll (
   Length from their current attributes to the attributes specified by Attributes.
 
   @param[in]   PageTableBase    The page table base.
-  @param[in]   EnablePML5Paging If PML5 paging is enabled.
+  @param[in]   PagingMode       The paging mode.
   @param[in]   BaseAddress      The physical address that is the start address of a memory region.
   @param[in]   Length           The size in bytes of the memory region.
   @param[in]   Attributes       The bit mask of attributes to set for the memory region.
-  @param[out]  IsSplitted       TRUE means page table splitted. FALSE means page table not splitted.
 
   @retval EFI_SUCCESS           The attributes were set for the memory region.
   @retval EFI_ACCESS_DENIED     The attributes for the memory resource range specified by
@@ -720,17 +510,16 @@ FlushTlbForAll (
 EFI_STATUS
 SmmSetMemoryAttributesEx (
   IN  UINTN                 PageTableBase,
-  IN  BOOLEAN               EnablePML5Paging,
+  IN  PAGING_MODE           PagingMode,
   IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
   IN  UINT64                Length,
-  IN  UINT64                Attributes,
-  OUT BOOLEAN               *IsSplitted  OPTIONAL
+  IN  UINT64                Attributes
   )
 {
   EFI_STATUS  Status;
   BOOLEAN     IsModified;
 
-  Status = ConvertMemoryPageAttributes (PageTableBase, EnablePML5Paging, BaseAddress, Length, Attributes, TRUE, IsSplitted, &IsModified);
+  Status = ConvertMemoryPageAttributes (PageTableBase, PagingMode, BaseAddress, Length, Attributes, TRUE, &IsModified);
   if (!EFI_ERROR (Status)) {
     if (IsModified) {
       //
@@ -748,11 +537,10 @@ SmmSetMemoryAttributesEx (
   Length from their current attributes to the attributes specified by Attributes.
 
   @param[in]   PageTableBase    The page table base.
-  @param[in]   EnablePML5Paging If PML5 paging is enabled.
+  @param[in]   PagingMode       The paging mode.
   @param[in]   BaseAddress      The physical address that is the start address of a memory region.
   @param[in]   Length           The size in bytes of the memory region.
   @param[in]   Attributes       The bit mask of attributes to clear for the memory region.
-  @param[out]  IsSplitted       TRUE means page table splitted. FALSE means page table not splitted.
 
   @retval EFI_SUCCESS           The attributes were cleared for the memory region.
   @retval EFI_ACCESS_DENIED     The attributes for the memory resource range specified by
@@ -771,17 +559,16 @@ SmmSetMemoryAttributesEx (
 EFI_STATUS
 SmmClearMemoryAttributesEx (
   IN  UINTN                 PageTableBase,
-  IN  BOOLEAN               EnablePML5Paging,
+  IN  PAGING_MODE           PagingMode,
   IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
   IN  UINT64                Length,
-  IN  UINT64                Attributes,
-  OUT BOOLEAN               *IsSplitted  OPTIONAL
+  IN  UINT64                Attributes
   )
 {
   EFI_STATUS  Status;
   BOOLEAN     IsModified;
 
-  Status = ConvertMemoryPageAttributes (PageTableBase, EnablePML5Paging, BaseAddress, Length, Attributes, FALSE, IsSplitted, &IsModified);
+  Status = ConvertMemoryPageAttributes (PageTableBase, PagingMode, BaseAddress, Length, Attributes, FALSE, &IsModified);
   if (!EFI_ERROR (Status)) {
     if (IsModified) {
       //
@@ -823,14 +610,10 @@ SmmSetMemoryAttributes (
   IN  UINT64                Attributes
   )
 {
-  IA32_CR4  Cr4;
-  UINTN     PageTableBase;
-  BOOLEAN   Enable5LevelPaging;
-
-  PageTableBase      = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;
-  Cr4.UintN          = AsmReadCr4 ();
-  Enable5LevelPaging = (BOOLEAN)(Cr4.Bits.LA57 == 1);
-  return SmmSetMemoryAttributesEx (PageTableBase, Enable5LevelPaging, BaseAddress, Length, Attributes, NULL);
+  UINTN  PageTableBase;
+
+  PageTableBase = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;
+  return SmmSetMemoryAttributesEx (PageTableBase, mPagingMode, BaseAddress, Length, Attributes);
 }
 
 /**
@@ -862,14 +645,10 @@ SmmClearMemoryAttributes (
   IN  UINT64                Attributes
   )
 {
-  IA32_CR4  Cr4;
-  UINTN     PageTableBase;
-  BOOLEAN   Enable5LevelPaging;
-
-  PageTableBase      = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;
-  Cr4.UintN          = AsmReadCr4 ();
-  Enable5LevelPaging = (BOOLEAN)(Cr4.Bits.LA57 == 1);
-  return SmmClearMemoryAttributesEx (PageTableBase, Enable5LevelPaging, BaseAddress, Length, Attributes, NULL);
+  UINTN  PageTableBase;
+
+  PageTableBase = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;
+  return SmmClearMemoryAttributesEx (PageTableBase, mPagingMode, BaseAddress, Length, Attributes);
 }
 
 /**
@@ -891,7 +670,7 @@ SetShadowStack (
   EFI_STATUS  Status;
 
   mIsShadowStack = TRUE;
-  Status         = SmmSetMemoryAttributesEx (Cr3, m5LevelPagingNeeded, BaseAddress, Length, EFI_MEMORY_RO, NULL);
+  Status         = SmmSetMemoryAttributesEx (Cr3, mPagingMode, BaseAddress, Length, EFI_MEMORY_RO);
   mIsShadowStack = FALSE;
 
   return Status;
@@ -915,7 +694,7 @@ SetNotPresentPage (
 {
   EFI_STATUS  Status;
 
-  Status = SmmSetMemoryAttributesEx (Cr3, m5LevelPagingNeeded, BaseAddress, Length, EFI_MEMORY_RP, NULL);
+  Status = SmmSetMemoryAttributesEx (Cr3, mPagingMode, BaseAddress, Length, EFI_MEMORY_RP);
   return Status;
 }
 
@@ -1799,7 +1578,7 @@ EnablePageTableProtection (
     //
     // Set entire pool including header, used-memory and left free-memory as ReadOnly in SMM page table.
     //
-    ConvertMemoryPageAttributes (PageTableBase, m5LevelPagingNeeded, Address, PoolSize, EFI_MEMORY_RO, TRUE, NULL, NULL);
+    ConvertMemoryPageAttributes (PageTableBase, mPagingMode, Address, PoolSize, EFI_MEMORY_RO, TRUE, NULL);
     Pool = Pool->NextPool;
   } while (Pool != HeadPool);
 }
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 3deb1ffd67..a25a96f68c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -1,7 +1,7 @@
 /** @file
 Page Fault (#PF) handler for X64 processors
 
-Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -353,7 +353,12 @@ SmmInitPageTable (
   m1GPageTableSupport           = Is1GPageSupport ();
   m5LevelPagingNeeded           = Is5LevelPagingNeeded ();
   mPhysicalAddressBits          = CalculateMaximumSupportAddress ();
-  PatchInstructionX86 (gPatch5LevelPagingNeeded, m5LevelPagingNeeded, 1);
+  if (m5LevelPagingNeeded) {
+    mPagingMode = m1GPageTableSupport ? Paging5Level1GB : Paging5Level;
+    PatchInstructionX86 (gPatch5LevelPagingNeeded, TRUE, 1);
+  } else {
+    mPagingMode = m1GPageTableSupport ? Paging4Level1GB : Paging4Level;
+  }
   DEBUG ((DEBUG_INFO, "5LevelPaging Needed             - %d\n", m5LevelPagingNeeded));
   DEBUG ((DEBUG_INFO, "1GPageTable Support             - %d\n", m1GPageTableSupport));
   DEBUG ((DEBUG_INFO, "PcdCpuSmmRestrictedMemoryAccess - %d\n", mCpuSmmRestrictedMemoryAccess));
-- 
2.39.1.windows.1


  parent reply	other threads:[~2023-04-12  8:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-12  8:53 [Patch V2 0/8] Use CpuPageTableLib to create and update smm page table duntan
2023-04-12  8:53 ` [Patch V2 1/8] OvmfPkg: Add CpuPageTableLib required by PiSmmCpuDxe duntan
2023-04-13  6:42   ` Gerd Hoffmann
2023-04-12  8:53 ` [Patch V2 2/8] UefiPayloadPkg: " duntan
2023-04-12  8:53 ` duntan [this message]
2023-04-12  8:53 ` [Patch V2 4/8] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX duntan
2023-04-12  8:53 ` [Patch V2 5/8] UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h duntan
2023-04-12  8:53 ` [Patch V2 6/8] UefiCpuPkg: Refinement to current smm page table generation code duntan
2023-04-12  8:53 ` [Patch V2 7/8] UefiCpuPkg: Refinement to code about updating smm page table duntan
2023-04-12  8:53 ` [Patch V2 8/8] UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function duntan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230412085343.1077-4-dun.tan@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox