Reviewed-by: Ray Ni <ray...@intel.com>
> -----Original Message-----
> From: Tan, Dun <dun....@intel.com>
> Sent: Thursday, March 23, 2023 3:41 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang....@intel.com>; Dong, Eric
> <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Kumar, Rahul R
> <rahul.r.ku...@intel.com>
> Subject: [Patch V4 15/21] UefiCpuPkg: Fix IA32 build failure in
> CpuPageTableLib.inf
>
> From: Zhiguang Liu <zhiguang....@intel.com>
>
> The definition of IA32_MAP_ATTRIBUTE has 64 bits, and one of the bit
> field PageTableBaseAddress is from bit 12 to bit 52. This means if the
> compiler treats the 64bits value as two UINT32 value, the field
> PageTableBaseAddress spans two UINT32 value. That's why when building in
> NOOPT mode in IA32, the below issue is noticed:
> unresolved external symbol __allshl
> This patch fix the build failure by seperate field PageTableBaseAddress
> into two fields, make sure no field spans two UINT32 value.
>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Rahul Kumar <rahul1.ku...@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang....@intel.com>
> Signed-off-by: Ray Ni <ray...@intel.com>
> ---
> UefiCpuPkg/Include/Library/CpuPageTableLib.h | 32
> ++++++++++++++++----------------
> UefiCpuPkg/Library/CpuPageTableLib/CpuPageTable.h | 125
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++--------------------------------------------------------------
> UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 22
> +++++++++++-----------
> 3 files changed, 90 insertions(+), 89 deletions(-)
>
> diff --git a/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> b/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> index c94d82ea65..5e545a35f6 100644
> --- a/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> +++ b/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> @@ -11,22 +11,22 @@
>
> typedef union {
> struct {
> - UINT64 Present : 1; // 0 = Not present in memory, 1 =
> Present in
> memory
> - UINT64 ReadWrite : 1; // 0 = Read-Only, 1= Read/Write
> - UINT64 UserSupervisor : 1; // 0 = Supervisor, 1=User
> - UINT64 WriteThrough : 1; // 0 = Write-Back caching,
> 1=Write-Through
> caching
> - UINT64 CacheDisabled : 1; // 0 = Cached, 1=Non-Cached
> - UINT64 Accessed : 1; // 0 = Not accessed, 1 = Accessed
> (set by CPU)
> - UINT64 Dirty : 1; // 0 = Not dirty, 1 = Dirty (set by
> CPU)
> - UINT64 Pat : 1; // PAT
> -
> - UINT64 Global : 1; // 0 = Not global, 1 = Global (if
> CR4.PGE = 1)
> - UINT64 Reserved1 : 3; // Ignored
> -
> - UINT64 PageTableBaseAddress : 40; // Page Table Base Address
> - UINT64 Reserved2 : 7; // Ignored
> - UINT64 ProtectionKey : 4; // Protection key
> - UINT64 Nx : 1; // No Execute bit
> + UINT32 Present : 1; // 0 = Not present in memory, 1
> = Present
> in memory
> + UINT32 ReadWrite : 1; // 0 = Read-Only, 1= Read/Write
> + UINT32 UserSupervisor : 1; // 0 = Supervisor, 1=User
> + UINT32 WriteThrough : 1; // 0 = Write-Back caching,
> 1=Write-
> Through caching
> + UINT32 CacheDisabled : 1; // 0 = Cached, 1=Non-Cached
> + UINT32 Accessed : 1; // 0 = Not accessed, 1 =
> Accessed (set by
> CPU)
> + UINT32 Dirty : 1; // 0 = Not dirty, 1 = Dirty
> (set by CPU)
> + UINT32 Pat : 1; // PAT
> + UINT32 Global : 1; // 0 = Not global, 1 = Global
> (if CR4.PGE = 1)
> + UINT32 Reserved1 : 3; // Ignored
> + UINT32 PageTableBaseAddressLow : 20; // Page Table Base Address
> Low
> +
> + UINT32 PageTableBaseAddressHigh : 20; // Page Table Base Address
> High
> + UINT32 Reserved2 : 7; // Ignored
> + UINT32 ProtectionKey : 4; // Protection key
> + UINT32 Nx : 1; // No Execute bit
> } Bits;
> UINT64 Uint64;
> } IA32_MAP_ATTRIBUTE;
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTable.h
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTable.h
> index 8d856d7c7e..2c67ecb469 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTable.h
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTable.h
> @@ -29,11 +29,12 @@ typedef enum {
> } IA32_PAGE_LEVEL;
>
> typedef struct {
> - UINT64 Present : 1; // 0 = Not present in memory, 1 =
> Present in
> memory
> - UINT64 ReadWrite : 1; // 0 = Read-Only, 1= Read/Write
> - UINT64 UserSupervisor : 1; // 0 = Supervisor, 1=User
> - UINT64 Reserved : 58;
> - UINT64 Nx : 1; // No Execute bit
> + UINT32 Present : 1; // 0 = Not present in memory, 1 =
> Present in
> memory
> + UINT32 ReadWrite : 1; // 0 = Read-Only, 1= Read/Write
> + UINT32 UserSupervisor : 1; // 0 = Supervisor, 1=User
> + UINT32 Reserved0 : 29;
> + UINT32 Reserved1 : 31;
> + UINT32 Nx : 1; // No Execute bit
> } IA32_PAGE_COMMON_ENTRY;
>
> ///
> @@ -41,20 +42,20 @@ typedef struct {
> ///
> typedef union {
> struct {
> - UINT64 Present : 1; // 0 = Not present in memory, 1 =
> Present in
> memory
> - UINT64 ReadWrite : 1; // 0 = Read-Only, 1= Read/Write
> - UINT64 UserSupervisor : 1; // 0 = Supervisor, 1=User
> - UINT64 WriteThrough : 1; // 0 = Write-Back caching,
> 1=Write-Through
> caching
> - UINT64 CacheDisabled : 1; // 0 = Cached, 1=Non-Cached
> - UINT64 Accessed : 1; // 0 = Not accessed, 1 = Accessed
> (set by CPU)
> - UINT64 Available0 : 1; // Ignored
> - UINT64 MustBeZero : 1; // Must Be Zero
> -
> - UINT64 Available2 : 4; // Ignored
> -
> - UINT64 PageTableBaseAddress : 40; // Page Table Base Address
> - UINT64 Available3 : 11; // Ignored
> - UINT64 Nx : 1; // No Execute bit
> + UINT32 Present : 1; // 0 = Not present in memory, 1
> = Present
> in memory
> + UINT32 ReadWrite : 1; // 0 = Read-Only, 1= Read/Write
> + UINT32 UserSupervisor : 1; // 0 = Supervisor, 1=User
> + UINT32 WriteThrough : 1; // 0 = Write-Back caching,
> 1=Write-
> Through caching
> + UINT32 CacheDisabled : 1; // 0 = Cached, 1=Non-Cached
> + UINT32 Accessed : 1; // 0 = Not accessed, 1 =
> Accessed (set by
> CPU)
> + UINT32 Available0 : 1; // Ignored
> + UINT32 MustBeZero : 1; // Must Be Zero
> + UINT32 Available2 : 4; // Ignored
> + UINT32 PageTableBaseAddressLow : 20; // Page Table Base Address
> Low
> +
> + UINT32 PageTableBaseAddressHigh : 20; // Page Table Base Address
> High
> + UINT32 Available3 : 11; // Ignored
> + UINT32 Nx : 1; // No Execute bit
> } Bits;
> UINT64 Uint64;
> } IA32_PAGE_NON_LEAF_ENTRY;
> @@ -86,23 +87,23 @@ typedef IA32_PAGE_NON_LEAF_ENTRY IA32_PDE;
> ///
> typedef union {
> struct {
> - UINT64 Present : 1; // 0 = Not present in memory, 1 =
> Present in
> memory
> - UINT64 ReadWrite : 1; // 0 = Read-Only, 1= Read/Write
> - UINT64 UserSupervisor : 1; // 0 = Supervisor, 1=User
> - UINT64 WriteThrough : 1; // 0 = Write-Back caching,
> 1=Write-Through
> caching
> - UINT64 CacheDisabled : 1; // 0 = Cached, 1=Non-Cached
> - UINT64 Accessed : 1; // 0 = Not accessed, 1 = Accessed
> (set by CPU)
> - UINT64 Dirty : 1; // 0 = Not dirty, 1 = Dirty (set by
> CPU)
> - UINT64 MustBeOne : 1; // Page Size. Must Be One
> -
> - UINT64 Global : 1; // 0 = Not global, 1 = Global (if
> CR4.PGE = 1)
> - UINT64 Available1 : 3; // Ignored
> - UINT64 Pat : 1; // PAT
> -
> - UINT64 PageTableBaseAddress : 39; // Page Table Base Address
> - UINT64 Available3 : 7; // Ignored
> - UINT64 ProtectionKey : 4; // Protection key
> - UINT64 Nx : 1; // No Execute bit
> + UINT32 Present : 1; // 0 = Not present in memory, 1
> = Present
> in memory
> + UINT32 ReadWrite : 1; // 0 = Read-Only, 1= Read/Write
> + UINT32 UserSupervisor : 1; // 0 = Supervisor, 1=User
> + UINT32 WriteThrough : 1; // 0 = Write-Back caching,
> 1=Write-
> Through caching
> + UINT32 CacheDisabled : 1; // 0 = Cached, 1=Non-Cached
> + UINT32 Accessed : 1; // 0 = Not accessed, 1 =
> Accessed (set by
> CPU)
> + UINT32 Dirty : 1; // 0 = Not dirty, 1 = Dirty
> (set by CPU)
> + UINT32 MustBeOne : 1; // Page Size. Must Be One
> + UINT32 Global : 1; // 0 = Not global, 1 = Global
> (if CR4.PGE = 1)
> + UINT32 Available1 : 3; // Ignored
> + UINT32 Pat : 1; // PAT
> + UINT32 PageTableBaseAddressLow : 19; // Page Table Base Address
> Low
> +
> + UINT32 PageTableBaseAddressHigh : 20; // Page Table Base Address
> High
> + UINT32 Available3 : 7; // Ignored
> + UINT32 ProtectionKey : 4; // Protection key
> + UINT32 Nx : 1; // No Execute bit
> } Bits;
> UINT64 Uint64;
> } IA32_PAGE_LEAF_ENTRY_BIG_PAGESIZE;
> @@ -123,22 +124,22 @@ typedef IA32_PAGE_LEAF_ENTRY_BIG_PAGESIZE
> IA32_PDPTE_1G;
> ///
> typedef union {
> struct {
> - UINT64 Present : 1; // 0 = Not present in memory, 1 =
> Present in
> memory
> - UINT64 ReadWrite : 1; // 0 = Read-Only, 1= Read/Write
> - UINT64 UserSupervisor : 1; // 0 = Supervisor, 1=User
> - UINT64 WriteThrough : 1; // 0 = Write-Back caching,
> 1=Write-Through
> caching
> - UINT64 CacheDisabled : 1; // 0 = Cached, 1=Non-Cached
> - UINT64 Accessed : 1; // 0 = Not accessed, 1 = Accessed
> (set by CPU)
> - UINT64 Dirty : 1; // 0 = Not dirty, 1 = Dirty (set by
> CPU)
> - UINT64 Pat : 1; // PAT
> -
> - UINT64 Global : 1; // 0 = Not global, 1 = Global (if
> CR4.PGE = 1)
> - UINT64 Available1 : 3; // Ignored
> -
> - UINT64 PageTableBaseAddress : 40; // Page Table Base Address
> - UINT64 Available3 : 7; // Ignored
> - UINT64 ProtectionKey : 4; // Protection key
> - UINT64 Nx : 1; // No Execute bit
> + UINT32 Present : 1; // 0 = Not present in memory, 1
> = Present
> in memory
> + UINT32 ReadWrite : 1; // 0 = Read-Only, 1= Read/Write
> + UINT32 UserSupervisor : 1; // 0 = Supervisor, 1=User
> + UINT32 WriteThrough : 1; // 0 = Write-Back caching,
> 1=Write-
> Through caching
> + UINT32 CacheDisabled : 1; // 0 = Cached, 1=Non-Cached
> + UINT32 Accessed : 1; // 0 = Not accessed, 1 =
> Accessed (set by
> CPU)
> + UINT32 Dirty : 1; // 0 = Not dirty, 1 = Dirty
> (set by CPU)
> + UINT32 Pat : 1; // PAT
> + UINT32 Global : 1; // 0 = Not global, 1 = Global
> (if CR4.PGE = 1)
> + UINT32 Available1 : 3; // Ignored
> + UINT32 PageTableBaseAddressLow : 20; // Page Table Base Address
> Low
> +
> + UINT32 PageTableBaseAddressHigh : 20; // Page Table Base Address
> High
> + UINT32 Available3 : 7; // Ignored
> + UINT32 ProtectionKey : 4; // Protection key
> + UINT32 Nx : 1; // No Execute bit
> } Bits;
> UINT64 Uint64;
> } IA32_PTE_4K;
> @@ -149,16 +150,16 @@ typedef union {
> ///
> typedef union {
> struct {
> - UINT64 Present : 1; // 0 = Not present in memory, 1 =
> Present in
> memory
> - UINT64 MustBeZero : 2; // Must Be Zero
> - UINT64 WriteThrough : 1; // 0 = Write-Back caching,
> 1=Write-Through
> caching
> - UINT64 CacheDisabled : 1; // 0 = Cached, 1=Non-Cached
> - UINT64 MustBeZero2 : 4; // Must Be Zero
> -
> - UINT64 Available : 3; // Ignored
> -
> - UINT64 PageTableBaseAddress : 40; // Page Table Base Address
> - UINT64 MustBeZero3 : 12; // Must Be Zero
> + UINT32 Present : 1; // 0 = Not present in memory, 1
> = Present
> in memory
> + UINT32 MustBeZero : 2; // Must Be Zero
> + UINT32 WriteThrough : 1; // 0 = Write-Back caching,
> 1=Write-
> Through caching
> + UINT32 CacheDisabled : 1; // 0 = Cached, 1=Non-Cached
> + UINT32 MustBeZero2 : 4; // Must Be Zero
> + UINT32 Available : 3; // Ignored
> + UINT32 PageTableBaseAddressLow : 20; // Page Table Base Address
> Low
> +
> + UINT32 PageTableBaseAddressHigh : 20; // Page Table Base Address
> High
> + UINT32 MustBeZero3 : 12; // Must Be Zero
> } Bits;
> UINT64 Uint64;
> } IA32_PDPTE_PAE;
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 797fc2f600..982652b58b 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -26,7 +26,7 @@ PageTableLibSetPte4K (
> IN IA32_MAP_ATTRIBUTE *Mask
> )
> {
> - if (Mask->Bits.PageTableBaseAddress) {
> + if (Mask->Bits.PageTableBaseAddressLow || Mask-
> >Bits.PageTableBaseAddressHigh) {
> Pte4K->Uint64 = (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS
> (Attribute) + Offset) | (Pte4K->Uint64 &
> ~IA32_PE_BASE_ADDRESS_MASK_40);
> }
>
> @@ -93,7 +93,7 @@ PageTableLibSetPleB (
> IN IA32_MAP_ATTRIBUTE *Mask
> )
> {
> - if (Mask->Bits.PageTableBaseAddress) {
> + if (Mask->Bits.PageTableBaseAddressLow || Mask-
> >Bits.PageTableBaseAddressHigh) {
> PleB->Uint64 = (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS
> (Attribute) + Offset) | (PleB->Uint64 &
> ~IA32_PE_BASE_ADDRESS_MASK_39);
> }
>
> @@ -238,7 +238,7 @@ IsAttributesAndMaskValidForNonPresentEntry (
> //
> if ((Mask->Bits.ReadWrite == 0) || (Mask->Bits.UserSupervisor == 0) ||
> (Mask->Bits.WriteThrough == 0) || (Mask->Bits.CacheDisabled == 0) ||
> (Mask->Bits.Accessed == 0) || (Mask->Bits.Dirty == 0) || (Mask-
> >Bits.Pat == 0) || (Mask->Bits.Global == 0) ||
> - (Mask->Bits.PageTableBaseAddress == 0) || (Mask->Bits.ProtectionKey
> == 0) || (Mask->Bits.Nx == 0))
> + ((Mask->Bits.PageTableBaseAddressLow == 0) && (Mask-
> >Bits.PageTableBaseAddressHigh == 0)) || (Mask->Bits.ProtectionKey == 0)
> || (Mask->Bits.Nx == 0))
> {
> return RETURN_INVALID_PARAMETER;
> }
> @@ -396,7 +396,7 @@ PageTableLibMapInLevel (
> // This function is called when the memory length is less than the
> region
> length of the parent level.
> // No need to split the page when the attributes equal.
> //
> - if (Mask->Bits.PageTableBaseAddress == 0) {
> + if ((Mask->Bits.PageTableBaseAddressLow == 0) && (Mask-
> >Bits.PageTableBaseAddressHigh == 0)) {
> return RETURN_SUCCESS;
> }
>
> @@ -696,7 +696,7 @@ PageTableMap (
> return RETURN_INVALID_PARAMETER;
> }
>
> - if ((LinearAddress % SIZE_4KB != 0) || (Length % SIZE_4KB != 0)) {
> + if (((UINTN)LinearAddress % SIZE_4KB != 0) || ((UINTN)Length %
> SIZE_4KB != 0)) {
> //
> // LinearAddress and Length should be multiple of 4K.
> //
> @@ -738,12 +738,12 @@ PageTableMap (
> *IsModified = FALSE;
> }
>
> - ParentAttribute.Uint64 = 0;
> - ParentAttribute.Bits.PageTableBaseAddress = 1;
> - ParentAttribute.Bits.Present = 1;
> - ParentAttribute.Bits.ReadWrite = 1;
> - ParentAttribute.Bits.UserSupervisor = 1;
> - ParentAttribute.Bits.Nx = 0;
> + ParentAttribute.Uint64 = 0;
> + ParentAttribute.Bits.PageTableBaseAddressLow = 1;
> + ParentAttribute.Bits.Present = 1;
> + ParentAttribute.Bits.ReadWrite = 1;
> + ParentAttribute.Bits.UserSupervisor = 1;
> + ParentAttribute.Bits.Nx = 0;
>
> //
> // Query the required buffer size without modifying the page table.
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101684): https://edk2.groups.io/g/devel/message/101684
Mute This Topic: https://groups.io/mt/97796393/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
