public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] UefiCpuPkg/MpInitLib: Add check for CR3/GDT/IDT.
@ 2020-09-02  0:43 Dong, Eric
  2020-09-02  7:42 ` [edk2-devel] " Laszlo Ersek
  0 siblings, 1 reply; 6+ messages in thread
From: Dong, Eric @ 2020-09-02  0:43 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Laszlo Ersek

AP needs to run from real mode to 32bit mode to LONG mode. Page table
(pointed by CR3) and GDT are necessary to set up to correct value when
CPU execution mode is switched to LONG mode.
AP uses the same location page table (CR3) and GDT as what BSP uses.
But when the page table or GDT is above 4GB, it's impossible for CPU
to use because GDTR.base and CR3 are 32bits before switching to LONG
mode.
This patch adds check for the CR3, GDT.Base and IDT.Base to not above
32 bits restriction.

Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c |   8 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.c    | 108 +++++++++++++++++++-----
 UefiCpuPkg/Library/MpInitLib/MpLib.h    |   6 +-
 3 files changed, 97 insertions(+), 25 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 2c00d72dde..27f12a75a8 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -393,13 +393,19 @@ MpInitChangeApLoopCallback (
   )
 {
   CPU_MP_DATA               *CpuMpData;
+  EFI_STATUS                Status;
 
   CpuMpData = GetCpuMpData ();
   CpuMpData->PmCodeSegment = GetProtectedModeCS ();
   CpuMpData->Pm16CodeSegment = GetProtectedMode16CS ();
   CpuMpData->ApLoopMode = PcdGet8 (PcdCpuApLoopMode);
   mNumberToFinish = CpuMpData->CpuCount - 1;
-  WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL, TRUE);
+  Status = WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL, TRUE);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "ERROR :: %a() Change Ap Loop Mode failed!\n", __FUNCTION__));
+    return;
+  }
+
   while (mNumberToFinish > 0) {
     CpuPause ();
   }
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 07426274f6..21b17a7b40 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -470,7 +470,7 @@ GetProcessorNumber (
 
   @return  CPU count detected
 **/
-UINTN
+EFI_STATUS
 CollectProcessorCount (
   IN CPU_MP_DATA         *CpuMpData
   )
@@ -478,12 +478,17 @@ CollectProcessorCount (
   UINTN                  Index;
   CPU_INFO_IN_HOB        *CpuInfoInHob;
   BOOLEAN                X2Apic;
+  EFI_STATUS             Status;
 
   //
   // Send 1st broadcast IPI to APs to wakeup APs
   //
   CpuMpData->InitFlag = ApInitConfig;
-  WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL, TRUE);
+  Status = WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL, TRUE);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
   CpuMpData->InitFlag = ApInitDone;
   ASSERT (CpuMpData->CpuCount <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
   //
@@ -520,7 +525,11 @@ CollectProcessorCount (
     //
     // Wakeup all APs to enable x2APIC mode
     //
-    WakeUpAP (CpuMpData, TRUE, 0, ApFuncEnableX2Apic, NULL, TRUE);
+    Status = WakeUpAP (CpuMpData, TRUE, 0, ApFuncEnableX2Apic, NULL, TRUE);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+
     //
     // Wait for all known APs finished
     //
@@ -546,7 +555,7 @@ CollectProcessorCount (
 
   DEBUG ((DEBUG_INFO, "MpInitLib: Find %d processors in system.\n", CpuMpData->CpuCount));
 
-  return CpuMpData->CpuCount;
+  return EFI_SUCCESS;
 }
 
 /**
@@ -990,7 +999,7 @@ WaitApWakeup (
   @param[in] CpuMpData          Pointer to CPU MP Data
 
 **/
-VOID
+EFI_STATUS
 FillExchangeInfoData (
   IN CPU_MP_DATA               *CpuMpData
   )
@@ -1001,6 +1010,35 @@ FillExchangeInfoData (
   IA32_CR4                         Cr4;
 
   ExchangeInfo                  = CpuMpData->MpCpuExchangeInfo;
+  ExchangeInfo->Cr3             = AsmReadCr3 ();
+  if (ExchangeInfo->Cr3 > 0xFFFFFFFF) {
+    //
+    // AP needs to run from real mode to 32bit mode to LONG mode. Page table
+    // (pointed by CR3) and GDT are necessary to set up to correct value when
+    // CPU execution mode is switched to LONG mode.
+    // AP uses the same location page table (CR3) and GDT as what BSP uses.
+    // But when the page table or GDT is above 4GB, it's impossible for CPU
+    // to use because GDTR.base and CR3 are 32bits before switching to LONG
+    // mode.
+    // Here add check for the CR3, GDT.Base and IDT.Base to not above 32 bits
+    // limitation.
+    //
+    return EFI_UNSUPPORTED;
+  }
+
+  //
+  // Get the BSP's data of GDT and IDT
+  //
+  AsmReadGdtr ((IA32_DESCRIPTOR *) &ExchangeInfo->GdtrProfile);
+  if (ExchangeInfo->GdtrProfile.Base > 0xFFFFFFFF) {
+    return EFI_UNSUPPORTED;
+  }
+
+  AsmReadIdtr ((IA32_DESCRIPTOR *) &ExchangeInfo->IdtrProfile);
+  if (ExchangeInfo->IdtrProfile.Base > 0xFFFFFFFF) {
+    return EFI_UNSUPPORTED;
+  }
+
   ExchangeInfo->Lock            = 0;
   ExchangeInfo->StackStart      = CpuMpData->Buffer;
   ExchangeInfo->StackSize       = CpuMpData->CpuApStackSize;
@@ -1009,9 +1047,6 @@ FillExchangeInfoData (
 
   ExchangeInfo->CodeSegment     = AsmReadCs ();
   ExchangeInfo->DataSegment     = AsmReadDs ();
-
-  ExchangeInfo->Cr3             = AsmReadCr3 ();
-
   ExchangeInfo->CFunction       = (UINTN) ApWakeupFunction;
   ExchangeInfo->ApIndex         = 0;
   ExchangeInfo->NumApsExecuting = 0;
@@ -1037,13 +1072,6 @@ FillExchangeInfoData (
 
   ExchangeInfo->SevEsIsEnabled  = CpuMpData->SevEsIsEnabled;
   ExchangeInfo->GhcbBase        = (UINTN) CpuMpData->GhcbBase;
-
-  //
-  // Get the BSP's data of GDT and IDT
-  //
-  AsmReadGdtr ((IA32_DESCRIPTOR *) &ExchangeInfo->GdtrProfile);
-  AsmReadIdtr ((IA32_DESCRIPTOR *) &ExchangeInfo->IdtrProfile);
-
   //
   // Find a 32-bit code segment
   //
@@ -1084,6 +1112,8 @@ FillExchangeInfoData (
                          (UINT32)ExchangeInfo->ModeOffset -
                          (UINT32)CpuMpData->AddressMap.ModeTransitionOffset;
   ExchangeInfo->ModeHighSegment = (UINT16)ExchangeInfo->CodeSegment;
+
+  return EFI_SUCCESS;
 }
 
 /**
@@ -1308,8 +1338,12 @@ SetSevEsJumpTable (
   @param[in] Procedure          The function to be invoked by AP
   @param[in] ProcedureArgument  The argument to be passed into AP function
   @param[in] WakeUpDisabledAps  Whether need to wake up disabled APs in broadcast mode.
+
+  @retval EFI_SUCCESS           Wake up the AP success.
+  @retval EFI_UNSUPPORTED       Invalid CR3, IDT, GDT value caused fail to wake up AP.
+
 **/
-VOID
+EFI_STATUS
 WakeUpAP (
   IN CPU_MP_DATA               *CpuMpData,
   IN BOOLEAN                   Broadcast,
@@ -1324,6 +1358,7 @@ WakeUpAP (
   CPU_AP_DATA                      *CpuData;
   BOOLEAN                          ResetVectorRequired;
   CPU_INFO_IN_HOB                  *CpuInfoInHob;
+  EFI_STATUS                       Status;
 
   CpuMpData->FinishedCount = 0;
   ResetVectorRequired = FALSE;
@@ -1333,7 +1368,10 @@ WakeUpAP (
     ResetVectorRequired = TRUE;
     AllocateResetVector (CpuMpData);
     AllocateSevEsAPMemory (CpuMpData);
-    FillExchangeInfoData (CpuMpData);
+    Status = FillExchangeInfoData (CpuMpData);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
     SaveLocalApicTimerSetting (CpuMpData);
   }
 
@@ -1500,6 +1538,8 @@ WakeUpAP (
   // S3SmmInitDone Ppi.
   //
   CpuMpData->WakeUpByInitSipiSipi = (CpuMpData->ApLoopMode == ApInHltLoop);
+
+  return EFI_SUCCESS;
 }
 
 /**
@@ -1945,6 +1985,7 @@ MpInitLibInitialize (
   UINTN                    ApResetVectorSize;
   UINTN                    BackupBufferAddr;
   UINTN                    ApIdtBase;
+  EFI_STATUS               Status;
 
   OldCpuMpData = GetCpuMpDataFromGuidedHob ();
   if (OldCpuMpData == NULL) {
@@ -2067,7 +2108,10 @@ MpInitLibInitialize (
       //
       // Wakeup all APs and calculate the processor count in system
       //
-      CollectProcessorCount (CpuMpData);
+      Status = CollectProcessorCount (CpuMpData);
+      if (EFI_ERROR (Status)) {
+        return Status;
+      }
     }
   } else {
     //
@@ -2118,7 +2162,11 @@ MpInitLibInitialize (
       //
       CpuMpData->InitFlag = ApInitReconfig;
     }
-    WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
+    Status = WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+
     //
     // Wait for all APs finished initialization
     //
@@ -2262,6 +2310,7 @@ SwitchBSPWorker (
   MSR_IA32_APIC_BASE_REGISTER  ApicBaseMsr;
   BOOLEAN                      OldInterruptState;
   BOOLEAN                      OldTimerInterruptState;
+  EFI_STATUS                   Status;
 
   //
   // Save and Disable Local APIC timer interrupt
@@ -2333,7 +2382,10 @@ SwitchBSPWorker (
   //
   // Need to wakeUp AP (future BSP).
   //
-  WakeUpAP (CpuMpData, FALSE, ProcessorNumber, FutureBSPProc, CpuMpData, TRUE);
+  Status = WakeUpAP (CpuMpData, FALSE, ProcessorNumber, FutureBSPProc, CpuMpData, TRUE);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
 
   AsmExchangeRole (&CpuMpData->BSPInfo, &CpuMpData->APInfo);
 
@@ -2669,14 +2721,21 @@ StartupAllCPUsWorker (
   CpuMpData->WaitEvent     = WaitEvent;
 
   if (!SingleThread) {
-    WakeUpAP (CpuMpData, TRUE, 0, Procedure, ProcedureArgument, FALSE);
+    Status = WakeUpAP (CpuMpData, TRUE, 0, Procedure, ProcedureArgument, FALSE);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
   } else {
     for (ProcessorNumber = 0; ProcessorNumber < ProcessorCount; ProcessorNumber++) {
       if (ProcessorNumber == CallerNumber) {
         continue;
       }
       if (CpuMpData->CpuData[ProcessorNumber].Waiting) {
-        WakeUpAP (CpuMpData, FALSE, ProcessorNumber, Procedure, ProcedureArgument, TRUE);
+        Status = WakeUpAP (CpuMpData, FALSE, ProcessorNumber, Procedure, ProcedureArgument, TRUE);
+        if (EFI_ERROR (Status)) {
+          return Status;
+        }
+
         break;
       }
     }
@@ -2795,7 +2854,10 @@ StartupThisAPWorker (
   CpuData->ExpectedTime = CalculateTimeout (TimeoutInMicroseconds, &CpuData->CurrentTime);
   CpuData->TotalTime    = 0;
 
-  WakeUpAP (CpuMpData, FALSE, ProcessorNumber, Procedure, ProcedureArgument, TRUE);
+  Status = WakeUpAP (CpuMpData, FALSE, ProcessorNumber, Procedure, ProcedureArgument, TRUE);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
 
   //
   // If WaitEvent is NULL, execute in blocking mode.
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 02652eaae1..9cf5c0f9b4 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -459,8 +459,12 @@ GetSevEsAPMemory (
   @param[in] Procedure          The function to be invoked by AP
   @param[in] ProcedureArgument  The argument to be passed into AP function
   @param[in] WakeUpDisabledAps  Whether need to wake up disabled APs in broadcast mode.
+
+  @retval EFI_SUCCESS           Wake up the AP success.
+  @retval EFI_UNSUPPORTED       Invalid CR3, IDT, GDT value caused fail to wake up AP.
+
 **/
-VOID
+EFI_STATUS
 WakeUpAP (
   IN CPU_MP_DATA               *CpuMpData,
   IN BOOLEAN                   Broadcast,
-- 
2.23.0.windows.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-09-03 16:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-02  0:43 [PATCH] UefiCpuPkg/MpInitLib: Add check for CR3/GDT/IDT Dong, Eric
2020-09-02  7:42 ` [edk2-devel] " Laszlo Ersek
2020-09-03  1:47   ` Dong, Eric
2020-09-03  7:59     ` Laszlo Ersek
2020-09-03 15:21       ` Dong, Eric
2020-09-03 16:26         ` Laszlo Ersek

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