2019-05-06 14:26:42

by Govind Singh

[permalink] [raw]
Subject: [PATCH v2 1/1] ath10k: Enable MSA region dump support for WCN3990

MSA memory region caries the hw descriptors information.
Dump MSA region in core dump as this is very helpful in debugging
hw issues.

Testing: Tested on WCN3990 HW
Tested FW: WLAN.HL.3.1-00959-QCAHLSWMTPLZ-1

Signed-off-by: Govind Singh <[email protected]>
---
drivers/net/wireless/ath/ath10k/coredump.c | 21 +++++++
drivers/net/wireless/ath/ath10k/coredump.h | 1 +
drivers/net/wireless/ath/ath10k/qmi.c | 6 ++
drivers/net/wireless/ath/ath10k/snoc.c | 67 ++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/snoc.h | 1 +
5 files changed, 96 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/coredump.c b/drivers/net/wireless/ath/ath10k/coredump.c
index eadae2f9206b..56d62035c988 100644
--- a/drivers/net/wireless/ath/ath10k/coredump.c
+++ b/drivers/net/wireless/ath/ath10k/coredump.c
@@ -962,6 +962,19 @@ static const struct ath10k_mem_region qca4019_hw10_mem_regions[] = {
},
};

+static const struct ath10k_mem_region wcn399x_hw10_mem_regions[] = {
+ {
+ /* MSA region start is not fixed, hence it is assigned at runtime */
+ .type = ATH10K_MEM_REGION_TYPE_MSA,
+ .len = 0x100000,
+ .name = "DRAM",
+ .section_table = {
+ .sections = NULL,
+ .size = 0,
+ },
+ },
+};
+
static const struct ath10k_hw_mem_layout hw_mem_layouts[] = {
{
.hw_id = QCA6174_HW_1_0_VERSION,
@@ -1059,6 +1072,14 @@ static const struct ath10k_hw_mem_layout hw_mem_layouts[] = {
.size = ARRAY_SIZE(qca4019_hw10_mem_regions),
},
},
+ {
+ .hw_id = WCN3990_HW_1_0_DEV_VERSION,
+ .hw_rev = ATH10K_HW_WCN3990,
+ .region_table = {
+ .regions = wcn399x_hw10_mem_regions,
+ .size = ARRAY_SIZE(wcn399x_hw10_mem_regions),
+ },
+ },
};

static u32 ath10k_coredump_get_ramdump_size(struct ath10k *ar)
diff --git a/drivers/net/wireless/ath/ath10k/coredump.h b/drivers/net/wireless/ath/ath10k/coredump.h
index 5dac653e1649..9802e90483f4 100644
--- a/drivers/net/wireless/ath/ath10k/coredump.h
+++ b/drivers/net/wireless/ath/ath10k/coredump.h
@@ -126,6 +126,7 @@ enum ath10k_mem_region_type {
ATH10K_MEM_REGION_TYPE_IRAM2 = 5,
ATH10K_MEM_REGION_TYPE_IOSRAM = 6,
ATH10K_MEM_REGION_TYPE_IOREG = 7,
+ ATH10K_MEM_REGION_TYPE_MSA = 8,
};

/* Define a section of the region which should be copied. As not all parts
diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c
index ba8f5a8f83d1..47da492a326b 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -817,9 +817,15 @@ ath10k_qmi_driver_event_post(struct ath10k_qmi *qmi,
static void ath10k_qmi_event_server_exit(struct ath10k_qmi *qmi)
{
struct ath10k *ar = qmi->ar;
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);

ath10k_qmi_remove_msa_permission(qmi);
ath10k_core_free_board_files(ar);
+ if (!test_bit(ATH10K_SNOC_FLAG_UNREGISTERING, &ar_snoc->flags)) {
+ ath10k_qmi_remove_msa_permission(qmi);
+ ath10k_snoc_fw_crashed_dump(ar);
+ ath10k_qmi_setup_msa_permissions(qmi);
+ }
ath10k_snoc_fw_indication(ar, ATH10K_QMI_EVENT_FW_DOWN_IND);
ath10k_dbg(ar, ATH10K_DBG_QMI, "wifi fw qmi service disconnected\n");
}
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 0be12996beba..252dd4ee782d 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -24,6 +24,7 @@
#include <linux/regulator/consumer.h>

#include "ce.h"
+#include "coredump.h"
#include "debug.h"
#include "hif.h"
#include "htc.h"
@@ -1586,6 +1587,72 @@ static int ath10k_hw_power_off(struct ath10k *ar)
return ret;
}

+static void ath10k_msa_dump_memory(struct ath10k *ar,
+ struct ath10k_fw_crash_data *crash_data)
+{
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+ const struct ath10k_hw_mem_layout *mem_layout;
+ const struct ath10k_mem_region *current_region;
+ struct ath10k_dump_ram_data_hdr *hdr;
+ size_t buf_len;
+ u8 *buf;
+
+ lockdep_assert_held(&ar->data_lock);
+
+ if (!crash_data && !crash_data->ramdump_buf)
+ return;
+
+ mem_layout = ath10k_coredump_get_mem_layout(ar);
+ if (!mem_layout)
+ return;
+
+ current_region = &mem_layout->region_table.regions[0];
+
+ buf = crash_data->ramdump_buf;
+ buf_len = crash_data->ramdump_buf_len;
+ memset(buf, 0, buf_len);
+
+ /* Reserve space for the header. */
+ hdr = (void *)buf;
+ buf += sizeof(*hdr);
+ buf_len -= sizeof(*hdr);
+
+ hdr->region_type = cpu_to_le32(current_region->type);
+ hdr->start = cpu_to_le32(ar_snoc->qmi->msa_va);
+ hdr->length = cpu_to_le32(ar_snoc->qmi->msa_mem_size);
+
+ if (current_region->len < ar_snoc->qmi->msa_mem_size) {
+ memcpy(buf, ar_snoc->qmi->msa_va, current_region->len);
+ ath10k_warn(ar, "msa dump length is less than msa size %x, %x\n",
+ current_region->len, ar_snoc->qmi->msa_mem_size);
+ } else {
+ memcpy(buf, ar_snoc->qmi->msa_va, ar_snoc->qmi->msa_mem_size);
+ }
+}
+
+void ath10k_snoc_fw_crashed_dump(struct ath10k *ar)
+{
+ struct ath10k_fw_crash_data *crash_data;
+ char guid[UUID_STRING_LEN + 1];
+
+ spin_lock_bh(&ar->data_lock);
+
+ ar->stats.fw_crash_counter++;
+
+ crash_data = ath10k_coredump_new(ar);
+
+ if (crash_data)
+ scnprintf(guid, sizeof(guid), "%pUl", &crash_data->guid);
+ else
+ scnprintf(guid, sizeof(guid), "n/a");
+
+ ath10k_err(ar, "firmware crashed! (guid %s)\n", guid);
+ ath10k_print_driver_info(ar);
+ ath10k_msa_dump_memory(ar, crash_data);
+
+ spin_unlock_bh(&ar->data_lock);
+}
+
static const struct of_device_id ath10k_snoc_dt_match[] = {
{ .compatible = "qcom,wcn3990-wifi",
.data = &drv_priv,
diff --git a/drivers/net/wireless/ath/ath10k/snoc.h b/drivers/net/wireless/ath/ath10k/snoc.h
index 25383de8f17d..6d28a6290a94 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.h
+++ b/drivers/net/wireless/ath/ath10k/snoc.h
@@ -101,5 +101,6 @@ static inline struct ath10k_snoc *ath10k_snoc_priv(struct ath10k *ar)
}

int ath10k_snoc_fw_indication(struct ath10k *ar, u64 type);
+void ath10k_snoc_fw_crashed_dump(struct ath10k *ar);

#endif /* _SNOC_H_ */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2019-05-06 16:35:34

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] ath10k: Enable MSA region dump support for WCN3990

On Mon, May 6, 2019 at 7:26 AM Govind Singh <[email protected]> wrote:
> --- a/drivers/net/wireless/ath/ath10k/snoc.c
> +++ b/drivers/net/wireless/ath/ath10k/snoc.c

> @@ -1586,6 +1587,72 @@ static int ath10k_hw_power_off(struct ath10k *ar)
> return ret;
> }
>
> +static void ath10k_msa_dump_memory(struct ath10k *ar,
> + struct ath10k_fw_crash_data *crash_data)
> +{
> + struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
> + const struct ath10k_hw_mem_layout *mem_layout;
> + const struct ath10k_mem_region *current_region;
> + struct ath10k_dump_ram_data_hdr *hdr;
> + size_t buf_len;
> + u8 *buf;
> +
> + lockdep_assert_held(&ar->data_lock);

I believe that's the wrong lock now. See below.

> +
> + if (!crash_data && !crash_data->ramdump_buf)
> + return;
> +
> + mem_layout = ath10k_coredump_get_mem_layout(ar);
> + if (!mem_layout)
> + return;
> +
> + current_region = &mem_layout->region_table.regions[0];
> +
> + buf = crash_data->ramdump_buf;
> + buf_len = crash_data->ramdump_buf_len;
> + memset(buf, 0, buf_len);
> +
> + /* Reserve space for the header. */
> + hdr = (void *)buf;
> + buf += sizeof(*hdr);
> + buf_len -= sizeof(*hdr);
> +
> + hdr->region_type = cpu_to_le32(current_region->type);
> + hdr->start = cpu_to_le32(ar_snoc->qmi->msa_va);
> + hdr->length = cpu_to_le32(ar_snoc->qmi->msa_mem_size);
> +
> + if (current_region->len < ar_snoc->qmi->msa_mem_size) {
> + memcpy(buf, ar_snoc->qmi->msa_va, current_region->len);
> + ath10k_warn(ar, "msa dump length is less than msa size %x, %x\n",
> + current_region->len, ar_snoc->qmi->msa_mem_size);
> + } else {
> + memcpy(buf, ar_snoc->qmi->msa_va, ar_snoc->qmi->msa_mem_size);
> + }
> +}
> +
> +void ath10k_snoc_fw_crashed_dump(struct ath10k *ar)
> +{
> + struct ath10k_fw_crash_data *crash_data;
> + char guid[UUID_STRING_LEN + 1];
> +
> + spin_lock_bh(&ar->data_lock);
> +
> + ar->stats.fw_crash_counter++;
> +
> + crash_data = ath10k_coredump_new(ar);

This will (for good reason) spit a lockdep warning after this, I think:

38faed150438 ath10k: perform crash dump collection in workqueue

You need to hold 'dump_mutex' now. I believe you only need to hold
'data_lock' for the sake of the crash counter.

Brian

> +
> + if (crash_data)
> + scnprintf(guid, sizeof(guid), "%pUl", &crash_data->guid);
> + else
> + scnprintf(guid, sizeof(guid), "n/a");
> +
> + ath10k_err(ar, "firmware crashed! (guid %s)\n", guid);
> + ath10k_print_driver_info(ar);
> + ath10k_msa_dump_memory(ar, crash_data);
> +
> + spin_unlock_bh(&ar->data_lock);
> +}