UefiCpuPkg: Apply ReadOnly on Ap loop buffers.

In the MpInitLib, pages are allocated for
the APs, and data is copied into them. Depending
on the allocation type, Nx needs to be removed
from the pages to allow the processors to execute.

Additionally, ReadOnly needs to be applied to the
pages after they have been filled with the
instructions that the APs need to execute.

Signed-off-by: Aaron Pop <aaronpop@microsoft.com>
Co-authored-by: Oliver Smith-Denny <osde@microsoft.com>

4 files changed, 163 insertions, 39 deletions

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 8c1428c..f5f6074 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -160,7 +160,7 @@ GetWakeupBuffer (
   @retval 0       Cannot find free memory below 4GB.

 **/

 UINTN

-AllocateCodeBuffer (

+AllocateCodePage (

   IN UINTN  BufferSize

   )

 {

@@ -417,14 +417,46 @@ AllocateApLoopCodeBuffer (
 /**

   Remove Nx protection for the range specific by BaseAddress and Length.

 

-  The PEI implementation uses CpuPageTableLib to change the attribute.

-  The DXE implementation uses gDS to change the attribute.

+  @param[in] BaseAddress  BaseAddress of the range.

+  @param[in] Length       Length of the range.

+**/

+VOID

+RemoveNxProtection (

+  IN EFI_PHYSICAL_ADDRESS  BaseAddress,

+  IN UINTN                 Length

+  )

+{

+  EFI_STATUS                       Status;

+  EFI_GCD_MEMORY_SPACE_DESCRIPTOR  MemDesc;

+

+  Status = gDS->GetMemorySpaceDescriptor (BaseAddress, &MemDesc);

+  if (!EFI_ERROR (Status)) {

+    if (((MemDesc.Capabilities & EFI_MEMORY_XP) == EFI_MEMORY_XP) && ((MemDesc.Attributes & EFI_MEMORY_XP) == EFI_MEMORY_XP)) {

+      Status = gDS->SetMemorySpaceAttributes (

+                      BaseAddress,

+                      Length,

+                      MemDesc.Attributes & (~EFI_MEMORY_XP)

+                      );

+

+      if (EFI_ERROR (Status)) {

+        DEBUG ((DEBUG_ERROR, "%a - Setting Nx on 0x%p returned %r\n", __func__, BaseAddress, Status));

+        ASSERT_EFI_ERROR (Status);

+      }

+    }

+  } else {

+    DEBUG ((DEBUG_ERROR, "%a - Memory Address was not found in Memory map! %lp %r\n", __func__, BaseAddress, Status));

+    ASSERT_EFI_ERROR (Status);

+  }

+}

+

+/**

+  Add ReadOnly protection to the range specified by BaseAddress and Length.

 

   @param[in] BaseAddress  BaseAddress of the range.

   @param[in] Length       Length of the range.

 **/

 VOID

-RemoveNxprotection (

+ApplyRoProtection (

   IN EFI_PHYSICAL_ADDRESS  BaseAddress,

   IN UINTN                 Length

   )

@@ -432,17 +464,23 @@ RemoveNxprotection (
   EFI_STATUS                       Status;

   EFI_GCD_MEMORY_SPACE_DESCRIPTOR  MemDesc;

 

-  //

-  // TODO: Check EFI_MEMORY_XP bit set or not once it's available in DXE GCD

-  //       service.

-  //

   Status = gDS->GetMemorySpaceDescriptor (BaseAddress, &MemDesc);

   if (!EFI_ERROR (Status)) {

-    gDS->SetMemorySpaceAttributes (

-           BaseAddress,

-           Length,

-           MemDesc.Attributes & (~EFI_MEMORY_XP)

-           );

+    if (((MemDesc.Capabilities & EFI_MEMORY_RO) == EFI_MEMORY_RO) && ((MemDesc.Attributes & EFI_MEMORY_RO) != EFI_MEMORY_RO)) {

+      Status = gDS->SetMemorySpaceAttributes (

+                      BaseAddress,

+                      Length,

+                      MemDesc.Attributes | EFI_MEMORY_RO

+                      );

+

+      if (EFI_ERROR (Status)) {

+        DEBUG ((DEBUG_ERROR, "%a - Setting Ro on 0x%p returned %r\n", __func__, BaseAddress, Status));

+        ASSERT_EFI_ERROR (Status);

+      }

+    }

+  } else {

+    DEBUG ((DEBUG_ERROR, "%a - Memory Address was not found in Memory map! %lp %r\n", __func__, BaseAddress, Status));

+    ASSERT_EFI_ERROR (Status);

   }

 }

 

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index ffaff18..96c0980 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -2072,6 +2072,7 @@ MpInitLibInitialize (
   UINTN                    ApResetVectorSizeAbove1Mb;

   UINTN                    BackupBufferAddr;

   UINTN                    ApIdtBase;

+  IA32_CR0                 Cr0;

 

   FirstMpHandOff = GetNextMpHandOffHob (NULL);

   if (FirstMpHandOff != NULL) {

@@ -2248,7 +2249,13 @@ MpInitLibInitialize (
   // Copy all 32-bit code and 64-bit code into memory with type of

   // EfiBootServicesCode to avoid page fault if NX memory protection is enabled.

   //

-  CpuMpData->WakeupBufferHigh = AllocateCodeBuffer (ApResetVectorSizeAbove1Mb);

+  CpuMpData->WakeupBufferHigh = AllocateCodePage (ApResetVectorSizeAbove1Mb);

+

+  Cr0.UintN = AsmReadCr0 ();

+  if (Cr0.Bits.PG != 0) {

+    RemoveNxProtection ((EFI_PHYSICAL_ADDRESS)(UINTN)CpuMpData->WakeupBufferHigh, ALIGN_VALUE (ApResetVectorSizeAbove1Mb, EFI_PAGE_SIZE));

+  }

+

   CopyMem (

     (VOID *)CpuMpData->WakeupBufferHigh,

     CpuMpData->AddressMap.RendezvousFunnelAddress +

@@ -2256,6 +2263,9 @@ MpInitLibInitialize (
     ApResetVectorSizeAbove1Mb

     );

   DEBUG ((DEBUG_INFO, "AP Vector: non-16-bit = %p/%x\n", CpuMpData->WakeupBufferHigh, ApResetVectorSizeAbove1Mb));

+  if (Cr0.Bits.PG != 0) {

+    ApplyRoProtection ((EFI_PHYSICAL_ADDRESS)(UINTN)CpuMpData->WakeupBufferHigh, ALIGN_VALUE (ApResetVectorSizeAbove1Mb, EFI_PAGE_SIZE));

+  }

 

   //

   // Save APIC mode for AP to sync

@@ -3484,7 +3494,7 @@ PrepareApLoopCode (
     // Make sure that the buffer memory is executable if NX protection is enabled

     // for EfiReservedMemoryType.

     //

-    RemoveNxprotection (Address, EFI_PAGES_TO_SIZE (FuncPages));

+    RemoveNxProtection (Address, EFI_PAGES_TO_SIZE (FuncPages));

   }

 

   mReservedTopOfApStack = (UINTN)Address + EFI_PAGES_TO_SIZE (StackPages+FuncPages);

@@ -3492,6 +3502,10 @@ PrepareApLoopCode (
   mReservedApLoop.Data = (VOID *)(UINTN)Address;

   ASSERT (mReservedApLoop.Data != NULL);

   CopyMem (mReservedApLoop.Data, ApLoopFunc, ApLoopFuncSize);

+  if (Cr0.Bits.PG != 0) {

+    ApplyRoProtection (Address, EFI_PAGES_TO_SIZE (FuncPages));

+  }

+

   if (!CpuMpData->UseSevEsAPMethod) {

     //

     // processors without SEV-ES and paging is enabled

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index b30dcb3..60cae3b 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -533,7 +533,7 @@ GetNextMpHandOffHob (
   @retval 0       Cannot find free memory below 4GB.

 **/

 UINTN

-AllocateCodeBuffer (

+AllocateCodePage (

   IN UINTN  BufferSize

   );

 

@@ -991,14 +991,23 @@ AllocateApLoopCodeBuffer (
 /**

   Remove Nx protection for the range specific by BaseAddress and Length.

 

-  The PEI implementation uses CpuPageTableLib to change the attribute.

-  The DXE implementation uses gDS to change the attribute.

-

   @param[in] BaseAddress  BaseAddress of the range.

   @param[in] Length       Length of the range.

 **/

 VOID

-RemoveNxprotection (

+RemoveNxProtection (

+  IN EFI_PHYSICAL_ADDRESS  BaseAddress,

+  IN UINTN                 Length

+  );

+

+/**

+Add ReadOnly protection to the range specified by BaseAddress and Length.

+

+@param[in] BaseAddress  BaseAddress of the range.

+@param[in] Length       Length of the range.

+**/

+VOID

+ApplyRoProtection (

   IN EFI_PHYSICAL_ADDRESS  BaseAddress,

   IN UINTN                 Length

   );

diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index bb28794..443508f 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -290,7 +290,7 @@ GetWakeupBuffer (
   @retval 0       Cannot find free memory below 4GB.

 **/

 UINTN

-AllocateCodeBuffer (

+AllocateCodePage (

   IN UINTN  BufferSize

   )

 {

@@ -819,31 +819,21 @@ AllocateApLoopCodeBuffer (
 }

 

 /**

-  Remove Nx protection for the range specific by BaseAddress and Length.

-

-  The PEI implementation uses CpuPageTableLib to change the attribute.

-  The DXE implementation uses gDS to change the attribute.

+  Determine the Paging Mode that the system is currently

+  using.

 

-  @param[in] BaseAddress  BaseAddress of the range.

-  @param[in] Length       Length of the range.

-**/

-VOID

-RemoveNxprotection (

-  IN EFI_PHYSICAL_ADDRESS  BaseAddress,

-  IN UINTN                 Length

+  @retval PAGING_MODE

+ **/

+PAGING_MODE

+DetermineCurrentPagingMode (

+  VOID

   )

 {

-  EFI_STATUS                  Status;

-  UINTN                       PageTable;

-  EFI_PHYSICAL_ADDRESS        Buffer;

-  UINTN                       BufferSize;

-  IA32_MAP_ATTRIBUTE          MapAttribute;

-  IA32_MAP_ATTRIBUTE          MapMask;

   PAGING_MODE                 PagingMode;

   IA32_CR4                    Cr4;

   BOOLEAN                     Page5LevelSupport;

-  UINT32                      RegEax;

   BOOLEAN                     Page1GSupport;

+  UINT32                      RegEax;

   CPUID_EXTENDED_CPU_SIG_EDX  RegEdx;

 

   if (sizeof (UINTN) == sizeof (UINT64)) {

@@ -877,12 +867,36 @@ RemoveNxprotection (
     PagingMode = PagingPae;

   }

 

+  return PagingMode;

+}

+

+/**

+  Remove Nx protection for the range specific by BaseAddress and Length.

+

+  @param[in] BaseAddress  BaseAddress of the range.

+  @param[in] Length       Length of the range.

+**/

+VOID

+RemoveNxProtection (

+  IN EFI_PHYSICAL_ADDRESS  BaseAddress,

+  IN UINTN                 Length

+  )

+{

+  EFI_STATUS            Status;

+  UINTN                 PageTable;

+  EFI_PHYSICAL_ADDRESS  Buffer;

+  UINTN                 BufferSize;

+  IA32_MAP_ATTRIBUTE    MapAttribute;

+  IA32_MAP_ATTRIBUTE    MapMask;

+  PAGING_MODE           PagingMode;

+

   MapAttribute.Uint64 = 0;

   MapMask.Uint64      = 0;

   MapMask.Bits.Nx     = 1;

   PageTable           = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;

   BufferSize          = 0;

 

+  PagingMode = DetermineCurrentPagingMode ();

   //

   // Get required buffer size for changing the pagetable.

   //

@@ -903,3 +917,52 @@ RemoveNxprotection (
   ASSERT_EFI_ERROR (Status);

   AsmWriteCr3 (PageTable);

 }

+

+/**

+  Add ReadOnly protection to the range specified by BaseAddress and Length.

+

+  @param[in] BaseAddress  BaseAddress of the range.

+  @param[in] Length       Length of the range.

+**/

+VOID

+ApplyRoProtection (

+  IN EFI_PHYSICAL_ADDRESS  BaseAddress,

+  IN UINTN                 Length

+  )

+{

+  EFI_STATUS            Status;

+  UINTN                 PageTable;

+  EFI_PHYSICAL_ADDRESS  Buffer;

+  UINTN                 BufferSize;

+  IA32_MAP_ATTRIBUTE    MapAttribute;

+  IA32_MAP_ATTRIBUTE    MapMask;

+  PAGING_MODE           PagingMode;

+

+  MapAttribute.Uint64    = 0;

+  MapMask.Uint64         = 0;

+  MapMask.Bits.ReadWrite = 1;

+  PageTable              = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64;

+  BufferSize             = 0;

+

+  PagingMode = DetermineCurrentPagingMode ();

+  //

+  // Get required buffer size for changing the pagetable.

+  //

+  Status = PageTableMap (&PageTable, PagingMode, 0, &BufferSize, BaseAddress, Length, &MapAttribute, &MapMask, NULL);

+  if (Status == EFI_BUFFER_TOO_SMALL) {

+    //

+    // Allocate required Buffer.

+    //

+    Status = PeiServicesAllocatePages (

+               EfiBootServicesData,

+               EFI_SIZE_TO_PAGES (BufferSize),

+               &Buffer

+               );

+    ASSERT_EFI_ERROR (Status);

+    Status = PageTableMap (&PageTable, PagingMode, (VOID *)(UINTN)Buffer, &BufferSize, BaseAddress, Length, &MapAttribute, &MapMask, NULL);

+  }

+

+  ASSERT_EFI_ERROR (Status);

+  AsmWriteCr3 (PageTable);

+  return;

+}