Re: [PATCH] [PATCH] Firmware security information in SYSFS

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Fri, Jul 17, 2020 at 11:46:39AM -0300, Daniel Gutson wrote:
> On Fri, Jul 17, 2020 at 11:41 AM Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > On Fri, Jul 17, 2020 at 8:28 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Thu, Jul 16, 2020 at 07:36:27PM -0300, Daniel Gutson wrote:
> > > > +What:                /sys/kernel/firmware-security/bioswe
> > >
> > > Ick, I stopped reading right here.
> > >
> > > No, this is not where this belongs.
> > >
> > > We already have /sys/firmware/, right?  And firmware-specific
> > > subdirectories below that.
> > >
> > > We also have /sys/devices/system/ and I think that would be a much
> > > better place for this, as it is easier to work with a real 'struct
> > > device' than a "raw" kobject any day.  Bonus is you get full support of
> > > userspace libraries when you do that, unlike when dealing with kobjects.
> > >
> > > Also, this really is a _SPECIFIC_ type of firmware that supports these
> > > features, right?  Why not call that out too?  This is not generic by any
> > > means.
> >
> > As I suggested in my previous review, I wouldn't worry too much about
> > the user interface at the start, but instead first work out how the
> > hardware
> > support fits in with the existing drivers and once that looks fine decide
> > on how to export it to user space.
> >
> > I agree the  /sys/kernel/firmware-security/bioswe sounds like the wrong
> > place, but I'm not sure if adding any other new directory in sysfs is
> > much better. I think the most promising would be to have it on the
> > sysfs directory for the device it refers to,
> 
> 
> My idea is to have all the firmware security information together in the
> same place; this information comes from many devices.
> This initial patch involves the SPI Controller, and I don't want to add
> more stuff until there
> is a consensus.
> So, do you have a suggestion where to put this information?
> /sys/devices/system/firmware-security?
> /sys/firmware/security?
> other?
> 
> Please advise.

It's fun to focus on things like this, as it's the most visible part,
but are you sure the "talk to the hardware" part is working properly?

If so, great, it should be a "class", as that way it is independent of
any hardware type, right?  Classes show how devices talk to userspace in
a common way (input, tty, led, block, etc.)  So why is this any
different from that?

But worry about the other part first please.  And if you ever find
yourself using a "raw" kobject, you know you have done something wrong
:)

thanks,

greg k-h