Hi Ray,

Would you please provide feedback for this patch?

It is already reviewed by Laszlo here: https://edk2.groups.io/g/devel/message/74122. Could you please let me know what it takes after your review? I can send a v3 for reviewed-by tags if necessary.

Thanks in advance,
Kun

On 04/14/2021 13:25, Kun Qin via groups.io wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3300

Current implementation of SetStaticPageTable routine in PiSmmCpuDxeSmm
driver will check a global variable mPhysicalAddressBits, and eventually
cap any value larger than 39 at 39.

This global variable is used in ConvertMemoryPageAttributes, which backs
SmmSetMemoryAttributes and SmmClearMemoryAttributes. Thus for a processor
that supports more than 39 bits width, trying to mark page table regions
higher than 39-bit will always return EFI_UNSUPPROTED.

This change updated the interface of SetStaticPageTable function to take
PhysicalAddressBits as an input parameter, in order to avoid changing/
accessing the global variable.

Cc: Eric Dong <eric.d...@intel.com>
Cc: Ray Ni <ray...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Rahul Kumar <rahul1.ku...@intel.com>

Fixes: 4eee0cc7cc0db74489b99c19eba056b53eda6358
Signed-off-by: Kun Qin <kuqi...@gmail.com>
---

Notes:
     v2:
     - SetStaticPageTable interface update [Ray]
     - Commit message updates, variable type change [Laszlo]

  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 30 +++++++++++---------
  1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 6902584b1fbd..d6f8dd94d303 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -211,11 +211,13 @@ CalculateMaximumSupportAddress (
  /**
    Set static page table.
- @param[in] PageTable Address of page table.
+  @param[in] PageTable              Address of page table.
+  @param[in] PhysicalAddressBits    The maximum physical address bits 
supported.
  **/
  VOID
  SetStaticPageTable (
-  IN UINTN               PageTable
+  IN UINTN               PageTable,
+  IN UINT8               PhysicalAddressBits
    )
  {
    UINT64                                        PageAddress;
@@ -237,26 +239,26 @@ SetStaticPageTable (
    // IA-32e paging translates 48-bit linear addresses to 52-bit physical 
addresses
    //  when 5-Level Paging is disabled.
    //
-  ASSERT (mPhysicalAddressBits <= 52);
-  if (!m5LevelPagingNeeded && mPhysicalAddressBits > 48) {
-    mPhysicalAddressBits = 48;
+  ASSERT (PhysicalAddressBits <= 52);
+  if (!m5LevelPagingNeeded && PhysicalAddressBits > 48) {
+    PhysicalAddressBits = 48;
    }
NumberOfPml5EntriesNeeded = 1;
-  if (mPhysicalAddressBits > 48) {
-    NumberOfPml5EntriesNeeded = (UINTN) LShiftU64 (1, mPhysicalAddressBits - 
48);
-    mPhysicalAddressBits = 48;
+  if (PhysicalAddressBits > 48) {
+    NumberOfPml5EntriesNeeded = (UINTN) LShiftU64 (1, PhysicalAddressBits - 
48);
+    PhysicalAddressBits = 48;
    }
NumberOfPml4EntriesNeeded = 1;
-  if (mPhysicalAddressBits > 39) {
-    NumberOfPml4EntriesNeeded = (UINTN) LShiftU64 (1, mPhysicalAddressBits - 
39);
-    mPhysicalAddressBits = 39;
+  if (PhysicalAddressBits > 39) {
+    NumberOfPml4EntriesNeeded = (UINTN) LShiftU64 (1, PhysicalAddressBits - 
39);
+    PhysicalAddressBits = 39;
    }
NumberOfPdpEntriesNeeded = 1;
-  ASSERT (mPhysicalAddressBits > 30);
-  NumberOfPdpEntriesNeeded = (UINTN) LShiftU64 (1, mPhysicalAddressBits - 30);
+  ASSERT (PhysicalAddressBits > 30);
+  NumberOfPdpEntriesNeeded = (UINTN) LShiftU64 (1, PhysicalAddressBits - 30);
//
    // By architecture only one PageMapLevel4 exists - so lets allocate storage 
for it.
@@ -438,7 +440,7 @@ SmmInitPageTable (
      // When access to non-SMRAM memory is restricted, create page table
      // that covers all memory space.
      //
-    SetStaticPageTable ((UINTN)PTEntry);
+    SetStaticPageTable ((UINTN)PTEntry, mPhysicalAddressBits);
    } else {
      //
      // Add pages to page pool



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74280): https://edk2.groups.io/g/devel/message/74280
Mute This Topic: https://groups.io/mt/82214548/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to