* [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/CpuPageTableLib: Init local variable before using it.
@ 2023-11-30 6:29 Zhiguang Liu
2023-11-30 6:29 ` [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/CpuPageTableLib/TestCase: Refine test case for PAE paging Zhiguang Liu
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Zhiguang Liu @ 2023-11-30 6:29 UTC (permalink / raw)
To: devel; +Cc: Zhiguang Liu, Ray Ni, Rahul Kumar, Gerd Hoffmann, Laszlo Ersek
The local variable OneOfPagingEntry is used before initialized, this
may cause reserved bit in page table entry is set especially in PAE
paging mode. The bug is random because it depends on the value in
stack.
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
index eff02619fa..36b2c4e6a3 100644
--- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
+++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
@@ -338,7 +338,7 @@ PageTableLibMapInLevel (
ParentAttribute = &LocalParentAttribute;
OriginalParentPagingEntry.Uint64 = ParentPagingEntry->Uint64;
-
+ OneOfPagingEntry.Uint64 = 0;
//
// RegionLength: 256T (1 << 48) 512G (1 << 39), 1G (1 << 30), 2M (1 << 21) or 4K (1 << 12).
//
@@ -367,8 +367,6 @@ PageTableLibMapInLevel (
if (RETURN_ERROR (Status)) {
return Status;
}
-
- OneOfPagingEntry.Pnle.Uint64 = 0;
} else {
PageTableLibSetPle (Level, &OneOfPagingEntry, 0, &PleBAttribute, &AllOneMask);
}
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111889): https://edk2.groups.io/g/devel/message/111889
Mute This Topic: https://groups.io/mt/102889278/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/CpuPageTableLib/TestCase: Refine test case for PAE paging.
2023-11-30 6:29 [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/CpuPageTableLib: Init local variable before using it Zhiguang Liu
@ 2023-11-30 6:29 ` Zhiguang Liu
2023-12-01 8:41 ` Ni, Ray
2023-11-30 6:29 ` [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/CpuMpPei: Use CpuPageTableLib to set memory attribute Zhiguang Liu
2023-12-01 8:40 ` [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/CpuPageTableLib: Init local variable before using it Ni, Ray
2 siblings, 1 reply; 7+ messages in thread
From: Zhiguang Liu @ 2023-11-30 6:29 UTC (permalink / raw)
To: devel; +Cc: Zhiguang Liu, Ray Ni, Rahul Kumar, Gerd Hoffmann, Laszlo Ersek
Refine test case:
1. Check PAE paging reserved bits is zero.
2. Set stack as random value.
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
.../CpuPageTableLib/UnitTest/RandomTest.c | 24 ++++++++++++++++++-
.../CpuPageTableLib/UnitTest/TestHelper.c | 14 ++++++++---
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
index f7a77d00e7..9ac3188be0 100644
--- a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
+++ b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
@@ -138,6 +138,23 @@ RandomBoolean (
return ((Probability > ((UINT8)Random64 (0, 100))) ? TRUE : FALSE);
}
+/**
+ Set 8K stack as random value.
+**/
+VOID
+SetRandomStack (
+ VOID
+ )
+{
+ UINT64 Buffer[SIZE_1KB];
+ UINTN Index;
+
+ for (Index = 0; Index < SIZE_1KB; Index++) {
+ Buffer[Index] = Random64 (0, MAX_UINT64);
+ Buffer[Index] = Buffer[Index];
+ }
+}
+
/**
Check if the Page table entry is valid
@@ -670,6 +687,7 @@ SingleMapEntryTest (
IsNotPresent = FALSE;
IsModified = FALSE;
+ SetRandomStack ();
GenerateSingleRandomMapEntry (MaxAddress, MapEntrys);
LastMapEntry = &MapEntrys->Maps[MapsIndex];
Status = PageTableParse (*PageTable, PagingMode, NULL, &MapCount);
@@ -1039,7 +1057,11 @@ TestCaseforRandomTest (
mSupportedBit.Bits.Pat = 1;
mSupportedBit.Bits.Global = 1;
mSupportedBit.Bits.ProtectionKey = 0xF;
- mSupportedBit.Bits.Nx = 1;
+ if (((CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT *)Context)->PagingMode == PagingPae) {
+ mSupportedBit.Bits.ProtectionKey = 0;
+ }
+
+ mSupportedBit.Bits.Nx = 1;
mRandomOption = ((CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT *)Context)->RandomOption;
mNumberIndex = 0;
diff --git a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
index 67776255c2..d2c50a6c8a 100644
--- a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
+++ b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
@@ -9,6 +9,7 @@
#include "CpuPageTableLibUnitTest.h"
#include "../CpuPageTable.h"
+#define IA32_PAE_RESERVED_MASK 0x7FF0000000000000ull
//
// Global Data to validate if the page table is legal
// mValidMaskNoLeaf[0] is not used
@@ -95,6 +96,7 @@ InitGlobalData (
@param[in] Level the level of PagingEntry.
@param[in] MaxLeafLevel Max leaf entry level.
@param[in] LinearAddress The linear address verified.
+ @param[in] PagingMode The paging mode.
@retval Leaf entry.
**/
@@ -103,13 +105,18 @@ IsPageTableEntryValid (
IN IA32_PAGING_ENTRY *PagingEntry,
IN UINTN Level,
IN UINTN MaxLeafLevel,
- IN UINT64 Address
+ IN UINT64 Address,
+ IN PAGING_MODE PagingMode
)
{
UINT64 Index;
IA32_PAGING_ENTRY *ChildPageEntry;
UNIT_TEST_STATUS Status;
+ if (PagingMode == PagingPae) {
+ UT_ASSERT_EQUAL (PagingEntry->Uint64 & IA32_PAE_RESERVED_MASK, 0);
+ }
+
if (PagingEntry->Pce.Present == 0) {
return UNIT_TEST_PASSED;
}
@@ -142,7 +149,7 @@ IsPageTableEntryValid (
ChildPageEntry = (IA32_PAGING_ENTRY *)(UINTN)(IA32_PNLE_PAGE_TABLE_BASE_ADDRESS (&PagingEntry->Pnle));
for (Index = 0; Index < 512; Index++) {
- Status = IsPageTableEntryValid (&ChildPageEntry[Index], Level-1, MaxLeafLevel, Address + (Index<<(9*(Level-1) + 3)));
+ Status = IsPageTableEntryValid (&ChildPageEntry[Index], Level-1, MaxLeafLevel, Address + (Index<<(9*(Level-1) + 3)), PagingMode);
if (Status != UNIT_TEST_PASSED) {
return Status;
}
@@ -190,9 +197,10 @@ IsPageTableValid (
if (PagingMode == PagingPae) {
UT_ASSERT_EQUAL (PagingEntry[Index].PdptePae.Bits.MustBeZero, 0);
UT_ASSERT_EQUAL (PagingEntry[Index].PdptePae.Bits.MustBeZero2, 0);
+ UT_ASSERT_EQUAL (PagingEntry[Index].PdptePae.Bits.MustBeZero3, 0);
}
- Status = IsPageTableEntryValid (&PagingEntry[Index], MaxLevel, MaxLeafLevel, Index << (9 * MaxLevel + 3));
+ Status = IsPageTableEntryValid (&PagingEntry[Index], MaxLevel, MaxLeafLevel, Index << (9 * MaxLevel + 3), PagingMode);
if (Status != UNIT_TEST_PASSED) {
return Status;
}
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111890): https://edk2.groups.io/g/devel/message/111890
Mute This Topic: https://groups.io/mt/102889279/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/CpuMpPei: Use CpuPageTableLib to set memory attribute.
2023-11-30 6:29 [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/CpuPageTableLib: Init local variable before using it Zhiguang Liu
2023-11-30 6:29 ` [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/CpuPageTableLib/TestCase: Refine test case for PAE paging Zhiguang Liu
@ 2023-11-30 6:29 ` Zhiguang Liu
2023-12-01 8:42 ` Ni, Ray
2023-12-01 8:40 ` [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/CpuPageTableLib: Init local variable before using it Ni, Ray
2 siblings, 1 reply; 7+ messages in thread
From: Zhiguang Liu @ 2023-11-30 6:29 UTC (permalink / raw)
To: devel; +Cc: Zhiguang Liu, Ray Ni, Rahul Kumar, Gerd Hoffmann, Laszlo Ersek
Currently, there are code to set memory attribute in CpuMpPei module.
However, the code doesn't handle the case of 5 level paging.
Use the CpuPageTableLib to set memory attribute for two purpose:
1. Add 5 level paging support
2. Clean up code
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
UefiCpuPkg/CpuMpPei/CpuPaging.c | 317 +++++++-------------------------
1 file changed, 69 insertions(+), 248 deletions(-)
diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
index b7ddb0005b..2dd7237141 100644
--- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
+++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
@@ -15,50 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Guid/MigratedFvInfo.h>
#include "CpuMpPei.h"
-
-#define IA32_PG_P BIT0
-#define IA32_PG_RW BIT1
-#define IA32_PG_U BIT2
-#define IA32_PG_A BIT5
-#define IA32_PG_D BIT6
-#define IA32_PG_PS BIT7
-#define IA32_PG_NX BIT63
-
-#define PAGE_ATTRIBUTE_BITS (IA32_PG_RW | IA32_PG_P)
-#define PAGE_PROGATE_BITS (IA32_PG_D | IA32_PG_A | IA32_PG_NX | IA32_PG_U | \
- PAGE_ATTRIBUTE_BITS)
-
-#define PAGING_PAE_INDEX_MASK 0x1FF
-#define PAGING_4K_ADDRESS_MASK_64 0x000FFFFFFFFFF000ull
-#define PAGING_2M_ADDRESS_MASK_64 0x000FFFFFFFE00000ull
-#define PAGING_1G_ADDRESS_MASK_64 0x000FFFFFC0000000ull
-#define PAGING_512G_ADDRESS_MASK_64 0x000FFF8000000000ull
-
-typedef enum {
- PageNone = 0,
- PageMin = 1,
- Page4K = PageMin,
- Page2M = 2,
- Page1G = 3,
- Page512G = 4,
- PageMax = Page512G
-} PAGE_ATTRIBUTE;
-
-typedef struct {
- PAGE_ATTRIBUTE Attribute;
- UINT64 Length;
- UINT64 AddressMask;
- UINTN AddressBitOffset;
- UINTN AddressBitLength;
-} PAGE_ATTRIBUTE_TABLE;
-
-PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = {
- { PageNone, 0, 0, 0, 0 },
- { Page4K, SIZE_4KB, PAGING_4K_ADDRESS_MASK_64, 12, 9 },
- { Page2M, SIZE_2MB, PAGING_2M_ADDRESS_MASK_64, 21, 9 },
- { Page1G, SIZE_1GB, PAGING_1G_ADDRESS_MASK_64, 30, 9 },
- { Page512G, SIZE_512GB, PAGING_512G_ADDRESS_MASK_64, 39, 9 },
-};
+#define PAGING_4K_ADDRESS_MASK_64 0x000FFFFFFFFFF000ull
EFI_PEI_NOTIFY_DESCRIPTOR mPostMemNotifyList[] = {
{
@@ -117,237 +74,101 @@ AllocatePageTableMemory (
return Address;
}
-/**
- Get the type of top level page table.
-
- @retval Page512G PML4 paging.
- @retval Page1G PAE paging.
-
-**/
-PAGE_ATTRIBUTE
-GetPageTableTopLevelType (
- VOID
- )
-{
- MSR_IA32_EFER_REGISTER MsrEfer;
-
- MsrEfer.Uint64 = AsmReadMsr64 (MSR_CORE_IA32_EFER);
-
- return (MsrEfer.Bits.LMA == 1) ? Page512G : Page1G;
-}
-
-/**
- Return page table entry matching the address.
-
- @param[in] Address The address to be checked.
- @param[out] PageAttributes The page attribute of the page entry.
-
- @return The page entry.
-**/
-VOID *
-GetPageTableEntry (
- IN PHYSICAL_ADDRESS Address,
- OUT PAGE_ATTRIBUTE *PageAttribute
- )
-{
- INTN Level;
- UINTN Index;
- UINT64 *PageTable;
- UINT64 AddressEncMask;
-
- AddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask);
- PageTable = (UINT64 *)(UINTN)(AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64);
- for (Level = (INTN)GetPageTableTopLevelType (); Level > 0; --Level) {
- Index = (UINTN)RShiftU64 (Address, mPageAttributeTable[Level].AddressBitOffset);
- Index &= PAGING_PAE_INDEX_MASK;
-
- //
- // No mapping?
- //
- if (PageTable[Index] == 0) {
- *PageAttribute = PageNone;
- return NULL;
- }
-
- //
- // Page memory?
- //
- if (((PageTable[Index] & IA32_PG_PS) != 0) || (Level == PageMin)) {
- *PageAttribute = (PAGE_ATTRIBUTE)Level;
- return &PageTable[Index];
- }
-
- //
- // Page directory or table
- //
- PageTable = (UINT64 *)(UINTN)(PageTable[Index] &
- ~AddressEncMask &
- PAGING_4K_ADDRESS_MASK_64);
- }
-
- *PageAttribute = PageNone;
- return NULL;
-}
-
-/**
- This function splits one page entry to smaller 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.
- @param[in] Recursively Do the split recursively or not.
-
- @retval RETURN_SUCCESS The page entry is splitted.
- @retval RETURN_INVALID_PARAMETER If target page attribute is invalid
- @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,
- IN BOOLEAN Recursively
- )
-{
- UINT64 BaseAddress;
- UINT64 *NewPageEntry;
- UINTN Index;
- UINT64 AddressEncMask;
- PAGE_ATTRIBUTE SplitTo;
-
- if ((SplitAttribute == PageNone) || (SplitAttribute >= PageAttribute)) {
- ASSERT (SplitAttribute != PageNone);
- ASSERT (SplitAttribute < PageAttribute);
- return RETURN_INVALID_PARAMETER;
- }
-
- NewPageEntry = AllocatePageTableMemory (1);
- if (NewPageEntry == NULL) {
- ASSERT (NewPageEntry != NULL);
- return RETURN_OUT_OF_RESOURCES;
- }
-
- //
- // One level down each step to achieve more compact page table.
- //
- SplitTo = PageAttribute - 1;
- AddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask) &
- mPageAttributeTable[SplitTo].AddressMask;
- BaseAddress = *PageEntry &
- ~PcdGet64 (PcdPteMemoryEncryptionAddressOrMask) &
- mPageAttributeTable[PageAttribute].AddressMask;
- for (Index = 0; Index < SIZE_4KB / sizeof (UINT64); Index++) {
- NewPageEntry[Index] = BaseAddress | AddressEncMask |
- ((*PageEntry) & PAGE_PROGATE_BITS);
-
- if (SplitTo != PageMin) {
- NewPageEntry[Index] |= IA32_PG_PS;
- }
-
- if (Recursively && (SplitTo > SplitAttribute)) {
- SplitPage (&NewPageEntry[Index], SplitTo, SplitAttribute, Recursively);
- }
-
- BaseAddress += mPageAttributeTable[SplitTo].Length;
- }
-
- (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask | PAGE_ATTRIBUTE_BITS;
-
- return RETURN_SUCCESS;
-}
-
/**
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.
+ by BaseAddress and Length to not present.
Caller should make sure BaseAddress and Length is at page boundary.
@param[in] BaseAddress Start address of a memory region.
@param[in] Length Size in bytes of the memory region.
- @param[in] Attributes Bit mask of attributes to modify.
-
- @retval RETURN_SUCCESS The attributes were modified for the memory
- region.
- @retval RETURN_INVALID_PARAMETER Length is zero; or,
- Attributes specified an illegal combination
- of attributes that cannot be set together; or
- Addressis not 4KB aligned.
+
+ @retval RETURN_SUCCESS The memory region is changed to not present.
@retval RETURN_OUT_OF_RESOURCES There are not enough system resources to modify
the attributes.
@retval RETURN_UNSUPPORTED Cannot modify the attributes of given memory.
**/
RETURN_STATUS
-EFIAPI
-ConvertMemoryPageAttributes (
+ConvertMemoryPageToNotPresent (
IN PHYSICAL_ADDRESS BaseAddress,
- IN UINT64 Length,
- IN UINT64 Attributes
+ IN UINT64 Length
)
{
- UINT64 *PageEntry;
- PAGE_ATTRIBUTE PageAttribute;
- RETURN_STATUS Status;
- EFI_PHYSICAL_ADDRESS MaximumAddress;
-
- if ((Length == 0) ||
- ((BaseAddress & (SIZE_4KB - 1)) != 0) ||
- ((Length & (SIZE_4KB - 1)) != 0))
- {
- ASSERT (Length > 0);
- ASSERT ((BaseAddress & (SIZE_4KB - 1)) == 0);
- ASSERT ((Length & (SIZE_4KB - 1)) == 0);
-
- return RETURN_INVALID_PARAMETER;
- }
-
- MaximumAddress = (EFI_PHYSICAL_ADDRESS)MAX_UINT32;
- if ((BaseAddress > MaximumAddress) ||
- (Length > MaximumAddress) ||
- (BaseAddress > MaximumAddress - (Length - 1)))
- {
- return RETURN_UNSUPPORTED;
- }
-
- //
- // Below logic is to check 2M/4K page to make sure we do not waste memory.
- //
- while (Length != 0) {
- PageEntry = GetPageTableEntry (BaseAddress, &PageAttribute);
- if (PageEntry == NULL) {
- return RETURN_UNSUPPORTED;
- }
+ EFI_STATUS Status;
+ UINTN PageTable;
+ EFI_PHYSICAL_ADDRESS Buffer;
+ UINTN BufferSize;
+ IA32_MAP_ATTRIBUTE MapAttribute;
+ IA32_MAP_ATTRIBUTE MapMask;
+ PAGING_MODE PagingMode;
+ IA32_CR4 Cr4;
+ BOOLEAN Page5LevelSupport;
+ UINT32 RegEax;
+ BOOLEAN Page1GSupport;
+ CPUID_EXTENDED_CPU_SIG_EDX RegEdx;
+
+ if (sizeof (UINTN) == sizeof (UINT64)) {
+ //
+ // Check Page5Level Support or not.
+ //
+ Cr4.UintN = AsmReadCr4 ();
+ Page5LevelSupport = (Cr4.Bits.LA57 ? TRUE : FALSE);
- if (PageAttribute != Page4K) {
- Status = SplitPage (PageEntry, PageAttribute, Page4K, FALSE);
- if (RETURN_ERROR (Status)) {
- return Status;
+ //
+ // Check Page1G Support or not.
+ //
+ Page1GSupport = FALSE;
+ AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
+ if (RegEax >= CPUID_EXTENDED_CPU_SIG) {
+ AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx.Uint32);
+ if (RegEdx.Bits.Page1GB != 0) {
+ Page1GSupport = TRUE;
}
-
- //
- // Do it again until the page is 4K.
- //
- continue;
}
//
- // Just take care of 'present' bit for Stack Guard.
+ // Decide Paging Mode according Page5LevelSupport & Page1GSupport.
//
- if ((Attributes & IA32_PG_P) != 0) {
- *PageEntry |= (UINT64)IA32_PG_P;
+ if (Page5LevelSupport) {
+ PagingMode = Page1GSupport ? Paging5Level1GB : Paging5Level;
} else {
- *PageEntry &= ~((UINT64)IA32_PG_P);
+ PagingMode = Page1GSupport ? Paging4Level1GB : Paging4Level;
}
+ } else {
+ PagingMode = PagingPae;
+ }
+
+ MapAttribute.Uint64 = 0;
+ MapMask.Uint64 = 0;
+ MapMask.Bits.Present = 1;
+ PageTable = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;
+ BufferSize = 0;
+ //
+ // Get required buffer size for the pagetable that will be created.
+ //
+ Status = PageTableMap (&PageTable, PagingMode, 0, &BufferSize, BaseAddress, Length, &MapAttribute, &MapMask, NULL);
+ if (Status == EFI_BUFFER_TOO_SMALL) {
//
- // Convert success, move to next
+ // Allocate required Buffer.
//
- BaseAddress += SIZE_4KB;
- Length -= SIZE_4KB;
+ Status = PeiServicesAllocatePages (
+ EfiBootServicesData,
+ EFI_SIZE_TO_PAGES (BufferSize),
+ &Buffer
+ );
+ ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ Status = PageTableMap (&PageTable, PagingMode, (VOID *)(UINTN)Buffer, &BufferSize, BaseAddress, Length, &MapAttribute, &MapMask, NULL);
}
- return RETURN_SUCCESS;
+ ASSERT_EFI_ERROR (Status);
+ AsmWriteCr3 (PageTable);
+ return Status;
}
/**
@@ -516,7 +337,7 @@ SetupStackGuardPage (
//
// Set Guard page at stack base address.
//
- ConvertMemoryPageAttributes (StackBase, EFI_PAGE_SIZE, 0);
+ ConvertMemoryPageToNotPresent (StackBase, EFI_PAGE_SIZE);
DEBUG ((
DEBUG_INFO,
"Stack Guard set at %lx [cpu%lu]!\n",
@@ -599,7 +420,7 @@ MemoryDiscoveredPpiNotifyCallback (
// Enable #PF exception, so if the code access SPI after disable NEM, it will generate
// the exception to avoid potential vulnerability.
//
- ConvertMemoryPageAttributes (MigratedFvInfo->FvOrgBase, MigratedFvInfo->FvLength, 0);
+ ConvertMemoryPageToNotPresent (MigratedFvInfo->FvOrgBase, MigratedFvInfo->FvLength);
Hob.Raw = GET_NEXT_HOB (Hob);
Hob.Raw = GetNextGuidHob (&gEdkiiMigratedFvInfoGuid, Hob.Raw);
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111891): https://edk2.groups.io/g/devel/message/111891
Mute This Topic: https://groups.io/mt/102889280/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/CpuPageTableLib: Init local variable before using it.
2023-11-30 6:29 [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/CpuPageTableLib: Init local variable before using it Zhiguang Liu
2023-11-30 6:29 ` [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/CpuPageTableLib/TestCase: Refine test case for PAE paging Zhiguang Liu
2023-11-30 6:29 ` [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/CpuMpPei: Use CpuPageTableLib to set memory attribute Zhiguang Liu
@ 2023-12-01 8:40 ` Ni, Ray
2 siblings, 0 replies; 7+ messages in thread
From: Ni, Ray @ 2023-12-01 8:40 UTC (permalink / raw)
To: Liu, Zhiguang, devel@edk2.groups.io
Cc: Kumar, Rahul R, Gerd Hoffmann, Laszlo Ersek
Reviewed-by: Ray Ni <ray.ni@intel.com>
Thanks,
Ray
> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Thursday, November 30, 2023 2:29 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH v2 1/3] UefiCpuPkg/CpuPageTableLib: Init local variable
> before using it.
>
> The local variable OneOfPagingEntry is used before initialized, this
> may cause reserved bit in page table entry is set especially in PAE
> paging mode. The bug is random because it depends on the value in
> stack.
>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
> UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index eff02619fa..36b2c4e6a3 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -338,7 +338,7 @@ PageTableLibMapInLevel (
> ParentAttribute = &LocalParentAttribute;
>
> OriginalParentPagingEntry.Uint64 = ParentPagingEntry->Uint64;
> -
> + OneOfPagingEntry.Uint64 = 0;
> //
> // RegionLength: 256T (1 << 48) 512G (1 << 39), 1G (1 << 30), 2M (1 << 21)
> or 4K (1 << 12).
> //
> @@ -367,8 +367,6 @@ PageTableLibMapInLevel (
> if (RETURN_ERROR (Status)) {
> return Status;
> }
> -
> - OneOfPagingEntry.Pnle.Uint64 = 0;
> } else {
> PageTableLibSetPle (Level, &OneOfPagingEntry, 0, &PleBAttribute,
> &AllOneMask);
> }
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111977): https://edk2.groups.io/g/devel/message/111977
Mute This Topic: https://groups.io/mt/102889278/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/CpuPageTableLib/TestCase: Refine test case for PAE paging.
2023-11-30 6:29 ` [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/CpuPageTableLib/TestCase: Refine test case for PAE paging Zhiguang Liu
@ 2023-12-01 8:41 ` Ni, Ray
0 siblings, 0 replies; 7+ messages in thread
From: Ni, Ray @ 2023-12-01 8:41 UTC (permalink / raw)
To: Liu, Zhiguang, devel@edk2.groups.io
Cc: Kumar, Rahul R, Gerd Hoffmann, Laszlo Ersek
Reviewed-by: Ray Ni <ray.ni@intel.com>
Thanks,
Ray
> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Thursday, November 30, 2023 2:29 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH v2 2/3] UefiCpuPkg/CpuPageTableLib/TestCase: Refine test
> case for PAE paging.
>
> Refine test case:
> 1. Check PAE paging reserved bits is zero.
> 2. Set stack as random value.
>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
> .../CpuPageTableLib/UnitTest/RandomTest.c | 24 ++++++++++++++++++-
> .../CpuPageTableLib/UnitTest/TestHelper.c | 14 ++++++++---
> 2 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> index f7a77d00e7..9ac3188be0 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> @@ -138,6 +138,23 @@ RandomBoolean (
> return ((Probability > ((UINT8)Random64 (0, 100))) ? TRUE : FALSE);
> }
>
> +/**
> + Set 8K stack as random value.
> +**/
> +VOID
> +SetRandomStack (
> + VOID
> + )
> +{
> + UINT64 Buffer[SIZE_1KB];
> + UINTN Index;
> +
> + for (Index = 0; Index < SIZE_1KB; Index++) {
> + Buffer[Index] = Random64 (0, MAX_UINT64);
> + Buffer[Index] = Buffer[Index];
> + }
> +}
> +
> /**
> Check if the Page table entry is valid
>
> @@ -670,6 +687,7 @@ SingleMapEntryTest (
> IsNotPresent = FALSE;
> IsModified = FALSE;
>
> + SetRandomStack ();
> GenerateSingleRandomMapEntry (MaxAddress, MapEntrys);
> LastMapEntry = &MapEntrys->Maps[MapsIndex];
> Status = PageTableParse (*PageTable, PagingMode, NULL, &MapCount);
> @@ -1039,7 +1057,11 @@ TestCaseforRandomTest (
> mSupportedBit.Bits.Pat = 1;
> mSupportedBit.Bits.Global = 1;
> mSupportedBit.Bits.ProtectionKey = 0xF;
> - mSupportedBit.Bits.Nx = 1;
> + if (((CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT *)Context)-
> >PagingMode == PagingPae) {
> + mSupportedBit.Bits.ProtectionKey = 0;
> + }
> +
> + mSupportedBit.Bits.Nx = 1;
>
> mRandomOption = ((CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> *)Context)->RandomOption;
> mNumberIndex = 0;
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> index 67776255c2..d2c50a6c8a 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> @@ -9,6 +9,7 @@
> #include "CpuPageTableLibUnitTest.h"
> #include "../CpuPageTable.h"
>
> +#define IA32_PAE_RESERVED_MASK 0x7FF0000000000000ull
> //
> // Global Data to validate if the page table is legal
> // mValidMaskNoLeaf[0] is not used
> @@ -95,6 +96,7 @@ InitGlobalData (
> @param[in] Level the level of PagingEntry.
> @param[in] MaxLeafLevel Max leaf entry level.
> @param[in] LinearAddress The linear address verified.
> + @param[in] PagingMode The paging mode.
>
> @retval Leaf entry.
> **/
> @@ -103,13 +105,18 @@ IsPageTableEntryValid (
> IN IA32_PAGING_ENTRY *PagingEntry,
> IN UINTN Level,
> IN UINTN MaxLeafLevel,
> - IN UINT64 Address
> + IN UINT64 Address,
> + IN PAGING_MODE PagingMode
> )
> {
> UINT64 Index;
> IA32_PAGING_ENTRY *ChildPageEntry;
> UNIT_TEST_STATUS Status;
>
> + if (PagingMode == PagingPae) {
> + UT_ASSERT_EQUAL (PagingEntry->Uint64 & IA32_PAE_RESERVED_MASK,
> 0);
> + }
> +
> if (PagingEntry->Pce.Present == 0) {
> return UNIT_TEST_PASSED;
> }
> @@ -142,7 +149,7 @@ IsPageTableEntryValid (
>
> ChildPageEntry = (IA32_PAGING_ENTRY
> *)(UINTN)(IA32_PNLE_PAGE_TABLE_BASE_ADDRESS (&PagingEntry->Pnle));
> for (Index = 0; Index < 512; Index++) {
> - Status = IsPageTableEntryValid (&ChildPageEntry[Index], Level-1,
> MaxLeafLevel, Address + (Index<<(9*(Level-1) + 3)));
> + Status = IsPageTableEntryValid (&ChildPageEntry[Index], Level-1,
> MaxLeafLevel, Address + (Index<<(9*(Level-1) + 3)), PagingMode);
> if (Status != UNIT_TEST_PASSED) {
> return Status;
> }
> @@ -190,9 +197,10 @@ IsPageTableValid (
> if (PagingMode == PagingPae) {
> UT_ASSERT_EQUAL (PagingEntry[Index].PdptePae.Bits.MustBeZero, 0);
> UT_ASSERT_EQUAL (PagingEntry[Index].PdptePae.Bits.MustBeZero2, 0);
> + UT_ASSERT_EQUAL (PagingEntry[Index].PdptePae.Bits.MustBeZero3, 0);
> }
>
> - Status = IsPageTableEntryValid (&PagingEntry[Index], MaxLevel,
> MaxLeafLevel, Index << (9 * MaxLevel + 3));
> + Status = IsPageTableEntryValid (&PagingEntry[Index], MaxLevel,
> MaxLeafLevel, Index << (9 * MaxLevel + 3), PagingMode);
> if (Status != UNIT_TEST_PASSED) {
> return Status;
> }
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111978): https://edk2.groups.io/g/devel/message/111978
Mute This Topic: https://groups.io/mt/102889279/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/CpuMpPei: Use CpuPageTableLib to set memory attribute.
2023-11-30 6:29 ` [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/CpuMpPei: Use CpuPageTableLib to set memory attribute Zhiguang Liu
@ 2023-12-01 8:42 ` Ni, Ray
2023-12-12 0:35 ` Laszlo Ersek
0 siblings, 1 reply; 7+ messages in thread
From: Ni, Ray @ 2023-12-01 8:42 UTC (permalink / raw)
To: Liu, Zhiguang, devel@edk2.groups.io
Cc: Kumar, Rahul R, Gerd Hoffmann, Laszlo Ersek
Reviewed-by: Ray Ni <ray.ni@intel.com>
Thanks,
Ray
> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Thursday, November 30, 2023 2:29 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH v2 3/3] UefiCpuPkg/CpuMpPei: Use CpuPageTableLib to set
> memory attribute.
>
> Currently, there are code to set memory attribute in CpuMpPei module.
> However, the code doesn't handle the case of 5 level paging.
> Use the CpuPageTableLib to set memory attribute for two purpose:
> 1. Add 5 level paging support
> 2. Clean up code
>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
> UefiCpuPkg/CpuMpPei/CpuPaging.c | 317 +++++++-------------------------
> 1 file changed, 69 insertions(+), 248 deletions(-)
>
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index b7ddb0005b..2dd7237141 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -15,50 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Guid/MigratedFvInfo.h>
>
> #include "CpuMpPei.h"
> -
> -#define IA32_PG_P BIT0
> -#define IA32_PG_RW BIT1
> -#define IA32_PG_U BIT2
> -#define IA32_PG_A BIT5
> -#define IA32_PG_D BIT6
> -#define IA32_PG_PS BIT7
> -#define IA32_PG_NX BIT63
> -
> -#define PAGE_ATTRIBUTE_BITS (IA32_PG_RW | IA32_PG_P)
> -#define PAGE_PROGATE_BITS (IA32_PG_D | IA32_PG_A | IA32_PG_NX |
> IA32_PG_U | \
> - PAGE_ATTRIBUTE_BITS)
> -
> -#define PAGING_PAE_INDEX_MASK 0x1FF
> -#define PAGING_4K_ADDRESS_MASK_64 0x000FFFFFFFFFF000ull
> -#define PAGING_2M_ADDRESS_MASK_64 0x000FFFFFFFE00000ull
> -#define PAGING_1G_ADDRESS_MASK_64 0x000FFFFFC0000000ull
> -#define PAGING_512G_ADDRESS_MASK_64 0x000FFF8000000000ull
> -
> -typedef enum {
> - PageNone = 0,
> - PageMin = 1,
> - Page4K = PageMin,
> - Page2M = 2,
> - Page1G = 3,
> - Page512G = 4,
> - PageMax = Page512G
> -} PAGE_ATTRIBUTE;
> -
> -typedef struct {
> - PAGE_ATTRIBUTE Attribute;
> - UINT64 Length;
> - UINT64 AddressMask;
> - UINTN AddressBitOffset;
> - UINTN AddressBitLength;
> -} PAGE_ATTRIBUTE_TABLE;
> -
> -PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = {
> - { PageNone, 0, 0, 0, 0 },
> - { Page4K, SIZE_4KB, PAGING_4K_ADDRESS_MASK_64, 12, 9 },
> - { Page2M, SIZE_2MB, PAGING_2M_ADDRESS_MASK_64, 21, 9 },
> - { Page1G, SIZE_1GB, PAGING_1G_ADDRESS_MASK_64, 30, 9 },
> - { Page512G, SIZE_512GB, PAGING_512G_ADDRESS_MASK_64, 39, 9 },
> -};
> +#define PAGING_4K_ADDRESS_MASK_64 0x000FFFFFFFFFF000ull
>
> EFI_PEI_NOTIFY_DESCRIPTOR mPostMemNotifyList[] = {
> {
> @@ -117,237 +74,101 @@ AllocatePageTableMemory (
> return Address;
> }
>
> -/**
> - Get the type of top level page table.
> -
> - @retval Page512G PML4 paging.
> - @retval Page1G PAE paging.
> -
> -**/
> -PAGE_ATTRIBUTE
> -GetPageTableTopLevelType (
> - VOID
> - )
> -{
> - MSR_IA32_EFER_REGISTER MsrEfer;
> -
> - MsrEfer.Uint64 = AsmReadMsr64 (MSR_CORE_IA32_EFER);
> -
> - return (MsrEfer.Bits.LMA == 1) ? Page512G : Page1G;
> -}
> -
> -/**
> - Return page table entry matching the address.
> -
> - @param[in] Address The address to be checked.
> - @param[out] PageAttributes The page attribute of the page entry.
> -
> - @return The page entry.
> -**/
> -VOID *
> -GetPageTableEntry (
> - IN PHYSICAL_ADDRESS Address,
> - OUT PAGE_ATTRIBUTE *PageAttribute
> - )
> -{
> - INTN Level;
> - UINTN Index;
> - UINT64 *PageTable;
> - UINT64 AddressEncMask;
> -
> - AddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask);
> - PageTable = (UINT64 *)(UINTN)(AsmReadCr3 () &
> PAGING_4K_ADDRESS_MASK_64);
> - for (Level = (INTN)GetPageTableTopLevelType (); Level > 0; --Level) {
> - Index = (UINTN)RShiftU64 (Address,
> mPageAttributeTable[Level].AddressBitOffset);
> - Index &= PAGING_PAE_INDEX_MASK;
> -
> - //
> - // No mapping?
> - //
> - if (PageTable[Index] == 0) {
> - *PageAttribute = PageNone;
> - return NULL;
> - }
> -
> - //
> - // Page memory?
> - //
> - if (((PageTable[Index] & IA32_PG_PS) != 0) || (Level == PageMin)) {
> - *PageAttribute = (PAGE_ATTRIBUTE)Level;
> - return &PageTable[Index];
> - }
> -
> - //
> - // Page directory or table
> - //
> - PageTable = (UINT64 *)(UINTN)(PageTable[Index] &
> - ~AddressEncMask &
> - PAGING_4K_ADDRESS_MASK_64);
> - }
> -
> - *PageAttribute = PageNone;
> - return NULL;
> -}
> -
> -/**
> - This function splits one page entry to smaller 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.
> - @param[in] Recursively Do the split recursively or not.
> -
> - @retval RETURN_SUCCESS The page entry is splitted.
> - @retval RETURN_INVALID_PARAMETER If target page attribute is invalid
> - @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,
> - IN BOOLEAN Recursively
> - )
> -{
> - UINT64 BaseAddress;
> - UINT64 *NewPageEntry;
> - UINTN Index;
> - UINT64 AddressEncMask;
> - PAGE_ATTRIBUTE SplitTo;
> -
> - if ((SplitAttribute == PageNone) || (SplitAttribute >= PageAttribute)) {
> - ASSERT (SplitAttribute != PageNone);
> - ASSERT (SplitAttribute < PageAttribute);
> - return RETURN_INVALID_PARAMETER;
> - }
> -
> - NewPageEntry = AllocatePageTableMemory (1);
> - if (NewPageEntry == NULL) {
> - ASSERT (NewPageEntry != NULL);
> - return RETURN_OUT_OF_RESOURCES;
> - }
> -
> - //
> - // One level down each step to achieve more compact page table.
> - //
> - SplitTo = PageAttribute - 1;
> - AddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask) &
> - mPageAttributeTable[SplitTo].AddressMask;
> - BaseAddress = *PageEntry &
> - ~PcdGet64 (PcdPteMemoryEncryptionAddressOrMask) &
> - mPageAttributeTable[PageAttribute].AddressMask;
> - for (Index = 0; Index < SIZE_4KB / sizeof (UINT64); Index++) {
> - NewPageEntry[Index] = BaseAddress | AddressEncMask |
> - ((*PageEntry) & PAGE_PROGATE_BITS);
> -
> - if (SplitTo != PageMin) {
> - NewPageEntry[Index] |= IA32_PG_PS;
> - }
> -
> - if (Recursively && (SplitTo > SplitAttribute)) {
> - SplitPage (&NewPageEntry[Index], SplitTo, SplitAttribute, Recursively);
> - }
> -
> - BaseAddress += mPageAttributeTable[SplitTo].Length;
> - }
> -
> - (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> PAGE_ATTRIBUTE_BITS;
> -
> - return RETURN_SUCCESS;
> -}
> -
> /**
> 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.
> + by BaseAddress and Length to not present.
>
> Caller should make sure BaseAddress and Length is at page boundary.
>
> @param[in] BaseAddress Start address of a memory region.
> @param[in] Length Size in bytes of the memory region.
> - @param[in] Attributes Bit mask of attributes to modify.
> -
> - @retval RETURN_SUCCESS The attributes were modified for the
> memory
> - region.
> - @retval RETURN_INVALID_PARAMETER Length is zero; or,
> - Attributes specified an illegal combination
> - of attributes that cannot be set together; or
> - Addressis not 4KB aligned.
> +
> + @retval RETURN_SUCCESS The memory region is changed to not
> present.
> @retval RETURN_OUT_OF_RESOURCES There are not enough system
> resources to modify
> the attributes.
> @retval RETURN_UNSUPPORTED Cannot modify the attributes of given
> memory.
>
> **/
> RETURN_STATUS
> -EFIAPI
> -ConvertMemoryPageAttributes (
> +ConvertMemoryPageToNotPresent (
> IN PHYSICAL_ADDRESS BaseAddress,
> - IN UINT64 Length,
> - IN UINT64 Attributes
> + IN UINT64 Length
> )
> {
> - UINT64 *PageEntry;
> - PAGE_ATTRIBUTE PageAttribute;
> - RETURN_STATUS Status;
> - EFI_PHYSICAL_ADDRESS MaximumAddress;
> -
> - if ((Length == 0) ||
> - ((BaseAddress & (SIZE_4KB - 1)) != 0) ||
> - ((Length & (SIZE_4KB - 1)) != 0))
> - {
> - ASSERT (Length > 0);
> - ASSERT ((BaseAddress & (SIZE_4KB - 1)) == 0);
> - ASSERT ((Length & (SIZE_4KB - 1)) == 0);
> -
> - return RETURN_INVALID_PARAMETER;
> - }
> -
> - MaximumAddress = (EFI_PHYSICAL_ADDRESS)MAX_UINT32;
> - if ((BaseAddress > MaximumAddress) ||
> - (Length > MaximumAddress) ||
> - (BaseAddress > MaximumAddress - (Length - 1)))
> - {
> - return RETURN_UNSUPPORTED;
> - }
> -
> - //
> - // Below logic is to check 2M/4K page to make sure we do not waste
> memory.
> - //
> - while (Length != 0) {
> - PageEntry = GetPageTableEntry (BaseAddress, &PageAttribute);
> - if (PageEntry == NULL) {
> - return RETURN_UNSUPPORTED;
> - }
> + EFI_STATUS Status;
> + UINTN PageTable;
> + EFI_PHYSICAL_ADDRESS Buffer;
> + UINTN BufferSize;
> + IA32_MAP_ATTRIBUTE MapAttribute;
> + IA32_MAP_ATTRIBUTE MapMask;
> + PAGING_MODE PagingMode;
> + IA32_CR4 Cr4;
> + BOOLEAN Page5LevelSupport;
> + UINT32 RegEax;
> + BOOLEAN Page1GSupport;
> + CPUID_EXTENDED_CPU_SIG_EDX RegEdx;
> +
> + if (sizeof (UINTN) == sizeof (UINT64)) {
> + //
> + // Check Page5Level Support or not.
> + //
> + Cr4.UintN = AsmReadCr4 ();
> + Page5LevelSupport = (Cr4.Bits.LA57 ? TRUE : FALSE);
>
> - if (PageAttribute != Page4K) {
> - Status = SplitPage (PageEntry, PageAttribute, Page4K, FALSE);
> - if (RETURN_ERROR (Status)) {
> - return Status;
> + //
> + // Check Page1G Support or not.
> + //
> + Page1GSupport = FALSE;
> + AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
> + if (RegEax >= CPUID_EXTENDED_CPU_SIG) {
> + AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL,
> &RegEdx.Uint32);
> + if (RegEdx.Bits.Page1GB != 0) {
> + Page1GSupport = TRUE;
> }
> -
> - //
> - // Do it again until the page is 4K.
> - //
> - continue;
> }
>
> //
> - // Just take care of 'present' bit for Stack Guard.
> + // Decide Paging Mode according Page5LevelSupport & Page1GSupport.
> //
> - if ((Attributes & IA32_PG_P) != 0) {
> - *PageEntry |= (UINT64)IA32_PG_P;
> + if (Page5LevelSupport) {
> + PagingMode = Page1GSupport ? Paging5Level1GB : Paging5Level;
> } else {
> - *PageEntry &= ~((UINT64)IA32_PG_P);
> + PagingMode = Page1GSupport ? Paging4Level1GB : Paging4Level;
> }
> + } else {
> + PagingMode = PagingPae;
> + }
> +
> + MapAttribute.Uint64 = 0;
> + MapMask.Uint64 = 0;
> + MapMask.Bits.Present = 1;
> + PageTable = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;
> + BufferSize = 0;
>
> + //
> + // Get required buffer size for the pagetable that will be created.
> + //
> + Status = PageTableMap (&PageTable, PagingMode, 0, &BufferSize,
> BaseAddress, Length, &MapAttribute, &MapMask, NULL);
> + if (Status == EFI_BUFFER_TOO_SMALL) {
> //
> - // Convert success, move to next
> + // Allocate required Buffer.
> //
> - BaseAddress += SIZE_4KB;
> - Length -= SIZE_4KB;
> + Status = PeiServicesAllocatePages (
> + EfiBootServicesData,
> + EFI_SIZE_TO_PAGES (BufferSize),
> + &Buffer
> + );
> + ASSERT_EFI_ERROR (Status);
> + if (EFI_ERROR (Status)) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + Status = PageTableMap (&PageTable, PagingMode, (VOID *)(UINTN)Buffer,
> &BufferSize, BaseAddress, Length, &MapAttribute, &MapMask, NULL);
> }
>
> - return RETURN_SUCCESS;
> + ASSERT_EFI_ERROR (Status);
> + AsmWriteCr3 (PageTable);
> + return Status;
> }
>
> /**
> @@ -516,7 +337,7 @@ SetupStackGuardPage (
> //
> // Set Guard page at stack base address.
> //
> - ConvertMemoryPageAttributes (StackBase, EFI_PAGE_SIZE, 0);
> + ConvertMemoryPageToNotPresent (StackBase, EFI_PAGE_SIZE);
> DEBUG ((
> DEBUG_INFO,
> "Stack Guard set at %lx [cpu%lu]!\n",
> @@ -599,7 +420,7 @@ MemoryDiscoveredPpiNotifyCallback (
> // Enable #PF exception, so if the code access SPI after disable NEM, it will
> generate
> // the exception to avoid potential vulnerability.
> //
> - ConvertMemoryPageAttributes (MigratedFvInfo->FvOrgBase,
> MigratedFvInfo->FvLength, 0);
> + ConvertMemoryPageToNotPresent (MigratedFvInfo->FvOrgBase,
> MigratedFvInfo->FvLength);
>
> Hob.Raw = GET_NEXT_HOB (Hob);
> Hob.Raw = GetNextGuidHob (&gEdkiiMigratedFvInfoGuid, Hob.Raw);
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111979): https://edk2.groups.io/g/devel/message/111979
Mute This Topic: https://groups.io/mt/102889280/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/CpuMpPei: Use CpuPageTableLib to set memory attribute.
2023-12-01 8:42 ` Ni, Ray
@ 2023-12-12 0:35 ` Laszlo Ersek
0 siblings, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2023-12-12 0:35 UTC (permalink / raw)
To: devel, ray.ni, Liu, Zhiguang; +Cc: Kumar, Rahul R, Gerd Hoffmann
On 12/1/23 09:42, Ni, Ray wrote:
> Reviewed-by: Ray Ni <ray.ni@intel.com>
This series seems to have been merged meanwhile -- that's good, because
I could have usefully reviewed only patch#1, and only ACK patches #2 and
#3, due to time constraints.
However... please do report on the list whenever a series is merged! I
started reviewing patch#1, only to find that it was already upstream. I
could have saved that time if I had seen, on the list, a statement of
merging.
So, for the record: commit range
ef3fde64aa78598a4c21556629936fb228390e8c..7e18c9a788e543ab71cdc0485989cf5d00cdccc2.
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112347): https://edk2.groups.io/g/devel/message/112347
Mute This Topic: https://groups.io/mt/102889280/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-12-12 0:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-30 6:29 [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/CpuPageTableLib: Init local variable before using it Zhiguang Liu
2023-11-30 6:29 ` [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/CpuPageTableLib/TestCase: Refine test case for PAE paging Zhiguang Liu
2023-12-01 8:41 ` Ni, Ray
2023-11-30 6:29 ` [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/CpuMpPei: Use CpuPageTableLib to set memory attribute Zhiguang Liu
2023-12-01 8:42 ` Ni, Ray
2023-12-12 0:35 ` Laszlo Ersek
2023-12-01 8:40 ` [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/CpuPageTableLib: Init local variable before using it Ni, Ray
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox