Skip to content

Commit d493647

Browse files
xuweiintelmergify[bot]
authored andcommitted
UefiCpuPkg/PiSmmCpuDxeSmm: Add sync barrier before BSP invokes SmmCoreEntry
This patch introduces a synchronization point between the BSP and APs to ensure all APs have entered their SMM wait-loop (while (TRUE) in APHandler ()) before the BSP calls into the SMI handler logic via gSmmCpuPrivate ->SmmCoreEntry(). Previously, the BSP would invoke ReleaseAllAPs() and immediately proceed to SmmCoreEntry() without confirming whether APs had reached the stable waiting state. If SmmStartupThisAp() was called inside the SMI handler shortly after ReleaseAllAPs(), it might lead to a race condition: APs are issued two consecutive wait signals (SmmCpuSyncWaitForBsp()). BSP sends two consecutive releases (ReleaseAllAPs() + SmmStartupThisAp()) If an AP has not yet responded to the first release, the second release may overwrite the semaphore state, and the AP might miss the notification, causing it to hang or behave unpredictably. To address this: A SmmCpuSyncWaitForAPs() is added in BSP after mmCpuPlatformHookBeforeMmiHandler() and before entering SmmCoreEntry(). A matching SmmCpuSyncReleaseBsp() is added in AP immediately after its own SmmCpuPlatformHookBeforeMmiHandler() This ensures that BSP does not enter SMI handler logic or dispatch any AP-related requests before all APs are confirmed to be idle and ready. Debug sync point markers (e.g., /// cloud-hypervisor#6, tianocore#7) are updated accordingly. This change eliminates a subtle but critical race condition in multi-processor/multi-socket systems during SMM entry and improves overall synchronization safety. Signed-off-by: Wei6 Xu <wei6.xu@intel.com>
1 parent d15b572 commit d493647

File tree

1 file changed

+36
-12
lines changed

1 file changed

+36
-12
lines changed

UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,11 @@ BSPHandler (
561561
// Wait for all APs to complete their MTRR programming
562562
//
563563
SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #5: Wait APs
564+
565+
//
566+
// Notify all APs to continue
567+
//
568+
ReleaseAllAPs (); /// #6: Signal APs
564569
}
565570
}
566571

@@ -569,6 +574,13 @@ BSPHandler (
569574
//
570575
SmmCpuPlatformHookBeforeMmiHandler ();
571576

577+
if ((SyncMode == MmCpuSyncModeTradition) || SmmCpuFeaturesNeedConfigureMtrrs ()) {
578+
//
579+
// Wait for all APs of arrival at this point
580+
//
581+
SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #7: Wait APs
582+
}
583+
572584
//
573585
// The BUSY lock is initialized to Acquired state
574586
//
@@ -630,18 +642,18 @@ BSPHandler (
630642
// Notify all APs to exit
631643
//
632644
*mSmmMpSyncData->InsideSmm = FALSE;
633-
ReleaseAllAPs (); /// #6: Signal APs
645+
ReleaseAllAPs (); /// #8: Signal APs
634646

635647
if (SmmCpuFeaturesNeedConfigureMtrrs ()) {
636648
//
637649
// Wait for all APs the readiness to program MTRRs
638650
//
639-
SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #7: Wait APs
651+
SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #9: Wait APs
640652

641653
//
642654
// Signal APs to restore MTRRs
643655
//
644-
ReleaseAllAPs (); /// #8: Signal APs
656+
ReleaseAllAPs (); /// #10: Signal APs
645657

646658
//
647659
// Restore OS MTRRs
@@ -654,12 +666,12 @@ BSPHandler (
654666
//
655667
// Wait for all APs to complete their pending tasks including MTRR programming if needed.
656668
//
657-
SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #9: Wait APs
669+
SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #11: Wait APs
658670

659671
//
660672
// Signal APs to Reset states/semaphore for this processor
661673
//
662-
ReleaseAllAPs (); /// #10: Signal APs
674+
ReleaseAllAPs (); /// #12: Signal APs
663675
}
664676

665677
if (mSmmDebugAgentSupport) {
@@ -684,7 +696,7 @@ BSPHandler (
684696
// Gather APs to exit SMM synchronously. Note the Present flag is cleared by now but
685697
// WaitForAllAps does not depend on the Present flag.
686698
//
687-
SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #11: Wait APs
699+
SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #13: Wait APs
688700

689701
//
690702
// At this point, all APs should have exited from APHandler().
@@ -845,18 +857,30 @@ APHandler (
845857
// Signal BSP the completion of this AP
846858
//
847859
SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #5: Signal BSP
860+
861+
//
862+
// Wait for BSP's signal to continue
863+
//
864+
SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #6: Wait BSP
848865
}
849866

850867
//
851868
// Perform SMM CPU Platform Hook before executing MMI Handler
852869
//
853870
SmmCpuPlatformHookBeforeMmiHandler ();
854871

872+
if ((SyncMode == MmCpuSyncModeTradition) || SmmCpuFeaturesNeedConfigureMtrrs ()) {
873+
//
874+
// Notify BSP of arrival at this point
875+
//
876+
SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #7: Signal BSP
877+
}
878+
855879
while (TRUE) {
856880
//
857881
// Wait for something to happen
858882
//
859-
SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #6: Wait BSP
883+
SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #8: Wait BSP
860884

861885
//
862886
// Check if BSP wants to exit SMM
@@ -896,12 +920,12 @@ APHandler (
896920
//
897921
// Notify BSP the readiness of this AP to program MTRRs
898922
//
899-
SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #7: Signal BSP
923+
SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #9: Signal BSP
900924

901925
//
902926
// Wait for the signal from BSP to program MTRRs
903927
//
904-
SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #8: Wait BSP
928+
SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #10: Wait BSP
905929

906930
//
907931
// Restore OS MTRRs
@@ -914,12 +938,12 @@ APHandler (
914938
//
915939
// Notify BSP the readiness of this AP to Reset states/semaphore for this processor
916940
//
917-
SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #9: Signal BSP
941+
SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #11: Signal BSP
918942

919943
//
920944
// Wait for the signal from BSP to Reset states/semaphore for this processor
921945
//
922-
SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #10: Wait BSP
946+
SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #12: Wait BSP
923947
}
924948

925949
//
@@ -930,7 +954,7 @@ APHandler (
930954
//
931955
// Notify BSP the readiness of this AP to exit SMM
932956
//
933-
SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #11: Signal BSP
957+
SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #13: Signal BSP
934958
}
935959

936960
/**

0 commit comments

Comments
 (0)