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 | 443 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 9 +++++++--
5 files changed, 156 insertions(+), 328 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..73ad9fb017 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,28 +332,30 @@ 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;
+ IA32_MAP_ENTRY *Map;
+ UINTN Count;
+ UINTN Index;
ASSERT (Attributes != 0);
ASSERT ((Attributes & ~EFI_MEMORY_ATTRIBUTE_MASK) == 0);
ASSERT ((BaseAddress & (SIZE_4KB - 1)) == 0);
ASSERT ((Length & (SIZE_4KB - 1)) == 0);
+ ASSERT (PageTableBase != 0);
if (Length == 0) {
return RETURN_INVALID_PARAMETER;
@@ -599,61 +374,121 @@ ConvertMemoryPageAttributes (
return RETURN_UNSUPPORTED;
}
- // DEBUG ((DEBUG_ERROR, "ConvertMemoryPageAttributes(%x) - %016lx, %016lx, %02lx\n", IsSet, BaseAddress, Length, Attributes));
+ PagingAttribute.Uint64 = 0;
+ PagingAttribute.Uint64 = mAddressEncMask | BaseAddress;
+ PagingAttrMask.Uint64 = 0;
- if (IsSplitted != NULL) {
- *IsSplitted = FALSE;
- }
-
- 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;
- }
+ //
+ // When map range to present range, provide all attributes.
+ //
+ PagingAttribute.Bits.Present = 1;
+ PagingAttrMask.Uint64 = MAX_UINT64;
- if (IsSplitted != NULL) {
- *IsSplitted = TRUE;
- }
+ //
+ // By default memory is Ring 3 accessble.
+ //
+ PagingAttribute.Bits.UserSupervisor = 1;
+
+ DEBUG_CODE (
+ if (((Attributes & EFI_MEMORY_RO) == 0) || (((Attributes & EFI_MEMORY_XP) == 0) && (mXdSupported))) {
+ //
+ // When mapping a range to present and EFI_MEMORY_RO or EFI_MEMORY_XP is not specificed,
+ // check if [BaseAddress, BaseAddress + Length] contains present range.
+ // Existing Present range in [BaseAddress, BaseAddress + Length] is set to NX disable and ReadOnly.
+ //
+ Count = 0;
+ Map = NULL;
+ Status = PageTableParse (PageTableBase, mPagingMode, NULL, &Count);
+ while (Status == RETURN_BUFFER_TOO_SMALL) {
+ if (Map != NULL) {
+ FreePool (Map);
+ }
- if (IsModified != NULL) {
- *IsModified = TRUE;
+ Map = AllocatePool (Count * sizeof (IA32_MAP_ENTRY));
+ ASSERT (Map != NULL);
+ Status = PageTableParse (PageTableBase, mPagingMode, Map, &Count);
+ }
+
+ ASSERT_RETURN_ERROR (Status);
+
+ for (Index = 0; Index < Count; Index++) {
+ if ((BaseAddress < Map[Index].LinearAddress +
+ Map[Index].Length) && (BaseAddress + Length > Map[Index].LinearAddress))
+ {
+ DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes: Existing Present range in [0x%lx, 0x%lx] is set to NX disable and ReadOnly\n", BaseAddress, BaseAddress + Length));
+ break;
+ }
+ }
+
+ FreePool (Map);
}
- //
- // Just split current page
- // Convert success in next around
- //
+ );
}
}
+ if (PagingAttrMask.Uint64 == 0) {
+ return RETURN_SUCCESS;
+ }
+
+ PageTableBufferSize = 0;
+ Status = PageTableMap (&PageTableBase, PagingMode, NULL, &PageTableBufferSize, BaseAddress, Length, &PagingAttribute, &PagingAttrMask, IsModified);
+
+ 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);
+ }
+
+ if (Status == RETURN_INVALID_PARAMETER) {
+ //
+ // The only reason that PageTableMap returns RETURN_INVALID_PARAMETER here is to modify other attributes
+ // of a 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));
+ }
+
+ ASSERT_RETURN_ERROR (Status);
+ ASSERT (PageTableBufferSize == 0);
+
return RETURN_SUCCESS;
}
@@ -697,11 +532,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 +554,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 +581,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 +603,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 +654,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 +689,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 +714,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 +738,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 +1622,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.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104925): https://edk2.groups.io/g/devel/message/104925
Mute This Topic: https://groups.io/mt/98922930/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
> 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);
1. this change assumes the default value in assembly is 0 while old logic doesn't
have such assumption. Can you patch the instruction no matter m5LevelPagingNeeded?
> + DEBUG_CODE (
> + if (((Attributes & EFI_MEMORY_RO) == 0) || (((Attributes &
> EFI_MEMORY_XP) == 0) && (mXdSupported))) {
> + //
> + // When mapping a range to present and EFI_MEMORY_RO or
> EFI_MEMORY_XP is not specificed,
> + // check if [BaseAddress, BaseAddress + Length] contains present range.
> + // Existing Present range in [BaseAddress, BaseAddress + Length] is set to
> NX disable and ReadOnly.
> + //
> + Count = 0;
> + Map = NULL;
> + Status = PageTableParse (PageTableBase, mPagingMode, NULL, &Count);
> + while (Status == RETURN_BUFFER_TOO_SMALL) {
> + if (Map != NULL) {
> + FreePool (Map);
> + }
>
> - if (IsModified != NULL) {
> - *IsModified = TRUE;
> + Map = AllocatePool (Count * sizeof (IA32_MAP_ENTRY));
> + ASSERT (Map != NULL);
> + Status = PageTableParse (PageTableBase, mPagingMode, Map, &Count);
> + }
> +
> + ASSERT_RETURN_ERROR (Status);
> +
> + for (Index = 0; Index < Count; Index++) {
> + if ((BaseAddress < Map[Index].LinearAddress +
> + Map[Index].Length) && (BaseAddress + Length >
> Map[Index].LinearAddress))
> + {
> + DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes:
> Existing Present range in [0x%lx, 0x%lx] is set to NX disable and ReadOnly\n",
> BaseAddress, BaseAddress + Length));
> + break;
> + }
> + }
> +
> + FreePool (Map);
> }
2. What's the purpose of the above DEBUG_CODE()?
Because when mapping a range of memory from not-present to present,
the function clears all other attributes but only set the "present" bit.
If part of the range is "present" already, the function might reset
its other attributes. This is not expected by caller.
So, you want to notify caller?
Can you split this logic to a separate commit?
If the sub-range's attributes match to what you are going to set
for the entire range, caller can ignore such error message, right?
>
> - //
> - // Just split current page
> - // Convert success in next around
> - //
> + );
> }
> }
>
> + if (PagingAttrMask.Uint64 == 0) {
> + return RETURN_SUCCESS;
> + }
> +
> + PageTableBufferSize = 0;
> + Status = PageTableMap (&PageTableBase, PagingMode, NULL,
> &PageTableBufferSize, BaseAddress, Length, &PagingAttribute,
> &PagingAttrMask, IsModified);
> +
> + 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);
> + }
> +
> + if (Status == RETURN_INVALID_PARAMETER) {
> + //
> + // The only reason that PageTableMap returns
> RETURN_INVALID_PARAMETER here is to modify other attributes
> + // of a 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));
3. Don't quite understand. Can you describe in a clearer way?
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105520): https://edk2.groups.io/g/devel/message/105520
Mute This Topic: https://groups.io/mt/98922930/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.