Re: [edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable & SmmFeatureControl capability

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

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
> > >
> > >
> > >
> > > 
> > >