UefiCpuPkg: Add PCD to control SMRR enable & SmmFeatureControl support

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.

Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

6 files changed, 48 insertions, 23 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
@@ -35,3 +35,7 @@
 

 [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..75a0ec8e94 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
@@ -38,16 +38,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #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;

@@ -98,7 +88,7 @@ CpuFeaturesLibInitialization (
     // Check MTRR_CAP MSR bit 11 for SMRR support

     //

     if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MTRR_CAP) & BIT11) != 0) {

-      mSmrrSupported = TRUE;

+      ASSERT (FeaturePcdGet (PcdSmrrEnable));

     }

   }

 

@@ -111,7 +101,7 @@ CpuFeaturesLibInitialization (
   //

   if (FamilyId == 0x06) {

     if ((ModelId == 0x1C) || (ModelId == 0x26) || (ModelId == 0x27) || (ModelId == 0x35) || (ModelId == 0x36)) {

-      mSmrrSupported = FALSE;

+      ASSERT (!FeaturePcdGet (PcdSmrrEnable));

     }

   }

 

@@ -216,13 +206,12 @@ SmmCpuFeaturesInitializeProcessor (
   // 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 ((FeaturePcdGet (PcdSmrrEnable)) && (mSmrrPhysBaseMsr == SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE)) {

     FeatureControl = AsmReadMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL);

     if ((FeatureControl & BIT3) == 0) {

+      ASSERT ((FeatureControl & BIT0) == 0);

       if ((FeatureControl & BIT0) == 0) {

         AsmWriteMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL, FeatureControl | BIT3);

-      } else {

-        mSmrrSupported = FALSE;

       }

     }

   }

@@ -234,7 +223,7 @@ SmmCpuFeaturesInitializeProcessor (
   // 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

@@ -287,7 +276,7 @@ SmmCpuFeaturesInitializeProcessor (
       // 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;

+        ASSERT (FeaturePcdGet (PcdSmmFeatureControlEnable));

       }

     }

   }

@@ -383,7 +372,7 @@ SmmCpuFeaturesDisableSmrr (
   VOID

   )

 {

-  if (mSmrrSupported && mNeedConfigureMtrrs) {

+  if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) {

     AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) & ~EFI_MSR_SMRR_PHYS_MASK_VALID);

   }

 }

@@ -398,7 +387,7 @@ SmmCpuFeaturesReenableSmrr (
   VOID

   )

 {

-  if (mSmrrSupported && mNeedConfigureMtrrs) {

+  if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) {

     AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);

   }

 }

@@ -419,7 +408,7 @@ 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;

   }

@@ -460,7 +449,7 @@ SmmCpuFeaturesIsSmmRegisterSupported (
   IN SMM_REG_NAME  RegName

   )

 {

-  if (mSmmFeatureControlSupported && (RegName == SmmRegFeatureControl)) {

+  if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName == SmmRegFeatureControl)) {

     return TRUE;

   }

 

@@ -486,7 +475,7 @@ SmmCpuFeaturesGetSmmRegister (
   IN SMM_REG_NAME  RegName

   )

 {

-  if (mSmmFeatureControlSupported && (RegName == SmmRegFeatureControl)) {

+  if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName == SmmRegFeatureControl)) {

     return AsmReadMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL);

   }

 

@@ -512,7 +501,7 @@ SmmCpuFeaturesSetSmmRegister (
   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
@@ -70,5 +70,9 @@
   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
@@ -36,3 +36,7 @@
 

 [FixedPcd]

   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## SOMETIMES_CONSUMES

+

+[FeaturePcd]

+  gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable  ## CONSUMES

+  gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable  ## CONSUMES

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 4fe79cecbf..718323d904 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -158,6 +158,18 @@
   # @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.

diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
index 219c1963bf..d17bcfd10c 100644
--- a/UefiCpuPkg/UefiCpuPkg.uni
+++ b/UefiCpuPkg/UefiCpuPkg.uni
@@ -100,6 +100,18 @@
                                                                                            "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."