Reviewed-by: Ray Ni <ray...@intel.com> > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, > Jiaxin > Sent: Friday, July 29, 2022 2:26 PM > To: devel@edk2.groups.io > Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Zeng, Star > <star.z...@intel.com>; Kinney, Michael D <michael.d.kin...@intel.com> > Subject: [edk2-devel] [PATCH v1] 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. > > Change-Id: I6835e4b0e12c5e6f52effb60fd9224e3eb97fc0d > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Star Zeng <star.z...@intel.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Signed-off-by: Jiaxin Wu <jiaxin...@intel.com> > --- > .../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.i > nf > b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.i > nf > index ec97041d8b..3eacab48db 100644 > --- > a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.i > nf > +++ > b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.i > nf > @@ -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|B > OOLEAN|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|BOOLEA > N|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_HE > LP #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_PROMP > T #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 > > > > >
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92640): https://edk2.groups.io/g/devel/message/92640 Mute This Topic: https://groups.io/mt/92685826/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
