public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "duntan" <dun.tan@intel.com>
To: devel@edk2.groups.io
Cc: Ray Ni <ray.ni@intel.com>, Laszlo Ersek <lersek@redhat.com>,
	Rahul Kumar <rahul1.kumar@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>
Subject: [edk2-devel] [Patch V2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm:Map SMRAM in 4K page granularity
Date: Tue,  6 Feb 2024 09:57:57 +0800	[thread overview]
Message-ID: <20240206015757.1816-4-dun.tan@intel.com> (raw)
In-Reply-To: <20240206015757.1816-1-dun.tan@intel.com>

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]
-=-=-=-=-=-=-=-=-=-=-=-



  parent reply	other threads:[~2024-02-06  1:58 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

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=20240206015757.1816-4-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