Reviewed-by: Ray Ni <[email protected]> Thanks, Ray > -----Original Message----- > From: Wu, Jiaxin <[email protected]> > Sent: Friday, December 15, 2023 5:55 PM > To: [email protected] > Cc: Laszlo Ersek <[email protected]>; Dong, Eric <[email protected]>; Ni, > Ray <[email protected]>; Zeng, Star <[email protected]>; Kumar, Rahul R > <[email protected]>; Gerd Hoffmann <[email protected]> > Subject: [PATCH v4 1/8] UefiCpuPkg/PiSmmCpuDxeSmm: Optimize > Semaphore Sync between BSP and AP > > 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 <[email protected]> > Cc: Eric Dong <[email protected]> > Cc: Ray Ni <[email protected]> > Cc: Zeng Star <[email protected]> > Cc: Rahul Kumar <[email protected]> > Cc: Gerd Hoffmann <[email protected]> > Signed-off-by: Jiaxin Wu <[email protected]> > Reviewed-by: Laszlo Ersek <[email protected]> > --- > 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 (#112682): https://edk2.groups.io/g/devel/message/112682 Mute This Topic: https://groups.io/mt/103187891/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
