public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization
@ 2024-02-06  1:57 duntan
  2024-02-06  1:57 ` [edk2-devel] [Patch V2 1/3] UefiCpuPkg: Reduce and optimize access to attribute duntan
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: duntan @ 2024-02-06  1:57 UTC (permalink / raw)
  To: devel

In V2 patch set, only the order of 2 lines of code was adjusted.

Original description:
This patch set is to fix potential issue in CpuPageTableLib and SMM page table initialization

The first patch "UefiCpuPkg: Reduce and optimize access to attribute" is to reduce and optimize access to
attribute in page table modification code in CpuPageTableLib. 

The patch "UefiCpuPkg: Add more Paging mode enumeration" and " UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity"
is to map SMRAM in 4K page granularity during SMM page table initialization(SmmInitPageTable).
This is to avoid the SMRAM paging-structure layout change(page split) when SMM ready to lock (PerformRemainingTasks).

Dun Tan (2):
  UefiCpuPkg: Add more Paging mode enumeration
  UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity

Zhou Jianfeng (1):
  UefiCpuPkg: Reduce and optimize access to attribute

 UefiCpuPkg/Include/Library/CpuPageTableLib.h         |   8 +++++++-
 UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c |  86 +++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c   | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------
 3 files changed, 152 insertions(+), 58 deletions(-)

-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115135): https://edk2.groups.io/g/devel/message/115135
Mute This Topic: https://groups.io/mt/104191038/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [edk2-devel] [Patch V2 1/3] UefiCpuPkg: Reduce and optimize access to attribute
  2024-02-06  1:57 [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization duntan
@ 2024-02-06  1:57 ` duntan
  2024-02-06  1:57 ` [edk2-devel] [Patch V2 2/3] UefiCpuPkg: Add more Paging mode enumeration duntan
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: duntan @ 2024-02-06  1:57 UTC (permalink / raw)
  To: devel; +Cc: Zhou Jianfeng, Ray Ni, Laszlo Ersek, Rahul Kumar, Gerd Hoffmann

From: Zhou Jianfeng <jianfeng.zhou@intel.com>

This commit is to reduce and optimize access to
attribute in CpuPageTableLib.

Unreasonable writing to attribute of page table may
leads to expection.
The assembly code for C code Pnle->Bits.Present =
Attribute->Bits.Present looks like:
   and dword [rcx], 0xfffffffe
   and eax, 0x1
   or [rcx], eax
In case Pnle->Bits.Present and Attribute->Bits.Present
is 1, Pnle->Bits.Present will be set to 0 for short
time(2 instructions) which is unexpected. If some other
core is accessing the page, it may leads to expection.
This change reduce and optimize access to attribute of
page table, attribute of page table is set only when it
need to be changed.

Signed-off-by: Zhou Jianfeng <jianfeng.zhou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------
 1 file changed, 53 insertions(+), 33 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
index 36b2c4e6a3..ae4caf8dfe 100644
--- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
+++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
@@ -26,52 +26,59 @@ PageTableLibSetPte4K (
   IN IA32_MAP_ATTRIBUTE  *Mask
   )
 {
+  IA32_PTE_4K  LocalPte4K;
+
+  LocalPte4K.Uint64 = Pte4K->Uint64;
   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);
+    LocalPte4K.Uint64 = (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribute) + Offset) | (Pte4K->Uint64 & ~IA32_PE_BASE_ADDRESS_MASK_40);
   }
 
   if (Mask->Bits.Present) {
-    Pte4K->Bits.Present = Attribute->Bits.Present;
+    LocalPte4K.Bits.Present = Attribute->Bits.Present;
   }
 
   if (Mask->Bits.ReadWrite) {
-    Pte4K->Bits.ReadWrite = Attribute->Bits.ReadWrite;
+    LocalPte4K.Bits.ReadWrite = Attribute->Bits.ReadWrite;
   }
 
   if (Mask->Bits.UserSupervisor) {
-    Pte4K->Bits.UserSupervisor = Attribute->Bits.UserSupervisor;
+    LocalPte4K.Bits.UserSupervisor = Attribute->Bits.UserSupervisor;
   }
 
   if (Mask->Bits.WriteThrough) {
-    Pte4K->Bits.WriteThrough = Attribute->Bits.WriteThrough;
+    LocalPte4K.Bits.WriteThrough = Attribute->Bits.WriteThrough;
   }
 
   if (Mask->Bits.CacheDisabled) {
-    Pte4K->Bits.CacheDisabled = Attribute->Bits.CacheDisabled;
+    LocalPte4K.Bits.CacheDisabled = Attribute->Bits.CacheDisabled;
   }
 
   if (Mask->Bits.Accessed) {
-    Pte4K->Bits.Accessed = Attribute->Bits.Accessed;
+    LocalPte4K.Bits.Accessed = Attribute->Bits.Accessed;
   }
 
   if (Mask->Bits.Dirty) {
-    Pte4K->Bits.Dirty = Attribute->Bits.Dirty;
+    LocalPte4K.Bits.Dirty = Attribute->Bits.Dirty;
   }
 
   if (Mask->Bits.Pat) {
-    Pte4K->Bits.Pat = Attribute->Bits.Pat;
+    LocalPte4K.Bits.Pat = Attribute->Bits.Pat;
   }
 
   if (Mask->Bits.Global) {
-    Pte4K->Bits.Global = Attribute->Bits.Global;
+    LocalPte4K.Bits.Global = Attribute->Bits.Global;
   }
 
   if (Mask->Bits.ProtectionKey) {
-    Pte4K->Bits.ProtectionKey = Attribute->Bits.ProtectionKey;
+    LocalPte4K.Bits.ProtectionKey = Attribute->Bits.ProtectionKey;
   }
 
   if (Mask->Bits.Nx) {
-    Pte4K->Bits.Nx = Attribute->Bits.Nx;
+    LocalPte4K.Bits.Nx = Attribute->Bits.Nx;
+  }
+
+  if (Pte4K->Uint64 != LocalPte4K.Uint64) {
+    Pte4K->Uint64 = LocalPte4K.Uint64;
   }
 }
 
@@ -93,54 +100,61 @@ PageTableLibSetPleB (
   IN IA32_MAP_ATTRIBUTE                 *Mask
   )
 {
+  IA32_PAGE_LEAF_ENTRY_BIG_PAGESIZE  LocalPleB;
+
+  LocalPleB.Uint64 = PleB->Uint64;
   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);
+    LocalPleB.Uint64 = (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribute) + Offset) | (PleB->Uint64 & ~IA32_PE_BASE_ADDRESS_MASK_39);
   }
 
-  PleB->Bits.MustBeOne = 1;
+  LocalPleB.Bits.MustBeOne = 1;
 
   if (Mask->Bits.Present) {
-    PleB->Bits.Present = Attribute->Bits.Present;
+    LocalPleB.Bits.Present = Attribute->Bits.Present;
   }
 
   if (Mask->Bits.ReadWrite) {
-    PleB->Bits.ReadWrite = Attribute->Bits.ReadWrite;
+    LocalPleB.Bits.ReadWrite = Attribute->Bits.ReadWrite;
   }
 
   if (Mask->Bits.UserSupervisor) {
-    PleB->Bits.UserSupervisor = Attribute->Bits.UserSupervisor;
+    LocalPleB.Bits.UserSupervisor = Attribute->Bits.UserSupervisor;
   }
 
   if (Mask->Bits.WriteThrough) {
-    PleB->Bits.WriteThrough = Attribute->Bits.WriteThrough;
+    LocalPleB.Bits.WriteThrough = Attribute->Bits.WriteThrough;
   }
 
   if (Mask->Bits.CacheDisabled) {
-    PleB->Bits.CacheDisabled = Attribute->Bits.CacheDisabled;
+    LocalPleB.Bits.CacheDisabled = Attribute->Bits.CacheDisabled;
   }
 
   if (Mask->Bits.Accessed) {
-    PleB->Bits.Accessed = Attribute->Bits.Accessed;
+    LocalPleB.Bits.Accessed = Attribute->Bits.Accessed;
   }
 
   if (Mask->Bits.Dirty) {
-    PleB->Bits.Dirty = Attribute->Bits.Dirty;
+    LocalPleB.Bits.Dirty = Attribute->Bits.Dirty;
   }
 
   if (Mask->Bits.Pat) {
-    PleB->Bits.Pat = Attribute->Bits.Pat;
+    LocalPleB.Bits.Pat = Attribute->Bits.Pat;
   }
 
   if (Mask->Bits.Global) {
-    PleB->Bits.Global = Attribute->Bits.Global;
+    LocalPleB.Bits.Global = Attribute->Bits.Global;
   }
 
   if (Mask->Bits.ProtectionKey) {
-    PleB->Bits.ProtectionKey = Attribute->Bits.ProtectionKey;
+    LocalPleB.Bits.ProtectionKey = Attribute->Bits.ProtectionKey;
   }
 
   if (Mask->Bits.Nx) {
-    PleB->Bits.Nx = Attribute->Bits.Nx;
+    LocalPleB.Bits.Nx = Attribute->Bits.Nx;
+  }
+
+  if (PleB->Uint64 != LocalPleB.Uint64) {
+    PleB->Uint64 = LocalPleB.Uint64;
   }
 }
 
@@ -186,24 +200,27 @@ PageTableLibSetPnle (
   IN IA32_MAP_ATTRIBUTE        *Mask
   )
 {
+  IA32_PAGE_NON_LEAF_ENTRY  LocalPnle;
+
+  LocalPnle.Uint64 = Pnle->Uint64;
   if (Mask->Bits.Present) {
-    Pnle->Bits.Present = Attribute->Bits.Present;
+    LocalPnle.Bits.Present = Attribute->Bits.Present;
   }
 
   if (Mask->Bits.ReadWrite) {
-    Pnle->Bits.ReadWrite = Attribute->Bits.ReadWrite;
+    LocalPnle.Bits.ReadWrite = Attribute->Bits.ReadWrite;
   }
 
   if (Mask->Bits.UserSupervisor) {
-    Pnle->Bits.UserSupervisor = Attribute->Bits.UserSupervisor;
+    LocalPnle.Bits.UserSupervisor = Attribute->Bits.UserSupervisor;
   }
 
   if (Mask->Bits.Nx) {
-    Pnle->Bits.Nx = Attribute->Bits.Nx;
+    LocalPnle.Bits.Nx = Attribute->Bits.Nx;
   }
 
-  Pnle->Bits.Accessed   = 0;
-  Pnle->Bits.MustBeZero = 0;
+  LocalPnle.Bits.Accessed   = 0;
+  LocalPnle.Bits.MustBeZero = 0;
 
   //
   // Set the attributes (WT, CD, A) to 0.
@@ -211,8 +228,11 @@ PageTableLibSetPnle (
   // So, it implictly requires PAT[0] is Write Back.
   // Create a new parameter if caller requires to use a different memory type for accessing page directories.
   //
-  Pnle->Bits.WriteThrough  = 0;
-  Pnle->Bits.CacheDisabled = 0;
+  LocalPnle.Bits.WriteThrough  = 0;
+  LocalPnle.Bits.CacheDisabled = 0;
+  if (Pnle->Uint64 != LocalPnle.Uint64) {
+    Pnle->Uint64 = LocalPnle.Uint64;
+  }
 }
 
 /**
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115136): https://edk2.groups.io/g/devel/message/115136
Mute This Topic: https://groups.io/mt/104191039/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [edk2-devel] [Patch V2 2/3] UefiCpuPkg: Add more Paging mode enumeration
  2024-02-06  1:57 [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization duntan
  2024-02-06  1:57 ` [edk2-devel] [Patch V2 1/3] UefiCpuPkg: Reduce and optimize access to attribute duntan
@ 2024-02-06  1:57 ` duntan
  2024-02-06  1:57 ` [edk2-devel] [Patch V2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity duntan
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: duntan @ 2024-02-06  1:57 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Laszlo Ersek, Rahul Kumar, Gerd Hoffmann

Add more Paging mode enumeration in CpuPageTableLib
to support forced mapping a range in 4K page
granularity.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 UefiCpuPkg/Include/Library/CpuPageTableLib.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Include/Library/CpuPageTableLib.h b/UefiCpuPkg/Include/Library/CpuPageTableLib.h
index 78aa83b8de..6225a1d8e9 100644
--- a/UefiCpuPkg/Include/Library/CpuPageTableLib.h
+++ b/UefiCpuPkg/Include/Library/CpuPageTableLib.h
@@ -46,11 +46,17 @@ typedef enum {
   // High byte in paging mode indicates the max levels of the page table.
   // Low byte in paging mode indicates the max level that can be a leaf entry.
   //
-  PagingPae = 0x0302,
+  PagingPae4KB = 0x0301,
+  PagingPae2MB = 0x0302,
+  PagingPae    = 0x0302,
 
+  Paging4Level4KB = 0x0401,
+  Paging4Level2MB = 0x0402,
   Paging4Level    = 0x0402,
   Paging4Level1GB = 0x0403,
 
+  Paging5Level4KB = 0x0501,
+  Paging5Level2MB = 0x0502,
   Paging5Level    = 0x0502,
   Paging5Level1GB = 0x0503,
 
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115137): https://edk2.groups.io/g/devel/message/115137
Mute This Topic: https://groups.io/mt/104191040/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [edk2-devel] [Patch V2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity
  2024-02-06  1:57 [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization duntan
  2024-02-06  1:57 ` [edk2-devel] [Patch V2 1/3] UefiCpuPkg: Reduce and optimize access to attribute duntan
  2024-02-06  1:57 ` [edk2-devel] [Patch V2 2/3] UefiCpuPkg: Add more Paging mode enumeration duntan
@ 2024-02-06  1:57 ` duntan
  2024-02-06  2:27 ` [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization Ni, Ray
  2024-02-06  4:52 ` Wu, Jiaxin
  4 siblings, 0 replies; 7+ messages in thread
From: duntan @ 2024-02-06  1:57 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Laszlo Ersek, Rahul Kumar, Gerd Hoffmann

This patch is to map SMRAM in 4K page granularity
during SMM page table initialization(SmmInitPageTable)
so as to avoid the SMRAM paging-structure layout
change when SMI happens (PerformRemainingTasks).
The reason is to avoid the Paging-Structure change
impact to the multiple Processors. Refer SDM section
"4.10.4" & "4.10.5".

Currently, SMM BSP needs to update the SMRAM range
paging attribute in smm page table according to the
SmmMemoryAttributesTable when SMM ready to lock
happens. If the SMRAM range is not 4k mapped in page
table, the page table update process may split 1G/2M
paging entries to 4k ones.Meanwhile, all APs are still
running in SMI, which might access the affected
linear-address range between the time of modification
and the time of invalidation access. That will be
a potential problem leading exception happens.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 92 insertions(+), 24 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 12f3c0b8e8..b8c356bfe8 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -1647,49 +1647,115 @@ EdkiiSmmClearMemoryAttributes (
 }
 
 /**
-  Create page table based on input PagingMode and PhysicalAddressBits in smm.
-
-  @param[in]      PagingMode           The paging mode.
-  @param[in]      PhysicalAddressBits  The bits of physical address to map.
+  Create page table based on input PagingMode, LinearAddress and Length.
 
-  @retval         PageTable Address
+  @param[in, out]  PageTable           The pointer to the page table.
+  @param[in]       PagingMode          The paging mode.
+  @param[in]       LinearAddress       The start of the linear address range.
+  @param[in]       Length              The length of the linear address range.
 
 **/
-UINTN
-GenSmmPageTable (
-  IN PAGING_MODE  PagingMode,
-  IN UINT8        PhysicalAddressBits
+VOID
+GenPageTable (
+  IN OUT UINTN        *PageTable,
+  IN     PAGING_MODE  PagingMode,
+  IN     UINT64       LinearAddress,
+  IN     UINT64       Length
   )
 {
+  RETURN_STATUS       Status;
   UINTN               PageTableBufferSize;
-  UINTN               PageTable;
   VOID                *PageTableBuffer;
   IA32_MAP_ATTRIBUTE  MapAttribute;
   IA32_MAP_ATTRIBUTE  MapMask;
-  RETURN_STATUS       Status;
-  UINTN               GuardPage;
-  UINTN               Index;
-  UINT64              Length;
 
-  Length                           = LShiftU64 (1, PhysicalAddressBits);
-  PageTable                        = 0;
-  PageTableBufferSize              = 0;
   MapMask.Uint64                   = MAX_UINT64;
-  MapAttribute.Uint64              = mAddressEncMask;
+  MapAttribute.Uint64              = mAddressEncMask|LinearAddress;
   MapAttribute.Bits.Present        = 1;
   MapAttribute.Bits.ReadWrite      = 1;
   MapAttribute.Bits.UserSupervisor = 1;
   MapAttribute.Bits.Accessed       = 1;
   MapAttribute.Bits.Dirty          = 1;
+  PageTableBufferSize              = 0;
+
+  Status = PageTableMap (
+             PageTable,
+             PagingMode,
+             NULL,
+             &PageTableBufferSize,
+             LinearAddress,
+             Length,
+             &MapAttribute,
+             &MapMask,
+             NULL
+             );
+  if (Status == RETURN_BUFFER_TOO_SMALL) {
+    DEBUG ((DEBUG_INFO, "GenSMMPageTable: 0x%x bytes needed for initial SMM page table\n", PageTableBufferSize));
+    PageTableBuffer = AllocatePageTableMemory (EFI_SIZE_TO_PAGES (PageTableBufferSize));
+    ASSERT (PageTableBuffer != NULL);
+    Status = PageTableMap (
+               PageTable,
+               PagingMode,
+               PageTableBuffer,
+               &PageTableBufferSize,
+               LinearAddress,
+               Length,
+               &MapAttribute,
+               &MapMask,
+               NULL
+               );
+  }
 
-  Status = PageTableMap (&PageTable, PagingMode, NULL, &PageTableBufferSize, 0, Length, &MapAttribute, &MapMask, NULL);
-  ASSERT (Status == RETURN_BUFFER_TOO_SMALL);
-  DEBUG ((DEBUG_INFO, "GenSMMPageTable: 0x%x bytes needed for initial SMM page table\n", PageTableBufferSize));
-  PageTableBuffer = AllocatePageTableMemory (EFI_SIZE_TO_PAGES (PageTableBufferSize));
-  ASSERT (PageTableBuffer != NULL);
-  Status = PageTableMap (&PageTable, PagingMode, PageTableBuffer, &PageTableBufferSize, 0, Length, &MapAttribute, &MapMask, NULL);
   ASSERT (Status == RETURN_SUCCESS);
   ASSERT (PageTableBufferSize == 0);
+}
+
+/**
+  Create page table based on input PagingMode and PhysicalAddressBits in smm.
+
+  @param[in]      PagingMode           The paging mode.
+  @param[in]      PhysicalAddressBits  The bits of physical address to map.
+
+  @retval         PageTable Address
+
+**/
+UINTN
+GenSmmPageTable (
+  IN PAGING_MODE  PagingMode,
+  IN UINT8        PhysicalAddressBits
+  )
+{
+  UINTN          PageTable;
+  RETURN_STATUS  Status;
+  UINTN          GuardPage;
+  UINTN          Index;
+  UINT64         Length;
+  PAGING_MODE    SmramPagingMode;
+
+  PageTable = 0;
+  Length    = LShiftU64 (1, PhysicalAddressBits);
+  ASSERT (Length > mCpuHotPlugData.SmrrBase + mCpuHotPlugData.SmrrSize);
+
+  if (sizeof (UINTN) == sizeof (UINT64)) {
+    SmramPagingMode = m5LevelPagingNeeded ? Paging5Level4KB : Paging4Level4KB;
+  } else {
+    SmramPagingMode = PagingPae4KB;
+  }
+
+  ASSERT (mCpuHotPlugData.SmrrBase % SIZE_4KB == 0);
+  ASSERT (mCpuHotPlugData.SmrrSize % SIZE_4KB == 0);
+  GenPageTable (&PageTable, PagingMode, 0, mCpuHotPlugData.SmrrBase);
+
+  //
+  // Map smram range in 4K page granularity to avoid subsequent page split when smm ready to lock.
+  // If BSP are splitting the 1G/2M paging entries to 512 2M/4K paging entries, and all APs are
+  // still running in SMI at the same time, which might access the affected linear-address range
+  // between the time of modification and the time of invalidation access. That will be a potential
+  // problem leading exception happen.
+  //
+  GenPageTable (&PageTable, SmramPagingMode, mCpuHotPlugData.SmrrBase, mCpuHotPlugData.SmrrSize);
+
+  GenPageTable (&PageTable, PagingMode, mCpuHotPlugData.SmrrBase + mCpuHotPlugData.SmrrSize, Length - mCpuHotPlugData.SmrrBase - mCpuHotPlugData.SmrrSize);
 
   if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
     //
@@ -1698,6 +1764,7 @@ GenSmmPageTable (
     for (Index = 0; Index < gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus; Index++) {
       GuardPage = mSmmStackArrayBase + EFI_PAGE_SIZE + Index * (mSmmStackSize + mSmmShadowStackSize);
       Status    = ConvertMemoryPageAttributes (PageTable, PagingMode, GuardPage, SIZE_4KB, EFI_MEMORY_RP, TRUE, NULL);
+      ASSERT (Status == RETURN_SUCCESS);
     }
   }
 
@@ -1706,6 +1773,7 @@ GenSmmPageTable (
     // Mark [0, 4k] as non-present
     //
     Status = ConvertMemoryPageAttributes (PageTable, PagingMode, 0, SIZE_4KB, EFI_MEMORY_RP, TRUE, NULL);
+    ASSERT (Status == RETURN_SUCCESS);
   }
 
   return (UINTN)PageTable;
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115138): https://edk2.groups.io/g/devel/message/115138
Mute This Topic: https://groups.io/mt/104191041/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization
  2024-02-06  1:57 [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization duntan
                   ` (2 preceding siblings ...)
  2024-02-06  1:57 ` [edk2-devel] [Patch V2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity duntan
@ 2024-02-06  2:27 ` Ni, Ray
  2024-02-06  4:52 ` Wu, Jiaxin
  4 siblings, 0 replies; 7+ messages in thread
From: Ni, Ray @ 2024-02-06  2:27 UTC (permalink / raw)
  To: Gao, Liming, Kinney, Michael D; +Cc: Tan, Dun, devel@edk2.groups.io

Liming, Mike,
I just realized soft freeze started yesterday.
I would like to merge the 3 patches as they fix a critical SMM hang issue found in reboot cycling test.

Thanks,
Ray
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> Sent: Tuesday, February 6, 2024 9:58 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib
> and SMM page table initialization
> 
> In V2 patch set, only the order of 2 lines of code was adjusted.
> 
> Original description:
> This patch set is to fix potential issue in CpuPageTableLib and SMM page table
> initialization
> 
> The first patch "UefiCpuPkg: Reduce and optimize access to attribute" is to
> reduce and optimize access to
> attribute in page table modification code in CpuPageTableLib.
> 
> The patch "UefiCpuPkg: Add more Paging mode enumeration" and "
> UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity"
> is to map SMRAM in 4K page granularity during SMM page table
> initialization(SmmInitPageTable).
> This is to avoid the SMRAM paging-structure layout change(page split) when
> SMM ready to lock (PerformRemainingTasks).
> 
> Dun Tan (2):
>   UefiCpuPkg: Add more Paging mode enumeration
>   UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity
> 
> Zhou Jianfeng (1):
>   UefiCpuPkg: Reduce and optimize access to attribute
> 
>  UefiCpuPkg/Include/Library/CpuPageTableLib.h         |   8 +++++++-
>  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c |  86
> +++++++++++++++++++++++++++++++++++++++++++++++++++++---------
> ------------------------
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c   | 116
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++------------------------
>  3 files changed, 152 insertions(+), 58 deletions(-)
> 
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115139): https://edk2.groups.io/g/devel/message/115139
Mute This Topic: https://groups.io/mt/104191038/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization
  2024-02-06  1:57 [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization duntan
                   ` (3 preceding siblings ...)
  2024-02-06  2:27 ` [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization Ni, Ray
@ 2024-02-06  4:52 ` Wu, Jiaxin
  2024-02-06  8:53   ` Ni, Ray
  4 siblings, 1 reply; 7+ messages in thread
From: Wu, Jiaxin @ 2024-02-06  4:52 UTC (permalink / raw)
  To: devel@edk2.groups.io, Tan, Dun

Series Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> Sent: Tuesday, February 6, 2024 9:58 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib
> and SMM page table initialization
> 
> In V2 patch set, only the order of 2 lines of code was adjusted.
> 
> Original description:
> This patch set is to fix potential issue in CpuPageTableLib and SMM page table
> initialization
> 
> The first patch "UefiCpuPkg: Reduce and optimize access to attribute" is to
> reduce and optimize access to
> attribute in page table modification code in CpuPageTableLib.
> 
> The patch "UefiCpuPkg: Add more Paging mode enumeration" and "
> UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity"
> is to map SMRAM in 4K page granularity during SMM page table
> initialization(SmmInitPageTable).
> This is to avoid the SMRAM paging-structure layout change(page split) when
> SMM ready to lock (PerformRemainingTasks).
> 
> Dun Tan (2):
>   UefiCpuPkg: Add more Paging mode enumeration
>   UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity
> 
> Zhou Jianfeng (1):
>   UefiCpuPkg: Reduce and optimize access to attribute
> 
>  UefiCpuPkg/Include/Library/CpuPageTableLib.h         |   8 +++++++-
>  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c |  86
> +++++++++++++++++++++++++++++++++++++++++++++++++++++---------
> ------------------------
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c   | 116
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++------------------------
>  3 files changed, 152 insertions(+), 58 deletions(-)
> 
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115143): https://edk2.groups.io/g/devel/message/115143
Mute This Topic: https://groups.io/mt/104191038/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization
  2024-02-06  4:52 ` Wu, Jiaxin
@ 2024-02-06  8:53   ` Ni, Ray
  0 siblings, 0 replies; 7+ messages in thread
From: Ni, Ray @ 2024-02-06  8:53 UTC (permalink / raw)
  To: devel@edk2.groups.io, Wu, Jiaxin, Tan, Dun

Merged through
https://github.com/tianocore/edk2/pull/5347

Thanks,
Ray
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu,
> Jiaxin
> Sent: Tuesday, February 6, 2024 12:53 PM
> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
> Subject: Re: [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib
> and SMM page table initialization
> 
> Series Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
> 
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> > Sent: Tuesday, February 6, 2024 9:58 AM
> > To: devel@edk2.groups.io
> > Subject: [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib
> > and SMM page table initialization
> >
> > In V2 patch set, only the order of 2 lines of code was adjusted.
> >
> > Original description:
> > This patch set is to fix potential issue in CpuPageTableLib and SMM page
> table
> > initialization
> >
> > The first patch "UefiCpuPkg: Reduce and optimize access to attribute" is to
> > reduce and optimize access to
> > attribute in page table modification code in CpuPageTableLib.
> >
> > The patch "UefiCpuPkg: Add more Paging mode enumeration" and "
> > UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity"
> > is to map SMRAM in 4K page granularity during SMM page table
> > initialization(SmmInitPageTable).
> > This is to avoid the SMRAM paging-structure layout change(page split) when
> > SMM ready to lock (PerformRemainingTasks).
> >
> > Dun Tan (2):
> >   UefiCpuPkg: Add more Paging mode enumeration
> >   UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity
> >
> > Zhou Jianfeng (1):
> >   UefiCpuPkg: Reduce and optimize access to attribute
> >
> >  UefiCpuPkg/Include/Library/CpuPageTableLib.h         |   8 +++++++-
> >  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c |  86
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> --
> > ------------------------
> >  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c   | 116
> >
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > ++++++++++++++++++++++++++++++++++------------------------
> >  3 files changed, 152 insertions(+), 58 deletions(-)
> >
> > --
> > 2.31.1.windows.1
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115152): https://edk2.groups.io/g/devel/message/115152
Mute This Topic: https://groups.io/mt/104191038/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-02-06  8:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-06  1:57 [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization duntan
2024-02-06  1:57 ` [edk2-devel] [Patch V2 1/3] UefiCpuPkg: Reduce and optimize access to attribute duntan
2024-02-06  1:57 ` [edk2-devel] [Patch V2 2/3] UefiCpuPkg: Add more Paging mode enumeration duntan
2024-02-06  1:57 ` [edk2-devel] [Patch V2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity duntan
2024-02-06  2:27 ` [edk2-devel] [Patch V2 0/3] Fix potential issue in CpuPageTableLib and SMM page table initialization Ni, Ray
2024-02-06  4:52 ` Wu, Jiaxin
2024-02-06  8:53   ` Ni, Ray

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox