Based on PI spec description, disabled APs should not been include in the StartupAllAPs task. Current implementation always change AP state to CpuStateReady in WakeUpAP function which also let Disabled APs to do the task.
This patch reduce the CPU state to only three state: Idle, busy and disabled. The possible change process is: Idel -> busy -> Idle Idle -> disabled -> Idle Done Tests: 1.PI SCT Test 2.Boot OS / S3 Cc: Ruiyu Ni <[email protected]> Cc: Jeff Fan <[email protected]> Cc: Laszlo Ersek <[email protected]> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <[email protected]> --- UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 +++++++++++++--------------------- UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 -- 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index f2ff40417a..3945771764 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -460,8 +460,6 @@ CollectProcessorCount ( IN CPU_MP_DATA *CpuMpData ) { - UINTN Index; - // // Send 1st broadcast IPI to APs to wakeup APs // @@ -499,12 +497,6 @@ CollectProcessorCount ( // Enable x2APIC on BSP // SetApicMode (LOCAL_APIC_MODE_X2APIC); - // - // Set BSP/Aps state to IDLE - // - for (Index = 0; Index < CpuMpData->CpuCount; Index++) { - SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); - } } DEBUG ((DEBUG_INFO, "APIC MODE is %d\n", GetApicMode ())); // @@ -648,7 +640,7 @@ ApWakeupFunction ( CpuFlushTlb (); } - if (GetApState (&CpuMpData->CpuData[ProcessorNumber]) == CpuStateReady) { + if (GetApState (&CpuMpData->CpuData[ProcessorNumber]) == CpuStateIdle) { Procedure = (EFI_AP_PROCEDURE)CpuMpData->CpuData[ProcessorNumber].ApFunction; Parameter = (VOID *) CpuMpData->CpuData[ProcessorNumber].ApFunctionArgument; if (Procedure != NULL) { @@ -691,7 +683,7 @@ ApWakeupFunction ( } } } - SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateFinished); + SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateIdle); } } @@ -1005,7 +997,6 @@ WakeUpAP ( CpuData = &CpuMpData->CpuData[Index]; CpuData->ApFunction = (UINTN) Procedure; CpuData->ApFunctionArgument = (UINTN) ProcedureArgument; - SetApState (CpuData, CpuStateReady); if (CpuMpData->InitFlag != ApInitConfig) { *(UINT32 *) CpuData->StartupApSignal = WAKEUP_AP_SIGNAL; } @@ -1052,7 +1043,6 @@ WakeUpAP ( CpuData = &CpuMpData->CpuData[ProcessorNumber]; CpuData->ApFunction = (UINTN) Procedure; CpuData->ApFunctionArgument = (UINTN) ProcedureArgument; - SetApState (CpuData, CpuStateReady); // // Wakeup specified AP // @@ -1345,11 +1335,10 @@ CheckThisAP ( // // If the AP finishes for StartupThisAP(), return EFI_SUCCESS. // - if (GetApState(CpuData) == CpuStateFinished) { + if (GetApState(CpuData) != CpuStateBusy) { if (CpuData->Finished != NULL) { *(CpuData->Finished) = TRUE; } - SetApState (CpuData, CpuStateIdle); return EFI_SUCCESS; } else { // @@ -1410,10 +1399,9 @@ CheckAllAPs ( // Only BSP and corresponding AP access this unit of CPU Data. This means the AP will not modify the // value of state after setting the it to CpuStateFinished, so BSP can safely make use of its value. // - if (GetApState(CpuData) == CpuStateFinished) { + if (GetApState(CpuData) != CpuStateBusy) { CpuMpData->RunningCount ++; CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE; - SetApState(CpuData, CpuStateIdle); // // If in Single Thread mode, then search for the next waiting AP for execution. @@ -1625,6 +1613,13 @@ MpInitLibInitialize ( ); } if (MaxLogicalProcessorNumber > 1) { + // + // Initial AP to idle state, later ApWakeupFunction based on + // this state to decide whether need to execute AP procedure. + // + for (Index = 0; Index < CpuMpData->CpuCount; Index++) { + SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); + } // // Wakeup APs to do some AP initialize sync // @@ -1636,9 +1631,6 @@ MpInitLibInitialize ( CpuPause (); } CpuMpData->InitFlag = ApInitDone; - for (Index = 0; Index < CpuMpData->CpuCount; Index++) { - SetApState (&CpuMpData->CpuData[Index], CpuStateIdle); - } } } @@ -1838,7 +1830,7 @@ SwitchBSPWorker ( // // Wait for old BSP finished AP task // - while (GetApState (&CpuMpData->CpuData[CallerNumber]) != CpuStateFinished) { + while (GetApState (&CpuMpData->CpuData[CallerNumber]) == CpuStateBusy) { CpuPause (); } @@ -2112,7 +2104,7 @@ StartupAllAPsWorker ( ApState = GetApState (CpuData); if (ApState != CpuStateDisabled) { HasEnabledAp = TRUE; - if (ApState != CpuStateIdle) { + if (ApState == CpuStateBusy) { // // If any enabled APs are busy, return EFI_NOT_READY. // diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h index e7f9a4de0a..90c09fb8fb 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h @@ -83,9 +83,7 @@ typedef enum { // typedef enum { CpuStateIdle, - CpuStateReady, CpuStateBusy, - CpuStateFinished, CpuStateDisabled } CPU_STATE; -- 2.15.0.windows.1 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
