2018-01-10 13:54:09

by Arend Van Spriel

[permalink] [raw]
Subject: [PATCH V2 0/3] sysfs: allow user-space request for devcoredump

Since commit 833c95456a70 ("device coredump: add new device coredump class")
device drivers have a unified way to provide binary data obtained from a
failing_device to user-space. However, there may be use-cases in which the
driver has no reason to obtain the data, but user-space wants to initiate
it. This adds a coredump device attribute in sysfs when the driver bound to
the device supports the newly added coredump driver callback. As the
devcoredump API offers more than one option, eg. single buffer vs. scatter
list, it is left up to the driver how to invoke the devcoredump API. As an
example this series includes a driver implementation as RFC. There are other
drivers having some form of device firmware coredump that could benefit
from this, but decided to tackle that in another cycle.

These patches apply to the driver-core-next branch of the driver-core
repository.

Arend van Spriel (3):
sysfs: add attribute specification for /sysfs/devices/.../coredump
drivers: base: add coredump driver ops
brcmfmac: pcie: implement .coredump() driver callback

Documentation/ABI/testing/sysfs-devices-coredump | 10 ++++++
drivers/base/dd.c | 40 ++++++++++++++++++----
.../wireless/broadcom/brcm80211/brcmfmac/pcie.c | 22 ++++++++++++
include/linux/device.h | 2 +-
4 files changed, 66 insertions(+), 8 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-devices-coredump

--
1.9.1


2018-01-10 13:54:08

by Arend Van Spriel

[permalink] [raw]
Subject: [PATCH V2 2/3] drivers: base: add coredump driver ops

This adds the coredump driver operation. When the driver defines it
a coredump file is added in the sysfs folder of the device upon
driver binding. The file is removed when the driver is unbound.
User-space can trigger a coredump for this device by echo'ing to
the coredump file. Instead of obtaining raw coredump data from
the driver it is left to the coredump driver operation to invoke
the devcoredump API as there are more ways to do that.

Signed-off-by: Arend van Spriel <[email protected]>
---
V2:
- extended the commit message.
---
drivers/base/dd.c | 40 +++++++++++++++++++++++++++++++++-------
include/linux/device.h | 2 +-
2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 533c82f..de6fd09 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -288,6 +288,18 @@ static void driver_bound(struct device *dev)
kobject_uevent(&dev->kobj, KOBJ_BIND);
}

+static ssize_t coredump_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ device_lock(dev);
+ if (dev->driver->coredump)
+ dev->driver->coredump(dev);
+ device_unlock(dev);
+
+ return count;
+}
+static DEVICE_ATTR_WO(coredump);
+
static int driver_sysfs_add(struct device *dev)
{
int ret;
@@ -297,14 +309,26 @@ static int driver_sysfs_add(struct device *dev)
BUS_NOTIFY_BIND_DRIVER, dev);

ret = sysfs_create_link(&dev->driver->p->kobj, &dev->kobj,
+ kobject_name(&dev->kobj));
+ if (ret)
+ goto fail;
+
+ ret = sysfs_create_link(&dev->kobj, &dev->driver->p->kobj,
+ "driver");
+ if (ret)
+ goto rm_dev;
+
+ if (!IS_ENABLED(CONFIG_DEV_COREDUMP) || !dev->driver->coredump ||
+ !device_create_file(dev, &dev_attr_coredump))
+ return 0;
+
+ sysfs_remove_link(&dev->kobj, "driver");
+
+rm_dev:
+ sysfs_remove_link(&dev->driver->p->kobj,
kobject_name(&dev->kobj));
- if (ret == 0) {
- ret = sysfs_create_link(&dev->kobj, &dev->driver->p->kobj,
- "driver");
- if (ret)
- sysfs_remove_link(&dev->driver->p->kobj,
- kobject_name(&dev->kobj));
- }
+
+fail:
return ret;
}

@@ -313,6 +337,8 @@ static void driver_sysfs_remove(struct device *dev)
struct device_driver *drv = dev->driver;

if (drv) {
+ if (drv->coredump)
+ device_remove_file(dev, &dev_attr_coredump);
sysfs_remove_link(&drv->p->kobj, kobject_name(&dev->kobj));
sysfs_remove_link(&dev->kobj, "driver");
}
diff --git a/include/linux/device.h b/include/linux/device.h
index 46cece5..cd3b47e 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -287,6 +287,7 @@ struct device_driver {
const struct attribute_group **groups;

const struct dev_pm_ops *pm;
+ int (*coredump) (struct device *dev);

struct driver_private *p;
};
@@ -300,7 +301,6 @@ extern struct device_driver *driver_find(const char *name,
extern int driver_probe_done(void);
extern void wait_for_device_probe(void);

-
/* sysfs interface for exporting driver attributes */

struct driver_attribute {
--
1.9.1

2018-01-10 13:54:06

by Arend Van Spriel

[permalink] [raw]
Subject: [PATCH V2 1/3] sysfs: add attribute specification for /sysfs/devices/.../coredump

This patch adds the specification for /sysfs/devices/.../coredump
which allows user-space to trigger a device coredump obtaining
binary data from the device for (fault) analysis. It relies on
CONFIG_DEV_COREDUMP being enabled.

Signed-off-by: Arend van Spriel <[email protected]>
---
Documentation/ABI/testing/sysfs-devices-coredump | 10 ++++++++++
1 file changed, 10 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-devices-coredump

diff --git a/Documentation/ABI/testing/sysfs-devices-coredump b/Documentation/ABI/testing/sysfs-devices-coredump
new file mode 100644
index 0000000..5989255
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-coredump
@@ -0,0 +1,10 @@
+What: /sys/devices/.../coredump
+Date: December 2017
+Contact: Arend van Spriel <[email protected]>
+Description:
+ The /sys/devices/.../coredump attribute is only present when the
+ device is bound to a driver, which provides the .coredump()
+ callback. The attribute is write only. Anything written to this
+ file will trigger the .coredump() callback.
+
+ Available when CONFIG_DEV_COREDUMP is enabled.
--
1.9.1

2018-01-10 13:54:45

by Arend Van Spriel

[permalink] [raw]
Subject: [RFC 3/3] brcmfmac: pcie: implement .coredump() driver callback

This implements the .coredump() driver callback obtaining binary data
from the device and using devcoredump function to expose it in sysfs.

Signed-off-by: Arend van Spriel <[email protected]>
---
.../wireless/broadcom/brcm80211/brcmfmac/pcie.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 3c87157..55d0d25 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -22,6 +22,7 @@
#include <linux/interrupt.h>
#include <linux/bcma/bcma.h>
#include <linux/sched.h>
+#include <linux/devcoredump.h>
#include <asm/unaligned.h>

#include <soc.h>
@@ -1744,6 +1745,26 @@ static void brcmf_pcie_setup(struct device *dev, int ret,
device_release_driver(dev);
}

+static int brcmf_pcie_dev_coredump(struct device *dev)
+{
+ struct brcmf_bus *bus_if = dev_get_drvdata(dev);
+ struct brcmf_pciedev *buspub = bus_if->bus_priv.pcie;
+ struct brcmf_pciedev_info *devinfo = buspub->devinfo;
+ size_t len;
+ void *dump;
+
+ len = devinfo->ci->ramsize - devinfo->ci->srsize;
+ dump = vzalloc(len);
+ if (!dump)
+ return -ENOMEM;
+
+ brcmf_dbg(PCIE, "dump at 0x%08X: len=%zu\n", devinfo->ci->rambase, len);
+ brcmf_pcie_copy_dev_tomem(devinfo, devinfo->ci->rambase, dump, len);
+
+ dev_coredumpv(dev, dump, len, GFP_KERNEL);
+ return 0;
+}
+
static int
brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
@@ -2002,6 +2023,7 @@ static int brcmf_pcie_pm_leave_D3(struct device *dev)
.id_table = brcmf_pcie_devid_table,
.probe = brcmf_pcie_probe,
.remove = brcmf_pcie_remove,
+ .driver.coredump = brcmf_pcie_dev_coredump,
#ifdef CONFIG_PM
.driver.pm = &brcmf_pciedrvr_pm,
#endif
--
1.9.1

2018-01-11 08:34:53

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH V2 0/3] sysfs: allow user-space request for devcoredump

On Wed, Jan 10, 2018 at 2:53 PM, Arend van Spriel <[email protected]> wrote:
> Since commit 833c95456a70 ("device coredump: add new device coredump class")
> device drivers have a unified way to provide binary data obtained from a
> failing_device to user-space. However, there may be use-cases in which the
> driver has no reason to obtain the data, but user-space wants to initiate
> it. This adds a coredump device attribute in sysfs when the driver bound to
> the device supports the newly added coredump driver callback. As the
> devcoredump API offers more than one option, eg. single buffer vs. scatter
> list, it is left up to the driver how to invoke the devcoredump API. As an
> example this series includes a driver implementation as RFC. There are other
> drivers having some form of device firmware coredump that could benefit
> from this, but decided to tackle that in another cycle.
>
> These patches apply to the driver-core-next branch of the driver-core
> repository.

Please drop this series. I have V3 coming with minor update.

Regards,
Arend