* [PATCH v5 2/5] devcoredump: Add per device sysfs entry to enable/disable coredump
2022-08-10 16:00 [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled Manish Mandlik
@ 2022-08-10 16:00 ` Manish Mandlik
2022-08-10 16:00 ` [PATCH v5 3/5] Bluetooth: Add support for hci devcoredump Manish Mandlik
` (5 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Manish Mandlik @ 2022-08-10 16:00 UTC (permalink / raw)
To: Arend van Spriel, Greg Kroah-Hartman, marcel, luiz.dentz
Cc: Johannes Berg, Dan Williams, Jason Gunthorpe,
Signed-off-by : Manish Mandlik, linux-bluetooth, Thomas Gleixner,
Rafael J . Wysocki, chromeos-bluetooth-upstreaming, Won Chung,
Abhishek Pandit-Subedi, Ira Weiny, linux-kernel
The /sys/class/devcoredump/disabled provides only one-way disable
functionality. Also, disabling devcoredump using it disables the
devcoredump functionality for everyone who is using it.
Provide a way to selectively enable/disable devcoredump for the device
which is bound to a driver that implements the '.coredump()' callback.
This adds the 'coredump_disabled' driver attribute. When the driver
implements the '.coredump()' callback, 'coredump_disabled' file is added
along with the 'coredump' file in the sysfs folder of the device upon
driver binding. The file is removed when the driver is unbound.
Drivers can use this attribute to enable/disable devcoredump and the
userspace can write 0 or 1 to /sys/devices/.../coredump_disabled sysfs
entry to control enabling/disabling of devcoredump for that device.
Signed-off-by: Manish Mandlik <mmandlik@google.com>
Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---
Changes in v5:
- Use sysfs_emit(), kstrtobool() and attribute groups
- Move 'coredump_disabled' at appropriate location in 'struct device'
Changes in v4:
- New patch in the series
drivers/base/dd.c | 33 +++++++++++++++++++++++++++++++--
drivers/base/devcoredump.c | 2 +-
include/linux/device.h | 3 +++
3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 11b0fb6414d3..fa01901983c8 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -426,6 +426,35 @@ static ssize_t coredump_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_WO(coredump);
+static ssize_t coredump_disabled_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%d\n", dev->coredump_disabled);
+}
+
+static ssize_t coredump_disabled_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ bool disabled;
+
+ if (kstrtobool(buf, &disabled) < 0)
+ return -EINVAL;
+
+ dev->coredump_disabled = disabled;
+
+ return count;
+}
+static DEVICE_ATTR_RW(coredump_disabled);
+
+static struct attribute *dev_coredump_attrs[] = {
+ &dev_attr_coredump.attr,
+ &dev_attr_coredump_disabled.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(dev_coredump);
+
static int driver_sysfs_add(struct device *dev)
{
int ret;
@@ -447,7 +476,7 @@ static int driver_sysfs_add(struct device *dev)
if (!IS_ENABLED(CONFIG_DEV_COREDUMP) || !dev->driver->coredump)
return 0;
- ret = device_create_file(dev, &dev_attr_coredump);
+ ret = device_add_groups(dev, dev_coredump_groups);
if (!ret)
return 0;
@@ -467,7 +496,7 @@ static void driver_sysfs_remove(struct device *dev)
if (drv) {
if (drv->coredump)
- device_remove_file(dev, &dev_attr_coredump);
+ device_remove_groups(dev, dev_coredump_groups);
sysfs_remove_link(&drv->p->kobj, kobject_name(&dev->kobj));
sysfs_remove_link(&dev->kobj, "driver");
}
diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
index f4d794d6bb85..c5e9af9f3181 100644
--- a/drivers/base/devcoredump.c
+++ b/drivers/base/devcoredump.c
@@ -255,7 +255,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
struct devcd_entry *devcd;
struct device *existing;
- if (devcd_disabled)
+ if (devcd_disabled || dev->coredump_disabled)
goto free;
existing = class_find_device(&devcd_class, NULL, dev,
diff --git a/include/linux/device.h b/include/linux/device.h
index dc941997795c..41aedc74a5a8 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -526,6 +526,8 @@ struct device_physical_location {
* should be set by the subsystem / bus driver that discovered
* the device.
*
+ * @coredump_disabled: Can be used to selectively enable/disable the coredump
+ * functionality for a particular device via sysfs entry.
* @offline_disabled: If set, the device is permanently online.
* @offline: Set after successful invocation of bus type's .offline().
* @of_node_reused: Set if the device-tree node is shared with an ancestor
@@ -637,6 +639,7 @@ struct device {
enum device_removable removable;
+ bool coredump_disabled:1;
bool offline_disabled:1;
bool offline:1;
bool of_node_reused:1;
--
2.37.1.559.g78731f0fdb-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5 3/5] Bluetooth: Add support for hci devcoredump
2022-08-10 16:00 [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled Manish Mandlik
2022-08-10 16:00 ` [PATCH v5 2/5] devcoredump: Add per device sysfs entry to enable/disable coredump Manish Mandlik
@ 2022-08-10 16:00 ` Manish Mandlik
2022-08-10 16:19 ` Greg Kroah-Hartman
` (2 more replies)
2022-08-10 16:00 ` [PATCH v5 4/5] Bluetooth: btusb: Add btusb devcoredump support Manish Mandlik
` (4 subsequent siblings)
6 siblings, 3 replies; 16+ messages in thread
From: Manish Mandlik @ 2022-08-10 16:00 UTC (permalink / raw)
To: Arend van Spriel, Greg Kroah-Hartman, marcel, luiz.dentz
Cc: Johannes Berg, Dan Williams, Jason Gunthorpe,
Signed-off-by : Manish Mandlik, linux-bluetooth, Thomas Gleixner,
Rafael J . Wysocki, chromeos-bluetooth-upstreaming, Won Chung,
Abhishek Pandit-Subedi, David S. Miller, Eric Dumazet,
Jakub Kicinski, Johan Hedberg, Paolo Abeni, linux-kernel, netdev
From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Add devcoredump APIs to hci core so that drivers only have to provide
the dump skbs instead of managing the synchronization and timeouts.
The devcoredump APIs should be used in the following manner:
- hci_devcoredump_init is called to allocate the dump.
- hci_devcoredump_append is called to append any skbs with dump data
OR hci_devcoredump_append_pattern is called to insert a pattern.
- hci_devcoredump_complete is called when all dump packets have been
sent OR hci_devcoredump_abort is called to indicate an error and
cancel an ongoing dump collection.
The high level APIs just prepare some skbs with the appropriate data and
queue it for the dump to process. Packets part of the crashdump can be
intercepted in the driver in interrupt context and forwarded directly to
the devcoredump APIs.
Internally, there are 5 states for the dump: idle, active, complete,
abort and timeout. A devcoredump will only be in active state after it
has been initialized. Once active, it accepts data to be appended,
patterns to be inserted (i.e. memset) and a completion event or an abort
event to generate a devcoredump. The timeout is initialized at the same
time the dump is initialized (defaulting to 10s) and will be cleared
either when the timeout occurs or the dump is complete or aborted.
Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Signed-off-by: Manish Mandlik <mmandlik@google.com>
Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---
(no changes since v4)
Changes in v4:
- Add .enabled() and .coredump() to hci_devcoredump struct
Changes in v3:
- Add attribute to enable/disable and set default state to disabled
Changes in v2:
- Move hci devcoredump implementation to new files
- Move dump queue and dump work to hci_devcoredump struct
- Add CONFIG_DEV_COREDUMP conditional compile
include/net/bluetooth/coredump.h | 119 +++++++
include/net/bluetooth/hci_core.h | 5 +
net/bluetooth/Makefile | 2 +
net/bluetooth/coredump.c | 524 +++++++++++++++++++++++++++++++
net/bluetooth/hci_core.c | 9 +
net/bluetooth/hci_sync.c | 2 +
6 files changed, 661 insertions(+)
create mode 100644 include/net/bluetooth/coredump.h
create mode 100644 net/bluetooth/coredump.c
diff --git a/include/net/bluetooth/coredump.h b/include/net/bluetooth/coredump.h
new file mode 100644
index 000000000000..be09290927c0
--- /dev/null
+++ b/include/net/bluetooth/coredump.h
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Google Corporation
+ */
+
+#ifndef __COREDUMP_H
+#define __COREDUMP_H
+
+#define DEVCOREDUMP_TIMEOUT msecs_to_jiffies(10000) /* 10 sec */
+
+typedef bool (*coredump_enabled_t)(struct hci_dev *hdev);
+typedef void (*coredump_t)(struct hci_dev *hdev);
+typedef int (*dmp_hdr_t)(struct hci_dev *hdev, char *buf, size_t size);
+typedef void (*notify_change_t)(struct hci_dev *hdev, int state);
+
+/* struct hci_devcoredump - Devcoredump state
+ *
+ * @supported: Indicates if FW dump collection is supported by driver
+ * @state: Current state of dump collection
+ * @alloc_size: Total size of the dump
+ * @head: Start of the dump
+ * @tail: Pointer to current end of dump
+ * @end: head + alloc_size for easy comparisons
+ *
+ * @dump_q: Dump queue for state machine to process
+ * @dump_rx: Devcoredump state machine work
+ * @dump_timeout: Devcoredump timeout work
+ *
+ * @enabled: Checks if the devcoredump is enabled for the device
+ *
+ * @coredump: Called from the driver's .coredump() function.
+ * @dmp_hdr: Create a dump header to identify controller/fw/driver info
+ * @notify_change: Notify driver when devcoredump state has changed
+ */
+struct hci_devcoredump {
+ bool supported;
+
+ enum devcoredump_state {
+ HCI_DEVCOREDUMP_IDLE,
+ HCI_DEVCOREDUMP_ACTIVE,
+ HCI_DEVCOREDUMP_DONE,
+ HCI_DEVCOREDUMP_ABORT,
+ HCI_DEVCOREDUMP_TIMEOUT
+ } state;
+
+ size_t alloc_size;
+ char *head;
+ char *tail;
+ char *end;
+
+ struct sk_buff_head dump_q;
+ struct work_struct dump_rx;
+ struct delayed_work dump_timeout;
+
+ coredump_enabled_t enabled;
+
+ coredump_t coredump;
+ dmp_hdr_t dmp_hdr;
+ notify_change_t notify_change;
+};
+
+#ifdef CONFIG_DEV_COREDUMP
+
+void hci_devcoredump_reset(struct hci_dev *hdev);
+void hci_devcoredump_rx(struct work_struct *work);
+void hci_devcoredump_timeout(struct work_struct *work);
+
+int hci_devcoredump_register(struct hci_dev *hdev, coredump_t coredump,
+ dmp_hdr_t dmp_hdr, notify_change_t notify_change);
+int hci_devcoredump_init(struct hci_dev *hdev, u32 dmp_size);
+int hci_devcoredump_append(struct hci_dev *hdev, struct sk_buff *skb);
+int hci_devcoredump_append_pattern(struct hci_dev *hdev, u8 pattern, u32 len);
+int hci_devcoredump_complete(struct hci_dev *hdev);
+int hci_devcoredump_abort(struct hci_dev *hdev);
+
+#else
+
+static inline void hci_devcoredump_reset(struct hci_dev *hdev) {}
+static inline void hci_devcoredump_rx(struct work_struct *work) {}
+static inline void hci_devcoredump_timeout(struct work_struct *work) {}
+
+static inline int hci_devcoredump_register(struct hci_dev *hdev,
+ coredump_t coredump,
+ dmp_hdr_t dmp_hdr,
+ notify_change_t notify_change)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int hci_devcoredump_init(struct hci_dev *hdev, u32 dmp_size)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int hci_devcoredump_append(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int hci_devcoredump_append_pattern(struct hci_dev *hdev,
+ u8 pattern, u32 len)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int hci_devcoredump_complete(struct hci_dev *hdev)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int hci_devcoredump_abort(struct hci_dev *hdev)
+{
+ return -EOPNOTSUPP;
+}
+
+#endif /* CONFIG_DEV_COREDUMP */
+
+#endif /* __COREDUMP_H */
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e7862903187d..fb5ef1c6dd10 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -32,6 +32,7 @@
#include <net/bluetooth/hci.h>
#include <net/bluetooth/hci_sync.h>
#include <net/bluetooth/hci_sock.h>
+#include <net/bluetooth/coredump.h>
/* HCI priority */
#define HCI_PRIO_MAX 7
@@ -585,6 +586,10 @@ struct hci_dev {
const char *fw_info;
struct dentry *debugfs;
+#ifdef CONFIG_DEV_COREDUMP
+ struct hci_devcoredump dump;
+#endif
+
struct device dev;
struct rfkill *rfkill;
diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
index 0e7b7db42750..141ac1fda0bf 100644
--- a/net/bluetooth/Makefile
+++ b/net/bluetooth/Makefile
@@ -17,6 +17,8 @@ bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \
ecdh_helper.o hci_request.o mgmt_util.o mgmt_config.o hci_codec.o \
eir.o hci_sync.o
+bluetooth-$(CONFIG_DEV_COREDUMP) += coredump.o
+
bluetooth-$(CONFIG_BT_BREDR) += sco.o
bluetooth-$(CONFIG_BT_LE) += iso.o
bluetooth-$(CONFIG_BT_HS) += a2mp.o amp.o
diff --git a/net/bluetooth/coredump.c b/net/bluetooth/coredump.c
new file mode 100644
index 000000000000..b412056457c8
--- /dev/null
+++ b/net/bluetooth/coredump.c
@@ -0,0 +1,524 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Google Corporation
+ */
+
+#include <linux/devcoredump.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+enum hci_devcoredump_pkt_type {
+ HCI_DEVCOREDUMP_PKT_INIT,
+ HCI_DEVCOREDUMP_PKT_SKB,
+ HCI_DEVCOREDUMP_PKT_PATTERN,
+ HCI_DEVCOREDUMP_PKT_COMPLETE,
+ HCI_DEVCOREDUMP_PKT_ABORT,
+};
+
+struct hci_devcoredump_skb_cb {
+ u16 pkt_type;
+};
+
+struct hci_devcoredump_skb_pattern {
+ u8 pattern;
+ u32 len;
+} __packed;
+
+#define hci_dmp_cb(skb) ((struct hci_devcoredump_skb_cb *)((skb)->cb))
+
+#define MAX_DEVCOREDUMP_HDR_SIZE 512 /* bytes */
+
+static int hci_devcoredump_update_hdr_state(char *buf, size_t size, int state)
+{
+ if (!buf)
+ return 0;
+
+ return snprintf(buf, size, "Bluetooth devcoredump\nState: %d\n", state);
+}
+
+/* Call with hci_dev_lock only. */
+static int hci_devcoredump_update_state(struct hci_dev *hdev, int state)
+{
+ hdev->dump.state = state;
+
+ return hci_devcoredump_update_hdr_state(hdev->dump.head,
+ hdev->dump.alloc_size, state);
+}
+
+static int hci_devcoredump_mkheader(struct hci_dev *hdev, char *buf,
+ size_t buf_size)
+{
+ char *ptr = buf;
+ size_t rem = buf_size;
+ size_t read = 0;
+
+ read = hci_devcoredump_update_hdr_state(ptr, rem, HCI_DEVCOREDUMP_IDLE);
+ read += 1; /* update_hdr_state adds \0 at the end upon state rewrite */
+ rem -= read;
+ ptr += read;
+
+ if (hdev->dump.dmp_hdr) {
+ /* dmp_hdr() should return number of bytes written */
+ read = hdev->dump.dmp_hdr(hdev, ptr, rem);
+ rem -= read;
+ ptr += read;
+ }
+
+ read = snprintf(ptr, rem, "--- Start dump ---\n");
+ rem -= read;
+ ptr += read;
+
+ return buf_size - rem;
+}
+
+/* Do not call with hci_dev_lock since this calls driver code. */
+static void hci_devcoredump_notify(struct hci_dev *hdev, int state)
+{
+ if (hdev->dump.notify_change)
+ hdev->dump.notify_change(hdev, state);
+}
+
+/* Call with hci_dev_lock only. */
+void hci_devcoredump_reset(struct hci_dev *hdev)
+{
+ hdev->dump.head = NULL;
+ hdev->dump.tail = NULL;
+ hdev->dump.alloc_size = 0;
+
+ hci_devcoredump_update_state(hdev, HCI_DEVCOREDUMP_IDLE);
+
+ cancel_delayed_work(&hdev->dump.dump_timeout);
+ skb_queue_purge(&hdev->dump.dump_q);
+}
+
+/* Call with hci_dev_lock only. */
+static void hci_devcoredump_free(struct hci_dev *hdev)
+{
+ if (hdev->dump.head)
+ vfree(hdev->dump.head);
+
+ hci_devcoredump_reset(hdev);
+}
+
+/* Call with hci_dev_lock only. */
+static int hci_devcoredump_alloc(struct hci_dev *hdev, u32 size)
+{
+ hdev->dump.head = vmalloc(size);
+ if (!hdev->dump.head)
+ return -ENOMEM;
+
+ hdev->dump.alloc_size = size;
+ hdev->dump.tail = hdev->dump.head;
+ hdev->dump.end = hdev->dump.head + size;
+
+ hci_devcoredump_update_state(hdev, HCI_DEVCOREDUMP_IDLE);
+
+ return 0;
+}
+
+/* Call with hci_dev_lock only. */
+static bool hci_devcoredump_copy(struct hci_dev *hdev, char *buf, u32 size)
+{
+ if (hdev->dump.tail + size > hdev->dump.end)
+ return false;
+
+ memcpy(hdev->dump.tail, buf, size);
+ hdev->dump.tail += size;
+
+ return true;
+}
+
+/* Call with hci_dev_lock only. */
+static bool hci_devcoredump_memset(struct hci_dev *hdev, u8 pattern, u32 len)
+{
+ if (hdev->dump.tail + len > hdev->dump.end)
+ return false;
+
+ memset(hdev->dump.tail, pattern, len);
+ hdev->dump.tail += len;
+
+ return true;
+}
+
+/* Call with hci_dev_lock only. */
+static int hci_devcoredump_prepare(struct hci_dev *hdev, u32 dump_size)
+{
+ char *dump_hdr;
+ int dump_hdr_size;
+ u32 size;
+ int err = 0;
+
+ dump_hdr = vmalloc(MAX_DEVCOREDUMP_HDR_SIZE);
+ if (!dump_hdr) {
+ err = -ENOMEM;
+ goto hdr_free;
+ }
+
+ dump_hdr_size = hci_devcoredump_mkheader(hdev, dump_hdr,
+ MAX_DEVCOREDUMP_HDR_SIZE);
+ size = dump_hdr_size + dump_size;
+
+ if (hci_devcoredump_alloc(hdev, size)) {
+ err = -ENOMEM;
+ goto hdr_free;
+ }
+
+ /* Insert the device header */
+ if (!hci_devcoredump_copy(hdev, dump_hdr, dump_hdr_size)) {
+ bt_dev_err(hdev, "Failed to insert header");
+ hci_devcoredump_free(hdev);
+
+ err = -ENOMEM;
+ goto hdr_free;
+ }
+
+hdr_free:
+ if (dump_hdr)
+ vfree(dump_hdr);
+
+ return err;
+}
+
+/* Bluetooth devcoredump state machine.
+ *
+ * Devcoredump states:
+ *
+ * HCI_DEVCOREDUMP_IDLE: The default state.
+ *
+ * HCI_DEVCOREDUMP_ACTIVE: A devcoredump will be in this state once it has
+ * been initialized using hci_devcoredump_init(). Once active, the
+ * driver can append data using hci_devcoredump_append() or insert
+ * a pattern using hci_devcoredump_append_pattern().
+ *
+ * HCI_DEVCOREDUMP_DONE: Once the dump collection is complete, the drive
+ * can signal the completion using hci_devcoredump_complete(). A
+ * devcoredump is generated indicating the completion event and
+ * then the state machine is reset to the default state.
+ *
+ * HCI_DEVCOREDUMP_ABORT: The driver can cancel ongoing dump collection in
+ * case of any error using hci_devcoredump_abort(). A devcoredump
+ * is still generated with the available data indicating the abort
+ * event and then the state machine is reset to the default state.
+ *
+ * HCI_DEVCOREDUMP_TIMEOUT: A timeout timer for HCI_DEVCOREDUMP_TIMEOUT sec
+ * is started during devcoredump initialization. Once the timeout
+ * occurs, the driver is notified, a devcoredump is generated with
+ * the available data indicating the timeout event and then the
+ * state machine is reset to the default state.
+ *
+ * The driver must register using hci_devcoredump_register() before using the
+ * hci devcoredump APIs.
+ */
+void hci_devcoredump_rx(struct work_struct *work)
+{
+ struct hci_dev *hdev = container_of(work, struct hci_dev, dump.dump_rx);
+ struct sk_buff *skb;
+ struct hci_devcoredump_skb_pattern *pattern;
+ u32 dump_size;
+ int start_state;
+
+#define DBG_UNEXPECTED_STATE() \
+ bt_dev_dbg(hdev, \
+ "Unexpected packet (%d) for state (%d). ", \
+ hci_dmp_cb(skb)->pkt_type, hdev->dump.state)
+
+ while ((skb = skb_dequeue(&hdev->dump.dump_q))) {
+ hci_dev_lock(hdev);
+ start_state = hdev->dump.state;
+
+ switch (hci_dmp_cb(skb)->pkt_type) {
+ case HCI_DEVCOREDUMP_PKT_INIT:
+ if (hdev->dump.state != HCI_DEVCOREDUMP_IDLE) {
+ DBG_UNEXPECTED_STATE();
+ goto loop_continue;
+ }
+
+ if (skb->len != sizeof(dump_size)) {
+ bt_dev_dbg(hdev, "Invalid dump init pkt");
+ goto loop_continue;
+ }
+
+ dump_size = *((u32 *)skb->data);
+ if (!dump_size) {
+ bt_dev_err(hdev, "Zero size dump init pkt");
+ goto loop_continue;
+ }
+
+ if (hci_devcoredump_prepare(hdev, dump_size)) {
+ bt_dev_err(hdev, "Failed to prepare for dump");
+ goto loop_continue;
+ }
+
+ hci_devcoredump_update_state(hdev,
+ HCI_DEVCOREDUMP_ACTIVE);
+ queue_delayed_work(hdev->workqueue,
+ &hdev->dump.dump_timeout,
+ DEVCOREDUMP_TIMEOUT);
+ break;
+
+ case HCI_DEVCOREDUMP_PKT_SKB:
+ if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) {
+ DBG_UNEXPECTED_STATE();
+ goto loop_continue;
+ }
+
+ if (!hci_devcoredump_copy(hdev, skb->data, skb->len))
+ bt_dev_dbg(hdev, "Failed to insert skb");
+ break;
+
+ case HCI_DEVCOREDUMP_PKT_PATTERN:
+ if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) {
+ DBG_UNEXPECTED_STATE();
+ goto loop_continue;
+ }
+
+ if (skb->len != sizeof(*pattern)) {
+ bt_dev_dbg(hdev, "Invalid pattern skb");
+ goto loop_continue;
+ }
+
+ pattern = (void *)skb->data;
+
+ if (!hci_devcoredump_memset(hdev, pattern->pattern,
+ pattern->len))
+ bt_dev_dbg(hdev, "Failed to set pattern");
+ break;
+
+ case HCI_DEVCOREDUMP_PKT_COMPLETE:
+ if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) {
+ DBG_UNEXPECTED_STATE();
+ goto loop_continue;
+ }
+
+ hci_devcoredump_update_state(hdev,
+ HCI_DEVCOREDUMP_DONE);
+ dump_size = hdev->dump.tail - hdev->dump.head;
+
+ bt_dev_info(hdev,
+ "Devcoredump complete with size %u "
+ "(expect %u)",
+ dump_size, hdev->dump.alloc_size);
+
+ dev_coredumpv(&hdev->dev, hdev->dump.head, dump_size,
+ GFP_KERNEL);
+ break;
+
+ case HCI_DEVCOREDUMP_PKT_ABORT:
+ if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) {
+ DBG_UNEXPECTED_STATE();
+ goto loop_continue;
+ }
+
+ hci_devcoredump_update_state(hdev,
+ HCI_DEVCOREDUMP_ABORT);
+ dump_size = hdev->dump.tail - hdev->dump.head;
+
+ bt_dev_info(hdev,
+ "Devcoredump aborted with size %u "
+ "(expect %u)",
+ dump_size, hdev->dump.alloc_size);
+
+ /* Emit a devcoredump with the available data */
+ dev_coredumpv(&hdev->dev, hdev->dump.head, dump_size,
+ GFP_KERNEL);
+ break;
+
+ default:
+ bt_dev_dbg(hdev,
+ "Unknown packet (%d) for state (%d). ",
+ hci_dmp_cb(skb)->pkt_type, hdev->dump.state);
+ break;
+ }
+
+loop_continue:
+ kfree_skb(skb);
+ hci_dev_unlock(hdev);
+
+ if (start_state != hdev->dump.state)
+ hci_devcoredump_notify(hdev, hdev->dump.state);
+
+ hci_dev_lock(hdev);
+ if (hdev->dump.state == HCI_DEVCOREDUMP_DONE ||
+ hdev->dump.state == HCI_DEVCOREDUMP_ABORT)
+ hci_devcoredump_reset(hdev);
+ hci_dev_unlock(hdev);
+ }
+}
+EXPORT_SYMBOL(hci_devcoredump_rx);
+
+void hci_devcoredump_timeout(struct work_struct *work)
+{
+ struct hci_dev *hdev = container_of(work, struct hci_dev,
+ dump.dump_timeout.work);
+ u32 dump_size;
+
+ hci_devcoredump_notify(hdev, HCI_DEVCOREDUMP_TIMEOUT);
+
+ hci_dev_lock(hdev);
+
+ cancel_work_sync(&hdev->dump.dump_rx);
+
+ hci_devcoredump_update_state(hdev, HCI_DEVCOREDUMP_TIMEOUT);
+ dump_size = hdev->dump.tail - hdev->dump.head;
+ bt_dev_info(hdev, "Devcoredump timeout with size %u (expect %u)",
+ dump_size, hdev->dump.alloc_size);
+
+ /* Emit a devcoredump with the available data */
+ dev_coredumpv(&hdev->dev, hdev->dump.head, dump_size, GFP_KERNEL);
+
+ hci_devcoredump_reset(hdev);
+
+ hci_dev_unlock(hdev);
+}
+EXPORT_SYMBOL(hci_devcoredump_timeout);
+
+int hci_devcoredump_register(struct hci_dev *hdev, coredump_t coredump,
+ dmp_hdr_t dmp_hdr, notify_change_t notify_change)
+{
+ /* Driver must implement coredump() and dmp_hdr() functions for
+ * bluetooth devcoredump. The coredump() should trigger a coredump
+ * event on the controller when the device's coredump sysfs entry is
+ * written to. The dmp_hdr() should create a dump header to identify
+ * the controller/fw/driver info.
+ */
+ if (!coredump || !dmp_hdr)
+ return -EINVAL;
+
+ hci_dev_lock(hdev);
+ hdev->dump.coredump = coredump;
+ hdev->dump.dmp_hdr = dmp_hdr;
+ hdev->dump.notify_change = notify_change;
+ hdev->dump.supported = true;
+ hci_dev_unlock(hdev);
+
+ return 0;
+}
+EXPORT_SYMBOL(hci_devcoredump_register);
+
+static inline bool hci_devcoredump_enabled(struct hci_dev *hdev)
+{
+ /* The 'supported' flag is true when the driver registers with the HCI
+ * devcoredump API, whereas, the 'enabled' is controlled via a sysfs
+ * entry. For drivers like btusb which supports multiple vendor drivers,
+ * it is possible that the vendor driver does not support but the
+ * interface is provided by the base btusb driver. So, check both.
+ */
+ if (hdev->dump.supported && hdev->dump.enabled)
+ return hdev->dump.enabled(hdev);
+
+ return false;
+}
+
+int hci_devcoredump_init(struct hci_dev *hdev, u32 dmp_size)
+{
+ struct sk_buff *skb = NULL;
+
+ if (!hci_devcoredump_enabled(hdev))
+ return -EOPNOTSUPP;
+
+ skb = alloc_skb(sizeof(dmp_size), GFP_ATOMIC);
+ if (!skb) {
+ bt_dev_err(hdev, "Failed to allocate devcoredump init");
+ return -ENOMEM;
+ }
+
+ hci_dmp_cb(skb)->pkt_type = HCI_DEVCOREDUMP_PKT_INIT;
+ skb_put_data(skb, &dmp_size, sizeof(dmp_size));
+
+ skb_queue_tail(&hdev->dump.dump_q, skb);
+ queue_work(hdev->workqueue, &hdev->dump.dump_rx);
+
+ return 0;
+}
+EXPORT_SYMBOL(hci_devcoredump_init);
+
+int hci_devcoredump_append(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ if (!skb)
+ return -ENOMEM;
+
+ if (!hci_devcoredump_enabled(hdev)) {
+ kfree_skb(skb);
+ return -EOPNOTSUPP;
+ }
+
+ hci_dmp_cb(skb)->pkt_type = HCI_DEVCOREDUMP_PKT_SKB;
+
+ skb_queue_tail(&hdev->dump.dump_q, skb);
+ queue_work(hdev->workqueue, &hdev->dump.dump_rx);
+
+ return 0;
+}
+EXPORT_SYMBOL(hci_devcoredump_append);
+
+int hci_devcoredump_append_pattern(struct hci_dev *hdev, u8 pattern, u32 len)
+{
+ struct hci_devcoredump_skb_pattern p;
+ struct sk_buff *skb = NULL;
+
+ if (!hci_devcoredump_enabled(hdev))
+ return -EOPNOTSUPP;
+
+ skb = alloc_skb(sizeof(p), GFP_ATOMIC);
+ if (!skb) {
+ bt_dev_err(hdev, "Failed to allocate devcoredump pattern");
+ return -ENOMEM;
+ }
+
+ p.pattern = pattern;
+ p.len = len;
+
+ hci_dmp_cb(skb)->pkt_type = HCI_DEVCOREDUMP_PKT_PATTERN;
+ skb_put_data(skb, &p, sizeof(p));
+
+ skb_queue_tail(&hdev->dump.dump_q, skb);
+ queue_work(hdev->workqueue, &hdev->dump.dump_rx);
+
+ return 0;
+}
+EXPORT_SYMBOL(hci_devcoredump_append_pattern);
+
+int hci_devcoredump_complete(struct hci_dev *hdev)
+{
+ struct sk_buff *skb = NULL;
+
+ if (!hci_devcoredump_enabled(hdev))
+ return -EOPNOTSUPP;
+
+ skb = alloc_skb(0, GFP_ATOMIC);
+ if (!skb) {
+ bt_dev_err(hdev, "Failed to allocate devcoredump complete");
+ return -ENOMEM;
+ }
+
+ hci_dmp_cb(skb)->pkt_type = HCI_DEVCOREDUMP_PKT_COMPLETE;
+
+ skb_queue_tail(&hdev->dump.dump_q, skb);
+ queue_work(hdev->workqueue, &hdev->dump.dump_rx);
+
+ return 0;
+}
+EXPORT_SYMBOL(hci_devcoredump_complete);
+
+int hci_devcoredump_abort(struct hci_dev *hdev)
+{
+ struct sk_buff *skb = NULL;
+
+ if (!hci_devcoredump_enabled(hdev))
+ return -EOPNOTSUPP;
+
+ skb = alloc_skb(0, GFP_ATOMIC);
+ if (!skb) {
+ bt_dev_err(hdev, "Failed to allocate devcoredump abort");
+ return -ENOMEM;
+ }
+
+ hci_dmp_cb(skb)->pkt_type = HCI_DEVCOREDUMP_PKT_ABORT;
+
+ skb_queue_tail(&hdev->dump.dump_q, skb);
+ queue_work(hdev->workqueue, &hdev->dump.dump_rx);
+
+ return 0;
+}
+EXPORT_SYMBOL(hci_devcoredump_abort);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b3a5a3cc9372..9a697190c7a8 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2510,14 +2510,23 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
INIT_WORK(&hdev->tx_work, hci_tx_work);
INIT_WORK(&hdev->power_on, hci_power_on);
INIT_WORK(&hdev->error_reset, hci_error_reset);
+#ifdef CONFIG_DEV_COREDUMP
+ INIT_WORK(&hdev->dump.dump_rx, hci_devcoredump_rx);
+#endif
hci_cmd_sync_init(hdev);
INIT_DELAYED_WORK(&hdev->power_off, hci_power_off);
+#ifdef CONFIG_DEV_COREDUMP
+ INIT_DELAYED_WORK(&hdev->dump.dump_timeout, hci_devcoredump_timeout);
+#endif
skb_queue_head_init(&hdev->rx_q);
skb_queue_head_init(&hdev->cmd_q);
skb_queue_head_init(&hdev->raw_q);
+#ifdef CONFIG_DEV_COREDUMP
+ skb_queue_head_init(&hdev->dump.dump_q);
+#endif
init_waitqueue_head(&hdev->req_wait_q);
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index e6d804b82b67..09d74ae2b81c 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -4337,6 +4337,8 @@ int hci_dev_open_sync(struct hci_dev *hdev)
goto done;
}
+ hci_devcoredump_reset(hdev);
+
set_bit(HCI_RUNNING, &hdev->flags);
hci_sock_dev_event(hdev, HCI_DEV_OPEN);
--
2.37.1.559.g78731f0fdb-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v5 3/5] Bluetooth: Add support for hci devcoredump
2022-08-10 16:00 ` [PATCH v5 3/5] Bluetooth: Add support for hci devcoredump Manish Mandlik
@ 2022-08-10 16:19 ` Greg Kroah-Hartman
2022-08-12 18:59 ` kernel test robot
2022-08-12 19:19 ` kernel test robot
2 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2022-08-10 16:19 UTC (permalink / raw)
To: Manish Mandlik
Cc: Arend van Spriel, marcel, luiz.dentz, Johannes Berg, Dan Williams,
Jason Gunthorpe, linux-bluetooth, Thomas Gleixner,
Rafael J . Wysocki, chromeos-bluetooth-upstreaming, Won Chung,
Abhishek Pandit-Subedi, David S. Miller, Eric Dumazet,
Jakub Kicinski, Johan Hedberg, Paolo Abeni, linux-kernel, netdev
On Wed, Aug 10, 2022 at 09:00:36AM -0700, Manish Mandlik wrote:
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2510,14 +2510,23 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
> INIT_WORK(&hdev->tx_work, hci_tx_work);
> INIT_WORK(&hdev->power_on, hci_power_on);
> INIT_WORK(&hdev->error_reset, hci_error_reset);
> +#ifdef CONFIG_DEV_COREDUMP
> + INIT_WORK(&hdev->dump.dump_rx, hci_devcoredump_rx);
> +#endif
>
> hci_cmd_sync_init(hdev);
>
> INIT_DELAYED_WORK(&hdev->power_off, hci_power_off);
> +#ifdef CONFIG_DEV_COREDUMP
> + INIT_DELAYED_WORK(&hdev->dump.dump_timeout, hci_devcoredump_timeout);
> +#endif
>
> skb_queue_head_init(&hdev->rx_q);
> skb_queue_head_init(&hdev->cmd_q);
> skb_queue_head_init(&hdev->raw_q);
> +#ifdef CONFIG_DEV_COREDUMP
> + skb_queue_head_init(&hdev->dump.dump_q);
> +#endif
Putting #ifdef in .c files is messy, why not put all of this behind a
function that you properly handle in a .h file instead?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 3/5] Bluetooth: Add support for hci devcoredump
2022-08-10 16:00 ` [PATCH v5 3/5] Bluetooth: Add support for hci devcoredump Manish Mandlik
2022-08-10 16:19 ` Greg Kroah-Hartman
@ 2022-08-12 18:59 ` kernel test robot
2022-08-12 19:19 ` kernel test robot
2 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2022-08-12 18:59 UTC (permalink / raw)
To: Manish Mandlik, Arend van Spriel, Greg Kroah-Hartman, marcel,
luiz.dentz
Cc: kbuild-all, Johannes Berg, Dan Williams, Jason Gunthorpe,
Signed-off-by : Manish Mandlik, linux-bluetooth, Thomas Gleixner,
Rafael J . Wysocki, chromeos-bluetooth-upstreaming, Won Chung,
Abhishek Pandit-Subedi, Eric Dumazet, Jakub Kicinski,
Johan Hedberg, Paolo Abeni, linux-kernel, netdev
Hi Manish,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bluetooth/master]
[also build test WARNING on bluetooth-next/master driver-core/driver-core-testing linus/master next-20220812]
[cannot apply to v5.19]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Manish-Mandlik/sysfs-Add-attribute-info-for-sys-devices-coredump_disabled/20220811-000313
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git master
config: loongarch-randconfig-r022-20220811 (https://download.01.org/0day-ci/archive/20220813/202208130238.wyNvbcE7-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/6fe2192077ebdca91aef91e907f79d9e38960a21
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Manish-Mandlik/sysfs-Add-attribute-info-for-sys-devices-coredump_disabled/20220811-000313
git checkout 6fe2192077ebdca91aef91e907f79d9e38960a21
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash net/bluetooth/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from net/bluetooth/coredump.c:8:
net/bluetooth/coredump.c: In function 'hci_devcoredump_rx':
>> include/net/bluetooth/bluetooth.h:255:17: warning: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
255 | BT_INFO("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
| ^~~~~~
include/net/bluetooth/bluetooth.h:242:41: note: in definition of macro 'BT_INFO'
242 | #define BT_INFO(fmt, ...) bt_info(fmt "\n", ##__VA_ARGS__)
| ^~~
net/bluetooth/coredump.c:298:25: note: in expansion of macro 'bt_dev_info'
298 | bt_dev_info(hdev,
| ^~~~~~~~~~~
>> include/net/bluetooth/bluetooth.h:255:17: warning: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
255 | BT_INFO("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
| ^~~~~~
include/net/bluetooth/bluetooth.h:242:41: note: in definition of macro 'BT_INFO'
242 | #define BT_INFO(fmt, ...) bt_info(fmt "\n", ##__VA_ARGS__)
| ^~~
net/bluetooth/coredump.c:317:25: note: in expansion of macro 'bt_dev_info'
317 | bt_dev_info(hdev,
| ^~~~~~~~~~~
net/bluetooth/coredump.c: In function 'hci_devcoredump_timeout':
>> include/net/bluetooth/bluetooth.h:255:17: warning: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
255 | BT_INFO("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
| ^~~~~~
include/net/bluetooth/bluetooth.h:242:41: note: in definition of macro 'BT_INFO'
242 | #define BT_INFO(fmt, ...) bt_info(fmt "\n", ##__VA_ARGS__)
| ^~~
net/bluetooth/coredump.c:364:9: note: in expansion of macro 'bt_dev_info'
364 | bt_dev_info(hdev, "Devcoredump timeout with size %u (expect %u)",
| ^~~~~~~~~~~
vim +255 include/net/bluetooth/bluetooth.h
9b392e0e0b6d02 Luiz Augusto von Dentz 2022-03-03 253
6f558b70fb39fc Loic Poulain 2015-08-30 254 #define bt_dev_info(hdev, fmt, ...) \
9b392e0e0b6d02 Luiz Augusto von Dentz 2022-03-03 @255 BT_INFO("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
594b31ea7dc610 Frederic Danis 2015-09-23 256 #define bt_dev_warn(hdev, fmt, ...) \
9b392e0e0b6d02 Luiz Augusto von Dentz 2022-03-03 257 BT_WARN("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
6f558b70fb39fc Loic Poulain 2015-08-30 258 #define bt_dev_err(hdev, fmt, ...) \
9b392e0e0b6d02 Luiz Augusto von Dentz 2022-03-03 259 BT_ERR("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
6f558b70fb39fc Loic Poulain 2015-08-30 260 #define bt_dev_dbg(hdev, fmt, ...) \
9b392e0e0b6d02 Luiz Augusto von Dentz 2022-03-03 261 BT_DBG("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
6f558b70fb39fc Loic Poulain 2015-08-30 262
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 3/5] Bluetooth: Add support for hci devcoredump
2022-08-10 16:00 ` [PATCH v5 3/5] Bluetooth: Add support for hci devcoredump Manish Mandlik
2022-08-10 16:19 ` Greg Kroah-Hartman
2022-08-12 18:59 ` kernel test robot
@ 2022-08-12 19:19 ` kernel test robot
2 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2022-08-12 19:19 UTC (permalink / raw)
To: Manish Mandlik, Arend van Spriel, Greg Kroah-Hartman, marcel,
luiz.dentz
Cc: llvm, kbuild-all, Johannes Berg, Dan Williams, Jason Gunthorpe,
Signed-off-by : Manish Mandlik, linux-bluetooth, Thomas Gleixner,
Rafael J . Wysocki, chromeos-bluetooth-upstreaming, Won Chung,
Abhishek Pandit-Subedi, Eric Dumazet, Jakub Kicinski,
Johan Hedberg, Paolo Abeni, linux-kernel, netdev
Hi Manish,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bluetooth/master]
[also build test WARNING on bluetooth-next/master driver-core/driver-core-testing linus/master next-20220812]
[cannot apply to v5.19]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Manish-Mandlik/sysfs-Add-attribute-info-for-sys-devices-coredump_disabled/20220811-000313
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git master
config: x86_64-randconfig-a003 (https://download.01.org/0day-ci/archive/20220813/202208130346.2UmF05bO-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/6fe2192077ebdca91aef91e907f79d9e38960a21
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Manish-Mandlik/sysfs-Add-attribute-info-for-sys-devices-coredump_disabled/20220811-000313
git checkout 6fe2192077ebdca91aef91e907f79d9e38960a21
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash net/bluetooth/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/bluetooth/coredump.c:301:20: warning: format specifies type 'unsigned int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat]
dump_size, hdev->dump.alloc_size);
^~~~~~~~~~~~~~~~~~~~~
include/net/bluetooth/bluetooth.h:255:43: note: expanded from macro 'bt_dev_info'
BT_INFO("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/net/bluetooth/bluetooth.h:242:47: note: expanded from macro 'BT_INFO'
#define BT_INFO(fmt, ...) bt_info(fmt "\n", ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
net/bluetooth/coredump.c:320:20: warning: format specifies type 'unsigned int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat]
dump_size, hdev->dump.alloc_size);
^~~~~~~~~~~~~~~~~~~~~
include/net/bluetooth/bluetooth.h:255:43: note: expanded from macro 'bt_dev_info'
BT_INFO("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/net/bluetooth/bluetooth.h:242:47: note: expanded from macro 'BT_INFO'
#define BT_INFO(fmt, ...) bt_info(fmt "\n", ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
net/bluetooth/coredump.c:365:18: warning: format specifies type 'unsigned int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat]
dump_size, hdev->dump.alloc_size);
^~~~~~~~~~~~~~~~~~~~~
include/net/bluetooth/bluetooth.h:255:43: note: expanded from macro 'bt_dev_info'
BT_INFO("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/net/bluetooth/bluetooth.h:242:47: note: expanded from macro 'BT_INFO'
#define BT_INFO(fmt, ...) bt_info(fmt "\n", ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
3 warnings generated.
vim +301 net/bluetooth/coredump.c
182
183 /* Bluetooth devcoredump state machine.
184 *
185 * Devcoredump states:
186 *
187 * HCI_DEVCOREDUMP_IDLE: The default state.
188 *
189 * HCI_DEVCOREDUMP_ACTIVE: A devcoredump will be in this state once it has
190 * been initialized using hci_devcoredump_init(). Once active, the
191 * driver can append data using hci_devcoredump_append() or insert
192 * a pattern using hci_devcoredump_append_pattern().
193 *
194 * HCI_DEVCOREDUMP_DONE: Once the dump collection is complete, the drive
195 * can signal the completion using hci_devcoredump_complete(). A
196 * devcoredump is generated indicating the completion event and
197 * then the state machine is reset to the default state.
198 *
199 * HCI_DEVCOREDUMP_ABORT: The driver can cancel ongoing dump collection in
200 * case of any error using hci_devcoredump_abort(). A devcoredump
201 * is still generated with the available data indicating the abort
202 * event and then the state machine is reset to the default state.
203 *
204 * HCI_DEVCOREDUMP_TIMEOUT: A timeout timer for HCI_DEVCOREDUMP_TIMEOUT sec
205 * is started during devcoredump initialization. Once the timeout
206 * occurs, the driver is notified, a devcoredump is generated with
207 * the available data indicating the timeout event and then the
208 * state machine is reset to the default state.
209 *
210 * The driver must register using hci_devcoredump_register() before using the
211 * hci devcoredump APIs.
212 */
213 void hci_devcoredump_rx(struct work_struct *work)
214 {
215 struct hci_dev *hdev = container_of(work, struct hci_dev, dump.dump_rx);
216 struct sk_buff *skb;
217 struct hci_devcoredump_skb_pattern *pattern;
218 u32 dump_size;
219 int start_state;
220
221 #define DBG_UNEXPECTED_STATE() \
222 bt_dev_dbg(hdev, \
223 "Unexpected packet (%d) for state (%d). ", \
224 hci_dmp_cb(skb)->pkt_type, hdev->dump.state)
225
226 while ((skb = skb_dequeue(&hdev->dump.dump_q))) {
227 hci_dev_lock(hdev);
228 start_state = hdev->dump.state;
229
230 switch (hci_dmp_cb(skb)->pkt_type) {
231 case HCI_DEVCOREDUMP_PKT_INIT:
232 if (hdev->dump.state != HCI_DEVCOREDUMP_IDLE) {
233 DBG_UNEXPECTED_STATE();
234 goto loop_continue;
235 }
236
237 if (skb->len != sizeof(dump_size)) {
238 bt_dev_dbg(hdev, "Invalid dump init pkt");
239 goto loop_continue;
240 }
241
242 dump_size = *((u32 *)skb->data);
243 if (!dump_size) {
244 bt_dev_err(hdev, "Zero size dump init pkt");
245 goto loop_continue;
246 }
247
248 if (hci_devcoredump_prepare(hdev, dump_size)) {
249 bt_dev_err(hdev, "Failed to prepare for dump");
250 goto loop_continue;
251 }
252
253 hci_devcoredump_update_state(hdev,
254 HCI_DEVCOREDUMP_ACTIVE);
255 queue_delayed_work(hdev->workqueue,
256 &hdev->dump.dump_timeout,
257 DEVCOREDUMP_TIMEOUT);
258 break;
259
260 case HCI_DEVCOREDUMP_PKT_SKB:
261 if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) {
262 DBG_UNEXPECTED_STATE();
263 goto loop_continue;
264 }
265
266 if (!hci_devcoredump_copy(hdev, skb->data, skb->len))
267 bt_dev_dbg(hdev, "Failed to insert skb");
268 break;
269
270 case HCI_DEVCOREDUMP_PKT_PATTERN:
271 if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) {
272 DBG_UNEXPECTED_STATE();
273 goto loop_continue;
274 }
275
276 if (skb->len != sizeof(*pattern)) {
277 bt_dev_dbg(hdev, "Invalid pattern skb");
278 goto loop_continue;
279 }
280
281 pattern = (void *)skb->data;
282
283 if (!hci_devcoredump_memset(hdev, pattern->pattern,
284 pattern->len))
285 bt_dev_dbg(hdev, "Failed to set pattern");
286 break;
287
288 case HCI_DEVCOREDUMP_PKT_COMPLETE:
289 if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) {
290 DBG_UNEXPECTED_STATE();
291 goto loop_continue;
292 }
293
294 hci_devcoredump_update_state(hdev,
295 HCI_DEVCOREDUMP_DONE);
296 dump_size = hdev->dump.tail - hdev->dump.head;
297
298 bt_dev_info(hdev,
299 "Devcoredump complete with size %u "
300 "(expect %u)",
> 301 dump_size, hdev->dump.alloc_size);
302
303 dev_coredumpv(&hdev->dev, hdev->dump.head, dump_size,
304 GFP_KERNEL);
305 break;
306
307 case HCI_DEVCOREDUMP_PKT_ABORT:
308 if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) {
309 DBG_UNEXPECTED_STATE();
310 goto loop_continue;
311 }
312
313 hci_devcoredump_update_state(hdev,
314 HCI_DEVCOREDUMP_ABORT);
315 dump_size = hdev->dump.tail - hdev->dump.head;
316
317 bt_dev_info(hdev,
318 "Devcoredump aborted with size %u "
319 "(expect %u)",
320 dump_size, hdev->dump.alloc_size);
321
322 /* Emit a devcoredump with the available data */
323 dev_coredumpv(&hdev->dev, hdev->dump.head, dump_size,
324 GFP_KERNEL);
325 break;
326
327 default:
328 bt_dev_dbg(hdev,
329 "Unknown packet (%d) for state (%d). ",
330 hci_dmp_cb(skb)->pkt_type, hdev->dump.state);
331 break;
332 }
333
334 loop_continue:
335 kfree_skb(skb);
336 hci_dev_unlock(hdev);
337
338 if (start_state != hdev->dump.state)
339 hci_devcoredump_notify(hdev, hdev->dump.state);
340
341 hci_dev_lock(hdev);
342 if (hdev->dump.state == HCI_DEVCOREDUMP_DONE ||
343 hdev->dump.state == HCI_DEVCOREDUMP_ABORT)
344 hci_devcoredump_reset(hdev);
345 hci_dev_unlock(hdev);
346 }
347 }
348 EXPORT_SYMBOL(hci_devcoredump_rx);
349
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 4/5] Bluetooth: btusb: Add btusb devcoredump support
2022-08-10 16:00 [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled Manish Mandlik
2022-08-10 16:00 ` [PATCH v5 2/5] devcoredump: Add per device sysfs entry to enable/disable coredump Manish Mandlik
2022-08-10 16:00 ` [PATCH v5 3/5] Bluetooth: Add support for hci devcoredump Manish Mandlik
@ 2022-08-10 16:00 ` Manish Mandlik
2022-08-10 16:20 ` Greg Kroah-Hartman
2022-08-10 16:00 ` [PATCH v5 5/5] Bluetooth: btintel: Add Intel " Manish Mandlik
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Manish Mandlik @ 2022-08-10 16:00 UTC (permalink / raw)
To: Arend van Spriel, Greg Kroah-Hartman, marcel, luiz.dentz
Cc: Johannes Berg, Dan Williams, Jason Gunthorpe,
Signed-off-by : Manish Mandlik, linux-bluetooth, Thomas Gleixner,
Rafael J . Wysocki, chromeos-bluetooth-upstreaming, Won Chung,
Abhishek Pandit-Subedi, Johan Hedberg, linux-kernel
This patch implements the btusb driver side .coredump() callback to
trigger a devcoredump via sysfs and .enable_coredump() callback to
check if the devcoredump functionality is enabled for a device.
Signed-off-by: Manish Mandlik <mmandlik@google.com>
Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---
(no changes since v4)
Changes in v4:
- New patch in the series
drivers/bluetooth/btusb.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 30dd443f395f..b00851327aa3 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1510,6 +1510,15 @@ static void btusb_isoc_tx_complete(struct urb *urb)
kfree_skb(skb);
}
+#ifdef CONFIG_DEV_COREDUMP
+static bool btusb_coredump_enabled(struct hci_dev *hdev)
+{
+ struct btusb_data *data = hci_get_drvdata(hdev);
+
+ return !data->intf->dev.coredump_disabled;
+}
+#endif
+
static int btusb_open(struct hci_dev *hdev)
{
struct btusb_data *data = hci_get_drvdata(hdev);
@@ -3765,6 +3774,9 @@ static int btusb_probe(struct usb_interface *intf,
hdev->send = btusb_send_frame;
hdev->notify = btusb_notify;
hdev->wakeup = btusb_wakeup;
+#ifdef CONFIG_DEV_COREDUMP
+ hdev->dump.enabled = btusb_coredump_enabled;
+#endif
#ifdef CONFIG_PM
err = btusb_config_oob_wake(hdev);
@@ -4180,6 +4192,17 @@ static int btusb_resume(struct usb_interface *intf)
}
#endif
+#ifdef CONFIG_DEV_COREDUMP
+static void btusb_coredump(struct device *dev)
+{
+ struct btusb_data *data = dev_get_drvdata(dev);
+ struct hci_dev *hdev = data->hdev;
+
+ if (!dev->coredump_disabled && hdev->dump.coredump)
+ hdev->dump.coredump(hdev);
+}
+#endif
+
static struct usb_driver btusb_driver = {
.name = "btusb",
.probe = btusb_probe,
@@ -4191,6 +4214,14 @@ static struct usb_driver btusb_driver = {
.id_table = btusb_table,
.supports_autosuspend = 1,
.disable_hub_initiated_lpm = 1,
+
+#ifdef CONFIG_DEV_COREDUMP
+ .drvwrap = {
+ .driver = {
+ .coredump = btusb_coredump,
+ },
+ },
+#endif
};
module_usb_driver(btusb_driver);
--
2.37.1.559.g78731f0fdb-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v5 4/5] Bluetooth: btusb: Add btusb devcoredump support
2022-08-10 16:00 ` [PATCH v5 4/5] Bluetooth: btusb: Add btusb devcoredump support Manish Mandlik
@ 2022-08-10 16:20 ` Greg Kroah-Hartman
0 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2022-08-10 16:20 UTC (permalink / raw)
To: Manish Mandlik
Cc: Arend van Spriel, marcel, luiz.dentz, Johannes Berg, Dan Williams,
Jason Gunthorpe, linux-bluetooth, Thomas Gleixner,
Rafael J . Wysocki, chromeos-bluetooth-upstreaming, Won Chung,
Abhishek Pandit-Subedi, Johan Hedberg, linux-kernel
On Wed, Aug 10, 2022 at 09:00:37AM -0700, Manish Mandlik wrote:
> This patch implements the btusb driver side .coredump() callback to
> trigger a devcoredump via sysfs and .enable_coredump() callback to
> check if the devcoredump functionality is enabled for a device.
>
> Signed-off-by: Manish Mandlik <mmandlik@google.com>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> ---
>
> (no changes since v4)
>
> Changes in v4:
> - New patch in the series
>
> drivers/bluetooth/btusb.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 30dd443f395f..b00851327aa3 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -1510,6 +1510,15 @@ static void btusb_isoc_tx_complete(struct urb *urb)
> kfree_skb(skb);
> }
>
> +#ifdef CONFIG_DEV_COREDUMP
> +static bool btusb_coredump_enabled(struct hci_dev *hdev)
> +{
> + struct btusb_data *data = hci_get_drvdata(hdev);
> +
> + return !data->intf->dev.coredump_disabled;
> +}
> +#endif
Again, #ifdef in .c files is unmaintainable over time, please do not do
it if at all possible.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 5/5] Bluetooth: btintel: Add Intel devcoredump support
2022-08-10 16:00 [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled Manish Mandlik
` (2 preceding siblings ...)
2022-08-10 16:00 ` [PATCH v5 4/5] Bluetooth: btusb: Add btusb devcoredump support Manish Mandlik
@ 2022-08-10 16:00 ` Manish Mandlik
2022-08-10 16:21 ` Greg Kroah-Hartman
2022-08-10 16:03 ` [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled Johannes Berg
` (2 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Manish Mandlik @ 2022-08-10 16:00 UTC (permalink / raw)
To: Arend van Spriel, Greg Kroah-Hartman, marcel, luiz.dentz
Cc: Johannes Berg, Dan Williams, Jason Gunthorpe,
Signed-off-by : Manish Mandlik, linux-bluetooth, Thomas Gleixner,
Rafael J . Wysocki, chromeos-bluetooth-upstreaming, Won Chung,
Abhishek Pandit-Subedi, Chethan Tumkur Narayan, Johan Hedberg,
linux-kernel
From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Intercept debug exception events from the controller and put them into
a devcoredump using hci devcoredump APIs. The debug exception contains
data in a TLV format and it will be parsed in userspace.
Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Signed-off-by: Manish Mandlik <mmandlik@google.com>
Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Reviewed-by: Chethan Tumkur Narayan <chethan.tumkur.narayan@intel.com>
---
Hi Luiz,
The implementation of btintel_coredump() is currently under internal
review. So I'll send it later as a separate patch. This patch includes
only a placeholder for that.
Thanks,
Manish.
(no changes since v4)
Changes in v4:
- Add btintel_coredump() placeholder
Changes in v2:
- Create a local struct to store coredump_info in btintel.c
- Call btintel_register_devcoredump_support() from btintel.c
- Fix strncpy() destination bound warning
drivers/bluetooth/btintel.c | 73 ++++++++++++++++++++++++++++++++++++-
drivers/bluetooth/btintel.h | 11 +++++-
drivers/bluetooth/btusb.c | 51 ++++++++++++++++++++++----
3 files changed, 125 insertions(+), 10 deletions(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 818681c89db8..667cc11b19c3 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -32,6 +32,13 @@ struct cmd_write_boot_params {
u8 fw_build_yy;
} __packed;
+#define DRIVER_NAME_LEN 16
+static struct {
+ char driver_name[DRIVER_NAME_LEN];
+ u8 hw_variant;
+ u32 fw_build_num;
+} coredump_info;
+
int btintel_check_bdaddr(struct hci_dev *hdev)
{
struct hci_rp_read_bd_addr *bda;
@@ -304,6 +311,9 @@ int btintel_version_info(struct hci_dev *hdev, struct intel_version *ver)
return -EINVAL;
}
+ coredump_info.hw_variant = ver->hw_variant;
+ coredump_info.fw_build_num = ver->fw_build_num;
+
bt_dev_info(hdev, "%s revision %u.%u build %u week %u %u",
variant, ver->fw_revision >> 4, ver->fw_revision & 0x0f,
ver->fw_build_num, ver->fw_build_ww,
@@ -497,6 +507,9 @@ static int btintel_version_info_tlv(struct hci_dev *hdev,
return -EINVAL;
}
+ coredump_info.hw_variant = INTEL_HW_VARIANT(version->cnvi_bt);
+ coredump_info.fw_build_num = version->build_num;
+
bt_dev_info(hdev, "%s timestamp %u.%u buildtype %u build %u", variant,
2000 + (version->timestamp >> 8), version->timestamp & 0xff,
version->build_type, version->build_num);
@@ -1391,6 +1404,59 @@ int btintel_set_quality_report(struct hci_dev *hdev, bool enable)
}
EXPORT_SYMBOL_GPL(btintel_set_quality_report);
+static void btintel_coredump(struct hci_dev *hdev)
+{
+ /* Trigger coredump */
+}
+
+static int btintel_dmp_hdr(struct hci_dev *hdev, char *buf, size_t size)
+{
+ char *ptr = buf;
+ size_t rem = size;
+ size_t read = 0;
+
+ read = snprintf(ptr, rem, "Controller Name: 0x%X\n",
+ coredump_info.hw_variant);
+ rem -= read;
+ ptr += read;
+
+ read = snprintf(ptr, rem, "Firmware Version: 0x%X\n",
+ coredump_info.fw_build_num);
+ rem -= read;
+ ptr += read;
+
+ read = snprintf(ptr, rem, "Driver: %s\n", coredump_info.driver_name);
+ rem -= read;
+ ptr += read;
+
+ read = snprintf(ptr, rem, "Vendor: Intel\n");
+ rem -= read;
+ ptr += read;
+
+ return size - rem;
+}
+
+static int btintel_register_devcoredump_support(struct hci_dev *hdev)
+{
+ struct intel_debug_features features;
+ int err;
+
+ err = btintel_read_debug_features(hdev, &features);
+ if (err) {
+ bt_dev_info(hdev, "Error reading debug features");
+ return err;
+ }
+
+ if (!(features.page1[0] & 0x3f)) {
+ bt_dev_info(hdev, "Telemetry exception format not supported");
+ return -EOPNOTSUPP;
+ }
+
+ hci_devcoredump_register(hdev, btintel_coredump, btintel_dmp_hdr, NULL);
+
+ return err;
+}
+
static const struct firmware *btintel_legacy_rom_get_fw(struct hci_dev *hdev,
struct intel_version *ver)
{
@@ -2464,6 +2530,7 @@ static int btintel_setup_combined(struct hci_dev *hdev)
btintel_set_msft_opcode(hdev, ver.hw_variant);
err = btintel_bootloader_setup(hdev, &ver);
+ btintel_register_devcoredump_support(hdev);
break;
default:
bt_dev_err(hdev, "Unsupported Intel hw variant (%u)",
@@ -2538,6 +2605,7 @@ static int btintel_setup_combined(struct hci_dev *hdev)
btintel_set_msft_opcode(hdev, ver.hw_variant);
err = btintel_bootloader_setup(hdev, &ver);
+ btintel_register_devcoredump_support(hdev);
break;
case 0x17:
case 0x18:
@@ -2560,6 +2628,7 @@ static int btintel_setup_combined(struct hci_dev *hdev)
INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
err = btintel_bootloader_setup_tlv(hdev, &ver_tlv);
+ btintel_register_devcoredump_support(hdev);
break;
default:
bt_dev_err(hdev, "Unsupported Intel hw variant (%u)",
@@ -2608,7 +2677,7 @@ static int btintel_shutdown_combined(struct hci_dev *hdev)
return 0;
}
-int btintel_configure_setup(struct hci_dev *hdev)
+int btintel_configure_setup(struct hci_dev *hdev, const char *driver_name)
{
hdev->manufacturer = 2;
hdev->setup = btintel_setup_combined;
@@ -2617,6 +2686,8 @@ int btintel_configure_setup(struct hci_dev *hdev)
hdev->set_diag = btintel_set_diag_combined;
hdev->set_bdaddr = btintel_set_bdaddr;
+ strncpy(coredump_info.driver_name, driver_name, DRIVER_NAME_LEN - 1);
+
return 0;
}
EXPORT_SYMBOL_GPL(btintel_configure_setup);
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index e0060e58573c..8e72e59b5bde 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -137,6 +137,12 @@ struct intel_offload_use_cases {
__u8 preset[8];
} __packed;
+#define INTEL_TLV_TYPE_ID 0x1
+
+#define INTEL_TLV_SYSTEM_EXCEPTION 0x0
+#define INTEL_TLV_FATAL_EXCEPTION 0x1
+#define INTEL_TLV_DEBUG_EXCEPTION 0x2
+
#define INTEL_HW_PLATFORM(cnvx_bt) ((u8)(((cnvx_bt) & 0x0000ff00) >> 8))
#define INTEL_HW_VARIANT(cnvx_bt) ((u8)(((cnvx_bt) & 0x003f0000) >> 16))
#define INTEL_CNVX_TOP_TYPE(cnvx_top) ((cnvx_top) & 0x00000fff)
@@ -206,7 +212,7 @@ int btintel_read_boot_params(struct hci_dev *hdev,
struct intel_boot_params *params);
int btintel_download_firmware(struct hci_dev *dev, struct intel_version *ver,
const struct firmware *fw, u32 *boot_param);
-int btintel_configure_setup(struct hci_dev *hdev);
+int btintel_configure_setup(struct hci_dev *hdev, const char *driver_name);
void btintel_bootup(struct hci_dev *hdev, const void *ptr, unsigned int len);
void btintel_secure_send_result(struct hci_dev *hdev,
const void *ptr, unsigned int len);
@@ -287,7 +293,8 @@ static inline int btintel_download_firmware(struct hci_dev *dev,
return -EOPNOTSUPP;
}
-static inline int btintel_configure_setup(struct hci_dev *hdev)
+static inline int btintel_configure_setup(struct hci_dev *hdev,
+ const char *driver_name)
{
return -ENODEV;
}
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index b00851327aa3..eb30f9188c25 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2213,16 +2213,44 @@ static int btusb_recv_bulk_intel(struct btusb_data *data, void *buffer,
return btusb_recv_bulk(data, buffer, count);
}
+static int btusb_intel_diagnostics(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ struct intel_tlv *tlv = (void *)&skb->data[5];
+
+ /* The first event is always an event type TLV */
+ if (tlv->type != INTEL_TLV_TYPE_ID)
+ goto recv_frame;
+
+ switch (tlv->val[0]) {
+ case INTEL_TLV_SYSTEM_EXCEPTION:
+ case INTEL_TLV_FATAL_EXCEPTION:
+ case INTEL_TLV_DEBUG_EXCEPTION:
+ /* Generate devcoredump from exception */
+ if (!hci_devcoredump_init(hdev, skb->len)) {
+ hci_devcoredump_append(hdev, skb);
+ hci_devcoredump_complete(hdev);
+ } else {
+ bt_dev_err(hdev, "Failed to generate devcoredump");
+ kfree_skb(skb);
+ }
+ return 0;
+ }
+
+recv_frame:
+ return hci_recv_frame(hdev, skb);
+}
+
static int btusb_recv_event_intel(struct hci_dev *hdev, struct sk_buff *skb)
{
- if (btintel_test_flag(hdev, INTEL_BOOTLOADER)) {
- struct hci_event_hdr *hdr = (void *)skb->data;
+ struct hci_event_hdr *hdr = (void *)skb->data;
+ const char diagnostics_hdr[] = { 0x87, 0x80, 0x03 };
- if (skb->len > HCI_EVENT_HDR_SIZE && hdr->evt == 0xff &&
- hdr->plen > 0) {
- const void *ptr = skb->data + HCI_EVENT_HDR_SIZE + 1;
- unsigned int len = skb->len - HCI_EVENT_HDR_SIZE - 1;
+ if (skb->len > HCI_EVENT_HDR_SIZE && hdr->evt == 0xff &&
+ hdr->plen > 0) {
+ const void *ptr = skb->data + HCI_EVENT_HDR_SIZE + 1;
+ unsigned int len = skb->len - HCI_EVENT_HDR_SIZE - 1;
+ if (btintel_test_flag(hdev, INTEL_BOOTLOADER)) {
switch (skb->data[2]) {
case 0x02:
/* When switching to the operational firmware
@@ -2241,6 +2269,15 @@ static int btusb_recv_event_intel(struct hci_dev *hdev, struct sk_buff *skb)
break;
}
}
+
+ /* Handle all diagnostics events separately. May still call
+ * hci_recv_frame.
+ */
+ if (len >= sizeof(diagnostics_hdr) &&
+ memcmp(&skb->data[2], diagnostics_hdr,
+ sizeof(diagnostics_hdr)) == 0) {
+ return btusb_intel_diagnostics(hdev, skb);
+ }
}
return hci_recv_frame(hdev, skb);
@@ -3822,7 +3859,7 @@ static int btusb_probe(struct usb_interface *intf,
/* Combined Intel Device setup to support multiple setup routine */
if (id->driver_info & BTUSB_INTEL_COMBINED) {
- err = btintel_configure_setup(hdev);
+ err = btintel_configure_setup(hdev, btusb_driver.name);
if (err)
goto out_free_dev;
--
2.37.1.559.g78731f0fdb-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v5 5/5] Bluetooth: btintel: Add Intel devcoredump support
2022-08-10 16:00 ` [PATCH v5 5/5] Bluetooth: btintel: Add Intel " Manish Mandlik
@ 2022-08-10 16:21 ` Greg Kroah-Hartman
0 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2022-08-10 16:21 UTC (permalink / raw)
To: Manish Mandlik
Cc: Arend van Spriel, marcel, luiz.dentz, Johannes Berg, Dan Williams,
Jason Gunthorpe, linux-bluetooth, Thomas Gleixner,
Rafael J . Wysocki, chromeos-bluetooth-upstreaming, Won Chung,
Abhishek Pandit-Subedi, Chethan Tumkur Narayan, Johan Hedberg,
linux-kernel
On Wed, Aug 10, 2022 at 09:00:38AM -0700, Manish Mandlik wrote:
> From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
>
> Intercept debug exception events from the controller and put them into
> a devcoredump using hci devcoredump APIs. The debug exception contains
> data in a TLV format and it will be parsed in userspace.
>
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> Signed-off-by: Manish Mandlik <mmandlik@google.com>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> Reviewed-by: Chethan Tumkur Narayan <chethan.tumkur.narayan@intel.com>
> ---
> Hi Luiz,
>
> The implementation of btintel_coredump() is currently under internal
> review. So I'll send it later as a separate patch. This patch includes
> only a placeholder for that.
That's odd, let's wait for that to be submitted, as we don't add
frameworks without full functionality, right? How do we know this will
ever be fixed in the future?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled
2022-08-10 16:00 [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled Manish Mandlik
` (3 preceding siblings ...)
2022-08-10 16:00 ` [PATCH v5 5/5] Bluetooth: btintel: Add Intel " Manish Mandlik
@ 2022-08-10 16:03 ` Johannes Berg
2022-08-10 16:21 ` Greg Kroah-Hartman
2022-08-10 16:22 ` Greg Kroah-Hartman
2022-08-10 17:25 ` [v5,1/5] " bluez.test.bot
6 siblings, 1 reply; 16+ messages in thread
From: Johannes Berg @ 2022-08-10 16:03 UTC (permalink / raw)
To: Manish Mandlik, Arend van Spriel, Greg Kroah-Hartman, marcel,
luiz.dentz
Cc: Dan Williams, Jason Gunthorpe, linux-bluetooth, Thomas Gleixner,
Rafael J . Wysocki, chromeos-bluetooth-upstreaming, Won Chung,
Abhishek Pandit-Subedi, linux-kernel
On Wed, 2022-08-10 at 09:00 -0700, Manish Mandlik wrote:
> This patch adds the specification for /sys/devices/.../coredump_disabled
> attribute which allows the userspace to enable/disable devcoredump for a
> particular device and drivers can use it to enable/disable functionality
> accordingly. It is available when the CONFIG_DEV_COREDUMP is enabled and
> driver has implemented the .coredump() callback.
>
It would be nice to say _why_? What problem does this solve? You could
just create the dump and discard it, instead, for example?
johannes
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled
2022-08-10 16:03 ` [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled Johannes Berg
@ 2022-08-10 16:21 ` Greg Kroah-Hartman
[not found] ` <CAGPPCLAV=mnMcteCnqFT5zdbWdZuFQRv6-oxC3qAnLh8_8H61Q@mail.gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Greg Kroah-Hartman @ 2022-08-10 16:21 UTC (permalink / raw)
To: Johannes Berg
Cc: Manish Mandlik, Arend van Spriel, marcel, luiz.dentz,
Dan Williams, Jason Gunthorpe, linux-bluetooth, Thomas Gleixner,
Rafael J . Wysocki, chromeos-bluetooth-upstreaming, Won Chung,
Abhishek Pandit-Subedi, linux-kernel
On Wed, Aug 10, 2022 at 06:03:37PM +0200, Johannes Berg wrote:
> On Wed, 2022-08-10 at 09:00 -0700, Manish Mandlik wrote:
> > This patch adds the specification for /sys/devices/.../coredump_disabled
> > attribute which allows the userspace to enable/disable devcoredump for a
> > particular device and drivers can use it to enable/disable functionality
> > accordingly. It is available when the CONFIG_DEV_COREDUMP is enabled and
> > driver has implemented the .coredump() callback.
> >
>
> It would be nice to say _why_? What problem does this solve? You could
> just create the dump and discard it, instead, for example?
Agreed, I do not understand the need for this at all.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled
2022-08-10 16:00 [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled Manish Mandlik
` (4 preceding siblings ...)
2022-08-10 16:03 ` [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled Johannes Berg
@ 2022-08-10 16:22 ` Greg Kroah-Hartman
2022-08-10 17:25 ` [v5,1/5] " bluez.test.bot
6 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2022-08-10 16:22 UTC (permalink / raw)
To: Manish Mandlik
Cc: Arend van Spriel, marcel, luiz.dentz, Johannes Berg, Dan Williams,
Jason Gunthorpe, linux-bluetooth, Thomas Gleixner,
Rafael J . Wysocki, chromeos-bluetooth-upstreaming, Won Chung,
Abhishek Pandit-Subedi, linux-kernel
On Wed, Aug 10, 2022 at 09:00:34AM -0700, Manish Mandlik wrote:
> +Date: July 2022
You wrote this patch in August...
^ permalink raw reply [flat|nested] 16+ messages in thread* RE: [v5,1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled
2022-08-10 16:00 [PATCH v5 1/5] sysfs: Add attribute info for /sys/devices/.../coredump_disabled Manish Mandlik
` (5 preceding siblings ...)
2022-08-10 16:22 ` Greg Kroah-Hartman
@ 2022-08-10 17:25 ` bluez.test.bot
6 siblings, 0 replies; 16+ messages in thread
From: bluez.test.bot @ 2022-08-10 17:25 UTC (permalink / raw)
To: linux-bluetooth, mmandlik
[-- Attachment #1: Type: text/plain, Size: 3683 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=666640
---Test result---
Test Summary:
CheckPatch FAIL 6.70 seconds
GitLint PASS 5.02 seconds
SubjectPrefix FAIL 1.74 seconds
BuildKernel PASS 34.25 seconds
BuildKernel32 PASS 30.05 seconds
Incremental Build with patchesPASS 115.07 seconds
TestRunner: Setup PASS 489.71 seconds
TestRunner: l2cap-tester PASS 17.60 seconds
TestRunner: bnep-tester PASS 6.69 seconds
TestRunner: mgmt-tester PASS 104.89 seconds
TestRunner: rfcomm-tester PASS 10.08 seconds
TestRunner: sco-tester PASS 10.01 seconds
TestRunner: smp-tester PASS 10.15 seconds
TestRunner: userchan-tester PASS 7.00 seconds
Details
##############################
Test: CheckPatch - FAIL - 6.70 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
[v5,3/5] Bluetooth: Add support for hci devcoredump\Traceback (most recent call last):
File "scripts/spdxcheck.py", line 6, in <module>
from ply import lex, yacc
ModuleNotFoundError: No module named 'ply'
WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#152:
new file mode 100644
WARNING:SPDX_LICENSE_TAG: Improper SPDX comment style for 'include/net/bluetooth/coredump.h', please use '/*' instead
#157: FILE: include/net/bluetooth/coredump.h:1:
+// SPDX-License-Identifier: GPL-2.0-only
WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#157: FILE: include/net/bluetooth/coredump.h:1:
+// SPDX-License-Identifier: GPL-2.0-only
WARNING:SPLIT_STRING: quoted string split across lines
#617: FILE: net/bluetooth/coredump.c:300:
+ "Devcoredump complete with size %u "
+ "(expect %u)",
WARNING:SPLIT_STRING: quoted string split across lines
#636: FILE: net/bluetooth/coredump.c:319:
+ "Devcoredump aborted with size %u "
+ "(expect %u)",
WARNING:OOM_MESSAGE: Possible unnecessary 'out of memory' message
#739: FILE: net/bluetooth/coredump.c:422:
+ if (!skb) {
+ bt_dev_err(hdev, "Failed to allocate devcoredump init");
WARNING:OOM_MESSAGE: Possible unnecessary 'out of memory' message
#782: FILE: net/bluetooth/coredump.c:465:
+ if (!skb) {
+ bt_dev_err(hdev, "Failed to allocate devcoredump pattern");
WARNING:OOM_MESSAGE: Possible unnecessary 'out of memory' message
#808: FILE: net/bluetooth/coredump.c:491:
+ if (!skb) {
+ bt_dev_err(hdev, "Failed to allocate devcoredump complete");
WARNING:OOM_MESSAGE: Possible unnecessary 'out of memory' message
#830: FILE: net/bluetooth/coredump.c:513:
+ if (!skb) {
+ bt_dev_err(hdev, "Failed to allocate devcoredump abort");
total: 0 errors, 9 warnings, 0 checks, 699 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/12940740.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: SubjectPrefix - FAIL - 1.74 seconds
Check subject contains "Bluetooth" prefix
"Bluetooth: " is not specified in the subject
"Bluetooth: " is not specified in the subject
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 16+ messages in thread