Re: [edk2-devel] [PATCH 08/18] UefiCpuPkg:Abstract some DxeMpLib code to function

["Ni, Ray" <ray.ni@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 9795 bytes --]

Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com>
Sent: Friday, May 10, 2024 18:08
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [PATCH 08/18] UefiCpuPkg:Abstract some DxeMpLib code to function

Abstract some DxeMpLib code to function in this commit.
Some of these internal functions will be moved to common
MpLib.c in following commits. Then PeiMpLib can reuse
the code.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: 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>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 220 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------
 1 file changed, 139 insertions(+), 81 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 57ddb86600..d13cebbee6 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -422,6 +422,144 @@ RelocateApLoop (
   ASSERT (FALSE);
 }

+/**
+  Allocate buffer for ApLoopCode.
+
+  @param[in]      Pages    Number of pages to allocate.
+  @param[in, out] Address  Pointer to the allocated buffer.
+**/
+VOID
+AllocateApLoopCodeBuffer (
+  IN UINTN                     Pages,
+  IN OUT EFI_PHYSICAL_ADDRESS  *Address
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = gBS->AllocatePages (
+                  AllocateMaxAddress,
+                  EfiReservedMemoryType,
+                  Pages,
+                  Address
+                  );
+  ASSERT_EFI_ERROR (Status);
+}
+
+/**
+  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;
+
+  //
+  // 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)
+           );
+  }
+}
+
+/**
+  Prepare ApLoopCode.
+
+  @param[in] CpuMpData  Pointer to CpuMpData.
+**/
+VOID
+PrepareApLoopCode (
+  IN CPU_MP_DATA  *CpuMpData
+  )
+{
+  EFI_PHYSICAL_ADDRESS     Address;
+  MP_ASSEMBLY_ADDRESS_MAP  *AddressMap;
+  UINT8                    *ApLoopFunc;
+  UINTN                    ApLoopFuncSize;
+  UINTN                    StackPages;
+  UINTN                    FuncPages;
+  IA32_CR0                 Cr0;
+
+  AddressMap = &CpuMpData->AddressMap;
+  if (CpuMpData->UseSevEsAPMethod) {
+    //
+    // 64-bit AMD processors with SEV-ES
+    //
+    Address        = BASE_4GB - 1;
+    ApLoopFunc     = AddressMap->RelocateApLoopFuncAddressAmdSev;
+    ApLoopFuncSize = AddressMap->RelocateApLoopFuncSizeAmdSev;
+  } else {
+    //
+    // Intel processors (32-bit or 64-bit), 32-bit AMD processors, or 64-bit AMD processors without SEV-ES
+    //
+    Address        = MAX_ADDRESS;
+    ApLoopFunc     = AddressMap->RelocateApLoopFuncAddressGeneric;
+    ApLoopFuncSize = AddressMap->RelocateApLoopFuncSizeGeneric;
+  }
+
+  //
+  // Avoid APs access invalid buffer data which allocated by BootServices,
+  // so we will allocate reserved data for AP loop code. We also need to
+  // allocate this buffer below 4GB due to APs may be transferred to 32bit
+  // protected mode on long mode DXE.
+  // Allocating it in advance since memory services are not available in
+  // Exit Boot Services callback function.
+  //
+  // +------------+ (TopOfApStack)
+  // |  Stack * N |
+  // +------------+ (stack base, 4k aligned)
+  // |  Padding   |
+  // +------------+
+  // |  Ap Loop   |
+  // +------------+ ((low address, 4k-aligned)
+  //
+
+  StackPages = EFI_SIZE_TO_PAGES (CpuMpData->CpuCount * AP_SAFE_STACK_SIZE);
+  FuncPages  = EFI_SIZE_TO_PAGES (ApLoopFuncSize);
+
+  AllocateApLoopCodeBuffer (StackPages + FuncPages, &Address);
+  ASSERT (Address != 0);
+
+  Cr0.UintN = AsmReadCr0 ();
+  if (Cr0.Bits.PG != 0) {
+    //
+    // Make sure that the buffer memory is executable if NX protection is enabled
+    // for EfiReservedMemoryType.
+    //
+    RemoveNxprotection (Address, EFI_PAGES_TO_SIZE (FuncPages));
+  }
+
+  mReservedTopOfApStack = (UINTN)Address + EFI_PAGES_TO_SIZE (StackPages+FuncPages);
+  ASSERT ((mReservedTopOfApStack & (UINTN)(CPU_STACK_ALIGNMENT - 1)) == 0);
+  mReservedApLoop.Data = (VOID *)(UINTN)Address;
+  ASSERT (mReservedApLoop.Data != NULL);
+  CopyMem (mReservedApLoop.Data, ApLoopFunc, ApLoopFuncSize);
+  if (!CpuMpData->UseSevEsAPMethod) {
+    //
+    // processors without SEV-ES and paging is enabled
+    //
+    mApPageTable = CreatePageTable (
+                     (UINTN)Address,
+                     EFI_PAGES_TO_SIZE (StackPages+FuncPages)
+                     );
+  }
+}
+
 /**
   Callback function for ExitBootServices.

@@ -477,16 +615,10 @@ InitMpGlobalData (
   )
 {
   EFI_STATUS                       Status;
-  EFI_PHYSICAL_ADDRESS             Address;
   UINTN                            Index;
   EFI_GCD_MEMORY_SPACE_DESCRIPTOR  MemDesc;
   UINTN                            StackBase;
   CPU_INFO_IN_HOB                  *CpuInfoInHob;
-  MP_ASSEMBLY_ADDRESS_MAP          *AddressMap;
-  UINT8                            *ApLoopFunc;
-  UINTN                            ApLoopFuncSize;
-  UINTN                            StackPages;
-  UINTN                            FuncPages;

   SaveCpuMpData (CpuMpData);

@@ -541,81 +673,7 @@ InitMpGlobalData (
     }
   }

-  AddressMap = &CpuMpData->AddressMap;
-  if (CpuMpData->UseSevEsAPMethod) {
-    //
-    // 64-bit AMD processors with SEV-ES
-    //
-    Address        = BASE_4GB - 1;
-    ApLoopFunc     = AddressMap->RelocateApLoopFuncAddressAmdSev;
-    ApLoopFuncSize = AddressMap->RelocateApLoopFuncSizeAmdSev;
-  } else {
-    //
-    // Intel processors (32-bit or 64-bit), 32-bit AMD processors, or 64-bit AMD processors without SEV-ES
-    //
-    Address        = MAX_ADDRESS;
-    ApLoopFunc     = AddressMap->RelocateApLoopFuncAddressGeneric;
-    ApLoopFuncSize = AddressMap->RelocateApLoopFuncSizeGeneric;
-  }
-
-  //
-  // Avoid APs access invalid buffer data which allocated by BootServices,
-  // so we will allocate reserved data for AP loop code. We also need to
-  // allocate this buffer below 4GB due to APs may be transferred to 32bit
-  // protected mode on long mode DXE.
-  // Allocating it in advance since memory services are not available in
-  // Exit Boot Services callback function.
-  //
-  // +------------+ (TopOfApStack)
-  // |  Stack * N |
-  // +------------+ (stack base, 4k aligned)
-  // |  Padding   |
-  // +------------+
-  // |  Ap Loop   |
-  // +------------+ ((low address, 4k-aligned)
-  //
-
-  StackPages = EFI_SIZE_TO_PAGES (CpuMpData->CpuCount * AP_SAFE_STACK_SIZE);
-  FuncPages  = EFI_SIZE_TO_PAGES (ApLoopFuncSize);
-
-  Status = gBS->AllocatePages (
-                  AllocateMaxAddress,
-                  EfiReservedMemoryType,
-                  StackPages + FuncPages,
-                  &Address
-                  );
-  ASSERT_EFI_ERROR (Status);
-
-  //
-  // Make sure that the buffer memory is executable if NX protection is enabled
-  // for EfiReservedMemoryType.
-  //
-  // TODO: Check EFI_MEMORY_XP bit set or not once it's available in DXE GCD
-  //       service.
-  //
-  Status = gDS->GetMemorySpaceDescriptor (Address, &MemDesc);
-  if (!EFI_ERROR (Status)) {
-    gDS->SetMemorySpaceAttributes (
-           Address,
-           EFI_PAGES_TO_SIZE (FuncPages),
-           MemDesc.Attributes & (~EFI_MEMORY_XP)
-           );
-  }
-
-  mReservedTopOfApStack = (UINTN)Address + EFI_PAGES_TO_SIZE (StackPages+FuncPages);
-  ASSERT ((mReservedTopOfApStack & (UINTN)(CPU_STACK_ALIGNMENT - 1)) == 0);
-  mReservedApLoop.Data = (VOID *)(UINTN)Address;
-  ASSERT (mReservedApLoop.Data != NULL);
-  CopyMem (mReservedApLoop.Data, ApLoopFunc, ApLoopFuncSize);
-  if (!CpuMpData->UseSevEsAPMethod) {
-    //
-    // processors without SEV-ES
-    //
-    mApPageTable = CreatePageTable (
-                     (UINTN)Address,
-                     EFI_PAGES_TO_SIZE (StackPages+FuncPages)
-                     );
-  }
+  PrepareApLoopCode (CpuMpData);

   Status = gBS->CreateEvent (
                   EVT_TIMER | EVT_NOTIFY_SIGNAL,
--
2.31.1.windows.1



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



[-- Attachment #2: Type: text/html, Size: 18424 bytes --]