From 76ec17526b7ebd0893f8e2e27ee4e4baf017eeef Mon Sep 17 00:00:00 2001 From: "Wu, Jiaxin" Date: Fri, 29 Jul 2022 14:25:55 +0800 Subject: [PATCH] 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 Reviewed-by: Ray Ni Cc: Star Zeng Cc: Michael D Kinney Signed-off-by: Jiaxin Wu --- .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 4 +++ .../SmmCpuFeaturesLibCommon.c | 35 +++++++------------ .../SmmCpuFeaturesLibStm.inf | 4 +++ .../StandaloneMmCpuFeaturesLib.inf | 4 +++ UefiCpuPkg/UefiCpuPkg.dec | 12 +++++++ UefiCpuPkg/UefiCpuPkg.uni | 12 +++++++ 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 @@ -37,16 +37,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #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 // @@ -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.

+ # TRUE - SMRR will be enabled.
+ # FALSE - SMRR will not be enabled.
+ # @Prompt Enable SMRR. + gUefiCpuPkgTokenSpaceGuid.PcdSmrrEnable|TRUE|BOOLEAN|0x3213210D + + ## Indicates if SmmFeatureControl will be enabled.

+ # TRUE - SmmFeatureControl will be enabled.
+ # FALSE - SmmFeatureControl will not be enabled.
+ # @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.
\n" "FALSE - unlocked.
" +#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.

\n" + "TRUE - SMRR will be enabled.
\n" + "FALSE - SMRR will not be enabled.
" + +#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.

\n" + "TRUE - SmmFeatureControl will be enabled.
\n" + "FALSE - SmmFeatureControl will not be enabled.
" + #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."