* [PATCH v4 0/2] firmware: google: Expose coreboot tables and CBMEM @ 2020-04-07 8:29 patrick.rudolph 2020-04-07 8:29 ` [PATCH v4 1/2] firmware: google: Expose CBMEM over sysfs patrick.rudolph 2020-04-07 8:29 ` [PATCH v4 2/2] firmware: google: Expose coreboot tables " patrick.rudolph 0 siblings, 2 replies; 10+ messages in thread From: patrick.rudolph @ 2020-04-07 8:29 UTC (permalink / raw) To: linux-kernel Cc: coreboot, Patrick Rudolph, Thomas Gleixner, Greg Kroah-Hartman, Alexios Zavras, Allison Randal, Julius Werner, Samuel Holland, Stephen Boyd From: Patrick Rudolph <patrick.rudolph@9elements.com> As user land tools currently use /dev/mem to access coreboot tables and CBMEM, provide a better way by using read-only sysfs attributes. Unconditionally expose all tables and buffers making future changes in coreboot possible without modifying a kernel driver. Changes in v2: - Add ABI documentation - Add 0x prefix on hex values - Remove wrong ioremap hint as found by CI Changes in v3: - Use BIN_ATTR_RO Changes in v4: - Use temporary memremap instead of persistent ioremap - Constify a struct - Get rid of unused headers - Use dev_{get|set}_drvdata - Use dev_groups to automatically handle attributes - Updated file description - Updated ABI documentation Patrick Rudolph (2): firmware: google: Expose CBMEM over sysfs firmware: google: Expose coreboot tables over sysfs Documentation/ABI/stable/sysfs-bus-coreboot | 74 +++++++++++ drivers/firmware/google/Kconfig | 9 ++ drivers/firmware/google/Makefile | 1 + drivers/firmware/google/cbmem-coreboot.c | 128 ++++++++++++++++++++ drivers/firmware/google/coreboot_table.c | 58 +++++++++ drivers/firmware/google/coreboot_table.h | 14 +++ 6 files changed, 284 insertions(+) create mode 100644 Documentation/ABI/stable/sysfs-bus-coreboot create mode 100644 drivers/firmware/google/cbmem-coreboot.c -- 2.24.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/2] firmware: google: Expose CBMEM over sysfs 2020-04-07 8:29 [PATCH v4 0/2] firmware: google: Expose coreboot tables and CBMEM patrick.rudolph @ 2020-04-07 8:29 ` patrick.rudolph 2020-04-20 0:07 ` Samuel Holland 2020-06-25 7:05 ` Stephen Boyd 2020-04-07 8:29 ` [PATCH v4 2/2] firmware: google: Expose coreboot tables " patrick.rudolph 1 sibling, 2 replies; 10+ messages in thread From: patrick.rudolph @ 2020-04-07 8:29 UTC (permalink / raw) To: linux-kernel Cc: coreboot, Patrick Rudolph, Thomas Gleixner, Greg Kroah-Hartman, Alexios Zavras, Allison Randal, Julius Werner, Samuel Holland, Stephen Boyd From: Patrick Rudolph <patrick.rudolph@9elements.com> Make all CBMEM buffers available to userland. This is useful for tools that are currently using /dev/mem. Make the id, size and address available, as well as the raw table data. Tools can easily scan the right CBMEM buffer by reading /sys/bus/coreboot/drivers/cbmem/coreboot*/cbmem_attributes/id or /sys/bus/coreboot/devices/coreboot*/cbmem_attributes/id The binary table data can then be read from /sys/bus/coreboot/drivers/cbmem/coreboot*/cbmem_attributes/data or /sys/bus/coreboot/devices/coreboot*/cbmem_attributes/data Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> --- -v2: - Add ABI documentation - Add 0x prefix on hex values -v3: - Use BIN_ATTR_RO -v4: - Use temporary memremap instead of persistent ioremap - Constify a struct - Get rid of unused headers - Use dev_{get|set}_drvdata - Use dev_groups to automatically handle attributes - Updated file description - Updated ABI documentation --- Documentation/ABI/stable/sysfs-bus-coreboot | 44 +++++++ drivers/firmware/google/Kconfig | 9 ++ drivers/firmware/google/Makefile | 1 + drivers/firmware/google/cbmem-coreboot.c | 128 ++++++++++++++++++++ drivers/firmware/google/coreboot_table.h | 14 +++ 5 files changed, 196 insertions(+) create mode 100644 Documentation/ABI/stable/sysfs-bus-coreboot create mode 100644 drivers/firmware/google/cbmem-coreboot.c diff --git a/Documentation/ABI/stable/sysfs-bus-coreboot b/Documentation/ABI/stable/sysfs-bus-coreboot new file mode 100644 index 000000000000..6055906f41f2 --- /dev/null +++ b/Documentation/ABI/stable/sysfs-bus-coreboot @@ -0,0 +1,44 @@ +What: /sys/bus/coreboot/devices/.../cbmem_attributes/id +Date: Apr 2020 +KernelVersion: 5.6 +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> +Description: + coreboot device directory can contain a file named + cbmem_attributes/id if the device corresponds to a CBMEM + buffer. + The file holds an ASCII representation of the CBMEM ID in hex + (e.g. 0xdeadbeef). + +What: /sys/bus/coreboot/devices/.../cbmem_attributes/size +Date: Apr 2020 +KernelVersion: 5.6 +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> +Description: + coreboot device directory can contain a file named + cbmem_attributes/size if the device corresponds to a CBMEM + buffer. + The file holds an representation as decimal number of the + CBMEM buffer size (e.g. 64). + +What: /sys/bus/coreboot/devices/.../cbmem_attributes/address +Date: Apr 2020 +KernelVersion: 5.6 +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> +Description: + coreboot device directory can contain a file named + cbmem_attributes/address if the device corresponds to a CBMEM + buffer. + The file holds an ASCII representation of the physical address + of the CBMEM buffer in hex (e.g. 0x000000008000d000) and should + be used for debugging only. + +What: /sys/bus/coreboot/devices/.../cbmem_attributes/data +Date: Apr 2020 +KernelVersion: 5.6 +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> +Description: + coreboot device directory can contain a file named + cbmem_attributes/data if the device corresponds to a CBMEM + buffer. + The file holds a read-only binary representation of the CBMEM + buffer. diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig index a3a6ca659ffa..11a67c397ab3 100644 --- a/drivers/firmware/google/Kconfig +++ b/drivers/firmware/google/Kconfig @@ -58,6 +58,15 @@ config GOOGLE_FRAMEBUFFER_COREBOOT This option enables the kernel to search for a framebuffer in the coreboot table. If found, it is registered with simplefb. +config GOOGLE_CBMEM_COREBOOT + tristate "Coreboot CBMEM access" + depends on GOOGLE_COREBOOT_TABLE + help + This option exposes all available CBMEM buffers to userland. + The CBMEM id, size and address as well as the raw table data + are exported as sysfs attributes of the corresponding coreboot + table. + config GOOGLE_MEMCONSOLE_COREBOOT tristate "Firmware Memory Console" depends on GOOGLE_COREBOOT_TABLE diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile index d17caded5d88..62053cd6d058 100644 --- a/drivers/firmware/google/Makefile +++ b/drivers/firmware/google/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_GOOGLE_SMI) += gsmi.o obj-$(CONFIG_GOOGLE_COREBOOT_TABLE) += coreboot_table.o +obj-$(CONFIG_GOOGLE_CBMEM_COREBOOT) += cbmem-coreboot.o obj-$(CONFIG_GOOGLE_FRAMEBUFFER_COREBOOT) += framebuffer-coreboot.o obj-$(CONFIG_GOOGLE_MEMCONSOLE) += memconsole.o obj-$(CONFIG_GOOGLE_MEMCONSOLE_COREBOOT) += memconsole-coreboot.o diff --git a/drivers/firmware/google/cbmem-coreboot.c b/drivers/firmware/google/cbmem-coreboot.c new file mode 100644 index 000000000000..f76704a6eec7 --- /dev/null +++ b/drivers/firmware/google/cbmem-coreboot.c @@ -0,0 +1,128 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * cbmem-coreboot.c + * + * Exports CBMEM as attributes in sysfs. + * + * Copyright 2012-2013 David Herrmann <dh.herrmann@gmail.com> + * Copyright 2017 Google Inc. + * Copyright 2019 9elements Agency GmbH + */ + +#include <linux/device.h> +#include <linux/kernel.h> +#include <linux/string.h> +#include <linux/module.h> +#include <linux/io.h> + +#include "coreboot_table.h" + +#define CB_TAG_CBMEM_ENTRY 0x31 + +struct cb_priv { + struct lb_cbmem_entry entry; +}; + +static ssize_t id_show(struct device *dev, + struct device_attribute *attr, char *buffer) +{ + const struct cb_priv *priv = dev_get_drvdata(dev); + + return sprintf(buffer, "%#08x\n", priv->entry.id); +} + +static ssize_t size_show(struct device *dev, + struct device_attribute *attr, char *buffer) +{ + const struct cb_priv *priv = dev_get_drvdata(dev); + + return sprintf(buffer, "%u\n", priv->entry.entry_size); +} + +static ssize_t address_show(struct device *dev, + struct device_attribute *attr, char *buffer) +{ + const struct cb_priv *priv = dev_get_drvdata(dev); + + return sprintf(buffer, "%#016llx\n", priv->entry.address); +} + +static DEVICE_ATTR_RO(id); +static DEVICE_ATTR_RO(size); +static DEVICE_ATTR_RO(address); + +static struct attribute *cb_mem_attrs[] = { + &dev_attr_address.attr, + &dev_attr_id.attr, + &dev_attr_size.attr, + NULL +}; + +static ssize_t data_read(struct file *filp, struct kobject *kobj, + struct bin_attribute *bin_attr, + char *buffer, loff_t offset, size_t count) +{ + const struct device *dev = kobj_to_dev(kobj); + const struct cb_priv *priv = dev_get_drvdata(dev); + void *ptr; + + /* CBMEM is always RAM with unknown caching attributes. */ + ptr = memremap(priv->entry.address, priv->entry.entry_size, + MEMREMAP_WB | MEMREMAP_WT); + if (!ptr) + return -ENOMEM; + + count = memory_read_from_buffer(buffer, count, &offset, ptr, + priv->entry.entry_size); + memunmap(ptr); + + return count; +} + +static BIN_ATTR_RO(data, 0); + +static struct bin_attribute *cb_mem_bin_attrs[] = { + &bin_attr_data, + NULL +}; + +static const struct attribute_group cb_mem_attr_group = { + .name = "cbmem_attributes", + .attrs = cb_mem_attrs, + .bin_attrs = cb_mem_bin_attrs, +}; + +static const struct attribute_group *attribute_groups[] = { + &cb_mem_attr_group, + NULL, +}; + +static int cbmem_probe(struct coreboot_device *cdev) +{ + struct device *dev = &cdev->dev; + struct cb_priv *priv; + + priv = devm_kmalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + memcpy(&priv->entry, &cdev->cbmem_entry, sizeof(priv->entry)); + + dev_set_drvdata(dev, priv); + + return 0; +} + +static struct coreboot_driver cbmem_driver = { + .probe = cbmem_probe, + .drv = { + .name = "cbmem", + .dev_groups = attribute_groups, + }, + .tag = CB_TAG_CBMEM_ENTRY, +}; + +module_coreboot_driver(cbmem_driver); + +MODULE_AUTHOR("9elements Agency GmbH"); +MODULE_LICENSE("GPL"); diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h index 7b7b4a6eedda..fc20b8649882 100644 --- a/drivers/firmware/google/coreboot_table.h +++ b/drivers/firmware/google/coreboot_table.h @@ -59,6 +59,19 @@ struct lb_framebuffer { u8 reserved_mask_size; }; +/* + * There can be more than one of these records as there is one per cbmem entry. + * Describes a buffer in memory containing runtime data. + */ +struct lb_cbmem_entry { + u32 tag; + u32 size; + + u64 address; + u32 entry_size; + u32 id; +}; + /* A device, additionally with information from coreboot. */ struct coreboot_device { struct device dev; @@ -66,6 +79,7 @@ struct coreboot_device { struct coreboot_table_entry entry; struct lb_cbmem_ref cbmem_ref; struct lb_framebuffer framebuffer; + struct lb_cbmem_entry cbmem_entry; }; }; -- 2.24.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] firmware: google: Expose CBMEM over sysfs 2020-04-07 8:29 ` [PATCH v4 1/2] firmware: google: Expose CBMEM over sysfs patrick.rudolph @ 2020-04-20 0:07 ` Samuel Holland 2020-06-25 7:05 ` Stephen Boyd 1 sibling, 0 replies; 10+ messages in thread From: Samuel Holland @ 2020-04-20 0:07 UTC (permalink / raw) To: patrick.rudolph, linux-kernel Cc: coreboot, Thomas Gleixner, Greg Kroah-Hartman, Alexios Zavras, Allison Randal, Julius Werner, Stephen Boyd On 4/7/20 3:29 AM, patrick.rudolph@9elements.com wrote: > From: Patrick Rudolph <patrick.rudolph@9elements.com> > > Make all CBMEM buffers available to userland. This is useful for tools > that are currently using /dev/mem. > > Make the id, size and address available, as well as the raw table data. > > Tools can easily scan the right CBMEM buffer by reading > /sys/bus/coreboot/drivers/cbmem/coreboot*/cbmem_attributes/id > or > /sys/bus/coreboot/devices/coreboot*/cbmem_attributes/id > > The binary table data can then be read from > /sys/bus/coreboot/drivers/cbmem/coreboot*/cbmem_attributes/data > or > /sys/bus/coreboot/devices/coreboot*/cbmem_attributes/data > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> > --- > -v2: > - Add ABI documentation > - Add 0x prefix on hex values > -v3: > - Use BIN_ATTR_RO > -v4: > - Use temporary memremap instead of persistent ioremap > - Constify a struct > - Get rid of unused headers > - Use dev_{get|set}_drvdata > - Use dev_groups to automatically handle attributes > - Updated file description > - Updated ABI documentation > --- > Documentation/ABI/stable/sysfs-bus-coreboot | 44 +++++++ > drivers/firmware/google/Kconfig | 9 ++ > drivers/firmware/google/Makefile | 1 + > drivers/firmware/google/cbmem-coreboot.c | 128 ++++++++++++++++++++ > drivers/firmware/google/coreboot_table.h | 14 +++ > 5 files changed, 196 insertions(+) > create mode 100644 Documentation/ABI/stable/sysfs-bus-coreboot > create mode 100644 drivers/firmware/google/cbmem-coreboot.c > > diff --git a/Documentation/ABI/stable/sysfs-bus-coreboot b/Documentation/ABI/stable/sysfs-bus-coreboot > new file mode 100644 > index 000000000000..6055906f41f2 > --- /dev/null > +++ b/Documentation/ABI/stable/sysfs-bus-coreboot > @@ -0,0 +1,44 @@ > +What: /sys/bus/coreboot/devices/.../cbmem_attributes/id > +Date: Apr 2020 > +KernelVersion: 5.6 I guess these will be 5.8 now. > +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> > +Description: > + coreboot device directory can contain a file named > + cbmem_attributes/id if the device corresponds to a CBMEM > + buffer. > + The file holds an ASCII representation of the CBMEM ID in hex > + (e.g. 0xdeadbeef). > + > +What: /sys/bus/coreboot/devices/.../cbmem_attributes/size > +Date: Apr 2020 > +KernelVersion: 5.6 > +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> > +Description: > + coreboot device directory can contain a file named > + cbmem_attributes/size if the device corresponds to a CBMEM > + buffer. > + The file holds an representation as decimal number of the nit: "a representation" (maybe "a decimal representation"?) > + CBMEM buffer size (e.g. 64). > + > +What: /sys/bus/coreboot/devices/.../cbmem_attributes/address > +Date: Apr 2020 > +KernelVersion: 5.6 > +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> > +Description: > + coreboot device directory can contain a file named > + cbmem_attributes/address if the device corresponds to a CBMEM > + buffer. > + The file holds an ASCII representation of the physical address > + of the CBMEM buffer in hex (e.g. 0x000000008000d000) and should > + be used for debugging only. > + > +What: /sys/bus/coreboot/devices/.../cbmem_attributes/data > +Date: Apr 2020 > +KernelVersion: 5.6 > +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> > +Description: > + coreboot device directory can contain a file named > + cbmem_attributes/data if the device corresponds to a CBMEM > + buffer. > + The file holds a read-only binary representation of the CBMEM > + buffer. > diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig > index a3a6ca659ffa..11a67c397ab3 100644 > --- a/drivers/firmware/google/Kconfig > +++ b/drivers/firmware/google/Kconfig > @@ -58,6 +58,15 @@ config GOOGLE_FRAMEBUFFER_COREBOOT > This option enables the kernel to search for a framebuffer in > the coreboot table. If found, it is registered with simplefb. > > +config GOOGLE_CBMEM_COREBOOT > + tristate "Coreboot CBMEM access" > + depends on GOOGLE_COREBOOT_TABLE > + help > + This option exposes all available CBMEM buffers to userland. > + The CBMEM id, size and address as well as the raw table data > + are exported as sysfs attributes of the corresponding coreboot > + table. > + > config GOOGLE_MEMCONSOLE_COREBOOT > tristate "Firmware Memory Console" > depends on GOOGLE_COREBOOT_TABLE > diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile > index d17caded5d88..62053cd6d058 100644 > --- a/drivers/firmware/google/Makefile > +++ b/drivers/firmware/google/Makefile > @@ -2,6 +2,7 @@ > > obj-$(CONFIG_GOOGLE_SMI) += gsmi.o > obj-$(CONFIG_GOOGLE_COREBOOT_TABLE) += coreboot_table.o > +obj-$(CONFIG_GOOGLE_CBMEM_COREBOOT) += cbmem-coreboot.o > obj-$(CONFIG_GOOGLE_FRAMEBUFFER_COREBOOT) += framebuffer-coreboot.o > obj-$(CONFIG_GOOGLE_MEMCONSOLE) += memconsole.o > obj-$(CONFIG_GOOGLE_MEMCONSOLE_COREBOOT) += memconsole-coreboot.o > diff --git a/drivers/firmware/google/cbmem-coreboot.c b/drivers/firmware/google/cbmem-coreboot.c > new file mode 100644 > index 000000000000..f76704a6eec7 > --- /dev/null > +++ b/drivers/firmware/google/cbmem-coreboot.c > @@ -0,0 +1,128 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * cbmem-coreboot.c > + * > + * Exports CBMEM as attributes in sysfs. > + * > + * Copyright 2012-2013 David Herrmann <dh.herrmann@gmail.com> > + * Copyright 2017 Google Inc. > + * Copyright 2019 9elements Agency GmbH > + */ > + > +#include <linux/device.h> > +#include <linux/kernel.h> > +#include <linux/string.h> > +#include <linux/module.h> > +#include <linux/io.h> > + > +#include "coreboot_table.h" > + > +#define CB_TAG_CBMEM_ENTRY 0x31 > + > +struct cb_priv { > + struct lb_cbmem_entry entry; > +}; > + > +static ssize_t id_show(struct device *dev, > + struct device_attribute *attr, char *buffer) > +{ > + const struct cb_priv *priv = dev_get_drvdata(dev); > + > + return sprintf(buffer, "%#08x\n", priv->entry.id); > +} > + > +static ssize_t size_show(struct device *dev, > + struct device_attribute *attr, char *buffer) > +{ > + const struct cb_priv *priv = dev_get_drvdata(dev); > + > + return sprintf(buffer, "%u\n", priv->entry.entry_size); > +} > + > +static ssize_t address_show(struct device *dev, > + struct device_attribute *attr, char *buffer) > +{ > + const struct cb_priv *priv = dev_get_drvdata(dev); > + > + return sprintf(buffer, "%#016llx\n", priv->entry.address); > +} > + > +static DEVICE_ATTR_RO(id); > +static DEVICE_ATTR_RO(size); > +static DEVICE_ATTR_RO(address); > + > +static struct attribute *cb_mem_attrs[] = { > + &dev_attr_address.attr, > + &dev_attr_id.attr, > + &dev_attr_size.attr, > + NULL > +}; > + > +static ssize_t data_read(struct file *filp, struct kobject *kobj, > + struct bin_attribute *bin_attr, > + char *buffer, loff_t offset, size_t count) > +{ > + const struct device *dev = kobj_to_dev(kobj); > + const struct cb_priv *priv = dev_get_drvdata(dev); > + void *ptr; > + > + /* CBMEM is always RAM with unknown caching attributes. */ > + ptr = memremap(priv->entry.address, priv->entry.entry_size, > + MEMREMAP_WB | MEMREMAP_WT); > + if (!ptr) > + return -ENOMEM; > + > + count = memory_read_from_buffer(buffer, count, &offset, ptr, > + priv->entry.entry_size); > + memunmap(ptr); > + > + return count; > +} > + > +static BIN_ATTR_RO(data, 0); > + > +static struct bin_attribute *cb_mem_bin_attrs[] = { > + &bin_attr_data, > + NULL > +}; > + > +static const struct attribute_group cb_mem_attr_group = { > + .name = "cbmem_attributes", > + .attrs = cb_mem_attrs, > + .bin_attrs = cb_mem_bin_attrs, > +}; > + > +static const struct attribute_group *attribute_groups[] = { > + &cb_mem_attr_group, > + NULL, > +}; > + > +static int cbmem_probe(struct coreboot_device *cdev) > +{ > + struct device *dev = &cdev->dev; > + struct cb_priv *priv; > + > + priv = devm_kmalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + memcpy(&priv->entry, &cdev->cbmem_entry, sizeof(priv->entry)); I don't think it is necessary to create a second copy of the table entry, when it is already available at cdev->cbmem_entry. You could do: dev_set_drvdata(dev, cdev); and that removes the need for struct cb_priv. Otherwise, Reviewed-by: Samuel Holland <samuel@sholland.org> Tested-by: Samuel Holland <samuel@sholland.org> I hacked nvramtool to pull the CMOS layout from /sys/bus/coreboot/devices/coreboot0/attributes/data, and that seemed to work. Cheers, Samuel > + > + dev_set_drvdata(dev, priv); > + > + return 0; > +} > + > +static struct coreboot_driver cbmem_driver = { > + .probe = cbmem_probe, > + .drv = { > + .name = "cbmem", > + .dev_groups = attribute_groups, > + }, > + .tag = CB_TAG_CBMEM_ENTRY, > +}; > + > +module_coreboot_driver(cbmem_driver); > + > +MODULE_AUTHOR("9elements Agency GmbH"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h > index 7b7b4a6eedda..fc20b8649882 100644 > --- a/drivers/firmware/google/coreboot_table.h > +++ b/drivers/firmware/google/coreboot_table.h > @@ -59,6 +59,19 @@ struct lb_framebuffer { > u8 reserved_mask_size; > }; > > +/* > + * There can be more than one of these records as there is one per cbmem entry. > + * Describes a buffer in memory containing runtime data. > + */ > +struct lb_cbmem_entry { > + u32 tag; > + u32 size; > + > + u64 address; > + u32 entry_size; > + u32 id; > +}; > + > /* A device, additionally with information from coreboot. */ > struct coreboot_device { > struct device dev; > @@ -66,6 +79,7 @@ struct coreboot_device { > struct coreboot_table_entry entry; > struct lb_cbmem_ref cbmem_ref; > struct lb_framebuffer framebuffer; > + struct lb_cbmem_entry cbmem_entry; > }; > }; > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] firmware: google: Expose CBMEM over sysfs 2020-04-07 8:29 ` [PATCH v4 1/2] firmware: google: Expose CBMEM over sysfs patrick.rudolph 2020-04-20 0:07 ` Samuel Holland @ 2020-06-25 7:05 ` Stephen Boyd 2020-06-25 20:51 ` Julius Werner 1 sibling, 1 reply; 10+ messages in thread From: Stephen Boyd @ 2020-06-25 7:05 UTC (permalink / raw) To: linux-kernel, patrick.rudolph Cc: coreboot, Patrick Rudolph, Thomas Gleixner, Greg Kroah-Hartman, Alexios Zavras, Allison Randal, Julius Werner, Samuel Holland Quoting patrick.rudolph@9elements.com (2020-04-07 01:29:06) > From: Patrick Rudolph <patrick.rudolph@9elements.com> > > Make all CBMEM buffers available to userland. This is useful for tools > that are currently using /dev/mem. > > Make the id, size and address available, as well as the raw table data. > > Tools can easily scan the right CBMEM buffer by reading > /sys/bus/coreboot/drivers/cbmem/coreboot*/cbmem_attributes/id > or > /sys/bus/coreboot/devices/coreboot*/cbmem_attributes/id > > The binary table data can then be read from > /sys/bus/coreboot/drivers/cbmem/coreboot*/cbmem_attributes/data > or > /sys/bus/coreboot/devices/coreboot*/cbmem_attributes/data > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> > --- Sorry, this fell off my radar. Looks close though so please resend. > diff --git a/Documentation/ABI/stable/sysfs-bus-coreboot b/Documentation/ABI/stable/sysfs-bus-coreboot > new file mode 100644 > index 000000000000..6055906f41f2 > --- /dev/null > +++ b/Documentation/ABI/stable/sysfs-bus-coreboot > @@ -0,0 +1,44 @@ > +What: /sys/bus/coreboot/devices/.../cbmem_attributes/id > +Date: Apr 2020 > +KernelVersion: 5.6 > +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> > +Description: > + coreboot device directory can contain a file named > + cbmem_attributes/id if the device corresponds to a CBMEM > + buffer. > + The file holds an ASCII representation of the CBMEM ID in hex > + (e.g. 0xdeadbeef). > + > +What: /sys/bus/coreboot/devices/.../cbmem_attributes/size > +Date: Apr 2020 > +KernelVersion: 5.6 > +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> > +Description: > + coreboot device directory can contain a file named > + cbmem_attributes/size if the device corresponds to a CBMEM > + buffer. > + The file holds an representation as decimal number of the > + CBMEM buffer size (e.g. 64). > + > +What: /sys/bus/coreboot/devices/.../cbmem_attributes/address > +Date: Apr 2020 > +KernelVersion: 5.6 > +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> > +Description: > + coreboot device directory can contain a file named > + cbmem_attributes/address if the device corresponds to a CBMEM > + buffer. > + The file holds an ASCII representation of the physical address > + of the CBMEM buffer in hex (e.g. 0x000000008000d000) and should > + be used for debugging only. If this is for debugging purposes only perhaps it should go into debugfs. We try to not leak information about physical addresses to userspace and this would let an attacker understand where memory may be. That's not ideal and should be avoided. > + > +What: /sys/bus/coreboot/devices/.../cbmem_attributes/data > +Date: Apr 2020 > +KernelVersion: 5.6 > +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> > +Description: > + coreboot device directory can contain a file named > + cbmem_attributes/data if the device corresponds to a CBMEM > + buffer. > + The file holds a read-only binary representation of the CBMEM > + buffer. > diff --git a/drivers/firmware/google/cbmem-coreboot.c b/drivers/firmware/google/cbmem-coreboot.c > new file mode 100644 > index 000000000000..f76704a6eec7 > --- /dev/null > +++ b/drivers/firmware/google/cbmem-coreboot.c > @@ -0,0 +1,128 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * cbmem-coreboot.c > + * > + * Exports CBMEM as attributes in sysfs. > + * > + * Copyright 2012-2013 David Herrmann <dh.herrmann@gmail.com> > + * Copyright 2017 Google Inc. > + * Copyright 2019 9elements Agency GmbH > + */ > + [..] > + &bin_attr_data, > + NULL > +}; > + > +static const struct attribute_group cb_mem_attr_group = { > + .name = "cbmem_attributes", > + .attrs = cb_mem_attrs, > + .bin_attrs = cb_mem_bin_attrs, > +}; > + > +static const struct attribute_group *attribute_groups[] = { > + &cb_mem_attr_group, > + NULL, Nitpick: Drop the comma on sentinel so nothing can come after lest a compile error happens. > +}; > + > +static int cbmem_probe(struct coreboot_device *cdev) > +{ > + struct device *dev = &cdev->dev; > + struct cb_priv *priv; > + > + priv = devm_kmalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + memcpy(&priv->entry, &cdev->cbmem_entry, sizeof(priv->entry)); > + > + dev_set_drvdata(dev, priv); Agreed, avoid the memcpy(). > + > + return 0; > +} ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] firmware: google: Expose CBMEM over sysfs 2020-06-25 7:05 ` Stephen Boyd @ 2020-06-25 20:51 ` Julius Werner 2020-06-26 7:27 ` Stephen Boyd 0 siblings, 1 reply; 10+ messages in thread From: Julius Werner @ 2020-06-25 20:51 UTC (permalink / raw) To: Stephen Boyd Cc: LKML, Patrick Rudolph, Coreboot, Thomas Gleixner, Greg Kroah-Hartman, Alexios Zavras, Allison Randal, Julius Werner, Samuel Holland > > +What: /sys/bus/coreboot/devices/.../cbmem_attributes/address > > +Date: Apr 2020 > > +KernelVersion: 5.6 > > +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> > > +Description: > > + coreboot device directory can contain a file named > > + cbmem_attributes/address if the device corresponds to a CBMEM > > + buffer. > > + The file holds an ASCII representation of the physical address > > + of the CBMEM buffer in hex (e.g. 0x000000008000d000) and should > > + be used for debugging only. > > If this is for debugging purposes only perhaps it should go into > debugfs. We try to not leak information about physical addresses to > userspace and this would let an attacker understand where memory may be. > That's not ideal and should be avoided. This is memory allocated by firmware and not subject to (k)ASLR, so nothing valuable can be leaked here. The same addresses could already be parsed out of /sys/firmware/log. Before this interface we usually accessed this stuff via /dev/mem (and tools that want to remain backwards-compatible will probably want to keep doing that), so having a quick shorthand to grab physical addresses can be convenient. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] firmware: google: Expose CBMEM over sysfs 2020-06-25 20:51 ` Julius Werner @ 2020-06-26 7:27 ` Stephen Boyd 2020-07-01 0:22 ` Julius Werner 0 siblings, 1 reply; 10+ messages in thread From: Stephen Boyd @ 2020-06-26 7:27 UTC (permalink / raw) To: Julius Werner Cc: LKML, Patrick Rudolph, Coreboot, Thomas Gleixner, Greg Kroah-Hartman, Alexios Zavras, Allison Randal, Julius Werner, Samuel Holland Quoting Julius Werner (2020-06-25 13:51:34) > > > +What: /sys/bus/coreboot/devices/.../cbmem_attributes/address > > > +Date: Apr 2020 > > > +KernelVersion: 5.6 > > > +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> > > > +Description: > > > + coreboot device directory can contain a file named > > > + cbmem_attributes/address if the device corresponds to a CBMEM > > > + buffer. > > > + The file holds an ASCII representation of the physical address > > > + of the CBMEM buffer in hex (e.g. 0x000000008000d000) and should > > > + be used for debugging only. > > > > If this is for debugging purposes only perhaps it should go into > > debugfs. We try to not leak information about physical addresses to > > userspace and this would let an attacker understand where memory may be. > > That's not ideal and should be avoided. > > This is memory allocated by firmware and not subject to (k)ASLR, so > nothing valuable can be leaked here. The same addresses could already > be parsed out of /sys/firmware/log. Before this interface we usually > accessed this stuff via /dev/mem (and tools that want to remain > backwards-compatible will probably want to keep doing that), so having > a quick shorthand to grab physical addresses can be convenient. Ok. Regardless of the concern of the physical address is there any usage of this attribute by userspace? The description makes it sound like it's a pure debug feature, which implies that it should be in debugfs and not in sysfs. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] firmware: google: Expose CBMEM over sysfs 2020-06-26 7:27 ` Stephen Boyd @ 2020-07-01 0:22 ` Julius Werner 0 siblings, 0 replies; 10+ messages in thread From: Julius Werner @ 2020-07-01 0:22 UTC (permalink / raw) To: Stephen Boyd Cc: Julius Werner, LKML, Patrick Rudolph, Coreboot, Thomas Gleixner, Greg Kroah-Hartman, Alexios Zavras, Allison Randal, Samuel Holland > Ok. Regardless of the concern of the physical address is there any usage > of this attribute by userspace? The description makes it sound like it's > a pure debug feature, which implies that it should be in debugfs and not > in sysfs. I'll leave that up to Patrick. I doubt we'd want to create a whole separate debugfs hierarchy just for this. Like I said you can just read it out of the log too, this would just make it a little bit more convenient. It's not like it would be the only informational attribute in sysfs... ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 2/2] firmware: google: Expose coreboot tables over sysfs 2020-04-07 8:29 [PATCH v4 0/2] firmware: google: Expose coreboot tables and CBMEM patrick.rudolph 2020-04-07 8:29 ` [PATCH v4 1/2] firmware: google: Expose CBMEM over sysfs patrick.rudolph @ 2020-04-07 8:29 ` patrick.rudolph 2020-04-20 0:07 ` Samuel Holland 2020-06-25 7:10 ` Stephen Boyd 1 sibling, 2 replies; 10+ messages in thread From: patrick.rudolph @ 2020-04-07 8:29 UTC (permalink / raw) To: linux-kernel Cc: coreboot, Patrick Rudolph, Greg Kroah-Hartman, Thomas Gleixner, Allison Randal, Alexios Zavras, Julius Werner, Stephen Boyd, Samuel Holland From: Patrick Rudolph <patrick.rudolph@9elements.com> Make all coreboot table entries available to userland. This is useful for tools that are currently using /dev/mem. Besides the tag and size also expose the raw table data itself. Update the ABI documentation to explain the new sysfs interface. Tools can easily scan for the right coreboot table by reading /sys/bus/coreboot/devices/coreboot*/attributes/id The binary table data can then be read from /sys/bus/coreboot/devices/coreboot*/attributes/data Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> --- -v2: - Add ABI documentation - Add 0x prefix on hex values - Remove wrong ioremap hint as found by CI -v3: - Use BIN_ATTR_RO -v4: - Updated ABI documentation --- Documentation/ABI/stable/sysfs-bus-coreboot | 30 +++++++++++ drivers/firmware/google/coreboot_table.c | 58 +++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/Documentation/ABI/stable/sysfs-bus-coreboot b/Documentation/ABI/stable/sysfs-bus-coreboot index 6055906f41f2..328153a1b3f4 100644 --- a/Documentation/ABI/stable/sysfs-bus-coreboot +++ b/Documentation/ABI/stable/sysfs-bus-coreboot @@ -42,3 +42,33 @@ Description: buffer. The file holds a read-only binary representation of the CBMEM buffer. + +What: /sys/bus/coreboot/devices/.../attributes/id +Date: Apr 2020 +KernelVersion: 5.6 +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> +Description: + coreboot device directory can contain a file named attributes/id. + The file holds an ASCII representation of the coreboot table ID + in hex (e.g. 0x000000ef). On coreboot enabled platforms the ID is + usually called TAG. + +What: /sys/bus/coreboot/devices/.../attributes/size +Date: Apr 2020 +KernelVersion: 5.6 +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> +Description: + coreboot device directory can contain a file named + attributes/size. + The file holds an ASCII representation as decimal number of the + coreboot table size (e.g. 64). + +What: /sys/bus/coreboot/devices/.../attributes/data +Date: Apr 2020 +KernelVersion: 5.6 +Contact: Patrick Rudolph <patrick.rudolph@9elements.com> +Description: + coreboot device directory can contain a file named + attributes/data. + The file holds a read-only binary representation of the coreboot + table. diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c index 0205987a4fd4..d0fc3eb93f4f 100644 --- a/drivers/firmware/google/coreboot_table.c +++ b/drivers/firmware/google/coreboot_table.c @@ -3,9 +3,11 @@ * coreboot_table.c * * Module providing coreboot table access. + * Exports coreboot tables as attributes in sysfs. * * Copyright 2017 Google Inc. * Copyright 2017 Samuel Holland <samuel@sholland.org> + * Copyright 2019 9elements Agency GmbH */ #include <linux/acpi.h> @@ -84,6 +86,60 @@ void coreboot_driver_unregister(struct coreboot_driver *driver) } EXPORT_SYMBOL(coreboot_driver_unregister); +static ssize_t id_show(struct device *dev, + struct device_attribute *attr, char *buffer) +{ + struct coreboot_device *device = CB_DEV(dev); + + return sprintf(buffer, "0x%08x\n", device->entry.tag); +} + +static ssize_t size_show(struct device *dev, + struct device_attribute *attr, char *buffer) +{ + struct coreboot_device *device = CB_DEV(dev); + + return sprintf(buffer, "%u\n", device->entry.size); +} + +static DEVICE_ATTR_RO(id); +static DEVICE_ATTR_RO(size); + +static struct attribute *cb_dev_attrs[] = { + &dev_attr_id.attr, + &dev_attr_size.attr, + NULL +}; + +static ssize_t data_read(struct file *filp, struct kobject *kobj, + struct bin_attribute *bin_attr, + char *buffer, loff_t offset, size_t count) +{ + struct device *dev = kobj_to_dev(kobj); + struct coreboot_device *device = CB_DEV(dev); + + return memory_read_from_buffer(buffer, count, &offset, + &device->entry, device->entry.size); +} + +static BIN_ATTR_RO(data, 0); + +static struct bin_attribute *cb_dev_bin_attrs[] = { + &bin_attr_data, + NULL +}; + +static const struct attribute_group cb_dev_attr_group = { + .name = "attributes", + .attrs = cb_dev_attrs, + .bin_attrs = cb_dev_bin_attrs, +}; + +static const struct attribute_group *cb_dev_attr_groups[] = { + &cb_dev_attr_group, + NULL +}; + static int coreboot_table_populate(struct device *dev, void *ptr) { int i, ret; @@ -104,6 +160,8 @@ static int coreboot_table_populate(struct device *dev, void *ptr) device->dev.parent = dev; device->dev.bus = &coreboot_bus_type; device->dev.release = coreboot_device_release; + device->dev.groups = cb_dev_attr_groups; + memcpy(&device->entry, ptr_entry, entry->size); ret = device_register(&device->dev); -- 2.24.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] firmware: google: Expose coreboot tables over sysfs 2020-04-07 8:29 ` [PATCH v4 2/2] firmware: google: Expose coreboot tables " patrick.rudolph @ 2020-04-20 0:07 ` Samuel Holland 2020-06-25 7:10 ` Stephen Boyd 1 sibling, 0 replies; 10+ messages in thread From: Samuel Holland @ 2020-04-20 0:07 UTC (permalink / raw) To: patrick.rudolph, linux-kernel Cc: coreboot, Greg Kroah-Hartman, Thomas Gleixner, Allison Randal, Alexios Zavras, Julius Werner, Stephen Boyd On 4/7/20 3:29 AM, patrick.rudolph@9elements.com wrote: > From: Patrick Rudolph <patrick.rudolph@9elements.com> > > Make all coreboot table entries available to userland. This is useful for > tools that are currently using /dev/mem. > > Besides the tag and size also expose the raw table data itself. > > Update the ABI documentation to explain the new sysfs interface. > > Tools can easily scan for the right coreboot table by reading > /sys/bus/coreboot/devices/coreboot*/attributes/id > The binary table data can then be read from > /sys/bus/coreboot/devices/coreboot*/attributes/data > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Reviewed-by: Samuel Holland <samuel@sholland.org> Tested-by: Samuel Holland <samuel@sholland.org> Cheers, Samuel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] firmware: google: Expose coreboot tables over sysfs 2020-04-07 8:29 ` [PATCH v4 2/2] firmware: google: Expose coreboot tables " patrick.rudolph 2020-04-20 0:07 ` Samuel Holland @ 2020-06-25 7:10 ` Stephen Boyd 1 sibling, 0 replies; 10+ messages in thread From: Stephen Boyd @ 2020-06-25 7:10 UTC (permalink / raw) To: linux-kernel, patrick.rudolph Cc: coreboot, Patrick Rudolph, Greg Kroah-Hartman, Thomas Gleixner, Allison Randal, Alexios Zavras, Julius Werner, Samuel Holland Quoting patrick.rudolph@9elements.com (2020-04-07 01:29:07) > From: Patrick Rudolph <patrick.rudolph@9elements.com> > > Make all coreboot table entries available to userland. This is useful for > tools that are currently using /dev/mem. > > Besides the tag and size also expose the raw table data itself. > > Update the ABI documentation to explain the new sysfs interface. > > Tools can easily scan for the right coreboot table by reading > /sys/bus/coreboot/devices/coreboot*/attributes/id > The binary table data can then be read from > /sys/bus/coreboot/devices/coreboot*/attributes/data > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org> Minor nits below: > -v2: > - Add ABI documentation > - Add 0x prefix on hex values > - Remove wrong ioremap hint as found by CI > -v3: > - Use BIN_ATTR_RO > -v4: > - Updated ABI documentation > --- > Documentation/ABI/stable/sysfs-bus-coreboot | 30 +++++++++++ > drivers/firmware/google/coreboot_table.c | 58 +++++++++++++++++++++ > 2 files changed, 88 insertions(+) > > diff --git a/Documentation/ABI/stable/sysfs-bus-coreboot b/Documentation/ABI/stable/sysfs-bus-coreboot > index 6055906f41f2..328153a1b3f4 100644 > --- a/Documentation/ABI/stable/sysfs-bus-coreboot > +++ b/Documentation/ABI/stable/sysfs-bus-coreboot > diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c > index 0205987a4fd4..d0fc3eb93f4f 100644 > --- a/drivers/firmware/google/coreboot_table.c > +++ b/drivers/firmware/google/coreboot_table.c > @@ -3,9 +3,11 @@ > * coreboot_table.c > * > * Module providing coreboot table access. > + * Exports coreboot tables as attributes in sysfs. > * > * Copyright 2017 Google Inc. > * Copyright 2017 Samuel Holland <samuel@sholland.org> > + * Copyright 2019 9elements Agency GmbH > */ > > #include <linux/acpi.h> > @@ -84,6 +86,60 @@ void coreboot_driver_unregister(struct coreboot_driver *driver) > } > EXPORT_SYMBOL(coreboot_driver_unregister); > > +static ssize_t id_show(struct device *dev, > + struct device_attribute *attr, char *buffer) > +{ > + struct coreboot_device *device = CB_DEV(dev); Wouldn't hurt to throw const in front of these. > + > + return sprintf(buffer, "0x%08x\n", device->entry.tag); > +} > + > +static ssize_t size_show(struct device *dev, > + struct device_attribute *attr, char *buffer) > +{ > + struct coreboot_device *device = CB_DEV(dev); And these. But the function is so short probably doesn't really matter. > + > + return sprintf(buffer, "%u\n", device->entry.size); > +} > + > +static DEVICE_ATTR_RO(id); > +static DEVICE_ATTR_RO(size); > + > +static struct attribute *cb_dev_attrs[] = { > + &dev_attr_id.attr, > + &dev_attr_size.attr, > + NULL > +}; > + > +static ssize_t data_read(struct file *filp, struct kobject *kobj, > + struct bin_attribute *bin_attr, > + char *buffer, loff_t offset, size_t count) > +{ > + struct device *dev = kobj_to_dev(kobj); > + struct coreboot_device *device = CB_DEV(dev); > + > + return memory_read_from_buffer(buffer, count, &offset, > + &device->entry, device->entry.size); > +} > + > +static BIN_ATTR_RO(data, 0); Still no way to figure out the actual size? Can we add the bin_attribute at runtime? > + > +static struct bin_attribute *cb_dev_bin_attrs[] = { > + &bin_attr_data, > + NULL > +}; > + ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-07-01 0:23 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-07 8:29 [PATCH v4 0/2] firmware: google: Expose coreboot tables and CBMEM patrick.rudolph 2020-04-07 8:29 ` [PATCH v4 1/2] firmware: google: Expose CBMEM over sysfs patrick.rudolph 2020-04-20 0:07 ` Samuel Holland 2020-06-25 7:05 ` Stephen Boyd 2020-06-25 20:51 ` Julius Werner 2020-06-26 7:27 ` Stephen Boyd 2020-07-01 0:22 ` Julius Werner 2020-04-07 8:29 ` [PATCH v4 2/2] firmware: google: Expose coreboot tables " patrick.rudolph 2020-04-20 0:07 ` Samuel Holland 2020-06-25 7:10 ` Stephen Boyd
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).