public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH v1] UefiCpuPkg/SmmCpu: Refine semaphore sync between BSP and AP
@ 2023-08-09  9:04 Wu, Jiaxin
  0 siblings, 0 replies; 2+ messages in thread
From: Wu, Jiaxin @ 2023-08-09  9:04 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Zeng Star, Rahul Kumar, Gerd Hoffmann

For SMM CPU semaphore sync, there is no need atomic semaphore
operation, just use the flag to indicate it has complete the
wait/release. Based on this, this patch is to refine 2 functions
(WaitForAllAPs & ReleaseAllAPs) and define 2 new functions
(WaitForBsp & ReleaseBsp) used for the semaphore sync between
BSP & AP.

Sync flow like below:
1. BSP to Release All APs ---> 1. AP to Wait BSP
   ReleaseAllAPs ()               WaitForBsp
2. BSP to Wait All APs    <--- 2. AP to Release BSP
   WaitForAllAPs ()               ReleaseBsp

With this change, SMM CPU semaphore sync for SMI exit performance
will be significant improved.

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 | 68 ++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 17 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 25d058c5b9..0bf460e81c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -120,11 +120,11 @@ LockdownSemaphore (
 
   return Value;
 }
 
 /**
-  Wait all APs to performs an atomic compare exchange operation to release semaphore.
+  Used for BSP to wait all APs.
 
   @param   NumberOfAPs      AP number
 
 **/
 VOID
@@ -133,18 +133,19 @@ WaitForAllAPs (
   )
 {
   UINTN  BspIndex;
 
   BspIndex = mSmmMpSyncData->BspIndex;
-  while (NumberOfAPs-- > 0) {
-    WaitForSemaphore (mSmmMpSyncData->CpuData[BspIndex].Run);
+  while (NumberOfAPs != *mSmmMpSyncData->CpuData[BspIndex].Run) {
+    CpuPause ();
   }
+
+  *mSmmMpSyncData->CpuData[BspIndex].Run = 0;
 }
 
 /**
-  Performs an atomic compare exchange operation to release semaphore
-  for each AP.
+  Used for BSP to release all APs.
 
 **/
 VOID
 ReleaseAllAPs (
   VOID
@@ -152,15 +153,48 @@ ReleaseAllAPs (
 {
   UINTN  Index;
 
   for (Index = 0; Index < mMaxNumberOfCpus; Index++) {
     if (IsPresentAp (Index)) {
-      ReleaseSemaphore (mSmmMpSyncData->CpuData[Index].Run);
+      ASSERT (*mSmmMpSyncData->CpuData[Index].Run == 0);
+      *mSmmMpSyncData->CpuData[Index].Run  = 1;
     }
   }
 }
 
+/**
+  Used for Ap to wait BSP.
+
+  @param      ApSem      IN:  32-bit unsigned integer
+                         OUT: original integer 0
+**/
+VOID
+WaitForBsp  (
+  IN OUT  volatile UINT32 *ApSem
+  )
+{
+  while (*ApSem == 0) {
+    CpuPause ();
+  }
+
+  *ApSem = 0;
+}
+
+/**
+  Used for Ap to release BSP.
+
+  @param      BspSem     IN:  32-bit unsigned integer
+                         OUT: original integer + 1
+**/
+VOID
+ReleaseBsp   (
+  IN OUT  volatile UINT32  *BspSem
+  )
+{
+  InterlockedIncrement (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]
@@ -898,50 +932,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 +1011,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 +1028,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.
 
-- 
2.16.2.windows.1



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



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

* Re: [edk2-devel] [PATCH v1] UefiCpuPkg/SmmCpu: Refine semaphore sync between BSP and AP
       [not found] <1779ABA9805F8956.9264@groups.io>
@ 2023-08-16  0:26 ` Wu, Jiaxin
  0 siblings, 0 replies; 2+ messages in thread
From: Wu, Jiaxin @ 2023-08-16  0:26 UTC (permalink / raw)
  To: devel@edk2.groups.io, Wu, Jiaxin, Kinney, Michael D
  Cc: Dong, Eric, Ni, Ray, Zeng, Star, Kumar, Rahul R, Gerd Hoffmann

+Mike for this review. 



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu,
> Jiaxin
> Sent: Wednesday, August 9, 2023 5:04 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd
> Hoffmann <kraxel@redhat.com>
> Subject: [edk2-devel] [PATCH v1] UefiCpuPkg/SmmCpu: Refine semaphore
> sync between BSP and AP
> 
> For SMM CPU semaphore sync, there is no need atomic semaphore
> operation, just use the flag to indicate it has complete the
> wait/release. Based on this, this patch is to refine 2 functions
> (WaitForAllAPs & ReleaseAllAPs) and define 2 new functions
> (WaitForBsp & ReleaseBsp) used for the semaphore sync between
> BSP & AP.
> 
> Sync flow like below:
> 1. BSP to Release All APs ---> 1. AP to Wait BSP
>    ReleaseAllAPs ()               WaitForBsp
> 2. BSP to Wait All APs    <--- 2. AP to Release BSP
>    WaitForAllAPs ()               ReleaseBsp
> 
> With this change, SMM CPU semaphore sync for SMI exit performance
> will be significant improved.
> 
> 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 | 68
> ++++++++++++++++++++++++++---------
>  1 file changed, 51 insertions(+), 17 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index 25d058c5b9..0bf460e81c 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -120,11 +120,11 @@ LockdownSemaphore (
> 
>    return Value;
>  }
> 
>  /**
> -  Wait all APs to performs an atomic compare exchange operation to release
> semaphore.
> +  Used for BSP to wait all APs.
> 
>    @param   NumberOfAPs      AP number
> 
>  **/
>  VOID
> @@ -133,18 +133,19 @@ WaitForAllAPs (
>    )
>  {
>    UINTN  BspIndex;
> 
>    BspIndex = mSmmMpSyncData->BspIndex;
> -  while (NumberOfAPs-- > 0) {
> -    WaitForSemaphore (mSmmMpSyncData->CpuData[BspIndex].Run);
> +  while (NumberOfAPs != *mSmmMpSyncData->CpuData[BspIndex].Run) {
> +    CpuPause ();
>    }
> +
> +  *mSmmMpSyncData->CpuData[BspIndex].Run = 0;
>  }
> 
>  /**
> -  Performs an atomic compare exchange operation to release semaphore
> -  for each AP.
> +  Used for BSP to release all APs.
> 
>  **/
>  VOID
>  ReleaseAllAPs (
>    VOID
> @@ -152,15 +153,48 @@ ReleaseAllAPs (
>  {
>    UINTN  Index;
> 
>    for (Index = 0; Index < mMaxNumberOfCpus; Index++) {
>      if (IsPresentAp (Index)) {
> -      ReleaseSemaphore (mSmmMpSyncData->CpuData[Index].Run);
> +      ASSERT (*mSmmMpSyncData->CpuData[Index].Run == 0);
> +      *mSmmMpSyncData->CpuData[Index].Run  = 1;
>      }
>    }
>  }
> 
> +/**
> +  Used for Ap to wait BSP.
> +
> +  @param      ApSem      IN:  32-bit unsigned integer
> +                         OUT: original integer 0
> +**/
> +VOID
> +WaitForBsp  (
> +  IN OUT  volatile UINT32 *ApSem
> +  )
> +{
> +  while (*ApSem == 0) {
> +    CpuPause ();
> +  }
> +
> +  *ApSem = 0;
> +}
> +
> +/**
> +  Used for Ap to release BSP.
> +
> +  @param      BspSem     IN:  32-bit unsigned integer
> +                         OUT: original integer + 1
> +**/
> +VOID
> +ReleaseBsp   (
> +  IN OUT  volatile UINT32  *BspSem
> +  )
> +{
> +  InterlockedIncrement (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]
> @@ -898,50 +932,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 +1011,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 +1028,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.
> 
> --
> 2.16.2.windows.1
> 
> 
> 
> 
> 



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



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

end of thread, other threads:[~2023-08-16  0:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-09  9:04 [edk2-devel] [PATCH v1] UefiCpuPkg/SmmCpu: Refine semaphore sync between BSP and AP Wu, Jiaxin
     [not found] <1779ABA9805F8956.9264@groups.io>
2023-08-16  0:26 ` Wu, Jiaxin

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