[PATCH 0/2] firmware: google: Expose coreboot tables and CBMEM

[PATCH 0/2] firmware: google: Expose coreboot tables and CBMEM

[patrick.rudolph]

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 sysfs attributes.

Unconditionally expose all tables and buffers making future changes in
coreboot possible without modifying a kernel driver.

Patrick Rudolph (2):
  firmware: google: Expose CBMEM over sysfs
  firmware: google: Expose coreboot tables over sysfs

 drivers/firmware/google/Kconfig          |   6 +
 drivers/firmware/google/Makefile         |   1 +
 drivers/firmware/google/cbmem-coreboot.c | 164 +++++++++++++++++++++++
 drivers/firmware/google/coreboot_table.c |  59 ++++++++
 drivers/firmware/google/coreboot_table.h |  13 ++
 5 files changed, 243 insertions(+)
 create mode 100644 drivers/firmware/google/cbmem-coreboot.c

-- 
2.21.0

[PATCH 1/2] firmware: google: Expose CBMEM over sysfs

[patrick.rudolph]

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
The binary table data can then be read from
/sys/bus/coreboot/drivers/cbmem/coreboot*/cbmem_attributes/data

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 drivers/firmware/google/Kconfig          |   9 ++
 drivers/firmware/google/Makefile         |   1 +
 drivers/firmware/google/cbmem-coreboot.c | 162 +++++++++++++++++++++++
 drivers/firmware/google/coreboot_table.h |  13 ++
 4 files changed, 185 insertions(+)
 create mode 100644 drivers/firmware/google/cbmem-coreboot.c

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..94b93d8351fe
--- /dev/null
+++ b/drivers/firmware/google/cbmem-coreboot.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * fmap-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 <linux/slab.h>
+
+#include "coreboot_table.h"
+
+#define CB_TAG_CBMEM_ENTRY 0x31
+
+struct cb_priv {
+	void __iomem *remap;
+	struct lb_cbmem_entry entry;
+};
+
+static ssize_t id_show(struct device *dev,
+		       struct device_attribute *attr, char *buffer)
+{
+	const struct cb_priv *priv;
+
+	priv = (const struct cb_priv *)dev_get_platdata(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;
+
+	priv = (const struct cb_priv *)dev_get_platdata(dev);
+
+	return sprintf(buffer, "%u\n", priv->entry.size);
+}
+
+static ssize_t address_show(struct device *dev,
+			    struct device_attribute *attr, char *buffer)
+{
+	const struct cb_priv *priv;
+
+	priv = (const struct cb_priv *)dev_get_platdata(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 cbmem_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);
+	const struct cb_priv *priv;
+
+	priv = (const struct cb_priv *)dev_get_platdata(dev);
+
+	return memory_read_from_buffer(buffer, count, &offset,
+				       priv->remap, priv->entry.size);
+}
+
+static struct bin_attribute coreboot_attr_data = {
+	.attr = { .name = "data", .mode = 0444 },
+	.read = cbmem_data_read,
+};
+
+static struct bin_attribute *cb_mem_bin_attrs[] = {
+	&coreboot_attr_data,
+	NULL
+};
+
+static struct attribute_group cb_mem_attr_group = {
+	.name = "cbmem_attributes",
+	.attrs = cb_mem_attrs,
+	.bin_attrs = cb_mem_bin_attrs,
+};
+
+static int cbmem_probe(struct coreboot_device *cdev)
+{
+	struct device *dev = &cdev->dev;
+	struct cb_priv *priv;
+	int err;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	memcpy(&priv->entry, &cdev->cbmem_entry, sizeof(priv->entry));
+
+	priv->remap = memremap(priv->entry.address,
+			       priv->entry.entry_size, MEMREMAP_WB);
+	if (!priv->remap) {
+		err = -ENOMEM;
+		goto failure;
+	}
+
+	err = sysfs_create_group(&dev->kobj, &cb_mem_attr_group);
+	if (err) {
+		dev_err(dev, "failed to create sysfs attributes: %d\n", err);
+		memunmap(priv->remap);
+		goto failure;
+	}
+
+	dev->platform_data = priv;
+
+	return 0;
+failure:
+	kfree(priv);
+	return err;
+}
+
+static int cbmem_remove(struct coreboot_device *cdev)
+{
+	struct device *dev = &cdev->dev;
+	const struct cb_priv *priv;
+
+	priv = (const struct cb_priv *)dev_get_platdata(dev);
+
+	sysfs_remove_group(&dev->kobj, &cb_mem_attr_group);
+
+	if (priv)
+		memunmap(priv->remap);
+	kfree(priv);
+
+	dev->platform_data = NULL;
+
+	return 0;
+}
+
+static struct coreboot_driver cbmem_driver = {
+	.probe = cbmem_probe,
+	.remove = cbmem_remove,
+	.drv = {
+		.name = "cbmem",
+	},
+	.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..506048c724d8 100644
--- a/drivers/firmware/google/coreboot_table.h
+++ b/drivers/firmware/google/coreboot_table.h
@@ -59,6 +59,18 @@ 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 +78,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.21.0

[PATCH 2/2] firmware: google: Expose coreboot tables over sysfs

[patrick.rudolph]

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.

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>
---
 drivers/firmware/google/coreboot_table.c | 60 ++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index 8d132e4f008a..baddf28a2103 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -6,6 +6,7 @@
  *
  * Copyright 2017 Google Inc.
  * Copyright 2017 Samuel Holland <samuel@sholland.org>
+ * Copyright 2019 9elements Agency GmbH
  */
 
 #include <linux/acpi.h>
@@ -84,6 +85,63 @@ 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, "%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 table_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 struct bin_attribute coreboot_attr_data = {
+	.attr = { .name = "data", .mode = 0444 },
+	.read = table_data_read,
+};
+
+static struct bin_attribute *cb_dev_bin_attrs[] = {
+	&coreboot_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 +162,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.21.0

Re: [PATCH 0/2] firmware: google: Expose coreboot tables and CBMEM

[Stephen Boyd]

Quoting patrick.rudolph@9elements.com (2019-11-15 08:15:14)
> 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 sysfs attributes.
> 
> Unconditionally expose all tables and buffers making future changes in
> coreboot possible without modifying a kernel driver.
> 
> Patrick Rudolph (2):
>   firmware: google: Expose CBMEM over sysfs
>   firmware: google: Expose coreboot tables over sysfs
> 
>  drivers/firmware/google/Kconfig          |   6 +
>  drivers/firmware/google/Makefile         |   1 +
>  drivers/firmware/google/cbmem-coreboot.c | 164 +++++++++++++++++++++++
>  drivers/firmware/google/coreboot_table.c |  59 ++++++++
>  drivers/firmware/google/coreboot_table.h |  13 ++

There should be some Documentation/ABI updates here too so we know how
the sysfs files work.

Re: [PATCH 1/2] firmware: google: Expose CBMEM over sysfs

[Greg Kroah-Hartman]

On Fri, Nov 15, 2019 at 05:15:15PM +0100, 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
> The binary table data can then be read from
> /sys/bus/coreboot/drivers/cbmem/coreboot*/cbmem_attributes/data
> 
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
>  drivers/firmware/google/Kconfig          |   9 ++
>  drivers/firmware/google/Makefile         |   1 +
>  drivers/firmware/google/cbmem-coreboot.c | 162 +++++++++++++++++++++++
>  drivers/firmware/google/coreboot_table.h |  13 ++
>  4 files changed, 185 insertions(+)
>  create mode 100644 drivers/firmware/google/cbmem-coreboot.c

As Stephen said, you have to document new sysfs attributes (or changes
or removals) in Documentation/ABI so we have a clue as to how to review
these changes to see if they match the code or not.

Please do so and resend the series with that addition and we will be
glad to review.

thanks,

greg k-h

Re: [PATCH 1/2] firmware: google: Expose CBMEM over sysfs

[kbuild test robot]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.4-rc7 next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/patrick-rudolph-9elements-com/firmware-google-Expose-coreboot-tables-and-CBMEM/20191116-033417
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 96b95eff4a591dbac582c2590d067e356a18aacb
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-32-g233d4e1-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/firmware/google/cbmem-coreboot.c:79:44: sparse: sparse: incorrect type in argument 4 (different address spaces) @@    expected void const *from @@    got void [noderef] <asvoid const *from @@
>> drivers/firmware/google/cbmem-coreboot.c:79:44: sparse:    expected void const *from
>> drivers/firmware/google/cbmem-coreboot.c:79:44: sparse:    got void [noderef] <asn:2> *const remap
>> drivers/firmware/google/cbmem-coreboot.c:110:21: sparse: sparse: incorrect type in assignment (different address spaces) @@    expected void [noderef] <asn:2> *remap @@    got n:2> *remap @@
>> drivers/firmware/google/cbmem-coreboot.c:110:21: sparse:    expected void [noderef] <asn:2> *remap
>> drivers/firmware/google/cbmem-coreboot.c:110:21: sparse:    got void *
>> drivers/firmware/google/cbmem-coreboot.c:120:30: sparse: sparse: incorrect type in argument 1 (different address spaces) @@    expected void *addr @@    got void [noderef] <asvoid *addr @@
>> drivers/firmware/google/cbmem-coreboot.c:120:30: sparse:    expected void *addr
>> drivers/firmware/google/cbmem-coreboot.c:120:30: sparse:    got void [noderef] <asn:2> *remap
>> drivers/firmware/google/cbmem-coreboot.c:142:30: sparse: sparse: incorrect type in argument 1 (different address spaces) @@    expected void *addr @@    got void [noderef] <asn:2> *void *addr @@
   drivers/firmware/google/cbmem-coreboot.c:142:30: sparse:    expected void *addr
   drivers/firmware/google/cbmem-coreboot.c:142:30: sparse:    got void [noderef] <asn:2> *const remap

vim +79 drivers/firmware/google/cbmem-coreboot.c

    68	
    69	static ssize_t cbmem_data_read(struct file *filp, struct kobject *kobj,
    70				       struct bin_attribute *bin_attr,
    71				       char *buffer, loff_t offset, size_t count)
    72	{
    73		struct device *dev = kobj_to_dev(kobj);
    74		const struct cb_priv *priv;
    75	
    76		priv = (const struct cb_priv *)dev_get_platdata(dev);
    77	
    78		return memory_read_from_buffer(buffer, count, &offset,
  > 79					       priv->remap, priv->entry.size);
    80	}
    81	
    82	static struct bin_attribute coreboot_attr_data = {
    83		.attr = { .name = "data", .mode = 0444 },
    84		.read = cbmem_data_read,
    85	};
    86	
    87	static struct bin_attribute *cb_mem_bin_attrs[] = {
    88		&coreboot_attr_data,
    89		NULL
    90	};
    91	
    92	static struct attribute_group cb_mem_attr_group = {
    93		.name = "cbmem_attributes",
    94		.attrs = cb_mem_attrs,
    95		.bin_attrs = cb_mem_bin_attrs,
    96	};
    97	
    98	static int cbmem_probe(struct coreboot_device *cdev)
    99	{
   100		struct device *dev = &cdev->dev;
   101		struct cb_priv *priv;
   102		int err;
   103	
   104		priv = kzalloc(sizeof(*priv), GFP_KERNEL);
   105		if (!priv)
   106			return -ENOMEM;
   107	
   108		memcpy(&priv->entry, &cdev->cbmem_entry, sizeof(priv->entry));
   109	
 > 110		priv->remap = memremap(priv->entry.address,
   111				       priv->entry.entry_size, MEMREMAP_WB);
   112		if (!priv->remap) {
   113			err = -ENOMEM;
   114			goto failure;
   115		}
   116	
   117		err = sysfs_create_group(&dev->kobj, &cb_mem_attr_group);
   118		if (err) {
   119			dev_err(dev, "failed to create sysfs attributes: %d\n", err);
 > 120			memunmap(priv->remap);
   121			goto failure;
   122		}
   123	
   124		dev->platform_data = priv;
   125	
   126		return 0;
   127	failure:
   128		kfree(priv);
   129		return err;
   130	}
   131	
   132	static int cbmem_remove(struct coreboot_device *cdev)
   133	{
   134		struct device *dev = &cdev->dev;
   135		const struct cb_priv *priv;
   136	
   137		priv = (const struct cb_priv *)dev_get_platdata(dev);
   138	
   139		sysfs_remove_group(&dev->kobj, &cb_mem_attr_group);
   140	
   141		if (priv)
 > 142			memunmap(priv->remap);
   143		kfree(priv);
   144	
   145		dev->platform_data = NULL;
   146	
   147		return 0;
   148	}
   149	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

Re: [PATCH 1/2] firmware: google: Expose CBMEM over sysfs

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5166 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.4-rc7 next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/patrick-rudolph-9elements-com/firmware-google-Expose-coreboot-tables-and-CBMEM/20191116-033417
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 96b95eff4a591dbac582c2590d067e356a18aacb
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-32-g233d4e1-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/firmware/google/cbmem-coreboot.c:79:44: sparse: sparse: incorrect type in argument 4 (different address spaces) @@    expected void const *from @@    got void [noderef] <asvoid const *from @@
>> drivers/firmware/google/cbmem-coreboot.c:79:44: sparse:    expected void const *from
>> drivers/firmware/google/cbmem-coreboot.c:79:44: sparse:    got void [noderef] <asn:2> *const remap
>> drivers/firmware/google/cbmem-coreboot.c:110:21: sparse: sparse: incorrect type in assignment (different address spaces) @@    expected void [noderef] <asn:2> *remap @@    got n:2> *remap @@
>> drivers/firmware/google/cbmem-coreboot.c:110:21: sparse:    expected void [noderef] <asn:2> *remap
>> drivers/firmware/google/cbmem-coreboot.c:110:21: sparse:    got void *
>> drivers/firmware/google/cbmem-coreboot.c:120:30: sparse: sparse: incorrect type in argument 1 (different address spaces) @@    expected void *addr @@    got void [noderef] <asvoid *addr @@
>> drivers/firmware/google/cbmem-coreboot.c:120:30: sparse:    expected void *addr
>> drivers/firmware/google/cbmem-coreboot.c:120:30: sparse:    got void [noderef] <asn:2> *remap
>> drivers/firmware/google/cbmem-coreboot.c:142:30: sparse: sparse: incorrect type in argument 1 (different address spaces) @@    expected void *addr @@    got void [noderef] <asn:2> *void *addr @@
   drivers/firmware/google/cbmem-coreboot.c:142:30: sparse:    expected void *addr
   drivers/firmware/google/cbmem-coreboot.c:142:30: sparse:    got void [noderef] <asn:2> *const remap

vim +79 drivers/firmware/google/cbmem-coreboot.c

    68	
    69	static ssize_t cbmem_data_read(struct file *filp, struct kobject *kobj,
    70				       struct bin_attribute *bin_attr,
    71				       char *buffer, loff_t offset, size_t count)
    72	{
    73		struct device *dev = kobj_to_dev(kobj);
    74		const struct cb_priv *priv;
    75	
    76		priv = (const struct cb_priv *)dev_get_platdata(dev);
    77	
    78		return memory_read_from_buffer(buffer, count, &offset,
  > 79					       priv->remap, priv->entry.size);
    80	}
    81	
    82	static struct bin_attribute coreboot_attr_data = {
    83		.attr = { .name = "data", .mode = 0444 },
    84		.read = cbmem_data_read,
    85	};
    86	
    87	static struct bin_attribute *cb_mem_bin_attrs[] = {
    88		&coreboot_attr_data,
    89		NULL
    90	};
    91	
    92	static struct attribute_group cb_mem_attr_group = {
    93		.name = "cbmem_attributes",
    94		.attrs = cb_mem_attrs,
    95		.bin_attrs = cb_mem_bin_attrs,
    96	};
    97	
    98	static int cbmem_probe(struct coreboot_device *cdev)
    99	{
   100		struct device *dev = &cdev->dev;
   101		struct cb_priv *priv;
   102		int err;
   103	
   104		priv = kzalloc(sizeof(*priv), GFP_KERNEL);
   105		if (!priv)
   106			return -ENOMEM;
   107	
   108		memcpy(&priv->entry, &cdev->cbmem_entry, sizeof(priv->entry));
   109	
 > 110		priv->remap = memremap(priv->entry.address,
   111				       priv->entry.entry_size, MEMREMAP_WB);
   112		if (!priv->remap) {
   113			err = -ENOMEM;
   114			goto failure;
   115		}
   116	
   117		err = sysfs_create_group(&dev->kobj, &cb_mem_attr_group);
   118		if (err) {
   119			dev_err(dev, "failed to create sysfs attributes: %d\n", err);
 > 120			memunmap(priv->remap);
   121			goto failure;
   122		}
   123	
   124		dev->platform_data = priv;
   125	
   126		return 0;
   127	failure:
   128		kfree(priv);
   129		return err;
   130	}
   131	
   132	static int cbmem_remove(struct coreboot_device *cdev)
   133	{
   134		struct device *dev = &cdev->dev;
   135		const struct cb_priv *priv;
   136	
   137		priv = (const struct cb_priv *)dev_get_platdata(dev);
   138	
   139		sysfs_remove_group(&dev->kobj, &cb_mem_attr_group);
   140	
   141		if (priv)
 > 142			memunmap(priv->remap);
   143		kfree(priv);
   144	
   145		dev->platform_data = NULL;
   146	
   147		return 0;
   148	}
   149	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation

Re: [PATCH 1/2] firmware: google: Expose CBMEM over sysfs

[Greg Kroah-Hartman]

On Wed, Nov 20, 2019 at 02:39:46PM +0100, patrick.rudolph@9elements.com wrote:
> +static int cbmem_probe(struct coreboot_device *cdev)
> +{
> +	struct device *dev = &cdev->dev;
> +	struct cb_priv *priv;
> +	int err;
> +
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	memcpy(&priv->entry, &cdev->cbmem_entry, sizeof(priv->entry));
> +
> +	priv->remap = memremap(priv->entry.address,
> +			       priv->entry.entry_size, MEMREMAP_WB);
> +	if (!priv->remap) {
> +		err = -ENOMEM;
> +		goto failure;
> +	}
> +
> +	err = sysfs_create_group(&dev->kobj, &cb_mem_attr_group);

Ick, you just raced userspace and lost :(

Please set the default group for the driver (dev_groups), so that the
driver core will correctly create and remove this group without you
having to do anything.

thanks,

greg k-h

[PATCH 1/2] firmware: google: Expose CBMEM over sysfs

[patrick.rudolph]

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
---
 Documentation/ABI/stable/sysfs-bus-coreboot |  43 ++++++
 drivers/firmware/google/Kconfig             |   9 ++
 drivers/firmware/google/Makefile            |   1 +
 drivers/firmware/google/cbmem-coreboot.c    | 162 ++++++++++++++++++++
 drivers/firmware/google/coreboot_table.h    |  13 ++
 5 files changed, 228 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..1b04b8abc858
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-bus-coreboot
@@ -0,0 +1,43 @@
+What:		/sys/bus/coreboot/devices/.../cbmem_attributes/id
+Date:		Nov 2019
+KernelVersion:	5.5
+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:		Nov 2019
+KernelVersion:	5.5
+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:		Nov 2019
+KernelVersion:	5.5
+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).
+
+What:		/sys/bus/coreboot/devices/.../cbmem_attributes/data
+Date:		Nov 2019
+KernelVersion:	5.5
+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..672f25313055
--- /dev/null
+++ b/drivers/firmware/google/cbmem-coreboot.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * fmap-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 <linux/slab.h>
+
+#include "coreboot_table.h"
+
+#define CB_TAG_CBMEM_ENTRY 0x31
+
+struct cb_priv {
+	void *remap;
+	struct lb_cbmem_entry entry;
+};
+
+static ssize_t id_show(struct device *dev,
+		       struct device_attribute *attr, char *buffer)
+{
+	const struct cb_priv *priv;
+
+	priv = (const struct cb_priv *)dev_get_platdata(dev);
+
+	return sprintf(buffer, "0x%08x\n", priv->entry.id);
+}
+
+static ssize_t size_show(struct device *dev,
+			 struct device_attribute *attr, char *buffer)
+{
+	const struct cb_priv *priv;
+
+	priv = (const struct cb_priv *)dev_get_platdata(dev);
+
+	return sprintf(buffer, "%u\n", priv->entry.size);
+}
+
+static ssize_t address_show(struct device *dev,
+			    struct device_attribute *attr, char *buffer)
+{
+	const struct cb_priv *priv;
+
+	priv = (const struct cb_priv *)dev_get_platdata(dev);
+
+	return sprintf(buffer, "0x%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 cbmem_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);
+	const struct cb_priv *priv;
+
+	priv = (const struct cb_priv *)dev_get_platdata(dev);
+
+	return memory_read_from_buffer(buffer, count, &offset,
+				       priv->remap, priv->entry.size);
+}
+
+static struct bin_attribute coreboot_attr_data = {
+	.attr = { .name = "data", .mode = 0444 },
+	.read = cbmem_data_read,
+};
+
+static struct bin_attribute *cb_mem_bin_attrs[] = {
+	&coreboot_attr_data,
+	NULL
+};
+
+static struct attribute_group cb_mem_attr_group = {
+	.name = "cbmem_attributes",
+	.attrs = cb_mem_attrs,
+	.bin_attrs = cb_mem_bin_attrs,
+};
+
+static int cbmem_probe(struct coreboot_device *cdev)
+{
+	struct device *dev = &cdev->dev;
+	struct cb_priv *priv;
+	int err;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	memcpy(&priv->entry, &cdev->cbmem_entry, sizeof(priv->entry));
+
+	priv->remap = memremap(priv->entry.address,
+			       priv->entry.entry_size, MEMREMAP_WB);
+	if (!priv->remap) {
+		err = -ENOMEM;
+		goto failure;
+	}
+
+	err = sysfs_create_group(&dev->kobj, &cb_mem_attr_group);
+	if (err) {
+		dev_err(dev, "failed to create sysfs attributes: %d\n", err);
+		memunmap(priv->remap);
+		goto failure;
+	}
+
+	dev->platform_data = priv;
+
+	return 0;
+failure:
+	kfree(priv);
+	return err;
+}
+
+static int cbmem_remove(struct coreboot_device *cdev)
+{
+	struct device *dev = &cdev->dev;
+	const struct cb_priv *priv;
+
+	priv = (const struct cb_priv *)dev_get_platdata(dev);
+
+	sysfs_remove_group(&dev->kobj, &cb_mem_attr_group);
+
+	if (priv)
+		memunmap(priv->remap);
+	kfree(priv);
+
+	dev->platform_data = NULL;
+
+	return 0;
+}
+
+static struct coreboot_driver cbmem_driver = {
+	.probe = cbmem_probe,
+	.remove = cbmem_remove,
+	.drv = {
+		.name = "cbmem",
+	},
+	.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..506048c724d8 100644
--- a/drivers/firmware/google/coreboot_table.h
+++ b/drivers/firmware/google/coreboot_table.h
@@ -59,6 +59,18 @@ 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 +78,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.21.0