public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] UefiCpuPkg: Use Top of each AP's stack to save CpuMpData
@ 2022-08-16  7:57 Yuanhao Xie
  2022-08-16  9:14 ` [edk2-devel] " Ni, Ray
  2022-08-26 14:27 ` Lendacky, Thomas
  0 siblings, 2 replies; 3+ messages in thread
From: Yuanhao Xie @ 2022-08-16  7:57 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar

To remove the dependency of CPU register, 4/8 byte at the top of the stack
is occupied for CpuMpData. BIST information is also taken care here.
This modification is only for PEI phase, since in DXE phase CpuMpData is
accessed via global variable.

Signed-off-by: Yuanhao <yuanhao.xie@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
 .../Library/MpInitLib/Ia32/MpFuncs.nasm       |  5 +++
 UefiCpuPkg/Library/MpInitLib/MpLib.c          | 41 ++++++++++++++-----
 UefiCpuPkg/Library/MpInitLib/MpLib.h          |  8 ++++
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       | 10 +++--
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm |  6 +++
 5 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 28301bb8f0..4714f2d527 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -179,6 +179,11 @@ ProgramStack:
     mov         esp, dword [edi + CPU_INFO_IN_HOB.ApTopOfStack]
 
 CProcedureInvoke:
+    ;
+    ; Reserve 4 bytes for storing CpuMpData.
+    ; Using sub esp instead of push ebp to avoid overwriting the existed CpuMpData
+    ;
+    sub        esp, 4
     push       ebp               ; push BIST data at top of AP stack
     xor        ebp, ebp          ; clear ebp for call stack trace
     push       ebp
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 8d1f24370a..a9188eb59f 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -571,6 +571,7 @@ InitializeApData (
 {
   CPU_INFO_IN_HOB                *CpuInfoInHob;
   MSR_IA32_PLATFORM_ID_REGISTER  PlatformIdMsr;
+  AP_STACK_DATA                  *ApStackData;
 
   CpuInfoInHob                                = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
   CpuInfoInHob[ProcessorNumber].InitialApicId = GetInitialApicId ();
@@ -578,6 +579,12 @@ InitializeApData (
   CpuInfoInHob[ProcessorNumber].Health        = BistData;
   CpuInfoInHob[ProcessorNumber].ApTopOfStack  = ApTopOfStack;
 
+  //
+  // AP_STACK_DATA is stored at the top of AP Stack
+  //
+  ApStackData         = (AP_STACK_DATA *)((UINTN)ApTopOfStack - sizeof (AP_STACK_DATA));
+  ApStackData->MpData = CpuMpData;
+
   CpuMpData->CpuData[ProcessorNumber].Waiting    = FALSE;
   CpuMpData->CpuData[ProcessorNumber].CpuHealthy = (BistData == 0) ? TRUE : FALSE;
 
@@ -623,6 +630,7 @@ ApWakeupFunction (
   CPU_INFO_IN_HOB   *CpuInfoInHob;
   UINT64            ApTopOfStack;
   UINTN             CurrentApicMode;
+  AP_STACK_DATA     *ApStackData;
 
   //
   // AP finished assembly code and begin to execute C code
@@ -648,7 +656,9 @@ ApWakeupFunction (
       // This is first time AP wakeup, get BIST information from AP stack
       //
       ApTopOfStack = CpuMpData->Buffer + (ProcessorNumber + 1) * CpuMpData->CpuApStackSize;
-      BistData     = *(UINT32 *)((UINTN)ApTopOfStack - sizeof (UINTN));
+      ApStackData  = (AP_STACK_DATA *)((UINTN)ApTopOfStack - sizeof (AP_STACK_DATA));
+      BistData     = (UINT32)ApStackData->Bist;
+
       //
       // CpuMpData->CpuData[0].VolatileRegisters is initialized based on BSP environment,
       //   to initialize AP in InitConfig path.
@@ -1796,30 +1806,41 @@ MpInitLibInitialize (
   AsmGetAddressMap (&AddressMap);
   GetApResetVectorSize (&AddressMap, &ApResetVectorSizeBelow1Mb, &ApResetVectorSizeAbove1Mb);
   ApStackSize = PcdGet32 (PcdCpuApStackSize);
-  ApLoopMode  = GetApLoopMode (&MonitorFilterSize);
+  //
+  // ApStackSize must be power of 2
+  //
+  ASSERT ((ApStackSize & (ApStackSize - 1)) == 0);
+  ApLoopMode = GetApLoopMode (&MonitorFilterSize);
 
   //
   // Save BSP's Control registers for APs.
   //
   SaveVolatileRegisters (&VolatileRegisters);
 
+  //
+  // Allocate extra ApStackSize to let AP stack align on ApStackSize bounday
+  //
   BufferSize  = ApStackSize * MaxLogicalProcessorNumber;
+  BufferSize += ApStackSize;
   BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber;
   BufferSize += ApResetVectorSizeBelow1Mb;
   BufferSize  = ALIGN_VALUE (BufferSize, 8);
   BufferSize += VolatileRegisters.Idtr.Limit + 1;
   BufferSize += sizeof (CPU_MP_DATA);
   BufferSize += (sizeof (CPU_AP_DATA) + sizeof (CPU_INFO_IN_HOB))* MaxLogicalProcessorNumber;
-  MpBuffer    = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
+  //
+  // Allocate extra ApStackSize to let stack align on ApStackSize bounday
+  //
+  MpBuffer = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
   ASSERT (MpBuffer != NULL);
   ZeroMem (MpBuffer, BufferSize);
-  Buffer = (UINTN)MpBuffer;
+  Buffer = ALIGN_VALUE ((UINTN)MpBuffer, ApStackSize);
 
   //
-  //  The layout of the Buffer is as below:
+  //  The layout of the Buffer is as below (lower address on top):
   //
-  //    +--------------------+ <-- Buffer
-  //        AP Stacks (N)
+  //    +--------------------+ <-- Buffer (Pointer of CpuMpData is stored in the top of each AP's stack.)
+  //        AP Stacks (N)                 (StackTop = (RSP + ApStackSize) & ~ApStackSize))
   //    +--------------------+ <-- MonitorBuffer
   //    AP Monitor Filters (N)
   //    +--------------------+ <-- BackupBufferAddr (CpuMpData->BackupBuffer)
@@ -1827,7 +1848,7 @@ MpInitLibInitialize (
   //    +--------------------+
   //           Padding
   //    +--------------------+ <-- ApIdtBase (8-byte boundary)
-  //           AP IDT          All APs share one separate IDT. So AP can get address of CPU_MP_DATA from IDT Base.
+  //           AP IDT          All APs share one separate IDT.
   //    +--------------------+ <-- CpuMpData
   //         CPU_MP_DATA
   //    +--------------------+ <-- CpuMpData->CpuData
@@ -1866,8 +1887,8 @@ MpInitLibInitialize (
   // Make sure no memory usage outside of the allocated buffer.
   //
   ASSERT (
-    (CpuMpData->CpuInfoInHob + sizeof (CPU_INFO_IN_HOB) * MaxLogicalProcessorNumber) ==
-    Buffer + BufferSize
+    (CpuMpData->CpuInfoInHob + sizeof (CPU_INFO_IN_HOB) * MaxLogicalProcessorNumber) <=
+    (UINTN)MpBuffer + BufferSize
     );
 
   //
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 974fb76019..69b621a340 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -302,6 +302,14 @@ struct _CPU_MP_DATA {
   UINT64         GhcbBase;
 };
 
+//
+// AP_STACK_DATA is stored at the top of each AP stack.
+//
+typedef struct {
+  UINTN          Bist;
+  CPU_MP_DATA    *MpData;
+} AP_STACK_DATA;
+
 #define AP_SAFE_STACK_SIZE   128
 #define AP_RESET_STACK_SIZE  AP_SAFE_STACK_SIZE
 
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 65400b95a2..e732371ddd 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -89,7 +89,7 @@ EnableDebugAgent (
 /**
   Get pointer to CPU MP Data structure.
   For BSP, the pointer is retrieved from HOB.
-  For AP, the structure is just after IDT.
+  For AP, the structure is stored in the top of each AP's stack.
 
   @return  The pointer to CPU MP Data structure.
 **/
@@ -100,15 +100,17 @@ GetCpuMpData (
 {
   CPU_MP_DATA                  *CpuMpData;
   MSR_IA32_APIC_BASE_REGISTER  ApicBaseMsr;
-  IA32_DESCRIPTOR              Idtr;
+  UINTN                        ApTopOfStack;
+  AP_STACK_DATA                *ApStackData;
 
   ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
   if (ApicBaseMsr.Bits.BSP == 1) {
     CpuMpData = GetCpuMpDataFromGuidedHob ();
     ASSERT (CpuMpData != NULL);
   } else {
-    AsmReadIdtr (&Idtr);
-    CpuMpData = (CPU_MP_DATA *)(Idtr.Base + Idtr.Limit + 1);
+    ApTopOfStack = ALIGN_VALUE ((UINTN)&ApTopOfStack, (UINTN)PcdGet32 (PcdCpuApStackSize));
+    ApStackData  = (AP_STACK_DATA *)((UINTN)ApTopOfStack- sizeof (AP_STACK_DATA));
+    CpuMpData    = (CPU_MP_DATA *)ApStackData->MpData;
   }
 
   return CpuMpData;
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 1daaa72b1e..322bdd03e6 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -237,11 +237,17 @@ ProgramStack:
     mov         rsp, qword [rdi + CPU_INFO_IN_HOB.ApTopOfStack]
 
 CProcedureInvoke:
+    ;
+    ; Reserve 8 bytes for storing CpuMpData.
+    ; Using sub rsp instead of push rbp to avoid overwriting existed CpuMpData
+    ;
+    sub        rsp, 8
     push       rbp               ; Push BIST data at top of AP stack
     xor        rbp, rbp          ; Clear ebp for call stack trace
     push       rbp
     mov        rbp, rsp
 
+    push       qword 0          ; Push 8 bytes for alignment
     mov        rax, qword [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitializeFloatingPointUnits)]
     sub        rsp, 20h
     call       rax               ; Call assembly function to initialize FPU per UEFI spec
-- 
2.36.1.windows.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-26 14:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-16  7:57 [PATCH] UefiCpuPkg: Use Top of each AP's stack to save CpuMpData Yuanhao Xie
2022-08-16  9:14 ` [edk2-devel] " Ni, Ray
2022-08-26 14:27 ` Lendacky, Thomas

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