public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch 1/2] UefiCpuPkg/MpInitLib: Not use disabled AP.
@ 2018-06-28 11:29 Eric Dong
  2018-06-28 11:29 ` [Patch 2/2] UefiCpuPkg/MpInitLib: Remove redundant parameter Eric Dong
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Dong @ 2018-06-28 11:29 UTC (permalink / raw)
  To: edk2-devel; +Cc: Ruiyu Ni, Jeff Fan, Laszlo Ersek

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



^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-06-29  7:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-28 11:29 [Patch 1/2] UefiCpuPkg/MpInitLib: Not use disabled AP Eric Dong
2018-06-28 11:29 ` [Patch 2/2] UefiCpuPkg/MpInitLib: Remove redundant parameter Eric Dong
2018-06-28 15:37   ` Laszlo Ersek
2018-06-29  1:48     ` Dong, Eric
2018-06-29  7:19       ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox