Messages in this thread | | | Date | Fri, 31 Jul 2020 16:15:03 +0200 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH] Platform lockdown information in SYSFS |
| |
On Fri, Jul 31, 2020 at 10:30:43AM -0300, Daniel Gutson wrote: > On Fri, Jul 31, 2020 at 4:01 AM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Thu, Jul 30, 2020 at 06:41:36PM -0300, Daniel Gutson wrote: > > > This patch exports information about the platform lockdown > > > firmware configuration in the sysfs filesystem. > > > In this initial patch, I include some configuration attributes > > > for the system SPI chip. > > > > > > This initial version exports the BIOS Write Enable (bioswe), > > > BIOS Lock Enable (ble), and the SMM Bios Write Protect (SMM_BWP) > > > fields of the Bios Control register. The idea is to keep adding more > > > flags, not only from the BC but also from other registers in following > > > versions. > > > > > > The goal is that the attributes are avilable to fwupd when SecureBoot > > > is turned on. > > > > > > The patch provides a new misc driver, as proposed in the previous patch, > > > that provides a registration function for HW Driver devices to register > > > class_attributes. > > > In this case, the intel SPI flash chip (intel-spi) registers three > > > class_attributes corresponding to the fields mentioned above. > > > > Better, but you are abusing sysfs (note, no CAPS) a lot here... > > > > > > > > > > Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com> > > > --- > > > .../ABI/stable/sysfs-class-platform-lockdown | 23 +++++++ > > > MAINTAINERS | 7 +++ > > > drivers/misc/Kconfig | 9 +++ > > > drivers/misc/Makefile | 1 + > > > drivers/misc/platform-lockdown-attrs.c | 57 +++++++++++++++++ > > > drivers/mtd/spi-nor/controllers/Kconfig | 1 + > > > .../mtd/spi-nor/controllers/intel-spi-pci.c | 49 +++++++++++++++ > > > drivers/mtd/spi-nor/controllers/intel-spi.c | 62 +++++++++++++++++++ > > > .../platform_data/platform-lockdown-attrs.h | 19 ++++++ > > > 9 files changed, 228 insertions(+) > > > create mode 100644 Documentation/ABI/stable/sysfs-class-platform-lockdown > > > create mode 100644 drivers/misc/platform-lockdown-attrs.c > > > create mode 100644 include/linux/platform_data/platform-lockdown-attrs.h > > > > > > diff --git a/Documentation/ABI/stable/sysfs-class-platform-lockdown b/Documentation/ABI/stable/sysfs-class-platform-lockdown > > > new file mode 100644 > > > index 000000000000..6034d6cbefac > > > --- /dev/null > > > +++ b/Documentation/ABI/stable/sysfs-class-platform-lockdown > > > @@ -0,0 +1,23 @@ > > > +What: /sys/class/platform-lockdown/bioswe > > > +Date: July 2020 > > > +KernelVersion: 5.8.0 > > > +Contact: Daniel Gutson <daniel.gutson@eclypsium.com> > > > +Description: If the system firmware set BIOS Write Enable. > > > + 0: writes disabled, 1: writes enabled. > > > +Users: https://github.com/fwupd/fwupd > > > + > > > +What: /sys/class/platform-lockdown/ble > > > +Date: July 2020 > > > +KernelVersion: 5.8.0 > > > +Contact: Daniel Gutson <daniel.gutson@eclypsium.com> > > > +Description: If the system firmware set Bios Lock Enable. > > > + 0: SMM lock disabled, 1: SMM lock enabled. > > > +Users: https://github.com/fwupd/fwupd > > > + > > > +What: /sys/class/platform-lockdown/smm_bwp > > > +Date: July 2020 > > > +KernelVersion: 5.8.0 > > > +Contact: Daniel Gutson <daniel.gutson@eclypsium.com> > > > +Description: If the system firmware set SMM Bios Write Protect. > > > + 0: writes disabled unless in SMM, 1: writes enabled. > > > +Users: https://github.com/fwupd/fwupd > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index f0569cf304ca..771ed1693d28 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -13608,6 +13608,13 @@ S: Maintained > > > F: Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.yaml > > > F: drivers/iio/chemical/pms7003.c > > > > > > +PLATFORM LOCKDOWN ATTRIBUTES MODULE > > > +M: Daniel Gutson <daniel.gutson@eclypsium.com> > > > +S: Supported > > > +F: Documentation/ABI/sysfs-class-platform-lockdown > > > +F: drivers/misc/platform-lockdown-attrs.c > > > +F: include/linux/platform_data/platform-lockdown-attrs.h > > > + > > > PLX DMA DRIVER > > > M: Logan Gunthorpe <logang@deltatee.com> > > > S: Maintained > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > > > index e1b1ba5e2b92..058d4ba3cefd 100644 > > > --- a/drivers/misc/Kconfig > > > +++ b/drivers/misc/Kconfig > > > @@ -456,6 +456,15 @@ config PVPANIC > > > a paravirtualized device provided by QEMU; it lets a virtual machine > > > (guest) communicate panic events to the host. > > > > > > +config PLATFORM_LOCKDOWN_ATTRS > > > + tristate "Platform lockdown information in the SYSFS" > > > > "Platform lockdown information for some hardware information displayed > > in sysfs" ? > > > > > + depends on SYSFS > > > + help > > > + This kernel module is a helper driver to provide information about > > > + platform lockdown settings and configuration. > > > > Is that what this really is? > > > > > + This module is used by other device drivers -such as the intel-spi- > > > + to publish the information in /sys/class/platform-lockdown. > > > + > > > source "drivers/misc/c2port/Kconfig" > > > source "drivers/misc/eeprom/Kconfig" > > > source "drivers/misc/cb710/Kconfig" > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > > > index c7bd01ac6291..e29b45c564f9 100644 > > > --- a/drivers/misc/Makefile > > > +++ b/drivers/misc/Makefile > > > @@ -57,3 +57,4 @@ obj-$(CONFIG_PVPANIC) += pvpanic.o > > > obj-$(CONFIG_HABANA_AI) += habanalabs/ > > > obj-$(CONFIG_UACCE) += uacce/ > > > obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o > > > +obj-$(CONFIG_PLATFORM_LOCKDOWN_ATTRS) += platform-lockdown-attrs.o > > > diff --git a/drivers/misc/platform-lockdown-attrs.c b/drivers/misc/platform-lockdown-attrs.c > > > new file mode 100644 > > > index 000000000000..d08b3d895064 > > > --- /dev/null > > > +++ b/drivers/misc/platform-lockdown-attrs.c > > > @@ -0,0 +1,57 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Platform lockdown attributes kernel module > > > + * > > > + * Copyright (C) 2020 Daniel Gutson <daniel.gutson@eclypsium.com> > > > + * Copyright (C) 2020 Eclypsium Inc. > > > + */ > > > +#include <linux/kobject.h> > > > +#include <linux/sysfs.h> > > > +#include <linux/module.h> > > > +#include <linux/init.h> > > > +#include <linux/list.h> > > > +#include <linux/slab.h> > > > +#include <linux/platform_data/platform-lockdown-attrs.h> > > > + > > > +static struct class platform_lockdown_class = { > > > + .name = "platform-lockdown", > > > + .owner = THIS_MODULE, > > > +}; > > > + > > > +int register_platform_lockdown_attribute( > > > + struct class_attribute *lockdown_attribute) > > > +{ > > > + /* attempt to create the file: */ > > > + return class_create_file(&platform_lockdown_class, > > > + lockdown_attribute); > > > +} > > > +EXPORT_SYMBOL_GPL(register_platform_lockdown_attribute); > > > > nit, global symbols should put the noun first: > > platform_lockdown_attribute_register() > > > > But, this all is not ok. > > > > You are trying to create/remove random sysfs files that end up in a > > single class. What you want to do is create a device that is associated > > with this class, and make that device a child of the device you are > > registering these attributes for. > > > > Think of this as an input device. You don't put the random input > > attributes all in one place, you create a new device that represents the > > input interface and register that. Then userspace can iterate over all > > devices that are of the input class and see the individual attributes of > > them. > > So I create the child device. > And these attributes, will be class attributes or device attributes of > the child device?
Device attributes of course :)
greg k-h
| |