[edk2-devel] [Patch V2] UefiCpuPkg: Use Top of each AP's stack to save CpuMpData

Yuanhao Xie posted 1 patch 1 year, 8 months ago
Failed in applying to current master (apply log)
.../Library/MpInitLib/Ia32/MpFuncs.nasm       |  8 ++++
UefiCpuPkg/Library/MpInitLib/MpLib.c          | 37 ++++++++++++++-----
UefiCpuPkg/Library/MpInitLib/MpLib.h          |  8 ++++
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       | 10 +++--
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm |  9 +++++
5 files changed, 59 insertions(+), 13 deletions(-)
[edk2-devel] [Patch V2] UefiCpuPkg: Use Top of each AP's stack to save CpuMpData
Posted by Yuanhao Xie 1 year, 8 months ago
From: Yuanhao Xie <yuanhao.xie@intel.com>

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.

Change-Id: I7564279544622617c322310b4c7028ac0e11a9c4
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       |  8 ++++
 UefiCpuPkg/Library/MpInitLib/MpLib.c          | 37 ++++++++++++++-----
 UefiCpuPkg/Library/MpInitLib/MpLib.h          |  8 ++++
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       | 10 +++--
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm |  9 +++++
 5 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 28301bb8f0..2625d28401 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -179,6 +179,14 @@ ProgramStack:
     mov         esp, dword [edi + CPU_INFO_IN_HOB.ApTopOfStack]
 
 CProcedureInvoke:
+    ;
+    ; Reserve 4 bytes for CpuMpData.
+    ; When the AP wakes up again via INIT-SIPI-SIPI, push 0 will cause the existing CpuMpData to be overwritten with 0.
+    ; CpuMpData is filled in via InitializeApData() during the first time INIT-SIPI-SIPI,
+    ; while overwirrten may occurs when under ApInHltLoop but InitFlag is not set to ApInitConfig.
+    ; Therefore reservation is implemented by sub esp instead of push 0.
+    ;
+    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..a4ce1c6d2e 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,14 +1806,22 @@ 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);
 
-  BufferSize  = ApStackSize * MaxLogicalProcessorNumber;
+  BufferSize = ApStackSize * MaxLogicalProcessorNumber;
+  //
+  // Allocate extra ApStackSize to let AP stack align on ApStackSize bounday
+  //
+  BufferSize += ApStackSize;
   BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber;
   BufferSize += ApResetVectorSizeBelow1Mb;
   BufferSize  = ALIGN_VALUE (BufferSize, 8);
@@ -1813,13 +1831,13 @@ MpInitLibInitialize (
   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 +1845,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
@@ -1864,10 +1882,11 @@ MpInitLibInitialize (
 
   //
   // Make sure no memory usage outside of the allocated buffer.
+  // (ApStackSize - (Buffer - (UINTN)MpBuffer)) is the redundant caused by alignment
   //
   ASSERT (
     (CpuMpData->CpuInfoInHob + sizeof (CPU_INFO_IN_HOB) * MaxLogicalProcessorNumber) ==
-    Buffer + BufferSize
+    (UINTN)MpBuffer + BufferSize - (ApStackSize - Buffer + (UINTN)MpBuffer)
     );
 
   //
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..6751cae6f1 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -237,11 +237,20 @@ ProgramStack:
     mov         rsp, qword [rdi + CPU_INFO_IN_HOB.ApTopOfStack]
 
 CProcedureInvoke:
+    ;
+    ; Reserve 8 bytes for CpuMpData.
+    ; When the AP wakes up again via INIT-SIPI-SIPI, push 0 will cause the existing CpuMpData to be overwritten with 0.
+    ; CpuMpData is filled in via InitializeApData() during the first time INIT-SIPI-SIPI,
+    ; while overwirrten may occurs when under ApInHltLoop but InitFlag is not set to ApInitConfig.
+    ; Therefore reservation is implemented by sub rsp instead of push 0.
+    ;
+    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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92567): https://edk2.groups.io/g/devel/message/92567
Mute This Topic: https://groups.io/mt/93119724/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2] UefiCpuPkg: Use Top of each AP's stack to save CpuMpData
Posted by Ni, Ray 1 year, 8 months ago
Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Xie, Yuanhao <yuanhao.xie@intel.com>
> Sent: Friday, August 19, 2022 2:17 PM
> To: devel@edk2.groups.io
> Cc: Xie, Yuanhao <yuanhao.xie@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R
> <rahul.r.kumar@intel.com>
> Subject: [Patch V2] UefiCpuPkg: Use Top of each AP's stack to save
> CpuMpData
> 
> From: Yuanhao Xie <yuanhao.xie@intel.com>
> 
> 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.
> 
> Change-Id: I7564279544622617c322310b4c7028ac0e11a9c4
> 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       |  8 ++++
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          | 37 ++++++++++++++-----
>  UefiCpuPkg/Library/MpInitLib/MpLib.h          |  8 ++++
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       | 10 +++--
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm |  9 +++++
>  5 files changed, 59 insertions(+), 13 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> index 28301bb8f0..2625d28401 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> @@ -179,6 +179,14 @@ ProgramStack:
>      mov         esp, dword [edi + CPU_INFO_IN_HOB.ApTopOfStack]
> 
>  CProcedureInvoke:
> +    ;
> +    ; Reserve 4 bytes for CpuMpData.
> +    ; When the AP wakes up again via INIT-SIPI-SIPI, push 0 will cause the
> existing CpuMpData to be overwritten with 0.
> +    ; CpuMpData is filled in via InitializeApData() during the first time INIT-
> SIPI-SIPI,
> +    ; while overwirrten may occurs when under ApInHltLoop but InitFlag is
> not set to ApInitConfig.
> +    ; Therefore reservation is implemented by sub esp instead of push 0.
> +    ;
> +    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..a4ce1c6d2e 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,14 +1806,22 @@ 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);
> 
> -  BufferSize  = ApStackSize * MaxLogicalProcessorNumber;
> +  BufferSize = ApStackSize * MaxLogicalProcessorNumber;
> +  //
> +  // Allocate extra ApStackSize to let AP stack align on ApStackSize bounday
> +  //
> +  BufferSize += ApStackSize;
>    BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber;
>    BufferSize += ApResetVectorSizeBelow1Mb;
>    BufferSize  = ALIGN_VALUE (BufferSize, 8);
> @@ -1813,13 +1831,13 @@ MpInitLibInitialize (
>    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 +1845,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
> @@ -1864,10 +1882,11 @@ MpInitLibInitialize (
> 
>    //
>    // Make sure no memory usage outside of the allocated buffer.
> +  // (ApStackSize - (Buffer - (UINTN)MpBuffer)) is the redundant caused by
> alignment
>    //
>    ASSERT (
>      (CpuMpData->CpuInfoInHob + sizeof (CPU_INFO_IN_HOB) *
> MaxLogicalProcessorNumber) ==
> -    Buffer + BufferSize
> +    (UINTN)MpBuffer + BufferSize - (ApStackSize - Buffer +
> (UINTN)MpBuffer)
>      );
> 
>    //
> 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..6751cae6f1 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> @@ -237,11 +237,20 @@ ProgramStack:
>      mov         rsp, qword [rdi + CPU_INFO_IN_HOB.ApTopOfStack]
> 
>  CProcedureInvoke:
> +    ;
> +    ; Reserve 8 bytes for CpuMpData.
> +    ; When the AP wakes up again via INIT-SIPI-SIPI, push 0 will cause the
> existing CpuMpData to be overwritten with 0.
> +    ; CpuMpData is filled in via InitializeApData() during the first time INIT-
> SIPI-SIPI,
> +    ; while overwirrten may occurs when under ApInHltLoop but InitFlag is
> not set to ApInitConfig.
> +    ; Therefore reservation is implemented by sub rsp instead of push 0.
> +    ;
> +    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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92801): https://edk2.groups.io/g/devel/message/92801
Mute This Topic: https://groups.io/mt/93119724/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-