* [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table
@ 2023-06-08 2:27 duntan
2023-06-08 2:27 ` [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry duntan
` (13 more replies)
0 siblings, 14 replies; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel
In V5 patch set:
1.Remove duplicated patches to add CpuPageTableLib into some DSC files.
2.Seperate the DEBUG_CODE of ConvertMemoryPageAttributes() to a new patch 'Add DEBUG_CODE for special case when clear RP'. Also fix issues in the DEBUG_CODE and add more detailed debug message about it
3.Fix boundary bug in 'Avoid setting non-present range to RO/NX' code and adjust the code to make it easier to understand.
4.Use local variable instead of AllocatePoll in 'Sort mSmmCpuSmramRanges in FindSmramInfo' and 'Sort mProtectionMemRange when ReadyToLock'
5.Remove mXdSupported check in 'Refinement to smm runtime InitPaging() code'
Dun Tan (14):
OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry
MdeModulePkg: Remove RO and NX protection when unset guard page
UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute.
UefiCpuPkg: Add DEBUG_CODE for special case when clear RP
UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX
UefiCpuPkg/PiSmmCpuDxeSmm: Add 2 function to disable/enable CR0.WP
UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR0.WP before modify page table
UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h
UefiCpuPkg: Add GenSmmPageTable() to create smm page table
UefiCpuPkg: Use GenSmmPageTable() to create Smm S3 page table
UefiCpuPkg: Sort mSmmCpuSmramRanges in FindSmramInfo
UefiCpuPkg: Sort mProtectionMemRange when ReadyToLock
UefiCpuPkg: Refinement to smm runtime InitPaging() code
UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function
MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 2 +-
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 6 +++---
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 5 +++--
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 3 +--
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c | 2 +-
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 132 ------------------------------------------------------------------------------------------------------------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 40 ++++++++++++++++++++++++++++++++++++++--
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 +
UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 795 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 324 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 229 ++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 3 +--
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 20 ++++----------------
14 files changed, 668 insertions(+), 1015 deletions(-)
--
2.31.1.windows.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 10:33 ` Ni, Ray
2023-06-19 10:26 ` [edk2-devel] " Gerd Hoffmann
2023-06-08 2:27 ` [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page duntan
` (12 subsequent siblings)
13 siblings, 2 replies; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann,
Tom Lendacky, Ray Ni
Remove code that apply AddressEncMask to non-leaf entry when split
smm page table by MemEncryptSevLib. In FvbServicesSmm driver, it
calls MemEncryptSevClearMmioPageEncMask to clear AddressEncMask
bit in page table for a specific range. In AMD SEV feature, this
AddressEncMask bit in page table is used to indicate if the memory
is guest private memory or shared memory. But all memory used by
page table are treated as encrypted regardless of encryption bit.
So remove the EncMask bit for smm non-leaf page table entry
doesn't impact AMD SEV feature.
If page split happens in the AddressEncMask bit clear process,
there will be some new non-leaf entries with AddressEncMask
applied in smm page table. When ReadyToLock, code in PiSmmCpuDxe
module will use CpuPageTableLib to modify smm page table. So
remove code to apply AddressEncMask for new non-leaf entries
since CpuPageTableLib doesn't consume the EncMask PCD.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Ray Ni <ray.ni@intel.com>
---
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index cf2441b551..aba2e8c081 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -233,7 +233,7 @@ Split2MPageTo4K (
// Fill in 2M page entry.
//
*PageEntry2M = ((UINT64)(UINTN)PageTableEntry1 |
- IA32_PG_P | IA32_PG_RW | AddressEncMask);
+ IA32_PG_P | IA32_PG_RW);
}
/**
@@ -352,7 +352,7 @@ SetPageTablePoolReadOnly (
PhysicalAddress += LevelSize[Level - 1];
}
- PageTable[Index] = (UINT64)(UINTN)NewPageTable | AddressEncMask |
+ PageTable[Index] = (UINT64)(UINTN)NewPageTable |
IA32_PG_P | IA32_PG_RW;
PageTable = NewPageTable;
}
@@ -440,7 +440,7 @@ Split1GPageTo2M (
// Fill in 1G page entry.
//
*PageEntry1G = ((UINT64)(UINTN)PageDirectoryEntry |
- IA32_PG_P | IA32_PG_RW | AddressEncMask);
+ IA32_PG_P | IA32_PG_RW);
PhysicalAddress2M = PhysicalAddress;
for (IndexOfPageDirectoryEntries = 0;
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
2023-06-08 2:27 ` [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 10:08 ` Ni, Ray
2023-06-08 12:18 ` [edk2-devel] " Ard Biesheuvel
2023-06-08 2:27 ` [Patch V5 03/14] UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute duntan
` (11 subsequent siblings)
13 siblings, 2 replies; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Liming Gao, Ray Ni, Jian J Wang
Remove RO and NX protection when unset guard page.
When UnsetGuardPage(), remove all the memory attribute protection
for guarded page.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
---
MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
index 8f3bab6fee..7daeeccf13 100644
--- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
+++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
@@ -553,7 +553,7 @@ UnsetGuardPage (
mSmmMemoryAttribute,
BaseAddress,
EFI_PAGE_SIZE,
- EFI_MEMORY_RP
+ EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMORY_XP
);
ASSERT_EFI_ERROR (Status);
mOnGuarding = FALSE;
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 03/14] UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute.
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
2023-06-08 2:27 ` [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry duntan
2023-06-08 2:27 ` [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 10:24 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 04/14] UefiCpuPkg: Add DEBUG_CODE for special case when clear RP duntan
` (10 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
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 | 409 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 7 ++++++-
5 files changed, 122 insertions(+), 326 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..12723e5750 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,89 @@ ConvertMemoryPageAttributes (
return RETURN_UNSUPPORTED;
}
- // DEBUG ((DEBUG_ERROR, "ConvertMemoryPageAttributes(%x) - %016lx, %016lx, %02lx\n", IsSet, BaseAddress, Length, Attributes));
-
- if (IsSplitted != NULL) {
- *IsSplitted = FALSE;
- }
-
if (IsModified != NULL) {
*IsModified = FALSE;
}
- //
- // 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;
- }
+ PagingAttribute.Uint64 = 0;
+ PagingAttribute.Uint64 = mAddressEncMask | BaseAddress;
+ PagingAttrMask.Uint64 = 0;
- 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_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;
+ }
+ }
+ if ((Attributes & EFI_MEMORY_XP) != 0) {
+ if (mXdSupported) {
+ PagingAttribute.Bits.Nx = IsSet ? 1 : 0;
+ PagingAttrMask.Bits.Nx = 1;
+ }
+ }
+
+ 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;
- }
+ //
+ // When map range to present range, provide all attributes.
+ //
+ PagingAttribute.Bits.Present = 1;
+ PagingAttrMask.Uint64 = MAX_UINT64;
//
- // Just split current page
- // Convert success in next around
+ // By default memory is Ring 3 accessble.
//
+ PagingAttribute.Bits.UserSupervisor = 1;
}
}
+ 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: Only change EFI_MEMORY_XP/EFI_MEMORY_RO for non-present range in [0x%lx, 0x%lx] is not permitted\n", BaseAddress, BaseAddress + Length));
+ }
+
+ ASSERT_RETURN_ERROR (Status);
+ ASSERT (PageTableBufferSize == 0);
+
return RETURN_SUCCESS;
}
@@ -697,11 +500,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 +522,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 +549,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 +571,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 +622,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 +657,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 +682,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 +706,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 +1590,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..0bed857cae 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
@@ -354,6 +354,11 @@ SmmInitPageTable (
m5LevelPagingNeeded = Is5LevelPagingNeeded ();
mPhysicalAddressBits = CalculateMaximumSupportAddress ();
PatchInstructionX86 (gPatch5LevelPagingNeeded, m5LevelPagingNeeded, 1);
+ if (m5LevelPagingNeeded) {
+ mPagingMode = m1GPageTableSupport ? Paging5Level1GB : Paging5Level;
+ } 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 04/14] UefiCpuPkg: Add DEBUG_CODE for special case when clear RP
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
` (2 preceding siblings ...)
2023-06-08 2:27 ` [Patch V5 03/14] UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 10:33 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 05/14] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX duntan
` (9 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
In ConvertMemoryPageAttributes() function, when clear RP for a
specific range [BaseAddress, BaseAddress + Length], it means to
set the present bit to 1 and assign default value for other
attributes in page table. The default attributes for the input
specific range are NX disabled and ReadOnly. If there is existing
present range in [BaseAddress, BaseAddress + Length] and the
attributes are not NX disabled or not ReadOnly, then output the
DEBUG message to indicate that the NX and ReadOnly attributes of
the existing present range are modified in the function.
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/SmmCpuMemoryManagement.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 12723e5750..862b3e9720 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -349,6 +349,8 @@ ConvertMemoryPageAttributes (
IA32_MAP_ENTRY *Map;
UINTN Count;
UINTN Index;
+ UINT64 OverlappedRangeBase;
+ UINT64 OverlappedRangeLimit;
ASSERT (Attributes != 0);
ASSERT ((Attributes & ~EFI_MEMORY_ATTRIBUTE_MASK) == 0);
@@ -430,6 +432,52 @@ ConvertMemoryPageAttributes (
// By default memory is Ring 3 accessble.
//
PagingAttribute.Bits.UserSupervisor = 1;
+
+ DEBUG_CODE_BEGIN ();
+ 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 or ReadOnly.
+ //
+ Count = 0;
+ Map = NULL;
+ Status = PageTableParse (PageTableBase, mPagingMode, NULL, &Count);
+
+ while (Status == RETURN_BUFFER_TOO_SMALL) {
+ if (Map != NULL) {
+ FreePool (Map);
+ }
+
+ 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 (Map[Index].LinearAddress >= BaseAddress + Length) {
+ break;
+ }
+
+ if ((BaseAddress < Map[Index].LinearAddress + Map[Index].Length) && (BaseAddress + Length > Map[Index].LinearAddress)) {
+ OverlappedRangeBase = MAX (BaseAddress, Map[Index].LinearAddress);
+ OverlappedRangeLimit = MIN (BaseAddress + Length, Map[Index].LinearAddress + Map[Index].Length);
+
+ if (((Attributes & EFI_MEMORY_RO) == 0) && (Map[Index].Attribute.Bits.ReadWrite == 1)) {
+ DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes: [0x%lx, 0x%lx] is set from ReadWrite to ReadOnly\n", OverlappedRangeBase, OverlappedRangeLimit));
+ }
+
+ if (((Attributes & EFI_MEMORY_XP) == 0) && (mXdSupported) && (Map[Index].Attribute.Bits.Nx == 1)) {
+ DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes: [0x%lx, 0x%lx] is set from NX enabled to NX disabled\n", OverlappedRangeBase, OverlappedRangeLimit));
+ }
+ }
+ }
+
+ FreePool (Map);
+ }
+
+ DEBUG_CODE_END ();
}
}
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 05/14] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
` (3 preceding siblings ...)
2023-06-08 2:27 ` [Patch V5 04/14] UefiCpuPkg: Add DEBUG_CODE for special case when clear RP duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 10:32 ` [edk2-devel] " Ni, Ray
2023-06-08 2:27 ` [Patch V5 06/14] UefiCpuPkg/PiSmmCpuDxeSmm: Add 2 function to disable/enable CR0.WP duntan
` (8 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
In PiSmmCpuDxeSmm code, SetMemMapAttributes() marks memory ranges
in SmmMemoryAttributesTable to RO/NX. There may exist non-present
range in these memory ranges. Set other attributes for a non-present
range is not permitted in CpuPageTableMapLib. So add code to handle
this case. Only map the present ranges in SmmMemoryAttributesTable
to RO or NX.
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/SmmCpuMemoryManagement.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------
1 file changed, 107 insertions(+), 22 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 862b3e9720..3c79927c7b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -918,6 +918,70 @@ PatchGdtIdtMap (
);
}
+/**
+ This function set [Base, Limit] to the input MemoryAttribute.
+
+ @param Base Start address of range.
+ @param Limit Limit address of range.
+ @param Attribute The bit mask of attributes to modify for the memory region.
+ @param Map Pointer to the array of Cr3 IA32_MAP_ENTRY.
+ @param Count Count of IA32_MAP_ENTRY in Map.
+**/
+VOID
+SetMemMapWithNonPresentRange (
+ UINT64 Base,
+ UINT64 Limit,
+ UINT64 Attribute,
+ IA32_MAP_ENTRY *Map,
+ UINTN Count
+ )
+{
+ UINTN Index;
+ UINT64 NonPresentRangeStart;
+
+ NonPresentRangeStart = 0;
+ for (Index = 0; Index < Count; Index++) {
+ if ((Map[Index].LinearAddress > NonPresentRangeStart) &&
+ (Base < Map[Index].LinearAddress) && (Limit > NonPresentRangeStart))
+ {
+ //
+ // We should NOT set attributes for non-present ragne.
+ //
+ //
+ // There is a non-present ( [NonPresentStart, Map[Index].LinearAddress] ) range before current Map[Index]
+ // and it is overlapped with [Base, Limit].
+ //
+ if (Base < NonPresentRangeStart) {
+ SmmSetMemoryAttributes (
+ Base,
+ NonPresentRangeStart - Base,
+ Attribute
+ );
+ }
+
+ Base = Map[Index].LinearAddress;
+ }
+
+ NonPresentRangeStart = Map[Index].LinearAddress + Map[Index].Length;
+ if (NonPresentRangeStart >= Limit) {
+ break;
+ }
+ }
+
+ Limit = MIN (NonPresentRangeStart, Limit);
+
+ if (Base < Limit) {
+ //
+ // There is no non-present range in current [Base, Limit] anymore.
+ //
+ SmmSetMemoryAttributes (
+ Base,
+ Limit - Base,
+ Attribute
+ );
+ }
+}
+
/**
This function sets memory attribute according to MemoryAttributesTable.
**/
@@ -932,6 +996,11 @@ SetMemMapAttributes (
UINTN DescriptorSize;
UINTN Index;
EDKII_PI_SMM_MEMORY_ATTRIBUTES_TABLE *MemoryAttributesTable;
+ UINTN PageTable;
+ EFI_STATUS Status;
+ IA32_MAP_ENTRY *Map;
+ UINTN Count;
+ UINT64 MemoryAttribute;
SmmGetSystemConfigurationTable (&gEdkiiPiSmmMemoryAttributesTableGuid, (VOID **)&MemoryAttributesTable);
if (MemoryAttributesTable == NULL) {
@@ -958,36 +1027,52 @@ SetMemMapAttributes (
MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, DescriptorSize);
}
+ Count = 0;
+ Map = NULL;
+ PageTable = AsmReadCr3 ();
+ Status = PageTableParse (PageTable, mPagingMode, NULL, &Count);
+ while (Status == RETURN_BUFFER_TOO_SMALL) {
+ if (Map != NULL) {
+ FreePool (Map);
+ }
+
+ Map = AllocatePool (Count * sizeof (IA32_MAP_ENTRY));
+ ASSERT (Map != NULL);
+ Status = PageTableParse (PageTable, mPagingMode, Map, &Count);
+ }
+
+ ASSERT_RETURN_ERROR (Status);
+
MemoryMap = MemoryMapStart;
for (Index = 0; Index < MemoryMapEntryCount; Index++) {
DEBUG ((DEBUG_VERBOSE, "SetAttribute: Memory Entry - 0x%lx, 0x%x\n", MemoryMap->PhysicalStart, MemoryMap->NumberOfPages));
- switch (MemoryMap->Type) {
- case EfiRuntimeServicesCode:
- SmmSetMemoryAttributes (
- MemoryMap->PhysicalStart,
- EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
- EFI_MEMORY_RO
- );
- break;
- case EfiRuntimeServicesData:
- SmmSetMemoryAttributes (
- MemoryMap->PhysicalStart,
- EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
- EFI_MEMORY_XP
- );
- break;
- default:
- SmmSetMemoryAttributes (
- MemoryMap->PhysicalStart,
- EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
- EFI_MEMORY_XP
- );
- break;
+ if (MemoryMap->Type == EfiRuntimeServicesCode) {
+ MemoryAttribute = EFI_MEMORY_RO;
+ } else {
+ ASSERT ((MemoryMap->Type == EfiRuntimeServicesData) || (MemoryMap->Type == EfiConventionalMemory));
+ //
+ // Set other type memory as NX.
+ //
+ MemoryAttribute = EFI_MEMORY_XP;
}
+ //
+ // There may exist non-present range overlaps with the MemoryMap range.
+ // Do not change other attributes of non-present range while still remaining it as non-present
+ //
+ SetMemMapWithNonPresentRange (
+ MemoryMap->PhysicalStart,
+ MemoryMap->PhysicalStart + EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
+ MemoryAttribute,
+ Map,
+ Count
+ );
+
MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, DescriptorSize);
}
+ FreePool (Map);
+
PatchSmmSaveStateMap ();
PatchGdtIdtMap ();
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 06/14] UefiCpuPkg/PiSmmCpuDxeSmm: Add 2 function to disable/enable CR0.WP
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
` (4 preceding siblings ...)
2023-06-08 2:27 ` [Patch V5 05/14] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 2:27 ` [Patch V5 07/14] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR0.WP before modify page table duntan
` (7 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
Add two functions to disable/enable CR0.WP. These two unctions
will also be used in later commits. This commit doesn't change any
functionality.
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>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 24 ++++++++++++++++++++++++
UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------
2 files changed, 90 insertions(+), 49 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index ba341cadc6..e0c4ca76dc 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -1565,4 +1565,28 @@ SmmWaitForApArrival (
VOID
);
+/**
+ Disable Write Protect on pages marked as read-only if Cr0.Bits.WP is 1.
+
+ @param[out] WpEnabled If Cr0.WP is enabled.
+ @param[out] CetEnabled If CET is enabled.
+**/
+VOID
+DisableReadOnlyPageWriteProtect (
+ OUT BOOLEAN *WpEnabled,
+ OUT BOOLEAN *CetEnabled
+ );
+
+/**
+ Enable Write Protect on pages marked as read-only.
+
+ @param[out] WpEnabled If Cr0.WP should be enabled.
+ @param[out] CetEnabled If CET should be enabled.
+**/
+VOID
+EnableReadOnlyPageWriteProtect (
+ BOOLEAN WpEnabled,
+ BOOLEAN CetEnabled
+ );
+
#endif
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 3c79927c7b..d35058ed00 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -40,6 +40,64 @@ PAGE_TABLE_POOL *mPageTablePool = NULL;
//
BOOLEAN mIsReadOnlyPageTable = FALSE;
+/**
+ Disable Write Protect on pages marked as read-only if Cr0.Bits.WP is 1.
+
+ @param[out] WpEnabled If Cr0.WP is enabled.
+ @param[out] CetEnabled If CET is enabled.
+**/
+VOID
+DisableReadOnlyPageWriteProtect (
+ OUT BOOLEAN *WpEnabled,
+ OUT BOOLEAN *CetEnabled
+ )
+{
+ IA32_CR0 Cr0;
+
+ *CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
+ Cr0.UintN = AsmReadCr0 ();
+ *WpEnabled = (Cr0.Bits.WP != 0) ? TRUE : FALSE;
+ if (*WpEnabled) {
+ if (*CetEnabled) {
+ //
+ // CET must be disabled if WP is disabled. Disable CET before clearing CR0.WP.
+ //
+ DisableCet ();
+ }
+
+ Cr0.Bits.WP = 0;
+ AsmWriteCr0 (Cr0.UintN);
+ }
+}
+
+/**
+ Enable Write Protect on pages marked as read-only.
+
+ @param[out] WpEnabled If Cr0.WP should be enabled.
+ @param[out] CetEnabled If CET should be enabled.
+**/
+VOID
+EnableReadOnlyPageWriteProtect (
+ BOOLEAN WpEnabled,
+ BOOLEAN CetEnabled
+ )
+{
+ IA32_CR0 Cr0;
+
+ if (WpEnabled) {
+ Cr0.UintN = AsmReadCr0 ();
+ Cr0.Bits.WP = 1;
+ AsmWriteCr0 (Cr0.UintN);
+
+ if (CetEnabled) {
+ //
+ // re-enable CET.
+ //
+ EnableCet ();
+ }
+ }
+}
+
/**
Initialize a buffer pool for page table use only.
@@ -62,10 +120,9 @@ InitializePageTablePool (
IN UINTN PoolPages
)
{
- VOID *Buffer;
- BOOLEAN CetEnabled;
- BOOLEAN WpEnabled;
- IA32_CR0 Cr0;
+ VOID *Buffer;
+ BOOLEAN WpEnabled;
+ BOOLEAN CetEnabled;
//
// Always reserve at least PAGE_TABLE_POOL_UNIT_PAGES, including one page for
@@ -102,34 +159,9 @@ InitializePageTablePool (
// If page table memory has been marked as RO, mark the new pool pages as read-only.
//
if (mIsReadOnlyPageTable) {
- CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
- Cr0.UintN = AsmReadCr0 ();
- WpEnabled = (Cr0.Bits.WP != 0) ? TRUE : FALSE;
- if (WpEnabled) {
- if (CetEnabled) {
- //
- // CET must be disabled if WP is disabled. Disable CET before clearing CR0.WP.
- //
- DisableCet ();
- }
-
- Cr0.Bits.WP = 0;
- AsmWriteCr0 (Cr0.UintN);
- }
-
+ DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
SmmSetMemoryAttributes ((EFI_PHYSICAL_ADDRESS)(UINTN)Buffer, EFI_PAGES_TO_SIZE (PoolPages), EFI_MEMORY_RO);
- if (WpEnabled) {
- Cr0.UintN = AsmReadCr0 ();
- Cr0.Bits.WP = 1;
- AsmWriteCr0 (Cr0.UintN);
-
- if (CetEnabled) {
- //
- // re-enable CET.
- //
- EnableCet ();
- }
- }
+ EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
}
return TRUE;
@@ -1780,6 +1812,7 @@ SetPageTableAttributes (
VOID
)
{
+ BOOLEAN WpEnabled;
BOOLEAN CetEnabled;
if (!IfReadOnlyPageTableNeeded ()) {
@@ -1792,15 +1825,7 @@ SetPageTableAttributes (
// Disable write protection, because we need mark page table to be write protected.
// We need *write* page table memory, to mark itself to be *read only*.
//
- CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
- if (CetEnabled) {
- //
- // CET must be disabled if WP is disabled.
- //
- DisableCet ();
- }
-
- AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);
+ DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
// Set memory used by page table as Read Only.
DEBUG ((DEBUG_INFO, "Start...\n"));
@@ -1809,20 +1834,12 @@ SetPageTableAttributes (
//
// Enable write protection, after page table attribute updated.
//
- AsmWriteCr0 (AsmReadCr0 () | CR0_WP);
+ EnableReadOnlyPageWriteProtect (TRUE, CetEnabled);
mIsReadOnlyPageTable = TRUE;
//
// Flush TLB after mark all page table pool as read only.
//
FlushTlbForAll ();
-
- if (CetEnabled) {
- //
- // re-enable CET.
- //
- EnableCet ();
- }
-
return;
}
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 07/14] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR0.WP before modify page table
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
` (5 preceding siblings ...)
2023-06-08 2:27 ` [Patch V5 06/14] UefiCpuPkg/PiSmmCpuDxeSmm: Add 2 function to disable/enable CR0.WP duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 2:27 ` [Patch V5 08/14] UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h duntan
` (6 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
Clear CR0.WP before modify smm page table. Currently, there is
an assumption that smm pagetable is always RW before ReadyToLock.
However, when AMD SEV is enabled, FvbServicesSmm driver calls
MemEncryptSevClearMmioPageEncMask to clear AddressEncMask bit
in smm page table for this range:
[PcdOvmfFdBaseAddress,PcdOvmfFdBaseAddress+PcdOvmfFirmwareFdSize]
If page slpit happens in this process, new memory for smm page
table is allocated. Then the newly allocated page table memory
is marked as RO in smm page table in this FvbServicesSmm driver,
which may lead to PF if smm code doesn't clear CR0.WP before
modify smm page table when ReadyToLock.
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>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 11 +++++++++++
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 5 +++++
2 files changed, 16 insertions(+)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index d35058ed00..4ee99d06d7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -1033,6 +1033,8 @@ SetMemMapAttributes (
IA32_MAP_ENTRY *Map;
UINTN Count;
UINT64 MemoryAttribute;
+ BOOLEAN WpEnabled;
+ BOOLEAN CetEnabled;
SmmGetSystemConfigurationTable (&gEdkiiPiSmmMemoryAttributesTableGuid, (VOID **)&MemoryAttributesTable);
if (MemoryAttributesTable == NULL) {
@@ -1075,6 +1077,8 @@ SetMemMapAttributes (
ASSERT_RETURN_ERROR (Status);
+ DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
+
MemoryMap = MemoryMapStart;
for (Index = 0; Index < MemoryMapEntryCount; Index++) {
DEBUG ((DEBUG_VERBOSE, "SetAttribute: Memory Entry - 0x%lx, 0x%x\n", MemoryMap->PhysicalStart, MemoryMap->NumberOfPages));
@@ -1103,6 +1107,7 @@ SetMemMapAttributes (
MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, DescriptorSize);
}
+ EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
FreePool (Map);
PatchSmmSaveStateMap ();
@@ -1409,9 +1414,13 @@ SetUefiMemMapAttributes (
UINTN MemoryMapEntryCount;
UINTN Index;
EFI_MEMORY_DESCRIPTOR *Entry;
+ BOOLEAN WpEnabled;
+ BOOLEAN CetEnabled;
DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n"));
+ DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
+
if (mUefiMemoryMap != NULL) {
MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;
MemoryMap = mUefiMemoryMap;
@@ -1490,6 +1499,8 @@ SetUefiMemMapAttributes (
}
}
+ EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
+
//
// Do not free mUefiMemoryAttributesTable, it will be checked in IsSmmCommBufferForbiddenAddress().
//
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 1b0b6673e1..5625ba0cac 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -574,6 +574,8 @@ InitPaging (
BOOLEAN Nx;
IA32_CR4 Cr4;
BOOLEAN Enable5LevelPaging;
+ BOOLEAN WpEnabled;
+ BOOLEAN CetEnabled;
Cr4.UintN = AsmReadCr4 ();
Enable5LevelPaging = (BOOLEAN)(Cr4.Bits.LA57 == 1);
@@ -620,6 +622,7 @@ InitPaging (
NumberOfPdptEntries = 4;
}
+ DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
//
// Go through page table and change 2MB-page into 4KB-page.
//
@@ -800,6 +803,8 @@ InitPaging (
} // end for PML4
} // end for PML5
+ EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
+
//
// Flush TLB
//
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 08/14] UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
` (6 preceding siblings ...)
2023-06-08 2:27 ` [Patch V5 07/14] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR0.WP before modify page table duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 10:21 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 09/14] UefiCpuPkg: Add GenSmmPageTable() to create smm page table duntan
` (5 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h and remove
extern for mSmmShadowStackSize in c files to simplify 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/SmmFuncsArch.c | 3 +--
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 2 --
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 1 +
UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 2 --
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 3 +--
5 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
index 6c48a53f67..636dc8d92f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
@@ -1,7 +1,7 @@
/** @file
SMM CPU misc functions for Ia32 arch specific.
-Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -14,7 +14,6 @@ EFI_PHYSICAL_ADDRESS mGdtBuffer;
UINTN mGdtBufferSize;
extern BOOLEAN mCetSupported;
-extern UINTN mSmmShadowStackSize;
X86_ASSEMBLY_PATCH_LABEL mPatchCetPl0Ssp;
X86_ASSEMBLY_PATCH_LABEL mPatchCetInterruptSsp;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index baf827cf9d..1878252eac 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -29,8 +29,6 @@ MM_COMPLETION mSmmStartupThisApToken;
//
UINT32 *mPackageFirstThreadIndex = NULL;
-extern UINTN mSmmShadowStackSize;
-
/**
Performs an atomic compare exchange operation to get semaphore.
The compare exchange operation must be performed using
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index e0c4ca76dc..a7da9673a5 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -262,6 +262,7 @@ extern EFI_SMM_CPU_PROTOCOL mSmmCpu;
extern EFI_MM_MP_PROTOCOL mSmmMp;
extern BOOLEAN m5LevelPagingNeeded;
extern PAGING_MODE mPagingMode;
+extern UINTN mSmmShadowStackSize;
///
/// The mode of the CPU at the time an SMI occurs
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 0bed857cae..46d8ff5d4e 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -13,8 +13,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#define PAGE_TABLE_PAGES 8
#define ACC_MAX_BIT BIT3
-extern UINTN mSmmShadowStackSize;
-
LIST_ENTRY mPagePool = INITIALIZE_LIST_HEAD_VARIABLE (mPagePool);
BOOLEAN m1GPageTableSupport = FALSE;
BOOLEAN mCpuSmmRestrictedMemoryAccess;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
index 00a284c369..c4f21e2155 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
@@ -1,7 +1,7 @@
/** @file
SMM CPU misc functions for x64 arch specific.
-Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -12,7 +12,6 @@ EFI_PHYSICAL_ADDRESS mGdtBuffer;
UINTN mGdtBufferSize;
extern BOOLEAN mCetSupported;
-extern UINTN mSmmShadowStackSize;
X86_ASSEMBLY_PATCH_LABEL mPatchCetPl0Ssp;
X86_ASSEMBLY_PATCH_LABEL mPatchCetInterruptSsp;
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 09/14] UefiCpuPkg: Add GenSmmPageTable() to create smm page table
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
` (7 preceding siblings ...)
2023-06-08 2:27 ` [Patch V5 08/14] UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 10:16 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 10/14] UefiCpuPkg: Use GenSmmPageTable() to create Smm S3 " duntan
` (4 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
This commit is code refinement to current smm pagetable generation
code. Add a new GenSmmPageTable() API to create smm page table
based on the PageTableMap() API in CpuPageTableLib. Caller only
needs to specify the paging mode and the PhysicalAddressBits to map.
This function can be used to create both IA32 pae paging and X64
5level, 4level paging.
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 | 2 +-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 15 +++++++++++++++
UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 220 ++++++++++++++++++++++++++--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
4 files changed, 107 insertions(+), 195 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index 9c8107080a..b11264ce4a 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -63,7 +63,7 @@ SmmInitPageTable (
InitializeIDTSmmStackGuard ();
}
- return Gen4GPageTable (TRUE);
+ return GenSmmPageTable (PagingPae, mPhysicalAddressBits);
}
/**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index a7da9673a5..5399659bc0 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -553,6 +553,21 @@ Gen4GPageTable (
IN BOOLEAN Is32BitPageTable
);
+/**
+ Create page table based on input PagingMode and PhysicalAddressBits in smm.
+
+ @param[in] PagingMode The paging mode.
+ @param[in] PhysicalAddressBits The bits of physical address to map.
+
+ @retval PageTable Address
+
+**/
+UINTN
+GenSmmPageTable (
+ IN PAGING_MODE PagingMode,
+ IN UINT8 PhysicalAddressBits
+ );
+
/**
Initialize global data for MP synchronization.
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 4ee99d06d7..4e93d26eb4 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -1640,6 +1640,71 @@ EdkiiSmmClearMemoryAttributes (
return SmmClearMemoryAttributes (BaseAddress, Length, Attributes);
}
+/**
+ Create page table based on input PagingMode and PhysicalAddressBits in smm.
+
+ @param[in] PagingMode The paging mode.
+ @param[in] PhysicalAddressBits The bits of physical address to map.
+
+ @retval PageTable Address
+
+**/
+UINTN
+GenSmmPageTable (
+ IN PAGING_MODE PagingMode,
+ IN UINT8 PhysicalAddressBits
+ )
+{
+ UINTN PageTableBufferSize;
+ UINTN PageTable;
+ VOID *PageTableBuffer;
+ IA32_MAP_ATTRIBUTE MapAttribute;
+ IA32_MAP_ATTRIBUTE MapMask;
+ RETURN_STATUS Status;
+ UINTN GuardPage;
+ UINTN Index;
+ UINT64 Length;
+
+ Length = LShiftU64 (1, PhysicalAddressBits);
+ PageTable = 0;
+ PageTableBufferSize = 0;
+ MapMask.Uint64 = MAX_UINT64;
+ MapAttribute.Uint64 = mAddressEncMask;
+ MapAttribute.Bits.Present = 1;
+ MapAttribute.Bits.ReadWrite = 1;
+ MapAttribute.Bits.UserSupervisor = 1;
+ MapAttribute.Bits.Accessed = 1;
+ MapAttribute.Bits.Dirty = 1;
+
+ Status = PageTableMap (&PageTable, PagingMode, NULL, &PageTableBufferSize, 0, Length, &MapAttribute, &MapMask, NULL);
+ ASSERT (Status == RETURN_BUFFER_TOO_SMALL);
+ DEBUG ((DEBUG_INFO, "GenSMMPageTable: 0x%x bytes needed for initial SMM page table\n", PageTableBufferSize));
+ PageTableBuffer = AllocatePageTableMemory (EFI_SIZE_TO_PAGES (PageTableBufferSize));
+ ASSERT (PageTableBuffer != NULL);
+ Status = PageTableMap (&PageTable, PagingMode, PageTableBuffer, &PageTableBufferSize, 0, Length, &MapAttribute, &MapMask, NULL);
+ ASSERT (Status == RETURN_SUCCESS);
+ ASSERT (PageTableBufferSize == 0);
+
+ if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
+ //
+ // Mark the 4KB guard page between known good stack and smm stack as non-present
+ //
+ for (Index = 0; Index < gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus; Index++) {
+ GuardPage = mSmmStackArrayBase + EFI_PAGE_SIZE + Index * (mSmmStackSize + mSmmShadowStackSize);
+ Status = ConvertMemoryPageAttributes (PageTable, PagingMode, GuardPage, SIZE_4KB, EFI_MEMORY_RP, TRUE, NULL);
+ }
+ }
+
+ if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0) {
+ //
+ // Mark [0, 4k] as non-present
+ //
+ Status = ConvertMemoryPageAttributes (PageTable, PagingMode, 0, SIZE_4KB, EFI_MEMORY_RP, TRUE, NULL);
+ }
+
+ return (UINTN)PageTable;
+}
+
/**
This function retrieves the attributes of the memory region specified by
BaseAddress and Length. If different attributes are got from different part
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 46d8ff5d4e..ddd9be66b5 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -167,160 +167,6 @@ CalculateMaximumSupportAddress (
return PhysicalAddressBits;
}
-/**
- Set static page table.
-
- @param[in] PageTable Address of page table.
- @param[in] PhysicalAddressBits The maximum physical address bits supported.
-**/
-VOID
-SetStaticPageTable (
- IN UINTN PageTable,
- IN UINT8 PhysicalAddressBits
- )
-{
- UINT64 PageAddress;
- UINTN NumberOfPml5EntriesNeeded;
- UINTN NumberOfPml4EntriesNeeded;
- UINTN NumberOfPdpEntriesNeeded;
- UINTN IndexOfPml5Entries;
- UINTN IndexOfPml4Entries;
- UINTN IndexOfPdpEntries;
- UINTN IndexOfPageDirectoryEntries;
- UINT64 *PageMapLevel5Entry;
- UINT64 *PageMapLevel4Entry;
- UINT64 *PageMap;
- UINT64 *PageDirectoryPointerEntry;
- UINT64 *PageDirectory1GEntry;
- UINT64 *PageDirectoryEntry;
-
- //
- // IA-32e paging translates 48-bit linear addresses to 52-bit physical addresses
- // when 5-Level Paging is disabled.
- //
- ASSERT (PhysicalAddressBits <= 52);
- if (!m5LevelPagingNeeded && (PhysicalAddressBits > 48)) {
- PhysicalAddressBits = 48;
- }
-
- NumberOfPml5EntriesNeeded = 1;
- if (PhysicalAddressBits > 48) {
- NumberOfPml5EntriesNeeded = (UINTN)LShiftU64 (1, PhysicalAddressBits - 48);
- PhysicalAddressBits = 48;
- }
-
- NumberOfPml4EntriesNeeded = 1;
- if (PhysicalAddressBits > 39) {
- NumberOfPml4EntriesNeeded = (UINTN)LShiftU64 (1, PhysicalAddressBits - 39);
- PhysicalAddressBits = 39;
- }
-
- NumberOfPdpEntriesNeeded = 1;
- ASSERT (PhysicalAddressBits > 30);
- NumberOfPdpEntriesNeeded = (UINTN)LShiftU64 (1, PhysicalAddressBits - 30);
-
- //
- // By architecture only one PageMapLevel4 exists - so lets allocate storage for it.
- //
- PageMap = (VOID *)PageTable;
-
- PageMapLevel4Entry = PageMap;
- PageMapLevel5Entry = NULL;
- if (m5LevelPagingNeeded) {
- //
- // By architecture only one PageMapLevel5 exists - so lets allocate storage for it.
- //
- PageMapLevel5Entry = PageMap;
- }
-
- PageAddress = 0;
-
- for ( IndexOfPml5Entries = 0
- ; IndexOfPml5Entries < NumberOfPml5EntriesNeeded
- ; IndexOfPml5Entries++, PageMapLevel5Entry++)
- {
- //
- // Each PML5 entry points to a page of PML4 entires.
- // So lets allocate space for them and fill them in in the IndexOfPml4Entries loop.
- // When 5-Level Paging is disabled, below allocation happens only once.
- //
- if (m5LevelPagingNeeded) {
- PageMapLevel4Entry = (UINT64 *)((*PageMapLevel5Entry) & ~mAddressEncMask & gPhyMask);
- if (PageMapLevel4Entry == NULL) {
- PageMapLevel4Entry = AllocatePageTableMemory (1);
- ASSERT (PageMapLevel4Entry != NULL);
- ZeroMem (PageMapLevel4Entry, EFI_PAGES_TO_SIZE (1));
-
- *PageMapLevel5Entry = (UINT64)(UINTN)PageMapLevel4Entry | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
- }
- }
-
- for (IndexOfPml4Entries = 0; IndexOfPml4Entries < (NumberOfPml5EntriesNeeded == 1 ? NumberOfPml4EntriesNeeded : 512); IndexOfPml4Entries++, PageMapLevel4Entry++) {
- //
- // Each PML4 entry points to a page of Page Directory Pointer entries.
- //
- PageDirectoryPointerEntry = (UINT64 *)((*PageMapLevel4Entry) & ~mAddressEncMask & gPhyMask);
- if (PageDirectoryPointerEntry == NULL) {
- PageDirectoryPointerEntry = AllocatePageTableMemory (1);
- ASSERT (PageDirectoryPointerEntry != NULL);
- ZeroMem (PageDirectoryPointerEntry, EFI_PAGES_TO_SIZE (1));
-
- *PageMapLevel4Entry = (UINT64)(UINTN)PageDirectoryPointerEntry | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
- }
-
- if (m1GPageTableSupport) {
- PageDirectory1GEntry = PageDirectoryPointerEntry;
- for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectory1GEntry++, PageAddress += SIZE_1GB) {
- if ((IndexOfPml4Entries == 0) && (IndexOfPageDirectoryEntries < 4)) {
- //
- // Skip the < 4G entries
- //
- continue;
- }
-
- //
- // Fill in the Page Directory entries
- //
- *PageDirectory1GEntry = PageAddress | mAddressEncMask | IA32_PG_PS | PAGE_ATTRIBUTE_BITS;
- }
- } else {
- PageAddress = BASE_4GB;
- for (IndexOfPdpEntries = 0; IndexOfPdpEntries < (NumberOfPml4EntriesNeeded == 1 ? NumberOfPdpEntriesNeeded : 512); IndexOfPdpEntries++, PageDirectoryPointerEntry++) {
- if ((IndexOfPml4Entries == 0) && (IndexOfPdpEntries < 4)) {
- //
- // Skip the < 4G entries
- //
- continue;
- }
-
- //
- // Each Directory Pointer entries points to a page of Page Directory entires.
- // So allocate space for them and fill them in in the IndexOfPageDirectoryEntries loop.
- //
- PageDirectoryEntry = (UINT64 *)((*PageDirectoryPointerEntry) & ~mAddressEncMask & gPhyMask);
- if (PageDirectoryEntry == NULL) {
- PageDirectoryEntry = AllocatePageTableMemory (1);
- ASSERT (PageDirectoryEntry != NULL);
- ZeroMem (PageDirectoryEntry, EFI_PAGES_TO_SIZE (1));
-
- //
- // Fill in a Page Directory Pointer Entries
- //
- *PageDirectoryPointerEntry = (UINT64)(UINTN)PageDirectoryEntry | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
- }
-
- for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PageAddress += SIZE_2MB) {
- //
- // Fill in the Page Directory entries
- //
- *PageDirectoryEntry = PageAddress | mAddressEncMask | IA32_PG_PS | PAGE_ATTRIBUTE_BITS;
- }
- }
- }
- }
- }
-}
-
/**
Create PageTable for SMM use.
@@ -332,15 +178,16 @@ SmmInitPageTable (
VOID
)
{
- EFI_PHYSICAL_ADDRESS Pages;
- UINT64 *PTEntry;
+ UINTN PageTable;
LIST_ENTRY *FreePage;
UINTN Index;
UINTN PageFaultHandlerHookAddress;
IA32_IDT_GATE_DESCRIPTOR *IdtEntry;
EFI_STATUS Status;
+ UINT64 *PdptEntry;
UINT64 *Pml4Entry;
UINT64 *Pml5Entry;
+ UINT8 PhysicalAddressBits;
//
// Initialize spin lock
@@ -357,59 +204,44 @@ SmmInitPageTable (
} 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));
DEBUG ((DEBUG_INFO, "PhysicalAddressBits - %d\n", mPhysicalAddressBits));
- //
- // Generate PAE page table for the first 4GB memory space
- //
- Pages = Gen4GPageTable (FALSE);
//
- // Set IA32_PG_PMNT bit to mask this entry
+ // Generate initial SMM page table.
+ // Only map [0, 4G] when PcdCpuSmmRestrictedMemoryAccess is FALSE.
//
- PTEntry = (UINT64 *)(UINTN)Pages;
- for (Index = 0; Index < 4; Index++) {
- PTEntry[Index] |= IA32_PG_PMNT;
- }
-
- //
- // Fill Page-Table-Level4 (PML4) entry
- //
- Pml4Entry = (UINT64 *)AllocatePageTableMemory (1);
- ASSERT (Pml4Entry != NULL);
- *Pml4Entry = Pages | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
- ZeroMem (Pml4Entry + 1, EFI_PAGE_SIZE - sizeof (*Pml4Entry));
-
- //
- // Set sub-entries number
- //
- SetSubEntriesNum (Pml4Entry, 3);
- PTEntry = Pml4Entry;
+ PhysicalAddressBits = mCpuSmmRestrictedMemoryAccess ? mPhysicalAddressBits : 32;
+ PageTable = GenSmmPageTable (mPagingMode, PhysicalAddressBits);
if (m5LevelPagingNeeded) {
+ Pml5Entry = (UINT64 *)PageTable;
//
- // Fill PML5 entry
- //
- Pml5Entry = (UINT64 *)AllocatePageTableMemory (1);
- ASSERT (Pml5Entry != NULL);
- *Pml5Entry = (UINTN)Pml4Entry | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
- ZeroMem (Pml5Entry + 1, EFI_PAGE_SIZE - sizeof (*Pml5Entry));
- //
- // Set sub-entries number
+ // Set Pml5Entry sub-entries number for smm PF handler usage.
//
SetSubEntriesNum (Pml5Entry, 1);
- PTEntry = Pml5Entry;
+ Pml4Entry = (UINT64 *)((*Pml5Entry) & ~mAddressEncMask & gPhyMask);
+ } else {
+ Pml4Entry = (UINT64 *)PageTable;
+ }
+
+ //
+ // Set IA32_PG_PMNT bit to mask first 4 PdptEntry.
+ //
+ PdptEntry = (UINT64 *)((*Pml4Entry) & ~mAddressEncMask & gPhyMask);
+ for (Index = 0; Index < 4; Index++) {
+ PdptEntry[Index] |= IA32_PG_PMNT;
}
- if (mCpuSmmRestrictedMemoryAccess) {
+ if (!mCpuSmmRestrictedMemoryAccess) {
//
- // When access to non-SMRAM memory is restricted, create page table
- // that covers all memory space.
+ // Set Pml4Entry sub-entries number for smm PF handler usage.
//
- SetStaticPageTable ((UINTN)PTEntry, mPhysicalAddressBits);
- } else {
+ SetSubEntriesNum (Pml4Entry, 3);
+
//
// Add pages to page pool
//
@@ -466,7 +298,7 @@ SmmInitPageTable (
//
// Return the address of PML4/PML5 (to set CR3)
//
- return (UINT32)(UINTN)PTEntry;
+ return (UINT32)PageTable;
}
/**
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 10/14] UefiCpuPkg: Use GenSmmPageTable() to create Smm S3 page table
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
` (8 preceding siblings ...)
2023-06-08 2:27 ` [Patch V5 09/14] UefiCpuPkg: Add GenSmmPageTable() to create smm page table duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 2:27 ` [Patch V5 11/14] UefiCpuPkg: Sort mSmmCpuSmramRanges in FindSmramInfo duntan
` (3 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
Use GenSmmPageTable() to create both IA32 and X64 Smm S3
page table.
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>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c | 2 +-
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 130 ----------------------------------------------------------------------------------------------------------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 20 ++++----------------
3 files changed, 5 insertions(+), 147 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c
index bba4a6f058..650090e534 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c
@@ -18,7 +18,7 @@ InitSmmS3Cr3 (
VOID
)
{
- mSmmS3ResumeState->SmmS3Cr3 = Gen4GPageTable (TRUE);
+ mSmmS3ResumeState->SmmS3Cr3 = GenSmmPageTable (PagingPae, mPhysicalAddressBits);
return;
}
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 1878252eac..f8b81fc96e 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -999,136 +999,6 @@ APHandler (
ReleaseSemaphore (mSmmMpSyncData->CpuData[BspIndex].Run);
}
-/**
- Create 4G PageTable in SMRAM.
-
- @param[in] Is32BitPageTable Whether the page table is 32-bit PAE
- @return PageTable Address
-
-**/
-UINT32
-Gen4GPageTable (
- IN BOOLEAN Is32BitPageTable
- )
-{
- VOID *PageTable;
- UINTN Index;
- UINT64 *Pte;
- UINTN PagesNeeded;
- UINTN Low2MBoundary;
- UINTN High2MBoundary;
- UINTN Pages;
- UINTN GuardPage;
- UINT64 *Pdpte;
- UINTN PageIndex;
- UINTN PageAddress;
-
- Low2MBoundary = 0;
- High2MBoundary = 0;
- PagesNeeded = 0;
- if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
- //
- // Add one more page for known good stack, then find the lower 2MB aligned address.
- //
- Low2MBoundary = (mSmmStackArrayBase + EFI_PAGE_SIZE) & ~(SIZE_2MB-1);
- //
- // Add two more pages for known good stack and stack guard page,
- // then find the lower 2MB aligned address.
- //
- High2MBoundary = (mSmmStackArrayEnd - mSmmStackSize - mSmmShadowStackSize + EFI_PAGE_SIZE * 2) & ~(SIZE_2MB-1);
- PagesNeeded = ((High2MBoundary - Low2MBoundary) / SIZE_2MB) + 1;
- }
-
- //
- // Allocate the page table
- //
- PageTable = AllocatePageTableMemory (5 + PagesNeeded);
- ASSERT (PageTable != NULL);
-
- PageTable = (VOID *)((UINTN)PageTable);
- Pte = (UINT64 *)PageTable;
-
- //
- // Zero out all page table entries first
- //
- ZeroMem (Pte, EFI_PAGES_TO_SIZE (1));
-
- //
- // Set Page Directory Pointers
- //
- for (Index = 0; Index < 4; Index++) {
- Pte[Index] = ((UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1)) | mAddressEncMask |
- (Is32BitPageTable ? IA32_PAE_PDPTE_ATTRIBUTE_BITS : PAGE_ATTRIBUTE_BITS);
- }
-
- Pte += EFI_PAGE_SIZE / sizeof (*Pte);
-
- //
- // Fill in Page Directory Entries
- //
- for (Index = 0; Index < EFI_PAGE_SIZE * 4 / sizeof (*Pte); Index++) {
- Pte[Index] = (Index << 21) | mAddressEncMask | IA32_PG_PS | PAGE_ATTRIBUTE_BITS;
- }
-
- Pdpte = (UINT64 *)PageTable;
- if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
- Pages = (UINTN)PageTable + EFI_PAGES_TO_SIZE (5);
- GuardPage = mSmmStackArrayBase + EFI_PAGE_SIZE;
- for (PageIndex = Low2MBoundary; PageIndex <= High2MBoundary; PageIndex += SIZE_2MB) {
- Pte = (UINT64 *)(UINTN)(Pdpte[BitFieldRead32 ((UINT32)PageIndex, 30, 31)] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 1));
- Pte[BitFieldRead32 ((UINT32)PageIndex, 21, 29)] = (UINT64)Pages | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
- //
- // Fill in Page Table Entries
- //
- Pte = (UINT64 *)Pages;
- PageAddress = PageIndex;
- for (Index = 0; Index < EFI_PAGE_SIZE / sizeof (*Pte); Index++) {
- if (PageAddress == GuardPage) {
- //
- // Mark the guard page as non-present
- //
- Pte[Index] = PageAddress | mAddressEncMask;
- GuardPage += (mSmmStackSize + mSmmShadowStackSize);
- if (GuardPage > mSmmStackArrayEnd) {
- GuardPage = 0;
- }
- } else {
- Pte[Index] = PageAddress | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
- }
-
- PageAddress += EFI_PAGE_SIZE;
- }
-
- Pages += EFI_PAGE_SIZE;
- }
- }
-
- if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0) {
- Pte = (UINT64 *)(UINTN)(Pdpte[0] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 1));
- if ((Pte[0] & IA32_PG_PS) == 0) {
- // 4K-page entries are already mapped. Just hide the first one anyway.
- Pte = (UINT64 *)(UINTN)(Pte[0] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 1));
- Pte[0] &= ~(UINT64)IA32_PG_P; // Hide page 0
- } else {
- // Create 4K-page entries
- Pages = (UINTN)AllocatePageTableMemory (1);
- ASSERT (Pages != 0);
-
- Pte[0] = (UINT64)(Pages | mAddressEncMask | PAGE_ATTRIBUTE_BITS);
-
- Pte = (UINT64 *)Pages;
- PageAddress = 0;
- Pte[0] = PageAddress | mAddressEncMask; // Hide page 0 but present left
- for (Index = 1; Index < EFI_PAGE_SIZE / sizeof (*Pte); Index++) {
- PageAddress += EFI_PAGE_SIZE;
- Pte[Index] = PageAddress | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
- }
- }
- }
-
- return (UINT32)(UINTN)PageTable;
-}
-
/**
Checks whether the input token is the current used token.
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c
index cb7a691745..01432d466c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c
@@ -35,26 +35,14 @@ InitSmmS3Cr3 (
VOID
)
{
- EFI_PHYSICAL_ADDRESS Pages;
- UINT64 *PTEntry;
-
- //
- // Generate PAE page table for the first 4GB memory space
- //
- Pages = Gen4GPageTable (FALSE);
-
//
- // Fill Page-Table-Level4 (PML4) entry
+ // Generate level4 page table for the first 4GB memory space
+ // Return the address of PML4 (to set CR3)
//
- PTEntry = (UINT64 *)AllocatePageTableMemory (1);
- ASSERT (PTEntry != NULL);
- *PTEntry = Pages | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
- ZeroMem (PTEntry + 1, EFI_PAGE_SIZE - sizeof (*PTEntry));
-
//
- // Return the address of PML4 (to set CR3)
+ // The SmmS3Cr3 is only used by S3Resume PEIM to switch CPU from 32bit to 64bit
//
- mSmmS3ResumeState->SmmS3Cr3 = (UINT32)(UINTN)PTEntry;
+ mSmmS3ResumeState->SmmS3Cr3 = (UINT32)GenSmmPageTable (Paging4Level, 32);
return;
}
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 11/14] UefiCpuPkg: Sort mSmmCpuSmramRanges in FindSmramInfo
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
` (9 preceding siblings ...)
2023-06-08 2:27 ` [Patch V5 10/14] UefiCpuPkg: Use GenSmmPageTable() to create Smm S3 " duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 10:16 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 12/14] UefiCpuPkg: Sort mProtectionMemRange when ReadyToLock duntan
` (2 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
Sort mSmmCpuSmramRanges after get the SMRAM info in
FindSmramInfo() function.
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/PiSmmCpuDxeSmm.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 2144d6ade8..2568ffd677 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -1197,6 +1197,32 @@ PiCpuSmmEntry (
return EFI_SUCCESS;
}
+/**
+ Function to compare 2 EFI_SMRAM_DESCRIPTOR based on CpuStart.
+
+ @param[in] Buffer1 pointer to Device Path poiner to compare
+ @param[in] Buffer2 pointer to second DevicePath pointer to compare
+
+ @retval 0 Buffer1 equal to Buffer2
+ @retval <0 Buffer1 is less than Buffer2
+ @retval >0 Buffer1 is greater than Buffer2
+**/
+INTN
+EFIAPI
+CpuSmramRangeCompare (
+ IN CONST VOID *Buffer1,
+ IN CONST VOID *Buffer2
+ )
+{
+ if (((EFI_SMRAM_DESCRIPTOR *)Buffer1)->CpuStart > ((EFI_SMRAM_DESCRIPTOR *)Buffer2)->CpuStart) {
+ return 1;
+ } else if (((EFI_SMRAM_DESCRIPTOR *)Buffer1)->CpuStart < ((EFI_SMRAM_DESCRIPTOR *)Buffer2)->CpuStart) {
+ return -1;
+ }
+
+ return 0;
+}
+
/**
Find out SMRAM information including SMRR base and SMRR size.
@@ -1218,6 +1244,7 @@ FindSmramInfo (
UINTN Index;
UINT64 MaxSize;
BOOLEAN Found;
+ EFI_SMRAM_DESCRIPTOR SmramDescriptor;
//
// Get SMM Access Protocol
@@ -1240,6 +1267,11 @@ FindSmramInfo (
mSmmCpuSmramRangeCount = Size / sizeof (EFI_SMRAM_DESCRIPTOR);
+ //
+ // Sort the mSmmCpuSmramRanges
+ //
+ QuickSort (mSmmCpuSmramRanges, mSmmCpuSmramRangeCount, sizeof (EFI_SMRAM_DESCRIPTOR), (BASE_SORT_COMPARE)CpuSmramRangeCompare, &SmramDescriptor);
+
//
// Find the largest SMRAM range between 1MB and 4GB that is at least 256K - 4K in size
//
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 12/14] UefiCpuPkg: Sort mProtectionMemRange when ReadyToLock
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
` (10 preceding siblings ...)
2023-06-08 2:27 ` [Patch V5 11/14] UefiCpuPkg: Sort mSmmCpuSmramRanges in FindSmramInfo duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 10:17 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 13/14] UefiCpuPkg: Refinement to smm runtime InitPaging() code duntan
2023-06-08 2:27 ` [Patch V5 14/14] UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function duntan
13 siblings, 1 reply; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
Sort mProtectionMemRange in InitProtectedMemRange() when
ReadyToLock.
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/SmmProfile.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 5625ba0cac..2a1f132b29 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -375,6 +375,32 @@ IsAddressSplit (
return FALSE;
}
+/**
+ Function to compare 2 MEMORY_PROTECTION_RANGE based on range base.
+
+ @param[in] Buffer1 pointer to Device Path poiner to compare
+ @param[in] Buffer2 pointer to second DevicePath pointer to compare
+
+ @retval 0 Buffer1 equal to Buffer2
+ @retval <0 Buffer1 is less than Buffer2
+ @retval >0 Buffer1 is greater than Buffer2
+**/
+INTN
+EFIAPI
+ProtectionRangeCompare (
+ IN CONST VOID *Buffer1,
+ IN CONST VOID *Buffer2
+ )
+{
+ if (((MEMORY_PROTECTION_RANGE *)Buffer1)->Range.Base > ((MEMORY_PROTECTION_RANGE *)Buffer2)->Range.Base) {
+ return 1;
+ } else if (((MEMORY_PROTECTION_RANGE *)Buffer1)->Range.Base < ((MEMORY_PROTECTION_RANGE *)Buffer2)->Range.Base) {
+ return -1;
+ }
+
+ return 0;
+}
+
/**
Initialize the protected memory ranges and the 4KB-page mapped memory ranges.
@@ -397,6 +423,7 @@ InitProtectedMemRange (
EFI_PHYSICAL_ADDRESS Base2MBAlignedAddress;
UINT64 High4KBPageSize;
UINT64 Low4KBPageSize;
+ MEMORY_PROTECTION_RANGE MemProtectionRange;
NumberOfDescriptors = 0;
NumberOfAddedDescriptors = mSmmCpuSmramRangeCount;
@@ -533,6 +560,11 @@ InitProtectedMemRange (
mSplitMemRangeCount = NumberOfSpliteRange;
+ //
+ // Sort the mProtectionMemRange
+ //
+ QuickSort (mProtectionMemRange, mProtectionMemRangeCount, sizeof (MEMORY_PROTECTION_RANGE), (BASE_SORT_COMPARE)ProtectionRangeCompare, &MemProtectionRange);
+
DEBUG ((DEBUG_INFO, "SMM Profile Memory Ranges:\n"));
for (Index = 0; Index < mProtectionMemRangeCount; Index++) {
DEBUG ((DEBUG_INFO, "mProtectionMemRange[%d].Base = %lx\n", Index, mProtectionMemRange[Index].Range.Base));
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 13/14] UefiCpuPkg: Refinement to smm runtime InitPaging() code
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
` (11 preceding siblings ...)
2023-06-08 2:27 ` [Patch V5 12/14] UefiCpuPkg: Sort mProtectionMemRange when ReadyToLock duntan
@ 2023-06-08 2:27 ` duntan
2023-06-08 10:18 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 14/14] UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function duntan
13 siblings, 1 reply; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
This commit is code refinement to current smm runtime InitPaging()
page table update code. In InitPaging(), if PcdCpuSmmProfileEnable
is TRUE, use ConvertMemoryPageAttributes() API to map the range in
mProtectionMemRange to the attrbute recorded in the attribute field
of mProtectionMemRange, map the range outside mProtectionMemRange
as non-present. If PcdCpuSmmProfileEnable is FALSE, only need to
set the ranges not in mSmmCpuSmramRanges as NX.
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/PiSmmCpuDxeSmm.h | 37 +++++++++++++++++++++++++++++++++++++
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 293 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
2 files changed, 101 insertions(+), 229 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 5399659bc0..12ad86028e 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -725,6 +725,43 @@ SmmBlockingStartupThisAp (
IN OUT VOID *ProcArguments OPTIONAL
);
+/**
+ 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.
+
+ Caller should make sure BaseAddress and Length is at page boundary.
+
+ @param[in] PageTableBase The page table base.
+ @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] IsModified TRUE means page table modified. FALSE means page table not modified.
+
+ @retval RETURN_SUCCESS The attributes were modified for the memory region.
+ @retval RETURN_ACCESS_DENIED The attributes for the memory resource range specified by
+ BaseAddress and Length cannot be modified.
+ @retval RETURN_INVALID_PARAMETER Length is zero.
+ Attributes specified an illegal combination of attributes that
+ cannot be set together.
+ @retval RETURN_OUT_OF_RESOURCES There are not enough system resources to modify the attributes of
+ the memory resource range.
+ @retval RETURN_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
+ range specified by BaseAddress and Length.
+**/
+RETURN_STATUS
+ConvertMemoryPageAttributes (
+ IN UINTN PageTableBase,
+ IN PAGING_MODE PagingMode,
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes,
+ IN BOOLEAN IsSet,
+ OUT BOOLEAN *IsModified OPTIONAL
+ );
+
/**
This function sets the attributes for the memory region specified by BaseAddress and
Length from their current attributes to the attributes specified by Attributes.
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 2a1f132b29..ab6b79ddf4 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -586,254 +586,89 @@ InitPaging (
VOID
)
{
- UINT64 Pml5Entry;
- UINT64 Pml4Entry;
- UINT64 *Pml5;
- UINT64 *Pml4;
- UINT64 *Pdpt;
- UINT64 *Pd;
- UINT64 *Pt;
- UINTN Address;
- UINTN Pml5Index;
- UINTN Pml4Index;
- UINTN PdptIndex;
- UINTN PdIndex;
- UINTN PtIndex;
- UINTN NumberOfPdptEntries;
- UINTN NumberOfPml4Entries;
- UINTN NumberOfPml5Entries;
- UINTN SizeOfMemorySpace;
- BOOLEAN Nx;
- IA32_CR4 Cr4;
- BOOLEAN Enable5LevelPaging;
- BOOLEAN WpEnabled;
- BOOLEAN CetEnabled;
-
- Cr4.UintN = AsmReadCr4 ();
- Enable5LevelPaging = (BOOLEAN)(Cr4.Bits.LA57 == 1);
-
- if (sizeof (UINTN) == sizeof (UINT64)) {
- if (!Enable5LevelPaging) {
- Pml5Entry = (UINTN)mSmmProfileCr3 | IA32_PG_P;
- Pml5 = &Pml5Entry;
- } else {
- Pml5 = (UINT64 *)(UINTN)mSmmProfileCr3;
- }
-
- SizeOfMemorySpace = HighBitSet64 (gPhyMask) + 1;
- ASSERT (SizeOfMemorySpace <= 52);
-
- //
- // Calculate the table entries of PML5E, PML4E and PDPTE.
- //
- NumberOfPml5Entries = 1;
- if (SizeOfMemorySpace > 48) {
- if (Enable5LevelPaging) {
- NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48);
- }
-
- SizeOfMemorySpace = 48;
- }
-
- NumberOfPml4Entries = 1;
- if (SizeOfMemorySpace > 39) {
- NumberOfPml4Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 39);
- SizeOfMemorySpace = 39;
- }
-
- NumberOfPdptEntries = 1;
- ASSERT (SizeOfMemorySpace > 30);
- NumberOfPdptEntries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 30);
+ RETURN_STATUS Status;
+ UINTN Index;
+ UINTN PageTable;
+ UINT64 Base;
+ UINT64 Length;
+ UINT64 Limit;
+ UINT64 PreviousAddress;
+ UINT64 MemoryAttrMask;
+ BOOLEAN WpEnabled;
+ BOOLEAN CetEnabled;
+
+ PageTable = AsmReadCr3 ();
+ if (sizeof (UINTN) == sizeof (UINT32)) {
+ Limit = BASE_4GB;
} else {
- Pml4Entry = (UINTN)mSmmProfileCr3 | IA32_PG_P;
- Pml4 = &Pml4Entry;
- Pml5Entry = (UINTN)Pml4 | IA32_PG_P;
- Pml5 = &Pml5Entry;
- NumberOfPml5Entries = 1;
- NumberOfPml4Entries = 1;
- NumberOfPdptEntries = 4;
+ Limit = (IsRestrictedMemoryAccess ()) ? LShiftU64 (1, mPhysicalAddressBits) : BASE_4GB;
}
DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
//
- // Go through page table and change 2MB-page into 4KB-page.
+ // [0, 4k] may be non-present.
//
- for (Pml5Index = 0; Pml5Index < NumberOfPml5Entries; Pml5Index++) {
- if ((Pml5[Pml5Index] & IA32_PG_P) == 0) {
- //
- // If PML5 entry does not exist, skip it
- //
- continue;
- }
+ PreviousAddress = ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0) ? BASE_4KB : 0;
- Pml4 = (UINT64 *)(UINTN)(Pml5[Pml5Index] & PHYSICAL_ADDRESS_MASK);
- for (Pml4Index = 0; Pml4Index < NumberOfPml4Entries; Pml4Index++) {
- if ((Pml4[Pml4Index] & IA32_PG_P) == 0) {
- //
- // If PML4 entry does not exist, skip it
- //
- continue;
+ DEBUG ((DEBUG_INFO, "Patch page table start ...\n"));
+ if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
+ for (Index = 0; Index < mProtectionMemRangeCount; Index++) {
+ MemoryAttrMask = 0;
+ if (mProtectionMemRange[Index].Nx == TRUE) {
+ MemoryAttrMask |= EFI_MEMORY_XP;
}
- Pdpt = (UINT64 *)(UINTN)(Pml4[Pml4Index] & ~mAddressEncMask & PHYSICAL_ADDRESS_MASK);
- for (PdptIndex = 0; PdptIndex < NumberOfPdptEntries; PdptIndex++, Pdpt++) {
- if ((*Pdpt & IA32_PG_P) == 0) {
- //
- // If PDPT entry does not exist, skip it
- //
- continue;
- }
-
- if ((*Pdpt & IA32_PG_PS) != 0) {
- //
- // This is 1G entry, skip it
- //
- continue;
- }
-
- Pd = (UINT64 *)(UINTN)(*Pdpt & ~mAddressEncMask & PHYSICAL_ADDRESS_MASK);
- if (Pd == 0) {
- continue;
- }
-
- for (PdIndex = 0; PdIndex < SIZE_4KB / sizeof (*Pd); PdIndex++, Pd++) {
- if ((*Pd & IA32_PG_P) == 0) {
- //
- // If PD entry does not exist, skip it
- //
- continue;
- }
-
- Address = (UINTN)LShiftU64 (
- LShiftU64 (
- LShiftU64 ((Pml5Index << 9) + Pml4Index, 9) + PdptIndex,
- 9
- ) + PdIndex,
- 21
- );
-
- //
- // If it is 2M page, check IsAddressSplit()
- //
- if (((*Pd & IA32_PG_PS) != 0) && IsAddressSplit (Address)) {
- //
- // Based on current page table, create 4KB page table for split area.
- //
- ASSERT (Address == (*Pd & PHYSICAL_ADDRESS_MASK));
-
- Pt = AllocatePageTableMemory (1);
- ASSERT (Pt != NULL);
+ if (mProtectionMemRange[Index].Present == FALSE) {
+ MemoryAttrMask = EFI_MEMORY_RP;
+ }
- // Split it
- for (PtIndex = 0; PtIndex < SIZE_4KB / sizeof (*Pt); PtIndex++) {
- Pt[PtIndex] = Address + ((PtIndex << 12) | mAddressEncMask | PAGE_ATTRIBUTE_BITS);
- } // end for PT
+ Base = mProtectionMemRange[Index].Range.Base;
+ Length = mProtectionMemRange[Index].Range.Top - Base;
+ if (MemoryAttrMask != 0) {
+ Status = ConvertMemoryPageAttributes (PageTable, mPagingMode, Base, Length, MemoryAttrMask, TRUE, NULL);
+ ASSERT_RETURN_ERROR (Status);
+ }
- *Pd = (UINT64)(UINTN)Pt | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
- } // end if IsAddressSplit
- } // end for PD
- } // end for PDPT
- } // end for PML4
- } // end for PML5
+ if (Base > PreviousAddress) {
+ //
+ // Mark the ranges not in mProtectionMemRange as non-present.
+ //
+ MemoryAttrMask = EFI_MEMORY_RP;
+ Status = ConvertMemoryPageAttributes (PageTable, mPagingMode, PreviousAddress, Base - PreviousAddress, MemoryAttrMask, TRUE, NULL);
+ ASSERT_RETURN_ERROR (Status);
+ }
- //
- // Go through page table and set several page table entries to absent or execute-disable.
- //
- DEBUG ((DEBUG_INFO, "Patch page table start ...\n"));
- for (Pml5Index = 0; Pml5Index < NumberOfPml5Entries; Pml5Index++) {
- if ((Pml5[Pml5Index] & IA32_PG_P) == 0) {
- //
- // If PML5 entry does not exist, skip it
- //
- continue;
+ PreviousAddress = Base + Length;
}
- Pml4 = (UINT64 *)(UINTN)(Pml5[Pml5Index] & PHYSICAL_ADDRESS_MASK);
- for (Pml4Index = 0; Pml4Index < NumberOfPml4Entries; Pml4Index++) {
- if ((Pml4[Pml4Index] & IA32_PG_P) == 0) {
+ //
+ // This assignment is for setting the last remaining range
+ //
+ MemoryAttrMask = EFI_MEMORY_RP;
+ } else {
+ MemoryAttrMask = EFI_MEMORY_XP;
+ for (Index = 0; Index < mSmmCpuSmramRangeCount; Index++) {
+ Base = mSmmCpuSmramRanges[Index].CpuStart;
+ if (Base > PreviousAddress) {
//
- // If PML4 entry does not exist, skip it
+ // Mark the ranges not in mSmmCpuSmramRanges as NX.
//
- continue;
+ Status = ConvertMemoryPageAttributes (PageTable, mPagingMode, PreviousAddress, Base - PreviousAddress, MemoryAttrMask, TRUE, NULL);
+ ASSERT_RETURN_ERROR (Status);
}
- Pdpt = (UINT64 *)(UINTN)(Pml4[Pml4Index] & ~mAddressEncMask & PHYSICAL_ADDRESS_MASK);
- for (PdptIndex = 0; PdptIndex < NumberOfPdptEntries; PdptIndex++, Pdpt++) {
- if ((*Pdpt & IA32_PG_P) == 0) {
- //
- // If PDPT entry does not exist, skip it
- //
- continue;
- }
-
- if ((*Pdpt & IA32_PG_PS) != 0) {
- //
- // This is 1G entry, set NX bit and skip it
- //
- if (mXdSupported) {
- *Pdpt = *Pdpt | IA32_PG_NX;
- }
-
- continue;
- }
-
- Pd = (UINT64 *)(UINTN)(*Pdpt & ~mAddressEncMask & PHYSICAL_ADDRESS_MASK);
- if (Pd == 0) {
- continue;
- }
+ PreviousAddress = mSmmCpuSmramRanges[Index].CpuStart + mSmmCpuSmramRanges[Index].PhysicalSize;
+ }
+ }
- for (PdIndex = 0; PdIndex < SIZE_4KB / sizeof (*Pd); PdIndex++, Pd++) {
- if ((*Pd & IA32_PG_P) == 0) {
- //
- // If PD entry does not exist, skip it
- //
- continue;
- }
-
- Address = (UINTN)LShiftU64 (
- LShiftU64 (
- LShiftU64 ((Pml5Index << 9) + Pml4Index, 9) + PdptIndex,
- 9
- ) + PdIndex,
- 21
- );
-
- if ((*Pd & IA32_PG_PS) != 0) {
- // 2MB page
-
- if (!IsAddressValid (Address, &Nx)) {
- //
- // Patch to remove Present flag and RW flag
- //
- *Pd = *Pd & (INTN)(INT32)(~PAGE_ATTRIBUTE_BITS);
- }
-
- if (Nx && mXdSupported) {
- *Pd = *Pd | IA32_PG_NX;
- }
- } else {
- // 4KB page
- Pt = (UINT64 *)(UINTN)(*Pd & ~mAddressEncMask & PHYSICAL_ADDRESS_MASK);
- if (Pt == 0) {
- continue;
- }
-
- for (PtIndex = 0; PtIndex < SIZE_4KB / sizeof (*Pt); PtIndex++, Pt++) {
- if (!IsAddressValid (Address, &Nx)) {
- *Pt = *Pt & (INTN)(INT32)(~PAGE_ATTRIBUTE_BITS);
- }
-
- if (Nx && mXdSupported) {
- *Pt = *Pt | IA32_PG_NX;
- }
-
- Address += SIZE_4KB;
- } // end for PT
- } // end if PS
- } // end for PD
- } // end for PDPT
- } // end for PML4
- } // end for PML5
+ if (PreviousAddress < Limit) {
+ //
+ // Set the last remaining range to EFI_MEMORY_RP/EFI_MEMORY_XP.
+ // This path applies to both SmmProfile enable/disable case.
+ //
+ Status = ConvertMemoryPageAttributes (PageTable, mPagingMode, PreviousAddress, Limit - PreviousAddress, MemoryAttrMask, TRUE, NULL);
+ ASSERT_RETURN_ERROR (Status);
+ }
EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Patch V5 14/14] UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
` (12 preceding siblings ...)
2023-06-08 2:27 ` [Patch V5 13/14] UefiCpuPkg: Refinement to smm runtime InitPaging() code duntan
@ 2023-06-08 2:27 ` duntan
13 siblings, 0 replies; 28+ messages in thread
From: duntan @ 2023-06-08 2:27 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
Remove unnecessary function SetNotPresentPage(). We can directly
use ConvertMemoryPageAttributes to set a range to non-present.
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>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 8 ++++++--
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 16 ----------------
UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 22 ----------------------
3 files changed, 6 insertions(+), 40 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 2568ffd677..c2452e6a29 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -1074,10 +1074,14 @@ PiCpuSmmEntry (
mSmmShadowStackSize
);
if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
- SetNotPresentPage (
+ ConvertMemoryPageAttributes (
Cr3,
+ mPagingMode,
(EFI_PHYSICAL_ADDRESS)(UINTN)Stacks + mSmmStackSize + EFI_PAGES_TO_SIZE (1) + (mSmmStackSize + mSmmShadowStackSize) * Index,
- EFI_PAGES_TO_SIZE (1)
+ EFI_PAGES_TO_SIZE (1),
+ EFI_MEMORY_RP,
+ TRUE,
+ NULL
);
}
}
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 12ad86028e..0dc4d758cc 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -1247,22 +1247,6 @@ SetShadowStack (
IN UINT64 Length
);
-/**
- Set not present memory.
-
- @param[in] Cr3 The page table base address.
- @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.
-
- @retval EFI_SUCCESS The not present memory is set.
-**/
-EFI_STATUS
-SetNotPresentPage (
- IN UINTN Cr3,
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length
- );
-
/**
Initialize the shadow stack related data structure.
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 4e93d26eb4..ab502b800b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -768,28 +768,6 @@ SetShadowStack (
return Status;
}
-/**
- Set not present memory.
-
- @param[in] Cr3 The page table base address.
- @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.
-
- @retval EFI_SUCCESS The not present memory is set.
-**/
-EFI_STATUS
-SetNotPresentPage (
- IN UINTN Cr3,
- IN EFI_PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length
- )
-{
- EFI_STATUS Status;
-
- Status = SmmSetMemoryAttributesEx (Cr3, mPagingMode, BaseAddress, Length, EFI_MEMORY_RP);
- return Status;
-}
-
/**
Retrieves a pointer to the system configuration table from the SMM System Table
based on a specified GUID.
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page
2023-06-08 2:27 ` [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page duntan
@ 2023-06-08 10:08 ` Ni, Ray
2023-06-08 12:18 ` [edk2-devel] " Ard Biesheuvel
1 sibling, 0 replies; 28+ messages in thread
From: Ni, Ray @ 2023-06-08 10:08 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io; +Cc: Gao, Liming, Wang, Jian J
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>
> Subject: [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection
> when unset guard page
>
> Remove RO and NX protection when unset guard page.
> When UnsetGuardPage(), remove all the memory attribute protection
> for guarded page.
>
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> ---
> MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> index 8f3bab6fee..7daeeccf13 100644
> --- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> +++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> @@ -553,7 +553,7 @@ UnsetGuardPage (
> mSmmMemoryAttribute,
> BaseAddress,
> EFI_PAGE_SIZE,
> - EFI_MEMORY_RP
> + EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMORY_XP
> );
> ASSERT_EFI_ERROR (Status);
> mOnGuarding = FALSE;
> --
> 2.31.1.windows.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Patch V5 09/14] UefiCpuPkg: Add GenSmmPageTable() to create smm page table
2023-06-08 2:27 ` [Patch V5 09/14] UefiCpuPkg: Add GenSmmPageTable() to create smm page table duntan
@ 2023-06-08 10:16 ` Ni, Ray
0 siblings, 0 replies; 28+ messages in thread
From: Ni, Ray @ 2023-06-08 10:16 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V5 09/14] UefiCpuPkg: Add GenSmmPageTable() to create
> smm page table
>
> This commit is code refinement to current smm pagetable generation
> code. Add a new GenSmmPageTable() API to create smm page table
> based on the PageTableMap() API in CpuPageTableLib. Caller only
> needs to specify the paging mode and the PhysicalAddressBits to map.
> This function can be used to create both IA32 pae paging and X64
> 5level, 4level paging.
>
> 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 | 2 +-
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 15
> +++++++++++++++
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 65
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 220
> ++++++++++++++++++++++++++----------------------------------------------------
> ----------------------------------------------------------------------------------------------
> ------------------------------------------------
> 4 files changed, 107 insertions(+), 195 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> index 9c8107080a..b11264ce4a 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> @@ -63,7 +63,7 @@ SmmInitPageTable (
> InitializeIDTSmmStackGuard ();
> }
>
> - return Gen4GPageTable (TRUE);
> + return GenSmmPageTable (PagingPae, mPhysicalAddressBits);
> }
>
> /**
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index a7da9673a5..5399659bc0 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -553,6 +553,21 @@ Gen4GPageTable (
> IN BOOLEAN Is32BitPageTable
> );
>
> +/**
> + Create page table based on input PagingMode and PhysicalAddressBits in
> smm.
> +
> + @param[in] PagingMode The paging mode.
> + @param[in] PhysicalAddressBits The bits of physical address to map.
> +
> + @retval PageTable Address
> +
> +**/
> +UINTN
> +GenSmmPageTable (
> + IN PAGING_MODE PagingMode,
> + IN UINT8 PhysicalAddressBits
> + );
> +
> /**
> Initialize global data for MP synchronization.
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 4ee99d06d7..4e93d26eb4 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -1640,6 +1640,71 @@ EdkiiSmmClearMemoryAttributes (
> return SmmClearMemoryAttributes (BaseAddress, Length, Attributes);
> }
>
> +/**
> + Create page table based on input PagingMode and PhysicalAddressBits in
> smm.
> +
> + @param[in] PagingMode The paging mode.
> + @param[in] PhysicalAddressBits The bits of physical address to map.
> +
> + @retval PageTable Address
> +
> +**/
> +UINTN
> +GenSmmPageTable (
> + IN PAGING_MODE PagingMode,
> + IN UINT8 PhysicalAddressBits
> + )
> +{
> + UINTN PageTableBufferSize;
> + UINTN PageTable;
> + VOID *PageTableBuffer;
> + IA32_MAP_ATTRIBUTE MapAttribute;
> + IA32_MAP_ATTRIBUTE MapMask;
> + RETURN_STATUS Status;
> + UINTN GuardPage;
> + UINTN Index;
> + UINT64 Length;
> +
> + Length = LShiftU64 (1, PhysicalAddressBits);
> + PageTable = 0;
> + PageTableBufferSize = 0;
> + MapMask.Uint64 = MAX_UINT64;
> + MapAttribute.Uint64 = mAddressEncMask;
> + MapAttribute.Bits.Present = 1;
> + MapAttribute.Bits.ReadWrite = 1;
> + MapAttribute.Bits.UserSupervisor = 1;
> + MapAttribute.Bits.Accessed = 1;
> + MapAttribute.Bits.Dirty = 1;
> +
> + Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, 0, Length, &MapAttribute, &MapMask, NULL);
> + ASSERT (Status == RETURN_BUFFER_TOO_SMALL);
> + DEBUG ((DEBUG_INFO, "GenSMMPageTable: 0x%x bytes needed for initial
> SMM page table\n", PageTableBufferSize));
> + PageTableBuffer = AllocatePageTableMemory (EFI_SIZE_TO_PAGES
> (PageTableBufferSize));
> + ASSERT (PageTableBuffer != NULL);
> + Status = PageTableMap (&PageTable, PagingMode, PageTableBuffer,
> &PageTableBufferSize, 0, Length, &MapAttribute, &MapMask, NULL);
> + ASSERT (Status == RETURN_SUCCESS);
> + ASSERT (PageTableBufferSize == 0);
> +
> + if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> + //
> + // Mark the 4KB guard page between known good stack and smm stack as
> non-present
> + //
> + for (Index = 0; Index < gSmmCpuPrivate-
> >SmmCoreEntryContext.NumberOfCpus; Index++) {
> + GuardPage = mSmmStackArrayBase + EFI_PAGE_SIZE + Index *
> (mSmmStackSize + mSmmShadowStackSize);
> + Status = ConvertMemoryPageAttributes (PageTable, PagingMode,
> GuardPage, SIZE_4KB, EFI_MEMORY_RP, TRUE, NULL);
> + }
> + }
> +
> + if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0) {
> + //
> + // Mark [0, 4k] as non-present
> + //
> + Status = ConvertMemoryPageAttributes (PageTable, PagingMode, 0,
> SIZE_4KB, EFI_MEMORY_RP, TRUE, NULL);
> + }
> +
> + return (UINTN)PageTable;
> +}
> +
> /**
> This function retrieves the attributes of the memory region specified by
> BaseAddress and Length. If different attributes are got from different part
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index 46d8ff5d4e..ddd9be66b5 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -167,160 +167,6 @@ CalculateMaximumSupportAddress (
> return PhysicalAddressBits;
> }
>
> -/**
> - Set static page table.
> -
> - @param[in] PageTable Address of page table.
> - @param[in] PhysicalAddressBits The maximum physical address bits
> supported.
> -**/
> -VOID
> -SetStaticPageTable (
> - IN UINTN PageTable,
> - IN UINT8 PhysicalAddressBits
> - )
> -{
> - UINT64 PageAddress;
> - UINTN NumberOfPml5EntriesNeeded;
> - UINTN NumberOfPml4EntriesNeeded;
> - UINTN NumberOfPdpEntriesNeeded;
> - UINTN IndexOfPml5Entries;
> - UINTN IndexOfPml4Entries;
> - UINTN IndexOfPdpEntries;
> - UINTN IndexOfPageDirectoryEntries;
> - UINT64 *PageMapLevel5Entry;
> - UINT64 *PageMapLevel4Entry;
> - UINT64 *PageMap;
> - UINT64 *PageDirectoryPointerEntry;
> - UINT64 *PageDirectory1GEntry;
> - UINT64 *PageDirectoryEntry;
> -
> - //
> - // IA-32e paging translates 48-bit linear addresses to 52-bit physical
> addresses
> - // when 5-Level Paging is disabled.
> - //
> - ASSERT (PhysicalAddressBits <= 52);
> - if (!m5LevelPagingNeeded && (PhysicalAddressBits > 48)) {
> - PhysicalAddressBits = 48;
> - }
> -
> - NumberOfPml5EntriesNeeded = 1;
> - if (PhysicalAddressBits > 48) {
> - NumberOfPml5EntriesNeeded = (UINTN)LShiftU64 (1, PhysicalAddressBits
> - 48);
> - PhysicalAddressBits = 48;
> - }
> -
> - NumberOfPml4EntriesNeeded = 1;
> - if (PhysicalAddressBits > 39) {
> - NumberOfPml4EntriesNeeded = (UINTN)LShiftU64 (1, PhysicalAddressBits
> - 39);
> - PhysicalAddressBits = 39;
> - }
> -
> - NumberOfPdpEntriesNeeded = 1;
> - ASSERT (PhysicalAddressBits > 30);
> - NumberOfPdpEntriesNeeded = (UINTN)LShiftU64 (1, PhysicalAddressBits -
> 30);
> -
> - //
> - // By architecture only one PageMapLevel4 exists - so lets allocate storage for
> it.
> - //
> - PageMap = (VOID *)PageTable;
> -
> - PageMapLevel4Entry = PageMap;
> - PageMapLevel5Entry = NULL;
> - if (m5LevelPagingNeeded) {
> - //
> - // By architecture only one PageMapLevel5 exists - so lets allocate storage
> for it.
> - //
> - PageMapLevel5Entry = PageMap;
> - }
> -
> - PageAddress = 0;
> -
> - for ( IndexOfPml5Entries = 0
> - ; IndexOfPml5Entries < NumberOfPml5EntriesNeeded
> - ; IndexOfPml5Entries++, PageMapLevel5Entry++)
> - {
> - //
> - // Each PML5 entry points to a page of PML4 entires.
> - // So lets allocate space for them and fill them in in the IndexOfPml4Entries
> loop.
> - // When 5-Level Paging is disabled, below allocation happens only once.
> - //
> - if (m5LevelPagingNeeded) {
> - PageMapLevel4Entry = (UINT64 *)((*PageMapLevel5Entry) &
> ~mAddressEncMask & gPhyMask);
> - if (PageMapLevel4Entry == NULL) {
> - PageMapLevel4Entry = AllocatePageTableMemory (1);
> - ASSERT (PageMapLevel4Entry != NULL);
> - ZeroMem (PageMapLevel4Entry, EFI_PAGES_TO_SIZE (1));
> -
> - *PageMapLevel5Entry = (UINT64)(UINTN)PageMapLevel4Entry |
> mAddressEncMask | PAGE_ATTRIBUTE_BITS;
> - }
> - }
> -
> - for (IndexOfPml4Entries = 0; IndexOfPml4Entries <
> (NumberOfPml5EntriesNeeded == 1 ? NumberOfPml4EntriesNeeded : 512);
> IndexOfPml4Entries++, PageMapLevel4Entry++) {
> - //
> - // Each PML4 entry points to a page of Page Directory Pointer entries.
> - //
> - PageDirectoryPointerEntry = (UINT64 *)((*PageMapLevel4Entry) &
> ~mAddressEncMask & gPhyMask);
> - if (PageDirectoryPointerEntry == NULL) {
> - PageDirectoryPointerEntry = AllocatePageTableMemory (1);
> - ASSERT (PageDirectoryPointerEntry != NULL);
> - ZeroMem (PageDirectoryPointerEntry, EFI_PAGES_TO_SIZE (1));
> -
> - *PageMapLevel4Entry = (UINT64)(UINTN)PageDirectoryPointerEntry |
> mAddressEncMask | PAGE_ATTRIBUTE_BITS;
> - }
> -
> - if (m1GPageTableSupport) {
> - PageDirectory1GEntry = PageDirectoryPointerEntry;
> - for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512;
> IndexOfPageDirectoryEntries++, PageDirectory1GEntry++, PageAddress +=
> SIZE_1GB) {
> - if ((IndexOfPml4Entries == 0) && (IndexOfPageDirectoryEntries < 4)) {
> - //
> - // Skip the < 4G entries
> - //
> - continue;
> - }
> -
> - //
> - // Fill in the Page Directory entries
> - //
> - *PageDirectory1GEntry = PageAddress | mAddressEncMask |
> IA32_PG_PS | PAGE_ATTRIBUTE_BITS;
> - }
> - } else {
> - PageAddress = BASE_4GB;
> - for (IndexOfPdpEntries = 0; IndexOfPdpEntries <
> (NumberOfPml4EntriesNeeded == 1 ? NumberOfPdpEntriesNeeded : 512);
> IndexOfPdpEntries++, PageDirectoryPointerEntry++) {
> - if ((IndexOfPml4Entries == 0) && (IndexOfPdpEntries < 4)) {
> - //
> - // Skip the < 4G entries
> - //
> - continue;
> - }
> -
> - //
> - // Each Directory Pointer entries points to a page of Page Directory
> entires.
> - // So allocate space for them and fill them in in the
> IndexOfPageDirectoryEntries loop.
> - //
> - PageDirectoryEntry = (UINT64 *)((*PageDirectoryPointerEntry) &
> ~mAddressEncMask & gPhyMask);
> - if (PageDirectoryEntry == NULL) {
> - PageDirectoryEntry = AllocatePageTableMemory (1);
> - ASSERT (PageDirectoryEntry != NULL);
> - ZeroMem (PageDirectoryEntry, EFI_PAGES_TO_SIZE (1));
> -
> - //
> - // Fill in a Page Directory Pointer Entries
> - //
> - *PageDirectoryPointerEntry = (UINT64)(UINTN)PageDirectoryEntry |
> mAddressEncMask | PAGE_ATTRIBUTE_BITS;
> - }
> -
> - for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries <
> 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PageAddress +=
> SIZE_2MB) {
> - //
> - // Fill in the Page Directory entries
> - //
> - *PageDirectoryEntry = PageAddress | mAddressEncMask | IA32_PG_PS
> | PAGE_ATTRIBUTE_BITS;
> - }
> - }
> - }
> - }
> - }
> -}
> -
> /**
> Create PageTable for SMM use.
>
> @@ -332,15 +178,16 @@ SmmInitPageTable (
> VOID
> )
> {
> - EFI_PHYSICAL_ADDRESS Pages;
> - UINT64 *PTEntry;
> + UINTN PageTable;
> LIST_ENTRY *FreePage;
> UINTN Index;
> UINTN PageFaultHandlerHookAddress;
> IA32_IDT_GATE_DESCRIPTOR *IdtEntry;
> EFI_STATUS Status;
> + UINT64 *PdptEntry;
> UINT64 *Pml4Entry;
> UINT64 *Pml5Entry;
> + UINT8 PhysicalAddressBits;
>
> //
> // Initialize spin lock
> @@ -357,59 +204,44 @@ SmmInitPageTable (
> } 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));
> DEBUG ((DEBUG_INFO, "PhysicalAddressBits - %d\n",
> mPhysicalAddressBits));
> - //
> - // Generate PAE page table for the first 4GB memory space
> - //
> - Pages = Gen4GPageTable (FALSE);
>
> //
> - // Set IA32_PG_PMNT bit to mask this entry
> + // Generate initial SMM page table.
> + // Only map [0, 4G] when PcdCpuSmmRestrictedMemoryAccess is FALSE.
> //
> - PTEntry = (UINT64 *)(UINTN)Pages;
> - for (Index = 0; Index < 4; Index++) {
> - PTEntry[Index] |= IA32_PG_PMNT;
> - }
> -
> - //
> - // Fill Page-Table-Level4 (PML4) entry
> - //
> - Pml4Entry = (UINT64 *)AllocatePageTableMemory (1);
> - ASSERT (Pml4Entry != NULL);
> - *Pml4Entry = Pages | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
> - ZeroMem (Pml4Entry + 1, EFI_PAGE_SIZE - sizeof (*Pml4Entry));
> -
> - //
> - // Set sub-entries number
> - //
> - SetSubEntriesNum (Pml4Entry, 3);
> - PTEntry = Pml4Entry;
> + PhysicalAddressBits = mCpuSmmRestrictedMemoryAccess ?
> mPhysicalAddressBits : 32;
> + PageTable = GenSmmPageTable (mPagingMode, PhysicalAddressBits);
>
> if (m5LevelPagingNeeded) {
> + Pml5Entry = (UINT64 *)PageTable;
> //
> - // Fill PML5 entry
> - //
> - Pml5Entry = (UINT64 *)AllocatePageTableMemory (1);
> - ASSERT (Pml5Entry != NULL);
> - *Pml5Entry = (UINTN)Pml4Entry | mAddressEncMask |
> PAGE_ATTRIBUTE_BITS;
> - ZeroMem (Pml5Entry + 1, EFI_PAGE_SIZE - sizeof (*Pml5Entry));
> - //
> - // Set sub-entries number
> + // Set Pml5Entry sub-entries number for smm PF handler usage.
> //
> SetSubEntriesNum (Pml5Entry, 1);
> - PTEntry = Pml5Entry;
> + Pml4Entry = (UINT64 *)((*Pml5Entry) & ~mAddressEncMask &
> gPhyMask);
> + } else {
> + Pml4Entry = (UINT64 *)PageTable;
> + }
> +
> + //
> + // Set IA32_PG_PMNT bit to mask first 4 PdptEntry.
> + //
> + PdptEntry = (UINT64 *)((*Pml4Entry) & ~mAddressEncMask & gPhyMask);
> + for (Index = 0; Index < 4; Index++) {
> + PdptEntry[Index] |= IA32_PG_PMNT;
> }
>
> - if (mCpuSmmRestrictedMemoryAccess) {
> + if (!mCpuSmmRestrictedMemoryAccess) {
> //
> - // When access to non-SMRAM memory is restricted, create page table
> - // that covers all memory space.
> + // Set Pml4Entry sub-entries number for smm PF handler usage.
> //
> - SetStaticPageTable ((UINTN)PTEntry, mPhysicalAddressBits);
> - } else {
> + SetSubEntriesNum (Pml4Entry, 3);
> +
> //
> // Add pages to page pool
> //
> @@ -466,7 +298,7 @@ SmmInitPageTable (
> //
> // Return the address of PML4/PML5 (to set CR3)
> //
> - return (UINT32)(UINTN)PTEntry;
> + return (UINT32)PageTable;
> }
>
> /**
> --
> 2.31.1.windows.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Patch V5 11/14] UefiCpuPkg: Sort mSmmCpuSmramRanges in FindSmramInfo
2023-06-08 2:27 ` [Patch V5 11/14] UefiCpuPkg: Sort mSmmCpuSmramRanges in FindSmramInfo duntan
@ 2023-06-08 10:16 ` Ni, Ray
0 siblings, 0 replies; 28+ messages in thread
From: Ni, Ray @ 2023-06-08 10:16 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann
Reviewed-by: Ray Ni <Ray.ni@intel.com>
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V5 11/14] UefiCpuPkg: Sort mSmmCpuSmramRanges in
> FindSmramInfo
>
> Sort mSmmCpuSmramRanges after get the SMRAM info in
> FindSmramInfo() function.
>
> 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/PiSmmCpuDxeSmm.c | 32
> ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index 2144d6ade8..2568ffd677 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -1197,6 +1197,32 @@ PiCpuSmmEntry (
> return EFI_SUCCESS;
> }
>
> +/**
> + Function to compare 2 EFI_SMRAM_DESCRIPTOR based on CpuStart.
> +
> + @param[in] Buffer1 pointer to Device Path poiner to compare
> + @param[in] Buffer2 pointer to second DevicePath pointer to compare
> +
> + @retval 0 Buffer1 equal to Buffer2
> + @retval <0 Buffer1 is less than Buffer2
> + @retval >0 Buffer1 is greater than Buffer2
> +**/
> +INTN
> +EFIAPI
> +CpuSmramRangeCompare (
> + IN CONST VOID *Buffer1,
> + IN CONST VOID *Buffer2
> + )
> +{
> + if (((EFI_SMRAM_DESCRIPTOR *)Buffer1)->CpuStart >
> ((EFI_SMRAM_DESCRIPTOR *)Buffer2)->CpuStart) {
> + return 1;
> + } else if (((EFI_SMRAM_DESCRIPTOR *)Buffer1)->CpuStart <
> ((EFI_SMRAM_DESCRIPTOR *)Buffer2)->CpuStart) {
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> /**
>
> Find out SMRAM information including SMRR base and SMRR size.
> @@ -1218,6 +1244,7 @@ FindSmramInfo (
> UINTN Index;
> UINT64 MaxSize;
> BOOLEAN Found;
> + EFI_SMRAM_DESCRIPTOR SmramDescriptor;
>
> //
> // Get SMM Access Protocol
> @@ -1240,6 +1267,11 @@ FindSmramInfo (
>
> mSmmCpuSmramRangeCount = Size / sizeof (EFI_SMRAM_DESCRIPTOR);
>
> + //
> + // Sort the mSmmCpuSmramRanges
> + //
> + QuickSort (mSmmCpuSmramRanges, mSmmCpuSmramRangeCount, sizeof
> (EFI_SMRAM_DESCRIPTOR),
> (BASE_SORT_COMPARE)CpuSmramRangeCompare, &SmramDescriptor);
> +
> //
> // Find the largest SMRAM range between 1MB and 4GB that is at least 256K
> - 4K in size
> //
> --
> 2.31.1.windows.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Patch V5 12/14] UefiCpuPkg: Sort mProtectionMemRange when ReadyToLock
2023-06-08 2:27 ` [Patch V5 12/14] UefiCpuPkg: Sort mProtectionMemRange when ReadyToLock duntan
@ 2023-06-08 10:17 ` Ni, Ray
0 siblings, 0 replies; 28+ messages in thread
From: Ni, Ray @ 2023-06-08 10:17 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V5 12/14] UefiCpuPkg: Sort mProtectionMemRange when
> ReadyToLock
>
> Sort mProtectionMemRange in InitProtectedMemRange() when
> ReadyToLock.
>
> 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/SmmProfile.c | 32
> ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index 5625ba0cac..2a1f132b29 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -375,6 +375,32 @@ IsAddressSplit (
> return FALSE;
> }
>
> +/**
> + Function to compare 2 MEMORY_PROTECTION_RANGE based on range
> base.
> +
> + @param[in] Buffer1 pointer to Device Path poiner to compare
> + @param[in] Buffer2 pointer to second DevicePath pointer to compare
> +
> + @retval 0 Buffer1 equal to Buffer2
> + @retval <0 Buffer1 is less than Buffer2
> + @retval >0 Buffer1 is greater than Buffer2
> +**/
> +INTN
> +EFIAPI
> +ProtectionRangeCompare (
> + IN CONST VOID *Buffer1,
> + IN CONST VOID *Buffer2
> + )
> +{
> + if (((MEMORY_PROTECTION_RANGE *)Buffer1)->Range.Base >
> ((MEMORY_PROTECTION_RANGE *)Buffer2)->Range.Base) {
> + return 1;
> + } else if (((MEMORY_PROTECTION_RANGE *)Buffer1)->Range.Base <
> ((MEMORY_PROTECTION_RANGE *)Buffer2)->Range.Base) {
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> /**
> Initialize the protected memory ranges and the 4KB-page mapped memory
> ranges.
>
> @@ -397,6 +423,7 @@ InitProtectedMemRange (
> EFI_PHYSICAL_ADDRESS Base2MBAlignedAddress;
> UINT64 High4KBPageSize;
> UINT64 Low4KBPageSize;
> + MEMORY_PROTECTION_RANGE MemProtectionRange;
>
> NumberOfDescriptors = 0;
> NumberOfAddedDescriptors = mSmmCpuSmramRangeCount;
> @@ -533,6 +560,11 @@ InitProtectedMemRange (
>
> mSplitMemRangeCount = NumberOfSpliteRange;
>
> + //
> + // Sort the mProtectionMemRange
> + //
> + QuickSort (mProtectionMemRange, mProtectionMemRangeCount, sizeof
> (MEMORY_PROTECTION_RANGE),
> (BASE_SORT_COMPARE)ProtectionRangeCompare, &MemProtectionRange);
> +
> DEBUG ((DEBUG_INFO, "SMM Profile Memory Ranges:\n"));
> for (Index = 0; Index < mProtectionMemRangeCount; Index++) {
> DEBUG ((DEBUG_INFO, "mProtectionMemRange[%d].Base = %lx\n", Index,
> mProtectionMemRange[Index].Range.Base));
> --
> 2.31.1.windows.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Patch V5 13/14] UefiCpuPkg: Refinement to smm runtime InitPaging() code
2023-06-08 2:27 ` [Patch V5 13/14] UefiCpuPkg: Refinement to smm runtime InitPaging() code duntan
@ 2023-06-08 10:18 ` Ni, Ray
0 siblings, 0 replies; 28+ messages in thread
From: Ni, Ray @ 2023-06-08 10:18 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V5 13/14] UefiCpuPkg: Refinement to smm runtime
> InitPaging() code
>
> This commit is code refinement to current smm runtime InitPaging()
> page table update code. In InitPaging(), if PcdCpuSmmProfileEnable
> is TRUE, use ConvertMemoryPageAttributes() API to map the range in
> mProtectionMemRange to the attrbute recorded in the attribute field
> of mProtectionMemRange, map the range outside mProtectionMemRange
> as non-present. If PcdCpuSmmProfileEnable is FALSE, only need to
> set the ranges not in mSmmCpuSmramRanges as NX.
>
> 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/PiSmmCpuDxeSmm.h | 37
> +++++++++++++++++++++++++++++++++++++
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 293
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++------------------------------------------------------------------------------------
> ----------------------------------------------------------------------------------------------
> ---------------------------------------------------
> 2 files changed, 101 insertions(+), 229 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index 5399659bc0..12ad86028e 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -725,6 +725,43 @@ SmmBlockingStartupThisAp (
> IN OUT VOID *ProcArguments OPTIONAL
> );
>
> +/**
> + 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.
> +
> + Caller should make sure BaseAddress and Length is at page boundary.
> +
> + @param[in] PageTableBase The page table base.
> + @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] IsModified TRUE means page table modified. FALSE means
> page table not modified.
> +
> + @retval RETURN_SUCCESS The attributes were modified for the
> memory region.
> + @retval RETURN_ACCESS_DENIED The attributes for the memory resource
> range specified by
> + BaseAddress and Length cannot be modified.
> + @retval RETURN_INVALID_PARAMETER Length is zero.
> + Attributes specified an illegal combination of attributes that
> + cannot be set together.
> + @retval RETURN_OUT_OF_RESOURCES There are not enough system
> resources to modify the attributes of
> + the memory resource range.
> + @retval RETURN_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
> + range specified by BaseAddress and Length.
> +**/
> +RETURN_STATUS
> +ConvertMemoryPageAttributes (
> + IN UINTN PageTableBase,
> + IN PAGING_MODE PagingMode,
> + IN PHYSICAL_ADDRESS BaseAddress,
> + IN UINT64 Length,
> + IN UINT64 Attributes,
> + IN BOOLEAN IsSet,
> + OUT BOOLEAN *IsModified OPTIONAL
> + );
> +
> /**
> This function sets the attributes for the memory region specified by
> BaseAddress and
> Length from their current attributes to the attributes specified by Attributes.
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index 2a1f132b29..ab6b79ddf4 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -586,254 +586,89 @@ InitPaging (
> VOID
> )
> {
> - UINT64 Pml5Entry;
> - UINT64 Pml4Entry;
> - UINT64 *Pml5;
> - UINT64 *Pml4;
> - UINT64 *Pdpt;
> - UINT64 *Pd;
> - UINT64 *Pt;
> - UINTN Address;
> - UINTN Pml5Index;
> - UINTN Pml4Index;
> - UINTN PdptIndex;
> - UINTN PdIndex;
> - UINTN PtIndex;
> - UINTN NumberOfPdptEntries;
> - UINTN NumberOfPml4Entries;
> - UINTN NumberOfPml5Entries;
> - UINTN SizeOfMemorySpace;
> - BOOLEAN Nx;
> - IA32_CR4 Cr4;
> - BOOLEAN Enable5LevelPaging;
> - BOOLEAN WpEnabled;
> - BOOLEAN CetEnabled;
> -
> - Cr4.UintN = AsmReadCr4 ();
> - Enable5LevelPaging = (BOOLEAN)(Cr4.Bits.LA57 == 1);
> -
> - if (sizeof (UINTN) == sizeof (UINT64)) {
> - if (!Enable5LevelPaging) {
> - Pml5Entry = (UINTN)mSmmProfileCr3 | IA32_PG_P;
> - Pml5 = &Pml5Entry;
> - } else {
> - Pml5 = (UINT64 *)(UINTN)mSmmProfileCr3;
> - }
> -
> - SizeOfMemorySpace = HighBitSet64 (gPhyMask) + 1;
> - ASSERT (SizeOfMemorySpace <= 52);
> -
> - //
> - // Calculate the table entries of PML5E, PML4E and PDPTE.
> - //
> - NumberOfPml5Entries = 1;
> - if (SizeOfMemorySpace > 48) {
> - if (Enable5LevelPaging) {
> - NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace -
> 48);
> - }
> -
> - SizeOfMemorySpace = 48;
> - }
> -
> - NumberOfPml4Entries = 1;
> - if (SizeOfMemorySpace > 39) {
> - NumberOfPml4Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 39);
> - SizeOfMemorySpace = 39;
> - }
> -
> - NumberOfPdptEntries = 1;
> - ASSERT (SizeOfMemorySpace > 30);
> - NumberOfPdptEntries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 30);
> + RETURN_STATUS Status;
> + UINTN Index;
> + UINTN PageTable;
> + UINT64 Base;
> + UINT64 Length;
> + UINT64 Limit;
> + UINT64 PreviousAddress;
> + UINT64 MemoryAttrMask;
> + BOOLEAN WpEnabled;
> + BOOLEAN CetEnabled;
> +
> + PageTable = AsmReadCr3 ();
> + if (sizeof (UINTN) == sizeof (UINT32)) {
> + Limit = BASE_4GB;
> } else {
> - Pml4Entry = (UINTN)mSmmProfileCr3 | IA32_PG_P;
> - Pml4 = &Pml4Entry;
> - Pml5Entry = (UINTN)Pml4 | IA32_PG_P;
> - Pml5 = &Pml5Entry;
> - NumberOfPml5Entries = 1;
> - NumberOfPml4Entries = 1;
> - NumberOfPdptEntries = 4;
> + Limit = (IsRestrictedMemoryAccess ()) ? LShiftU64 (1,
> mPhysicalAddressBits) : BASE_4GB;
> }
>
> DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
> //
> - // Go through page table and change 2MB-page into 4KB-page.
> + // [0, 4k] may be non-present.
> //
> - for (Pml5Index = 0; Pml5Index < NumberOfPml5Entries; Pml5Index++) {
> - if ((Pml5[Pml5Index] & IA32_PG_P) == 0) {
> - //
> - // If PML5 entry does not exist, skip it
> - //
> - continue;
> - }
> + PreviousAddress = ((PcdGet8 (PcdNullPointerDetectionPropertyMask) &
> BIT1) != 0) ? BASE_4KB : 0;
>
> - Pml4 = (UINT64 *)(UINTN)(Pml5[Pml5Index] &
> PHYSICAL_ADDRESS_MASK);
> - for (Pml4Index = 0; Pml4Index < NumberOfPml4Entries; Pml4Index++) {
> - if ((Pml4[Pml4Index] & IA32_PG_P) == 0) {
> - //
> - // If PML4 entry does not exist, skip it
> - //
> - continue;
> + DEBUG ((DEBUG_INFO, "Patch page table start ...\n"));
> + if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
> + for (Index = 0; Index < mProtectionMemRangeCount; Index++) {
> + MemoryAttrMask = 0;
> + if (mProtectionMemRange[Index].Nx == TRUE) {
> + MemoryAttrMask |= EFI_MEMORY_XP;
> }
>
> - Pdpt = (UINT64 *)(UINTN)(Pml4[Pml4Index] & ~mAddressEncMask &
> PHYSICAL_ADDRESS_MASK);
> - for (PdptIndex = 0; PdptIndex < NumberOfPdptEntries; PdptIndex++,
> Pdpt++) {
> - if ((*Pdpt & IA32_PG_P) == 0) {
> - //
> - // If PDPT entry does not exist, skip it
> - //
> - continue;
> - }
> -
> - if ((*Pdpt & IA32_PG_PS) != 0) {
> - //
> - // This is 1G entry, skip it
> - //
> - continue;
> - }
> -
> - Pd = (UINT64 *)(UINTN)(*Pdpt & ~mAddressEncMask &
> PHYSICAL_ADDRESS_MASK);
> - if (Pd == 0) {
> - continue;
> - }
> -
> - for (PdIndex = 0; PdIndex < SIZE_4KB / sizeof (*Pd); PdIndex++, Pd++) {
> - if ((*Pd & IA32_PG_P) == 0) {
> - //
> - // If PD entry does not exist, skip it
> - //
> - continue;
> - }
> -
> - Address = (UINTN)LShiftU64 (
> - LShiftU64 (
> - LShiftU64 ((Pml5Index << 9) + Pml4Index, 9) + PdptIndex,
> - 9
> - ) + PdIndex,
> - 21
> - );
> -
> - //
> - // If it is 2M page, check IsAddressSplit()
> - //
> - if (((*Pd & IA32_PG_PS) != 0) && IsAddressSplit (Address)) {
> - //
> - // Based on current page table, create 4KB page table for split area.
> - //
> - ASSERT (Address == (*Pd & PHYSICAL_ADDRESS_MASK));
> -
> - Pt = AllocatePageTableMemory (1);
> - ASSERT (Pt != NULL);
> + if (mProtectionMemRange[Index].Present == FALSE) {
> + MemoryAttrMask = EFI_MEMORY_RP;
> + }
>
> - // Split it
> - for (PtIndex = 0; PtIndex < SIZE_4KB / sizeof (*Pt); PtIndex++) {
> - Pt[PtIndex] = Address + ((PtIndex << 12) | mAddressEncMask |
> PAGE_ATTRIBUTE_BITS);
> - } // end for PT
> + Base = mProtectionMemRange[Index].Range.Base;
> + Length = mProtectionMemRange[Index].Range.Top - Base;
> + if (MemoryAttrMask != 0) {
> + Status = ConvertMemoryPageAttributes (PageTable, mPagingMode, Base,
> Length, MemoryAttrMask, TRUE, NULL);
> + ASSERT_RETURN_ERROR (Status);
> + }
>
> - *Pd = (UINT64)(UINTN)Pt | mAddressEncMask |
> PAGE_ATTRIBUTE_BITS;
> - } // end if IsAddressSplit
> - } // end for PD
> - } // end for PDPT
> - } // end for PML4
> - } // end for PML5
> + if (Base > PreviousAddress) {
> + //
> + // Mark the ranges not in mProtectionMemRange as non-present.
> + //
> + MemoryAttrMask = EFI_MEMORY_RP;
> + Status = ConvertMemoryPageAttributes (PageTable, mPagingMode,
> PreviousAddress, Base - PreviousAddress, MemoryAttrMask, TRUE, NULL);
> + ASSERT_RETURN_ERROR (Status);
> + }
>
> - //
> - // Go through page table and set several page table entries to absent or
> execute-disable.
> - //
> - DEBUG ((DEBUG_INFO, "Patch page table start ...\n"));
> - for (Pml5Index = 0; Pml5Index < NumberOfPml5Entries; Pml5Index++) {
> - if ((Pml5[Pml5Index] & IA32_PG_P) == 0) {
> - //
> - // If PML5 entry does not exist, skip it
> - //
> - continue;
> + PreviousAddress = Base + Length;
> }
>
> - Pml4 = (UINT64 *)(UINTN)(Pml5[Pml5Index] &
> PHYSICAL_ADDRESS_MASK);
> - for (Pml4Index = 0; Pml4Index < NumberOfPml4Entries; Pml4Index++) {
> - if ((Pml4[Pml4Index] & IA32_PG_P) == 0) {
> + //
> + // This assignment is for setting the last remaining range
> + //
> + MemoryAttrMask = EFI_MEMORY_RP;
> + } else {
> + MemoryAttrMask = EFI_MEMORY_XP;
> + for (Index = 0; Index < mSmmCpuSmramRangeCount; Index++) {
> + Base = mSmmCpuSmramRanges[Index].CpuStart;
> + if (Base > PreviousAddress) {
> //
> - // If PML4 entry does not exist, skip it
> + // Mark the ranges not in mSmmCpuSmramRanges as NX.
> //
> - continue;
> + Status = ConvertMemoryPageAttributes (PageTable, mPagingMode,
> PreviousAddress, Base - PreviousAddress, MemoryAttrMask, TRUE, NULL);
> + ASSERT_RETURN_ERROR (Status);
> }
>
> - Pdpt = (UINT64 *)(UINTN)(Pml4[Pml4Index] & ~mAddressEncMask &
> PHYSICAL_ADDRESS_MASK);
> - for (PdptIndex = 0; PdptIndex < NumberOfPdptEntries; PdptIndex++,
> Pdpt++) {
> - if ((*Pdpt & IA32_PG_P) == 0) {
> - //
> - // If PDPT entry does not exist, skip it
> - //
> - continue;
> - }
> -
> - if ((*Pdpt & IA32_PG_PS) != 0) {
> - //
> - // This is 1G entry, set NX bit and skip it
> - //
> - if (mXdSupported) {
> - *Pdpt = *Pdpt | IA32_PG_NX;
> - }
> -
> - continue;
> - }
> -
> - Pd = (UINT64 *)(UINTN)(*Pdpt & ~mAddressEncMask &
> PHYSICAL_ADDRESS_MASK);
> - if (Pd == 0) {
> - continue;
> - }
> + PreviousAddress = mSmmCpuSmramRanges[Index].CpuStart +
> mSmmCpuSmramRanges[Index].PhysicalSize;
> + }
> + }
>
> - for (PdIndex = 0; PdIndex < SIZE_4KB / sizeof (*Pd); PdIndex++, Pd++) {
> - if ((*Pd & IA32_PG_P) == 0) {
> - //
> - // If PD entry does not exist, skip it
> - //
> - continue;
> - }
> -
> - Address = (UINTN)LShiftU64 (
> - LShiftU64 (
> - LShiftU64 ((Pml5Index << 9) + Pml4Index, 9) + PdptIndex,
> - 9
> - ) + PdIndex,
> - 21
> - );
> -
> - if ((*Pd & IA32_PG_PS) != 0) {
> - // 2MB page
> -
> - if (!IsAddressValid (Address, &Nx)) {
> - //
> - // Patch to remove Present flag and RW flag
> - //
> - *Pd = *Pd & (INTN)(INT32)(~PAGE_ATTRIBUTE_BITS);
> - }
> -
> - if (Nx && mXdSupported) {
> - *Pd = *Pd | IA32_PG_NX;
> - }
> - } else {
> - // 4KB page
> - Pt = (UINT64 *)(UINTN)(*Pd & ~mAddressEncMask &
> PHYSICAL_ADDRESS_MASK);
> - if (Pt == 0) {
> - continue;
> - }
> -
> - for (PtIndex = 0; PtIndex < SIZE_4KB / sizeof (*Pt); PtIndex++, Pt++) {
> - if (!IsAddressValid (Address, &Nx)) {
> - *Pt = *Pt & (INTN)(INT32)(~PAGE_ATTRIBUTE_BITS);
> - }
> -
> - if (Nx && mXdSupported) {
> - *Pt = *Pt | IA32_PG_NX;
> - }
> -
> - Address += SIZE_4KB;
> - } // end for PT
> - } // end if PS
> - } // end for PD
> - } // end for PDPT
> - } // end for PML4
> - } // end for PML5
> + if (PreviousAddress < Limit) {
> + //
> + // Set the last remaining range to EFI_MEMORY_RP/EFI_MEMORY_XP.
> + // This path applies to both SmmProfile enable/disable case.
> + //
> + Status = ConvertMemoryPageAttributes (PageTable, mPagingMode,
> PreviousAddress, Limit - PreviousAddress, MemoryAttrMask, TRUE, NULL);
> + ASSERT_RETURN_ERROR (Status);
> + }
>
> EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
>
> --
> 2.31.1.windows.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Patch V5 08/14] UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h
2023-06-08 2:27 ` [Patch V5 08/14] UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h duntan
@ 2023-06-08 10:21 ` Ni, Ray
0 siblings, 0 replies; 28+ messages in thread
From: Ni, Ray @ 2023-06-08 10:21 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V5 08/14] UefiCpuPkg: Extern mSmmShadowStackSize in
> PiSmmCpuDxeSmm.h
>
> Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h and remove
> extern for mSmmShadowStackSize in c files to simplify 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/SmmFuncsArch.c | 3 +--
> UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 2 --
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 1 +
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 2 --
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 3 +--
> 5 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
> index 6c48a53f67..636dc8d92f 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
> @@ -1,7 +1,7 @@
> /** @file
> SMM CPU misc functions for Ia32 arch specific.
>
> -Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -14,7 +14,6 @@ EFI_PHYSICAL_ADDRESS mGdtBuffer;
> UINTN mGdtBufferSize;
>
> extern BOOLEAN mCetSupported;
> -extern UINTN mSmmShadowStackSize;
>
> X86_ASSEMBLY_PATCH_LABEL mPatchCetPl0Ssp;
> X86_ASSEMBLY_PATCH_LABEL mPatchCetInterruptSsp;
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index baf827cf9d..1878252eac 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -29,8 +29,6 @@ MM_COMPLETION mSmmStartupThisApToken;
> //
> UINT32 *mPackageFirstThreadIndex = NULL;
>
> -extern UINTN mSmmShadowStackSize;
> -
> /**
> Performs an atomic compare exchange operation to get semaphore.
> The compare exchange operation must be performed using
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index e0c4ca76dc..a7da9673a5 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -262,6 +262,7 @@ extern EFI_SMM_CPU_PROTOCOL mSmmCpu;
> extern EFI_MM_MP_PROTOCOL mSmmMp;
> extern BOOLEAN m5LevelPagingNeeded;
> extern PAGING_MODE mPagingMode;
> +extern UINTN mSmmShadowStackSize;
>
> ///
> /// The mode of the CPU at the time an SMI occurs
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index 0bed857cae..46d8ff5d4e 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -13,8 +13,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #define PAGE_TABLE_PAGES 8
> #define ACC_MAX_BIT BIT3
>
> -extern UINTN mSmmShadowStackSize;
> -
> LIST_ENTRY mPagePool = INITIALIZE_LIST_HEAD_VARIABLE
> (mPagePool);
> BOOLEAN m1GPageTableSupport = FALSE;
> BOOLEAN mCpuSmmRestrictedMemoryAccess;
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> index 00a284c369..c4f21e2155 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> @@ -1,7 +1,7 @@
> /** @file
> SMM CPU misc functions for x64 arch specific.
>
> -Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -12,7 +12,6 @@ EFI_PHYSICAL_ADDRESS mGdtBuffer;
> UINTN mGdtBufferSize;
>
> extern BOOLEAN mCetSupported;
> -extern UINTN mSmmShadowStackSize;
>
> X86_ASSEMBLY_PATCH_LABEL mPatchCetPl0Ssp;
> X86_ASSEMBLY_PATCH_LABEL mPatchCetInterruptSsp;
> --
> 2.31.1.windows.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Patch V5 03/14] UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute.
2023-06-08 2:27 ` [Patch V5 03/14] UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute duntan
@ 2023-06-08 10:24 ` Ni, Ray
0 siblings, 0 replies; 28+ messages in thread
From: Ni, Ray @ 2023-06-08 10:24 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V5 03/14] UefiCpuPkg: Use CpuPageTableLib to convert SMM
> paging attribute.
>
> 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 | 409
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++++++++++---------------------------
> ----------------------------------------------------------------------------------------------
> ----------------------------------------------------------------------------------------------
> ----------------------------------------------------------------------------------------------
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 7 ++++++-
> 5 files changed, 122 insertions(+), 326 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..12723e5750 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,89 @@ ConvertMemoryPageAttributes (
> return RETURN_UNSUPPORTED;
> }
>
> - // DEBUG ((DEBUG_ERROR, "ConvertMemoryPageAttributes(%x) -
> %016lx, %016lx, %02lx\n", IsSet, BaseAddress, Length, Attributes));
> -
> - if (IsSplitted != NULL) {
> - *IsSplitted = FALSE;
> - }
> -
> if (IsModified != NULL) {
> *IsModified = FALSE;
> }
>
> - //
> - // 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;
> - }
> + PagingAttribute.Uint64 = 0;
> + PagingAttribute.Uint64 = mAddressEncMask | BaseAddress;
> + PagingAttrMask.Uint64 = 0;
>
> - 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_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;
> + }
> + }
>
> + if ((Attributes & EFI_MEMORY_XP) != 0) {
> + if (mXdSupported) {
> + PagingAttribute.Bits.Nx = IsSet ? 1 : 0;
> + PagingAttrMask.Bits.Nx = 1;
> + }
> + }
> +
> + 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;
> - }
> + //
> + // When map range to present range, provide all attributes.
> + //
> + PagingAttribute.Bits.Present = 1;
> + PagingAttrMask.Uint64 = MAX_UINT64;
>
> //
> - // Just split current page
> - // Convert success in next around
> + // By default memory is Ring 3 accessble.
> //
> + PagingAttribute.Bits.UserSupervisor = 1;
> }
> }
>
> + 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: Only
> change EFI_MEMORY_XP/EFI_MEMORY_RO for non-present range in [0x%lx,
> 0x%lx] is not permitted\n", BaseAddress, BaseAddress + Length));
> + }
> +
> + ASSERT_RETURN_ERROR (Status);
> + ASSERT (PageTableBufferSize == 0);
> +
> return RETURN_SUCCESS;
> }
>
> @@ -697,11 +500,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 +522,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 +549,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 +571,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 +622,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 +657,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 +682,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 +706,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 +1590,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..0bed857cae 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
> @@ -354,6 +354,11 @@ SmmInitPageTable (
> m5LevelPagingNeeded = Is5LevelPagingNeeded ();
> mPhysicalAddressBits = CalculateMaximumSupportAddress ();
> PatchInstructionX86 (gPatch5LevelPagingNeeded, m5LevelPagingNeeded,
> 1);
> + if (m5LevelPagingNeeded) {
> + mPagingMode = m1GPageTableSupport ? Paging5Level1GB : Paging5Level;
> + } 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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [edk2-devel] [Patch V5 05/14] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX
2023-06-08 2:27 ` [Patch V5 05/14] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX duntan
@ 2023-06-08 10:32 ` Ni, Ray
0 siblings, 0 replies; 28+ messages in thread
From: Ni, Ray @ 2023-06-08 10:32 UTC (permalink / raw)
To: devel@edk2.groups.io, Tan, Dun; +Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [edk2-devel] [Patch V5 05/14] UefiCpuPkg/PiSmmCpuDxeSmm:
> Avoid setting non-present range to RO/NX
>
> In PiSmmCpuDxeSmm code, SetMemMapAttributes() marks memory ranges
> in SmmMemoryAttributesTable to RO/NX. There may exist non-present
> range in these memory ranges. Set other attributes for a non-present
> range is not permitted in CpuPageTableMapLib. So add code to handle
> this case. Only map the present ranges in SmmMemoryAttributesTable
> to RO or NX.
>
> 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/SmmCpuMemoryManagement.c | 129
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++---------------
> -------
> 1 file changed, 107 insertions(+), 22 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 862b3e9720..3c79927c7b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -918,6 +918,70 @@ PatchGdtIdtMap (
> );
> }
>
> +/**
> + This function set [Base, Limit] to the input MemoryAttribute.
> +
> + @param Base Start address of range.
> + @param Limit Limit address of range.
> + @param Attribute The bit mask of attributes to modify for the memory
> region.
> + @param Map Pointer to the array of Cr3 IA32_MAP_ENTRY.
> + @param Count Count of IA32_MAP_ENTRY in Map.
> +**/
> +VOID
> +SetMemMapWithNonPresentRange (
> + UINT64 Base,
> + UINT64 Limit,
> + UINT64 Attribute,
> + IA32_MAP_ENTRY *Map,
> + UINTN Count
> + )
> +{
> + UINTN Index;
> + UINT64 NonPresentRangeStart;
> +
> + NonPresentRangeStart = 0;
> + for (Index = 0; Index < Count; Index++) {
> + if ((Map[Index].LinearAddress > NonPresentRangeStart) &&
> + (Base < Map[Index].LinearAddress) && (Limit > NonPresentRangeStart))
> + {
> + //
> + // We should NOT set attributes for non-present ragne.
> + //
> + //
> + // There is a non-present ( [NonPresentStart,
> Map[Index].LinearAddress] ) range before current Map[Index]
> + // and it is overlapped with [Base, Limit].
> + //
> + if (Base < NonPresentRangeStart) {
> + SmmSetMemoryAttributes (
> + Base,
> + NonPresentRangeStart - Base,
> + Attribute
> + );
> + }
> +
> + Base = Map[Index].LinearAddress;
> + }
> +
> + NonPresentRangeStart = Map[Index].LinearAddress + Map[Index].Length;
> + if (NonPresentRangeStart >= Limit) {
> + break;
> + }
> + }
> +
> + Limit = MIN (NonPresentRangeStart, Limit);
> +
> + if (Base < Limit) {
> + //
> + // There is no non-present range in current [Base, Limit] anymore.
> + //
> + SmmSetMemoryAttributes (
> + Base,
> + Limit - Base,
> + Attribute
> + );
> + }
> +}
> +
> /**
> This function sets memory attribute according to MemoryAttributesTable.
> **/
> @@ -932,6 +996,11 @@ SetMemMapAttributes (
> UINTN DescriptorSize;
> UINTN Index;
> EDKII_PI_SMM_MEMORY_ATTRIBUTES_TABLE *MemoryAttributesTable;
> + UINTN PageTable;
> + EFI_STATUS Status;
> + IA32_MAP_ENTRY *Map;
> + UINTN Count;
> + UINT64 MemoryAttribute;
>
> SmmGetSystemConfigurationTable
> (&gEdkiiPiSmmMemoryAttributesTableGuid, (VOID
> **)&MemoryAttributesTable);
> if (MemoryAttributesTable == NULL) {
> @@ -958,36 +1027,52 @@ SetMemMapAttributes (
> MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap,
> DescriptorSize);
> }
>
> + Count = 0;
> + Map = NULL;
> + PageTable = AsmReadCr3 ();
> + Status = PageTableParse (PageTable, mPagingMode, NULL, &Count);
> + while (Status == RETURN_BUFFER_TOO_SMALL) {
> + if (Map != NULL) {
> + FreePool (Map);
> + }
> +
> + Map = AllocatePool (Count * sizeof (IA32_MAP_ENTRY));
> + ASSERT (Map != NULL);
> + Status = PageTableParse (PageTable, mPagingMode, Map, &Count);
> + }
> +
> + ASSERT_RETURN_ERROR (Status);
> +
> MemoryMap = MemoryMapStart;
> for (Index = 0; Index < MemoryMapEntryCount; Index++) {
> DEBUG ((DEBUG_VERBOSE, "SetAttribute: Memory Entry - 0x%lx, 0x%x\n",
> MemoryMap->PhysicalStart, MemoryMap->NumberOfPages));
> - switch (MemoryMap->Type) {
> - case EfiRuntimeServicesCode:
> - SmmSetMemoryAttributes (
> - MemoryMap->PhysicalStart,
> - EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
> - EFI_MEMORY_RO
> - );
> - break;
> - case EfiRuntimeServicesData:
> - SmmSetMemoryAttributes (
> - MemoryMap->PhysicalStart,
> - EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
> - EFI_MEMORY_XP
> - );
> - break;
> - default:
> - SmmSetMemoryAttributes (
> - MemoryMap->PhysicalStart,
> - EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
> - EFI_MEMORY_XP
> - );
> - break;
> + if (MemoryMap->Type == EfiRuntimeServicesCode) {
> + MemoryAttribute = EFI_MEMORY_RO;
> + } else {
> + ASSERT ((MemoryMap->Type == EfiRuntimeServicesData) ||
> (MemoryMap->Type == EfiConventionalMemory));
> + //
> + // Set other type memory as NX.
> + //
> + MemoryAttribute = EFI_MEMORY_XP;
> }
>
> + //
> + // There may exist non-present range overlaps with the MemoryMap
> range.
> + // Do not change other attributes of non-present range while still
> remaining it as non-present
> + //
> + SetMemMapWithNonPresentRange (
> + MemoryMap->PhysicalStart,
> + MemoryMap->PhysicalStart + EFI_PAGES_TO_SIZE
> ((UINTN)MemoryMap->NumberOfPages),
> + MemoryAttribute,
> + Map,
> + Count
> + );
> +
> MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap,
> DescriptorSize);
> }
>
> + FreePool (Map);
> +
> PatchSmmSaveStateMap ();
> PatchGdtIdtMap ();
>
> --
> 2.31.1.windows.1
>
>
>
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Patch V5 04/14] UefiCpuPkg: Add DEBUG_CODE for special case when clear RP
2023-06-08 2:27 ` [Patch V5 04/14] UefiCpuPkg: Add DEBUG_CODE for special case when clear RP duntan
@ 2023-06-08 10:33 ` Ni, Ray
0 siblings, 0 replies; 28+ messages in thread
From: Ni, Ray @ 2023-06-08 10:33 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V5 04/14] UefiCpuPkg: Add DEBUG_CODE for special case
> when clear RP
>
> In ConvertMemoryPageAttributes() function, when clear RP for a
> specific range [BaseAddress, BaseAddress + Length], it means to
> set the present bit to 1 and assign default value for other
> attributes in page table. The default attributes for the input
> specific range are NX disabled and ReadOnly. If there is existing
> present range in [BaseAddress, BaseAddress + Length] and the
> attributes are not NX disabled or not ReadOnly, then output the
> DEBUG message to indicate that the NX and ReadOnly attributes of
> the existing present range are modified in the function.
>
> 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/SmmCpuMemoryManagement.c | 48
> ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 12723e5750..862b3e9720 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -349,6 +349,8 @@ ConvertMemoryPageAttributes (
> IA32_MAP_ENTRY *Map;
> UINTN Count;
> UINTN Index;
> + UINT64 OverlappedRangeBase;
> + UINT64 OverlappedRangeLimit;
>
> ASSERT (Attributes != 0);
> ASSERT ((Attributes & ~EFI_MEMORY_ATTRIBUTE_MASK) == 0);
> @@ -430,6 +432,52 @@ ConvertMemoryPageAttributes (
> // By default memory is Ring 3 accessble.
> //
> PagingAttribute.Bits.UserSupervisor = 1;
> +
> + DEBUG_CODE_BEGIN ();
> + 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 or ReadOnly.
> + //
> + Count = 0;
> + Map = NULL;
> + Status = PageTableParse (PageTableBase, mPagingMode, NULL, &Count);
> +
> + while (Status == RETURN_BUFFER_TOO_SMALL) {
> + if (Map != NULL) {
> + FreePool (Map);
> + }
> +
> + 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 (Map[Index].LinearAddress >= BaseAddress + Length) {
> + break;
> + }
> +
> + if ((BaseAddress < Map[Index].LinearAddress + Map[Index].Length) &&
> (BaseAddress + Length > Map[Index].LinearAddress)) {
> + OverlappedRangeBase = MAX (BaseAddress,
> Map[Index].LinearAddress);
> + OverlappedRangeLimit = MIN (BaseAddress + Length,
> Map[Index].LinearAddress + Map[Index].Length);
> +
> + if (((Attributes & EFI_MEMORY_RO) == 0) &&
> (Map[Index].Attribute.Bits.ReadWrite == 1)) {
> + DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes:
> [0x%lx, 0x%lx] is set from ReadWrite to ReadOnly\n", OverlappedRangeBase,
> OverlappedRangeLimit));
> + }
> +
> + if (((Attributes & EFI_MEMORY_XP) == 0) && (mXdSupported) &&
> (Map[Index].Attribute.Bits.Nx == 1)) {
> + DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes:
> [0x%lx, 0x%lx] is set from NX enabled to NX disabled\n",
> OverlappedRangeBase, OverlappedRangeLimit));
> + }
> + }
> + }
> +
> + FreePool (Map);
> + }
> +
> + DEBUG_CODE_END ();
> }
> }
>
> --
> 2.31.1.windows.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry
2023-06-08 2:27 ` [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry duntan
@ 2023-06-08 10:33 ` Ni, Ray
2023-06-19 10:26 ` [edk2-devel] " Gerd Hoffmann
1 sibling, 0 replies; 28+ messages in thread
From: Ni, Ray @ 2023-06-08 10:33 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io
Cc: Ard Biesheuvel, Yao, Jiewen, Justen, Jordan L, Gerd Hoffmann,
Tom Lendacky
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, June 8, 2023 10:27 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen
> <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Gerd
> Hoffmann <kraxel@redhat.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask
> to non-leaf entry
>
> Remove code that apply AddressEncMask to non-leaf entry when split
> smm page table by MemEncryptSevLib. In FvbServicesSmm driver, it
> calls MemEncryptSevClearMmioPageEncMask to clear AddressEncMask
> bit in page table for a specific range. In AMD SEV feature, this
> AddressEncMask bit in page table is used to indicate if the memory
> is guest private memory or shared memory. But all memory used by
> page table are treated as encrypted regardless of encryption bit.
> So remove the EncMask bit for smm non-leaf page table entry
> doesn't impact AMD SEV feature.
> If page split happens in the AddressEncMask bit clear process,
> there will be some new non-leaf entries with AddressEncMask
> applied in smm page table. When ReadyToLock, code in PiSmmCpuDxe
> module will use CpuPageTableLib to modify smm page table. So
> remove code to apply AddressEncMask for new non-leaf entries
> since CpuPageTableLib doesn't consume the EncMask PCD.
>
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Ray Ni <ray.ni@intel.com>
> ---
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 6
> +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> index cf2441b551..aba2e8c081 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> @@ -233,7 +233,7 @@ Split2MPageTo4K (
> // Fill in 2M page entry.
> //
> *PageEntry2M = ((UINT64)(UINTN)PageTableEntry1 |
> - IA32_PG_P | IA32_PG_RW | AddressEncMask);
> + IA32_PG_P | IA32_PG_RW);
> }
>
> /**
> @@ -352,7 +352,7 @@ SetPageTablePoolReadOnly (
> PhysicalAddress += LevelSize[Level - 1];
> }
>
> - PageTable[Index] = (UINT64)(UINTN)NewPageTable | AddressEncMask |
> + PageTable[Index] = (UINT64)(UINTN)NewPageTable |
> IA32_PG_P | IA32_PG_RW;
> PageTable = NewPageTable;
> }
> @@ -440,7 +440,7 @@ Split1GPageTo2M (
> // Fill in 1G page entry.
> //
> *PageEntry1G = ((UINT64)(UINTN)PageDirectoryEntry |
> - IA32_PG_P | IA32_PG_RW | AddressEncMask);
> + IA32_PG_P | IA32_PG_RW);
>
> PhysicalAddress2M = PhysicalAddress;
> for (IndexOfPageDirectoryEntries = 0;
> --
> 2.31.1.windows.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [edk2-devel] [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page
2023-06-08 2:27 ` [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page duntan
2023-06-08 10:08 ` Ni, Ray
@ 2023-06-08 12:18 ` Ard Biesheuvel
2023-06-09 9:10 ` duntan
1 sibling, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2023-06-08 12:18 UTC (permalink / raw)
To: devel, dun.tan; +Cc: Liming Gao, Ray Ni, Jian J Wang
On Thu, 8 Jun 2023 at 04:28, duntan <dun.tan@intel.com> wrote:
>
> Remove RO and NX protection when unset guard page.
> When UnsetGuardPage(), remove all the memory attribute protection
> for guarded page.
>
Why is it acceptable to remove NX protections here?
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> ---
> MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> index 8f3bab6fee..7daeeccf13 100644
> --- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> +++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> @@ -553,7 +553,7 @@ UnsetGuardPage (
> mSmmMemoryAttribute,
> BaseAddress,
> EFI_PAGE_SIZE,
> - EFI_MEMORY_RP
> + EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMORY_XP
> );
> ASSERT_EFI_ERROR (Status);
> mOnGuarding = FALSE;
> --
> 2.31.1.windows.1
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [edk2-devel] [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page
2023-06-08 12:18 ` [edk2-devel] " Ard Biesheuvel
@ 2023-06-09 9:10 ` duntan
0 siblings, 0 replies; 28+ messages in thread
From: duntan @ 2023-06-09 9:10 UTC (permalink / raw)
To: devel@edk2.groups.io, ardb@kernel.org; +Cc: Gao, Liming, Ni, Ray, Wang, Jian J
Hi Ard,
Thanks for your question. This patch does cause a difference that NX protections maybe removed for some EfiConventionalMemory in SMRAM after SmmReadyToLock.
Before SmmReadyToLock, EfiConventionalMemory in SMRAM is always RW and executable.
When SmmReadyToLock, SetMemMapAttributes() in PiSmmCpuDxe driver applies EFI_MEMORY_XP for EfiConventionalMemory in SMRAM.
With this patch, after SmmReadyToLock, if AllocatePage() and FreePage() is called and HeapGuard is enabled for smm, the guarded page(when ungarded) is marked as executable.
To solve this issue, I'll add code to apply EFI_MEMORY_XP to the guarded page to be freed in UnsetGuardPage() if it happens after SmmReadyToLock. Will send the V6 patch.
Thanks,
Dun
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
Sent: Thursday, June 8, 2023 8:18 PM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
Subject: Re: [edk2-devel] [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page
On Thu, 8 Jun 2023 at 04:28, duntan <dun.tan@intel.com> wrote:
>
> Remove RO and NX protection when unset guard page.
> When UnsetGuardPage(), remove all the memory attribute protection for
> guarded page.
>
Why is it acceptable to remove NX protections here?
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> ---
> MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> index 8f3bab6fee..7daeeccf13 100644
> --- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> +++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> @@ -553,7 +553,7 @@ UnsetGuardPage (
> mSmmMemoryAttribute,
> BaseAddress,
> EFI_PAGE_SIZE,
> - EFI_MEMORY_RP
> +
> + EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMORY_XP
> );
> ASSERT_EFI_ERROR (Status);
> mOnGuarding = FALSE;
> --
> 2.31.1.windows.1
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [edk2-devel] [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry
2023-06-08 2:27 ` [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry duntan
2023-06-08 10:33 ` Ni, Ray
@ 2023-06-19 10:26 ` Gerd Hoffmann
1 sibling, 0 replies; 28+ messages in thread
From: Gerd Hoffmann @ 2023-06-19 10:26 UTC (permalink / raw)
To: devel, dun.tan
Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Tom Lendacky, Ray Ni
On Thu, Jun 08, 2023 at 10:27:29AM +0800, duntan wrote:
> Remove code that apply AddressEncMask to non-leaf entry when split
> smm page table by MemEncryptSevLib. In FvbServicesSmm driver, it
> calls MemEncryptSevClearMmioPageEncMask to clear AddressEncMask
> bit in page table for a specific range. In AMD SEV feature, this
> AddressEncMask bit in page table is used to indicate if the memory
> is guest private memory or shared memory. But all memory used by
> page table are treated as encrypted regardless of encryption bit.
> So remove the EncMask bit for smm non-leaf page table entry
> doesn't impact AMD SEV feature.
> If page split happens in the AddressEncMask bit clear process,
> there will be some new non-leaf entries with AddressEncMask
> applied in smm page table. When ReadyToLock, code in PiSmmCpuDxe
> module will use CpuPageTableLib to modify smm page table. So
> remove code to apply AddressEncMask for new non-leaf entries
> since CpuPageTableLib doesn't consume the EncMask PCD.
>
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Ray Ni <ray.ni@intel.com>
Whole series:
Tested-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2023-06-19 10:26 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-08 2:27 [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table duntan
2023-06-08 2:27 ` [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry duntan
2023-06-08 10:33 ` Ni, Ray
2023-06-19 10:26 ` [edk2-devel] " Gerd Hoffmann
2023-06-08 2:27 ` [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page duntan
2023-06-08 10:08 ` Ni, Ray
2023-06-08 12:18 ` [edk2-devel] " Ard Biesheuvel
2023-06-09 9:10 ` duntan
2023-06-08 2:27 ` [Patch V5 03/14] UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute duntan
2023-06-08 10:24 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 04/14] UefiCpuPkg: Add DEBUG_CODE for special case when clear RP duntan
2023-06-08 10:33 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 05/14] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX duntan
2023-06-08 10:32 ` [edk2-devel] " Ni, Ray
2023-06-08 2:27 ` [Patch V5 06/14] UefiCpuPkg/PiSmmCpuDxeSmm: Add 2 function to disable/enable CR0.WP duntan
2023-06-08 2:27 ` [Patch V5 07/14] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR0.WP before modify page table duntan
2023-06-08 2:27 ` [Patch V5 08/14] UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h duntan
2023-06-08 10:21 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 09/14] UefiCpuPkg: Add GenSmmPageTable() to create smm page table duntan
2023-06-08 10:16 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 10/14] UefiCpuPkg: Use GenSmmPageTable() to create Smm S3 " duntan
2023-06-08 2:27 ` [Patch V5 11/14] UefiCpuPkg: Sort mSmmCpuSmramRanges in FindSmramInfo duntan
2023-06-08 10:16 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 12/14] UefiCpuPkg: Sort mProtectionMemRange when ReadyToLock duntan
2023-06-08 10:17 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 13/14] UefiCpuPkg: Refinement to smm runtime InitPaging() code duntan
2023-06-08 10:18 ` Ni, Ray
2023-06-08 2:27 ` [Patch V5 14/14] UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function duntan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox