[edk2-devel] [PATCH v1 0/2] Optimize semaphore sync between BSP and AP

[edk2-devel] [PATCH v1 0/2] Optimize semaphore sync between BSP and AP

[Wu, Jiaxin]

The series patches are to optimize semaphore sync between BSP
and AP:
Patch 1: Define 3 functions (WaitForBsp & ReleaseBsp & ReleaseOneAp)
specific for BSP & AP sync, which will make the flow easy to
understand.
Patch 2: Reduce one round BSP and AP sync for exit so as to
improve SMM performance

Jiaxin Wu (2):
  UefiCpuPkg/PiSmmCpuDxeSmm: Optimize semaphore sync between BSP and AP
  UefiCpuPkg/PiSmmCpuDxeSmm: Reduce the times of BSP and AP sync for
    exit

 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 112 ++++++++++++++++++++++++----------
 1 file changed, 80 insertions(+), 32 deletions(-)

-- 
2.16.2.windows.1



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

[edk2-devel] [PATCH v1 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Optimize semaphore sync between BSP and AP

[Wu, Jiaxin]

This patch is to:
1. Define 2 new functions (WaitForBsp & ReleaseBsp) used
for the semaphore sync between BSP & AP.
2. Add ReleaseOneAp(), used for BSP to release one AP.

With the change, BSP & AP Sync flow will be easy understand:
BSP to Release All APs ---> AP to Wait BSP
ReleaseAllAPs ()            WaitForBsp
BSP to Wait All APs    <--- AP to Release BSP
WaitForAllAPs ()            ReleaseBsp

BSP to Release One Ap  ---> AP to Wait BSP
ReleaseOneAp ()             ReleaseBsp

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 72 ++++++++++++++++++++++++++++-------
 1 file changed, 58 insertions(+), 14 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 25d058c5b9..e96c7f51d6 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -120,10 +120,11 @@ LockdownSemaphore (
 
   return Value;
 }
 
 /**
+  Used for BSP to wait all APs.
   Wait all APs to performs an atomic compare exchange operation to release semaphore.
 
   @param   NumberOfAPs      AP number
 
 **/
@@ -139,10 +140,11 @@ WaitForAllAPs (
     WaitForSemaphore (mSmmMpSyncData->CpuData[BspIndex].Run);
   }
 }
 
 /**
+  Used for BSP to release all APs.
   Performs an atomic compare exchange operation to release semaphore
   for each AP.
 
 **/
 VOID
@@ -157,10 +159,52 @@ ReleaseAllAPs (
       ReleaseSemaphore (mSmmMpSyncData->CpuData[Index].Run);
     }
   }
 }
 
+/**
+  Used for BSP to release one AP.
+
+  @param      ApSem     IN:  32-bit unsigned integer
+                        OUT: original integer + 1
+**/
+VOID
+ReleaseOneAp   (
+  IN OUT  volatile UINT32  *ApSem
+  )
+{
+  ReleaseSemaphore (ApSem);
+}
+
+/**
+  Used for AP to wait BSP.
+
+  @param      ApSem      IN:  32-bit unsigned integer
+                         OUT: original integer 0
+**/
+VOID
+WaitForBsp  (
+  IN OUT  volatile UINT32  *ApSem
+  )
+{
+  WaitForSemaphore (ApSem);
+}
+
+/**
+  Used for AP to release BSP.
+
+  @param      BspSem     IN:  32-bit unsigned integer
+                         OUT: original integer + 1
+**/
+VOID
+ReleaseBsp   (
+  IN OUT  volatile UINT32  *BspSem
+  )
+{
+  ReleaseSemaphore (BspSem);
+}
+
 /**
   Check whether the index of CPU perform the package level register
   programming during System Management Mode initialization.
 
   The index of Processor specified by mPackageFirstThreadIndex[PackageIndex]
@@ -632,11 +676,11 @@ BSPHandler (
       // Signal all APs it's time for backup MTRRs
       //
       ReleaseAllAPs ();
 
       //
-      // WaitForSemaphore() may wait for ever if an AP happens to enter SMM at
+      // WaitForBsp() may wait for ever if an AP happens to enter SMM at
       // exactly this point. Please make sure PcdCpuSmmMaxSyncLoops has been set
       // to a large enough value to avoid this situation.
       // Note: For HT capable CPUs, threads within a core share the same set of MTRRs.
       // We do the backup first and then set MTRR to avoid race condition for threads
       // in the same core.
@@ -652,11 +696,11 @@ BSPHandler (
       // Let all processors program SMM MTRRs together
       //
       ReleaseAllAPs ();
 
       //
-      // WaitForSemaphore() may wait for ever if an AP happens to enter SMM at
+      // WaitForBsp() may wait for ever if an AP happens to enter SMM at
       // exactly this point. Please make sure PcdCpuSmmMaxSyncLoops has been set
       // to a large enough value to avoid this situation.
       //
       ReplaceOSMtrrs (CpuIndex);
 
@@ -898,50 +942,50 @@ APHandler (
 
   if ((SyncMode == SmmCpuSyncModeTradition) || SmmCpuFeaturesNeedConfigureMtrrs ()) {
     //
     // Notify BSP of arrival at this point
     //
-    ReleaseSemaphore (mSmmMpSyncData->CpuData[BspIndex].Run);
+    ReleaseBsp (mSmmMpSyncData->CpuData[BspIndex].Run);
   }
 
   if (SmmCpuFeaturesNeedConfigureMtrrs ()) {
     //
     // Wait for the signal from BSP to backup MTRRs
     //
-    WaitForSemaphore (mSmmMpSyncData->CpuData[CpuIndex].Run);
+    WaitForBsp (mSmmMpSyncData->CpuData[CpuIndex].Run);
 
     //
     // Backup OS MTRRs
     //
     MtrrGetAllMtrrs (&Mtrrs);
 
     //
     // Signal BSP the completion of this AP
     //
-    ReleaseSemaphore (mSmmMpSyncData->CpuData[BspIndex].Run);
+    ReleaseBsp (mSmmMpSyncData->CpuData[BspIndex].Run);
 
     //
     // Wait for BSP's signal to program MTRRs
     //
-    WaitForSemaphore (mSmmMpSyncData->CpuData[CpuIndex].Run);
+    WaitForBsp (mSmmMpSyncData->CpuData[CpuIndex].Run);
 
     //
     // Replace OS MTRRs with SMI MTRRs
     //
     ReplaceOSMtrrs (CpuIndex);
 
     //
     // Signal BSP the completion of this AP
     //
-    ReleaseSemaphore (mSmmMpSyncData->CpuData[BspIndex].Run);
+    ReleaseBsp (mSmmMpSyncData->CpuData[BspIndex].Run);
   }
 
   while (TRUE) {
     //
     // Wait for something to happen
     //
-    WaitForSemaphore (mSmmMpSyncData->CpuData[CpuIndex].Run);
+    WaitForBsp (mSmmMpSyncData->CpuData[CpuIndex].Run);
 
     //
     // Check if BSP wants to exit SMM
     //
     if (!(*mSmmMpSyncData->InsideSmm)) {
@@ -977,16 +1021,16 @@ APHandler (
 
   if (SmmCpuFeaturesNeedConfigureMtrrs ()) {
     //
     // Notify BSP the readiness of this AP to program MTRRs
     //
-    ReleaseSemaphore (mSmmMpSyncData->CpuData[BspIndex].Run);
+    ReleaseBsp (mSmmMpSyncData->CpuData[BspIndex].Run);
 
     //
     // Wait for the signal from BSP to program MTRRs
     //
-    WaitForSemaphore (mSmmMpSyncData->CpuData[CpuIndex].Run);
+    WaitForBsp (mSmmMpSyncData->CpuData[CpuIndex].Run);
 
     //
     // Restore OS MTRRs
     //
     SmmCpuFeaturesReenableSmrr ();
@@ -994,26 +1038,26 @@ APHandler (
   }
 
   //
   // Notify BSP the readiness of this AP to Reset states/semaphore for this processor
   //
-  ReleaseSemaphore (mSmmMpSyncData->CpuData[BspIndex].Run);
+  ReleaseBsp (mSmmMpSyncData->CpuData[BspIndex].Run);
 
   //
   // Wait for the signal from BSP to Reset states/semaphore for this processor
   //
-  WaitForSemaphore (mSmmMpSyncData->CpuData[CpuIndex].Run);
+  WaitForBsp (mSmmMpSyncData->CpuData[CpuIndex].Run);
 
   //
   // Reset states/semaphore for this processor
   //
   *(mSmmMpSyncData->CpuData[CpuIndex].Present) = FALSE;
 
   //
   // Notify BSP the readiness of this AP to exit SMM
   //
-  ReleaseSemaphore (mSmmMpSyncData->CpuData[BspIndex].Run);
+  ReleaseBsp (mSmmMpSyncData->CpuData[BspIndex].Run);
 }
 
 /**
   Checks whether the input token is the current used token.
 
@@ -1277,11 +1321,11 @@ InternalSmmStartupThisAp (
   mSmmMpSyncData->CpuData[CpuIndex].Status = CpuStatus;
   if (mSmmMpSyncData->CpuData[CpuIndex].Status != NULL) {
     *mSmmMpSyncData->CpuData[CpuIndex].Status = EFI_NOT_READY;
   }
 
-  ReleaseSemaphore (mSmmMpSyncData->CpuData[CpuIndex].Run);
+  ReleaseOneAp (mSmmMpSyncData->CpuData[CpuIndex].Run);
 
   if (Token == NULL) {
     AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
     ReleaseSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
   }
-- 
2.16.2.windows.1



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

[edk2-devel] [PATCH v1 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Reduce the times of BSP and AP sync for exit

[Wu, Jiaxin]

After review, there are unnecessary steps for BSP and AP sync for exit.
This patch is to reduce one round BSP and AP sync for exit so as to
improve SMM performance:
WaitForAllAPs <- ReleaseBsp
ReleaseAllAPs -> WaitForBsp

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 44 +++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index e96c7f51d6..5a42a5dd12 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -665,11 +665,13 @@ BSPHandler (
     //
     *mSmmMpSyncData->AllCpusInSync = TRUE;
     ApCount                        = LockdownSemaphore (mSmmMpSyncData->Counter) - 1;
 
     //
-    // Wait for all APs to get ready for programming MTRRs
+    // Wait for all APs:
+    // 1. Make sure all Aps have set the Present.
+    // 2. Get ready for programming MTRRs.
     //
     WaitForAllAPs (ApCount);
 
     if (SmmCpuFeaturesNeedConfigureMtrrs ()) {
       //
@@ -768,16 +770,16 @@ BSPHandler (
   // Notify all APs to exit
   //
   *mSmmMpSyncData->InsideSmm = FALSE;
   ReleaseAllAPs ();
 
-  //
-  // Wait for all APs to complete their pending tasks
-  //
-  WaitForAllAPs (ApCount);
-
   if (SmmCpuFeaturesNeedConfigureMtrrs ()) {
+    //
+    // Wait for all APs to complete their pending tasks
+    //
+    WaitForAllAPs (ApCount);
+
     //
     // Signal APs to restore MTRRs
     //
     ReleaseAllAPs ();
 
@@ -789,23 +791,23 @@ BSPHandler (
 
     //
     // Wait for all APs to complete MTRR programming
     //
     WaitForAllAPs (ApCount);
+
+    //
+    // Signal APs to Reset states/semaphore for this processor
+    //
+    ReleaseAllAPs ();
   }
 
   //
   // Stop source level debug in BSP handler, the code below will not be
   // debugged.
   //
   InitializeDebugAgent (DEBUG_AGENT_INIT_EXIT_SMI, NULL, NULL);
 
-  //
-  // Signal APs to Reset states/semaphore for this processor
-  //
-  ReleaseAllAPs ();
-
   //
   // Perform pending operations for hot-plug
   //
   SmmCpuUpdate ();
 
@@ -941,10 +943,12 @@ APHandler (
   *(mSmmMpSyncData->CpuData[CpuIndex].Present) = TRUE;
 
   if ((SyncMode == SmmCpuSyncModeTradition) || SmmCpuFeaturesNeedConfigureMtrrs ()) {
     //
     // Notify BSP of arrival at this point
+    // 1. Set the Present.
+    // 2. Get ready for programming MTRRs.
     //
     ReleaseBsp (mSmmMpSyncData->CpuData[BspIndex].Run);
   }
 
   if (SmmCpuFeaturesNeedConfigureMtrrs ()) {
@@ -1033,21 +1037,21 @@ APHandler (
     //
     // Restore OS MTRRs
     //
     SmmCpuFeaturesReenableSmrr ();
     MtrrSetAllMtrrs (&Mtrrs);
-  }
 
-  //
-  // Notify BSP the readiness of this AP to Reset states/semaphore for this processor
-  //
-  ReleaseBsp (mSmmMpSyncData->CpuData[BspIndex].Run);
+    //
+    // Notify BSP the readiness of this AP to Reset states/semaphore for this processor
+    //
+    ReleaseBsp (mSmmMpSyncData->CpuData[BspIndex].Run);
 
-  //
-  // Wait for the signal from BSP to Reset states/semaphore for this processor
-  //
-  WaitForBsp (mSmmMpSyncData->CpuData[CpuIndex].Run);
+    //
+    // Wait for the signal from BSP to Reset states/semaphore for this processor
+    //
+    WaitForBsp (mSmmMpSyncData->CpuData[CpuIndex].Run);
+  }
 
   //
   // Reset states/semaphore for this processor
   //
   *(mSmmMpSyncData->CpuData[CpuIndex].Present) = FALSE;
-- 
2.16.2.windows.1



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