Re: [PATCH 2/4] perf/x86/intel: Do not deploy workaround when TSX is deprecated

[Borislav Petkov]

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@xxxxxxxxxxxxxxx>
> Reviewed-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx>
> Tested-by: Neelima Krishnan <neelima.krishnan@xxxxxxxxx>
> ---
>  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?

	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?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette