Drop this patch replaced by new patch set "[edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable & SmmFeatureControl capability" since it's totally different solution for fix.
> -----Original Message----- > From: Wu, Jiaxin > Sent: Wednesday, June 29, 2022 9:38 AM > To: Ni, Ray <[email protected]>; [email protected] > Cc: Dong, Eric <[email protected]> > Subject: RE: [PATCH v1] UefiCpuPkg: Add PCD to control SMRR enable & > SmmFeatureControl support > > For below question 1&3: > Since we have defined the PCD for the feature control, should we still need > check the enable case? i though we only add the assert case for not support > case, because we must make sure has the capability to enable it. But for > support case, platform can still disable it via the pcd? > > For below question 2: > It's the intention because FeatureControl & BIT3 is 0, which means SMRR > Enable(BIT3) of MSR_FEATURE_CONTROL MSR(0x3A) is *not* set before > accessing SMRR base/mask MSRs, then ASSERT (!FeaturePcdGet > (PcdSmrrEnable)); > > > Thanks, > Jiaxin > > > > > -----Original Message----- > > From: Ni, Ray <[email protected]> > > Sent: Tuesday, June 28, 2022 5:17 PM > > To: Wu, Jiaxin <[email protected]>; [email protected] > > Cc: Dong, Eric <[email protected]> > > Subject: RE: [PATCH v1] UefiCpuPkg: Add PCD to control SMRR enable & > > SmmFeatureControl support > > > > > - // > > > - // Check CPUID(CPUID_VERSION_INFO).EDX[12] for MTRR capability > > > - // > > > - 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; > > > > 1. can we keep the logic but just replace the above line as "ASSERT > > (FeaturePcdGet (PcdSmrrEnable));"? > > > > > if ((FeatureControl & BIT3) == 0) { > > > - if ((FeatureControl & BIT0) == 0) { > > > + if (((FeatureControl & BIT0) == 0) && (FeaturePcdGet > > (PcdSmrrEnable))) > > > { > > > AsmWriteMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL, > > > FeatureControl | BIT3); > > > } else { > > > - mSmrrSupported = FALSE; > > > + ASSERT (!FeaturePcdGet (PcdSmrrEnable)); > > > > 2. If PcdSmrrEnable is TRUE but the FeatureControl MSR is locked (BIT0 is > > set), > > above assertion will be hit. We may need to reconsider the code logic. > > > > > - { > > > - // > > > - // 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; > > > > 3. can we keep the logic but just replace the above line as "ASSERT > > (FeaturePcdGet (PcdSmmFeatureControlEnable))"? -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#91433): https://edk2.groups.io/g/devel/message/91433 Mute This Topic: https://groups.io/mt/92040046/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
