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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 11, 2021 at 02:34:43PM -0700, Pawan Gupta wrote:
> > > 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.

So it seems to me then, the if test above should be changed to:

	if (boot_cpu_has(X86_FEATURE_TSX_FORCE_ABORT) && 
	   !boot_cpu_has(X86_FEATURE_RTM_ALWAYS_ABORT)) {
	   ...

and no need for the MSR read.

Please don't tell me there are configurations
where CPUID.RTM_ALWAYS_ABORT is clear but the
MSR_TSX_FORCE_ABORT[SDV_ENABLE_RTM] is there?!

This

"A new CPUID bit CPUID.RTM_ALWAYS_ABORT (CPUID 7.EDX[11]) is added to
indicate the status of always abort behavior."

tells me that the CPUID bit is always set by the microcode so we should
be ok.

If not, you should read that MSR early and do

	setup_force_cpu_cap(X86_FEATURE_RTM_ALWAYS_ABORT)

so that this "always abort" flag is always set when TSX transactions are
always aborted.

Thx.

-- 
Regards/Gruss,
    Boris.

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



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux