[PATCH v1] UefiCpuPkg: Add PCD to control SMRR enable & SmmFeatureControl support

["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, add PCD to control SMRR & SmmFeatureControl enable.

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/SmmCpuFeaturesLib.inf        |  4 ++
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c    | 84 ++++------------------
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf     |  4 ++
 .../StandaloneMmCpuFeaturesLib.inf                 |  4 ++
 UefiCpuPkg/UefiCpuPkg.dec                          | 12 ++++
 UefiCpuPkg/UefiCpuPkg.uni                          | 12 ++++
 6 files changed, 48 insertions(+), 72 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 35292dac31..7b5cef9700 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -33,5 +33,9 @@
   MemoryAllocationLib
   DebugLib
 
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## SOMETIMES_CONSUMES
+
+[FeaturePcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable  ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable  ## CONSUMES
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
index 78de7f8407..b88cdece2a 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
@@ -35,20 +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;
@@ -81,39 +71,27 @@ CpuFeaturesLibInitialization (
   UINTN   ModelId;
 
   //
   // Retrieve CPU Family and Model
   //
-  AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
+  AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, NULL);
   FamilyId = (RegEax >> 8) & 0xf;
   ModelId  = (RegEax >> 4) & 0xf;
   if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
     ModelId = ModelId | ((RegEax >> 12) & 0xf0);
   }
 
-  //
-  // Check CPUID(CPUID_VERSION_INFO).EDX[12] for MTRR capability
-  //
-  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;
-    }
-  }
-
   //
   // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
   // Volume 3C, Section 35.3 MSRs in the Intel(R) Atom(TM) Processor Family
   //
   // 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;
+      ASSERT (!FeaturePcdGet (PcdSmrrEnable));
     }
   }
 
   //
   // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
@@ -194,14 +172,10 @@ SmmCpuFeaturesInitializeProcessor (
   IN CPU_HOT_PLUG_DATA          *CpuHotPlugData
   )
 {
   SMRAM_SAVE_STATE_MAP  *CpuState;
   UINT64                FeatureControl;
-  UINT32                RegEax;
-  UINT32                RegEdx;
-  UINTN                 FamilyId;
-  UINTN                 ModelId;
 
   //
   // Configure SMBASE.
   //
   CpuState             = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
@@ -214,17 +188,17 @@ SmmCpuFeaturesInitializeProcessor (
   // 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)) {
+  if (mSmrrPhysBaseMsr == SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE) {
     FeatureControl = AsmReadMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL);
     if ((FeatureControl & BIT3) == 0) {
-      if ((FeatureControl & BIT0) == 0) {
+      if (((FeatureControl & BIT0) == 0) && (FeaturePcdGet (PcdSmrrEnable))) {
         AsmWriteMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL, FeatureControl | BIT3);
       } else {
-        mSmrrSupported = FALSE;
+        ASSERT (!FeaturePcdGet (PcdSmrrEnable));
       }
     }
   }
 
   //
@@ -232,11 +206,11 @@ SmmCpuFeaturesInitializeProcessor (
   // 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 (FeaturePcdGet (PcdSmrrEnable)) {
     //
     // 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
     //
@@ -256,44 +230,10 @@ SmmCpuFeaturesInitializeProcessor (
       AsmWriteMsr64 (mSmrrPhysMaskMsr, (~(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,11 +321,11 @@ VOID
 EFIAPI
 SmmCpuFeaturesDisableSmrr (
   VOID
   )
 {
-  if (mSmrrSupported && mNeedConfigureMtrrs) {
+  if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) {
     AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) & ~EFI_MSR_SMRR_PHYS_MASK_VALID);
   }
 }
 
 /**
@@ -396,11 +336,11 @@ VOID
 EFIAPI
 SmmCpuFeaturesReenableSmrr (
   VOID
   )
 {
-  if (mSmrrSupported && mNeedConfigureMtrrs) {
+  if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) {
     AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
   }
 }
 
 /**
@@ -417,11 +357,11 @@ SmmCpuFeaturesRendezvousEntry (
   )
 {
   //
   // If SMRR is supported and this is the first normal SMI, then enable SMRR
   //
-  if (mSmrrSupported && !mSmrrEnabled[CpuIndex]) {
+  if (FeaturePcdGet (PcdSmrrEnable) && !mSmrrEnabled[CpuIndex]) {
     AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
     mSmrrEnabled[CpuIndex] = TRUE;
   }
 }
 
@@ -458,11 +398,11 @@ EFIAPI
 SmmCpuFeaturesIsSmmRegisterSupported (
   IN UINTN         CpuIndex,
   IN SMM_REG_NAME  RegName
   )
 {
-  if (mSmmFeatureControlSupported && (RegName == SmmRegFeatureControl)) {
+  if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName == SmmRegFeatureControl)) {
     return TRUE;
   }
 
   return FALSE;
 }
@@ -484,11 +424,11 @@ EFIAPI
 SmmCpuFeaturesGetSmmRegister (
   IN UINTN         CpuIndex,
   IN SMM_REG_NAME  RegName
   )
 {
-  if (mSmmFeatureControlSupported && (RegName == SmmRegFeatureControl)) {
+  if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName == SmmRegFeatureControl)) {
     return AsmReadMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL);
   }
 
   return 0;
 }
@@ -510,11 +450,11 @@ SmmCpuFeaturesSetSmmRegister (
   IN UINTN         CpuIndex,
   IN SMM_REG_NAME  RegName,
   IN UINT64        Value
   )
 {
-  if (mSmmFeatureControlSupported && (RegName == SmmRegFeatureControl)) {
+  if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName == SmmRegFeatureControl)) {
     AsmWriteMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL, Value);
   }
 }
 
 /**
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
index 022351f593..85214ee31c 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
@@ -68,7 +68,11 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMsegSize                         ## SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStmExceptionStackSize         ## SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard                    ## CONSUMES
 
+[FeaturePcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable  ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable  ## CONSUMES
+
 [Depex]
   gEfiMpServiceProtocolGuid
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
index ec97041d8b..3eacab48db 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
@@ -34,5 +34,9 @@
   MemoryAllocationLib
   PcdLib
 
 [FixedPcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## SOMETIMES_CONSUMES
+
+[FeaturePcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable  ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable  ## CONSUMES
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 1951eb294c..55cbe7605f 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -153,10 +153,22 @@
   #   TRUE  - SMM Feature Control MSR will be locked.<BR>
   #   FALSE - SMM Feature Control MSR will not be locked.<BR>
   # @Prompt Lock SMM Feature Control MSR.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock|TRUE|BOOLEAN|0x3213210B
 
+  ## Indicates if SMRR will be enabled.<BR><BR>
+  #   TRUE  - SMRR will be enabled.<BR>
+  #   FALSE - SMRR will not be enabled.<BR>
+  # @Prompt Enable SMRR.
+  gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable|TRUE|BOOLEAN|0x3213210D
+
+  ## Indicates if SmmFeatureControl will be enabled.<BR><BR>
+  #   TRUE  - SmmFeatureControl will be enabled.<BR>
+  #   FALSE - SmmFeatureControl will not be enabled.<BR>
+  # @Prompt Support SmmFeatureControl.
+  gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable|TRUE|BOOLEAN|0x32132110
+
 [PcdsFixedAtBuild]
   ## List of exception vectors which need switching stack.
   #  This PCD will only take into effect if PcdCpuStackGuard is enabled.
   #  By default exception #DD(8), #PF(14) are supported.
   # @Prompt Specify exception vectors which need switching stack.
diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
index 219c1963bf..d17bcfd10c 100644
--- a/UefiCpuPkg/UefiCpuPkg.uni
+++ b/UefiCpuPkg/UefiCpuPkg.uni
@@ -98,10 +98,22 @@
 
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmFeatureControlMsrLock_HELP  #language en-US "Lock SMM Feature Control MSR?<BR><BR>\n"
                                                                                            "TRUE  - locked.<BR>\n"
                                                                                            "FALSE - unlocked.<BR>"
 
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdSmrrEnable_PROMPT  #language en-US "Indicates if SMRR will be enabled."
+
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdSmrrEnable_HELP  #language en-US "Indicates if SMRR will be enabled.<BR><BR>\n"
+                                                                                           "TRUE  - SMRR will be enabled.<BR>\n"
+                                                                                           "FALSE - SMRR will not be enabled.<BR>"
+
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdSmmFeatureControlEnable_PROMPT  #language en-US "Indicates if SmmFeatureControl will be enabled."
+
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdSmmFeatureControlEnable_HELP  #language en-US "Indicates if SmmFeatureControl will be enabled.<BR><BR>\n"
+                                                                                           "TRUE  - SmmFeatureControl will be enabled.<BR>\n"
+                                                                                           "FALSE - SmmFeatureControl will not be enabled.<BR>"
+
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdPeiTemporaryRamStackSize_PROMPT  #language en-US "Stack size in the temporary RAM"
 
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdPeiTemporaryRamStackSize_HELP  #language en-US "Specifies stack size in the temporary RAM. 0 means half of TemporaryRamSize."
 
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileSize_PROMPT  #language en-US "SMM profile data buffer size"
-- 
2.16.2.windows.1