diff options
author | Wei6 Xu <wei6.xu@intel.com> | 2025-06-19 13:58:02 +0800 |
---|---|---|
committer | mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> | 2025-07-01 17:46:33 +0000 |
commit | d49364779c53f0aba2fd445345c8999d548f6a90 (patch) | |
tree | 36184ebb419bf93f88331470189ccf5fb2f9cbe5 | |
parent | d15b57292f345db25f1908c5ef43d0fd0c9d409e (diff) | |
download | edk2-d49364779c53f0aba2fd445345c8999d548f6a90.zip edk2-d49364779c53f0aba2fd445345c8999d548f6a90.tar.gz edk2-d49364779c53f0aba2fd445345c8999d548f6a90.tar.bz2 |
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., /// #6, #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>
-rw-r--r-- | UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 48 |
1 files changed, 36 insertions, 12 deletions
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c index 7bc6808..7402a28 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c @@ -561,6 +561,11 @@ BSPHandler ( // Wait for all APs to complete their MTRR programming
//
SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #5: Wait APs
+
+ //
+ // Notify all APs to continue
+ //
+ ReleaseAllAPs (); /// #6: Signal APs
}
}
@@ -569,6 +574,13 @@ BSPHandler ( //
SmmCpuPlatformHookBeforeMmiHandler ();
+ if ((SyncMode == MmCpuSyncModeTradition) || SmmCpuFeaturesNeedConfigureMtrrs ()) {
+ //
+ // Wait for all APs of arrival at this point
+ //
+ SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #7: Wait APs
+ }
+
//
// The BUSY lock is initialized to Acquired state
//
@@ -630,18 +642,18 @@ BSPHandler ( // Notify all APs to exit
//
*mSmmMpSyncData->InsideSmm = FALSE;
- ReleaseAllAPs (); /// #6: Signal APs
+ ReleaseAllAPs (); /// #8: Signal APs
if (SmmCpuFeaturesNeedConfigureMtrrs ()) {
//
// Wait for all APs the readiness to program MTRRs
//
- SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #7: Wait APs
+ SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #9: Wait APs
//
// Signal APs to restore MTRRs
//
- ReleaseAllAPs (); /// #8: Signal APs
+ ReleaseAllAPs (); /// #10: Signal APs
//
// Restore OS MTRRs
@@ -654,12 +666,12 @@ BSPHandler ( //
// Wait for all APs to complete their pending tasks including MTRR programming if needed.
//
- SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #9: Wait APs
+ SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #11: Wait APs
//
// Signal APs to Reset states/semaphore for this processor
//
- ReleaseAllAPs (); /// #10: Signal APs
+ ReleaseAllAPs (); /// #12: Signal APs
}
if (mSmmDebugAgentSupport) {
@@ -684,7 +696,7 @@ BSPHandler ( // Gather APs to exit SMM synchronously. Note the Present flag is cleared by now but
// WaitForAllAps does not depend on the Present flag.
//
- SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #11: Wait APs
+ SmmCpuSyncWaitForAPs (mSmmMpSyncData->SyncContext, ApCount, CpuIndex); /// #13: Wait APs
//
// At this point, all APs should have exited from APHandler().
@@ -845,6 +857,11 @@ APHandler ( // Signal BSP the completion of this AP
//
SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #5: Signal BSP
+
+ //
+ // Wait for BSP's signal to continue
+ //
+ SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #6: Wait BSP
}
//
@@ -852,11 +869,18 @@ APHandler ( //
SmmCpuPlatformHookBeforeMmiHandler ();
+ if ((SyncMode == MmCpuSyncModeTradition) || SmmCpuFeaturesNeedConfigureMtrrs ()) {
+ //
+ // Notify BSP of arrival at this point
+ //
+ SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #7: Signal BSP
+ }
+
while (TRUE) {
//
// Wait for something to happen
//
- SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #6: Wait BSP
+ SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #8: Wait BSP
//
// Check if BSP wants to exit SMM
@@ -896,12 +920,12 @@ APHandler ( //
// Notify BSP the readiness of this AP to program MTRRs
//
- SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #7: Signal BSP
+ SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #9: Signal BSP
//
// Wait for the signal from BSP to program MTRRs
//
- SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #8: Wait BSP
+ SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #10: Wait BSP
//
// Restore OS MTRRs
@@ -914,12 +938,12 @@ APHandler ( //
// Notify BSP the readiness of this AP to Reset states/semaphore for this processor
//
- SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #9: Signal BSP
+ SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #11: Signal BSP
//
// Wait for the signal from BSP to Reset states/semaphore for this processor
//
- SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #10: Wait BSP
+ SmmCpuSyncWaitForBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #12: Wait BSP
}
//
@@ -930,7 +954,7 @@ APHandler ( //
// Notify BSP the readiness of this AP to exit SMM
//
- SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #11: Signal BSP
+ SmmCpuSyncReleaseBsp (mSmmMpSyncData->SyncContext, CpuIndex, BspIndex); /// #13: Signal BSP
}
/**
|