[PATCH v3] UefiCpuPkg/MpInitLib: Add check for Cr3/GDT/IDT.

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

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 and IDT to not above 4G limitation.

The check is avoided -- assumed successful -- if the new
PcdEnableCpuApCr3GdtIdtCheck is FALSE (which is the default). The reason
is that the 32-bit requirement is always ensured by edk2 itself; the
requirement is only possibly invalidated by a particular UEFI shell
application that manually moves the GDT/IDT/CR3 above 4GB. Platforms
that don't intend to be compatible with such UEFI applications need not
set the PCD to TRUE.

If the PCD is TRUE and the check fails, then the StartupAllAPs(),
StartupThisAP(), SwitchBSP() and EnableDisableAP() MP service APIs are
rejected at once. Reporting an error immediately is more graceful than
hanging when the APs attempt to switch to long mode.

Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
V3:
only add check for the DxeMplib, no need for PeiMpLib.

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       | 97 +++++++++++++++++++
 UefiCpuPkg/UefiCpuPkg.dec                     |  4 +
 UefiCpuPkg/UefiCpuPkg.uni                     |  2 +
 4 files changed, 104 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 1771575c69..7792df516e 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -74,5 +74,6 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStatusCheckIntervalInMicroSeconds  ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled                          ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase                       ## SOMETIMES_CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdEnableCpuApCr3GdtIdtCheck               ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                      ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                           ## CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 2c00d72dde..332b4447bb 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -29,6 +29,66 @@ VOID             *mReservedApLoopFunc = NULL;
 UINTN            mReservedTopOfApStack;
 volatile UINT32  mNumberToFinish = 0;
 
+/**
+  Check whether Cr3/GDT/IDT value valid for the APs.
+
+  @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 (&Gdtr);
+  //
+  // Here code needs to check both Gdtr.Base and Gdtr.Base + Gdtr.Limit
+  // below BASE_4GB, but Gdtr.Base + Gdtr.Limit below BASE_4GB also means
+  // Gdtr.Base below BASE_4GB. so here just add Gdtr.Base + Gdtr.Limit
+  // check.
+  //
+  if (Gdtr.Base + Gdtr.Limit >= BASE_4GB) {
+    return FALSE;
+  }
+
+  AsmReadIdtr (&Idtr);
+  //
+  // Here code needs to check both Idtr.Base and Idtr.Base + Idtr.Limit
+  // below BASE_4GB, but Idtr.Base + Idtr.Limit below BASE_4GB also means
+  // Idtr.Base below BASE_4GB. so here just add Idtr.Base + Idtr.Limit
+  // check.
+  //
+  if (Idtr.Base + Idtr.Limit >= BASE_4GB) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
 /**
   Enable Debug Agent to support source debugging on AP function.
 
@@ -394,6 +454,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 ();
@@ -676,6 +745,13 @@ MpInitLibStartupAllAPs (
 {
   EFI_STATUS              Status;
 
+  //
+  // Check whether Cr3/GDT/IDT valid for AP.
+  //
+  if (!ValidCr3GdtIdtCheck()) {
+    return EFI_UNSUPPORTED;
+  }
+
   //
   // Temporarily stop checkAllApsStatus for avoid resource dead-lock.
   //
@@ -783,6 +859,13 @@ MpInitLibStartupThisAP (
 {
   EFI_STATUS              Status;
 
+  //
+  // Check whether Cr3/GDT/IDT valid for AP.
+  //
+  if (!ValidCr3GdtIdtCheck()) {
+    return EFI_UNSUPPORTED;
+  }
+
   //
   // temporarily stop checkAllApsStatus for avoid resource dead-lock.
   //
@@ -839,6 +922,13 @@ MpInitLibSwitchBSP (
   EFI_TIMER_ARCH_PROTOCOL      *Timer;
   UINT64                       TimerPeriod;
 
+  //
+  // Check whether Cr3/GDT/IDT valid for AP.
+  //
+  if (!ValidCr3GdtIdtCheck()) {
+    return EFI_UNSUPPORTED;
+  }
+
   TimerPeriod = 0;
   //
   // Locate Timer Arch Protocol
@@ -912,6 +1002,13 @@ MpInitLibEnableDisableAP (
   EFI_STATUS     Status;
   BOOLEAN        TempStopCheckState;
 
+  //
+  // Check whether Cr3/GDT/IDT valid for AP.
+  //
+  if (!ValidCr3GdtIdtCheck()) {
+    return EFI_UNSUPPORTED;
+  }
+
   TempStopCheckState = FALSE;
   //
   // temporarily stop checkAllAPsStatus for initialize parameters.
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index d83c084467..08ec36e76c 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -148,6 +148,10 @@
   # @Prompt Lock SMM Feature Control MSR.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock|TRUE|BOOLEAN|0x3213210B
 
+  ## This value specifies whether need to check the Cr3/GDT/IDT value for the APs.
+  # @Prompt Enable Cr3/GDT/IDT value check for the APs.
+  gUefiCpuPkgTokenSpaceGuid.PcdEnableCpuApCr3GdtIdtCheck|FALSE|BOOLEAN|0x30000044
+
 [PcdsFixedAtBuild]
   ## List of exception vectors which need switching stack.
   #  This PCD will only take into effect if PcdCpuStackGuard is enabled.
diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
index 219c1963bf..3461a83003 100644
--- a/UefiCpuPkg/UefiCpuPkg.uni
+++ b/UefiCpuPkg/UefiCpuPkg.uni
@@ -96,6 +96,8 @@
 
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmFeatureControlMsrLock_PROMPT  #language en-US "Lock SMM Feature Control MSR"
 
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdEnableCpuApCr3GdtIdtCheck_PROMPT  #language en-US "Enable Cr3/GDT/IDT value check for the APs"
+
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmFeatureControlMsrLock_HELP  #language en-US "Lock SMM Feature Control MSR?<BR><BR>\n"
                                                                                            "TRUE  - locked.<BR>\n"
                                                                                            "FALSE - unlocked.<BR>"
-- 
2.23.0.windows.1