On 7/15/2024 11:39 PM, Sowmiya Sree Elavalagan wrote:
> In case of firmware assert snapshot of firmware memory is essential for
> debugging. Add firmware coredump collection support for PCI bus.
> Collect RDDM and firmware paging dumps from MHI and pack them in TLV
> format and also pack various memory shared during QMI phase in separate
> TLVs. Add necessary header and share the dumps to user space using dev
> coredump framework. Coredump collection is disabled by default and can
> be enabled using menuconfig. Dump collected for a radio is 55 MB
> approximately.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.2.1-00201-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>
> Signed-off-by: Sowmiya Sree Elavalagan <quic_ssreeela@xxxxxxxxxxx>
> ---
> v4:
> - Fixed Kasan warning vmalloc-out-of-bounds in ath12k_pci_coredump_download
> - Rebased on ToT
> v3:
> - Fixed SPDX comment style for coredump.c file
> Changed Kconfig description.
> v2:
> - Fixed errors shown by ath12k-check
> ---
...
> + dump_tlv = buf;
> + dump_tlv->type = cpu_to_le32(FW_CRASH_DUMP_RDDM_DATA);
> + dump_tlv->tlv_len = cpu_to_le32(dump_seg_sz[FW_CRASH_DUMP_RDDM_DATA]);
> + buf += COREDUMP_TLV_HDR_SIZE;
> +
> + /* append all segments together as they are all part of a single contiguous
> + * block of memory
> + */
> + for (i = 0; i < rddm_img->entries; i++) {
> + if (!rddm_img->mhi_buf[i].buf)
> + continue;
> +
> + memcpy_fromio(buf, (void const __iomem *)rddm_img->mhi_buf[i].buf,
> + rddm_img->mhi_buf[i].len);
> + buf += rddm_img->mhi_buf[i].len;
> + }
> +
> + mem_idx = FW_CRASH_DUMP_REMOTE_MEM_DATA;
> + for (; mem_idx < FW_CRASH_DUMP_TYPE_MAX; mem_idx++) {
> + if (!dump_seg_sz[i] || mem_idx == FW_CRASH_DUMP_NONE)
this looks really strange testing dump_seg_size[i]
the first time through the loop i will be set to the value of
rddm_img->entries since that is the value it will have from:
for (i = 0; i < rddm_img->entries; i++) {
but subsequent times through the loop i will be set to the value of
ab->qmi.mem_seg_count since that is the value it will have from:
for (i = 0; i < ab->qmi.mem_seg_count; i++) {
did you really want to test dump_seg_size[mem_idx]?
> + continue;
> +
> + dump_tlv = buf;
> + dump_tlv->type = cpu_to_le32(mem_idx);
> + dump_tlv->tlv_len = cpu_to_le32(dump_seg_sz[mem_idx]);
> + buf += COREDUMP_TLV_HDR_SIZE;
> +
> + for (i = 0; i < ab->qmi.mem_seg_count; i++) {
> + mem_type = ath12k_coredump_get_dump_type
> + (ab->qmi.target_mem[i].type);
> +
> + if (mem_type != mem_idx)
> + continue;
> +
> + if (!ab->qmi.target_mem[i].paddr) {
> + ath12k_dbg(ab, ATH12K_DBG_PCI,
> + "Skipping mem region type %d",
> + ab->qmi.target_mem[i].type);
> + continue;
> + }
> +
> + memcpy_fromio(buf, ab->qmi.target_mem[i].v.ioaddr,
> + ab->qmi.target_mem[i].size);
> + buf += ab->qmi.target_mem[i].size;
> + }
> + }
> +
> + queue_work(ab->workqueue, &ab->dump_work);
> +}
