[Patch 1/2] UefiCpuPkg/MpInitLib: Not use disabled AP.

[Eric Dong <eric.dong@intel.com>]

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 <ruiyu.ni@intel.com>
Cc: Jeff Fan <vanjeff_919@hotmail.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 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