public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "duntan" <dun.tan@intel.com>
To: devel@edk2.groups.io
Cc: Zhiguang Liu <zhiguang.liu@intel.com>,
	Eric Dong <eric.dong@intel.com>, Ray Ni <ray.ni@intel.com>,
	Rahul Kumar <rahul1.kumar@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>
Subject: [Patch V5 16/22] UefiCpuPkg: Fix IA32 build failure in CpuPageTableLib.inf
Date: Fri, 24 Mar 2023 14:00:14 +0800	[thread overview]
Message-ID: <20230324060020.940-17-dun.tan@intel.com> (raw)
In-Reply-To: <20230324060020.940-1-dun.tan@intel.com>

From: Zhiguang Liu <zhiguang.liu@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.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Tested-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Ray Ni <ray.ni@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 352b6df6c6..78aa83b8de 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 885f1601fc..3ea89aacaf 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);
   }
 
@@ -239,7 +239,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;
     }
@@ -399,7 +399,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;
       }
 
@@ -706,7 +706,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.
     //
@@ -750,12 +750,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


  parent reply	other threads:[~2023-03-24  6:01 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-24  5:59 [Patch V5 00/22] Fix issues in CpuPageTableLib duntan
2023-03-24  5:59 ` [Patch V5 01/22] UefiCpuPkg/CpuPageTableLib: Remove unneeded 'if' condition duntan
2023-03-24  6:00 ` [Patch V5 02/22] UefiCpuPkg/CpuPageTableLib: Add check for input Length duntan
2023-03-24  6:00 ` [Patch V5 03/22] UefiCpuPkg/CpuPageTableLib:Initialize some LocalVariable at beginning duntan
2023-03-24  6:00 ` [Patch V5 04/22] UefiCpuPkg/CpuPageTableLib: Fix the non-1:1 mapping issue duntan
2023-03-24  6:00 ` [Patch V5 05/22] UefiCpuPkg/CpuPageTableLib:Clear PageSize bit(Bit7) for non-leaf duntan
2023-03-24  6:00 ` [Patch V5 06/22] UefiCpuPkg/CpuPageTableLib: Fix issue when splitting leaf entry duntan
2023-03-24  6:00 ` [Patch V5 07/22] UefiCpuPkg/MpInitLib: Add code to initialize MapMask duntan
2023-03-24  6:00 ` [Patch V5 08/22] UefiCpuPkg/CpuPageTableLib:Add check for Mask and Attr duntan
2023-03-24  8:06   ` Ni, Ray
2023-03-24  6:00 ` [Patch V5 09/22] UefiCpuPkg/CpuPageTableLib: Add manual test to check " duntan
2023-03-24  8:06   ` Ni, Ray
2023-03-24  6:00 ` [Patch V5 10/22] UefiCpuPkg/CpuPageTableLib:Modify RandomBoolean() in RandomTest duntan
2023-03-24  6:00 ` [Patch V5 11/22] UefiCpuPkg/CpuPageTableLib: Add LastMapEntry pointer duntan
2023-03-24  8:07   ` Ni, Ray
2023-03-24  6:00 ` [Patch V5 12/22] UefiCpuPkg/CpuPageTableLib:Modify RandomTest to check Mask/Attr duntan
2023-03-24  8:07   ` Ni, Ray
2023-03-24  6:00 ` [Patch V5 13/22] UefiCpuPkg/CpuPageTableLib: Enable non-1:1 mapping in random test duntan
2023-03-24  6:00 ` [Patch V5 14/22] UefiCpuPkg/CpuPageTableLib: Add OUTPUT IsModified parameter duntan
2023-03-24  8:08   ` Ni, Ray
2023-03-24  6:00 ` [Patch V5 15/22] UefiCpuPkg/CpuPageTableLib: Modify RandomTest to check IsModified duntan
2023-03-24  6:00 ` duntan [this message]
2023-03-24  6:00 ` [Patch V5 17/22] UefiCpuPkg: Modify UnitTest code since tested API is changed duntan
2023-03-24  6:00 ` [Patch V5 18/22] UefiCpuPkg/CpuPageTableLib: Add check for page table creation duntan
2023-03-24  6:00 ` [Patch V5 19/22] UefiCpuPkg: Combine branch for non-present and leaf ParentEntry duntan
2023-03-24  8:09   ` Ni, Ray
2023-03-24  6:00 ` [Patch V5 20/22] UefiCpuPkg/CpuPageTableLib: Enable PAE paging duntan
2023-03-24  8:10   ` Ni, Ray
2023-03-24  6:00 ` [Patch V5 21/22] UefiCpuPkg/CpuPageTableLib: Add RandomTest for " duntan
2023-03-24  6:00 ` [Patch V5 22/22] UefiCpuPkg/CpuPageTableLib: Reduce the number of random tests duntan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230324060020.940-17-dun.tan@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox