[PATCH v3 1/2] Bluetooth: btintel_pcie: Refactor Device Coredump

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Device coredumps do not include HCI traces, so the dependency on
hdev_devcd*() for generating device coredumps has been removed. In the
current implementation, firmware traces are embedded in skb and sent to
the host for coredump processing. This refactor updates the driver to
use devcore_dumpv() to generate coredumps directly, streamlining the
process.

Test:
1. cd /sys/bus/pci/devices/0000:00:14.7/
2. echo 1 > coredump
3. check /sys/class/devcoredump/ to device coredump folder

Fixes: 07e6bddb54b4 ("Bluetooth: btintel_pcie: Add support for device coredump")
Reviewed-by: Paul Menzel <pmenzel@xxxxxxxxxxxxx>
Signed-off-by: Kiran K <kiran.k@xxxxxxxxx>
---
changes in v3:
  - Make a seperate patch to include vendor and driver name in device coredump

changes in v2:
  - Fix compiler warning reported by android toolchain

 drivers/bluetooth/btintel_pcie.c | 211 ++++++++++---------------------
 1 file changed, 65 insertions(+), 146 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 22a2953adbd6..a78e24aa5e38 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -15,6 +15,7 @@
 #include <linux/interrupt.h>
 
 #include <linux/unaligned.h>
+#include <linux/devcoredump.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
@@ -559,25 +560,6 @@ static void btintel_pcie_mac_init(struct btintel_pcie_data *data)
 	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
 }
 
-static int btintel_pcie_add_dmp_data(struct hci_dev *hdev, const void *data, int size)
-{
-	struct sk_buff *skb;
-	int err;
-
-	skb = alloc_skb(size, GFP_ATOMIC);
-	if (!skb)
-		return -ENOMEM;
-
-	skb_put_data(skb, data, size);
-	err = hci_devcd_append(hdev, skb);
-	if (err) {
-		bt_dev_err(hdev, "Failed to append data in the coredump");
-		return err;
-	}
-
-	return 0;
-}
-
 static int btintel_pcie_get_mac_access(struct btintel_pcie_data *data)
 {
 	u32 reg;
@@ -622,30 +604,33 @@ static void btintel_pcie_release_mac_access(struct btintel_pcie_data *data)
 	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
 }
 
-static void btintel_pcie_copy_tlv(struct sk_buff *skb, enum btintel_pcie_tlv_type type,
-				  void *data, int size)
+static void *btintel_pcie_copy_tlv(void *dest, enum btintel_pcie_tlv_type type,
+				   void *data, size_t size)
 {
 	struct intel_tlv *tlv;
 
-	tlv = skb_put(skb, sizeof(*tlv) + size);
+	tlv = dest;
 	tlv->type = type;
 	tlv->len = size;
 	memcpy(tlv->val, data, tlv->len);
+	return dest + sizeof(*tlv) + size;
 }
 
 static int btintel_pcie_read_dram_buffers(struct btintel_pcie_data *data)
 {
-	u32 offset, prev_size, wr_ptr_status, dump_size, i;
+	u32 offset, prev_size, wr_ptr_status, dump_size, data_len;
 	struct btintel_pcie_dbgc *dbgc = &data->dbgc;
-	u8 buf_idx, dump_time_len, fw_build;
 	struct hci_dev *hdev = data->hdev;
+	u8 *pdata, *p, buf_idx;
 	struct intel_tlv *tlv;
 	struct timespec64 now;
-	struct sk_buff *skb;
 	struct tm tm_now;
-	char buf[256];
-	u16 hdr_len;
-	int ret;
+	char fw_build[128];
+	char ts[128];
+
+	if (!IS_ENABLED(CONFIG_DEV_COREDUMP))
+		return -EOPNOTSUPP;
+
 
 	wr_ptr_status = btintel_pcie_rd_dev_mem(data, BTINTEL_PCIE_DBGC_CUR_DBGBUFF_STATUS);
 	offset = wr_ptr_status & BTINTEL_PCIE_DBG_OFFSET_BIT_MASK;
@@ -664,92 +649,79 @@ static int btintel_pcie_read_dram_buffers(struct btintel_pcie_data *data)
 
 	ktime_get_real_ts64(&now);
 	time64_to_tm(now.tv_sec, 0, &tm_now);
-	dump_time_len = snprintf(buf, sizeof(buf), "Dump Time: %02d-%02d-%04ld %02d:%02d:%02d",
+	snprintf(ts, sizeof(ts), "Dump Time: %02d-%02d-%04ld %02d:%02d:%02d",
 				 tm_now.tm_mday, tm_now.tm_mon + 1, tm_now.tm_year + 1900,
 				 tm_now.tm_hour, tm_now.tm_min, tm_now.tm_sec);
 
-	fw_build = snprintf(buf + dump_time_len, sizeof(buf) - dump_time_len,
+	snprintf(fw_build, sizeof(fw_build),
 			    "Firmware Timestamp: Year %u WW %02u buildtype %u build %u",
 			    2000 + (data->dmp_hdr.fw_timestamp >> 8),
 			    data->dmp_hdr.fw_timestamp & 0xff, data->dmp_hdr.fw_build_type,
 			    data->dmp_hdr.fw_build_num);
 
-	hdr_len = sizeof(*tlv) + sizeof(data->dmp_hdr.cnvi_bt) +
-		  sizeof(*tlv) + sizeof(data->dmp_hdr.write_ptr) +
-		  sizeof(*tlv) + sizeof(data->dmp_hdr.wrap_ctr) +
-		  sizeof(*tlv) + sizeof(data->dmp_hdr.trigger_reason) +
-		  sizeof(*tlv) + sizeof(data->dmp_hdr.fw_git_sha1) +
-		  sizeof(*tlv) + sizeof(data->dmp_hdr.cnvr_top) +
-		  sizeof(*tlv) + sizeof(data->dmp_hdr.cnvi_top) +
-		  sizeof(*tlv) + dump_time_len +
-		  sizeof(*tlv) + fw_build;
+	data_len = sizeof(*tlv) + sizeof(data->dmp_hdr.cnvi_bt) +
+		sizeof(*tlv) + sizeof(data->dmp_hdr.write_ptr) +
+		sizeof(*tlv) + sizeof(data->dmp_hdr.wrap_ctr) +
+		sizeof(*tlv) + sizeof(data->dmp_hdr.trigger_reason) +
+		sizeof(*tlv) + sizeof(data->dmp_hdr.fw_git_sha1) +
+		sizeof(*tlv) + sizeof(data->dmp_hdr.cnvr_top) +
+		sizeof(*tlv) + sizeof(data->dmp_hdr.cnvi_top) +
+		sizeof(*tlv) + strlen(ts) +
+		sizeof(*tlv) + strlen(fw_build);
 
-	dump_size = hdr_len + sizeof(hdr_len);
+	/*
+	 * sizeof(u32) - signature
+	 * sizeof(data_len) - to store tlv data size
+	 * data_len - TLV data
+	 */
+	dump_size = sizeof(u32) + sizeof(data_len) + data_len;
 
-	skb = alloc_skb(dump_size, GFP_KERNEL);
-	if (!skb)
-		return -ENOMEM;
 
 	/* Add debug buffers data length to dump size */
 	dump_size += BTINTEL_PCIE_DBGC_BUFFER_SIZE * dbgc->count;
 
-	ret = hci_devcd_init(hdev, dump_size);
-	if (ret) {
-		bt_dev_err(hdev, "Failed to init devcoredump, err %d", ret);
-		kfree_skb(skb);
-		return ret;
-	}
+	pdata = vmalloc(dump_size);
+	if (!pdata)
+		return -ENOMEM;
+	p = pdata;
 
-	skb_put_data(skb, &hdr_len, sizeof(hdr_len));
+	*(u32 *)p = BTINTEL_PCIE_MAGIC_NUM;
+	p += sizeof(u32);
 
-	btintel_pcie_copy_tlv(skb, BTINTEL_CNVI_BT, &data->dmp_hdr.cnvi_bt,
-			      sizeof(data->dmp_hdr.cnvi_bt));
+	*(u32 *)p = data_len;
+	p += sizeof(u32);
 
-	btintel_pcie_copy_tlv(skb, BTINTEL_WRITE_PTR, &data->dmp_hdr.write_ptr,
-			      sizeof(data->dmp_hdr.write_ptr));
+	p = btintel_pcie_copy_tlv(p, BTINTEL_DUMP_TIME, ts, strlen(ts));
+	p = btintel_pcie_copy_tlv(p, BTINTEL_FW_BUILD, fw_build,
+				  strlen(fw_build));
+	p = btintel_pcie_copy_tlv(p, BTINTEL_CNVI_BT, &data->dmp_hdr.cnvi_bt,
+				  sizeof(data->dmp_hdr.cnvi_bt));
+	p = btintel_pcie_copy_tlv(p, BTINTEL_WRITE_PTR, &data->dmp_hdr.write_ptr,
+				  sizeof(data->dmp_hdr.write_ptr));
+	p = btintel_pcie_copy_tlv(p, BTINTEL_WRAP_CTR, &data->dmp_hdr.wrap_ctr,
+				  sizeof(data->dmp_hdr.wrap_ctr));
 
 	data->dmp_hdr.wrap_ctr = btintel_pcie_rd_dev_mem(data,
 							 BTINTEL_PCIE_DBGC_DBGBUFF_WRAP_ARND);
 
-	btintel_pcie_copy_tlv(skb, BTINTEL_WRAP_CTR, &data->dmp_hdr.wrap_ctr,
-			      sizeof(data->dmp_hdr.wrap_ctr));
-
-	btintel_pcie_copy_tlv(skb, BTINTEL_TRIGGER_REASON, &data->dmp_hdr.trigger_reason,
-			      sizeof(data->dmp_hdr.trigger_reason));
-
-	btintel_pcie_copy_tlv(skb, BTINTEL_FW_SHA, &data->dmp_hdr.fw_git_sha1,
-			      sizeof(data->dmp_hdr.fw_git_sha1));
-
-	btintel_pcie_copy_tlv(skb, BTINTEL_CNVR_TOP, &data->dmp_hdr.cnvr_top,
-			      sizeof(data->dmp_hdr.cnvr_top));
-
-	btintel_pcie_copy_tlv(skb, BTINTEL_CNVI_TOP, &data->dmp_hdr.cnvi_top,
-			      sizeof(data->dmp_hdr.cnvi_top));
-
-	btintel_pcie_copy_tlv(skb, BTINTEL_DUMP_TIME, buf, dump_time_len);
-
-	btintel_pcie_copy_tlv(skb, BTINTEL_FW_BUILD, buf + dump_time_len, fw_build);
-
-	ret = hci_devcd_append(hdev, skb);
-	if (ret)
-		goto exit_err;
-
-	for (i = 0; i < dbgc->count; i++) {
-		ret = btintel_pcie_add_dmp_data(hdev, dbgc->bufs[i].data,
-						BTINTEL_PCIE_DBGC_BUFFER_SIZE);
-		if (ret)
-			break;
-	}
-
-exit_err:
-	hci_devcd_complete(hdev);
-	return ret;
+	p = btintel_pcie_copy_tlv(p, BTINTEL_TRIGGER_REASON, &data->dmp_hdr.trigger_reason,
+				  sizeof(data->dmp_hdr.trigger_reason));
+	p = btintel_pcie_copy_tlv(p, BTINTEL_FW_SHA, &data->dmp_hdr.fw_git_sha1,
+				  sizeof(data->dmp_hdr.fw_git_sha1));
+	p = btintel_pcie_copy_tlv(p, BTINTEL_CNVR_TOP, &data->dmp_hdr.cnvr_top,
+				  sizeof(data->dmp_hdr.cnvr_top));
+	p = btintel_pcie_copy_tlv(p, BTINTEL_CNVI_TOP, &data->dmp_hdr.cnvi_top,
+				  sizeof(data->dmp_hdr.cnvi_top));
+
+	memcpy(p, dbgc->bufs[0].data, dbgc->count * BTINTEL_PCIE_DBGC_BUFFER_SIZE);
+	dev_coredumpv(&hdev->dev, pdata, dump_size, GFP_KERNEL);
+	return 0;
 }
 
 static void btintel_pcie_dump_traces(struct hci_dev *hdev)
 {
 	struct btintel_pcie_data *data = hci_get_drvdata(hdev);
-	int ret = 0;
+	int ret;
 
 	ret = btintel_pcie_get_mac_access(data);
 	if (ret) {
@@ -765,51 +737,6 @@ static void btintel_pcie_dump_traces(struct hci_dev *hdev)
 		bt_dev_err(hdev, "Failed to dump traces: (%d)", ret);
 }
 
-static void btintel_pcie_dump_hdr(struct hci_dev *hdev, struct sk_buff *skb)
-{
-	struct btintel_pcie_data *data = hci_get_drvdata(hdev);
-	u16 len = skb->len;
-	u16 *hdrlen_ptr;
-	char buf[80];
-
-	hdrlen_ptr = skb_put_zero(skb, sizeof(len));
-
-	snprintf(buf, sizeof(buf), "Controller Name: 0x%X\n",
-		 INTEL_HW_VARIANT(data->dmp_hdr.cnvi_bt));
-	skb_put_data(skb, buf, strlen(buf));
-
-	snprintf(buf, sizeof(buf), "Firmware Build Number: %u\n",
-		 data->dmp_hdr.fw_build_num);
-	skb_put_data(skb, buf, strlen(buf));
-
-	snprintf(buf, sizeof(buf), "Driver: %s\n", data->dmp_hdr.driver_name);
-	skb_put_data(skb, buf, strlen(buf));
-
-	snprintf(buf, sizeof(buf), "Vendor: Intel\n");
-	skb_put_data(skb, buf, strlen(buf));
-
-	*hdrlen_ptr = skb->len - len;
-}
-
-static void btintel_pcie_dump_notify(struct hci_dev *hdev, int state)
-{
-	struct btintel_pcie_data *data = hci_get_drvdata(hdev);
-
-	switch (state) {
-	case HCI_DEVCOREDUMP_IDLE:
-		data->dmp_hdr.state = HCI_DEVCOREDUMP_IDLE;
-		break;
-	case HCI_DEVCOREDUMP_ACTIVE:
-		data->dmp_hdr.state = HCI_DEVCOREDUMP_ACTIVE;
-		break;
-	case HCI_DEVCOREDUMP_TIMEOUT:
-	case HCI_DEVCOREDUMP_ABORT:
-	case HCI_DEVCOREDUMP_DONE:
-		data->dmp_hdr.state = HCI_DEVCOREDUMP_IDLE;
-		break;
-	}
-}
-
 /* This function enables BT function by setting BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_INIT bit in
  * BTINTEL_PCIE_CSR_FUNC_CTRL_REG register and wait for MSI-X with
  * BTINTEL_PCIE_MSIX_HW_INT_CAUSES_GP0.
@@ -1383,6 +1310,11 @@ static void btintel_pcie_rx_work(struct work_struct *work)
 					struct btintel_pcie_data, rx_work);
 	struct sk_buff *skb;
 
+	if (test_bit(BTINTEL_PCIE_COREDUMP_INPROGRESS, &data->flags)) {
+		btintel_pcie_dump_traces(data->hdev);
+		clear_bit(BTINTEL_PCIE_COREDUMP_INPROGRESS, &data->flags);
+	}
+
 	if (test_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags)) {
 		/* Unlike usb products, controller will not send hardware
 		 * exception event on exception. Instead controller writes the
@@ -1395,11 +1327,6 @@ static void btintel_pcie_rx_work(struct work_struct *work)
 		clear_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags);
 	}
 
-	if (test_bit(BTINTEL_PCIE_COREDUMP_INPROGRESS, &data->flags)) {
-		btintel_pcie_dump_traces(data->hdev);
-		clear_bit(BTINTEL_PCIE_COREDUMP_INPROGRESS, &data->flags);
-	}
-
 	/* Process the sk_buf in queue and send to the HCI layer */
 	while ((skb = skb_dequeue(&data->rx_skb_q))) {
 		btintel_pcie_recv_frame(data, skb);
@@ -2190,13 +2117,6 @@ static int btintel_pcie_setup_internal(struct hci_dev *hdev)
 	if (ver_tlv.img_type == 0x02 || ver_tlv.img_type == 0x03)
 		data->dmp_hdr.fw_git_sha1 = ver_tlv.git_sha1;
 
-	err = hci_devcd_register(hdev, btintel_pcie_dump_traces, btintel_pcie_dump_hdr,
-				 btintel_pcie_dump_notify);
-	if (err) {
-		bt_dev_err(hdev, "Failed to register coredump (%d)", err);
-		goto exit_error;
-	}
-
 	btintel_print_fseq_info(hdev);
 exit_error:
 	kfree_skb(skb);
@@ -2325,7 +2245,6 @@ static void btintel_pcie_removal_work(struct work_struct *wk)
 	btintel_pcie_synchronize_irqs(data);
 
 	flush_work(&data->rx_work);
-	flush_work(&data->hdev->dump.dump_rx);
 
 	bt_dev_dbg(data->hdev, "Release bluetooth interface");
 	btintel_pcie_release_hdev(data);
-- 
2.43.0





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux