+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.d...@intel.com>; Ni, Ray <ray...@intel.com>; Zeng, Star > <star.z...@intel.com>; Kumar, Rahul R <rahul.r.ku...@intel.com>; Gerd > Hoffmann <kra...@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.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Zeng Star <star.z...@intel.com> > Cc: Rahul Kumar <rahul1.ku...@intel.com> > Cc: Gerd Hoffmann <kra...@redhat.com> > Signed-off-by: Jiaxin Wu <jiaxin...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-