tyler.durden
/
audk
mirror of https://github.com/acidanthera/audk.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

CpuPageTableLib: Fix a bug to avoid unnecessary changing to page table 

With the following paging structure that maps [0, 2G] with ReadWrite
bit set.
PML4[0] --> PDPTE[0] --> PDE[0-255]
              \-> PDPTE[1] --> PDE[0-255]

If ReadWrite bit is cleared in PML4[0] and PageTableMap() is called
to change [0, 2M] as read-only, today's logic unnecessarily changes
the paging structure in 2 aspects:
1. When setting PageTableBaseAddress in the entry, the code clears
    all attributes.
2. Even the ReadWrite bit in parent entry is not set, the code clears
    the ReadWrite bit in the leaf entry.

First change is wrong. It should not change other attributes when
setting the PA.
Second change is unnecessary. Because the parent entry already
declares the whole region as read-only, there is no need to clear
ReadWrite bit in the leaf entry again.

Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
This commit is contained in:
Ray Ni 2022-07-15 20:40:29 +08:00 committed by mergify[bot]
parent f4c845e46b
commit 9f53fd4ba7
1 changed files with 32 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
View File

@ -27,10 +27,7 @@ PageTableLibSetPte4K (
)
{
if (Mask->Bits.PageTableBaseAddress) {
//
// Reset all attributes when the physical address is changed.
//
Pte4K->Uint64 = IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribute) + Offset;
Pte4K->Uint64 = (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribute) + Offset) | (Pte4K->Uint64 & ~IA32_PE_BASE_ADDRESS_MASK_40);
}
if (Mask->Bits.Present) {
@ -97,10 +94,7 @@ PageTableLibSetPleB (
)
{
if (Mask->Bits.PageTableBaseAddress) {
//
// Reset all attributes when the physical address is changed.
//
PleB->Uint64 = IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribute) + Offset;
PleB->Uint64 = (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribute) + Offset) | (PleB->Uint64 & ~IA32_PE_BASE_ADDRESS_MASK_39);
}
PleB->Bits.MustBeOne = 1;
@ -277,6 +271,7 @@ PageTableLibMapInLevel (
IA32_PAGING_ENTRY OneOfPagingEntry;
IA32_MAP_ATTRIBUTE ChildAttribute;
IA32_MAP_ATTRIBUTE ChildMask;
IA32_MAP_ATTRIBUTE CurrentMask;
ASSERT (Level != 0);
ASSERT ((Attribute != NULL) && (Mask != NULL));
@ -464,7 +459,35 @@ PageTableLibMapInLevel (
// Create one entry mapping the entire region (1G, 2M or 4K).
//
if (Modify) {
PageTableLibSetPle (Level, CurrentPagingEntry, Offset, Attribute, Mask);
//
// When the inheritable attributes in parent entry could override the child attributes,
// e.g.: Present/ReadWrite/UserSupervisor is 0 in parent entry, or
// Nx is 1 in parent entry,
// we just skip setting any value to these attributes in child.
// We add assertion to make sure the requested settings don't conflict with parent attributes in this case.
//
CurrentMask.Uint64 = Mask->Uint64;
if (ParentAttribute->Bits.Present == 0) {
CurrentMask.Bits.Present = 0;
ASSERT (CreateNew || (Mask->Bits.Present == 0) || (Attribute->Bits.Present == 0));
}
if (ParentAttribute->Bits.ReadWrite == 0) {
CurrentMask.Bits.ReadWrite = 0;
ASSERT (CreateNew || (Mask->Bits.ReadWrite == 0) || (Attribute->Bits.ReadWrite == 0));
}
if (ParentAttribute->Bits.UserSupervisor == 0) {
CurrentMask.Bits.UserSupervisor = 0;
ASSERT (CreateNew || (Mask->Bits.UserSupervisor == 0) || (Attribute->Bits.UserSupervisor == 0));
}
if (ParentAttribute->Bits.Nx == 1) {
CurrentMask.Bits.Nx = 0;
ASSERT (CreateNew || (Mask->Bits.Nx == 0) || (Attribute->Bits.Nx == 1));
}
PageTableLibSetPle (Level, CurrentPagingEntry, Offset, Attribute, &CurrentMask);
}
} else {
//