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

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2954

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.

Change-Id: I231180f45d9f542641082c57d001e38e3f6759d5
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---

V2:
Change the check point. Just in the different caller to make the logic
clear. V1 patch add check just before the use of the code. It make the
logic complicated.

 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  1 +
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  9 +++
 UefiCpuPkg/Library/MpInitLib/MpLib.c          | 76 +++++++++++++++++++
 UefiCpuPkg/Library/MpInitLib/MpLib.h          | 12 +++
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |  1 +
 UefiCpuPkg/UefiCpuPkg.dec                     |  4 +
 6 files changed, 103 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 1771575c69..20851f251a 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -76,3 +76,4 @@
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase                       ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                      ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                           ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdEnableCpuApCr3GdtIdtCheck               ## CONSUMES
\ No newline at end of file
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 2c00d72dde..f598372c4d 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -394,6 +394,15 @@ MpInitChangeApLoopCallback (
 {
   CPU_MP_DATA               *CpuMpData;
 
+  //
+  // Check the CR3/GDT/IDT before waking up AP.
+  // If the check return fail, it will block later
+  // OS boot, so halt the system here.
+  //
+  if (!ValidCR3GdtIdtCheck()) {
+    CpuDeadLoop ();
+  }
+
   CpuMpData = GetCpuMpData ();
   CpuMpData->PmCodeSegment = GetProtectedModeCS ();
   CpuMpData->Pm16CodeSegment = GetProtectedMode16CS ();
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 07426274f6..69a0372df7 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1909,6 +1909,54 @@ CheckAllAPs (
   return EFI_NOT_READY;
 }
 
+/**
+  Check whether CR3/GDT/IDT value valid for AP.
+
+  @retval  TRUE          Pass the check.
+  @retval  FALSE         Fail the check.
+
+**/
+BOOLEAN
+ValidCR3GdtIdtCheck (
+  VOID
+  )
+{
+  IA32_DESCRIPTOR   Gdtr;
+  IA32_DESCRIPTOR   Idtr;
+
+  if (!PcdGetBool (PcdEnableCpuApCr3GdtIdtCheck)) {
+    return TRUE;
+  }
+
+  //
+  // 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. IDT also necessary if the
+  // exception happened.
+  // AP uses the same location page table (CR3) and GDT/IDT 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 range, IDT.Base and range are
+  // not above 32 bits limitation.
+  //
+  if (AsmReadCr3 () >= BASE_4GB) {
+    return FALSE;
+  }
+
+  AsmReadGdtr ((IA32_DESCRIPTOR *) &Gdtr);
+  if ((Gdtr.Base >= BASE_4GB) || (Gdtr.Base + Gdtr.Limit >= BASE_4GB)) {
+    return FALSE;
+  }
+
+  AsmReadIdtr ((IA32_DESCRIPTOR *) &Idtr);
+  if ((Idtr.Base >= BASE_4GB) || (Idtr.Base + Idtr.Limit >= BASE_4GB)) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
 /**
   MP Initialize Library initialization.
 
@@ -2318,6 +2366,13 @@ SwitchBSPWorker (
     return EFI_NOT_READY;
   }
 
+  //
+  // Check whether CR3/GDT/IDT valid for AP.
+  //
+  if (!ValidCR3GdtIdtCheck()) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   CpuMpData->BSPInfo.State = CPU_SWITCH_STATE_IDLE;
   CpuMpData->APInfo.State  = CPU_SWITCH_STATE_IDLE;
   CpuMpData->SwitchBspFlag = TRUE;
@@ -2420,6 +2475,13 @@ EnableDisableApWorker (
     return EFI_NOT_FOUND;
   }
 
+  //
+  // Check whether CR3/GDT/IDT valid for AP.
+  //
+  if (!ValidCR3GdtIdtCheck()) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   if (!EnableAP) {
     SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateDisabled);
   } else {
@@ -2607,6 +2669,13 @@ StartupAllCPUsWorker (
     return EFI_DEVICE_ERROR;
   }
 
+  //
+  // Check whether CR3/GDT/IDT valid for AP.
+  //
+  if (!ValidCR3GdtIdtCheck()) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   //
   // Update AP state
   //
@@ -2772,6 +2841,13 @@ StartupThisAPWorker (
     return EFI_INVALID_PARAMETER;
   }
 
+  //
+  // Check whether CR3/GDT/IDT valid for AP.
+  //
+  if (!ValidCR3GdtIdtCheck()) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   //
   // Update AP state
   //
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 02652eaae1..4ac5cb51e3 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -740,5 +740,17 @@ PlatformShadowMicrocode (
   IN OUT CPU_MP_DATA             *CpuMpData
   );
 
+/**
+  Check whether CR3/GDT/IDT value valid for AP.
+
+  @retval  TRUE          Pass the check.
+  @retval  FALSE         Fail the check.
+
+**/
+BOOLEAN
+ValidCR3GdtIdtCheck (
+  VOID
+  );
+
 #endif
 
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 34abf25d43..0ca86fcdaa 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -65,6 +65,7 @@
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled                      ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase                   ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                       ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdEnableCpuApCr3GdtIdtCheck           ## CONSUMES
 
 [Ppis]
   gEdkiiPeiShadowMicrocodePpiGuid        ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index d83c084467..467ec5e001 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -187,6 +187,10 @@
   # @Prompt Configure stack size for Application Processor (AP)
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize|0x8000|UINT32|0x00000003
 
+  ## This value specifies whether need to check the CR3/GDT/IDT value for AP.
+  # @Prompt Whether need to check the CR3/GDT/IDT value for AP
+  gUefiCpuPkgTokenSpaceGuid.PcdEnableCpuApCr3GdtIdtCheck|FALSE|BOOLEAN|0x30000044
+
   ## Specifies stack size in the temporary RAM. 0 means half of TemporaryRamSize.
   # @Prompt Stack size in the temporary RAM.
   gUefiCpuPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize|0|UINT32|0x10001003
-- 
2.23.0.windows.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-04 12:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <163188258B195C29.8972@groups.io>
2020-09-04 11:17 ` 回复: 回复: [edk2-devel] [PATCH v2] UefiCpuPkg/MpInitLib: Add check for CR3/GDT/IDT Fan Jeff
2020-09-04 11:48   ` Yao, Jiewen
2020-09-04 12:08     ` Laszlo Ersek
2020-09-03 15:11 Dong, Eric
2020-09-03 19:00 ` Laszlo Ersek
2020-09-03 19:55   ` Laszlo Ersek
2020-09-04  1:34     ` [edk2-devel] " Ni, Ray
2020-09-04  2:00       ` Dong, Eric
2020-09-04  2:18         ` 回复: " vanjeff_919
2020-09-04  6:58           ` Laszlo Ersek
2020-09-04  7:32             ` 回复: " Fan Jeff
2020-09-04  8:06               ` Yao, Jiewen
2020-09-04  8:43                 ` 回复: " Fan Jeff
2020-09-04  8:23               ` Laszlo Ersek

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