* Re: [edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable & SmmFeatureControl capability
2022-07-18 0:12 ` [edk2-devel] " Michael D Kinney
@ 2022-07-18 7:32 ` Wu, Jiaxin
2022-07-29 4:03 ` Wu, Jiaxin
1 sibling, 0 replies; 4+ messages in thread
From: Wu, Jiaxin @ 2022-07-18 7:32 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io; +Cc: Dong, Eric, Ni, Ray
Hi Mike,
Thanks the comments. Only IA-32 processor will check on every SMI since it needs configure Mtrr. Do you think the impact is acceptable or not?
For fixed PCD solution, the original concern: the fixed PCD will be treated as global variable. Should we need consider no compiler optimization case or it must be optimized away condition checks?
Thanks,
Jiaxin
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Monday, July 18, 2022 8:13 AM
> To: devel@edk2.groups.io; Wu, Jiaxin <jiaxin.wu@intel.com>; Kinney, Michael
> D <michael.d.kinney@intel.com>
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable
> & SmmFeatureControl capability
>
> 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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu,
> Jiaxin
> > Sent: Sunday, July 17, 2022 1:38 AM
> > To: devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@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@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Jiaxin Wu <jiaxin.wu@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
> >
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable & SmmFeatureControl capability
2022-07-18 0:12 ` [edk2-devel] " Michael D Kinney
2022-07-18 7:32 ` Wu, Jiaxin
@ 2022-07-29 4:03 ` Wu, Jiaxin
1 sibling, 0 replies; 4+ messages in thread
From: Wu, Jiaxin @ 2022-07-29 4:03 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io; +Cc: Dong, Eric, Ni, Ray
Due to the SMI latency impact for IA-32 processor, I will drop this change & replace with the PCD check. I will resend the new patch for review.
Thanks,
Jiaxin
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Monday, July 18, 2022 3:32 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable
> & SmmFeatureControl capability
>
> Hi Mike,
>
> Thanks the comments. Only IA-32 processor will check on every SMI since it
> needs configure Mtrr. Do you think the impact is acceptable or not?
>
> For fixed PCD solution, the original concern: the fixed PCD will be treated as
> global variable. Should we need consider no compiler optimization case or it
> must be optimized away condition checks?
>
> Thanks,
> Jiaxin
>
>
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Monday, July 18, 2022 8:13 AM
> > To: devel@edk2.groups.io; Wu, Jiaxin <jiaxin.wu@intel.com>; Kinney, Michael
> > D <michael.d.kinney@intel.com>
> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: RE: [edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR
> enable
> > & SmmFeatureControl capability
> >
> > 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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu,
> > Jiaxin
> > > Sent: Sunday, July 17, 2022 1:38 AM
> > > To: devel@edk2.groups.io
> > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@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@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Signed-off-by: Jiaxin Wu <jiaxin.wu@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
> > >
> > >
> > >
> > >
> > >
^ permalink raw reply [flat|nested] 4+ messages in thread