From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mx.groups.io with SMTP id smtpd.web08.5066.1659075961926526278 for ; Thu, 28 Jul 2022 23:26:02 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=LfcG5jQk; spf=pass (domain: intel.com, ip: 134.134.136.24, mailfrom: jiaxin.wu@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1659075962; x=1690611962; h=from:to:cc:subject:date:message-id; bh=zz3k6YIM0HdfGpLHHvXl7NGbVi7CviAdMkWjMiY0ezk=; b=LfcG5jQkmTd8ZxfXzwX8F9i/rEYXEjMT+jeQtV1NyW16+3rBGT+laEt/ AlG+oPIyDMpIJ7uoj3RmSY9+QUT2cVAO3uq52UW29dL4wo1JusokX+a+K oOgbRtBOdEYgwn6ED+AmsaO8KiiolJrg4cDS1+mHrGFjmBPAs7SyCQ80d ZJISKma7EXLPwqqZsUt/DYaRxlPPi66sNQZ0k1DrOiEqkshOMD+Q6mYkL SGlwLz3A73Hq4gGvhP71CWhH0EAP2gpAmpXgoH1P08LeC4nQ9ysP30qvS cQVBkatoWUeBtHlUdwei6okYtVSVbx6koD2oakF1LQQHHAmVxIz4wfO6t w==; X-IronPort-AV: E=McAfee;i="6400,9594,10422"; a="289472876" X-IronPort-AV: E=Sophos;i="5.93,200,1654585200"; d="scan'208";a="289472876" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jul 2022 23:26:01 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.93,200,1654585200"; d="scan'208";a="629257911" Received: from sh1gapp1009.ccr.corp.intel.com ([10.239.189.79]) by orsmga008.jf.intel.com with ESMTP; 28 Jul 2022 23:25:59 -0700 From: "Wu, Jiaxin" To: devel@edk2.groups.io Cc: Eric Dong , Ray Ni , Star Zeng , Michael D Kinney Subject: [PATCH v1] UefiCpuPkg: Add PCD to control SMRR enable & SmmFeatureControl support Date: Fri, 29 Jul 2022 14:25:55 +0800 Message-Id: <20220729062555.6272-1-jiaxin.wu@intel.com> X-Mailer: git-send-email 2.16.2.windows.1 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 Cc: Ray Ni Cc: Star Zeng Cc: Michael D Kinney Signed-off-by: Jiaxin Wu --- .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 4 +++ .../SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c | 35 ++++++++-------------- .../SmmCpuFeaturesLib/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 @@ -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..75a0ec8e94 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; @@ -96,11 +86,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; + ASSERT (FeaturePcdGet (PcdSmrrEnable)); } } // // Intel(R) 64 and IA-32 Architectures Software Developer's Manual @@ -109,11 +99,11 @@ 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; + ASSERT (!FeaturePcdGet (PcdSmrrEnable)); } } // // Intel(R) 64 and IA-32 Architectures Software Developer's Manual @@ -214,17 +204,16 @@ 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 ((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; } } } // @@ -232,11 +221,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 // @@ -285,11 +274,11 @@ SmmCpuFeaturesInitializeProcessor ( // // 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; + ASSERT (FeaturePcdGet (PcdSmmFeatureControlEnable)); } } } // @@ -381,11 +370,11 @@ VOID EFIAPI SmmCpuFeaturesDisableSmrr ( VOID ) { - if (mSmrrSupported && mNeedConfigureMtrrs) { + if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) { AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) & ~EFI_MSR_SMRR_PHYS_MASK_VALID); } } /** @@ -396,11 +385,11 @@ VOID EFIAPI SmmCpuFeaturesReenableSmrr ( VOID ) { - if (mSmrrSupported && mNeedConfigureMtrrs) { + if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) { AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID); } } /** @@ -417,11 +406,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 +447,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 +473,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 +499,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.
# FALSE - SMM Feature Control MSR will not be locked.
# @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. # 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?

\n" "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." #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileSize_PROMPT #language en-US "SMM profile data buffer size" -- 2.16.2.windows.1