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

From: Arnd Bergmann <arnd@arndb.de>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Daniel Gutson <daniel.gutson@eclypsium.com>,
	Derek Kiernan <derek.kiernan@xilinx.com>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Richard Hughes <hughsient@gmail.com>,
	Alex Bazhaniuk <alex@eclypsium.com>
Subject: Re: [PATCH] [PATCH] Firmware security information in SYSFS
Date: Fri, 17 Jul 2020 16:40:40 +0200	[thread overview]
Message-ID: <CAK8P3a1yy7YyeJH5k40yAXb23y9siBnfuqixb76t3BK9Xh=uXQ@mail.gmail.com> (raw)
In-Reply-To: <20200717062841.GA3238569@kroah.com>

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, but let's first see how the
information gets into the kernel.

     Arnd