public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch V2 0/6] Eliminate the second INIT-SIPI-SIPI sequence
@ 2023-06-25 14:39 Yuanhao Xie
  2023-06-25 14:39 ` [Patch V2 1/6] UefiCpuPkg: Refactor the logic for placing APs in HltLoop Yuanhao Xie
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Yuanhao Xie @ 2023-06-25 14:39 UTC (permalink / raw)
  To: devel

To speed up MP initialization, this set of patches replaces the 
time-consuming init-sipi-sipi process in the DXE phase. Instead, 
a start-up signal is used to wake up the APs and switch context
 from the PEI phase to the DXE phase. This optimization is 
 effective when both PEI and DXE operate in the same bit mode. 

The current HOB characterized by mCpuInitMpLibHobGuid has 
two purposes:
  Acting as a global variable for the PEI phase.
  Transferring information from the PEI phase to the DXE phase.
This series of patches creates a new HOB specifically designed 
to transfer only the minimal necessary information 
(MpHandoff structure) from the PEI phase to the DXE phase.

YuanhaoXie (6):
  UefiCpuPkg: Refactor the logic for placing APs in HltLoop.
  UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop.
  UefiCpuPkg: Create MpHandOff.
  UefiCpuPkg: ApWakeupFunction directly use CpuMpData.
  UefiCpuPkg: Eliminate the second INIT-SIPI-SIPI sequence.
  UefiCpuPkg: Enhance MpHandOff Handling.

 UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm |   4 ++--
 UefiCpuPkg/Library/MpInitLib/MpLib.c           | 374 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------------------------
 UefiCpuPkg/Library/MpInitLib/MpLib.h           |  71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c        |  54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm  |   3 +--
 5 files changed, 433 insertions(+), 73 deletions(-)

-- 
2.36.1.windows.1


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

* [Patch V2 1/6] UefiCpuPkg: Refactor the logic for placing APs in HltLoop.
  2023-06-25 14:39 [Patch V2 0/6] Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
@ 2023-06-25 14:39 ` Yuanhao Xie
  2023-06-25 14:39 ` [Patch V2 2/6] UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop Yuanhao Xie
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Yuanhao Xie @ 2023-06-25 14:39 UTC (permalink / raw)
  To: devel
  Cc: Gerd Hoffmann, Eric Dong, Ray Ni, Rahul Kumar, Tom Lendacky,
	Yuanhao Xie

Refactor the logic for placing APs in HltLoop into a separate function.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index f1f2840714..9560b39220 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -636,6 +636,28 @@ InitializeApData (
   SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateIdle);
 }
 
+/**
+  This function place APs in Halt loop.
+
+  @param[in] CpuMpData        Pointer to CPU MP Data
+**/
+VOID
+PlaceAPInHltLoop (
+  IN CPU_MP_DATA  *CpuMpData
+  )
+{
+  while (TRUE) {
+    DisableInterrupts ();
+    if (CpuMpData->UseSevEsAPMethod) {
+      SevEsPlaceApHlt (CpuMpData);
+    } else {
+      CpuSleep ();
+    }
+
+    CpuPause ();
+  }
+}
+
 /**
   This function will be called from AP reset code if BSP uses WakeUpAP.
 
@@ -812,19 +834,10 @@ ApWakeupFunction (
     // Place AP is specified loop mode
     //
     if (CpuMpData->ApLoopMode == ApInHltLoop) {
+      PlaceAPInHltLoop (CpuMpData);
       //
-      // Place AP in HLT-loop
+      // Never run here
       //
-      while (TRUE) {
-        DisableInterrupts ();
-        if (CpuMpData->UseSevEsAPMethod) {
-          SevEsPlaceApHlt (CpuMpData);
-        } else {
-          CpuSleep ();
-        }
-
-        CpuPause ();
-      }
     }
 
     while (TRUE) {
-- 
2.36.1.windows.1


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

* [Patch V2 2/6] UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop.
  2023-06-25 14:39 [Patch V2 0/6] Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
  2023-06-25 14:39 ` [Patch V2 1/6] UefiCpuPkg: Refactor the logic for placing APs in HltLoop Yuanhao Xie
@ 2023-06-25 14:39 ` Yuanhao Xie
  2023-06-25 14:39 ` [Patch V2 3/6] UefiCpuPkg: Create MpHandOff Yuanhao Xie
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Yuanhao Xie @ 2023-06-25 14:39 UTC (permalink / raw)
  To: devel
  Cc: Gerd Hoffmann, Eric Dong, Ray Ni, Rahul Kumar, Tom Lendacky,
	Yuanhao Xie

Refactor the logic for placing APs in
Mwait/Runloop into a separate function.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------
 1 file changed, 50 insertions(+), 33 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 9560b39220..e8dd640f9b 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -658,6 +658,54 @@ PlaceAPInHltLoop (
   }
 }
 
+/**
+  This function place APs in Mwait or Run loop.
+
+  @param[in] ApLoopMode                   Ap Loop Mode
+  @param[in] ApStartupSignalBuffer        Pointer to Ap Startup Signal Buffer
+  @param[in] ApTargetCState               Ap Target CState
+**/
+VOID
+PlaceAPInMwaitLoopOrRunLoop (
+  IN UINT8            ApLoopMode,
+  IN volatile UINT32  *ApStartupSignalBuffer,
+  IN UINT8            ApTargetCState
+  )
+{
+  while (TRUE) {
+    DisableInterrupts ();
+    if (ApLoopMode == ApInMwaitLoop) {
+      //
+      // Place AP in MWAIT-loop
+      //
+      AsmMonitor ((UINTN)ApStartupSignalBuffer, 0, 0);
+      if (*ApStartupSignalBuffer != WAKEUP_AP_SIGNAL) {
+        //
+        // Check AP start-up signal again.
+        // If AP start-up signal is not set, place AP into
+        // the specified C-state
+        //
+        AsmMwait (ApTargetCState << 4, 0);
+      }
+    } else if (ApLoopMode == ApInRunLoop) {
+      //
+      // Place AP in Run-loop
+      //
+      CpuPause ();
+    } else {
+      ASSERT (FALSE);
+    }
+
+    //
+    // If AP start-up signal is written, AP is waken up
+    // otherwise place AP in loop again
+    //
+    if (*ApStartupSignalBuffer == WAKEUP_AP_SIGNAL) {
+      break;
+    }
+  }
+}
+
 /**
   This function will be called from AP reset code if BSP uses WakeUpAP.
 
@@ -838,39 +886,8 @@ ApWakeupFunction (
       //
       // Never run here
       //
-    }
-
-    while (TRUE) {
-      DisableInterrupts ();
-      if (CpuMpData->ApLoopMode == ApInMwaitLoop) {
-        //
-        // Place AP in MWAIT-loop
-        //
-        AsmMonitor ((UINTN)ApStartupSignalBuffer, 0, 0);
-        if (*ApStartupSignalBuffer != WAKEUP_AP_SIGNAL) {
-          //
-          // Check AP start-up signal again.
-          // If AP start-up signal is not set, place AP into
-          // the specified C-state
-          //
-          AsmMwait (CpuMpData->ApTargetCState << 4, 0);
-        }
-      } else if (CpuMpData->ApLoopMode == ApInRunLoop) {
-        //
-        // Place AP in Run-loop
-        //
-        CpuPause ();
-      } else {
-        ASSERT (FALSE);
-      }
-
-      //
-      // If AP start-up signal is written, AP is waken up
-      // otherwise place AP in loop again
-      //
-      if (*ApStartupSignalBuffer == WAKEUP_AP_SIGNAL) {
-        break;
-      }
+    } else {
+      PlaceAPInMwaitLoopOrRunLoop (CpuMpData->ApLoopMode, ApStartupSignalBuffer, CpuMpData->ApTargetCState);
     }
   }
 }
-- 
2.36.1.windows.1


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

* [Patch V2 3/6] UefiCpuPkg: Create MpHandOff.
  2023-06-25 14:39 [Patch V2 0/6] Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
  2023-06-25 14:39 ` [Patch V2 1/6] UefiCpuPkg: Refactor the logic for placing APs in HltLoop Yuanhao Xie
  2023-06-25 14:39 ` [Patch V2 2/6] UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop Yuanhao Xie
@ 2023-06-25 14:39 ` Yuanhao Xie
  2023-06-26 11:21   ` Gerd Hoffmann
  2023-06-25 14:39 ` [Patch V2 4/6] UefiCpuPkg: ApWakeupFunction directly use CpuMpData Yuanhao Xie
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Yuanhao Xie @ 2023-06-25 14:39 UTC (permalink / raw)
  To: devel
  Cc: Gerd Hoffmann, Eric Dong, Ray Ni, Rahul Kumar, Tom Lendacky,
	Yuanhao Xie

Initially, the purpose of the Hob was twofold: it served as a way to
transfer information from PEI to DXE. However, during the DXE phase,
only a few fields from the CPU_MP_DATA which collected in PEI phase were
 needed. A new Hob was specifically created to transfer information
 to the DXE phase. This new Hob contained only the essential fields
 required for reuse in DXE. For instance, instead of directly including
  the BspNumber in MpHandOff, the DXE phase introduced the use of
  GetBspNumber() to collect the BspNumber from ApicID and CpuCount.

The SaveCpuMpData() function was updated to construct the MP_HAND_OFF
Hob. Additionally, the function introduced the MP_HAND_OFF_SIGNAL,
which solely served the purpose of awakening the APs
and transitioning their context from PEI to DXE. The
WaitLoopExecutionMode field indicated whether the bit mode of PEI
matched that of DXE. Both of them were filled only if the ApLoopMode
was not ApInHltLoop. In the case of ApLoopMode, it remained necessary
to wake up the APs using the init-sipi-sipi sequence.

The function GetMpHandOffHob() was added to facilitate access to the
collected MpHandOff in the DXE phase. The CpuMpData in the DXE phase
was updated by gathering information from MpHandOff. Since MpHandOff
replaced the usage of OldCpuMpData and contained essential information
from the PEI phase to the DXE phase. AmdSevUpdateCpuMpData was included
to maintain the original implementation of AmdSev, ensuring that
OldCpuMpData->NewCpuMpData pointed to CpuMpData.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c    | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 UefiCpuPkg/Library/MpInitLib/MpLib.h    |  66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c |  32 +++++++++++++++++++++++++++++++-
 3 files changed, 184 insertions(+), 14 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index e8dd640f9b..1252ee9673 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -14,6 +14,7 @@
 #include <Register/Amd/Ghcb.h>
 
 EFI_GUID  mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID;
+EFI_GUID  mMpHandOffGuid       = MP_HANDOFF_GUID;
 
 /**
   Save the volatile registers required to be restored following INIT IPI.
@@ -1814,6 +1815,54 @@ CheckAllAPs (
   return EFI_NOT_READY;
 }
 
+/**
+  This function Get BspNumber.
+
+  @param[in] MpHandOff        Pointer to MpHandOff
+  @return                     BspNumber
+**/
+UINT32
+GetBspNumber (
+  IN CONST MP_HAND_OFF  *MpHandOff
+  )
+{
+  UINT32  ApicId;
+  UINT32  BspNumber;
+  UINT32  Index;
+
+  //
+  // Get the processor number for the BSP
+  //
+  BspNumber = MAX_UINT32;
+  ApicId    = GetInitialApicId ();
+  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
+    if (MpHandOff->Info[Index].ApicId == ApicId) {
+      BspNumber = Index;
+    }
+  }
+
+  ASSERT (BspNumber != MAX_UINT32);
+
+  return BspNumber;
+}
+
+/**
+  Get pointer to CPU MP Data structure from GUIDed HOB.
+
+  @param[in] CpuMpData  The pointer to CPU MP Data structure.
+**/
+VOID
+AmdSevUpdateCpuMpData (
+  IN CPU_MP_DATA  *CpuMpData
+  )
+{
+  CPU_MP_DATA  *OldCpuMpData;
+
+  OldCpuMpData = GetCpuMpDataFromGuidedHob ();
+
+  OldCpuMpData->NewCpuMpData = CpuMpData;
+}
+
 /**
   MP Initialize Library initialization.
 
@@ -1833,7 +1882,7 @@ MpInitLibInitialize (
   VOID
   )
 {
-  CPU_MP_DATA              *OldCpuMpData;
+  MP_HAND_OFF              *MpHandOff;
   CPU_INFO_IN_HOB          *CpuInfoInHob;
   UINT32                   MaxLogicalProcessorNumber;
   UINT32                   ApStackSize;
@@ -1852,11 +1901,11 @@ MpInitLibInitialize (
   UINTN                    BackupBufferAddr;
   UINTN                    ApIdtBase;
 
-  OldCpuMpData = GetCpuMpDataFromGuidedHob ();
-  if (OldCpuMpData == NULL) {
+  MpHandOff = GetMpHandOffHob ();
+  if (MpHandOff == NULL) {
     MaxLogicalProcessorNumber = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
   } else {
-    MaxLogicalProcessorNumber = OldCpuMpData->CpuCount;
+    MaxLogicalProcessorNumber = MpHandOff->CpuCount;
   }
 
   ASSERT (MaxLogicalProcessorNumber != 0);
@@ -2000,7 +2049,7 @@ MpInitLibInitialize (
   //
   ProgramVirtualWireMode ();
 
-  if (OldCpuMpData == NULL) {
+  if (MpHandOff == NULL) {
     if (MaxLogicalProcessorNumber > 1) {
       //
       // Wakeup all APs and calculate the processor count in system
@@ -2012,15 +2061,18 @@ MpInitLibInitialize (
     // APs have been wakeup before, just get the CPU Information
     // from HOB
     //
-    OldCpuMpData->NewCpuMpData = CpuMpData;
-    CpuMpData->CpuCount        = OldCpuMpData->CpuCount;
-    CpuMpData->BspNumber       = OldCpuMpData->BspNumber;
-    CpuMpData->CpuInfoInHob    = OldCpuMpData->CpuInfoInHob;
-    CpuInfoInHob               = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
+    AmdSevUpdateCpuMpData (CpuMpData);
+    CpuMpData->CpuCount  = MpHandOff->CpuCount;
+    CpuMpData->BspNumber = GetBspNumber (MpHandOff);
+    CpuInfoInHob         = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
     for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
       InitializeSpinLock (&CpuMpData->CpuData[Index].ApLock);
-      CpuMpData->CpuData[Index].CpuHealthy = (CpuInfoInHob[Index].Health == 0) ? TRUE : FALSE;
+      CpuMpData->CpuData[Index].CpuHealthy = (MpHandOff->Info[Index].Health == 0) ? TRUE : FALSE;
       CpuMpData->CpuData[Index].ApFunction = 0;
+      CpuInfoInHob[Index].InitialApicId    = MpHandOff->Info[Index].ApicId;
+      CpuInfoInHob[Index].ApTopOfStack     = CpuMpData->Buffer + (Index + 1) * CpuMpData->CpuApStackSize;
+      CpuInfoInHob[Index].ApicId           = MpHandOff->Info[Index].ApicId;
+      CpuInfoInHob[Index].Health           = MpHandOff->Info[Index].Health;
     }
   }
 
@@ -2049,7 +2101,7 @@ MpInitLibInitialize (
   // Wakeup APs to do some AP initialize sync (Microcode & MTRR)
   //
   if (CpuMpData->CpuCount > 1) {
-    if (OldCpuMpData != NULL) {
+    if (MpHandOff != NULL) {
       //
       // Only needs to use this flag for DXE phase to update the wake up
       // buffer. Wakeup buffer allocated in PEI phase is no longer valid
@@ -2066,7 +2118,7 @@ MpInitLibInitialize (
       CpuPause ();
     }
 
-    if (OldCpuMpData != NULL) {
+    if (MpHandOff != NULL) {
       CpuMpData->InitFlag = ApInitDone;
     }
 
@@ -2793,6 +2845,28 @@ StartupThisAPWorker (
   return Status;
 }
 
+/**
+  Get pointer to MP_HAND_OFF GUIDed HOB.
+
+  @return  The pointer to MP_HAND_OFF structure.
+**/
+MP_HAND_OFF *
+GetMpHandOffHob (
+  VOID
+  )
+{
+  EFI_HOB_GUID_TYPE  *GuidHob;
+  MP_HAND_OFF        *MpHandOff;
+
+  MpHandOff = NULL;
+  GuidHob   = GetFirstGuidHob (&mMpHandOffGuid);
+  if (GuidHob != NULL) {
+    MpHandOff = (MP_HAND_OFF *)GET_GUID_HOB_DATA (GuidHob);
+  }
+
+  return MpHandOff;
+}
+
 /**
   Get pointer to CPU MP Data structure from GUIDed HOB.
 
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index b694c7b40f..daf9e62012 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -41,12 +41,25 @@
 #include <Guid/MicrocodePatchHob.h>
 
 #define WAKEUP_AP_SIGNAL  SIGNATURE_32 ('S', 'T', 'A', 'P')
+//
+// To trigger the start-up signal, BSP writes the specified
+// StartupSignalValue to the StartupSignalAddress of each processor.
+// This address is monitored by the APs, and as soon as they receive
+// the value that matches the MP_HAND_OFF_SIGNAL, they will wake up
+// and switch the context from PEI to DXE phase.
+//
+#define MP_HAND_OFF_SIGNAL  SIGNATURE_32 ('M', 'P', 'H', 'O')
 
 #define CPU_INIT_MP_LIB_HOB_GUID \
   { \
     0x58eb6a19, 0x3699, 0x4c68, { 0xa8, 0x36, 0xda, 0xcd, 0x8e, 0xdc, 0xad, 0x4a } \
   }
 
+#define MP_HANDOFF_GUID \
+  { \
+    0x11e2bd88, 0xed38, 0x4abd, {0xa3, 0x99, 0x21, 0xf2, 0x5f, 0xd0, 0x7a, 0x60 } \
+  }
+
 //
 //  The MP data for switch BSP
 //
@@ -59,6 +72,37 @@
 //
 #define DEFAULT_MAX_MICROCODE_PATCH_NUM  8
 
+//
+// The information required to transfer from the PEI phase to the
+// DXE phase is contained within the MP_HAND_OFF and PROCESSOR_HAND_OFF.
+// If the SizeOfPointer (WaitLoopExecutionMode) of both phases are equal,
+// and the APs is not in halt mode,
+// then the APs can be awakened by triggering the start-up
+// signal, rather than using INIT-SIPI-SIPI.
+// To trigger the start-up signal, BSP writes the specified
+// StartupSignalValue to the StartupSignalAddress of each processor.
+// This address is monitored by the APs.
+//
+typedef struct {
+  UINT32    ApicId;
+  UINT32    Health;
+  UINT64    StartupSignalAddress;
+  UINT64    StartupProcedureAddress;
+} PROCESSOR_HAND_OFF;
+
+typedef struct {
+  //
+  // The ProcessorIndex indicates the range of processors. If it is set to 0, it signifies
+  // processors from 0 to CpuCount - 1. Multiple instances in the HOB list describe
+  // processors from ProcessorIndex to ProcessorIndex + CpuCount - 1.
+  //
+  UINT32                ProcessorIndex;
+  UINT32                CpuCount;
+  UINT32                WaitLoopExecutionMode;
+  UINT32                StartupSignalValue;
+  PROCESSOR_HAND_OFF    Info[];
+} MP_HAND_OFF;
+
 //
 // Data structure for microcode patch information
 //
@@ -347,6 +391,7 @@ typedef
   );
 
 extern EFI_GUID  mCpuInitMpLibHobGuid;
+extern EFI_GUID  mMpHandOffGuid;
 
 /**
   Assembly code to place AP into safe loop mode.
@@ -452,6 +497,17 @@ SaveCpuMpData (
   IN CPU_MP_DATA  *CpuMpData
   );
 
+/**
+  This function Get BspNumber.
+
+  @param[in] MpHandOff        Pointer to MpHandOff
+  @return                     BspNumber
+**/
+UINT32
+GetBspNumber (
+  IN CONST MP_HAND_OFF  *MpHandOff
+  );
+
 /**
   Get available system memory below 1MB by specified size.
 
@@ -652,6 +708,16 @@ EnableDisableApWorker (
   IN  UINT32   *HealthFlag OPTIONAL
   );
 
+/**
+  Get pointer to MP_HAND_OFF GUIDed HOB.
+
+  @return  The pointer to MP_HAND_OFF structure.
+**/
+MP_HAND_OFF *
+GetMpHandOffHob (
+  VOID
+  );
+
 /**
   Get pointer to CPU MP Data structure from GUIDed HOB.
 
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 013f89b197..f80e00edcf 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -126,7 +126,37 @@ SaveCpuMpData (
   IN CPU_MP_DATA  *CpuMpData
   )
 {
-  UINT64  Data64;
+  UINT64           Data64;
+  UINTN            Index;
+  CPU_INFO_IN_HOB  *CpuInfoInHob;
+  MP_HAND_OFF      *MpHandOff;
+  UINTN            MpHandOffSize;
+
+  //
+  // When APs are in a state that can be waken up by a store operation to a memory address,
+  // report the MP_HAND_OFF data for DXE to use.
+  //
+  CpuInfoInHob  = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
+  MpHandOffSize = sizeof (MP_HAND_OFF) + sizeof (PROCESSOR_HAND_OFF) * CpuMpData->CpuCount;
+  MpHandOff     = (MP_HAND_OFF *)BuildGuidHob (&mMpHandOffGuid, MpHandOffSize);
+  ASSERT (MpHandOff != NULL);
+  ZeroMem (MpHandOff, MpHandOffSize);
+  MpHandOff->ProcessorIndex = 0;
+
+  MpHandOff->CpuCount = CpuMpData->CpuCount;
+  if (CpuMpData->ApLoopMode != ApInHltLoop) {
+    MpHandOff->StartupSignalValue    = MP_HAND_OFF_SIGNAL;
+    MpHandOff->WaitLoopExecutionMode = sizeof (VOID *);
+  }
+
+  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
+    MpHandOff->Info[Index].ApicId = CpuInfoInHob[Index].ApicId;
+    MpHandOff->Info[Index].Health = CpuInfoInHob[Index].Health;
+    if (CpuMpData->ApLoopMode != ApInHltLoop) {
+      MpHandOff->Info[Index].StartupSignalAddress    = (UINT64)(UINTN)CpuMpData->CpuData[Index].StartupApSignal;
+      MpHandOff->Info[Index].StartupProcedureAddress = (UINT64)(UINTN)&CpuMpData->CpuData[Index].ApFunction;
+    }
+  }
 
   //
   // Build location of CPU MP DATA buffer in HOB
-- 
2.36.1.windows.1


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

* [Patch V2 4/6] UefiCpuPkg: ApWakeupFunction directly use CpuMpData.
  2023-06-25 14:39 [Patch V2 0/6] Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
                   ` (2 preceding siblings ...)
  2023-06-25 14:39 ` [Patch V2 3/6] UefiCpuPkg: Create MpHandOff Yuanhao Xie
@ 2023-06-25 14:39 ` Yuanhao Xie
  2023-06-25 14:39 ` [Patch V2 5/6] UefiCpuPkg: Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
  2023-06-25 14:39 ` [Patch V2 6/6] UefiCpuPkg: Enhance MpHandOff Handling Yuanhao Xie
  5 siblings, 0 replies; 11+ messages in thread
From: Yuanhao Xie @ 2023-06-25 14:39 UTC (permalink / raw)
  To: devel
  Cc: Gerd Hoffmann, Eric Dong, Ray Ni, Rahul Kumar, Tom Lendacky,
	Yuanhao Xie

In the original design, once the APs finished executing their assembly
code and switched to executing C code, they would enter a continuous
loop within a function. In this function, they would collect CpuMpData
using the MP_CPU_EXCHANGE_INFO mechanism. However, in the updated
approach, CpuMpData can now be passed directly to the ApWakeUpFunction,
bypassing the need for MP_CPU_EXCHANGE_INFO. This modification is made
in preparation for eliminating the requirement of a second
INIT-SIPI-SIPI sequence in the DXE phase.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm |  4 ++--
 UefiCpuPkg/Library/MpInitLib/MpLib.c           | 12 +++---------
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm  |  3 +--
 3 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 59db4081d6..d117f09ef5 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -197,8 +197,8 @@ CProcedureInvoke:
 
     push       ebx               ; Push ApIndex
     mov        eax, esi
-    add        eax, MP_CPU_EXCHANGE_INFO_OFFSET
-    push       eax               ; push address of exchange info data buffer
+    add        eax, MP_CPU_EXCHANGE_INFO_FIELD (CpuMpData)
+    push       dword [eax]       ; push address of CpuMpData
 
     mov        edi, esi
     add        edi, MP_CPU_EXCHANGE_INFO_FIELD (CFunction)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 1252ee9673..15b24b1805 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -710,17 +710,16 @@ PlaceAPInMwaitLoopOrRunLoop (
 /**
   This function will be called from AP reset code if BSP uses WakeUpAP.
 
-  @param[in] ExchangeInfo     Pointer to the MP exchange info buffer
+  @param[in] CpuMpData        Pointer to CPU MP Data
   @param[in] ApIndex          Number of current executing AP
 **/
 VOID
 EFIAPI
 ApWakeupFunction (
-  IN MP_CPU_EXCHANGE_INFO  *ExchangeInfo,
-  IN UINTN                 ApIndex
+  IN CPU_MP_DATA  *CpuMpData,
+  IN UINTN        ApIndex
   )
 {
-  CPU_MP_DATA       *CpuMpData;
   UINTN             ProcessorNumber;
   EFI_AP_PROCEDURE  Procedure;
   VOID              *Parameter;
@@ -731,11 +730,6 @@ ApWakeupFunction (
   UINTN             CurrentApicMode;
   AP_STACK_DATA     *ApStackData;
 
-  //
-  // AP finished assembly code and begin to execute C code
-  //
-  CpuMpData = ExchangeInfo->CpuMpData;
-
   //
   // AP's local APIC settings will be lost after received INIT IPI
   // We need to re-initialize them at here
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 5bcdf7726b..40e80ffab4 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -259,8 +259,7 @@ CProcedureInvoke:
     add        rsp, 20h
 
     mov        edx, ebx          ; edx is ApIndex
-    mov        ecx, esi
-    add        ecx, MP_CPU_EXCHANGE_INFO_OFFSET ; rcx is address of exchange info data buffer
+    mov        rcx, qword [esi + MP_CPU_EXCHANGE_INFO_FIELD (CpuMpData)]
 
     mov        edi, esi
     add        edi, MP_CPU_EXCHANGE_INFO_FIELD (CFunction)
-- 
2.36.1.windows.1


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

* [Patch V2 5/6] UefiCpuPkg: Eliminate the second INIT-SIPI-SIPI sequence.
  2023-06-25 14:39 [Patch V2 0/6] Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
                   ` (3 preceding siblings ...)
  2023-06-25 14:39 ` [Patch V2 4/6] UefiCpuPkg: ApWakeupFunction directly use CpuMpData Yuanhao Xie
@ 2023-06-25 14:39 ` Yuanhao Xie
  2023-06-25 14:39 ` [Patch V2 6/6] UefiCpuPkg: Enhance MpHandOff Handling Yuanhao Xie
  5 siblings, 0 replies; 11+ messages in thread
From: Yuanhao Xie @ 2023-06-25 14:39 UTC (permalink / raw)
  To: devel
  Cc: Gerd Hoffmann, Eric Dong, Ray Ni, Rahul Kumar, Tom Lendacky,
	Yuanhao Xie

When both the PEI and DXE phases operate in the same execution
mode(32-bit/64-bit), the BSP send a special start-up signal during
the DXE phase to awaken the Application APs.

To eliminate the need for the INIT-SIPI-SIPI sequence at the beginning
of the DXE phase, the BSP call the SwitchApContext function to trigger
the special  start-up signal. By writing the specified
StartupSignalValue to the designated StartupSignalAddress, the BSP
wakes up the APs from mwait mode. Once the APs receive the
MP_HAND_OFF_SIGNAL value, they are awakened and proceed to execute the
SwitchContextPerAp procedure. They enter another while loop,
transitioning their context from the PEI phase to the DXE phase.

The original state transitions for an AP during the procedure are as
follows:
Idle ----> Ready ----> Busy ----> Idle
      [BSP]      [AP]      [AP]

Instead of init-sipi-sipi sequence, we make use of a
start-up signal to awaken the APs and transfer their context from
PEI to DXE. Consequently, APs, rather than the BSP, to set their state
to CpuStateReady.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 UefiCpuPkg/Library/MpInitLib/MpLib.h |   9 +++++++++
 2 files changed, 143 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 15b24b1805..391e6f19ef 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -680,7 +680,7 @@ PlaceAPInMwaitLoopOrRunLoop (
       // Place AP in MWAIT-loop
       //
       AsmMonitor ((UINTN)ApStartupSignalBuffer, 0, 0);
-      if (*ApStartupSignalBuffer != WAKEUP_AP_SIGNAL) {
+      if ((*ApStartupSignalBuffer != WAKEUP_AP_SIGNAL) && (*ApStartupSignalBuffer != MP_HAND_OFF_SIGNAL)) {
         //
         // Check AP start-up signal again.
         // If AP start-up signal is not set, place AP into
@@ -701,7 +701,7 @@ PlaceAPInMwaitLoopOrRunLoop (
     // If AP start-up signal is written, AP is waken up
     // otherwise place AP in loop again
     //
-    if (*ApStartupSignalBuffer == WAKEUP_AP_SIGNAL) {
+    if ((*ApStartupSignalBuffer == WAKEUP_AP_SIGNAL) || (*ApStartupSignalBuffer == MP_HAND_OFF_SIGNAL)) {
       break;
     }
   }
@@ -729,6 +729,7 @@ ApWakeupFunction (
   UINT64            ApTopOfStack;
   UINTN             CurrentApicMode;
   AP_STACK_DATA     *ApStackData;
+  UINT32            OriginalValue;
 
   //
   // AP's local APIC settings will be lost after received INIT IPI
@@ -769,6 +770,15 @@ ApWakeupFunction (
       // Clear AP start-up signal when AP waken up
       //
       ApStartupSignalBuffer = CpuMpData->CpuData[ProcessorNumber].StartupApSignal;
+      OriginalValue         = InterlockedCompareExchange32 (
+                                (UINT32 *)ApStartupSignalBuffer,
+                                MP_HAND_OFF_SIGNAL,
+                                0
+                                );
+      if (OriginalValue == MP_HAND_OFF_SIGNAL) {
+        SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateReady);
+      }
+
       InterlockedCompareExchange32 (
         (UINT32 *)ApStartupSignalBuffer,
         WAKEUP_AP_SIGNAL,
@@ -887,6 +897,32 @@ ApWakeupFunction (
   }
 }
 
+/**
+  This function serves as the entry point for APs when
+  they are awakened by the stores in the memory address
+  indicated by the MP_HANDOFF_INFO structure.
+
+  @param[in] CpuMpData        Pointer to PEI CPU MP Data
+**/
+VOID
+EFIAPI
+DxeApEntryPoint (
+  CPU_MP_DATA  *CpuMpData
+  )
+{
+  UINTN  ProcessorNumber;
+
+  GetProcessorNumber (CpuMpData, &ProcessorNumber);
+  InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount);
+  RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE);
+  PlaceAPInMwaitLoopOrRunLoop (
+    CpuMpData->ApLoopMode,
+    CpuMpData->CpuData[ProcessorNumber].StartupApSignal,
+    CpuMpData->ApTargetCState
+    );
+  ApWakeupFunction (CpuMpData, ProcessorNumber);
+}
+
 /**
   Wait for AP wakeup and write AP start-up signal till AP is waken up.
 
@@ -1457,6 +1493,70 @@ CalculateTimeout (
   }
 }
 
+/**
+  Switch Context for each AP.
+
+**/
+VOID
+EFIAPI
+SwitchContextPerAp (
+  VOID
+  )
+{
+  UINTN            ProcessorNumber;
+  CPU_MP_DATA      *CpuMpData;
+  CPU_INFO_IN_HOB  *CpuInfoInHob;
+
+  CpuMpData    = GetCpuMpData ();
+  CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
+  GetProcessorNumber (CpuMpData, &ProcessorNumber);
+
+  SwitchStack (
+    (SWITCH_STACK_ENTRY_POINT)(UINTN)DxeApEntryPoint,
+    (VOID *)(UINTN)CpuMpData,
+    NULL,
+    (VOID *)((UINTN)CpuInfoInHob[ProcessorNumber].ApTopOfStack)
+    );
+}
+
+/**
+  This function is intended to be invoked by the BSP in order
+  to wake up the AP. The BSP accomplishes this by triggering a
+  start-up signal, which in turn causes any APs that are
+  currently in a loop on the PEI-prepared memory to awaken and
+  begin running the procedure called SwitchContextPerAp.
+  This procedure allows the AP to switch to another section of
+  memory and continue its loop there.
+
+  @param[in] MpHandOff  Pointer to MP hand-off data structure.
+**/
+VOID
+SwitchApContext (
+  IN MP_HAND_OFF  *MpHandOff
+  )
+{
+  UINTN   Index;
+  UINT32  BspNumber;
+
+  BspNumber = GetBspNumber (MpHandOff);
+
+  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
+    if (Index != BspNumber) {
+      *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress = (UINTN)SwitchContextPerAp;
+      *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   = MpHandOff->StartupSignalValue;
+    }
+  }
+
+  //
+  // Wait all APs waken up if this is not the 1st broadcast of SIPI
+  //
+  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
+    if (Index != BspNumber) {
+      WaitApWakeup ((UINT32 *)(UINTN)(MpHandOff->Info[Index].StartupSignalAddress));
+    }
+  }
+}
+
 /**
   Checks whether timeout expires.
 
@@ -2068,6 +2168,38 @@ MpInitLibInitialize (
       CpuInfoInHob[Index].ApicId           = MpHandOff->Info[Index].ApicId;
       CpuInfoInHob[Index].Health           = MpHandOff->Info[Index].Health;
     }
+
+    DEBUG ((DEBUG_INFO, "MpHandOff->WaitLoopExecutionMode: %04d, sizeof (VOID *): %04d\n", MpHandOff->WaitLoopExecutionMode, sizeof (VOID *)));
+    if (MpHandOff->WaitLoopExecutionMode == sizeof (VOID *)) {
+      //
+      // In scenarios where both the PEI and DXE phases run in the same
+      // execution mode (32bit or 64bit), the BSP triggers
+      // a start-up signal during the DXE phase to wake up the APs. This causes any
+      // APs that are currently in a loop on the memory prepared during the PEI
+      // phase to awaken and run the SwitchContextPerAp procedure. This procedure
+      // enables the APs to switch to a different memory section and continue their
+      // looping process there.
+      //
+      CpuMpData->FinishedCount = 0;
+      CpuMpData->InitFlag      = ApInitDone;
+      SaveCpuMpData (CpuMpData);
+      SwitchApContext (MpHandOff);
+      ASSERT (CpuMpData->FinishedCount == (CpuMpData->CpuCount - 1));
+
+      //
+      // Set Apstate as Idle, otherwise Aps cannot be waken-up again.
+      // If any enabled AP is not idle, return EFI_NOT_READY during waken-up.
+      //
+      for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+        SetApState (&CpuMpData->CpuData[Index], CpuStateIdle);
+      }
+
+      //
+      // Initialize global data for MP support
+      //
+      InitMpGlobalData (CpuMpData);
+      return EFI_SUCCESS;
+    }
   }
 
   if (!GetMicrocodePatchInfoFromHob (
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index daf9e62012..468d3a5925 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -521,6 +521,15 @@ GetWakeupBuffer (
   IN UINTN  WakeupBufferSize
   );
 
+/**
+  Switch Context for each AP.
+
+**/
+VOID
+SwitchApContext (
+  IN MP_HAND_OFF  *MpHandOff
+  );
+
 /**
   Get available EfiBootServicesCode memory below 4GB by specified size.
 
-- 
2.36.1.windows.1


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

* [Patch V2 6/6] UefiCpuPkg: Enhance MpHandOff Handling.
  2023-06-25 14:39 [Patch V2 0/6] Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
                   ` (4 preceding siblings ...)
  2023-06-25 14:39 ` [Patch V2 5/6] UefiCpuPkg: Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
@ 2023-06-25 14:39 ` Yuanhao Xie
  2023-06-26  3:14   ` Ni, Ray
  5 siblings, 1 reply; 11+ messages in thread
From: Yuanhao Xie @ 2023-06-25 14:39 UTC (permalink / raw)
  To: devel
  Cc: Gerd Hoffmann, Eric Dong, Ray Ni, Rahul Kumar, Tom Lendacky,
	Yuanhao Xie

Enhance MpHandOff Handling for Systems with a Large Number of
Processors.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c    | 128 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------
 UefiCpuPkg/Library/MpInitLib/MpLib.h    |  22 +++++++++-------------
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c |  56 +++++++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 116 insertions(+), 90 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 391e6f19ef..087bf3ea92 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1528,32 +1528,48 @@ SwitchContextPerAp (
   This procedure allows the AP to switch to another section of
   memory and continue its loop there.
 
-  @param[in] MpHandOff  Pointer to MP hand-off data structure.
+  @param[in] BspNumber  Bsp Number
 **/
 VOID
 SwitchApContext (
-  IN MP_HAND_OFF  *MpHandOff
+  IN UINT32  BspNumber
   )
 {
-  UINTN   Index;
-  UINT32  BspNumber;
-
-  BspNumber = GetBspNumber (MpHandOff);
+  UINTN        Index;
+  UINTN        LimitationOfMpHandOffHob;
+  UINTN        NumberOfProcessorsInCurrentHob;
+  UINTN        CpuCountInCurrentHob;
+  MP_HAND_OFF  *MpHandOff;
+  BOOLEAN      BspFound;
+
+  BspFound = FALSE;
+
+  MpHandOff                = GetMpHandOffHob ();
+  CpuCountInCurrentHob     = 0;
+  LimitationOfMpHandOffHob = (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE) - sizeof (MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
+  while (CpuCountInCurrentHob < MpHandOff->CpuCount) {
+    //
+    // Get the processor number for the BSP
+    //
+    NumberOfProcessorsInCurrentHob = MIN (MpHandOff->CpuCount - CpuCountInCurrentHob, LimitationOfMpHandOffHob);
 
-  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
-    if (Index != BspNumber) {
-      *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress = (UINTN)SwitchContextPerAp;
-      *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   = MpHandOff->StartupSignalValue;
+    for (Index = 0; Index < NumberOfProcessorsInCurrentHob; Index++) {
+      if ((Index == BspNumber) && (BspFound == FALSE)) {
+        BspFound = TRUE;
+      } else {
+        *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress = (UINTN)SwitchContextPerAp;
+        *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   = MpHandOff->StartupSignalValue;
+        WaitApWakeup ((UINT32 *)(UINTN)(MpHandOff->Info[Index].StartupSignalAddress));
+      }
     }
-  }
 
-  //
-  // Wait all APs waken up if this is not the 1st broadcast of SIPI
-  //
-  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
-    if (Index != BspNumber) {
-      WaitApWakeup ((UINT32 *)(UINTN)(MpHandOff->Info[Index].StartupSignalAddress));
+    if (CpuCountInCurrentHob + NumberOfProcessorsInCurrentHob >= MpHandOff->CpuCount) {
+      break;
     }
+
+    MpHandOff = (MP_HAND_OFF *)((UINT8 *)MpHandOff + sizeof (MP_HAND_OFF) + sizeof (EFI_HOB_GUID_TYPE) + sizeof (PROCESSOR_HAND_OFF) * NumberOfProcessorsInCurrentHob);
+
+    CpuCountInCurrentHob += NumberOfProcessorsInCurrentHob;
   }
 }
 
@@ -1909,37 +1925,6 @@ CheckAllAPs (
   return EFI_NOT_READY;
 }
 
-/**
-  This function Get BspNumber.
-
-  @param[in] MpHandOff        Pointer to MpHandOff
-  @return                     BspNumber
-**/
-UINT32
-GetBspNumber (
-  IN CONST MP_HAND_OFF  *MpHandOff
-  )
-{
-  UINT32  ApicId;
-  UINT32  BspNumber;
-  UINT32  Index;
-
-  //
-  // Get the processor number for the BSP
-  //
-  BspNumber = MAX_UINT32;
-  ApicId    = GetInitialApicId ();
-  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
-    if (MpHandOff->Info[Index].ApicId == ApicId) {
-      BspNumber = Index;
-    }
-  }
-
-  ASSERT (BspNumber != MAX_UINT32);
-
-  return BspNumber;
-}
-
 /**
   Get pointer to CPU MP Data structure from GUIDed HOB.
 
@@ -1994,8 +1979,13 @@ MpInitLibInitialize (
   UINTN                    ApResetVectorSizeAbove1Mb;
   UINTN                    BackupBufferAddr;
   UINTN                    ApIdtBase;
+  UINTN                    LimitationOfMpHandOffHob;
+  UINTN                    NumberOfProcessorsInCurrentHob;
+  UINTN                    ProcessedCpuCount;
+  UINTN                    CurrentProcessorIndex;
 
   MpHandOff = GetMpHandOffHob ();
+
   if (MpHandOff == NULL) {
     MaxLogicalProcessorNumber = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
   } else {
@@ -2156,17 +2146,35 @@ MpInitLibInitialize (
     // from HOB
     //
     AmdSevUpdateCpuMpData (CpuMpData);
-    CpuMpData->CpuCount  = MpHandOff->CpuCount;
-    CpuMpData->BspNumber = GetBspNumber (MpHandOff);
-    CpuInfoInHob         = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
-    for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
-      InitializeSpinLock (&CpuMpData->CpuData[Index].ApLock);
-      CpuMpData->CpuData[Index].CpuHealthy = (MpHandOff->Info[Index].Health == 0) ? TRUE : FALSE;
-      CpuMpData->CpuData[Index].ApFunction = 0;
-      CpuInfoInHob[Index].InitialApicId    = MpHandOff->Info[Index].ApicId;
-      CpuInfoInHob[Index].ApTopOfStack     = CpuMpData->Buffer + (Index + 1) * CpuMpData->CpuApStackSize;
-      CpuInfoInHob[Index].ApicId           = MpHandOff->Info[Index].ApicId;
-      CpuInfoInHob[Index].Health           = MpHandOff->Info[Index].Health;
+    CpuMpData->CpuCount = MpHandOff->CpuCount;
+    CpuInfoInHob        = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
+
+    LimitationOfMpHandOffHob = (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE) - sizeof (MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
+    ProcessedCpuCount        = 0;
+    while (ProcessedCpuCount < MpHandOff->CpuCount) {
+      NumberOfProcessorsInCurrentHob = MIN (MpHandOff->CpuCount - ProcessedCpuCount, LimitationOfMpHandOffHob);
+
+      for (Index = 0; Index < NumberOfProcessorsInCurrentHob; Index++) {
+        CurrentProcessorIndex = Index+MpHandOff->ProcessorIndex;
+        if (MpHandOff->Info[Index].ApicId == GetInitialApicId ()) {
+          CpuMpData->BspNumber = (UINT32)Index;
+        }
+
+        InitializeSpinLock (&CpuMpData->CpuData[CurrentProcessorIndex].ApLock);
+        CpuMpData->CpuData[CurrentProcessorIndex].CpuHealthy = (MpHandOff->Info[Index].Health == 0) ? TRUE : FALSE;
+        CpuMpData->CpuData[CurrentProcessorIndex].ApFunction = 0;
+        CpuInfoInHob[CurrentProcessorIndex].InitialApicId    = MpHandOff->Info[Index].ApicId;
+        CpuInfoInHob[CurrentProcessorIndex].ApTopOfStack     = CpuMpData->Buffer + (Index + 1) * CpuMpData->CpuApStackSize;
+        CpuInfoInHob[CurrentProcessorIndex].ApicId           = MpHandOff->Info[Index].ApicId;
+        CpuInfoInHob[CurrentProcessorIndex].Health           = MpHandOff->Info[Index].Health;
+      }
+
+      if (ProcessedCpuCount+NumberOfProcessorsInCurrentHob >= MpHandOff->CpuCount) {
+        break;
+      }
+
+      MpHandOff          = (MP_HAND_OFF *)((UINT8 *)MpHandOff + sizeof (MP_HAND_OFF) + sizeof (EFI_HOB_GUID_TYPE) + sizeof (PROCESSOR_HAND_OFF) * NumberOfProcessorsInCurrentHob);
+      ProcessedCpuCount += NumberOfProcessorsInCurrentHob;
     }
 
     DEBUG ((DEBUG_INFO, "MpHandOff->WaitLoopExecutionMode: %04d, sizeof (VOID *): %04d\n", MpHandOff->WaitLoopExecutionMode, sizeof (VOID *)));
@@ -2183,7 +2191,7 @@ MpInitLibInitialize (
       CpuMpData->FinishedCount = 0;
       CpuMpData->InitFlag      = ApInitDone;
       SaveCpuMpData (CpuMpData);
-      SwitchApContext (MpHandOff);
+      SwitchApContext (CpuMpData->BspNumber);
       ASSERT (CpuMpData->FinishedCount == (CpuMpData->CpuCount - 1));
 
       //
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 468d3a5925..6af635a80c 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -497,17 +497,6 @@ SaveCpuMpData (
   IN CPU_MP_DATA  *CpuMpData
   );
 
-/**
-  This function Get BspNumber.
-
-  @param[in] MpHandOff        Pointer to MpHandOff
-  @return                     BspNumber
-**/
-UINT32
-GetBspNumber (
-  IN CONST MP_HAND_OFF  *MpHandOff
-  );
-
 /**
   Get available system memory below 1MB by specified size.
 
@@ -522,12 +511,19 @@ GetWakeupBuffer (
   );
 
 /**
-  Switch Context for each AP.
+  This function is intended to be invoked by the BSP in order
+  to wake up the AP. The BSP accomplishes this by triggering a
+  start-up signal, which in turn causes any APs that are
+  currently in a loop on the PEI-prepared memory to awaken and
+  begin running the procedure called SwitchContextPerAp.
+  This procedure allows the AP to switch to another section of
+  memory and continue its loop there.
 
+  @param[in] BspNumber  Bsp Number
 **/
 VOID
 SwitchApContext (
-  IN MP_HAND_OFF  *MpHandOff
+  IN UINT32  BspNumber
   );
 
 /**
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index f80e00edcf..18f83d9ed5 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -131,31 +131,53 @@ SaveCpuMpData (
   CPU_INFO_IN_HOB  *CpuInfoInHob;
   MP_HAND_OFF      *MpHandOff;
   UINTN            MpHandOffSize;
+  UINT32           LimitationOfMpHandOffHob;
+  UINT32           NumberOfProcessorsInCurrentHob;
+  UINT32           ProcessedCpuCount;
+  UINTN            CurrentProcessorIndex;
 
   //
   // When APs are in a state that can be waken up by a store operation to a memory address,
   // report the MP_HAND_OFF data for DXE to use.
   //
-  CpuInfoInHob  = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
-  MpHandOffSize = sizeof (MP_HAND_OFF) + sizeof (PROCESSOR_HAND_OFF) * CpuMpData->CpuCount;
-  MpHandOff     = (MP_HAND_OFF *)BuildGuidHob (&mMpHandOffGuid, MpHandOffSize);
-  ASSERT (MpHandOff != NULL);
-  ZeroMem (MpHandOff, MpHandOffSize);
-  MpHandOff->ProcessorIndex = 0;
-
-  MpHandOff->CpuCount = CpuMpData->CpuCount;
-  if (CpuMpData->ApLoopMode != ApInHltLoop) {
-    MpHandOff->StartupSignalValue    = MP_HAND_OFF_SIGNAL;
-    MpHandOff->WaitLoopExecutionMode = sizeof (VOID *);
-  }
+  CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
+  //
+  // Maximum number of processor could be saved in the HOB.
+  //
+  LimitationOfMpHandOffHob = (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE) - sizeof (MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
+  ProcessedCpuCount        = 0;
+  while (ProcessedCpuCount < CpuMpData->CpuCount) {
+    NumberOfProcessorsInCurrentHob = MIN ((UINT32)CpuMpData->CpuCount - ProcessedCpuCount, LimitationOfMpHandOffHob);
+    MpHandOffSize                  = sizeof (MP_HAND_OFF) + sizeof (PROCESSOR_HAND_OFF) * NumberOfProcessorsInCurrentHob;
+    MpHandOff                      = (MP_HAND_OFF *)BuildGuidHob (
+                                                      &mMpHandOffGuid,
+                                                      MpHandOffSize
+                                                      );
+    ASSERT (MpHandOff != NULL);
+    ZeroMem (MpHandOff, MpHandOffSize);
+    //
+    // Each individual processor can be uniquely identified by
+    // combining the processorIndex with the mMpHandOffGuid.
+    //
+    MpHandOff->ProcessorIndex = ProcessedCpuCount;
+    MpHandOff->CpuCount       = CpuMpData->CpuCount;
 
-  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
-    MpHandOff->Info[Index].ApicId = CpuInfoInHob[Index].ApicId;
-    MpHandOff->Info[Index].Health = CpuInfoInHob[Index].Health;
     if (CpuMpData->ApLoopMode != ApInHltLoop) {
-      MpHandOff->Info[Index].StartupSignalAddress    = (UINT64)(UINTN)CpuMpData->CpuData[Index].StartupApSignal;
-      MpHandOff->Info[Index].StartupProcedureAddress = (UINT64)(UINTN)&CpuMpData->CpuData[Index].ApFunction;
+      MpHandOff->StartupSignalValue    = MP_HAND_OFF_SIGNAL;
+      MpHandOff->WaitLoopExecutionMode = sizeof (VOID *);
     }
+
+    for (Index = MpHandOff->ProcessorIndex; Index < NumberOfProcessorsInCurrentHob + MpHandOff->ProcessorIndex; Index++) {
+      CurrentProcessorIndex                         = Index-MpHandOff->ProcessorIndex;
+      MpHandOff->Info[CurrentProcessorIndex].ApicId = CpuInfoInHob[Index].ApicId;
+      MpHandOff->Info[CurrentProcessorIndex].Health = CpuInfoInHob[Index].Health;
+      if (CpuMpData->ApLoopMode != ApInHltLoop) {
+        MpHandOff->Info[CurrentProcessorIndex].StartupSignalAddress    = (UINT64)(UINTN)CpuMpData->CpuData[Index].StartupApSignal;
+        MpHandOff->Info[CurrentProcessorIndex].StartupProcedureAddress = (UINT64)(UINTN)&CpuMpData->CpuData[Index].ApFunction;
+      }
+    }
+
+    ProcessedCpuCount += LimitationOfMpHandOffHob;
   }
 
   //
-- 
2.36.1.windows.1


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

* Re: [Patch V2 6/6] UefiCpuPkg: Enhance MpHandOff Handling.
  2023-06-25 14:39 ` [Patch V2 6/6] UefiCpuPkg: Enhance MpHandOff Handling Yuanhao Xie
@ 2023-06-26  3:14   ` Ni, Ray
  2023-06-26  6:03     ` Yuanhao Xie
  0 siblings, 1 reply; 11+ messages in thread
From: Ni, Ray @ 2023-06-26  3:14 UTC (permalink / raw)
  To: Xie, Yuanhao, devel@edk2.groups.io
  Cc: Gerd Hoffmann, Dong, Eric, Kumar, Rahul R, Tom Lendacky

Yuanhao,
I suggest we "attack" this "large number of processors" problem in a separate patch series.
There are several things that need to consider:
* Calculate maximum number of processor info within one MP_HAND_OFF hob
* Consumer code should not assume there is only one MP_HAND_OFF hob
* Consumer code should not assume the MP_HAND_OFF hobs are ordered by MP_HAND_OFF.ProcessorIndex
* Consumer code should report error if there is gap (some processors are not covered by any MP_HAND_OFF hob)
* Consumer code should consider to cache the MP_HAND_OFF hobs for performance (iterate the hob list as less as possible)

For this patch series, you could add code to cover following:
* producer code asserts that the total cpu count can be described by one MP_HAND_OFF hob
* consumer code asserts that there is only one MP_HAND_OFF hob instance and this one's ProcessorIndex is 0.



Thanks,
Ray

> -----Original Message-----
> From: Xie, Yuanhao <yuanhao.xie@intel.com>
> Sent: Sunday, June 25, 2023 10:39 PM
> To: devel@edk2.groups.io
> Cc: Gerd Hoffmann <kraxel@redhat.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Xie, Yuanhao
> <yuanhao.xie@intel.com>
> Subject: [Patch V2 6/6] UefiCpuPkg: Enhance MpHandOff Handling.
> 
> Enhance MpHandOff Handling for Systems with a Large Number of
> Processors.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c    | 128
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++------------------------------------------------------------
>  UefiCpuPkg/Library/MpInitLib/MpLib.h    |  22 +++++++++-------------
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c |  56
> +++++++++++++++++++++++++++++++++++++++-----------------
>  3 files changed, 116 insertions(+), 90 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 391e6f19ef..087bf3ea92 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1528,32 +1528,48 @@ SwitchContextPerAp (
>    This procedure allows the AP to switch to another section of
>    memory and continue its loop there.
> 
> -  @param[in] MpHandOff  Pointer to MP hand-off data structure.
> +  @param[in] BspNumber  Bsp Number
>  **/
>  VOID
>  SwitchApContext (
> -  IN MP_HAND_OFF  *MpHandOff
> +  IN UINT32  BspNumber
>    )
>  {
> -  UINTN   Index;
> -  UINT32  BspNumber;
> -
> -  BspNumber = GetBspNumber (MpHandOff);
> +  UINTN        Index;
> +  UINTN        LimitationOfMpHandOffHob;
> +  UINTN        NumberOfProcessorsInCurrentHob;
> +  UINTN        CpuCountInCurrentHob;
> +  MP_HAND_OFF  *MpHandOff;
> +  BOOLEAN      BspFound;
> +
> +  BspFound = FALSE;
> +
> +  MpHandOff                = GetMpHandOffHob ();
> +  CpuCountInCurrentHob     = 0;
> +  LimitationOfMpHandOffHob = (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE) - sizeof
> (MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
> +  while (CpuCountInCurrentHob < MpHandOff->CpuCount) {
> +    //
> +    // Get the processor number for the BSP
> +    //
> +    NumberOfProcessorsInCurrentHob = MIN (MpHandOff->CpuCount -
> CpuCountInCurrentHob, LimitationOfMpHandOffHob);
> 
> -  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> -    if (Index != BspNumber) {
> -      *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress =
> (UINTN)SwitchContextPerAp;
> -      *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   =
> MpHandOff->StartupSignalValue;
> +    for (Index = 0; Index < NumberOfProcessorsInCurrentHob; Index++) {
> +      if ((Index == BspNumber) && (BspFound == FALSE)) {
> +        BspFound = TRUE;
> +      } else {
> +        *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress =
> (UINTN)SwitchContextPerAp;
> +        *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   =
> MpHandOff->StartupSignalValue;
> +        WaitApWakeup ((UINT32 *)(UINTN)(MpHandOff-
> >Info[Index].StartupSignalAddress));
> +      }
>      }
> -  }
> 
> -  //
> -  // Wait all APs waken up if this is not the 1st broadcast of SIPI
> -  //
> -  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> -    if (Index != BspNumber) {
> -      WaitApWakeup ((UINT32 *)(UINTN)(MpHandOff-
> >Info[Index].StartupSignalAddress));
> +    if (CpuCountInCurrentHob + NumberOfProcessorsInCurrentHob >=
> MpHandOff->CpuCount) {
> +      break;
>      }
> +
> +    MpHandOff = (MP_HAND_OFF *)((UINT8 *)MpHandOff + sizeof
> (MP_HAND_OFF) + sizeof (EFI_HOB_GUID_TYPE) + sizeof
> (PROCESSOR_HAND_OFF) * NumberOfProcessorsInCurrentHob);
> +
> +    CpuCountInCurrentHob += NumberOfProcessorsInCurrentHob;
>    }
>  }
> 
> @@ -1909,37 +1925,6 @@ CheckAllAPs (
>    return EFI_NOT_READY;
>  }
> 
> -/**
> -  This function Get BspNumber.
> -
> -  @param[in] MpHandOff        Pointer to MpHandOff
> -  @return                     BspNumber
> -**/
> -UINT32
> -GetBspNumber (
> -  IN CONST MP_HAND_OFF  *MpHandOff
> -  )
> -{
> -  UINT32  ApicId;
> -  UINT32  BspNumber;
> -  UINT32  Index;
> -
> -  //
> -  // Get the processor number for the BSP
> -  //
> -  BspNumber = MAX_UINT32;
> -  ApicId    = GetInitialApicId ();
> -  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> -    if (MpHandOff->Info[Index].ApicId == ApicId) {
> -      BspNumber = Index;
> -    }
> -  }
> -
> -  ASSERT (BspNumber != MAX_UINT32);
> -
> -  return BspNumber;
> -}
> -
>  /**
>    Get pointer to CPU MP Data structure from GUIDed HOB.
> 
> @@ -1994,8 +1979,13 @@ MpInitLibInitialize (
>    UINTN                    ApResetVectorSizeAbove1Mb;
>    UINTN                    BackupBufferAddr;
>    UINTN                    ApIdtBase;
> +  UINTN                    LimitationOfMpHandOffHob;
> +  UINTN                    NumberOfProcessorsInCurrentHob;
> +  UINTN                    ProcessedCpuCount;
> +  UINTN                    CurrentProcessorIndex;
> 
>    MpHandOff = GetMpHandOffHob ();
> +
>    if (MpHandOff == NULL) {
>      MaxLogicalProcessorNumber = PcdGet32
> (PcdCpuMaxLogicalProcessorNumber);
>    } else {
> @@ -2156,17 +2146,35 @@ MpInitLibInitialize (
>      // from HOB
>      //
>      AmdSevUpdateCpuMpData (CpuMpData);
> -    CpuMpData->CpuCount  = MpHandOff->CpuCount;
> -    CpuMpData->BspNumber = GetBspNumber (MpHandOff);
> -    CpuInfoInHob         = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData-
> >CpuInfoInHob;
> -    for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
> -      InitializeSpinLock (&CpuMpData->CpuData[Index].ApLock);
> -      CpuMpData->CpuData[Index].CpuHealthy = (MpHandOff->Info[Index].Health
> == 0) ? TRUE : FALSE;
> -      CpuMpData->CpuData[Index].ApFunction = 0;
> -      CpuInfoInHob[Index].InitialApicId    = MpHandOff->Info[Index].ApicId;
> -      CpuInfoInHob[Index].ApTopOfStack     = CpuMpData->Buffer + (Index + 1) *
> CpuMpData->CpuApStackSize;
> -      CpuInfoInHob[Index].ApicId           = MpHandOff->Info[Index].ApicId;
> -      CpuInfoInHob[Index].Health           = MpHandOff->Info[Index].Health;
> +    CpuMpData->CpuCount = MpHandOff->CpuCount;
> +    CpuInfoInHob        = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData-
> >CpuInfoInHob;
> +
> +    LimitationOfMpHandOffHob = (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE) - sizeof
> (MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
> +    ProcessedCpuCount        = 0;
> +    while (ProcessedCpuCount < MpHandOff->CpuCount) {
> +      NumberOfProcessorsInCurrentHob = MIN (MpHandOff->CpuCount -
> ProcessedCpuCount, LimitationOfMpHandOffHob);
> +
> +      for (Index = 0; Index < NumberOfProcessorsInCurrentHob; Index++) {
> +        CurrentProcessorIndex = Index+MpHandOff->ProcessorIndex;
> +        if (MpHandOff->Info[Index].ApicId == GetInitialApicId ()) {
> +          CpuMpData->BspNumber = (UINT32)Index;
> +        }
> +
> +        InitializeSpinLock (&CpuMpData-
> >CpuData[CurrentProcessorIndex].ApLock);
> +        CpuMpData->CpuData[CurrentProcessorIndex].CpuHealthy = (MpHandOff-
> >Info[Index].Health == 0) ? TRUE : FALSE;
> +        CpuMpData->CpuData[CurrentProcessorIndex].ApFunction = 0;
> +        CpuInfoInHob[CurrentProcessorIndex].InitialApicId    = MpHandOff-
> >Info[Index].ApicId;
> +        CpuInfoInHob[CurrentProcessorIndex].ApTopOfStack     = CpuMpData-
> >Buffer + (Index + 1) * CpuMpData->CpuApStackSize;
> +        CpuInfoInHob[CurrentProcessorIndex].ApicId           = MpHandOff-
> >Info[Index].ApicId;
> +        CpuInfoInHob[CurrentProcessorIndex].Health           = MpHandOff-
> >Info[Index].Health;
> +      }
> +
> +      if (ProcessedCpuCount+NumberOfProcessorsInCurrentHob >= MpHandOff-
> >CpuCount) {
> +        break;
> +      }
> +
> +      MpHandOff          = (MP_HAND_OFF *)((UINT8 *)MpHandOff + sizeof
> (MP_HAND_OFF) + sizeof (EFI_HOB_GUID_TYPE) + sizeof
> (PROCESSOR_HAND_OFF) * NumberOfProcessorsInCurrentHob);
> +      ProcessedCpuCount += NumberOfProcessorsInCurrentHob;
>      }
> 
>      DEBUG ((DEBUG_INFO, "MpHandOff->WaitLoopExecutionMode: %04d, sizeof
> (VOID *): %04d\n", MpHandOff->WaitLoopExecutionMode, sizeof (VOID *)));
> @@ -2183,7 +2191,7 @@ MpInitLibInitialize (
>        CpuMpData->FinishedCount = 0;
>        CpuMpData->InitFlag      = ApInitDone;
>        SaveCpuMpData (CpuMpData);
> -      SwitchApContext (MpHandOff);
> +      SwitchApContext (CpuMpData->BspNumber);
>        ASSERT (CpuMpData->FinishedCount == (CpuMpData->CpuCount - 1));
> 
>        //
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 468d3a5925..6af635a80c 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -497,17 +497,6 @@ SaveCpuMpData (
>    IN CPU_MP_DATA  *CpuMpData
>    );
> 
> -/**
> -  This function Get BspNumber.
> -
> -  @param[in] MpHandOff        Pointer to MpHandOff
> -  @return                     BspNumber
> -**/
> -UINT32
> -GetBspNumber (
> -  IN CONST MP_HAND_OFF  *MpHandOff
> -  );
> -
>  /**
>    Get available system memory below 1MB by specified size.
> 
> @@ -522,12 +511,19 @@ GetWakeupBuffer (
>    );
> 
>  /**
> -  Switch Context for each AP.
> +  This function is intended to be invoked by the BSP in order
> +  to wake up the AP. The BSP accomplishes this by triggering a
> +  start-up signal, which in turn causes any APs that are
> +  currently in a loop on the PEI-prepared memory to awaken and
> +  begin running the procedure called SwitchContextPerAp.
> +  This procedure allows the AP to switch to another section of
> +  memory and continue its loop there.
> 
> +  @param[in] BspNumber  Bsp Number
>  **/
>  VOID
>  SwitchApContext (
> -  IN MP_HAND_OFF  *MpHandOff
> +  IN UINT32  BspNumber
>    );
> 
>  /**
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index f80e00edcf..18f83d9ed5 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> @@ -131,31 +131,53 @@ SaveCpuMpData (
>    CPU_INFO_IN_HOB  *CpuInfoInHob;
>    MP_HAND_OFF      *MpHandOff;
>    UINTN            MpHandOffSize;
> +  UINT32           LimitationOfMpHandOffHob;
> +  UINT32           NumberOfProcessorsInCurrentHob;
> +  UINT32           ProcessedCpuCount;
> +  UINTN            CurrentProcessorIndex;
> 
>    //
>    // When APs are in a state that can be waken up by a store operation to a
> memory address,
>    // report the MP_HAND_OFF data for DXE to use.
>    //
> -  CpuInfoInHob  = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
> -  MpHandOffSize = sizeof (MP_HAND_OFF) + sizeof (PROCESSOR_HAND_OFF) *
> CpuMpData->CpuCount;
> -  MpHandOff     = (MP_HAND_OFF *)BuildGuidHob (&mMpHandOffGuid,
> MpHandOffSize);
> -  ASSERT (MpHandOff != NULL);
> -  ZeroMem (MpHandOff, MpHandOffSize);
> -  MpHandOff->ProcessorIndex = 0;
> -
> -  MpHandOff->CpuCount = CpuMpData->CpuCount;
> -  if (CpuMpData->ApLoopMode != ApInHltLoop) {
> -    MpHandOff->StartupSignalValue    = MP_HAND_OFF_SIGNAL;
> -    MpHandOff->WaitLoopExecutionMode = sizeof (VOID *);
> -  }
> +  CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
> +  //
> +  // Maximum number of processor could be saved in the HOB.
> +  //
> +  LimitationOfMpHandOffHob = (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE) - sizeof
> (MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
> +  ProcessedCpuCount        = 0;
> +  while (ProcessedCpuCount < CpuMpData->CpuCount) {
> +    NumberOfProcessorsInCurrentHob = MIN ((UINT32)CpuMpData->CpuCount -
> ProcessedCpuCount, LimitationOfMpHandOffHob);
> +    MpHandOffSize                  = sizeof (MP_HAND_OFF) + sizeof
> (PROCESSOR_HAND_OFF) * NumberOfProcessorsInCurrentHob;
> +    MpHandOff                      = (MP_HAND_OFF *)BuildGuidHob (
> +                                                      &mMpHandOffGuid,
> +                                                      MpHandOffSize
> +                                                      );
> +    ASSERT (MpHandOff != NULL);
> +    ZeroMem (MpHandOff, MpHandOffSize);
> +    //
> +    // Each individual processor can be uniquely identified by
> +    // combining the processorIndex with the mMpHandOffGuid.
> +    //
> +    MpHandOff->ProcessorIndex = ProcessedCpuCount;
> +    MpHandOff->CpuCount       = CpuMpData->CpuCount;
> 
> -  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> -    MpHandOff->Info[Index].ApicId = CpuInfoInHob[Index].ApicId;
> -    MpHandOff->Info[Index].Health = CpuInfoInHob[Index].Health;
>      if (CpuMpData->ApLoopMode != ApInHltLoop) {
> -      MpHandOff->Info[Index].StartupSignalAddress    =
> (UINT64)(UINTN)CpuMpData->CpuData[Index].StartupApSignal;
> -      MpHandOff->Info[Index].StartupProcedureAddress =
> (UINT64)(UINTN)&CpuMpData->CpuData[Index].ApFunction;
> +      MpHandOff->StartupSignalValue    = MP_HAND_OFF_SIGNAL;
> +      MpHandOff->WaitLoopExecutionMode = sizeof (VOID *);
>      }
> +
> +    for (Index = MpHandOff->ProcessorIndex; Index <
> NumberOfProcessorsInCurrentHob + MpHandOff->ProcessorIndex; Index++) {
> +      CurrentProcessorIndex                         = Index-MpHandOff->ProcessorIndex;
> +      MpHandOff->Info[CurrentProcessorIndex].ApicId =
> CpuInfoInHob[Index].ApicId;
> +      MpHandOff->Info[CurrentProcessorIndex].Health =
> CpuInfoInHob[Index].Health;
> +      if (CpuMpData->ApLoopMode != ApInHltLoop) {
> +        MpHandOff->Info[CurrentProcessorIndex].StartupSignalAddress    =
> (UINT64)(UINTN)CpuMpData->CpuData[Index].StartupApSignal;
> +        MpHandOff->Info[CurrentProcessorIndex].StartupProcedureAddress =
> (UINT64)(UINTN)&CpuMpData->CpuData[Index].ApFunction;
> +      }
> +    }
> +
> +    ProcessedCpuCount += LimitationOfMpHandOffHob;
>    }
> 
>    //
> --
> 2.36.1.windows.1


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

* Re: [Patch V2 6/6] UefiCpuPkg: Enhance MpHandOff Handling.
  2023-06-26  3:14   ` Ni, Ray
@ 2023-06-26  6:03     ` Yuanhao Xie
  0 siblings, 0 replies; 11+ messages in thread
From: Yuanhao Xie @ 2023-06-26  6:03 UTC (permalink / raw)
  To: Ni, Ray
  Cc: Gerd Hoffmann, Dong, Eric, Kumar, Rahul R, Tom Lendacky,
	devel@edk2.groups.io

Hi Ray,

I will leave the enhance patch as the separated one and send it later on.

Thanks for the feedback.
Yuanhao
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Monday, June 26, 2023 11:14 AM
To: Xie, Yuanhao <yuanhao.xie@intel.com>; devel@edk2.groups.io
Cc: Gerd Hoffmann <kraxel@redhat.com>; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>
Subject: RE: [Patch V2 6/6] UefiCpuPkg: Enhance MpHandOff Handling.

Yuanhao,
I suggest we "attack" this "large number of processors" problem in a separate patch series.
There are several things that need to consider:
* Calculate maximum number of processor info within one MP_HAND_OFF hob
* Consumer code should not assume there is only one MP_HAND_OFF hob
* Consumer code should not assume the MP_HAND_OFF hobs are ordered by MP_HAND_OFF.ProcessorIndex
* Consumer code should report error if there is gap (some processors are not covered by any MP_HAND_OFF hob)
* Consumer code should consider to cache the MP_HAND_OFF hobs for performance (iterate the hob list as less as possible)

For this patch series, you could add code to cover following:
* producer code asserts that the total cpu count can be described by one MP_HAND_OFF hob
* consumer code asserts that there is only one MP_HAND_OFF hob instance and this one's ProcessorIndex is 0.



Thanks,
Ray

> -----Original Message-----
> From: Xie, Yuanhao <yuanhao.xie@intel.com>
> Sent: Sunday, June 25, 2023 10:39 PM
> To: devel@edk2.groups.io
> Cc: Gerd Hoffmann <kraxel@redhat.com>; Dong, Eric 
> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R 
> <rahul.r.kumar@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; 
> Xie, Yuanhao <yuanhao.xie@intel.com>
> Subject: [Patch V2 6/6] UefiCpuPkg: Enhance MpHandOff Handling.
> 
> Enhance MpHandOff Handling for Systems with a Large Number of 
> Processors.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c    | 128
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++------------------------------------------------------------
>  UefiCpuPkg/Library/MpInitLib/MpLib.h    |  22 +++++++++-------------
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c |  56
> +++++++++++++++++++++++++++++++++++++++-----------------
>  3 files changed, 116 insertions(+), 90 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 391e6f19ef..087bf3ea92 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1528,32 +1528,48 @@ SwitchContextPerAp (
>    This procedure allows the AP to switch to another section of
>    memory and continue its loop there.
> 
> -  @param[in] MpHandOff  Pointer to MP hand-off data structure.
> +  @param[in] BspNumber  Bsp Number
>  **/
>  VOID
>  SwitchApContext (
> -  IN MP_HAND_OFF  *MpHandOff
> +  IN UINT32  BspNumber
>    )
>  {
> -  UINTN   Index;
> -  UINT32  BspNumber;
> -
> -  BspNumber = GetBspNumber (MpHandOff);
> +  UINTN        Index;
> +  UINTN        LimitationOfMpHandOffHob;
> +  UINTN        NumberOfProcessorsInCurrentHob;
> +  UINTN        CpuCountInCurrentHob;
> +  MP_HAND_OFF  *MpHandOff;
> +  BOOLEAN      BspFound;
> +
> +  BspFound = FALSE;
> +
> +  MpHandOff                = GetMpHandOffHob ();
> +  CpuCountInCurrentHob     = 0;
> +  LimitationOfMpHandOffHob = (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE) - 
> + sizeof
> (MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
> +  while (CpuCountInCurrentHob < MpHandOff->CpuCount) {
> +    //
> +    // Get the processor number for the BSP
> +    //
> +    NumberOfProcessorsInCurrentHob = MIN (MpHandOff->CpuCount -
> CpuCountInCurrentHob, LimitationOfMpHandOffHob);
> 
> -  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> -    if (Index != BspNumber) {
> -      *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress =
> (UINTN)SwitchContextPerAp;
> -      *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   =
> MpHandOff->StartupSignalValue;
> +    for (Index = 0; Index < NumberOfProcessorsInCurrentHob; Index++) {
> +      if ((Index == BspNumber) && (BspFound == FALSE)) {
> +        BspFound = TRUE;
> +      } else {
> +        *(UINTN 
> + *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress =
> (UINTN)SwitchContextPerAp;
> +        *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   =
> MpHandOff->StartupSignalValue;
> +        WaitApWakeup ((UINT32 *)(UINTN)(MpHandOff-
> >Info[Index].StartupSignalAddress));
> +      }
>      }
> -  }
> 
> -  //
> -  // Wait all APs waken up if this is not the 1st broadcast of SIPI
> -  //
> -  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> -    if (Index != BspNumber) {
> -      WaitApWakeup ((UINT32 *)(UINTN)(MpHandOff-
> >Info[Index].StartupSignalAddress));
> +    if (CpuCountInCurrentHob + NumberOfProcessorsInCurrentHob >=
> MpHandOff->CpuCount) {
> +      break;
>      }
> +
> +    MpHandOff = (MP_HAND_OFF *)((UINT8 *)MpHandOff + sizeof
> (MP_HAND_OFF) + sizeof (EFI_HOB_GUID_TYPE) + sizeof
> (PROCESSOR_HAND_OFF) * NumberOfProcessorsInCurrentHob);
> +
> +    CpuCountInCurrentHob += NumberOfProcessorsInCurrentHob;
>    }
>  }
> 
> @@ -1909,37 +1925,6 @@ CheckAllAPs (
>    return EFI_NOT_READY;
>  }
> 
> -/**
> -  This function Get BspNumber.
> -
> -  @param[in] MpHandOff        Pointer to MpHandOff
> -  @return                     BspNumber
> -**/
> -UINT32
> -GetBspNumber (
> -  IN CONST MP_HAND_OFF  *MpHandOff
> -  )
> -{
> -  UINT32  ApicId;
> -  UINT32  BspNumber;
> -  UINT32  Index;
> -
> -  //
> -  // Get the processor number for the BSP
> -  //
> -  BspNumber = MAX_UINT32;
> -  ApicId    = GetInitialApicId ();
> -  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> -    if (MpHandOff->Info[Index].ApicId == ApicId) {
> -      BspNumber = Index;
> -    }
> -  }
> -
> -  ASSERT (BspNumber != MAX_UINT32);
> -
> -  return BspNumber;
> -}
> -
>  /**
>    Get pointer to CPU MP Data structure from GUIDed HOB.
> 
> @@ -1994,8 +1979,13 @@ MpInitLibInitialize (
>    UINTN                    ApResetVectorSizeAbove1Mb;
>    UINTN                    BackupBufferAddr;
>    UINTN                    ApIdtBase;
> +  UINTN                    LimitationOfMpHandOffHob;
> +  UINTN                    NumberOfProcessorsInCurrentHob;
> +  UINTN                    ProcessedCpuCount;
> +  UINTN                    CurrentProcessorIndex;
> 
>    MpHandOff = GetMpHandOffHob ();
> +
>    if (MpHandOff == NULL) {
>      MaxLogicalProcessorNumber = PcdGet32 
> (PcdCpuMaxLogicalProcessorNumber);
>    } else {
> @@ -2156,17 +2146,35 @@ MpInitLibInitialize (
>      // from HOB
>      //
>      AmdSevUpdateCpuMpData (CpuMpData);
> -    CpuMpData->CpuCount  = MpHandOff->CpuCount;
> -    CpuMpData->BspNumber = GetBspNumber (MpHandOff);
> -    CpuInfoInHob         = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData-
> >CpuInfoInHob;
> -    for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
> -      InitializeSpinLock (&CpuMpData->CpuData[Index].ApLock);
> -      CpuMpData->CpuData[Index].CpuHealthy = (MpHandOff->Info[Index].Health
> == 0) ? TRUE : FALSE;
> -      CpuMpData->CpuData[Index].ApFunction = 0;
> -      CpuInfoInHob[Index].InitialApicId    = MpHandOff->Info[Index].ApicId;
> -      CpuInfoInHob[Index].ApTopOfStack     = CpuMpData->Buffer + (Index + 1) *
> CpuMpData->CpuApStackSize;
> -      CpuInfoInHob[Index].ApicId           = MpHandOff->Info[Index].ApicId;
> -      CpuInfoInHob[Index].Health           = MpHandOff->Info[Index].Health;
> +    CpuMpData->CpuCount = MpHandOff->CpuCount;
> +    CpuInfoInHob        = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData-
> >CpuInfoInHob;
> +
> +    LimitationOfMpHandOffHob = (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE) - 
> + sizeof
> (MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
> +    ProcessedCpuCount        = 0;
> +    while (ProcessedCpuCount < MpHandOff->CpuCount) {
> +      NumberOfProcessorsInCurrentHob = MIN (MpHandOff->CpuCount -
> ProcessedCpuCount, LimitationOfMpHandOffHob);
> +
> +      for (Index = 0; Index < NumberOfProcessorsInCurrentHob; Index++) {
> +        CurrentProcessorIndex = Index+MpHandOff->ProcessorIndex;
> +        if (MpHandOff->Info[Index].ApicId == GetInitialApicId ()) {
> +          CpuMpData->BspNumber = (UINT32)Index;
> +        }
> +
> +        InitializeSpinLock (&CpuMpData-
> >CpuData[CurrentProcessorIndex].ApLock);
> +        CpuMpData->CpuData[CurrentProcessorIndex].CpuHealthy = 
> + (MpHandOff-
> >Info[Index].Health == 0) ? TRUE : FALSE;
> +        CpuMpData->CpuData[CurrentProcessorIndex].ApFunction = 0;
> +        CpuInfoInHob[CurrentProcessorIndex].InitialApicId    = MpHandOff-
> >Info[Index].ApicId;
> +        CpuInfoInHob[CurrentProcessorIndex].ApTopOfStack     = CpuMpData-
> >Buffer + (Index + 1) * CpuMpData->CpuApStackSize;
> +        CpuInfoInHob[CurrentProcessorIndex].ApicId           = MpHandOff-
> >Info[Index].ApicId;
> +        CpuInfoInHob[CurrentProcessorIndex].Health           = MpHandOff-
> >Info[Index].Health;
> +      }
> +
> +      if (ProcessedCpuCount+NumberOfProcessorsInCurrentHob >= 
> + MpHandOff-
> >CpuCount) {
> +        break;
> +      }
> +
> +      MpHandOff          = (MP_HAND_OFF *)((UINT8 *)MpHandOff + sizeof
> (MP_HAND_OFF) + sizeof (EFI_HOB_GUID_TYPE) + sizeof
> (PROCESSOR_HAND_OFF) * NumberOfProcessorsInCurrentHob);
> +      ProcessedCpuCount += NumberOfProcessorsInCurrentHob;
>      }
> 
>      DEBUG ((DEBUG_INFO, "MpHandOff->WaitLoopExecutionMode: %04d, 
> sizeof (VOID *): %04d\n", MpHandOff->WaitLoopExecutionMode, sizeof 
> (VOID *))); @@ -2183,7 +2191,7 @@ MpInitLibInitialize (
>        CpuMpData->FinishedCount = 0;
>        CpuMpData->InitFlag      = ApInitDone;
>        SaveCpuMpData (CpuMpData);
> -      SwitchApContext (MpHandOff);
> +      SwitchApContext (CpuMpData->BspNumber);
>        ASSERT (CpuMpData->FinishedCount == (CpuMpData->CpuCount - 1));
> 
>        //
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 468d3a5925..6af635a80c 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -497,17 +497,6 @@ SaveCpuMpData (
>    IN CPU_MP_DATA  *CpuMpData
>    );
> 
> -/**
> -  This function Get BspNumber.
> -
> -  @param[in] MpHandOff        Pointer to MpHandOff
> -  @return                     BspNumber
> -**/
> -UINT32
> -GetBspNumber (
> -  IN CONST MP_HAND_OFF  *MpHandOff
> -  );
> -
>  /**
>    Get available system memory below 1MB by specified size.
> 
> @@ -522,12 +511,19 @@ GetWakeupBuffer (
>    );
> 
>  /**
> -  Switch Context for each AP.
> +  This function is intended to be invoked by the BSP in order  to 
> + wake up the AP. The BSP accomplishes this by triggering a  start-up 
> + signal, which in turn causes any APs that are  currently in a loop 
> + on the PEI-prepared memory to awaken and  begin running the 
> + procedure called SwitchContextPerAp.
> +  This procedure allows the AP to switch to another section of  
> + memory and continue its loop there.
> 
> +  @param[in] BspNumber  Bsp Number
>  **/
>  VOID
>  SwitchApContext (
> -  IN MP_HAND_OFF  *MpHandOff
> +  IN UINT32  BspNumber
>    );
> 
>  /**
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index f80e00edcf..18f83d9ed5 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> @@ -131,31 +131,53 @@ SaveCpuMpData (
>    CPU_INFO_IN_HOB  *CpuInfoInHob;
>    MP_HAND_OFF      *MpHandOff;
>    UINTN            MpHandOffSize;
> +  UINT32           LimitationOfMpHandOffHob;
> +  UINT32           NumberOfProcessorsInCurrentHob;
> +  UINT32           ProcessedCpuCount;
> +  UINTN            CurrentProcessorIndex;
> 
>    //
>    // When APs are in a state that can be waken up by a store 
> operation to a memory address,
>    // report the MP_HAND_OFF data for DXE to use.
>    //
> -  CpuInfoInHob  = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
> -  MpHandOffSize = sizeof (MP_HAND_OFF) + sizeof (PROCESSOR_HAND_OFF) 
> *
> CpuMpData->CpuCount;
> -  MpHandOff     = (MP_HAND_OFF *)BuildGuidHob (&mMpHandOffGuid,
> MpHandOffSize);
> -  ASSERT (MpHandOff != NULL);
> -  ZeroMem (MpHandOff, MpHandOffSize);
> -  MpHandOff->ProcessorIndex = 0;
> -
> -  MpHandOff->CpuCount = CpuMpData->CpuCount;
> -  if (CpuMpData->ApLoopMode != ApInHltLoop) {
> -    MpHandOff->StartupSignalValue    = MP_HAND_OFF_SIGNAL;
> -    MpHandOff->WaitLoopExecutionMode = sizeof (VOID *);
> -  }
> +  CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;  
> + //  // Maximum number of processor could be saved in the HOB.
> +  //
> +  LimitationOfMpHandOffHob = (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE) - 
> + sizeof
> (MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
> +  ProcessedCpuCount        = 0;
> +  while (ProcessedCpuCount < CpuMpData->CpuCount) {
> +    NumberOfProcessorsInCurrentHob = MIN ((UINT32)CpuMpData->CpuCount 
> + -
> ProcessedCpuCount, LimitationOfMpHandOffHob);
> +    MpHandOffSize                  = sizeof (MP_HAND_OFF) + sizeof
> (PROCESSOR_HAND_OFF) * NumberOfProcessorsInCurrentHob;
> +    MpHandOff                      = (MP_HAND_OFF *)BuildGuidHob (
> +                                                      &mMpHandOffGuid,
> +                                                      MpHandOffSize
> +                                                      );
> +    ASSERT (MpHandOff != NULL);
> +    ZeroMem (MpHandOff, MpHandOffSize);
> +    //
> +    // Each individual processor can be uniquely identified by
> +    // combining the processorIndex with the mMpHandOffGuid.
> +    //
> +    MpHandOff->ProcessorIndex = ProcessedCpuCount;
> +    MpHandOff->CpuCount       = CpuMpData->CpuCount;
> 
> -  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> -    MpHandOff->Info[Index].ApicId = CpuInfoInHob[Index].ApicId;
> -    MpHandOff->Info[Index].Health = CpuInfoInHob[Index].Health;
>      if (CpuMpData->ApLoopMode != ApInHltLoop) {
> -      MpHandOff->Info[Index].StartupSignalAddress    =
> (UINT64)(UINTN)CpuMpData->CpuData[Index].StartupApSignal;
> -      MpHandOff->Info[Index].StartupProcedureAddress =
> (UINT64)(UINTN)&CpuMpData->CpuData[Index].ApFunction;
> +      MpHandOff->StartupSignalValue    = MP_HAND_OFF_SIGNAL;
> +      MpHandOff->WaitLoopExecutionMode = sizeof (VOID *);
>      }
> +
> +    for (Index = MpHandOff->ProcessorIndex; Index <
> NumberOfProcessorsInCurrentHob + MpHandOff->ProcessorIndex; Index++) {
> +      CurrentProcessorIndex                         = Index-MpHandOff->ProcessorIndex;
> +      MpHandOff->Info[CurrentProcessorIndex].ApicId =
> CpuInfoInHob[Index].ApicId;
> +      MpHandOff->Info[CurrentProcessorIndex].Health =
> CpuInfoInHob[Index].Health;
> +      if (CpuMpData->ApLoopMode != ApInHltLoop) {
> +        MpHandOff->Info[CurrentProcessorIndex].StartupSignalAddress    =
> (UINT64)(UINTN)CpuMpData->CpuData[Index].StartupApSignal;
> +        
> + MpHandOff->Info[CurrentProcessorIndex].StartupProcedureAddress =
> (UINT64)(UINTN)&CpuMpData->CpuData[Index].ApFunction;
> +      }
> +    }
> +
> +    ProcessedCpuCount += LimitationOfMpHandOffHob;
>    }
> 
>    //
> --
> 2.36.1.windows.1


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

* Re: [Patch V2 3/6] UefiCpuPkg: Create MpHandOff.
  2023-06-25 14:39 ` [Patch V2 3/6] UefiCpuPkg: Create MpHandOff Yuanhao Xie
@ 2023-06-26 11:21   ` Gerd Hoffmann
  2023-06-26 12:32     ` Yuanhao Xie
  0 siblings, 1 reply; 11+ messages in thread
From: Gerd Hoffmann @ 2023-06-26 11:21 UTC (permalink / raw)
  To: YuanhaoXie; +Cc: devel, Eric Dong, Ray Ni, Rahul Kumar, Tom Lendacky

  Hi,

> The SaveCpuMpData() function was updated to construct the MP_HAND_OFF
> Hob.

The HOB still has the same scaleability problem (~2700 cpus max).

take care,
  Gerd


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

* Re: [Patch V2 3/6] UefiCpuPkg: Create MpHandOff.
  2023-06-26 11:21   ` Gerd Hoffmann
@ 2023-06-26 12:32     ` Yuanhao Xie
  0 siblings, 0 replies; 11+ messages in thread
From: Yuanhao Xie @ 2023-06-26 12:32 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: devel@edk2.groups.io, Dong, Eric, Ni, Ray, Kumar, Rahul R,
	Tom Lendacky

Hi Gerd,

Please check
[Patch V2 6/6] UefiCpuPkg: Enhance MpHandOff Handling.

Best,
Yuanhao

-----Original Message-----
From: Gerd Hoffmann <kraxel@redhat.com> 
Sent: Monday, June 26, 2023 7:22 PM
To: Xie, Yuanhao <yuanhao.xie@intel.com>
Cc: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>
Subject: Re: [Patch V2 3/6] UefiCpuPkg: Create MpHandOff.

  Hi,

> The SaveCpuMpData() function was updated to construct the MP_HAND_OFF 
> Hob.

The HOB still has the same scaleability problem (~2700 cpus max).

take care,
  Gerd


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

end of thread, other threads:[~2023-06-26 12:32 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-25 14:39 [Patch V2 0/6] Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
2023-06-25 14:39 ` [Patch V2 1/6] UefiCpuPkg: Refactor the logic for placing APs in HltLoop Yuanhao Xie
2023-06-25 14:39 ` [Patch V2 2/6] UefiCpuPkg: Refactor the logic for placing APs in Mwait/Runloop Yuanhao Xie
2023-06-25 14:39 ` [Patch V2 3/6] UefiCpuPkg: Create MpHandOff Yuanhao Xie
2023-06-26 11:21   ` Gerd Hoffmann
2023-06-26 12:32     ` Yuanhao Xie
2023-06-25 14:39 ` [Patch V2 4/6] UefiCpuPkg: ApWakeupFunction directly use CpuMpData Yuanhao Xie
2023-06-25 14:39 ` [Patch V2 5/6] UefiCpuPkg: Eliminate the second INIT-SIPI-SIPI sequence Yuanhao Xie
2023-06-25 14:39 ` [Patch V2 6/6] UefiCpuPkg: Enhance MpHandOff Handling Yuanhao Xie
2023-06-26  3:14   ` Ni, Ray
2023-06-26  6:03     ` Yuanhao Xie

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