public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V2 1/1] OvmfPkg/PeilessStartupLib: move mPageTablePool to stack
@ 2022-09-27  7:07 Min Xu
  2022-09-27  8:21 ` Yao, Jiewen
  0 siblings, 1 reply; 3+ messages in thread
From: Min Xu @ 2022-09-27  7:07 UTC (permalink / raw)
  To: devel; +Cc: Min M Xu, Erdem Aktas, Gerd Hoffmann, James Bottomley, Jiewen Yao

From: Min M Xu <min.m.xu@intel.com>

PeilessStartupLib is running in SEC phase. In this phase global variable
is not allowed to be modified. This patch moves mPageTablePool to stack
and pass it as input parameter between functions.

Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 .../PeilessStartupLib/X64/VirtualMemory.c     | 117 ++++++++++--------
 1 file changed, 68 insertions(+), 49 deletions(-)

diff --git a/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c b/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
index 6877e521e485..b444c052d1bf 100644
--- a/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
+++ b/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
@@ -21,11 +21,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/PlatformInitLib.h>
 #include "PageTables.h"
 
-//
-// Global variable to keep track current available memory used as page table.
-//
-PAGE_TABLE_POOL  *mPageTablePool = NULL;
-
 UINTN  mLevelShift[5] = {
   0,
   PAGING_L1_ADDRESS_SHIFT,
@@ -273,14 +268,17 @@ ToSplitPageTable (
   reserve at least another PAGE_TABLE_POOL_UNIT_PAGES. But usually this won't
   happen in practice.
 
-  @param PoolPages  The least page number of the pool to be created.
+  @param[in]      PoolPages      The least page number of the pool to be created.
+  @param[in, out] PageTablePool  Pointer of Pointer to the current available memory
+                                  used as page table.
 
   @retval TRUE    The pool is initialized successfully.
   @retval FALSE   The memory is out of resource.
 **/
 BOOLEAN
 InitializePageTablePool (
-  IN UINTN  PoolPages
+  IN UINTN                PoolPages,
+  IN OUT PAGE_TABLE_POOL  **PageTablePool
   )
 {
   VOID  *Buffer;
@@ -303,20 +301,20 @@ InitializePageTablePool (
   //
   // Link all pools into a list for easier track later.
   //
-  if (mPageTablePool == NULL) {
-    mPageTablePool           = Buffer;
-    mPageTablePool->NextPool = mPageTablePool;
+  if (*PageTablePool == NULL) {
+    *(UINT64 *)(UINTN)PageTablePool = (UINT64)(UINTN)Buffer;
+    (*PageTablePool)->NextPool      = *PageTablePool;
   } else {
-    ((PAGE_TABLE_POOL *)Buffer)->NextPool = mPageTablePool->NextPool;
-    mPageTablePool->NextPool              = Buffer;
-    mPageTablePool                        = Buffer;
+    ((PAGE_TABLE_POOL *)Buffer)->NextPool = (*PageTablePool)->NextPool;
+    (*PageTablePool)->NextPool            = Buffer;
+    *PageTablePool                        = Buffer;
   }
 
   //
   // Reserve one page for pool header.
   //
-  mPageTablePool->FreePages = PoolPages - 1;
-  mPageTablePool->Offset    = EFI_PAGES_TO_SIZE (1);
+  (*PageTablePool)->FreePages = PoolPages - 1;
+  (*PageTablePool)->Offset    = EFI_PAGES_TO_SIZE (1);
 
   return TRUE;
 }
@@ -333,14 +331,17 @@ InitializePageTablePool (
   If there is not enough memory remaining to satisfy the request, then NULL is
   returned.
 
-  @param  Pages                 The number of 4 KB pages to allocate.
+  @param[in]      Pages                 The number of 4 KB pages to allocate.
+  @param[in, out] PageTablePool         Pointer of pointer to the current available
+                                        memory used as page table.
 
   @return A pointer to the allocated buffer or NULL if allocation fails.
 
 **/
 VOID *
 AllocatePageTableMemory (
-  IN UINTN  Pages
+  IN UINTN                Pages,
+  IN OUT PAGE_TABLE_POOL  **PageTablePool
   )
 {
   VOID  *Buffer;
@@ -349,30 +350,31 @@ AllocatePageTableMemory (
     return NULL;
   }
 
-  DEBUG ((DEBUG_INFO, "AllocatePageTableMemory. mPageTablePool=%p, Pages=%d\n", mPageTablePool, Pages));
+  DEBUG ((DEBUG_INFO, "AllocatePageTableMemory. PageTablePool=%p, Pages=%d\n", *PageTablePool, Pages));
   //
   // Renew the pool if necessary.
   //
-  if ((mPageTablePool == NULL) ||
-      (Pages > mPageTablePool->FreePages))
+  if ((*PageTablePool == NULL) ||
+      (Pages > (*PageTablePool)->FreePages))
   {
-    if (!InitializePageTablePool (Pages)) {
+    if (!InitializePageTablePool (Pages, PageTablePool)) {
       return NULL;
     }
   }
 
-  Buffer = (UINT8 *)mPageTablePool + mPageTablePool->Offset;
+  Buffer = (UINT8 *)(*PageTablePool) + (*PageTablePool)->Offset;
 
-  mPageTablePool->Offset    += EFI_PAGES_TO_SIZE (Pages);
-  mPageTablePool->FreePages -= Pages;
+  (*PageTablePool)->Offset    += EFI_PAGES_TO_SIZE (Pages);
+  (*PageTablePool)->FreePages -= Pages;
 
   DEBUG ((
     DEBUG_INFO,
-    "%a:%a: Buffer=0x%Lx Pages=%ld\n",
+    "%a:%a: Buffer=0x%Lx Pages=%ld, PageTablePool=%p\n",
     gEfiCallerBaseName,
     __FUNCTION__,
     Buffer,
-    Pages
+    Pages,
+    *PageTablePool
     ));
 
   return Buffer;
@@ -385,6 +387,8 @@ AllocatePageTableMemory (
   @param[in, out] PageEntry2M           Pointer to 2M page entry.
   @param[in]      StackBase             Stack base address.
   @param[in]      StackSize             Stack size.
+  @param[in, out] PageTablePool         Pointer to the current available memory used as
+                                        page table.
 
 **/
 VOID
@@ -392,7 +396,8 @@ Split2MPageTo4K (
   IN EFI_PHYSICAL_ADDRESS  PhysicalAddress,
   IN OUT UINT64            *PageEntry2M,
   IN EFI_PHYSICAL_ADDRESS  StackBase,
-  IN UINTN                 StackSize
+  IN UINTN                 StackSize,
+  IN OUT PAGE_TABLE_POOL   *PageTablePool
   )
 {
   EFI_PHYSICAL_ADDRESS  PhysicalAddress4K;
@@ -401,7 +406,7 @@ Split2MPageTo4K (
 
   DEBUG ((DEBUG_INFO, "Split2MPageTo4K\n"));
 
-  PageTableEntry = AllocatePageTableMemory (1);
+  PageTableEntry = AllocatePageTableMemory (1, &PageTablePool);
 
   if (PageTableEntry == NULL) {
     ASSERT (FALSE);
@@ -448,6 +453,8 @@ Split2MPageTo4K (
   @param[in, out] PageEntry1G           Pointer to 1G page entry.
   @param[in]      StackBase             Stack base address.
   @param[in]      StackSize             Stack size.
+  @param[in, out] PageTablePool         Pointer to the current available memory used as
+                                        page table.
 
 **/
 VOID
@@ -455,14 +462,16 @@ Split1GPageTo2M (
   IN EFI_PHYSICAL_ADDRESS  PhysicalAddress,
   IN OUT UINT64            *PageEntry1G,
   IN EFI_PHYSICAL_ADDRESS  StackBase,
-  IN UINTN                 StackSize
+  IN UINTN                 StackSize,
+  IN OUT PAGE_TABLE_POOL   *PageTablePool
   )
 {
   EFI_PHYSICAL_ADDRESS  PhysicalAddress2M;
   UINTN                 IndexOfPageDirectoryEntries;
   PAGE_TABLE_ENTRY      *PageDirectoryEntry;
 
-  PageDirectoryEntry = AllocatePageTableMemory (1);
+  DEBUG ((DEBUG_INFO, "Split1GPageTo2M\n"));
+  PageDirectoryEntry = AllocatePageTableMemory (1, &PageTablePool);
 
   if (PageDirectoryEntry == NULL) {
     ASSERT (FALSE);
@@ -480,7 +489,7 @@ Split1GPageTo2M (
       //
       // Need to split this 2M page that covers NULL or stack range.
       //
-      Split2MPageTo4K (PhysicalAddress2M, (UINT64 *)PageDirectoryEntry, StackBase, StackSize);
+      Split2MPageTo4K (PhysicalAddress2M, (UINT64 *)PageDirectoryEntry, StackBase, StackSize, PageTablePool);
     } else {
       //
       // Fill in the Page Directory entries
@@ -496,16 +505,19 @@ Split1GPageTo2M (
 /**
   Set one page of page table pool memory to be read-only.
 
-  @param[in] PageTableBase    Base address of page table (CR3).
-  @param[in] Address          Start address of a page to be set as read-only.
-  @param[in] Level4Paging     Level 4 paging flag.
+  @param[in]      PageTableBase    Base address of page table (CR3).
+  @param[in]      Address          Start address of a page to be set as read-only.
+  @param[in]      Level4Paging     Level 4 paging flag.
+  @param[in, out] PageTablePool    Pointer to the current available memory used as
+                                   page table.
 
 **/
 VOID
 SetPageTablePoolReadOnly (
   IN  UINTN                 PageTableBase,
   IN  EFI_PHYSICAL_ADDRESS  Address,
-  IN  BOOLEAN               Level4Paging
+  IN  BOOLEAN               Level4Paging,
+  IN OUT PAGE_TABLE_POOL    *PageTablePool
   )
 {
   UINTN                 Index;
@@ -573,7 +585,8 @@ SetPageTablePoolReadOnly (
       //
       ASSERT (Level > 1);
 
-      NewPageTable = AllocatePageTableMemory (1);
+      DEBUG ((DEBUG_INFO, "SetPageTablePoolReadOnly\n"));
+      NewPageTable = AllocatePageTableMemory (1, &PageTablePool);
 
       if (NewPageTable == NULL) {
         ASSERT (FALSE);
@@ -604,14 +617,17 @@ SetPageTablePoolReadOnly (
 /**
   Prevent the memory pages used for page table from been overwritten.
 
-  @param[in] PageTableBase    Base address of page table (CR3).
-  @param[in] Level4Paging     Level 4 paging flag.
+  @param[in]      PageTableBase    Base address of page table (CR3).
+  @param[in]      Level4Paging     Level 4 paging flag.
+  @param[in, out] PageTablePool    Pointer to the current available memory used as
+                                   page table.
 
 **/
 VOID
 EnablePageTableProtection (
-  IN  UINTN    PageTableBase,
-  IN  BOOLEAN  Level4Paging
+  IN  UINTN               PageTableBase,
+  IN  BOOLEAN             Level4Paging,
+  IN OUT PAGE_TABLE_POOL  *PageTablePool
   )
 {
   PAGE_TABLE_POOL       *HeadPool;
@@ -621,7 +637,7 @@ EnablePageTableProtection (
 
   DEBUG ((DEBUG_INFO, "EnablePageTableProtection\n"));
 
-  if (mPageTablePool == NULL) {
+  if (PageTablePool == NULL) {
     return;
   }
 
@@ -632,10 +648,10 @@ EnablePageTableProtection (
   AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);
 
   //
-  // SetPageTablePoolReadOnly might update mPageTablePool. It's safer to
+  // SetPageTablePoolReadOnly might update PageTablePool. It's safer to
   // remember original one in advance.
   //
-  HeadPool = mPageTablePool;
+  HeadPool = PageTablePool;
   Pool     = HeadPool;
   do {
     Address  = (EFI_PHYSICAL_ADDRESS)(UINTN)Pool;
@@ -647,7 +663,7 @@ EnablePageTableProtection (
     // protection to them one by one.
     //
     while (PoolSize > 0) {
-      SetPageTablePoolReadOnly (PageTableBase, Address, Level4Paging);
+      SetPageTablePoolReadOnly (PageTableBase, Address, Level4Paging, PageTablePool);
       Address  += PAGE_TABLE_POOL_UNIT_SIZE;
       PoolSize -= PAGE_TABLE_POOL_UNIT_SIZE;
     }
@@ -700,6 +716,7 @@ CreateIdentityMappingPageTables (
   BOOLEAN                         Page1GSupport;
   PAGE_TABLE_1G_ENTRY             *PageDirectory1GEntry;
   IA32_CR4                        Cr4;
+  PAGE_TABLE_POOL                 *PageTablePool;
 
   //
   // Set PageMapLevel5Entry to suppress incorrect compiler/analyzer warnings
@@ -785,13 +802,14 @@ CreateIdentityMappingPageTables (
     (UINT64)TotalPagesNum
     ));
 
-  BigPageAddress = (UINTN)AllocatePageTableMemory (TotalPagesNum);
+  PageTablePool  = NULL;
+  BigPageAddress = (UINTN)AllocatePageTableMemory (TotalPagesNum, &PageTablePool);
   if (BigPageAddress == 0) {
     ASSERT (FALSE);
     return 0;
   }
 
-  DEBUG ((DEBUG_INFO, "BigPageAddress = 0x%llx\n", BigPageAddress));
+  DEBUG ((DEBUG_INFO, "BigPageAddress = 0x%llx, PageTablePool=%p\n", BigPageAddress, PageTablePool));
 
   //
   // By architecture only one PageMapLevel4 exists - so lets allocate storage for it.
@@ -856,7 +874,8 @@ CreateIdentityMappingPageTables (
               PageAddress,
               (UINT64 *)PageDirectory1GEntry,
               StackBase,
-              StackSize
+              StackSize,
+              PageTablePool
               );
           } else {
             //
@@ -892,7 +911,7 @@ CreateIdentityMappingPageTables (
               //
               // Need to split this 2M page that covers NULL or stack range.
               //
-              Split2MPageTo4K (PageAddress, (UINT64 *)PageDirectoryEntry, StackBase, StackSize);
+              Split2MPageTo4K (PageAddress, (UINT64 *)PageDirectoryEntry, StackBase, StackSize, PageTablePool);
             } else {
               //
               // Fill in the Page Directory entries
@@ -929,7 +948,7 @@ CreateIdentityMappingPageTables (
   // Protect the page table by marking the memory used for page table to be
   // read-only.
   //
-  EnablePageTableProtection ((UINTN)PageMap, TRUE);
+  EnablePageTableProtection ((UINTN)PageMap, TRUE, PageTablePool);
 
   return (UINTN)PageMap;
 }
-- 
2.29.2.windows.2


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

* Re: [PATCH V2 1/1] OvmfPkg/PeilessStartupLib: move mPageTablePool to stack
  2022-09-27  7:07 [PATCH V2 1/1] OvmfPkg/PeilessStartupLib: move mPageTablePool to stack Min Xu
@ 2022-09-27  8:21 ` Yao, Jiewen
  2022-09-28  2:03   ` Yao, Jiewen
  0 siblings, 1 reply; 3+ messages in thread
From: Yao, Jiewen @ 2022-09-27  8:21 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Aktas, Erdem, Gerd Hoffmann, James Bottomley

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Tuesday, September 27, 2022 3:08 PM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com>; James
> Bottomley <jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH V2 1/1] OvmfPkg/PeilessStartupLib: move mPageTablePool
> to stack
> 
> From: Min M Xu <min.m.xu@intel.com>
> 
> PeilessStartupLib is running in SEC phase. In this phase global variable
> is not allowed to be modified. This patch moves mPageTablePool to stack
> and pass it as input parameter between functions.
> 
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>  .../PeilessStartupLib/X64/VirtualMemory.c     | 117 ++++++++++--------
>  1 file changed, 68 insertions(+), 49 deletions(-)
> 
> diff --git a/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
> b/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
> index 6877e521e485..b444c052d1bf 100644
> --- a/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
> +++ b/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
> @@ -21,11 +21,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Library/PlatformInitLib.h>
>  #include "PageTables.h"
> 
> -//
> -// Global variable to keep track current available memory used as page
> table.
> -//
> -PAGE_TABLE_POOL  *mPageTablePool = NULL;
> -
>  UINTN  mLevelShift[5] = {
>    0,
>    PAGING_L1_ADDRESS_SHIFT,
> @@ -273,14 +268,17 @@ ToSplitPageTable (
>    reserve at least another PAGE_TABLE_POOL_UNIT_PAGES. But usually
> this won't
>    happen in practice.
> 
> -  @param PoolPages  The least page number of the pool to be created.
> +  @param[in]      PoolPages      The least page number of the pool to be
> created.
> +  @param[in, out] PageTablePool  Pointer of Pointer to the current
> available memory
> +                                  used as page table.
> 
>    @retval TRUE    The pool is initialized successfully.
>    @retval FALSE   The memory is out of resource.
>  **/
>  BOOLEAN
>  InitializePageTablePool (
> -  IN UINTN  PoolPages
> +  IN UINTN                PoolPages,
> +  IN OUT PAGE_TABLE_POOL  **PageTablePool
>    )
>  {
>    VOID  *Buffer;
> @@ -303,20 +301,20 @@ InitializePageTablePool (
>    //
>    // Link all pools into a list for easier track later.
>    //
> -  if (mPageTablePool == NULL) {
> -    mPageTablePool           = Buffer;
> -    mPageTablePool->NextPool = mPageTablePool;
> +  if (*PageTablePool == NULL) {
> +    *(UINT64 *)(UINTN)PageTablePool = (UINT64)(UINTN)Buffer;
> +    (*PageTablePool)->NextPool      = *PageTablePool;
>    } else {
> -    ((PAGE_TABLE_POOL *)Buffer)->NextPool = mPageTablePool->NextPool;
> -    mPageTablePool->NextPool              = Buffer;
> -    mPageTablePool                        = Buffer;
> +    ((PAGE_TABLE_POOL *)Buffer)->NextPool = (*PageTablePool)->NextPool;
> +    (*PageTablePool)->NextPool            = Buffer;
> +    *PageTablePool                        = Buffer;
>    }
> 
>    //
>    // Reserve one page for pool header.
>    //
> -  mPageTablePool->FreePages = PoolPages - 1;
> -  mPageTablePool->Offset    = EFI_PAGES_TO_SIZE (1);
> +  (*PageTablePool)->FreePages = PoolPages - 1;
> +  (*PageTablePool)->Offset    = EFI_PAGES_TO_SIZE (1);
> 
>    return TRUE;
>  }
> @@ -333,14 +331,17 @@ InitializePageTablePool (
>    If there is not enough memory remaining to satisfy the request, then
> NULL is
>    returned.
> 
> -  @param  Pages                 The number of 4 KB pages to allocate.
> +  @param[in]      Pages                 The number of 4 KB pages to allocate.
> +  @param[in, out] PageTablePool         Pointer of pointer to the current
> available
> +                                        memory used as page table.
> 
>    @return A pointer to the allocated buffer or NULL if allocation fails.
> 
>  **/
>  VOID *
>  AllocatePageTableMemory (
> -  IN UINTN  Pages
> +  IN UINTN                Pages,
> +  IN OUT PAGE_TABLE_POOL  **PageTablePool
>    )
>  {
>    VOID  *Buffer;
> @@ -349,30 +350,31 @@ AllocatePageTableMemory (
>      return NULL;
>    }
> 
> -  DEBUG ((DEBUG_INFO, "AllocatePageTableMemory. mPageTablePool=%p,
> Pages=%d\n", mPageTablePool, Pages));
> +  DEBUG ((DEBUG_INFO, "AllocatePageTableMemory. PageTablePool=%p,
> Pages=%d\n", *PageTablePool, Pages));
>    //
>    // Renew the pool if necessary.
>    //
> -  if ((mPageTablePool == NULL) ||
> -      (Pages > mPageTablePool->FreePages))
> +  if ((*PageTablePool == NULL) ||
> +      (Pages > (*PageTablePool)->FreePages))
>    {
> -    if (!InitializePageTablePool (Pages)) {
> +    if (!InitializePageTablePool (Pages, PageTablePool)) {
>        return NULL;
>      }
>    }
> 
> -  Buffer = (UINT8 *)mPageTablePool + mPageTablePool->Offset;
> +  Buffer = (UINT8 *)(*PageTablePool) + (*PageTablePool)->Offset;
> 
> -  mPageTablePool->Offset    += EFI_PAGES_TO_SIZE (Pages);
> -  mPageTablePool->FreePages -= Pages;
> +  (*PageTablePool)->Offset    += EFI_PAGES_TO_SIZE (Pages);
> +  (*PageTablePool)->FreePages -= Pages;
> 
>    DEBUG ((
>      DEBUG_INFO,
> -    "%a:%a: Buffer=0x%Lx Pages=%ld\n",
> +    "%a:%a: Buffer=0x%Lx Pages=%ld, PageTablePool=%p\n",
>      gEfiCallerBaseName,
>      __FUNCTION__,
>      Buffer,
> -    Pages
> +    Pages,
> +    *PageTablePool
>      ));
> 
>    return Buffer;
> @@ -385,6 +387,8 @@ AllocatePageTableMemory (
>    @param[in, out] PageEntry2M           Pointer to 2M page entry.
>    @param[in]      StackBase             Stack base address.
>    @param[in]      StackSize             Stack size.
> +  @param[in, out] PageTablePool         Pointer to the current available
> memory used as
> +                                        page table.
> 
>  **/
>  VOID
> @@ -392,7 +396,8 @@ Split2MPageTo4K (
>    IN EFI_PHYSICAL_ADDRESS  PhysicalAddress,
>    IN OUT UINT64            *PageEntry2M,
>    IN EFI_PHYSICAL_ADDRESS  StackBase,
> -  IN UINTN                 StackSize
> +  IN UINTN                 StackSize,
> +  IN OUT PAGE_TABLE_POOL   *PageTablePool
>    )
>  {
>    EFI_PHYSICAL_ADDRESS  PhysicalAddress4K;
> @@ -401,7 +406,7 @@ Split2MPageTo4K (
> 
>    DEBUG ((DEBUG_INFO, "Split2MPageTo4K\n"));
> 
> -  PageTableEntry = AllocatePageTableMemory (1);
> +  PageTableEntry = AllocatePageTableMemory (1, &PageTablePool);
> 
>    if (PageTableEntry == NULL) {
>      ASSERT (FALSE);
> @@ -448,6 +453,8 @@ Split2MPageTo4K (
>    @param[in, out] PageEntry1G           Pointer to 1G page entry.
>    @param[in]      StackBase             Stack base address.
>    @param[in]      StackSize             Stack size.
> +  @param[in, out] PageTablePool         Pointer to the current available
> memory used as
> +                                        page table.
> 
>  **/
>  VOID
> @@ -455,14 +462,16 @@ Split1GPageTo2M (
>    IN EFI_PHYSICAL_ADDRESS  PhysicalAddress,
>    IN OUT UINT64            *PageEntry1G,
>    IN EFI_PHYSICAL_ADDRESS  StackBase,
> -  IN UINTN                 StackSize
> +  IN UINTN                 StackSize,
> +  IN OUT PAGE_TABLE_POOL   *PageTablePool
>    )
>  {
>    EFI_PHYSICAL_ADDRESS  PhysicalAddress2M;
>    UINTN                 IndexOfPageDirectoryEntries;
>    PAGE_TABLE_ENTRY      *PageDirectoryEntry;
> 
> -  PageDirectoryEntry = AllocatePageTableMemory (1);
> +  DEBUG ((DEBUG_INFO, "Split1GPageTo2M\n"));
> +  PageDirectoryEntry = AllocatePageTableMemory (1, &PageTablePool);
> 
>    if (PageDirectoryEntry == NULL) {
>      ASSERT (FALSE);
> @@ -480,7 +489,7 @@ Split1GPageTo2M (
>        //
>        // Need to split this 2M page that covers NULL or stack range.
>        //
> -      Split2MPageTo4K (PhysicalAddress2M, (UINT64 *)PageDirectoryEntry,
> StackBase, StackSize);
> +      Split2MPageTo4K (PhysicalAddress2M, (UINT64 *)PageDirectoryEntry,
> StackBase, StackSize, PageTablePool);
>      } else {
>        //
>        // Fill in the Page Directory entries
> @@ -496,16 +505,19 @@ Split1GPageTo2M (
>  /**
>    Set one page of page table pool memory to be read-only.
> 
> -  @param[in] PageTableBase    Base address of page table (CR3).
> -  @param[in] Address          Start address of a page to be set as read-only.
> -  @param[in] Level4Paging     Level 4 paging flag.
> +  @param[in]      PageTableBase    Base address of page table (CR3).
> +  @param[in]      Address          Start address of a page to be set as read-
> only.
> +  @param[in]      Level4Paging     Level 4 paging flag.
> +  @param[in, out] PageTablePool    Pointer to the current available
> memory used as
> +                                   page table.
> 
>  **/
>  VOID
>  SetPageTablePoolReadOnly (
>    IN  UINTN                 PageTableBase,
>    IN  EFI_PHYSICAL_ADDRESS  Address,
> -  IN  BOOLEAN               Level4Paging
> +  IN  BOOLEAN               Level4Paging,
> +  IN OUT PAGE_TABLE_POOL    *PageTablePool
>    )
>  {
>    UINTN                 Index;
> @@ -573,7 +585,8 @@ SetPageTablePoolReadOnly (
>        //
>        ASSERT (Level > 1);
> 
> -      NewPageTable = AllocatePageTableMemory (1);
> +      DEBUG ((DEBUG_INFO, "SetPageTablePoolReadOnly\n"));
> +      NewPageTable = AllocatePageTableMemory (1, &PageTablePool);
> 
>        if (NewPageTable == NULL) {
>          ASSERT (FALSE);
> @@ -604,14 +617,17 @@ SetPageTablePoolReadOnly (
>  /**
>    Prevent the memory pages used for page table from been overwritten.
> 
> -  @param[in] PageTableBase    Base address of page table (CR3).
> -  @param[in] Level4Paging     Level 4 paging flag.
> +  @param[in]      PageTableBase    Base address of page table (CR3).
> +  @param[in]      Level4Paging     Level 4 paging flag.
> +  @param[in, out] PageTablePool    Pointer to the current available
> memory used as
> +                                   page table.
> 
>  **/
>  VOID
>  EnablePageTableProtection (
> -  IN  UINTN    PageTableBase,
> -  IN  BOOLEAN  Level4Paging
> +  IN  UINTN               PageTableBase,
> +  IN  BOOLEAN             Level4Paging,
> +  IN OUT PAGE_TABLE_POOL  *PageTablePool
>    )
>  {
>    PAGE_TABLE_POOL       *HeadPool;
> @@ -621,7 +637,7 @@ EnablePageTableProtection (
> 
>    DEBUG ((DEBUG_INFO, "EnablePageTableProtection\n"));
> 
> -  if (mPageTablePool == NULL) {
> +  if (PageTablePool == NULL) {
>      return;
>    }
> 
> @@ -632,10 +648,10 @@ EnablePageTableProtection (
>    AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);
> 
>    //
> -  // SetPageTablePoolReadOnly might update mPageTablePool. It's safer to
> +  // SetPageTablePoolReadOnly might update PageTablePool. It's safer to
>    // remember original one in advance.
>    //
> -  HeadPool = mPageTablePool;
> +  HeadPool = PageTablePool;
>    Pool     = HeadPool;
>    do {
>      Address  = (EFI_PHYSICAL_ADDRESS)(UINTN)Pool;
> @@ -647,7 +663,7 @@ EnablePageTableProtection (
>      // protection to them one by one.
>      //
>      while (PoolSize > 0) {
> -      SetPageTablePoolReadOnly (PageTableBase, Address, Level4Paging);
> +      SetPageTablePoolReadOnly (PageTableBase, Address, Level4Paging,
> PageTablePool);
>        Address  += PAGE_TABLE_POOL_UNIT_SIZE;
>        PoolSize -= PAGE_TABLE_POOL_UNIT_SIZE;
>      }
> @@ -700,6 +716,7 @@ CreateIdentityMappingPageTables (
>    BOOLEAN                         Page1GSupport;
>    PAGE_TABLE_1G_ENTRY             *PageDirectory1GEntry;
>    IA32_CR4                        Cr4;
> +  PAGE_TABLE_POOL                 *PageTablePool;
> 
>    //
>    // Set PageMapLevel5Entry to suppress incorrect compiler/analyzer
> warnings
> @@ -785,13 +802,14 @@ CreateIdentityMappingPageTables (
>      (UINT64)TotalPagesNum
>      ));
> 
> -  BigPageAddress = (UINTN)AllocatePageTableMemory (TotalPagesNum);
> +  PageTablePool  = NULL;
> +  BigPageAddress = (UINTN)AllocatePageTableMemory (TotalPagesNum,
> &PageTablePool);
>    if (BigPageAddress == 0) {
>      ASSERT (FALSE);
>      return 0;
>    }
> 
> -  DEBUG ((DEBUG_INFO, "BigPageAddress = 0x%llx\n", BigPageAddress));
> +  DEBUG ((DEBUG_INFO, "BigPageAddress = 0x%llx, PageTablePool=%p\n",
> BigPageAddress, PageTablePool));
> 
>    //
>    // By architecture only one PageMapLevel4 exists - so lets allocate storage
> for it.
> @@ -856,7 +874,8 @@ CreateIdentityMappingPageTables (
>                PageAddress,
>                (UINT64 *)PageDirectory1GEntry,
>                StackBase,
> -              StackSize
> +              StackSize,
> +              PageTablePool
>                );
>            } else {
>              //
> @@ -892,7 +911,7 @@ CreateIdentityMappingPageTables (
>                //
>                // Need to split this 2M page that covers NULL or stack range.
>                //
> -              Split2MPageTo4K (PageAddress, (UINT64 *)PageDirectoryEntry,
> StackBase, StackSize);
> +              Split2MPageTo4K (PageAddress, (UINT64 *)PageDirectoryEntry,
> StackBase, StackSize, PageTablePool);
>              } else {
>                //
>                // Fill in the Page Directory entries
> @@ -929,7 +948,7 @@ CreateIdentityMappingPageTables (
>    // Protect the page table by marking the memory used for page table to
> be
>    // read-only.
>    //
> -  EnablePageTableProtection ((UINTN)PageMap, TRUE);
> +  EnablePageTableProtection ((UINTN)PageMap, TRUE, PageTablePool);
> 
>    return (UINTN)PageMap;
>  }
> --
> 2.29.2.windows.2


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

* Re: [PATCH V2 1/1] OvmfPkg/PeilessStartupLib: move mPageTablePool to stack
  2022-09-27  8:21 ` Yao, Jiewen
@ 2022-09-28  2:03   ` Yao, Jiewen
  0 siblings, 0 replies; 3+ messages in thread
From: Yao, Jiewen @ 2022-09-28  2:03 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Aktas, Erdem, Gerd Hoffmann, James Bottomley

Merged https://github.com/tianocore/edk2/pull/3420

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Tuesday, September 27, 2022 4:22 PM
> To: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io
> Cc: Aktas, Erdem <erdemaktas@google.com>; Gerd Hoffmann
> <kraxel@redhat.com>; James Bottomley <jejb@linux.ibm.com>
> Subject: RE: [PATCH V2 1/1] OvmfPkg/PeilessStartupLib: move
> mPageTablePool to stack
> 
> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> 
> > -----Original Message-----
> > From: Xu, Min M <min.m.xu@intel.com>
> > Sent: Tuesday, September 27, 2022 3:08 PM
> > To: devel@edk2.groups.io
> > Cc: Xu, Min M <min.m.xu@intel.com>; Aktas, Erdem
> > <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com>;
> James
> > Bottomley <jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>
> > Subject: [PATCH V2 1/1] OvmfPkg/PeilessStartupLib: move
> mPageTablePool
> > to stack
> >
> > From: Min M Xu <min.m.xu@intel.com>
> >
> > PeilessStartupLib is running in SEC phase. In this phase global variable
> > is not allowed to be modified. This patch moves mPageTablePool to stack
> > and pass it as input parameter between functions.
> >
> > Cc: Erdem Aktas <erdemaktas@google.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: James Bottomley <jejb@linux.ibm.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Signed-off-by: Min Xu <min.m.xu@intel.com>
> > ---
> >  .../PeilessStartupLib/X64/VirtualMemory.c     | 117 ++++++++++--------
> >  1 file changed, 68 insertions(+), 49 deletions(-)
> >
> > diff --git a/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
> > b/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
> > index 6877e521e485..b444c052d1bf 100644
> > --- a/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
> > +++ b/OvmfPkg/Library/PeilessStartupLib/X64/VirtualMemory.c
> > @@ -21,11 +21,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #include <Library/PlatformInitLib.h>
> >  #include "PageTables.h"
> >
> > -//
> > -// Global variable to keep track current available memory used as page
> > table.
> > -//
> > -PAGE_TABLE_POOL  *mPageTablePool = NULL;
> > -
> >  UINTN  mLevelShift[5] = {
> >    0,
> >    PAGING_L1_ADDRESS_SHIFT,
> > @@ -273,14 +268,17 @@ ToSplitPageTable (
> >    reserve at least another PAGE_TABLE_POOL_UNIT_PAGES. But usually
> > this won't
> >    happen in practice.
> >
> > -  @param PoolPages  The least page number of the pool to be created.
> > +  @param[in]      PoolPages      The least page number of the pool to be
> > created.
> > +  @param[in, out] PageTablePool  Pointer of Pointer to the current
> > available memory
> > +                                  used as page table.
> >
> >    @retval TRUE    The pool is initialized successfully.
> >    @retval FALSE   The memory is out of resource.
> >  **/
> >  BOOLEAN
> >  InitializePageTablePool (
> > -  IN UINTN  PoolPages
> > +  IN UINTN                PoolPages,
> > +  IN OUT PAGE_TABLE_POOL  **PageTablePool
> >    )
> >  {
> >    VOID  *Buffer;
> > @@ -303,20 +301,20 @@ InitializePageTablePool (
> >    //
> >    // Link all pools into a list for easier track later.
> >    //
> > -  if (mPageTablePool == NULL) {
> > -    mPageTablePool           = Buffer;
> > -    mPageTablePool->NextPool = mPageTablePool;
> > +  if (*PageTablePool == NULL) {
> > +    *(UINT64 *)(UINTN)PageTablePool = (UINT64)(UINTN)Buffer;
> > +    (*PageTablePool)->NextPool      = *PageTablePool;
> >    } else {
> > -    ((PAGE_TABLE_POOL *)Buffer)->NextPool = mPageTablePool-
> >NextPool;
> > -    mPageTablePool->NextPool              = Buffer;
> > -    mPageTablePool                        = Buffer;
> > +    ((PAGE_TABLE_POOL *)Buffer)->NextPool = (*PageTablePool)-
> >NextPool;
> > +    (*PageTablePool)->NextPool            = Buffer;
> > +    *PageTablePool                        = Buffer;
> >    }
> >
> >    //
> >    // Reserve one page for pool header.
> >    //
> > -  mPageTablePool->FreePages = PoolPages - 1;
> > -  mPageTablePool->Offset    = EFI_PAGES_TO_SIZE (1);
> > +  (*PageTablePool)->FreePages = PoolPages - 1;
> > +  (*PageTablePool)->Offset    = EFI_PAGES_TO_SIZE (1);
> >
> >    return TRUE;
> >  }
> > @@ -333,14 +331,17 @@ InitializePageTablePool (
> >    If there is not enough memory remaining to satisfy the request, then
> > NULL is
> >    returned.
> >
> > -  @param  Pages                 The number of 4 KB pages to allocate.
> > +  @param[in]      Pages                 The number of 4 KB pages to allocate.
> > +  @param[in, out] PageTablePool         Pointer of pointer to the current
> > available
> > +                                        memory used as page table.
> >
> >    @return A pointer to the allocated buffer or NULL if allocation fails.
> >
> >  **/
> >  VOID *
> >  AllocatePageTableMemory (
> > -  IN UINTN  Pages
> > +  IN UINTN                Pages,
> > +  IN OUT PAGE_TABLE_POOL  **PageTablePool
> >    )
> >  {
> >    VOID  *Buffer;
> > @@ -349,30 +350,31 @@ AllocatePageTableMemory (
> >      return NULL;
> >    }
> >
> > -  DEBUG ((DEBUG_INFO, "AllocatePageTableMemory.
> mPageTablePool=%p,
> > Pages=%d\n", mPageTablePool, Pages));
> > +  DEBUG ((DEBUG_INFO, "AllocatePageTableMemory. PageTablePool=%p,
> > Pages=%d\n", *PageTablePool, Pages));
> >    //
> >    // Renew the pool if necessary.
> >    //
> > -  if ((mPageTablePool == NULL) ||
> > -      (Pages > mPageTablePool->FreePages))
> > +  if ((*PageTablePool == NULL) ||
> > +      (Pages > (*PageTablePool)->FreePages))
> >    {
> > -    if (!InitializePageTablePool (Pages)) {
> > +    if (!InitializePageTablePool (Pages, PageTablePool)) {
> >        return NULL;
> >      }
> >    }
> >
> > -  Buffer = (UINT8 *)mPageTablePool + mPageTablePool->Offset;
> > +  Buffer = (UINT8 *)(*PageTablePool) + (*PageTablePool)->Offset;
> >
> > -  mPageTablePool->Offset    += EFI_PAGES_TO_SIZE (Pages);
> > -  mPageTablePool->FreePages -= Pages;
> > +  (*PageTablePool)->Offset    += EFI_PAGES_TO_SIZE (Pages);
> > +  (*PageTablePool)->FreePages -= Pages;
> >
> >    DEBUG ((
> >      DEBUG_INFO,
> > -    "%a:%a: Buffer=0x%Lx Pages=%ld\n",
> > +    "%a:%a: Buffer=0x%Lx Pages=%ld, PageTablePool=%p\n",
> >      gEfiCallerBaseName,
> >      __FUNCTION__,
> >      Buffer,
> > -    Pages
> > +    Pages,
> > +    *PageTablePool
> >      ));
> >
> >    return Buffer;
> > @@ -385,6 +387,8 @@ AllocatePageTableMemory (
> >    @param[in, out] PageEntry2M           Pointer to 2M page entry.
> >    @param[in]      StackBase             Stack base address.
> >    @param[in]      StackSize             Stack size.
> > +  @param[in, out] PageTablePool         Pointer to the current available
> > memory used as
> > +                                        page table.
> >
> >  **/
> >  VOID
> > @@ -392,7 +396,8 @@ Split2MPageTo4K (
> >    IN EFI_PHYSICAL_ADDRESS  PhysicalAddress,
> >    IN OUT UINT64            *PageEntry2M,
> >    IN EFI_PHYSICAL_ADDRESS  StackBase,
> > -  IN UINTN                 StackSize
> > +  IN UINTN                 StackSize,
> > +  IN OUT PAGE_TABLE_POOL   *PageTablePool
> >    )
> >  {
> >    EFI_PHYSICAL_ADDRESS  PhysicalAddress4K;
> > @@ -401,7 +406,7 @@ Split2MPageTo4K (
> >
> >    DEBUG ((DEBUG_INFO, "Split2MPageTo4K\n"));
> >
> > -  PageTableEntry = AllocatePageTableMemory (1);
> > +  PageTableEntry = AllocatePageTableMemory (1, &PageTablePool);
> >
> >    if (PageTableEntry == NULL) {
> >      ASSERT (FALSE);
> > @@ -448,6 +453,8 @@ Split2MPageTo4K (
> >    @param[in, out] PageEntry1G           Pointer to 1G page entry.
> >    @param[in]      StackBase             Stack base address.
> >    @param[in]      StackSize             Stack size.
> > +  @param[in, out] PageTablePool         Pointer to the current available
> > memory used as
> > +                                        page table.
> >
> >  **/
> >  VOID
> > @@ -455,14 +462,16 @@ Split1GPageTo2M (
> >    IN EFI_PHYSICAL_ADDRESS  PhysicalAddress,
> >    IN OUT UINT64            *PageEntry1G,
> >    IN EFI_PHYSICAL_ADDRESS  StackBase,
> > -  IN UINTN                 StackSize
> > +  IN UINTN                 StackSize,
> > +  IN OUT PAGE_TABLE_POOL   *PageTablePool
> >    )
> >  {
> >    EFI_PHYSICAL_ADDRESS  PhysicalAddress2M;
> >    UINTN                 IndexOfPageDirectoryEntries;
> >    PAGE_TABLE_ENTRY      *PageDirectoryEntry;
> >
> > -  PageDirectoryEntry = AllocatePageTableMemory (1);
> > +  DEBUG ((DEBUG_INFO, "Split1GPageTo2M\n"));
> > +  PageDirectoryEntry = AllocatePageTableMemory (1, &PageTablePool);
> >
> >    if (PageDirectoryEntry == NULL) {
> >      ASSERT (FALSE);
> > @@ -480,7 +489,7 @@ Split1GPageTo2M (
> >        //
> >        // Need to split this 2M page that covers NULL or stack range.
> >        //
> > -      Split2MPageTo4K (PhysicalAddress2M, (UINT64 *)PageDirectoryEntry,
> > StackBase, StackSize);
> > +      Split2MPageTo4K (PhysicalAddress2M, (UINT64
> *)PageDirectoryEntry,
> > StackBase, StackSize, PageTablePool);
> >      } else {
> >        //
> >        // Fill in the Page Directory entries
> > @@ -496,16 +505,19 @@ Split1GPageTo2M (
> >  /**
> >    Set one page of page table pool memory to be read-only.
> >
> > -  @param[in] PageTableBase    Base address of page table (CR3).
> > -  @param[in] Address          Start address of a page to be set as read-only.
> > -  @param[in] Level4Paging     Level 4 paging flag.
> > +  @param[in]      PageTableBase    Base address of page table (CR3).
> > +  @param[in]      Address          Start address of a page to be set as read-
> > only.
> > +  @param[in]      Level4Paging     Level 4 paging flag.
> > +  @param[in, out] PageTablePool    Pointer to the current available
> > memory used as
> > +                                   page table.
> >
> >  **/
> >  VOID
> >  SetPageTablePoolReadOnly (
> >    IN  UINTN                 PageTableBase,
> >    IN  EFI_PHYSICAL_ADDRESS  Address,
> > -  IN  BOOLEAN               Level4Paging
> > +  IN  BOOLEAN               Level4Paging,
> > +  IN OUT PAGE_TABLE_POOL    *PageTablePool
> >    )
> >  {
> >    UINTN                 Index;
> > @@ -573,7 +585,8 @@ SetPageTablePoolReadOnly (
> >        //
> >        ASSERT (Level > 1);
> >
> > -      NewPageTable = AllocatePageTableMemory (1);
> > +      DEBUG ((DEBUG_INFO, "SetPageTablePoolReadOnly\n"));
> > +      NewPageTable = AllocatePageTableMemory (1, &PageTablePool);
> >
> >        if (NewPageTable == NULL) {
> >          ASSERT (FALSE);
> > @@ -604,14 +617,17 @@ SetPageTablePoolReadOnly (
> >  /**
> >    Prevent the memory pages used for page table from been overwritten.
> >
> > -  @param[in] PageTableBase    Base address of page table (CR3).
> > -  @param[in] Level4Paging     Level 4 paging flag.
> > +  @param[in]      PageTableBase    Base address of page table (CR3).
> > +  @param[in]      Level4Paging     Level 4 paging flag.
> > +  @param[in, out] PageTablePool    Pointer to the current available
> > memory used as
> > +                                   page table.
> >
> >  **/
> >  VOID
> >  EnablePageTableProtection (
> > -  IN  UINTN    PageTableBase,
> > -  IN  BOOLEAN  Level4Paging
> > +  IN  UINTN               PageTableBase,
> > +  IN  BOOLEAN             Level4Paging,
> > +  IN OUT PAGE_TABLE_POOL  *PageTablePool
> >    )
> >  {
> >    PAGE_TABLE_POOL       *HeadPool;
> > @@ -621,7 +637,7 @@ EnablePageTableProtection (
> >
> >    DEBUG ((DEBUG_INFO, "EnablePageTableProtection\n"));
> >
> > -  if (mPageTablePool == NULL) {
> > +  if (PageTablePool == NULL) {
> >      return;
> >    }
> >
> > @@ -632,10 +648,10 @@ EnablePageTableProtection (
> >    AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);
> >
> >    //
> > -  // SetPageTablePoolReadOnly might update mPageTablePool. It's safer
> to
> > +  // SetPageTablePoolReadOnly might update PageTablePool. It's safer to
> >    // remember original one in advance.
> >    //
> > -  HeadPool = mPageTablePool;
> > +  HeadPool = PageTablePool;
> >    Pool     = HeadPool;
> >    do {
> >      Address  = (EFI_PHYSICAL_ADDRESS)(UINTN)Pool;
> > @@ -647,7 +663,7 @@ EnablePageTableProtection (
> >      // protection to them one by one.
> >      //
> >      while (PoolSize > 0) {
> > -      SetPageTablePoolReadOnly (PageTableBase, Address, Level4Paging);
> > +      SetPageTablePoolReadOnly (PageTableBase, Address, Level4Paging,
> > PageTablePool);
> >        Address  += PAGE_TABLE_POOL_UNIT_SIZE;
> >        PoolSize -= PAGE_TABLE_POOL_UNIT_SIZE;
> >      }
> > @@ -700,6 +716,7 @@ CreateIdentityMappingPageTables (
> >    BOOLEAN                         Page1GSupport;
> >    PAGE_TABLE_1G_ENTRY             *PageDirectory1GEntry;
> >    IA32_CR4                        Cr4;
> > +  PAGE_TABLE_POOL                 *PageTablePool;
> >
> >    //
> >    // Set PageMapLevel5Entry to suppress incorrect compiler/analyzer
> > warnings
> > @@ -785,13 +802,14 @@ CreateIdentityMappingPageTables (
> >      (UINT64)TotalPagesNum
> >      ));
> >
> > -  BigPageAddress = (UINTN)AllocatePageTableMemory (TotalPagesNum);
> > +  PageTablePool  = NULL;
> > +  BigPageAddress = (UINTN)AllocatePageTableMemory (TotalPagesNum,
> > &PageTablePool);
> >    if (BigPageAddress == 0) {
> >      ASSERT (FALSE);
> >      return 0;
> >    }
> >
> > -  DEBUG ((DEBUG_INFO, "BigPageAddress = 0x%llx\n", BigPageAddress));
> > +  DEBUG ((DEBUG_INFO, "BigPageAddress = 0x%llx,
> PageTablePool=%p\n",
> > BigPageAddress, PageTablePool));
> >
> >    //
> >    // By architecture only one PageMapLevel4 exists - so lets allocate
> storage
> > for it.
> > @@ -856,7 +874,8 @@ CreateIdentityMappingPageTables (
> >                PageAddress,
> >                (UINT64 *)PageDirectory1GEntry,
> >                StackBase,
> > -              StackSize
> > +              StackSize,
> > +              PageTablePool
> >                );
> >            } else {
> >              //
> > @@ -892,7 +911,7 @@ CreateIdentityMappingPageTables (
> >                //
> >                // Need to split this 2M page that covers NULL or stack range.
> >                //
> > -              Split2MPageTo4K (PageAddress, (UINT64 *)PageDirectoryEntry,
> > StackBase, StackSize);
> > +              Split2MPageTo4K (PageAddress, (UINT64 *)PageDirectoryEntry,
> > StackBase, StackSize, PageTablePool);
> >              } else {
> >                //
> >                // Fill in the Page Directory entries
> > @@ -929,7 +948,7 @@ CreateIdentityMappingPageTables (
> >    // Protect the page table by marking the memory used for page table to
> > be
> >    // read-only.
> >    //
> > -  EnablePageTableProtection ((UINTN)PageMap, TRUE);
> > +  EnablePageTableProtection ((UINTN)PageMap, TRUE, PageTablePool);
> >
> >    return (UINTN)PageMap;
> >  }
> > --
> > 2.29.2.windows.2


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

end of thread, other threads:[~2022-09-28  2:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-27  7:07 [PATCH V2 1/1] OvmfPkg/PeilessStartupLib: move mPageTablePool to stack Min Xu
2022-09-27  8:21 ` Yao, Jiewen
2022-09-28  2:03   ` Yao, Jiewen

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