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.
-DanielOn Wed, Sep 3, 2014 at 1:15 PM, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote:
From: Johannes Berg <johannes.berg@xxxxxxxxx>
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@xxxxxxxxx>
---
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@xxxxxxxxxxxxxxxx>
+L: linux-kernel@xxxxxxxxxxxxxxx
+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@xxxxxxxxxx>
M: Philip Kelleher <pjk1939@xxxxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxx>
+ * 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@xxxxxxxxxxxxxxxx>");
+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 = ""> + 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
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
