[edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable & SmmFeatureControl capability
Michael D Kinney
michael.d.kinney at intel.com
Mon Jul 18 00:12:42 UTC 2022
Are these checks made on every SMI?
What is the impact to SMI latency to do the check dynamically?
FeatureFlag and FixedAtBuild PCDs are declared as const global variables
which are used by optimizing compiler as constants in instructions or
optimize away condition checks all together. This option should still
be considered.
Mike
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Wu, Jiaxin
> Sent: Sunday, July 17, 2022 1:38 AM
> To: devel at edk2.groups.io
> Cc: Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>
> Subject: [edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable & SmmFeatureControl capability
>
> 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, dynamic check SMRR enable & SmmFeatureControl capability.
>
> Change-Id: I6835e4b0e12c5e6f52effb60fd9224e3eb97fc0d
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Signed-off-by: Jiaxin Wu <jiaxin.wu at intel.com>
> ---
> .../SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c | 248 ++++++++++++---------
> 1 file changed, 141 insertions(+), 107 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> index 78de7f8407..b2f31c993f 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> @@ -35,26 +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;
> -
> //
> // Set default value to assume MTRRs need to be configured on each SMI
> //
> BOOLEAN mNeedConfigureMtrrs = TRUE;
>
> @@ -62,26 +46,39 @@ BOOLEAN mNeedConfigureMtrrs = TRUE;
> // Array for state of SMRR enable on all CPUs
> //
> BOOLEAN *mSmrrEnabled;
>
> /**
> - Performs library initialization.
> + Return if SMRR is supported
>
> - This initialization function contains common functionality shared betwen all
> - library instance constructors.
> + @param[in] SmrrPhysBaseMsr Pointer to SmrrPhysBaseMsr.
> + @param[in] SmrrPhysMaskMsr Pointer to SmrrPhysMaskMsr.
> +
> + @retval TRUE SMRR is supported.
> + @retval FALSE SMRR is not supported.
>
> **/
> -VOID
> -CpuFeaturesLibInitialization (
> - VOID
> +BOOLEAN
> +IsSmrrSupported (
> + IN UINT32 *SmrrPhysBaseMsr OPTIONAL,
> + IN UINT32 *SmrrPhysMaskMsr OPTIONAL
> )
> {
> + BOOLEAN ReturnValue;
> +
> UINT32 RegEax;
> UINT32 RegEdx;
> UINTN FamilyId;
> UINTN ModelId;
>
> + UINT64 FeatureControl;
> +
> + //
> + // Set default value to assume SMRR is not supported
> + //
> + ReturnValue = FALSE;
> +
> //
> // Retrieve CPU Family and Model
> //
> AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
> FamilyId = (RegEax >> 8) & 0xf;
> @@ -96,11 +93,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;
> + ReturnValue = TRUE;
> }
> }
>
> //
> // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
> @@ -109,28 +106,79 @@ 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;
> + ReturnValue = FALSE;
> }
> }
>
> - //
> - // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
> - // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
> - //
> - // If CPU Family/Model is 06_0F or 06_17, then use Intel(R) Core(TM) 2
> - // Processor Family MSRs
> - //
> - if (FamilyId == 0x06) {
> - if ((ModelId == 0x17) || (ModelId == 0x0f)) {
> - mSmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE;
> - mSmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK;
> + if (ReturnValue) {
> + //
> + // Return the SmrrPhysBaseMsr & SmrrPhysMaskMsr if required & Smrr Supported
> + //
> + if (SmrrPhysBaseMsr != NULL) {
> + *SmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE;
> + }
> +
> + if (SmrrPhysBaseMsr != NULL) {
> + *SmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK;
> + }
> +
> + //
> + // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
> + // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
> + //
> + // If CPU Family/Model is 06_0F or 06_17, then use Intel(R) Core(TM) 2
> + // Processor Family MSRs
> + //
> + if (FamilyId == 0x06) {
> + if ((ModelId == 0x17) || (ModelId == 0x0f)) {
> + if (SmrrPhysBaseMsr != NULL) {
> + *SmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE;
> + }
> +
> + if (SmrrPhysMaskMsr != NULL) {
> + *SmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK;
> + }
> +
> + //
> + // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
> + // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
> + //
> + // 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.
> + //
> +
> + FeatureControl = AsmReadMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL);
> + if (((FeatureControl & BIT3) == 0) && ((FeatureControl & BIT0) == 1)) {
> + ReturnValue = FALSE;
> + }
> + }
> }
> }
>
> + return ReturnValue;
> +}
> +
> +/**
> + Performs library initialization.
> +
> + This initialization function contains common functionality shared betwen all
> + library instance constructors.
> +
> +**/
> +VOID
> +CpuFeaturesLibInitialization (
> + VOID
> + )
> +{
> + UINT32 RegEax;
> + UINT32 RegEdx;
> +
> //
> // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
> // Volume 3C, Section 34.4.2 SMRAM Caching
> // An IA-32 processor does not automatically write back and invalidate its
> // caches before entering SMM or before exiting SMM. Because of this behavior,
> @@ -193,50 +241,27 @@ SmmCpuFeaturesInitializeProcessor (
> IN EFI_PROCESSOR_INFORMATION *ProcessorInfo,
> IN CPU_HOT_PLUG_DATA *CpuHotPlugData
> )
> {
> SMRAM_SAVE_STATE_MAP *CpuState;
> - UINT64 FeatureControl;
> - UINT32 RegEax;
> - UINT32 RegEdx;
> - UINTN FamilyId;
> - UINTN ModelId;
> + UINT32 SmrrPhysBaseMsr;
> + UINT32 SmrrPhysMaskMsr;
>
> //
> // Configure SMBASE.
> //
> CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
> CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
>
> - //
> - // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
> - // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
> - //
> - // 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)) {
> - FeatureControl = AsmReadMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL);
> - if ((FeatureControl & BIT3) == 0) {
> - if ((FeatureControl & BIT0) == 0) {
> - AsmWriteMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL, FeatureControl | BIT3);
> - } else {
> - mSmrrSupported = FALSE;
> - }
> - }
> - }
> -
> //
> // If SMRR is supported, then program SMRR base/mask MSRs.
> // 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 (IsSmrrSupported (&SmrrPhysBaseMsr, &SmrrPhysMaskMsr)) {
> //
> // 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
> //
> @@ -250,50 +275,16 @@ SmmCpuFeaturesInitializeProcessor (
> if (IsMonarch) {
> DEBUG ((DEBUG_ERROR, "SMM Base/Size does not meet alignment/size requirement!\n"));
> CpuDeadLoop ();
> }
> } else {
> - AsmWriteMsr64 (mSmrrPhysBaseMsr, CpuHotPlugData->SmrrBase | MTRR_CACHE_WRITE_BACK);
> - AsmWriteMsr64 (mSmrrPhysMaskMsr, (~(CpuHotPlugData->SmrrSize - 1) & EFI_MSR_SMRR_MASK));
> + AsmWriteMsr64 (SmrrPhysBaseMsr, CpuHotPlugData->SmrrBase | MTRR_CACHE_WRITE_BACK);
> + AsmWriteMsr64 (SmrrPhysMaskMsr, (~(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,12 +372,14 @@ VOID
> EFIAPI
> SmmCpuFeaturesDisableSmrr (
> VOID
> )
> {
> - if (mSmrrSupported && mNeedConfigureMtrrs) {
> - AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) & ~EFI_MSR_SMRR_PHYS_MASK_VALID);
> + UINT32 SmrrPhysMaskMsr;
> +
> + if (IsSmrrSupported (NULL, &SmrrPhysMaskMsr) && mNeedConfigureMtrrs) {
> + AsmWriteMsr64 (SmrrPhysMaskMsr, AsmReadMsr64 (SmrrPhysMaskMsr) & ~EFI_MSR_SMRR_PHYS_MASK_VALID);
> }
> }
>
> /**
> Enable SMRR register if SMRR is supported and SmmCpuFeaturesNeedConfigureMtrrs()
> @@ -396,12 +389,14 @@ VOID
> EFIAPI
> SmmCpuFeaturesReenableSmrr (
> VOID
> )
> {
> - if (mSmrrSupported && mNeedConfigureMtrrs) {
> - AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
> + UINT32 SmrrPhysMaskMsr;
> +
> + if (IsSmrrSupported (NULL, &SmrrPhysMaskMsr) && mNeedConfigureMtrrs) {
> + AsmWriteMsr64 (SmrrPhysMaskMsr, AsmReadMsr64 (SmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
> }
> }
>
> /**
> Processor specific hook point each time a CPU enters System Management Mode.
> @@ -414,15 +409,17 @@ VOID
> EFIAPI
> SmmCpuFeaturesRendezvousEntry (
> IN UINTN CpuIndex
> )
> {
> + UINT32 SmrrPhysMaskMsr;
> +
> //
> // If SMRR is supported and this is the first normal SMI, then enable SMRR
> //
> - if (mSmrrSupported && !mSmrrEnabled[CpuIndex]) {
> - AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
> + if (!mSmrrEnabled[CpuIndex] && IsSmrrSupported (NULL, &SmrrPhysMaskMsr)) {
> + AsmWriteMsr64 (SmrrPhysMaskMsr, AsmReadMsr64 (SmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
> mSmrrEnabled[CpuIndex] = TRUE;
> }
> }
>
> /**
> @@ -458,12 +455,49 @@ EFIAPI
> SmmCpuFeaturesIsSmmRegisterSupported (
> IN UINTN CpuIndex,
> IN SMM_REG_NAME RegName
> )
> {
> - if (mSmmFeatureControlSupported && (RegName == SmmRegFeatureControl)) {
> - return TRUE;
> + UINT32 RegEax;
> + UINT32 RegEdx;
> + UINTN FamilyId;
> + UINTN ModelId;
> +
> + if (RegName == SmmRegFeatureControl) {
> + //
> + // 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) {
> + return TRUE;
> + }
> + }
> + }
> }
>
> return FALSE;
> }
>
> @@ -484,11 +518,11 @@ EFIAPI
> SmmCpuFeaturesGetSmmRegister (
> IN UINTN CpuIndex,
> IN SMM_REG_NAME RegName
> )
> {
> - if (mSmmFeatureControlSupported && (RegName == SmmRegFeatureControl)) {
> + if (SmmCpuFeaturesIsSmmRegisterSupported (CpuIndex, RegName)) {
> return AsmReadMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL);
> }
>
> return 0;
> }
> @@ -510,11 +544,11 @@ SmmCpuFeaturesSetSmmRegister (
> IN UINTN CpuIndex,
> IN SMM_REG_NAME RegName,
> IN UINT64 Value
> )
> {
> - if (mSmmFeatureControlSupported && (RegName == SmmRegFeatureControl)) {
> + if (SmmCpuFeaturesIsSmmRegisterSupported (CpuIndex, RegName)) {
> AsmWriteMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL, Value);
> }
> }
>
> /**
> --
> 2.16.2.windows.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91434): https://edk2.groups.io/g/devel/message/91434
Mute This Topic: https://groups.io/mt/92435488/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list