[MODERATED] Re: [PATCH v4 04/10] TAAv4 4

On Tue, Sep 10, 2019 at 04:34:49PM -0700, speck for Luck, Tony wrote:
> On Tue, Sep 10, 2019 at 11:33:34PM +0100, speck for Greg KH wrote:
> > $ ls /sys/devices/cpu/
> > allow_tsx_force_abort  format                      power      type
> > caps                   freeze_on_smi               rdpmc      uevent
> > events                 perf_event_mux_interval_ms  subsystem
> > 
> > Oh look, a tsx-specific cpu sysfs file on my laptop...
> > 
> > But of cource it's not actually documented in Documentation/ABI/ so that
> > does need to be fixed as well, otherwise you could have used:
> > $ ./scripts/get_abi.pl search tsx
> > $
> > 
> > and it would have printed out lots of good information about the
> > allow_tsx_force_abort sysfs file that is there.
> 
> That doesn't look like a super helpful template to follow though.

What template?  Documentation/ABI/ ?  That's required for all sysfs
files (or should be, lots get merged without documentation...)

> If my steering through the twisty maze is correct, then the
> "allow_tsx_force_abort" file appeared there courtesy of this
> chain:
> 
> kernel/events/core.c:pmu_dev_alloc()
> 	if (pmu->attr_update)
>                 ret = sysfs_update_groups(&pmu->dev->kobj, pmu->attr_update);
> 
> pmu->attr_update was set in arch/x86/events/core.c:init_hw_perf_events()
> 	pmu.attr_update = x86_pmu.attr_update;
> 
> x86_pmu.attr_update was set in arch/x86/events/intel/core.c:intel_pmu_init()
> 	x86_pmu.attr_update = attr_update;
> 
> attr_update[] is a static array
> which includes an element
> 
> 	group_default
> 
> which includes an element
> 		intel_pmu_attrs[]
> 
> which includes an element
> 
> 			static DEVICE_ATTR(allow_tsx_force_abort, 0644,
> 
> Pawan wants to include a file to enable/disable TSX ... which has nothing
> to do with PMU ... so bolting his file into the bottom of that call sequence
> isn't going to make anyone happy.
> 
> Does he need to build a similar call sequence from generic code down
> into Intel specific X86 code that attaches somewhere outside of perf/events?

So this is a perf-event-specfic thing?  Then no, that's the wrong place
for this, sorry.  But you have to admit, it does look relevant given the
total lack of documentation here :(

greg k-h