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

On Mon, Sep 23, 2019 at 03:25:53PM -0700, speck for Pawan Gupta wrote:
> On Mon, Sep 23, 2019 at 09:13:12PM +0200, 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...
> > > 
> > > Yes, but these are PMU attributes.
> > > https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/x86/events/core.c#L2211
> > > https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/x86/events/core.c#L1831
> > 
> > Ick, ok, then no, you are back to putting it in /sys/devices/system/cpu/
> > then.
> > 
> > But that "feels" wrong given there really isn't anything else in that
> > directory for cpu features like this, they are all buried in the
> > individual cpuX directories, right?
> > 
> > Is there any other cpu feature you can turn on/off like this today in
> > the system that is controlled in sysfs?  If so, where is it at?
> 
> I don't know if it counts, but there is one for turning SMT on/off.
> 
> 	$ echo off > /sys/devices/system/cpu/smt/control

{sigh}

Yeah, it looks like you copied some of the logic for creating that file
(and that shouldn't be calling sysfs_create_group either, I'll add it to
my todo file...)

But again, those files are created from arch-independent code.  Is tsx
in any other processors and should it go into the same file where these
smt sysfs files are at, or is it really an Intel-only thing?

Either way, my original comment of "do not use sysfs_create_group() when
you have a 'struct device'" still stands, that needs to be fixed up no
matter what here.

greg k-h