UefiCpuPkg/PiSmmCpuDxeSmm: Add sync barrier before BSP invokes SmmCoreEntry
Some checks failed
CodeQL / Analyze (IA32, CryptoPkg) (push) Has been cancelled
CodeQL / Analyze (IA32, MdeModulePkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, DynamicTablesPkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, FatPkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, FmpDevicePkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, IntelFsp2Pkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, IntelFsp2WrapperPkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, MdePkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, PcAtChipsetPkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, PrmPkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, SecurityPkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, ShellPkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, SourceLevelDebugPkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, StandaloneMmPkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, UefiCpuPkg) (push) Has been cancelled
CodeQL / Analyze (IA32,X64, UnitTestFrameworkPkg) (push) Has been cancelled
CodeQL / Analyze (X64, CryptoPkg) (push) Has been cancelled
CodeQL / Analyze (X64, MdeModulePkg) (push) Has been cancelled
UPL Build / Build UPL VS2022 (FIT_BUILD=FALSE, windows-latest, 3.12, DEBUG, VS2022) (push) Has been cancelled
UPL Build / Build UPL VS2022 (FIT_BUILD=TRUE, windows-latest, 3.12, DEBUG, VS2022) (push) Has been cancelled
UPL Build / Build UPL GCC (FIT_BUILD=FALSE, ubuntu-latest, 3.12, DEBUG, GCC) (push) Has been cancelled
UPL Build / Build UPL GCC (FIT_BUILD=TRUE, ubuntu-latest, 3.12, DEBUG, GCC) (push) Has been cancelled

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>
This commit is contained in:
Wei6 Xu 2025-06-19 13:58:02 +08:00 committed by mergify[bot]
parent 92c714f8b7
commit bbee92c9af

View File

@ -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,11 @@ BSPHandler (
//
SmmCpuPlatformHookBeforeMmiHandler ();
//
// 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 +640,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 +664,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 +694,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 +855,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 +867,16 @@ APHandler (
//
SmmCpuPlatformHookBeforeMmiHandler ();
//
// 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 +916,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 +934,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 +950,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
}
/**