mirror of
https://github.com/tianocore/edk2.git
synced 2025-08-26 13:14:41 +00:00
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
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:
parent
92c714f8b7
commit
bbee92c9af
@ -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
|
||||
}
|
||||
|
||||
/**
|
||||
|
Loading…
Reference in New Issue
Block a user