[PATCH 0/3] Remove the redundent Mp initialization

[PATCH 0/3] Remove the redundent Mp initialization

[Yuanhao Xie]

1. Removed the duplicated loading of microcode in DXE phase.
2. emoved repeated Mp initialization, taking into account the 
different bit mode (32bit/64bit) combinations of PEI and DXE stages.
3. Removed the unused MpLock.

Yuanhao (3):
  UefiCpuPkg: Remove the duplicate loading of microcode in DXE stage
  UefiCpuPkg: Skip Mp initialization in DXE phase
  UefiCpuPkg: Removed the unused MpLock

 UefiCpuPkg/Library/MpInitLib/MpLib.c | 177 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------
 UefiCpuPkg/Library/MpInitLib/MpLib.h |   2 +-
 2 files changed, 126 insertions(+), 53 deletions(-)

-- 
2.36.1.windows.1

[PATCH 1/3] UefiCpuPkg: Remove the duplicate loading of microcode in DXE stage

[Yuanhao Xie]

From: Yuanhao <yuanhao.xie@intel.com>

If both in the DXE and PEI phase Mp initialization is excuted, the
microcode loading during Mp initialization of the DXE stage can be
removed regardless of the bit modes.

Signed-off-by: Yuanhao Xie <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>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 58 +++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 1c053f87a4..5430688946 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -432,6 +432,27 @@ ApFuncEnableX2Apic (
   SetApicMode (LOCAL_APIC_MODE_X2APIC);
 }
 
+/**
+  Sync BSP's MTRR table to AP during waking upAp
+
+  @param[in, out] Buffer  Pointer to private data buffer.
+**/
+VOID
+EFIAPI
+ApMtrrSync (
+  IN OUT VOID  *Buffer
+  )
+{
+  CPU_MP_DATA  *CpuMpData;
+
+  CpuMpData = (CPU_MP_DATA *)Buffer;
+
+  //
+  // Sync BSP's MTRR table to AP
+  //
+  MtrrSetAllMtrrs (&CpuMpData->MtrrTable);
+}
+
 /**
   Do sync on APs.
 
@@ -1977,6 +1998,23 @@ MpInitLibInitialize (
       //
       CollectProcessorCount (CpuMpData);
     }
+
+    if (!GetMicrocodePatchInfoFromHob (
+           &CpuMpData->MicrocodePatchAddress,
+           &CpuMpData->MicrocodePatchRegionSize
+           ))
+    {
+      //
+      // The microcode patch information cache HOB does not exist, which means
+      // the microcode patches data has not been loaded into memory yet
+      //
+      ShadowMicrocodeUpdatePatch (CpuMpData);
+    }
+
+    //
+    // Detect and apply Microcode on BSP
+    //
+    MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
   } else {
     //
     // APs have been wakeup before, just get the CPU Information
@@ -1994,22 +2032,6 @@ MpInitLibInitialize (
     }
   }
 
-  if (!GetMicrocodePatchInfoFromHob (
-         &CpuMpData->MicrocodePatchAddress,
-         &CpuMpData->MicrocodePatchRegionSize
-         ))
-  {
-    //
-    // The microcode patch information cache HOB does not exist, which means
-    // the microcode patches data has not been loaded into memory yet
-    //
-    ShadowMicrocodeUpdatePatch (CpuMpData);
-  }
-
-  //
-  // Detect and apply Microcode on BSP
-  //
-  MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
   //
   // Store BSP's MTRR setting
   //
@@ -2026,9 +2048,11 @@ MpInitLibInitialize (
       // in DXE.
       //
       CpuMpData->InitFlag = ApInitReconfig;
+      WakeUpAP (CpuMpData, TRUE, 0, ApMtrrSync, CpuMpData, TRUE);
+    } else {
+      WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
     }
 
-    WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
     //
     // Wait for all APs finished initialization
     //
-- 
2.36.1.windows.1

[PATCH 2/3] UefiCpuPkg: Skip Mp initialization in DXE phase

[Yuanhao Xie]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 7710 bytes --]

From: Yuanhao <yuanhao.xie@intel.com>

If PEI and DXE stages are in the same bit mode, Mp initialization
 is removed in DXE phase.

Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------
 UefiCpuPkg/Library/MpInitLib/MpLib.h |   1 +
 2 files changed, 90 insertions(+), 39 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 5430688946..c7d1e58909 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1842,12 +1842,36 @@ MpInitLibInitialize (
   UINTN                    ApResetVectorSizeAbove1Mb;
   UINTN                    BackupBufferAddr;
   UINTN                    ApIdtBase;
+  BOOLEAN                  FirstTimeMpInitializing;
 
-  OldCpuMpData = GetCpuMpDataFromGuidedHob ();
-  if (OldCpuMpData == NULL) {
+  OldCpuMpData            = GetCpuMpDataFromGuidedHob ();
+  FirstTimeMpInitializing = (BOOLEAN)(OldCpuMpData == NULL);
+
+  if (FirstTimeMpInitializing) {
     MaxLogicalProcessorNumber = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
+    //
+    // The first time Mp is initialized, or the saved CpuMpData is lost, the
+    // complete process including preparing the buffer, filling the CpuMpData,
+    // loading the microcode, and waking up the Aps needs to be performed.
+    //
   } else {
     MaxLogicalProcessorNumber = OldCpuMpData->CpuCount;
+    //
+    // If both PEI and DXE phases are 64-bit, CpuMpdata collected in PEI stage
+    // can be reused in DXE stage, thus no need to redo the initialization.
+    // For the case where DXE is 64-bit and PEI is 32-bit, a partial
+    // initialization of Mp is required.
+    //
+    if (OldCpuMpData->SizeOfUINTN == sizeof (UINTN)) {
+      DEBUG ((DEBUG_INFO, "OldCpuMpData->SizeOfUINTN: %04d, sizeof (UINTN): %04d\n", OldCpuMpData->SizeOfUINTN, sizeof (UINTN)));
+
+      OldCpuMpData->NewCpuMpData = OldCpuMpData;
+      //
+      // Initialize global data for MP support
+      //
+      InitMpGlobalData (OldCpuMpData);
+      return EFI_SUCCESS;
+    }
   }
 
   ASSERT (MaxLogicalProcessorNumber != 0);
@@ -1991,7 +2015,7 @@ MpInitLibInitialize (
   //
   ProgramVirtualWireMode ();
 
-  if (OldCpuMpData == NULL) {
+  if (FirstTimeMpInitializing) {
     if (MaxLogicalProcessorNumber > 1) {
       //
       // Wakeup all APs and calculate the processor count in system
@@ -2015,58 +2039,84 @@ MpInitLibInitialize (
     // Detect and apply Microcode on BSP
     //
     MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
+
+    //
+    // CpuMpData->CpuCount was updated in CollectProcessorCount()
+    // during the first time Mp Initialization.
+    //
+    if (CpuMpData->CpuCount > 1) {
+      WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
+      //
+      // Wait for all APs to complete initialization.
+      //
+      while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
+        CpuPause ();
+      }
+    }
+    //
+    // After Mp is initialized, sizeof(UINTN) is saved and will be transferred to DXE.
+    // It is used to decide whether Mp initialization needs to be done again.
+    // SizeOfUINTN and other fields (CpuInfoInHob, CpuCount, BspNumber) declared at the top of
+    // the CPU_MP_DATA structure are independent of the bit mode( 32 or 64).
+    //
+    CpuMpData->SizeOfUINTN = sizeof (UINTN);
   } else {
     //
-    // APs have been wakeup before, just get the CPU Information
-    // from HOB
+    // For the else case where DXE is 64-bit and PEI is 32-bit, a partial
+    // initialization of Mp is required.
     //
+
     OldCpuMpData->NewCpuMpData = CpuMpData;
-    CpuMpData->CpuCount        = OldCpuMpData->CpuCount;
-    CpuMpData->BspNumber       = OldCpuMpData->BspNumber;
-    CpuMpData->CpuInfoInHob    = OldCpuMpData->CpuInfoInHob;
-    CpuInfoInHob               = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
+    //
+    // CpuCount， BspNumber, CpuInfoInHob as well as SizeOfUINTN are collected during PEI,
+    // and need to be saved in HOB for DXE. The rest fields of _CPU_MP_DATA is more like the temporary field.
+    // They will be updated during DXE if the DXE mode is different from PEI.
+    //
+    CpuMpData->CpuCount     = OldCpuMpData->CpuCount;
+    CpuMpData->BspNumber    = OldCpuMpData->BspNumber;
+    CpuMpData->CpuInfoInHob = OldCpuMpData->CpuInfoInHob;
+    CpuInfoInHob            = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
     for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+      //
+      // In PEI phase, or for first time initialization, initializeSpinLock is
+      // performed during CollectProcessorCount(). In DXE phase, it is done during traversal here.
+      //
       InitializeSpinLock (&CpuMpData->CpuData[Index].ApLock);
       CpuMpData->CpuData[Index].CpuHealthy = (CpuInfoInHob[Index].Health == 0) ? TRUE : FALSE;
       CpuMpData->CpuData[Index].ApFunction = 0;
     }
-  }
 
-  //
-  // Store BSP's MTRR setting
-  //
-  MtrrGetAllMtrrs (&CpuMpData->MtrrTable);
-
-  //
-  // Wakeup APs to do some AP initialize sync (Microcode & MTRR)
-  //
-  if (CpuMpData->CpuCount > 1) {
-    if (OldCpuMpData != 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
-      // in DXE.
-      //
-      CpuMpData->InitFlag = ApInitReconfig;
-      WakeUpAP (CpuMpData, TRUE, 0, ApMtrrSync, CpuMpData, TRUE);
-    } else {
-      WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
-    }
+    //
+    // Store BSP's MTRR setting
+    //
+    MtrrGetAllMtrrs (&CpuMpData->MtrrTable);
 
     //
-    // Wait for all APs finished initialization
+    // Wakeup APs to do some AP initialize sync (Microcode & MTRR)
     //
-    while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
-      CpuPause ();
-    }
+    if (CpuMpData->CpuCount > 1) {
+      if (OldCpuMpData != 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
+        // in DXE.
+        //
+        CpuMpData->InitFlag = ApInitReconfig;
+        WakeUpAP (CpuMpData, TRUE, 0, ApMtrrSync, CpuMpData, TRUE);
+        //
+        // Wait for all APs to complete initialization.
+        //
+        while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
+          CpuPause ();
+        }
 
-    if (OldCpuMpData != NULL) {
-      CpuMpData->InitFlag = ApInitDone;
+        CpuMpData->InitFlag = ApInitDone;
+      }
     }
+  }
 
-    for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
-      SetApState (&CpuMpData->CpuData[Index], CpuStateIdle);
-    }
+  for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+    SetApState (&CpuMpData->CpuData[Index], CpuStateIdle);
   }
 
   //
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index f5086e497e..d6a416b943 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -235,6 +235,7 @@ struct _CPU_MP_DATA {
   UINT64                           CpuInfoInHob;
   UINT32                           CpuCount;
   UINT32                           BspNumber;
+  UINT32                           SizeOfUINTN;
   //
   // The above fields data will be passed from PEI to DXE
   // Please make sure the fields offset same in the different
-- 
2.36.1.windows.1

[PATCH 3/3] UefiCpuPkg: Removed the unused MpLock

[Yuanhao Xie]

From: Yuanhao <yuanhao.xie@intel.com>

Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 -
 UefiCpuPkg/Library/MpInitLib/MpLib.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index c7d1e58909..25e0b3c585 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1942,7 +1942,6 @@ MpInitLibInitialize (
   CpuMpData->SwitchBspFlag    = FALSE;
   CpuMpData->CpuData          = (CPU_AP_DATA *)(CpuMpData + 1);
   CpuMpData->CpuInfoInHob     = (UINT64)(UINTN)(CpuMpData->CpuData + MaxLogicalProcessorNumber);
-  InitializeSpinLock (&CpuMpData->MpLock);
   CpuMpData->SevEsIsEnabled   = ConfidentialComputingGuestHas (CCAttrAmdSevEs);
   CpuMpData->SevSnpIsEnabled  = ConfidentialComputingGuestHas (CCAttrAmdSevSnp);
   CpuMpData->SevEsAPBuffer    = (UINTN)-1;
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index d6a416b943..b5bd695ead 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -241,7 +241,6 @@ struct _CPU_MP_DATA {
   // Please make sure the fields offset same in the different
   // architecture.
   //
-  SPIN_LOCK                        MpLock;
   UINTN                            Buffer;
   UINTN                            CpuApStackSize;
   MP_ASSEMBLY_ADDRESS_MAP          AddressMap;
-- 
2.36.1.windows.1

Re: [edk2-devel] [PATCH 0/3] Remove the redundent Mp initialization

[Ni, Ray]

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

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yuanhao Xie
> Sent: Thursday, November 3, 2022 1:38 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH 0/3] Remove the redundent Mp initialization
> 
> 1. Removed the duplicated loading of microcode in DXE phase.
> 2. emoved repeated Mp initialization, taking into account the
> different bit mode (32bit/64bit) combinations of PEI and DXE stages.
> 3. Removed the unused MpLock.
> 
> Yuanhao (3):
>   UefiCpuPkg: Remove the duplicate loading of microcode in DXE stage
>   UefiCpuPkg: Skip Mp initialization in DXE phase
>   UefiCpuPkg: Removed the unused MpLock
> 
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 177
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++----------------------------------------------------
>  UefiCpuPkg/Library/MpInitLib/MpLib.h |   2 +-
>  2 files changed, 126 insertions(+), 53 deletions(-)
> 
> --
> 2.36.1.windows.1
> 
> 
> 
> 
>