[PATCH v1] UefiCpuPkg: Dynamic check SMRR enable & SmmFeatureControl capability

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

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

Two SMM variables (mSmrrSupported & mSmmFeatureControlSupported) are global
variables, they control whether the SMRR and SMM Feature Control MSR will
be restored respectively.
To avoid the TOCTOU, dynamic check SMRR enable & SmmFeatureControl capability.

Change-Id: I6835e4b0e12c5e6f52effb60fd9224e3eb97fc0d
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c    | 248 ++++++++++++---------
 1 file changed, 141 insertions(+), 107 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
index 78de7f8407..b2f31c993f 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
@@ -35,26 +35,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 // MSRs required for configuration of SMM Code Access Check
 //
 #define SMM_FEATURES_LIB_IA32_MCA_CAP  0x17D
 #define   SMM_CODE_ACCESS_CHK_BIT      BIT58
 
-//
-// Set default value to assume SMRR is not supported
-//
-BOOLEAN  mSmrrSupported = FALSE;
-
-//
-// Set default value to assume MSR_SMM_FEATURE_CONTROL is not supported
-//
-BOOLEAN  mSmmFeatureControlSupported = FALSE;
-
-//
-// Set default value to assume IA-32 Architectural MSRs are used
-//
-UINT32  mSmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE;
-UINT32  mSmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK;
-
 //
 // Set default value to assume MTRRs need to be configured on each SMI
 //
 BOOLEAN  mNeedConfigureMtrrs = TRUE;
 
@@ -62,26 +46,39 @@ BOOLEAN  mNeedConfigureMtrrs = TRUE;
 // Array for state of SMRR enable on all CPUs
 //
 BOOLEAN  *mSmrrEnabled;
 
 /**
-  Performs library initialization.
+  Return if SMRR is supported
 
-  This initialization function contains common functionality shared betwen all
-  library instance constructors.
+  @param[in] SmrrPhysBaseMsr   Pointer to SmrrPhysBaseMsr.
+  @param[in] SmrrPhysMaskMsr   Pointer to SmrrPhysMaskMsr.
+
+  @retval TRUE  SMRR is supported.
+  @retval FALSE SMRR is not supported.
 
 **/
-VOID
-CpuFeaturesLibInitialization (
-  VOID
+BOOLEAN
+IsSmrrSupported (
+  IN UINT32  *SmrrPhysBaseMsr    OPTIONAL,
+  IN UINT32  *SmrrPhysMaskMsr    OPTIONAL
   )
 {
+  BOOLEAN  ReturnValue;
+
   UINT32  RegEax;
   UINT32  RegEdx;
   UINTN   FamilyId;
   UINTN   ModelId;
 
+  UINT64  FeatureControl;
+
+  //
+  // Set default value to assume SMRR is not supported
+  //
+  ReturnValue = FALSE;
+
   //
   // Retrieve CPU Family and Model
   //
   AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
   FamilyId = (RegEax >> 8) & 0xf;
@@ -96,11 +93,11 @@ CpuFeaturesLibInitialization (
   if ((RegEdx & BIT12) != 0) {
     //
     // Check MTRR_CAP MSR bit 11 for SMRR support
     //
     if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MTRR_CAP) & BIT11) != 0) {
-      mSmrrSupported = TRUE;
+      ReturnValue = TRUE;
     }
   }
 
   //
   // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
@@ -109,28 +106,79 @@ CpuFeaturesLibInitialization (
   // If CPU Family/Model is 06_1CH, 06_26H, 06_27H, 06_35H or 06_36H, then
   // SMRR Physical Base and SMM Physical Mask MSRs are not available.
   //
   if (FamilyId == 0x06) {
     if ((ModelId == 0x1C) || (ModelId == 0x26) || (ModelId == 0x27) || (ModelId == 0x35) || (ModelId == 0x36)) {
-      mSmrrSupported = FALSE;
+      ReturnValue = FALSE;
     }
   }
 
-  //
-  // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
-  // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
-  //
-  // If CPU Family/Model is 06_0F or 06_17, then use Intel(R) Core(TM) 2
-  // Processor Family MSRs
-  //
-  if (FamilyId == 0x06) {
-    if ((ModelId == 0x17) || (ModelId == 0x0f)) {
-      mSmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE;
-      mSmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK;
+  if (ReturnValue) {
+    //
+    // Return the SmrrPhysBaseMsr & SmrrPhysMaskMsr if required & Smrr Supported
+    //
+    if (SmrrPhysBaseMsr != NULL) {
+      *SmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE;
+    }
+
+    if (SmrrPhysBaseMsr != NULL) {
+      *SmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK;
+    }
+
+    //
+    // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+    // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
+    //
+    // If CPU Family/Model is 06_0F or 06_17, then use Intel(R) Core(TM) 2
+    // Processor Family MSRs
+    //
+    if (FamilyId == 0x06) {
+      if ((ModelId == 0x17) || (ModelId == 0x0f)) {
+        if (SmrrPhysBaseMsr != NULL) {
+          *SmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE;
+        }
+
+        if (SmrrPhysMaskMsr != NULL) {
+          *SmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK;
+        }
+
+        //
+        // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+        // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
+        //
+        // If Intel(R) Core(TM) Core(TM) 2 Processor Family MSRs are being used, then
+        // make sure SMRR Enable(BIT3) of MSR_FEATURE_CONTROL MSR(0x3A) is set before
+        // accessing SMRR base/mask MSRs.  If Lock(BIT0) of MSR_FEATURE_CONTROL MSR(0x3A)
+        // is set, then the MSR is locked and can not be modified.
+        //
+
+        FeatureControl = AsmReadMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL);
+        if (((FeatureControl & BIT3) == 0) && ((FeatureControl & BIT0) == 1)) {
+          ReturnValue = FALSE;
+        }
+      }
     }
   }
 
+  return ReturnValue;
+}
+
+/**
+  Performs library initialization.
+
+  This initialization function contains common functionality shared betwen all
+  library instance constructors.
+
+**/
+VOID
+CpuFeaturesLibInitialization (
+  VOID
+  )
+{
+  UINT32  RegEax;
+  UINT32  RegEdx;
+
   //
   // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
   // Volume 3C, Section 34.4.2 SMRAM Caching
   //   An IA-32 processor does not automatically write back and invalidate its
   //   caches before entering SMM or before exiting SMM. Because of this behavior,
@@ -193,50 +241,27 @@ SmmCpuFeaturesInitializeProcessor (
   IN EFI_PROCESSOR_INFORMATION  *ProcessorInfo,
   IN CPU_HOT_PLUG_DATA          *CpuHotPlugData
   )
 {
   SMRAM_SAVE_STATE_MAP  *CpuState;
-  UINT64                FeatureControl;
-  UINT32                RegEax;
-  UINT32                RegEdx;
-  UINTN                 FamilyId;
-  UINTN                 ModelId;
+  UINT32                SmrrPhysBaseMsr;
+  UINT32                SmrrPhysMaskMsr;
 
   //
   // Configure SMBASE.
   //
   CpuState             = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
   CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
 
-  //
-  // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
-  // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
-  //
-  // If Intel(R) Core(TM) Core(TM) 2 Processor Family MSRs are being used, then
-  // make sure SMRR Enable(BIT3) of MSR_FEATURE_CONTROL MSR(0x3A) is set before
-  // accessing SMRR base/mask MSRs.  If Lock(BIT0) of MSR_FEATURE_CONTROL MSR(0x3A)
-  // is set, then the MSR is locked and can not be modified.
-  //
-  if (mSmrrSupported && (mSmrrPhysBaseMsr == SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE)) {
-    FeatureControl = AsmReadMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL);
-    if ((FeatureControl & BIT3) == 0) {
-      if ((FeatureControl & BIT0) == 0) {
-        AsmWriteMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL, FeatureControl | BIT3);
-      } else {
-        mSmrrSupported = FALSE;
-      }
-    }
-  }
-
   //
   // If SMRR is supported, then program SMRR base/mask MSRs.
   // The EFI_MSR_SMRR_PHYS_MASK_VALID bit is not set until the first normal SMI.
   // The code that initializes SMM environment is running in normal mode
   // from SMRAM region.  If SMRR is enabled here, then the SMRAM region
   // is protected and the normal mode code execution will fail.
   //
-  if (mSmrrSupported) {
+  if (IsSmrrSupported (&SmrrPhysBaseMsr, &SmrrPhysMaskMsr)) {
     //
     // SMRR size cannot be less than 4-KBytes
     // SMRR size must be of length 2^n
     // SMRR base alignment cannot be less than SMRR length
     //
@@ -250,50 +275,16 @@ SmmCpuFeaturesInitializeProcessor (
       if (IsMonarch) {
         DEBUG ((DEBUG_ERROR, "SMM Base/Size does not meet alignment/size requirement!\n"));
         CpuDeadLoop ();
       }
     } else {
-      AsmWriteMsr64 (mSmrrPhysBaseMsr, CpuHotPlugData->SmrrBase | MTRR_CACHE_WRITE_BACK);
-      AsmWriteMsr64 (mSmrrPhysMaskMsr, (~(CpuHotPlugData->SmrrSize - 1) & EFI_MSR_SMRR_MASK));
+      AsmWriteMsr64 (SmrrPhysBaseMsr, CpuHotPlugData->SmrrBase | MTRR_CACHE_WRITE_BACK);
+      AsmWriteMsr64 (SmrrPhysMaskMsr, (~(CpuHotPlugData->SmrrSize - 1) & EFI_MSR_SMRR_MASK));
       mSmrrEnabled[CpuIndex] = FALSE;
     }
   }
 
-  //
-  // Retrieve CPU Family and Model
-  //
-  AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
-  FamilyId = (RegEax >> 8) & 0xf;
-  ModelId  = (RegEax >> 4) & 0xf;
-  if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
-    ModelId = ModelId | ((RegEax >> 12) & 0xf0);
-  }
-
-  //
-  // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
-  // Volume 3C, Section 35.10.1 MSRs in 4th Generation Intel(R) Core(TM)
-  // Processor Family.
-  //
-  // If CPU Family/Model is 06_3C, 06_45, or 06_46 then use 4th Generation
-  // Intel(R) Core(TM) Processor Family MSRs.
-  //
-  if (FamilyId == 0x06) {
-    if ((ModelId == 0x3C) || (ModelId == 0x45) || (ModelId == 0x46) ||
-        (ModelId == 0x3D) || (ModelId == 0x47) || (ModelId == 0x4E) || (ModelId == 0x4F) ||
-        (ModelId == 0x3F) || (ModelId == 0x56) || (ModelId == 0x57) || (ModelId == 0x5C) ||
-        (ModelId == 0x8C))
-    {
-      //
-      // Check to see if the CPU supports the SMM Code Access Check feature
-      // Do not access this MSR unless the CPU supports the SmmRegFeatureControl
-      //
-      if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MCA_CAP) & SMM_CODE_ACCESS_CHK_BIT) != 0) {
-        mSmmFeatureControlSupported = TRUE;
-      }
-    }
-  }
-
   //
   //  Call internal worker function that completes the CPU initialization
   //
   FinishSmmCpuFeaturesInitializeProcessor ();
 }
@@ -381,12 +372,14 @@ VOID
 EFIAPI
 SmmCpuFeaturesDisableSmrr (
   VOID
   )
 {
-  if (mSmrrSupported && mNeedConfigureMtrrs) {
-    AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) & ~EFI_MSR_SMRR_PHYS_MASK_VALID);
+  UINT32  SmrrPhysMaskMsr;
+
+  if (IsSmrrSupported (NULL, &SmrrPhysMaskMsr) && mNeedConfigureMtrrs) {
+    AsmWriteMsr64 (SmrrPhysMaskMsr, AsmReadMsr64 (SmrrPhysMaskMsr) & ~EFI_MSR_SMRR_PHYS_MASK_VALID);
   }
 }
 
 /**
   Enable SMRR register if SMRR is supported and SmmCpuFeaturesNeedConfigureMtrrs()
@@ -396,12 +389,14 @@ VOID
 EFIAPI
 SmmCpuFeaturesReenableSmrr (
   VOID
   )
 {
-  if (mSmrrSupported && mNeedConfigureMtrrs) {
-    AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
+  UINT32  SmrrPhysMaskMsr;
+
+  if (IsSmrrSupported (NULL, &SmrrPhysMaskMsr) && mNeedConfigureMtrrs) {
+    AsmWriteMsr64 (SmrrPhysMaskMsr, AsmReadMsr64 (SmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
   }
 }
 
 /**
   Processor specific hook point each time a CPU enters System Management Mode.
@@ -414,15 +409,17 @@ VOID
 EFIAPI
 SmmCpuFeaturesRendezvousEntry (
   IN UINTN  CpuIndex
   )
 {
+  UINT32  SmrrPhysMaskMsr;
+
   //
   // If SMRR is supported and this is the first normal SMI, then enable SMRR
   //
-  if (mSmrrSupported && !mSmrrEnabled[CpuIndex]) {
-    AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
+  if (!mSmrrEnabled[CpuIndex] && IsSmrrSupported (NULL, &SmrrPhysMaskMsr)) {
+    AsmWriteMsr64 (SmrrPhysMaskMsr, AsmReadMsr64 (SmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
     mSmrrEnabled[CpuIndex] = TRUE;
   }
 }
 
 /**
@@ -458,12 +455,49 @@ EFIAPI
 SmmCpuFeaturesIsSmmRegisterSupported (
   IN UINTN         CpuIndex,
   IN SMM_REG_NAME  RegName
   )
 {
-  if (mSmmFeatureControlSupported && (RegName == SmmRegFeatureControl)) {
-    return TRUE;
+  UINT32  RegEax;
+  UINT32  RegEdx;
+  UINTN   FamilyId;
+  UINTN   ModelId;
+
+  if (RegName == SmmRegFeatureControl) {
+    //
+    // Retrieve CPU Family and Model
+    //
+    AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
+    FamilyId = (RegEax >> 8) & 0xf;
+    ModelId  = (RegEax >> 4) & 0xf;
+    if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
+      ModelId = ModelId | ((RegEax >> 12) & 0xf0);
+    }
+
+    //
+    // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+    // Volume 3C, Section 35.10.1 MSRs in 4th Generation Intel(R) Core(TM)
+    // Processor Family.
+    //
+    // If CPU Family/Model is 06_3C, 06_45, or 06_46 then use 4th Generation
+    // Intel(R) Core(TM) Processor Family MSRs.
+    //
+    if (FamilyId == 0x06) {
+      if ((ModelId == 0x3C) || (ModelId == 0x45) || (ModelId == 0x46) ||
+          (ModelId == 0x3D) || (ModelId == 0x47) || (ModelId == 0x4E) || (ModelId == 0x4F) ||
+          (ModelId == 0x3F) || (ModelId == 0x56) || (ModelId == 0x57) || (ModelId == 0x5C) ||
+          (ModelId == 0x8C))
+      {
+        //
+        // Check to see if the CPU supports the SMM Code Access Check feature
+        // Do not access this MSR unless the CPU supports the SmmRegFeatureControl
+        //
+        if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MCA_CAP) & SMM_CODE_ACCESS_CHK_BIT) != 0) {
+          return TRUE;
+        }
+      }
+    }
   }
 
   return FALSE;
 }
 
@@ -484,11 +518,11 @@ EFIAPI
 SmmCpuFeaturesGetSmmRegister (
   IN UINTN         CpuIndex,
   IN SMM_REG_NAME  RegName
   )
 {
-  if (mSmmFeatureControlSupported && (RegName == SmmRegFeatureControl)) {
+  if (SmmCpuFeaturesIsSmmRegisterSupported (CpuIndex, RegName)) {
     return AsmReadMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL);
   }
 
   return 0;
 }
@@ -510,11 +544,11 @@ SmmCpuFeaturesSetSmmRegister (
   IN UINTN         CpuIndex,
   IN SMM_REG_NAME  RegName,
   IN UINT64        Value
   )
 {
-  if (mSmmFeatureControlSupported && (RegName == SmmRegFeatureControl)) {
+  if (SmmCpuFeaturesIsSmmRegisterSupported (CpuIndex, RegName)) {
     AsmWriteMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL, Value);
   }
 }
 
 /**
-- 
2.16.2.windows.1