Re: [edk2-devel] [PATCH v2 2/5] UefiCpuPkg/CpuMpPei: Enable PAE page table if CR0.PG is not set
1. can you please change the commit title as "UefiCpuPkg/CpuMpPei:
Conditionally enable PAE paging in 32bit mode"?
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Wu, Jiaxin
> Sent: Friday, May 12, 2023 12:16 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Zeng, Star
> ; Gerd Hoffmann ; Kumar, Rahul R
>
> Subject: [edk2-devel] [PATCH v2 2/5] UefiCpuPkg/CpuMpPei: Enable PAE page
> table if CR0.PG is not set
>
> Some security features depends on the page table enabling. So, This patch
> is to enable the page table if page table has not been enabled during the
> transition from Temporary RAM to Permanent RAM.
2. No need to emphasize paging is not enabled during temp-ram to permanent-ram
migration.
We can just say "Some security features depend on the page table enabling. So,
This patch
is to enable paging if it is not enabled (32bit mode)"
3. following commit message might not be needed.
>
> Note: If page table is not enabled before this point, which means the system
> IA-32e Mode is not activated. Because on Intel 64 processors, IA-32e Mode
> operation requires physical address extensions with 4 or 5 levels of enhanced
> paging structures (see Section 4.5, "4 - Level Paging and 5 -Level Paging"
> and Section 9.8, "Initializing IA-32e Mode"). So, just enable PAE page table
> if CR0.PG is not set.
>
> -UINTN
> -CreatePageTable (
> +EFI_STATUS
> +EnablePaePageTable (
>VOID
>)
> {
> - RETURN_STATUS Status;
> - UINTN PhysicalAddressBits;
> - UINTN NumberOfEntries;
> - PAGE_ATTRIBUTETopLevelPageAttr;
> - UINTN PageTable;
> - PAGE_ATTRIBUTEMaxMemoryPage;
> - UINTN Index;
> - UINT64AddressEncMask;
> - UINT64*PageEntry;
> - EFI_PHYSICAL_ADDRESS PhysicalAddress;
> -
> - TopLevelPageAttr= (PAGE_ATTRIBUTE)GetPageTableTopLevelType ();
> - PhysicalAddressBits = GetPhysicalAddressWidth ();
> - NumberOfEntries = (UINTN)1 << (PhysicalAddressBits -
> -
> mPageAttributeTable[TopLevelPageAttr].AddressBitOffset);
> + EFI_STATUS Status;
> + PAGING_MODE PagingMode;
> +
> + UINTN PageTable;
> + VOID*Buffer;
> + UINTN BufferSize;
> + IA32_MAP_ATTRIBUTE MapAttribute;
> + IA32_MAP_ATTRIBUTE MapMask;
> +
> + PagingMode = PagingPae;
4. No need of local variable PagingMode.
> + PageTable = 0;
> + Buffer = NULL;
> + BufferSize = 0;
> + MapAttribute.Uint64 = 0;
> + MapMask.Uint64 = MAX_UINT64;
> + MapAttribute.Bits.Present = 1;
> + MapAttribute.Bits.ReadWrite = 1;
>
> - PageTable = (UINTN)AllocatePageTableMemory (1);
> - if (PageTable == 0) {
> -return 0;
> + //
> + // Get required buffer size for the pagetable that will be created.
> + // The Max size of LinearAddress for PAE is 2^32.
5. Let's say "1:1 map 4GB in 32bit mode." Because people might say "PAE can
support up to 2^36 address, why 2^32 here".
> + // Create PageTable in permanent memory.
> + // The Max size of LinearAddress for PAE is 2^32.
6. above comments seem to be redundant.
>EFI_PEI_HOB_POINTERSHob;
> + IA32_CR0Cr0;
>
>//
>// Paging must be setup first. Otherwise the exception TSS setup during MP
>// initialization later will not contain paging information and then fail
>// the task switch (for the sake of stack switch).
> @@ -635,12 +575,29 @@ MemoryDiscoveredPpiNotifyCallback (
>if (IsIa32PaeSupported ()) {
> Hob.Raw= GetFirstGuidHob ();
> InitStackGuard = PcdGetBool (PcdCpuStackGuard);
>}
>
> - if (InitStackGuard || (Hob.Raw != NULL)) {
> -EnablePaging ();
> + //
> + // Some security features depends on the page table enabling. So, here
> + // is to enable the page table if page table has not been enabled yet.
> + // If page table is not enabled before this point, which means the system
> + // IA-32e Mode is not activated. Because on Intel 64 processors, IA-32e
> Mode
> + // operation requires physical address extensions with 4 or 5 levels of
> + // enhanced paging structures (see Section 4.5, "4 - Level Paging and 5 -
> + // Level Paging" and Section 9.8, "Initializing IA-32e Mode"). So, just
> + // enable PAE page table if CR0.PG is not set.
7. Maybe simply say " Some security features depend on the page table enabling.
So, here
is to enable paging if it is not enabled (only in 32bit mode)"
> + /
[edk2-devel] [PATCH v2 2/5] UefiCpuPkg/CpuMpPei: Enable PAE page table if CR0.PG is not set
Some security features depends on the page table enabling. So, This patch
is to enable the page table if page table has not been enabled during the
transition from Temporary RAM to Permanent RAM.
Note: If page table is not enabled before this point, which means the system
IA-32e Mode is not activated. Because on Intel 64 processors, IA-32e Mode
operation requires physical address extensions with 4 or 5 levels of enhanced
paging structures (see Section 4.5, "4 - Level Paging and 5 -Level Paging"
and Section 9.8, "Initializing IA-32e Mode"). So, just enable PAE page table
if CR0.PG is not set.
Cc: Eric Dong
Cc: Ray Ni
Cc: Zeng Star
Cc: Gerd Hoffmann
Cc: Rahul Kumar
Signed-off-by: Jiaxin Wu
---
UefiCpuPkg/CpuMpPei/CpuMpPei.h | 1 +
UefiCpuPkg/CpuMpPei/CpuMpPei.inf | 1 +
UefiCpuPkg/CpuMpPei/CpuPaging.c | 215 ---
3 files changed, 88 insertions(+), 129 deletions(-)
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
index 0649c48d14..1b9a94e18f 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
@@ -26,10 +26,11 @@
#include
#include
#include
#include
#include
+#include
extern EFI_PEI_PPI_DESCRIPTOR mPeiCpuMpPpiDesc;
/**
This service retrieves the number of logical processor in the platform
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
index 7444bdb968..865be5627e 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
@@ -44,10 +44,11 @@
CpuExceptionHandlerLib
MpInitLib
BaseMemoryLib
CpuLib
MemoryAllocationLib
+ CpuPageTableLib
[Guids]
gEdkiiMigratedFvInfoGuid ##
SOMETIMES_CONSUMES ## HOB
[Ppis]
diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
index a471f089c8..b282e1cf14 100644
--- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
+++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
@@ -115,42 +115,10 @@ AllocatePageTableMemory (
}
return Address;
}
-/**
- Get the address width supported by current processor.
-
- @retval 32 If processor is in 32-bit mode.
- @retval 36-48 If processor is in 64-bit mode.
-
-**/
-UINTN
-GetPhysicalAddressWidth (
- VOID
- )
-{
- UINT32 RegEax;
-
- if (sizeof (UINTN) == 4) {
-return 32;
- }
-
- AsmCpuid (CPUID_EXTENDED_FUNCTION, , NULL, NULL, NULL);
- if (RegEax >= CPUID_VIR_PHY_ADDRESS_SIZE) {
-AsmCpuid (CPUID_VIR_PHY_ADDRESS_SIZE, , NULL, NULL, NULL);
-RegEax &= 0xFF;
-if (RegEax > 48) {
- return 48;
-}
-
-return (UINTN)RegEax;
- }
-
- return 36;
-}
-
/**
Get the type of top level page table.
@retval Page512G PML4 paging.
@retval Page1GPAE paging.
@@ -381,120 +349,91 @@ ConvertMemoryPageAttributes (
return RETURN_SUCCESS;
}
/**
- Get maximum size of page memory supported by current processor.
-
- @param[in] TopLevelType The type of top level page entry.
+ Enable PAE Page Table.
- @retval Page1G If processor supports 1G page and PML4.
- @retval Page2M For all other situations.
+ @retval EFI_SUCCESS The PAE Page Table was enabled successfully.
+ @retval EFI_OUT_OF_RESOURCES The PAE Page Table could not be enabled due
to lack of available memory.
**/
-PAGE_ATTRIBUTE
-GetMaxMemoryPage (
- IN PAGE_ATTRIBUTE TopLevelType
- )
-{
- UINT32 RegEax;
- UINT32 RegEdx;
-
- if (TopLevelType == Page512G) {
-AsmCpuid (CPUID_EXTENDED_FUNCTION, , NULL, NULL, NULL);
-if (RegEax >= CPUID_EXTENDED_CPU_SIG) {
- AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, );
- if ((RegEdx & BIT26) != 0) {
-return Page1G;
- }
-}
- }
-
- return Page2M;
-}
-
-/**
- Create PML4 or PAE page table.
-
- @return The address of page table.
-
-**/
-UINTN
-CreatePageTable (
+EFI_STATUS
+EnablePaePageTable (
VOID
)
{
- RETURN_STATUS Status;
- UINTN PhysicalAddressBits;
- UINTN NumberOfEntries;
- PAGE_ATTRIBUTETopLevelPageAttr;
- UINTN PageTable;
- PAGE_ATTRIBUTEMaxMemoryPage;
- UINTN Index;
- UINT64AddressEncMask;
- UINT64*PageEntry;
- EFI_PHYSICAL_ADDRESS PhysicalAddress;
-
- TopLevelPageAttr= (PAGE_ATTRIBUTE)GetPageTableTopLevelType ();
- PhysicalAddressBits = GetPhysicalAddressWidth ();
- NumberOfEntries = (UINTN)1 << (PhysicalAddressBits -
-
mPageAttributeTable[TopLevelPageAttr].AddressBitOffset);
+ EFI_STATUS Status;
+ PAGING_MODE PagingMode;
+
+ UINTN PageTable;
+ VOID*Buffer;
+ UINTN BufferSize;
+ IA32_MAP_ATTRIBUTE MapAttribute;
+ IA32_MAP_ATTRIBUTE MapMask;
+
+ PagingMode = PagingPae;
+ PageTable = 0;
+ Buffer = NULL;
+ BufferSize = 0;
+ MapAttribute.Uint64 =
