* [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable & SmmFeatureControl capability
@ 2022-07-17 8:37 Wu, Jiaxin
2022-07-18 0:12 ` [edk2-devel] " Michael D Kinney
0 siblings, 1 reply; 4+ messages in thread
From: Wu, Jiaxin @ 2022-07-17 8:37 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni
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 related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable & SmmFeatureControl capability
2022-07-17 8:37 [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable & SmmFeatureControl capability Wu, Jiaxin
@ 2022-07-18 0:12 ` Michael D Kinney
2022-07-18 7:32 ` Wu, Jiaxin
2022-07-29 4:03 ` Wu, Jiaxin
0 siblings, 2 replies; 4+ messages in thread
From: Michael D Kinney @ 2022-07-18 0:12 UTC (permalink / raw)
To: devel@edk2.groups.io, Wu, Jiaxin, Kinney, Michael D; +Cc: Dong, Eric, Ni, Ray
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-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
end of thread, other threads:[~2022-07-29 4:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-17 8:37 [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable & SmmFeatureControl capability Wu, Jiaxin
2022-07-18 0:12 ` [edk2-devel] " Michael D Kinney
2022-07-18 7:32 ` Wu, Jiaxin
2022-07-29 4:03 ` Wu, Jiaxin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox