This patch is to define 3 new functions (WaitForBsp & ReleaseBsp &
ReleaseOneAp) used for the semaphore sync between BSP & AP. With the
change, BSP and AP Sync flow will be easy understand as below:
BSP: ReleaseAllAPs or ReleaseOneAp --> AP: WaitForBsp
BSP: WaitForAllAPs                 <-- AP: ReleaseBsp

Cc: Laszlo Ersek <lersek@redhat.com>
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 b279f5dfcc..54542262a2 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 - 1
+**/
+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
+      // WaitForAllAPs() 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
+      // WaitForAllAPs() 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 (#112109): https://edk2.groups.io/g/devel/message/112109
Mute This Topic: https://groups.io/mt/103010163/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 12/6/23 11:01, Wu, Jiaxin wrote:
> This patch is to define 3 new functions (WaitForBsp & ReleaseBsp &
> ReleaseOneAp) used for the semaphore sync between BSP & AP. With the
> change, BSP and AP Sync flow will be easy understand as below:
> BSP: ReleaseAllAPs or ReleaseOneAp --> AP: WaitForBsp
> BSP: WaitForAllAPs                 <-- AP: ReleaseBsp
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> 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(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>




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