Messages in this thread | | | Date | Fri, 11 Jun 2021 14:34:43 -0700 | From | Pawan Gupta <> | Subject | Re: [PATCH 2/4] perf/x86/intel: Do not deploy workaround when TSX is deprecated |
| |
On 11.06.2021 09:50, Borislav Petkov wrote: >On Wed, Jun 09, 2021 at 02:12:38PM -0700, Pawan Gupta wrote: >> Earlier workaround added by commit 400816f60c54 ("perf/x86/intel: >> Implement support for TSX Force Abort") for perf counter interactions >> [1] are not required on some client systems which received a microcode >> update that deprecates TSX. >> >> Bypass the perf workaround when such microcode is enumerated. >> >> [1] Performance Monitoring Impact of Intel® Transactional Synchronization Extension Memory >> http://cdrdv2.intel.com/v1/dl/getContent/604224 >> >> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com> >> Reviewed-by: Andi Kleen <ak@linux.intel.com> >> Reviewed-by: Tony Luck <tony.luck@intel.com> >> Tested-by: Neelima Krishnan <neelima.krishnan@intel.com> >> --- >> arch/x86/events/intel/core.c | 22 ++++++++++++++++++---- >> 1 file changed, 18 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c >> index e28892270c58..b5953e1e59a2 100644 >> --- a/arch/x86/events/intel/core.c >> +++ b/arch/x86/events/intel/core.c >> @@ -6016,10 +6016,24 @@ __init int intel_pmu_init(void) >> intel_pmu_pebs_data_source_skl(pmem); >> >> if (boot_cpu_has(X86_FEATURE_TSX_FORCE_ABORT)) { >> - x86_pmu.flags |= PMU_FL_TFA; >> - x86_pmu.get_event_constraints = tfa_get_event_constraints; >> - x86_pmu.enable_all = intel_tfa_pmu_enable_all; >> - x86_pmu.commit_scheduling = intel_tfa_commit_scheduling; >> + u64 msr; >> + >> + rdmsrl(MSR_TSX_FORCE_ABORT, msr); >> + /* Systems that enumerate CPUID.RTM_ALWAYS_ABORT or >> + * support MSR_TSX_FORCE_ABORT[SDV_ENABLE_RTM] bit have >> + * TSX deprecated by default. TSX force abort hooks are >> + * not required on these systems. > >So if they're not required, why aren't you simply disabling the force >abort "workaround" by clearing the feature flag?
Feature flag also enumerates MSR_TSX_FORCE_ABORT, which is still present after the microcode update. Patch 3/4 in this series clears the TSX CPUID bits using MSR_TSX_FORCE_ABORT. So we do need the feature flag X86_FEATURE_TSX_FORCE_ABORT.
> > if (boot_cpu_has(X86_FEATURE_TSX_FORCE_ABORT)) { > if (boot_cpu_has(X86_FEATURE_RTM_ALWAYS_ABORT)) > setup_clear_cpu_cap(X86_FEATURE_TSX_FORCE_ABORT); > } > >so that it doesn't get enabled in the first place?
Feature flag is needed as explained above.
Thanks, Pawan
| |