Re: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/DxeIpl: Align Page table Level setting with previous level.

["Ni, Ray" <ray.ni@intel.com>]

Minor comments:
IA32 -> 32bit protected mode.
X64 -> 64bit long mode.

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Jiaxin
> Sent: Tuesday, May 9, 2023 6:23 PM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan <dandan.bi@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
> Subject: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/DxeIpl: Align Page table
> Level setting with previous level.
> 
> System paging 5 level enabled or not can be checked via CR4.LA57, system
> preferred Page table Level (PcdUse5LevelPageTable) must align
> with previous level for X64 mode.
> 
> This patch is to do the wise check:
> If X64, Page table Level setting in PcdUse5LevelPageTable must align with
> previous level.
> If IA32, Page table Level is decided by PcdUse5LevelPageTable and feature
> capability.
> 
> Change-Id: Ia7f7e365c7354cc49f971209bfcbc5af5aded062
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zeng Star <star.zeng@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 39
> ++++++++++++++++--------
>  1 file changed, 27 insertions(+), 12 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> index 18b121d768..301e200cd8 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> @@ -737,22 +737,37 @@ CreateIdentityMappingPageTables (
>      } else {
>        PhysicalAddressBits = 36;
>      }
>    }
> 
> -  Page5LevelSupport = FALSE;
> -  if (PcdGetBool (PcdUse5LevelPageTable)) {
> -    AsmCpuidEx (
> -      CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS,
> -      CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_SUB_LEAF_INFO,
> -      NULL,
> -      NULL,
> -      &EcxFlags.Uint32,
> -      NULL
> -      );
> -    if (EcxFlags.Bits.FiveLevelPage != 0) {
> -      Page5LevelSupport = TRUE;
> +  //
> +  // Check run in X64 or IA32
> +  //
> +  if (sizeof (UINTN) == sizeof (UINT64)) {
> +    //
> +    // If X64, Page table Level must align with previous level.
> +    //
> +    Cr4.UintN = AsmReadCr4 ();
> +    Page5LevelSupport = Cr4.Bits.LA57 ? TRUE : FALSE;
> +    ASSERT (PcdGetBool (PcdUse5LevelPageTable) == Page5LevelSupport);
> +  } else {
> +    //
> +    // If IA32, Page table Level is decided by PCD and feature capbility.
> +    //
> +    Page5LevelSupport = FALSE;
> +    if (PcdGetBool (PcdUse5LevelPageTable)) {
> +      AsmCpuidEx (
> +        CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS,
> +        CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_SUB_LEAF_INFO,
> +        NULL,
> +        NULL,
> +        &EcxFlags.Uint32,
> +        NULL
> +        );
> +      if (EcxFlags.Bits.FiveLevelPage != 0) {
> +        Page5LevelSupport = TRUE;
> +      }
>      }
>    }
> 
>    DEBUG ((DEBUG_INFO, "AddressBits=%u 5LevelPaging=%u 1GPage=%u\n",
> PhysicalAddressBits, Page5LevelSupport, Page1GSupport));
> 
> --
> 2.16.2.windows.1
> 
> 
> 
> 
>