[RFC] firmware coredump: add new firmware coredump class

[RFC] firmware coredump: add new firmware coredump class

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Many devices run firmware, and as other software such firmware has
bugs. When it misbehaves, however, it is often much harder to debug
than software running on the host.

Introduce a "firmware coredump" mechanism to allow dumping internal
firmware state through a generalized mechanism. As all devices are
different and information needed can vary accordingly, this doesn't
prescribe a file format - it just provides mechanism to get data to
be able to capture it in a generalized way (e.g. in distributions.)

Note that generalized capturing of such data may result in privacy
issues, so users generally need to be involved. In order to allow
certain users/system integrators/... to disable the feature at all,
introduce a Kconfig option to override the drivers that would like
to have the feature.

For now, this provides two ways of dumping data:
 1) with a vmalloc'ed area, that is then given to the fwcoredump
    subsystem and freed after retrieval or timeout
 2) with a generalized reader/free function method

We could/should add more options, e.g. a list of pages, since the
vmalloc area is very limited on some architectures.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 MAINTAINERS                |   7 ++
 drivers/base/Kconfig       |  21 +++++
 drivers/base/Makefile      |   1 +
 drivers/base/fwcoredump.c  | 222 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fwcoredump.h |  35 +++++++
 5 files changed, 286 insertions(+)
 create mode 100644 drivers/base/fwcoredump.c
 create mode 100644 include/linux/fwcoredump.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 2f85f55c8fb8..394bda1cde52 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3729,6 +3729,13 @@ F:	Documentation/firmware_class/
 F:	drivers/base/firmware*.c
 F:	include/linux/firmware.h
 
+FIRMWARE COREDUMP (fwcoredump)
+M:	Johannes Berg <johannes@sipsolutions.net>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	drivers/base/fwcoredump.c
+F:	include/linux/fwcoredump.h
+
 FLASH ADAPTER DRIVER (IBM Flash Adapter 900GB Full Height PCI Flash Card)
 M:	Joshua Morris <josh.h.morris@us.ibm.com>
 M:	Philip Kelleher <pjk1939@linux.vnet.ibm.com>
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 4e7f0ff83ae7..31eabab20c8a 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -165,6 +165,27 @@ config FW_LOADER_USER_HELPER_FALLBACK
 
 	  If you are unsure about this, say N here.
 
+config WANT_FW_COREDUMP
+	bool
+	help
+	  Drivers should "select" this option if they desire to use the
+	  firmware coredump mechanism.
+
+config DISABLE_FW_COREDUMP
+	bool "Disable firmware coredump" if EXPERT
+	help
+	  Disable the firmware coredump mechanism despite drivers wanting to
+	  use it; this allows for more sensitive systems or systems that
+	  don't want to ever access the information to not have the code,
+	  nor keep any data.
+
+	  If unsure, say N.
+
+config FW_COREDUMP
+	bool
+	default y if WANT_FW_COREDUMP
+	depends on !DISABLE_FW_COREDUMP
+
 config DEBUG_DRIVER
 	bool "Driver Core verbose debug messages"
 	depends on DEBUG_KERNEL
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 4aab26ec0292..2af1be519653 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o
 obj-$(CONFIG_REGMAP)	+= regmap/
 obj-$(CONFIG_SOC_BUS) += soc.o
 obj-$(CONFIG_PINCTRL) += pinctrl.o
+obj-$(CONFIG_FW_COREDUMP) += fwcoredump.o
 
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
 
diff --git a/drivers/base/fwcoredump.c b/drivers/base/fwcoredump.c
new file mode 100644
index 000000000000..f70bef0727a9
--- /dev/null
+++ b/drivers/base/fwcoredump.c
@@ -0,0 +1,222 @@
+/******************************************************************************
+ *
+ * This file is provided under the GPLv2 license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright(c) 2014 Intel Mobile Communications GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * The full GNU General Public License is included in this distribution
+ * in the file called COPYING.
+ *
+ * Contact Information:
+ *  Intel Linux Wireless <ilw@linux.intel.com>
+ * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
+ */
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/fwcoredump.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/workqueue.h>
+
+MODULE_AUTHOR("Johannes Berg <johannes@sipsolutions.net>");
+MODULE_DESCRIPTION("Firmware Coredump support");
+MODULE_LICENSE("GPL");
+
+/* if data isn't read by userspace after 5 minutes delete it */
+#define FWC_TIMEOUT	(HZ * 60 * 5)
+
+struct fwc_entry {
+	struct device fwc_dev;
+	const void *data;
+	size_t datalen;
+	struct module *owner;
+	ssize_t (*read)(char *buffer, loff_t offset, size_t count,
+			const void *data, size_t datalen);
+	void (*free)(const void *data);
+	struct delayed_work del_wk;
+};
+
+static struct fwc_entry *dev_to_fwc(struct device *dev)
+{
+	return container_of(dev, struct fwc_entry, fwc_dev);
+}
+
+static void fwc_dev_release(struct device *dev)
+{
+	struct fwc_entry *fwc = dev_to_fwc(dev);
+
+	fwc->free(fwc->data);
+	kfree(fwc);
+}
+
+static void fwc_del(struct work_struct *wk)
+{
+	struct fwc_entry *fwc;
+
+	fwc = container_of(wk, struct fwc_entry, del_wk.work);
+
+	device_del(&fwc->fwc_dev);
+	put_device(&fwc->fwc_dev);
+}
+
+static ssize_t fwc_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 fwc_entry *fwc = dev_to_fwc(dev);
+
+	return fwc->read(buffer, offset, count, fwc->data, fwc->datalen);
+}
+
+static ssize_t fwc_data_write(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 fwc_entry *fwc = dev_to_fwc(dev);
+
+	schedule_delayed_work(&fwc->del_wk, 0);
+
+	return count;
+}
+
+static struct bin_attribute fwc_attr_data = {
+	.attr = { .name = "data", .mode = S_IRUSR | S_IWUSR, },
+	.size = 0,
+	.read = fwc_data_read,
+	.write = fwc_data_write,
+};
+
+static struct bin_attribute *fwc_dev_bin_attrs[] = {
+	&fwc_attr_data, NULL,
+};
+
+static const struct attribute_group fwc_dev_group = {
+	.bin_attrs = fwc_dev_bin_attrs,
+};
+
+static const struct attribute_group *fwc_dev_groups[] = {
+	&fwc_dev_group, NULL,
+};
+
+static struct class fwc_class = {
+	.name		= "fwcoredump",
+	.dev_release	= fwc_dev_release,
+	.dev_groups	= fwc_dev_groups,
+};
+
+static ssize_t fwc_readv(char *buffer, loff_t offset, size_t count,
+			 const void *data, size_t datalen)
+{
+	if (offset > datalen)
+		return -EINVAL;
+
+	if (offset + count > datalen)
+		count = datalen - offset;
+
+	if (count)
+		memcpy(buffer, ((u8 *)data) + offset, count);
+
+	return count;
+}
+
+/**
+ * fw_coredumpv - create firmware coredump with vmalloc data
+ * @dev: the struct device for the crashed device
+ * @data: vmalloc data containing the firmware coredump
+ * @datalen: length of the data
+ * @gfp: allocation flags
+ */
+void fw_coredumpv(struct device *dev, const void *data, size_t datalen,
+		  gfp_t gfp)
+{
+	fw_coredumpm(dev, NULL, data, datalen, gfp, fwc_readv, vfree);
+}
+EXPORT_SYMBOL(fw_coredumpv);
+
+/**
+ * fw_coredumpm - create firmware coredump with read/free methods
+ * @dev: the struct device for the crashed device
+ * @data: data cookie for the @read/@free functions
+ * @datalen: length of the data
+ * @gfp: allocation flags
+ * @read: function to read from the given buffer
+ * @free: function to free the given buffer
+ */
+void fw_coredumpm(struct device *dev, struct module *owner,
+		  const void *data, size_t datalen, gfp_t gfp,
+		  ssize_t (*read)(char *buffer, loff_t offset, size_t count,
+				  const void *data, size_t datalen),
+		  void (*free)(const void *data))
+{
+	static atomic_t fwc_count = ATOMIC_INIT(0);
+	struct fwc_entry *fwc;
+
+	if (!try_module_get(owner))
+		return;
+
+	fwc = kzalloc(sizeof(*fwc), gfp);
+	if (!fwc)
+		goto put_module;
+
+	fwc->owner = owner;
+	fwc->data = data;
+	fwc->datalen = datalen;
+	fwc->read = read;
+	fwc->free = free;
+
+	device_initialize(&fwc->fwc_dev);
+
+	dev_set_name(&fwc->fwc_dev, "fwc%d", atomic_inc_return(&fwc_count));
+	fwc->fwc_dev.class = &fwc_class;
+
+	if (device_add(&fwc->fwc_dev))
+		goto put_device;
+
+	if (sysfs_create_link(&fwc->fwc_dev.kobj, &dev->kobj, "failing_device"))
+		/* nothing - symlink will be missing but that's ok */;
+
+	INIT_DELAYED_WORK(&fwc->del_wk, fwc_del);
+	schedule_delayed_work(&fwc->del_wk, FWC_TIMEOUT);
+
+	return;
+ put_device:
+	put_device(&fwc->fwc_dev);
+ put_module:
+	module_put(owner);
+}
+EXPORT_SYMBOL(fw_coredumpm);
+
+static int __init fwcoredump_init(void)
+{
+	return class_register(&fwc_class);
+}
+module_init(fwcoredump_init);
+
+static int fwc_free(struct device *dev, void *data)
+{
+	struct fwc_entry *fwc = dev_to_fwc(dev);
+
+	flush_delayed_work(&fwc->del_wk);
+	return 0;
+}
+
+static void __exit fwcoredump_exit(void)
+{
+	class_for_each_device(&fwc_class, NULL, NULL, fwc_free);
+	class_unregister(&fwc_class);
+}
+module_exit(fwcoredump_exit);
diff --git a/include/linux/fwcoredump.h b/include/linux/fwcoredump.h
new file mode 100644
index 000000000000..168f2d6abfc0
--- /dev/null
+++ b/include/linux/fwcoredump.h
@@ -0,0 +1,35 @@
+#ifndef __FWCOREDUMP_H
+#define __FWCOREDUMP_H
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/vmalloc.h>
+
+#ifdef CONFIG_FW_COREDUMP
+void fw_coredumpv(struct device *dev, const void *data, size_t datalen,
+		  gfp_t gfp);
+
+void fw_coredumpm(struct device *dev, struct module *owner,
+		  const void *data, size_t datalen, gfp_t gfp,
+		  ssize_t (*read)(char *buffer, loff_t offset, size_t count,
+				  const void *data, size_t datalen),
+		  void (*free)(const void *data));
+#else
+static inline void fw_coredumpv(struct device *dev, const void *data,
+				size_t datalen, gfp_t gfp)
+{
+	vfree(data);
+}
+
+static inline void
+fw_coredumpm(struct device *dev, struct module *owner,
+	     const void *data, size_t datalen, gfp_t gfp,
+	     ssize_t (*read)(char *buffer, loff_t offset, size_t count,
+			     const void *data, size_t datalen),
+	     void (*free)(const void *data))
+{
+	free(data);
+}
+#endif /* CONFIG_FW_COREDUMP */
+
+#endif /* __FWCOREDUMP_H */
-- 
2.1.0

Re: [RFC] firmware coredump: add new firmware coredump class

[Daniel Vetter]

[super-embarrassing resend, the previous one contained html gunk.]

If the idea is to also convert gpu crash dumps to this we should add
dri-devel. And there the crashes are usually not due to firmware, but
because the shaders and command batches userspace submitted have
issues, so this should also be renamed to dev_coredump I think.

On the overall design I wonder whether this shouldn't work more like a
real core dump and dump to a real file. At least currently the dumps
i915 creates are only useful as a general guide to where things went
wrong, but if we actually want to submit them as traces to the
hardware people we need to dump a _lot_ more. Otoh with the future of
shared virtual address spaces between gpu/cpu we might just do a real
core dump, so maybe this use case should be out of scope for your
patch here.

On the logic itself I'm not sure whether the timeout is all that
useful - at least in i915 our crash recovery works well enough that
reporters often don't realize right away when it happened, but only
later on when looking through logs to explain the tiny corruptions. If
the crashdupm has evapored meanwhile that's not that useful.

Also, at least for gpus it's usually not interesting to grab
subsequent dumps: Often the gpu is in a bad mood due to the first
crash, or it's just a massive row of duplicated dumps. So in i915 we
only record the first crash and keep it around forever. And tooling
can still free it by writing to the file. This also ensures that we
don't waste excessive amounts of memory with crash dumps.

And if we want to use this for i915 we need some way for tools to go
from the i915 drm class device node to the error state, not just from
the error state back to the device.
-Daniel

On Wed, Sep 3, 2014 at 1:15 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Many devices run firmware, and as other software such firmware has
> bugs. When it misbehaves, however, it is often much harder to debug
> than software running on the host.
>
> Introduce a "firmware coredump" mechanism to allow dumping internal
> firmware state through a generalized mechanism. As all devices are
> different and information needed can vary accordingly, this doesn't
> prescribe a file format - it just provides mechanism to get data to
> be able to capture it in a generalized way (e.g. in distributions.)
>
> Note that generalized capturing of such data may result in privacy
> issues, so users generally need to be involved. In order to allow
> certain users/system integrators/... to disable the feature at all,
> introduce a Kconfig option to override the drivers that would like
> to have the feature.
>
> For now, this provides two ways of dumping data:
>  1) with a vmalloc'ed area, that is then given to the fwcoredump
>     subsystem and freed after retrieval or timeout
>  2) with a generalized reader/free function method
>
> We could/should add more options, e.g. a list of pages, since the
> vmalloc area is very limited on some architectures.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  MAINTAINERS                |   7 ++
>  drivers/base/Kconfig       |  21 +++++
>  drivers/base/Makefile      |   1 +
>  drivers/base/fwcoredump.c  | 222 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fwcoredump.h |  35 +++++++
>  5 files changed, 286 insertions(+)
>  create mode 100644 drivers/base/fwcoredump.c
>  create mode 100644 include/linux/fwcoredump.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2f85f55c8fb8..394bda1cde52 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3729,6 +3729,13 @@ F:       Documentation/firmware_class/
>  F:     drivers/base/firmware*.c
>  F:     include/linux/firmware.h
>
> +FIRMWARE COREDUMP (fwcoredump)
> +M:     Johannes Berg <johannes@sipsolutions.net>
> +L:     linux-kernel@vger.kernel.org
> +S:     Maintained
> +F:     drivers/base/fwcoredump.c
> +F:     include/linux/fwcoredump.h
> +
>  FLASH ADAPTER DRIVER (IBM Flash Adapter 900GB Full Height PCI Flash Card)
>  M:     Joshua Morris <josh.h.morris@us.ibm.com>
>  M:     Philip Kelleher <pjk1939@linux.vnet.ibm.com>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 4e7f0ff83ae7..31eabab20c8a 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -165,6 +165,27 @@ config FW_LOADER_USER_HELPER_FALLBACK
>
>           If you are unsure about this, say N here.
>
> +config WANT_FW_COREDUMP
> +       bool
> +       help
> +         Drivers should "select" this option if they desire to use the
> +         firmware coredump mechanism.
> +
> +config DISABLE_FW_COREDUMP
> +       bool "Disable firmware coredump" if EXPERT
> +       help
> +         Disable the firmware coredump mechanism despite drivers wanting to
> +         use it; this allows for more sensitive systems or systems that
> +         don't want to ever access the information to not have the code,
> +         nor keep any data.
> +
> +         If unsure, say N.
> +
> +config FW_COREDUMP
> +       bool
> +       default y if WANT_FW_COREDUMP
> +       depends on !DISABLE_FW_COREDUMP
> +
>  config DEBUG_DRIVER
>         bool "Driver Core verbose debug messages"
>         depends on DEBUG_KERNEL
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 4aab26ec0292..2af1be519653 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o
>  obj-$(CONFIG_REGMAP)   += regmap/
>  obj-$(CONFIG_SOC_BUS) += soc.o
>  obj-$(CONFIG_PINCTRL) += pinctrl.o
> +obj-$(CONFIG_FW_COREDUMP) += fwcoredump.o
>
>  ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
>
> diff --git a/drivers/base/fwcoredump.c b/drivers/base/fwcoredump.c
> new file mode 100644
> index 000000000000..f70bef0727a9
> --- /dev/null
> +++ b/drivers/base/fwcoredump.c
> @@ -0,0 +1,222 @@
> +/******************************************************************************
> + *
> + * This file is provided under the GPLv2 license.
> + *
> + * GPL LICENSE SUMMARY
> + *
> + * Copyright(c) 2014 Intel Mobile Communications GmbH
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * The full GNU General Public License is included in this distribution
> + * in the file called COPYING.
> + *
> + * Contact Information:
> + *  Intel Linux Wireless <ilw@linux.intel.com>
> + * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
> + */
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/fwcoredump.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/workqueue.h>
> +
> +MODULE_AUTHOR("Johannes Berg <johannes@sipsolutions.net>");
> +MODULE_DESCRIPTION("Firmware Coredump support");
> +MODULE_LICENSE("GPL");
> +
> +/* if data isn't read by userspace after 5 minutes delete it */
> +#define FWC_TIMEOUT    (HZ * 60 * 5)
> +
> +struct fwc_entry {
> +       struct device fwc_dev;
> +       const void *data;
> +       size_t datalen;
> +       struct module *owner;
> +       ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> +                       const void *data, size_t datalen);
> +       void (*free)(const void *data);
> +       struct delayed_work del_wk;
> +};
> +
> +static struct fwc_entry *dev_to_fwc(struct device *dev)
> +{
> +       return container_of(dev, struct fwc_entry, fwc_dev);
> +}
> +
> +static void fwc_dev_release(struct device *dev)
> +{
> +       struct fwc_entry *fwc = dev_to_fwc(dev);
> +
> +       fwc->free(fwc->data);
> +       kfree(fwc);
> +}
> +
> +static void fwc_del(struct work_struct *wk)
> +{
> +       struct fwc_entry *fwc;
> +
> +       fwc = container_of(wk, struct fwc_entry, del_wk.work);
> +
> +       device_del(&fwc->fwc_dev);
> +       put_device(&fwc->fwc_dev);
> +}
> +
> +static ssize_t fwc_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 fwc_entry *fwc = dev_to_fwc(dev);
> +
> +       return fwc->read(buffer, offset, count, fwc->data, fwc->datalen);
> +}
> +
> +static ssize_t fwc_data_write(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 fwc_entry *fwc = dev_to_fwc(dev);
> +
> +       schedule_delayed_work(&fwc->del_wk, 0);
> +
> +       return count;
> +}
> +
> +static struct bin_attribute fwc_attr_data = {
> +       .attr = { .name = "data", .mode = S_IRUSR | S_IWUSR, },
> +       .size = 0,
> +       .read = fwc_data_read,
> +       .write = fwc_data_write,
> +};
> +
> +static struct bin_attribute *fwc_dev_bin_attrs[] = {
> +       &fwc_attr_data, NULL,
> +};
> +
> +static const struct attribute_group fwc_dev_group = {
> +       .bin_attrs = fwc_dev_bin_attrs,
> +};
> +
> +static const struct attribute_group *fwc_dev_groups[] = {
> +       &fwc_dev_group, NULL,
> +};
> +
> +static struct class fwc_class = {
> +       .name           = "fwcoredump",
> +       .dev_release    = fwc_dev_release,
> +       .dev_groups     = fwc_dev_groups,
> +};
> +
> +static ssize_t fwc_readv(char *buffer, loff_t offset, size_t count,
> +                        const void *data, size_t datalen)
> +{
> +       if (offset > datalen)
> +               return -EINVAL;
> +
> +       if (offset + count > datalen)
> +               count = datalen - offset;
> +
> +       if (count)
> +               memcpy(buffer, ((u8 *)data) + offset, count);
> +
> +       return count;
> +}
> +
> +/**
> + * fw_coredumpv - create firmware coredump with vmalloc data
> + * @dev: the struct device for the crashed device
> + * @data: vmalloc data containing the firmware coredump
> + * @datalen: length of the data
> + * @gfp: allocation flags
> + */
> +void fw_coredumpv(struct device *dev, const void *data, size_t datalen,
> +                 gfp_t gfp)
> +{
> +       fw_coredumpm(dev, NULL, data, datalen, gfp, fwc_readv, vfree);
> +}
> +EXPORT_SYMBOL(fw_coredumpv);
> +
> +/**
> + * fw_coredumpm - create firmware coredump with read/free methods
> + * @dev: the struct device for the crashed device
> + * @data: data cookie for the @read/@free functions
> + * @datalen: length of the data
> + * @gfp: allocation flags
> + * @read: function to read from the given buffer
> + * @free: function to free the given buffer
> + */
> +void fw_coredumpm(struct device *dev, struct module *owner,
> +                 const void *data, size_t datalen, gfp_t gfp,
> +                 ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> +                                 const void *data, size_t datalen),
> +                 void (*free)(const void *data))
> +{
> +       static atomic_t fwc_count = ATOMIC_INIT(0);
> +       struct fwc_entry *fwc;
> +
> +       if (!try_module_get(owner))
> +               return;
> +
> +       fwc = kzalloc(sizeof(*fwc), gfp);
> +       if (!fwc)
> +               goto put_module;
> +
> +       fwc->owner = owner;
> +       fwc->data = data;
> +       fwc->datalen = datalen;
> +       fwc->read = read;
> +       fwc->free = free;
> +
> +       device_initialize(&fwc->fwc_dev);
> +
> +       dev_set_name(&fwc->fwc_dev, "fwc%d", atomic_inc_return(&fwc_count));
> +       fwc->fwc_dev.class = &fwc_class;
> +
> +       if (device_add(&fwc->fwc_dev))
> +               goto put_device;
> +
> +       if (sysfs_create_link(&fwc->fwc_dev.kobj, &dev->kobj, "failing_device"))
> +               /* nothing - symlink will be missing but that's ok */;
> +
> +       INIT_DELAYED_WORK(&fwc->del_wk, fwc_del);
> +       schedule_delayed_work(&fwc->del_wk, FWC_TIMEOUT);
> +
> +       return;
> + put_device:
> +       put_device(&fwc->fwc_dev);
> + put_module:
> +       module_put(owner);
> +}
> +EXPORT_SYMBOL(fw_coredumpm);
> +
> +static int __init fwcoredump_init(void)
> +{
> +       return class_register(&fwc_class);
> +}
> +module_init(fwcoredump_init);
> +
> +static int fwc_free(struct device *dev, void *data)
> +{
> +       struct fwc_entry *fwc = dev_to_fwc(dev);
> +
> +       flush_delayed_work(&fwc->del_wk);
> +       return 0;
> +}
> +
> +static void __exit fwcoredump_exit(void)
> +{
> +       class_for_each_device(&fwc_class, NULL, NULL, fwc_free);
> +       class_unregister(&fwc_class);
> +}
> +module_exit(fwcoredump_exit);
> diff --git a/include/linux/fwcoredump.h b/include/linux/fwcoredump.h
> new file mode 100644
> index 000000000000..168f2d6abfc0
> --- /dev/null
> +++ b/include/linux/fwcoredump.h
> @@ -0,0 +1,35 @@
> +#ifndef __FWCOREDUMP_H
> +#define __FWCOREDUMP_H
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/vmalloc.h>
> +
> +#ifdef CONFIG_FW_COREDUMP
> +void fw_coredumpv(struct device *dev, const void *data, size_t datalen,
> +                 gfp_t gfp);
> +
> +void fw_coredumpm(struct device *dev, struct module *owner,
> +                 const void *data, size_t datalen, gfp_t gfp,
> +                 ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> +                                 const void *data, size_t datalen),
> +                 void (*free)(const void *data));
> +#else
> +static inline void fw_coredumpv(struct device *dev, const void *data,
> +                               size_t datalen, gfp_t gfp)
> +{
> +       vfree(data);
> +}
> +
> +static inline void
> +fw_coredumpm(struct device *dev, struct module *owner,
> +            const void *data, size_t datalen, gfp_t gfp,
> +            ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> +                            const void *data, size_t datalen),
> +            void (*free)(const void *data))
> +{
> +       free(data);
> +}
> +#endif /* CONFIG_FW_COREDUMP */
> +
> +#endif /* __FWCOREDUMP_H */
> --
> 2.1.0
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [RFC] firmware coredump: add new firmware coredump class

[Johannes Berg]

On Wed, 2014-09-03 at 16:19 +0200, Daniel Vetter wrote:
> [super-embarrassing resend, the previous one contained html gunk.]
> 
> If the idea is to also convert gpu crash dumps to this we should add
> dri-devel. And there the crashes are usually not due to firmware, but
> because the shaders and command batches userspace submitted have
> issues, so this should also be renamed to dev_coredump I think.

I don't know if the idea is to convert gpu crash dumps - I was just
wondering if you could and would want to use such a generic framework.
If the answer turns out to be no, that's perfectly reasonable I think.

However, renaming seems easy to do anyway :)

> On the overall design I wonder whether this shouldn't work more like a
> real core dump and dump to a real file. At least currently the dumps
> i915 creates are only useful as a general guide to where things went
> wrong, but if we actually want to submit them as traces to the
> hardware people we need to dump a _lot_ more. Otoh with the future of
> shared virtual address spaces between gpu/cpu we might just do a real
> core dump, so maybe this use case should be out of scope for your
> patch here.

I'm not really sure I'd want to actually sys_write() to a file here -
sounds like a big can of worms. If you have direct access (like shared
memory space) it seems we could still use the same mechanisms with the
coredumpm() method, no?

> On the logic itself I'm not sure whether the timeout is all that
> useful - at least in i915 our crash recovery works well enough that
> reporters often don't realize right away when it happened, but only
> later on when looking through logs to explain the tiny corruptions. If
> the crashdupm has evapored meanwhile that's not that useful.

Right. We might want to make it configurable, maybe even in Kconfig. I
was thinking that there would be userspace that would (automatically)
pick it up, and if such userspace doesn't exist or isn't running then
we'd want to free the memory eventually.

> Also, at least for gpus it's usually not interesting to grab
> subsequent dumps: Often the gpu is in a bad mood due to the first
> crash, or it's just a massive row of duplicated dumps. So in i915 we
> only record the first crash and keep it around forever. And tooling
> can still free it by writing to the file. This also ensures that we
> don't waste excessive amounts of memory with crash dumps.

Right, we discussed this but then I completely forgot. I think keeping
the first one is reasonable. If userspace has already picked it up
you'll still get multiple and maybe want to have a policy there as well.

> And if we want to use this for i915 we need some way for tools to go
> from the i915 drm class device node to the error state, not just from
> the error state back to the device.

Interesting. That's probably not all that difficult to do (maybe even
set up a child/parent relationship?) but I actually wanted to avoid a
hard dependency since there may be cases where the failing device
disappears, e.g. in the case of USB. I have to think about this case
more, I guess.

johannes

Re: [RFC] firmware coredump: add new firmware coredump class

[Daniel Vetter]

On Wed, Sep 3, 2014 at 9:05 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
>
>> And if we want to use this for i915 we need some way for tools to go
>> from the i915 drm class device node to the error state, not just from
>> the error state back to the device.
>
> Interesting. That's probably not all that difficult to do (maybe even
> set up a child/parent relationship?) but I actually wanted to avoid a
> hard dependency since there may be cases where the failing device
> disappears, e.g. in the case of USB. I have to think about this case
> more, I guess.

Yeah a hard parent/child won't because the device might drop off the
bus while committing seppuku. But I think we need to be able to follow
the link both ways.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [RFC] firmware coredump: add new firmware coredump class

[Seth Forshee]

On Wed, Sep 03, 2014 at 01:15:45PM +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Many devices run firmware, and as other software such firmware has
> bugs. When it misbehaves, however, it is often much harder to debug
> than software running on the host.
> 
> Introduce a "firmware coredump" mechanism to allow dumping internal
> firmware state through a generalized mechanism. As all devices are
> different and information needed can vary accordingly, this doesn't
> prescribe a file format - it just provides mechanism to get data to
> be able to capture it in a generalized way (e.g. in distributions.)
> 
> Note that generalized capturing of such data may result in privacy
> issues, so users generally need to be involved. In order to allow
> certain users/system integrators/... to disable the feature at all,
> introduce a Kconfig option to override the drivers that would like
> to have the feature.
> 
> For now, this provides two ways of dumping data:
>  1) with a vmalloc'ed area, that is then given to the fwcoredump
>     subsystem and freed after retrieval or timeout
>  2) with a generalized reader/free function method
> 
> We could/should add more options, e.g. a list of pages, since the
> vmalloc area is very limited on some architectures.

Overall I think this looks pretty sensible. The thing that worries me
though is firmware which might crash repeatedly in a short period of
time, resulting in a proliferation of coredumps and eating up all
available RAM. How about a limit of one active coredump per device or
something similar?

Thanks,
Seth

> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  MAINTAINERS                |   7 ++
>  drivers/base/Kconfig       |  21 +++++
>  drivers/base/Makefile      |   1 +
>  drivers/base/fwcoredump.c  | 222 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fwcoredump.h |  35 +++++++
>  5 files changed, 286 insertions(+)
>  create mode 100644 drivers/base/fwcoredump.c
>  create mode 100644 include/linux/fwcoredump.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2f85f55c8fb8..394bda1cde52 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3729,6 +3729,13 @@ F:	Documentation/firmware_class/
>  F:	drivers/base/firmware*.c
>  F:	include/linux/firmware.h
>  
> +FIRMWARE COREDUMP (fwcoredump)
> +M:	Johannes Berg <johannes@sipsolutions.net>
> +L:	linux-kernel@vger.kernel.org
> +S:	Maintained
> +F:	drivers/base/fwcoredump.c
> +F:	include/linux/fwcoredump.h
> +
>  FLASH ADAPTER DRIVER (IBM Flash Adapter 900GB Full Height PCI Flash Card)
>  M:	Joshua Morris <josh.h.morris@us.ibm.com>
>  M:	Philip Kelleher <pjk1939@linux.vnet.ibm.com>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 4e7f0ff83ae7..31eabab20c8a 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -165,6 +165,27 @@ config FW_LOADER_USER_HELPER_FALLBACK
>  
>  	  If you are unsure about this, say N here.
>  
> +config WANT_FW_COREDUMP
> +	bool
> +	help
> +	  Drivers should "select" this option if they desire to use the
> +	  firmware coredump mechanism.
> +
> +config DISABLE_FW_COREDUMP
> +	bool "Disable firmware coredump" if EXPERT
> +	help
> +	  Disable the firmware coredump mechanism despite drivers wanting to
> +	  use it; this allows for more sensitive systems or systems that
> +	  don't want to ever access the information to not have the code,
> +	  nor keep any data.
> +
> +	  If unsure, say N.
> +
> +config FW_COREDUMP
> +	bool
> +	default y if WANT_FW_COREDUMP
> +	depends on !DISABLE_FW_COREDUMP
> +
>  config DEBUG_DRIVER
>  	bool "Driver Core verbose debug messages"
>  	depends on DEBUG_KERNEL
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 4aab26ec0292..2af1be519653 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o
>  obj-$(CONFIG_REGMAP)	+= regmap/
>  obj-$(CONFIG_SOC_BUS) += soc.o
>  obj-$(CONFIG_PINCTRL) += pinctrl.o
> +obj-$(CONFIG_FW_COREDUMP) += fwcoredump.o
>  
>  ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
>  
> diff --git a/drivers/base/fwcoredump.c b/drivers/base/fwcoredump.c
> new file mode 100644
> index 000000000000..f70bef0727a9
> --- /dev/null
> +++ b/drivers/base/fwcoredump.c
> @@ -0,0 +1,222 @@
> +/******************************************************************************
> + *
> + * This file is provided under the GPLv2 license.
> + *
> + * GPL LICENSE SUMMARY
> + *
> + * Copyright(c) 2014 Intel Mobile Communications GmbH
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * The full GNU General Public License is included in this distribution
> + * in the file called COPYING.
> + *
> + * Contact Information:
> + *  Intel Linux Wireless <ilw@linux.intel.com>
> + * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
> + */
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/fwcoredump.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/workqueue.h>
> +
> +MODULE_AUTHOR("Johannes Berg <johannes@sipsolutions.net>");
> +MODULE_DESCRIPTION("Firmware Coredump support");
> +MODULE_LICENSE("GPL");
> +
> +/* if data isn't read by userspace after 5 minutes delete it */
> +#define FWC_TIMEOUT	(HZ * 60 * 5)
> +
> +struct fwc_entry {
> +	struct device fwc_dev;
> +	const void *data;
> +	size_t datalen;
> +	struct module *owner;
> +	ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> +			const void *data, size_t datalen);
> +	void (*free)(const void *data);
> +	struct delayed_work del_wk;
> +};
> +
> +static struct fwc_entry *dev_to_fwc(struct device *dev)
> +{
> +	return container_of(dev, struct fwc_entry, fwc_dev);
> +}
> +
> +static void fwc_dev_release(struct device *dev)
> +{
> +	struct fwc_entry *fwc = dev_to_fwc(dev);
> +
> +	fwc->free(fwc->data);
> +	kfree(fwc);
> +}
> +
> +static void fwc_del(struct work_struct *wk)
> +{
> +	struct fwc_entry *fwc;
> +
> +	fwc = container_of(wk, struct fwc_entry, del_wk.work);
> +
> +	device_del(&fwc->fwc_dev);
> +	put_device(&fwc->fwc_dev);
> +}
> +
> +static ssize_t fwc_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 fwc_entry *fwc = dev_to_fwc(dev);
> +
> +	return fwc->read(buffer, offset, count, fwc->data, fwc->datalen);
> +}
> +
> +static ssize_t fwc_data_write(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 fwc_entry *fwc = dev_to_fwc(dev);
> +
> +	schedule_delayed_work(&fwc->del_wk, 0);
> +
> +	return count;
> +}
> +
> +static struct bin_attribute fwc_attr_data = {
> +	.attr = { .name = "data", .mode = S_IRUSR | S_IWUSR, },
> +	.size = 0,
> +	.read = fwc_data_read,
> +	.write = fwc_data_write,
> +};
> +
> +static struct bin_attribute *fwc_dev_bin_attrs[] = {
> +	&fwc_attr_data, NULL,
> +};
> +
> +static const struct attribute_group fwc_dev_group = {
> +	.bin_attrs = fwc_dev_bin_attrs,
> +};
> +
> +static const struct attribute_group *fwc_dev_groups[] = {
> +	&fwc_dev_group, NULL,
> +};
> +
> +static struct class fwc_class = {
> +	.name		= "fwcoredump",
> +	.dev_release	= fwc_dev_release,
> +	.dev_groups	= fwc_dev_groups,
> +};
> +
> +static ssize_t fwc_readv(char *buffer, loff_t offset, size_t count,
> +			 const void *data, size_t datalen)
> +{
> +	if (offset > datalen)
> +		return -EINVAL;
> +
> +	if (offset + count > datalen)
> +		count = datalen - offset;
> +
> +	if (count)
> +		memcpy(buffer, ((u8 *)data) + offset, count);
> +
> +	return count;
> +}
> +
> +/**
> + * fw_coredumpv - create firmware coredump with vmalloc data
> + * @dev: the struct device for the crashed device
> + * @data: vmalloc data containing the firmware coredump
> + * @datalen: length of the data
> + * @gfp: allocation flags
> + */
> +void fw_coredumpv(struct device *dev, const void *data, size_t datalen,
> +		  gfp_t gfp)
> +{
> +	fw_coredumpm(dev, NULL, data, datalen, gfp, fwc_readv, vfree);
> +}
> +EXPORT_SYMBOL(fw_coredumpv);
> +
> +/**
> + * fw_coredumpm - create firmware coredump with read/free methods
> + * @dev: the struct device for the crashed device
> + * @data: data cookie for the @read/@free functions
> + * @datalen: length of the data
> + * @gfp: allocation flags
> + * @read: function to read from the given buffer
> + * @free: function to free the given buffer
> + */
> +void fw_coredumpm(struct device *dev, struct module *owner,
> +		  const void *data, size_t datalen, gfp_t gfp,
> +		  ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> +				  const void *data, size_t datalen),
> +		  void (*free)(const void *data))
> +{
> +	static atomic_t fwc_count = ATOMIC_INIT(0);
> +	struct fwc_entry *fwc;
> +
> +	if (!try_module_get(owner))
> +		return;
> +
> +	fwc = kzalloc(sizeof(*fwc), gfp);
> +	if (!fwc)
> +		goto put_module;
> +
> +	fwc->owner = owner;
> +	fwc->data = data;
> +	fwc->datalen = datalen;
> +	fwc->read = read;
> +	fwc->free = free;
> +
> +	device_initialize(&fwc->fwc_dev);
> +
> +	dev_set_name(&fwc->fwc_dev, "fwc%d", atomic_inc_return(&fwc_count));
> +	fwc->fwc_dev.class = &fwc_class;
> +
> +	if (device_add(&fwc->fwc_dev))
> +		goto put_device;
> +
> +	if (sysfs_create_link(&fwc->fwc_dev.kobj, &dev->kobj, "failing_device"))
> +		/* nothing - symlink will be missing but that's ok */;
> +
> +	INIT_DELAYED_WORK(&fwc->del_wk, fwc_del);
> +	schedule_delayed_work(&fwc->del_wk, FWC_TIMEOUT);
> +
> +	return;
> + put_device:
> +	put_device(&fwc->fwc_dev);
> + put_module:
> +	module_put(owner);
> +}
> +EXPORT_SYMBOL(fw_coredumpm);
> +
> +static int __init fwcoredump_init(void)
> +{
> +	return class_register(&fwc_class);
> +}
> +module_init(fwcoredump_init);
> +
> +static int fwc_free(struct device *dev, void *data)
> +{
> +	struct fwc_entry *fwc = dev_to_fwc(dev);
> +
> +	flush_delayed_work(&fwc->del_wk);
> +	return 0;
> +}
> +
> +static void __exit fwcoredump_exit(void)
> +{
> +	class_for_each_device(&fwc_class, NULL, NULL, fwc_free);
> +	class_unregister(&fwc_class);
> +}
> +module_exit(fwcoredump_exit);
> diff --git a/include/linux/fwcoredump.h b/include/linux/fwcoredump.h
> new file mode 100644
> index 000000000000..168f2d6abfc0
> --- /dev/null
> +++ b/include/linux/fwcoredump.h
> @@ -0,0 +1,35 @@
> +#ifndef __FWCOREDUMP_H
> +#define __FWCOREDUMP_H
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/vmalloc.h>
> +
> +#ifdef CONFIG_FW_COREDUMP
> +void fw_coredumpv(struct device *dev, const void *data, size_t datalen,
> +		  gfp_t gfp);
> +
> +void fw_coredumpm(struct device *dev, struct module *owner,
> +		  const void *data, size_t datalen, gfp_t gfp,
> +		  ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> +				  const void *data, size_t datalen),
> +		  void (*free)(const void *data));
> +#else
> +static inline void fw_coredumpv(struct device *dev, const void *data,
> +				size_t datalen, gfp_t gfp)
> +{
> +	vfree(data);
> +}
> +
> +static inline void
> +fw_coredumpm(struct device *dev, struct module *owner,
> +	     const void *data, size_t datalen, gfp_t gfp,
> +	     ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> +			     const void *data, size_t datalen),
> +	     void (*free)(const void *data))
> +{
> +	free(data);
> +}
> +#endif /* CONFIG_FW_COREDUMP */
> +
> +#endif /* __FWCOREDUMP_H */
> -- 
> 2.1.0
> 

Re: [RFC] firmware coredump: add new firmware coredump class

[Johannes Berg]

On Wed, 2014-09-03 at 09:38 -0500, Seth Forshee wrote:

> Overall I think this looks pretty sensible. The thing that worries me
> though is firmware which might crash repeatedly in a short period of
> time, resulting in a proliferation of coredumps and eating up all
> available RAM. How about a limit of one active coredump per device or
> something similar?

Oh, right, I completely forgot about that. Not sure what context I can
iterate the class device list, but worst case I have to add another
list.

We likely want to store the first dump then, I suppose?

johannes

Re: [RFC] firmware coredump: add new firmware coredump class

[Greg Kroah-Hartman]

On Wed, Sep 03, 2014 at 09:00:42PM +0200, Johannes Berg wrote:
> On Wed, 2014-09-03 at 09:38 -0500, Seth Forshee wrote:
> 
> > Overall I think this looks pretty sensible. The thing that worries me
> > though is firmware which might crash repeatedly in a short period of
> > time, resulting in a proliferation of coredumps and eating up all
> > available RAM. How about a limit of one active coredump per device or
> > something similar?
> 
> Oh, right, I completely forgot about that. Not sure what context I can
> iterate the class device list, but worst case I have to add another
> list.

Before you add another device, walk the list of all devices of the class
to see if there is already a device with the same parent.  If so, delete
that one and then continue and add the one you wanted to create.

> We likely want to store the first dump then, I suppose?

Or the last one?  I don't know...

greg k-h

Re: [RFC] firmware coredump: add new firmware coredump class

[Johannes Berg]

On Wed, 2014-09-03 at 12:15 -0700, Greg Kroah-Hartman wrote:
> On Wed, Sep 03, 2014 at 09:00:42PM +0200, Johannes Berg wrote:
> > On Wed, 2014-09-03 at 09:38 -0500, Seth Forshee wrote:
> > 
> > > Overall I think this looks pretty sensible. The thing that worries me
> > > though is firmware which might crash repeatedly in a short period of
> > > time, resulting in a proliferation of coredumps and eating up all
> > > available RAM. How about a limit of one active coredump per device or
> > > something similar?
> > 
> > Oh, right, I completely forgot about that. Not sure what context I can
> > iterate the class device list, but worst case I have to add another
> > list.
> 
> Before you add another device, walk the list of all devices of the class
> to see if there is already a device with the same parent.  If so, delete
> that one and then continue and add the one you wanted to create.

Or throw away the new data, yeah. The "context" part was just that I was
aiming to make this function callable say without being able to sleep
(hence also the gfp_t argument) but I don't know for sure I can iterate
the class device list that way. I can look it up though - no need for
anyone else to answer that question :)

> > We likely want to store the first dump then, I suppose?
> 
> Or the last one?  I don't know...

It might even have to be device-specific policy? But in general I'd
think that the first would make more sense because mostly we'd try to
recover from the issue, but if that results in another issue that's
usually not all that helpful to know about.

johannes