2020-08-07 23:33:08

by Gurbir Arora

[permalink] [raw]
Subject: [PATCH 0/3] Introduce mini-dump support for remoteproc.

Sometimes firmware sizes can be in ten's of MB's and reading
all the memory during coredump can consume lot of time and
memory.
Introducing support for mini-dumps. Mini-dump contains smallest
amount of useful information, that could help to debug subsystem
crashes.
During bootup memory is allocated in SMEM (Shared memory)
in the form of a table that contains the physical
addresses and sizes of the regions that are supposed to be
collected during coredump. This memory is shared amongst all
processors in a Qualcomm platform, so all remoteprocs
fill in their entry in the global table once they are out
of reset.
This patch series adds support for parsing the global minidump
table and uses the current coredump frameork to expose this memory
to userspace during remoteproc's recovery.

This patch series also integrates the patch:
https://patchwork.kernel.org/patch/11695541/ sent by Siddharth.

Gurbir Arora (2):
remoteproc: core: Add coredump to remoteproc ops
remoteproc: qcom: Add capability to collect minidumps

Rishabh Bhatnagar (1):
remoteproc: qcom: Add minidump id for sm8150 modem remoteproc

drivers/remoteproc/qcom_minidump.h | 64 +++++++++
drivers/remoteproc/qcom_q6v5_pas.c | 21 ++-
drivers/remoteproc/remoteproc_core.c | 7 +-
drivers/remoteproc/remoteproc_coredump.c | 211 +++++++++++++++++++++++++++-
drivers/remoteproc/remoteproc_elf_helpers.h | 27 ++++
drivers/remoteproc/remoteproc_internal.h | 2 +
include/linux/remoteproc.h | 3 +
7 files changed, 331 insertions(+), 4 deletions(-)
create mode 100644 drivers/remoteproc/qcom_minidump.h

--
1.9.1


2020-08-07 23:35:17

by Gurbir Arora

[permalink] [raw]
Subject: [PATCH 3/3] remoteproc: qcom: Add minidump id for sm8150 modem remoteproc

Add minidump id for modem in sm8150 chipset.

Signed-off-by: Rishabh Bhatnagar <[email protected]>
Signed-off-by: Gurbir Arora <[email protected]>
---
drivers/remoteproc/qcom_q6v5_pas.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 270c640..ce2a83d 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -623,6 +623,7 @@ static int adsp_remove(struct platform_device *pdev)
.crash_reason_smem = 421,
.firmware_name = "modem.mdt",
.pas_id = 4,
+ .minidump_id = 3,
.has_aggre2_clk = false,
.auto_boot = false,
.active_pd_names = (char*[]){
--
1.9.1

2020-08-07 23:35:50

by Gurbir Arora

[permalink] [raw]
Subject: [PATCH 1/3] remoteproc: core: Add coredump to remoteproc ops

Each remoteproc might have different requirements for coredumps and might
want to choose the type of dumps it wants to collect. This change allows
remoteproc drivers to specify their own custom dump function to be executed
in place of rproc_coredump. If the coredump op is not specified by the
remoteproc driver it will be set to rproc_coredump by default.

Signed-off-by: Rishabh Bhatnagar <[email protected]>
Signed-off-by: Siddharth Gupta <[email protected]>
Signed-off-by: Gurbir Arora <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 6 +++++-
include/linux/remoteproc.h | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 7f90eee..283ecb6 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1681,7 +1681,7 @@ int rproc_trigger_recovery(struct rproc *rproc)
goto unlock_mutex;

/* generate coredump */
- rproc_coredump(rproc);
+ rproc->ops->coredump(rproc);

/* load firmware */
ret = request_firmware(&firmware_p, rproc->firmware, dev);
@@ -2113,6 +2113,10 @@ static int rproc_alloc_ops(struct rproc *rproc, const struct rproc_ops *ops)
rproc->ops->sanity_check = rproc_elf_sanity_check;
rproc->ops->get_boot_addr = rproc_elf_get_boot_addr;

+ /* Default to rproc_coredump if no coredump functions is specified */
+ if (!rproc->ops->coredump)
+ rproc->ops->coredump = rproc_coredump;
+
return 0;
}

diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 2fa68bf..0ed1a2b 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -392,6 +392,7 @@ struct rproc_ops {
int (*load)(struct rproc *rproc, const struct firmware *fw);
int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
u64 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
+ void (*coredump)(struct rproc *rproc);
unsigned long (*panic)(struct rproc *rproc);
};

--
1.9.1

2020-08-07 23:35:59

by Gurbir Arora

[permalink] [raw]
Subject: [PATCH 2/3] remoteproc: qcom: Add capability to collect minidumps

This patch adds support for collecting minidump in the
event of remoteproc crash. Parse the minidump table
based on remoteproc's unique minidump-id, read all
memory regions from the remoteproc's minidump table
entry and expose the memory to userspace.
The remoteproc platform driver can choose to collect
a full/mini dump by specifying the coredump op.

Signed-off-by: Rishabh Bhatnagar <[email protected]>
Signed-off-by: Gurbir Arora <[email protected]>
---
drivers/remoteproc/qcom_minidump.h | 64 +++++++++
drivers/remoteproc/qcom_q6v5_pas.c | 20 ++-
drivers/remoteproc/remoteproc_core.c | 1 +
drivers/remoteproc/remoteproc_coredump.c | 211 +++++++++++++++++++++++++++-
drivers/remoteproc/remoteproc_elf_helpers.h | 27 ++++
drivers/remoteproc/remoteproc_internal.h | 2 +
include/linux/remoteproc.h | 2 +
7 files changed, 324 insertions(+), 3 deletions(-)
create mode 100644 drivers/remoteproc/qcom_minidump.h

diff --git a/drivers/remoteproc/qcom_minidump.h b/drivers/remoteproc/qcom_minidump.h
new file mode 100644
index 0000000..437e030
--- /dev/null
+++ b/drivers/remoteproc/qcom_minidump.h
@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef __QCOM_MINIDUMP_H
+#define __QCOM_MINIDUMP_H
+
+#define MAX_NUM_OF_SS 10
+#define MAX_REGION_NAME_LENGTH 16
+#define SBL_MINIDUMP_SMEM_ID 602
+#define MD_REGION_VALID ('V' << 24 | 'A' << 16 | 'L' << 8 | 'I' << 0)
+#define MD_SS_ENCR_DONE ('D' << 24 | 'O' << 16 | 'N' << 8 | 'E' << 0)
+#define MD_SS_ENABLED ('E' << 24 | 'N' << 16 | 'B' << 8 | 'L' << 0)
+
+/**
+ * md_ss_region - Minidump region
+ * @name : Name of the region to be dumped
+ * @seq_num: : Use to differentiate regions with same name.
+ * @md_valid : This entry to be dumped (if set to 1)
+ * @region_base_address : Physical address of region to be dumped
+ * @region_size : Size of the region
+ */
+struct md_ss_region {
+ char name[MAX_REGION_NAME_LENGTH];
+ u32 seq_num;
+ u32 md_valid;
+ u64 region_base_address;
+ u64 region_size;
+};
+
+/**
+ * md_ss_toc: Subsystem's SMEM Table of content
+ * @md_ss_toc_init : Subsystem toc init status
+ * @md_ss_enable_status : if set to 1, this region would be copied during coredump
+ * @encryption_status: Encryption status for this subsystem
+ * @encryption_required : Decides to encrypt the subsystem regions or not
+ * @ss_region_count : Number of regions added in this subsystem toc
+ * @md_ss_smem_regions_baseptr : regions base pointer of the subsystem
+ */
+struct md_ss_toc {
+ u32 md_ss_toc_init;
+ u32 md_ss_enable_status;
+ u32 encryption_status;
+ u32 encryption_required;
+ u32 ss_region_count;
+ u64 md_ss_smem_regions_baseptr;
+};
+
+/**
+ * md_global_toc: Global Table of Content
+ * @md_toc_init : Global Minidump init status
+ * @md_revision : Minidump revision
+ * @md_enable_status : Minidump enable status
+ * @md_ss_toc : Array of subsystems toc
+ */
+struct md_global_toc {
+ u32 md_toc_init;
+ u32 md_revision;
+ u32 md_enable_status;
+ struct md_ss_toc md_ss_toc[MAX_NUM_OF_SS];
+};
+
+#endif
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 3837f23..270c640 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -33,6 +33,7 @@ struct adsp_data {
int crash_reason_smem;
const char *firmware_name;
int pas_id;
+ int minidump_id;
bool has_aggre2_clk;
bool auto_boot;

@@ -258,6 +259,15 @@ static unsigned long adsp_panic(struct rproc *rproc)
.panic = adsp_panic,
};

+static struct rproc_ops adsp_minidump_ops = {
+ .start = adsp_start,
+ .stop = adsp_stop,
+ .da_to_va = adsp_da_to_va,
+ .load = adsp_load,
+ .coredump = rproc_minidump,
+ .panic = adsp_panic,
+};
+
static int adsp_init_clock(struct qcom_adsp *adsp)
{
int ret;
@@ -398,14 +408,20 @@ static int adsp_probe(struct platform_device *pdev)
if (ret < 0 && ret != -EINVAL)
return ret;

- rproc = rproc_alloc(&pdev->dev, pdev->name, &adsp_ops,
- fw_name, sizeof(*adsp));
+ if (desc->minidump_id)
+ rproc = rproc_alloc(&pdev->dev, pdev->name, &adsp_minidump_ops,
+ fw_name, sizeof(*adsp));
+ else
+ rproc = rproc_alloc(&pdev->dev, pdev->name, &adsp_ops, fw_name,
+ sizeof(*adsp));
+
if (!rproc) {
dev_err(&pdev->dev, "unable to allocate remoteproc\n");
return -ENOMEM;
}

rproc->auto_boot = desc->auto_boot;
+ rproc->minidump_id = desc->minidump_id;
rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);

adsp = (struct qcom_adsp *)rproc->priv;
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 283ecb6..1835821 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1324,6 +1324,7 @@ void rproc_resource_cleanup(struct rproc *rproc)
list_for_each_entry_safe(rvdev, rvtmp, &rproc->rvdevs, node)
kref_put(&rvdev->refcount, rproc_vdev_release);

+ rproc_cleanup_priv(rproc);
rproc_coredump_cleanup(rproc);
}
EXPORT_SYMBOL(rproc_resource_cleanup);
diff --git a/drivers/remoteproc/remoteproc_coredump.c b/drivers/remoteproc/remoteproc_coredump.c
index bb15a29..232804d 100644
--- a/drivers/remoteproc/remoteproc_coredump.c
+++ b/drivers/remoteproc/remoteproc_coredump.c
@@ -12,6 +12,11 @@
#include <linux/remoteproc.h>
#include "remoteproc_internal.h"
#include "remoteproc_elf_helpers.h"
+#include <linux/soc/qcom/smem.h>
+#include "qcom_minidump.h"
+
+static struct md_global_toc *g_md_toc;
+#define MAX_STRTBL_SIZE 512

struct rproc_coredump_state {
struct rproc *rproc;
@@ -20,7 +25,7 @@ struct rproc_coredump_state {
};

/**
- * rproc_coredump_cleanup() - clean up dump_segments list
+ * rproc_coredump_cleanup() - clean up dump_segment's list
* @rproc: the remote processor handle
*/
void rproc_coredump_cleanup(struct rproc *rproc)
@@ -34,6 +39,18 @@ void rproc_coredump_cleanup(struct rproc *rproc)
}

/**
+ * rproc_coredump_cleanup() - free the dump_segment's private elements.
+ * @rproc: the remote processor handle
+ */
+void rproc_cleanup_priv(struct rproc *rproc)
+{
+ struct rproc_dump_segment *entry;
+
+ list_for_each_entry(entry, &rproc->dump_segments, node)
+ kfree(entry->priv);
+}
+
+/**
* rproc_coredump_add_segment() - add segment of device memory to coredump
* @rproc: handle of a remote processor
* @da: device address
@@ -323,3 +340,195 @@ void rproc_coredump(struct rproc *rproc)
*/
wait_for_completion(&dump_state.dump_done);
}
+
+static void do_rproc_minidump(struct rproc *rproc)
+{
+ struct rproc_dump_segment *segment;
+ void *shdr;
+ void *ehdr;
+ size_t data_size;
+ size_t offset;
+ void *data;
+ u8 class = rproc->elf_class;
+ int shnum;
+ struct rproc_coredump_state dump_state;
+ unsigned int dump_conf = rproc->dump_conf;
+ char *str_tbl = "STR_TBL";
+
+ if (list_empty(&rproc->dump_segments) ||
+ dump_conf == RPROC_COREDUMP_DISABLED)
+ return;
+
+ if (class == ELFCLASSNONE) {
+ dev_err(&rproc->dev, "Elf class is not set\n");
+ return;
+ }
+
+ /*
+ * We allocate two extra section headers. The first one is null.
+ * Second section header is for the string table. Also space is
+ * allocated for string table.
+ */
+ data_size = elf_size_of_hdr(class) + 2 * elf_size_of_shdr(class) +
+ MAX_STRTBL_SIZE;
+ shnum = 2;
+
+ list_for_each_entry(segment, &rproc->dump_segments, node) {
+ data_size += elf_size_of_shdr(class);
+ if (dump_conf == RPROC_COREDUMP_DEFAULT)
+ data_size += segment->size;
+ shnum++;
+ }
+
+ data = vmalloc(data_size);
+ if (!data)
+ return;
+
+ ehdr = data;
+ memset(ehdr, 0, elf_size_of_hdr(class));
+ /* e_ident field is common for both elf32 and elf64 */
+ elf_hdr_init_ident(ehdr, class);
+
+ elf_hdr_set_e_type(class, ehdr, ET_CORE);
+ elf_hdr_set_e_machine(class, ehdr, rproc->elf_machine);
+ elf_hdr_set_e_version(class, ehdr, EV_CURRENT);
+ elf_hdr_set_e_entry(class, ehdr, rproc->bootaddr);
+ elf_hdr_set_e_shoff(class, ehdr, elf_size_of_hdr(class));
+ elf_hdr_set_e_ehsize(class, ehdr, elf_size_of_hdr(class));
+ elf_hdr_set_e_shentsize(class, ehdr, elf_size_of_shdr(class));
+ elf_hdr_set_e_shnum(class, ehdr, shnum);
+ elf_hdr_set_e_shstrndx(class, ehdr, 1);
+
+ /* Set the first section header as null and move to the next one. */
+ shdr = data + elf_hdr_get_e_shoff(class, ehdr);
+ memset(shdr, 0, elf_size_of_shdr(class));
+ shdr += elf_size_of_shdr(class);
+
+ /* Initialize the string table. */
+ offset = elf_hdr_get_e_shoff(class, ehdr) +
+ elf_size_of_shdr(class) * elf_hdr_get_e_shnum(class, ehdr);
+ memset(data + offset, 0, MAX_STRTBL_SIZE);
+
+ /* Fill in the string table section header. */
+ memset(shdr, 0, elf_size_of_shdr(class));
+ elf_shdr_set_sh_type(class, shdr, SHT_STRTAB);
+ elf_shdr_set_sh_offset(class, shdr, offset);
+ elf_shdr_set_sh_size(class, shdr, MAX_STRTBL_SIZE);
+ elf_shdr_set_sh_entsize(class, shdr, 0);
+ elf_shdr_set_sh_flags(class, shdr, 0);
+ elf_shdr_set_sh_name(class, shdr, set_section_name(str_tbl, ehdr, class));
+ offset += elf_shdr_get_sh_size(class, shdr);
+ shdr += elf_size_of_shdr(class);
+
+ list_for_each_entry(segment, &rproc->dump_segments, node) {
+ memset(shdr, 0, elf_size_of_shdr(class));
+ elf_shdr_set_sh_type(class, shdr, SHT_PROGBITS);
+ elf_shdr_set_sh_offset(class, shdr, offset);
+ elf_shdr_set_sh_addr(class, shdr, segment->da);
+ elf_shdr_set_sh_size(class, shdr, segment->size);
+ elf_shdr_set_sh_entsize(class, shdr, 0);
+ elf_shdr_set_sh_flags(class, shdr, SHF_WRITE);
+ elf_shdr_set_sh_name(class, shdr,
+ set_section_name(segment->priv, ehdr, class));
+
+ /* No need to copy segments for inline dumps */
+ if (dump_conf == RPROC_COREDUMP_DEFAULT)
+ rproc_copy_segment(rproc, data + offset, segment, 0,
+ segment->size);
+ offset += elf_shdr_get_sh_size(class, shdr);
+ shdr += elf_size_of_shdr(class);
+ }
+
+ if (dump_conf == RPROC_COREDUMP_DEFAULT) {
+ dev_coredumpv(&rproc->dev, data, data_size, GFP_KERNEL);
+ return;
+ }
+
+ /* Initialize the dump state struct to be used by rproc_coredump_read */
+ dump_state.rproc = rproc;
+ dump_state.header = data;
+ init_completion(&dump_state.dump_done);
+
+ dev_coredumpm(&rproc->dev, NULL, &dump_state, data_size, GFP_KERNEL,
+ rproc_coredump_read, rproc_coredump_free);
+
+ /* Wait until the dump is read and free is called. Data is freed
+ * by devcoredump framework automatically after 5 minutes.
+ */
+ wait_for_completion(&dump_state.dump_done);
+}
+
+static void add_minidump_segments(struct rproc *rproc, struct md_ss_toc *minidump_ss)
+{
+ struct md_ss_region __iomem *region_info, *ptr;
+ int i, seg_cnt = minidump_ss->ss_region_count;
+ void __iomem *offset;
+ dma_addr_t da;
+ size_t size;
+ char *name;
+
+ if (!list_empty(&rproc->dump_segments))
+ goto minidump;
+
+ region_info = ioremap((unsigned long)minidump_ss->md_ss_smem_regions_baseptr,
+ seg_cnt * sizeof(struct md_ss_region));
+
+ if (!region_info) {
+ dev_err(&rproc->dev, "invalid minidump subsystem table\n");
+ return;
+ }
+
+ /* Iterate over the subsystem's segments and create rproc custom dump segments */
+ ptr = region_info;
+ offset = ptr;
+ for (i = 0; i < seg_cnt; i++) {
+ offset = region_info;
+ offset += sizeof(region_info->name) + sizeof(region_info->seq_num);
+ if (__raw_readl(offset) == MD_REGION_VALID) {
+ name = kmalloc(MAX_REGION_NAME_LENGTH, GFP_KERNEL);
+ memcpy(name, region_info, sizeof(region_info->name));
+ offset += sizeof(region_info->md_valid);
+ memcpy(&da, offset, sizeof(region_info->region_base_address));
+ offset += sizeof(region_info->region_base_address);
+ memcpy(&size, offset, sizeof(region_info->region_size));
+ rproc_coredump_add_custom_segment(rproc, da, size, NULL, name);
+ }
+ region_info++;
+ }
+
+minidump:
+ return do_rproc_minidump(rproc);
+}
+
+void rproc_minidump(struct rproc *rproc)
+{
+ struct md_ss_toc *minidump_ss;
+ size_t size;
+
+ /* Get Global minidump ToC*/
+ if (!g_md_toc)
+ g_md_toc = qcom_smem_get(QCOM_SMEM_HOST_ANY,
+ SBL_MINIDUMP_SMEM_ID, &size);
+
+ /* check if global table pointer exists and init is set */
+ if (IS_ERR(g_md_toc) || !(g_md_toc->md_toc_init)) {
+ dev_err(&rproc->dev, "SMEM is not initialized.\n");
+ return;
+ }
+
+ /* Get subsystem table of contents using the minidump id */
+ minidump_ss = &g_md_toc->md_ss_toc[rproc->minidump_id];
+
+ /**
+ * Collect minidump if SS ToC is valid and segment table
+ * is initialized in memory and encryption status is set.
+ */
+ if (minidump_ss->md_ss_smem_regions_baseptr != 0 &&
+ minidump_ss->md_ss_toc_init == 1 &&
+ minidump_ss->md_ss_enable_status == MD_SS_ENABLED &&
+ minidump_ss->encryption_status == MD_SS_ENCR_DONE)
+ return add_minidump_segments(rproc, minidump_ss);
+
+ dev_err(&rproc->dev, "Minidump not ready!! Aborting\n");
+}
+EXPORT_SYMBOL(rproc_minidump);
diff --git a/drivers/remoteproc/remoteproc_elf_helpers.h b/drivers/remoteproc/remoteproc_elf_helpers.h
index 4b6be7b..d83ebca 100644
--- a/drivers/remoteproc/remoteproc_elf_helpers.h
+++ b/drivers/remoteproc/remoteproc_elf_helpers.h
@@ -11,6 +11,7 @@
#include <linux/elf.h>
#include <linux/types.h>

+#define MAX_NAME_LENGTH 16
/**
* fw_elf_get_class - Get elf class
* @fw: the ELF firmware image
@@ -65,6 +66,7 @@ static inline void elf_hdr_init_ident(struct elf32_hdr *hdr, u8 class)
ELF_GEN_FIELD_GET_SET(hdr, e_version, u32)
ELF_GEN_FIELD_GET_SET(hdr, e_ehsize, u32)
ELF_GEN_FIELD_GET_SET(hdr, e_phentsize, u16)
+ELF_GEN_FIELD_GET_SET(hdr, e_shentsize, u16)

ELF_GEN_FIELD_GET_SET(phdr, p_paddr, u64)
ELF_GEN_FIELD_GET_SET(phdr, p_vaddr, u64)
@@ -74,6 +76,9 @@ static inline void elf_hdr_init_ident(struct elf32_hdr *hdr, u8 class)
ELF_GEN_FIELD_GET_SET(phdr, p_offset, u64)
ELF_GEN_FIELD_GET_SET(phdr, p_flags, u32)
ELF_GEN_FIELD_GET_SET(phdr, p_align, u64)
+ELF_GEN_FIELD_GET_SET(shdr, sh_type, u32)
+ELF_GEN_FIELD_GET_SET(shdr, sh_flags, u32)
+ELF_GEN_FIELD_GET_SET(shdr, sh_entsize, u16)

ELF_GEN_FIELD_GET_SET(shdr, sh_size, u64)
ELF_GEN_FIELD_GET_SET(shdr, sh_offset, u64)
@@ -93,4 +98,26 @@ static inline void elf_hdr_init_ident(struct elf32_hdr *hdr, u8 class)
ELF_STRUCT_SIZE(phdr)
ELF_STRUCT_SIZE(hdr)

+static inline unsigned int set_section_name(const char *name, void *ehdr,
+ u8 class)
+{
+ u16 shstrndx = elf_hdr_get_e_shstrndx(class, ehdr);
+ void *shdr;
+ char *strtab;
+ static int strtable_idx = 1;
+ int idx, ret = 0;
+
+ shdr = ehdr + elf_size_of_hdr(class) + shstrndx * elf_size_of_shdr(class);
+ strtab = ehdr + elf_shdr_get_sh_offset(class, shdr);
+ idx = strtable_idx;
+ if (!strtab || !name)
+ return 0;
+
+ ret = idx;
+ idx += strlcpy((strtab + idx), name, MAX_NAME_LENGTH);
+ strtable_idx = idx + 1;
+
+ return ret;
+}
+
#endif /* REMOTEPROC_ELF_LOADER_H */
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index c340028..1bb69ee 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -51,7 +51,9 @@ struct dentry *rproc_create_trace_file(const char *name, struct rproc *rproc,

/* from remoteproc_coredump.c */
void rproc_coredump_cleanup(struct rproc *rproc);
+void rproc_cleanup_priv(struct rproc *rproc);
void rproc_coredump(struct rproc *rproc);
+void rproc_minidump(struct rproc *rproc);

#ifdef CONFIG_REMOTEPROC_CDEV
void rproc_init_cdev(void);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 0ed1a2b..f6f1f34 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -513,6 +513,7 @@ struct rproc_dump_segment {
* @nb_vdev: number of vdev currently handled by rproc
* @char_dev: character device of the rproc
* @cdev_put_on_release: flag to indicate if remoteproc should be shutdown on @char_dev release
+ * @minidump_id: Unique id to identify remoteproc in the minidump table
*/
struct rproc {
struct list_head node;
@@ -552,6 +553,7 @@ struct rproc {
u16 elf_machine;
struct cdev cdev;
bool cdev_put_on_release;
+ int minidump_id;
};

/**
--
1.9.1

2020-08-08 01:54:21

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/3] remoteproc: qcom: Add capability to collect minidumps

Hi Gurbir,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20200807]
[cannot apply to linux/master linus/master v5.8 v5.8-rc7 v5.8-rc6 v5.8]
[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]

url: https://github.com/0day-ci/linux/commits/Gurbir-Arora/Introduce-mini-dump-support-for-remoteproc/20200808-073315
base: 471e638c4c5df4c0035a76a561ada4d28228e5fd
config: nios2-randconfig-s032-20200807 (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.2-118-ge1578773-dirty
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=nios2

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>


sparse warnings: (new ones prefixed by >>)

>> drivers/remoteproc/remoteproc_coredump.c:489:38: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const *s @@ got struct md_ss_region [noderef] __iomem *[assigned] region_info @@
>> drivers/remoteproc/remoteproc_coredump.c:489:38: sparse: expected void const *s
drivers/remoteproc/remoteproc_coredump.c:489:38: sparse: got struct md_ss_region [noderef] __iomem *[assigned] region_info
>> drivers/remoteproc/remoteproc_coredump.c:491:37: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const *s @@ got void [noderef] __iomem *[assigned] offset @@
drivers/remoteproc/remoteproc_coredump.c:491:37: sparse: expected void const *s
drivers/remoteproc/remoteproc_coredump.c:491:37: sparse: got void [noderef] __iomem *[assigned] offset
drivers/remoteproc/remoteproc_coredump.c:493:39: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const *s @@ got void [noderef] __iomem *[assigned] offset @@
drivers/remoteproc/remoteproc_coredump.c:493:39: sparse: expected void const *s
drivers/remoteproc/remoteproc_coredump.c:493:39: sparse: got void [noderef] __iomem *[assigned] offset

vim +489 drivers/remoteproc/remoteproc_coredump.c

460
461 static void add_minidump_segments(struct rproc *rproc, struct md_ss_toc *minidump_ss)
462 {
463 struct md_ss_region __iomem *region_info, *ptr;
464 int i, seg_cnt = minidump_ss->ss_region_count;
465 void __iomem *offset;
466 dma_addr_t da;
467 size_t size;
468 char *name;
469
470 if (!list_empty(&rproc->dump_segments))
471 goto minidump;
472
473 region_info = ioremap((unsigned long)minidump_ss->md_ss_smem_regions_baseptr,
474 seg_cnt * sizeof(struct md_ss_region));
475
476 if (!region_info) {
477 dev_err(&rproc->dev, "invalid minidump subsystem table\n");
478 return;
479 }
480
481 /* Iterate over the subsystem's segments and create rproc custom dump segments */
482 ptr = region_info;
483 offset = ptr;
484 for (i = 0; i < seg_cnt; i++) {
485 offset = region_info;
486 offset += sizeof(region_info->name) + sizeof(region_info->seq_num);
487 if (__raw_readl(offset) == MD_REGION_VALID) {
488 name = kmalloc(MAX_REGION_NAME_LENGTH, GFP_KERNEL);
> 489 memcpy(name, region_info, sizeof(region_info->name));
490 offset += sizeof(region_info->md_valid);
> 491 memcpy(&da, offset, sizeof(region_info->region_base_address));
492 offset += sizeof(region_info->region_base_address);
493 memcpy(&size, offset, sizeof(region_info->region_size));
494 rproc_coredump_add_custom_segment(rproc, da, size, NULL, name);
495 }
496 region_info++;
497 }
498
499 minidump:
500 return do_rproc_minidump(rproc);
501 }
502

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (4.24 kB)
.config.gz (24.22 kB)
Download all attachments

2020-08-08 02:23:35

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/3] remoteproc: qcom: Add capability to collect minidumps

Hi Gurbir,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20200807]
[cannot apply to linux/master linus/master v5.8 v5.8-rc7 v5.8-rc6 v5.8]
[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]

url: https://github.com/0day-ci/linux/commits/Gurbir-Arora/Introduce-mini-dump-support-for-remoteproc/20200808-073315
base: 471e638c4c5df4c0035a76a561ada4d28228e5fd
config: x86_64-randconfig-s022-20200807 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.2-118-ge1578773-dirty
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

ld: drivers/remoteproc/remoteproc_coredump.o: in function `rproc_minidump':
>> drivers/remoteproc/remoteproc_coredump.c:510: undefined reference to `qcom_smem_get'

sparse warnings: (new ones prefixed by >>)

>> drivers/remoteproc/remoteproc_coredump.c:489:25: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const *from @@ got struct md_ss_region [noderef] __iomem *[assigned] region_info @@
>> drivers/remoteproc/remoteproc_coredump.c:489:25: sparse: expected void const *from
>> drivers/remoteproc/remoteproc_coredump.c:489:25: sparse: got struct md_ss_region [noderef] __iomem *[assigned] region_info
>> drivers/remoteproc/remoteproc_coredump.c:491:25: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const *from @@ got void [noderef] __iomem *[assigned] offset @@
drivers/remoteproc/remoteproc_coredump.c:491:25: sparse: expected void const *from
>> drivers/remoteproc/remoteproc_coredump.c:491:25: sparse: got void [noderef] __iomem *[assigned] offset
drivers/remoteproc/remoteproc_coredump.c:493:25: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const *from @@ got void [noderef] __iomem *[assigned] offset @@
drivers/remoteproc/remoteproc_coredump.c:493:25: sparse: expected void const *from
drivers/remoteproc/remoteproc_coredump.c:493:25: sparse: got void [noderef] __iomem *[assigned] offset

vim +510 drivers/remoteproc/remoteproc_coredump.c

460
461 static void add_minidump_segments(struct rproc *rproc, struct md_ss_toc *minidump_ss)
462 {
463 struct md_ss_region __iomem *region_info, *ptr;
464 int i, seg_cnt = minidump_ss->ss_region_count;
465 void __iomem *offset;
466 dma_addr_t da;
467 size_t size;
468 char *name;
469
470 if (!list_empty(&rproc->dump_segments))
471 goto minidump;
472
473 region_info = ioremap((unsigned long)minidump_ss->md_ss_smem_regions_baseptr,
474 seg_cnt * sizeof(struct md_ss_region));
475
476 if (!region_info) {
477 dev_err(&rproc->dev, "invalid minidump subsystem table\n");
478 return;
479 }
480
481 /* Iterate over the subsystem's segments and create rproc custom dump segments */
482 ptr = region_info;
483 offset = ptr;
484 for (i = 0; i < seg_cnt; i++) {
485 offset = region_info;
486 offset += sizeof(region_info->name) + sizeof(region_info->seq_num);
487 if (__raw_readl(offset) == MD_REGION_VALID) {
488 name = kmalloc(MAX_REGION_NAME_LENGTH, GFP_KERNEL);
> 489 memcpy(name, region_info, sizeof(region_info->name));
490 offset += sizeof(region_info->md_valid);
> 491 memcpy(&da, offset, sizeof(region_info->region_base_address));
492 offset += sizeof(region_info->region_base_address);
493 memcpy(&size, offset, sizeof(region_info->region_size));
494 rproc_coredump_add_custom_segment(rproc, da, size, NULL, name);
495 }
496 region_info++;
497 }
498
499 minidump:
500 return do_rproc_minidump(rproc);
501 }
502
503 void rproc_minidump(struct rproc *rproc)
504 {
505 struct md_ss_toc *minidump_ss;
506 size_t size;
507
508 /* Get Global minidump ToC*/
509 if (!g_md_toc)
> 510 g_md_toc = qcom_smem_get(QCOM_SMEM_HOST_ANY,

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (4.51 kB)
.config.gz (34.89 kB)
Download all attachments

2020-08-08 02:44:31

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/3] remoteproc: qcom: Add capability to collect minidumps

Hi Gurbir,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20200807]
[cannot apply to linux/master linus/master v5.8 v5.8-rc7 v5.8-rc6 v5.8]
[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]

url: https://github.com/0day-ci/linux/commits/Gurbir-Arora/Introduce-mini-dump-support-for-remoteproc/20200808-073315
base: 471e638c4c5df4c0035a76a561ada4d28228e5fd
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 ARCH=i386

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from arch/x86/include/asm/page_32.h:35,
from arch/x86/include/asm/page.h:14,
from arch/x86/include/asm/thread_info.h:12,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/swait.h:7,
from include/linux/completion.h:12,
from drivers/remoteproc/remoteproc_coredump.c:8:
In function 'memcpy',
inlined from 'add_minidump_segments' at drivers/remoteproc/remoteproc_coredump.c:491:4,
inlined from 'rproc_minidump' at drivers/remoteproc/remoteproc_coredump.c:530:10:
>> include/linux/string.h:393:4: error: call to '__write_overflow' declared with attribute error: detected write beyond size of object passed as 1st parameter
393 | __write_overflow();
| ^~~~~~~~~~~~~~~~~~

vim +/__write_overflow +393 include/linux/string.h

6974f0c4555e28 Daniel Micay 2017-07-12 386
6974f0c4555e28 Daniel Micay 2017-07-12 387 __FORTIFY_INLINE void *memcpy(void *p, const void *q, __kernel_size_t size)
6974f0c4555e28 Daniel Micay 2017-07-12 388 {
6974f0c4555e28 Daniel Micay 2017-07-12 389 size_t p_size = __builtin_object_size(p, 0);
6974f0c4555e28 Daniel Micay 2017-07-12 390 size_t q_size = __builtin_object_size(q, 0);
6974f0c4555e28 Daniel Micay 2017-07-12 391 if (__builtin_constant_p(size)) {
6974f0c4555e28 Daniel Micay 2017-07-12 392 if (p_size < size)
6974f0c4555e28 Daniel Micay 2017-07-12 @393 __write_overflow();
6974f0c4555e28 Daniel Micay 2017-07-12 394 if (q_size < size)
6974f0c4555e28 Daniel Micay 2017-07-12 395 __read_overflow2();
6974f0c4555e28 Daniel Micay 2017-07-12 396 }
6974f0c4555e28 Daniel Micay 2017-07-12 397 if (p_size < size || q_size < size)
6974f0c4555e28 Daniel Micay 2017-07-12 398 fortify_panic(__func__);
47227d27e2fcb0 Daniel Axtens 2020-06-03 399 return __underlying_memcpy(p, q, size);
6974f0c4555e28 Daniel Micay 2017-07-12 400 }
6974f0c4555e28 Daniel Micay 2017-07-12 401

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (3.20 kB)
.config.gz (72.99 kB)
Download all attachments

2020-08-10 22:55:39

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH 1/3] remoteproc: core: Add coredump to remoteproc ops

On Fri, Aug 07, 2020 at 04:31:59PM -0700, Gurbir Arora wrote:
> Each remoteproc might have different requirements for coredumps and might
> want to choose the type of dumps it wants to collect. This change allows
> remoteproc drivers to specify their own custom dump function to be executed
> in place of rproc_coredump. If the coredump op is not specified by the
> remoteproc driver it will be set to rproc_coredump by default.
>
> Signed-off-by: Rishabh Bhatnagar <[email protected]>
> Signed-off-by: Siddharth Gupta <[email protected]>
> Signed-off-by: Gurbir Arora <[email protected]>
> ---
> drivers/remoteproc/remoteproc_core.c | 6 +++++-
> include/linux/remoteproc.h | 1 +
> 2 files changed, 6 insertions(+), 1 deletion(-)

This is not the patch Siddharth sent.

Please fix it problem along with the 3 kernel test robot warnings this set has
generated and send again after 5.9-rc1 has been released.

Thanks,
Mathieu

>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 7f90eee..283ecb6 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1681,7 +1681,7 @@ int rproc_trigger_recovery(struct rproc *rproc)
> goto unlock_mutex;
>
> /* generate coredump */
> - rproc_coredump(rproc);
> + rproc->ops->coredump(rproc);
>
> /* load firmware */
> ret = request_firmware(&firmware_p, rproc->firmware, dev);
> @@ -2113,6 +2113,10 @@ static int rproc_alloc_ops(struct rproc *rproc, const struct rproc_ops *ops)
> rproc->ops->sanity_check = rproc_elf_sanity_check;
> rproc->ops->get_boot_addr = rproc_elf_get_boot_addr;
>
> + /* Default to rproc_coredump if no coredump functions is specified */
> + if (!rproc->ops->coredump)
> + rproc->ops->coredump = rproc_coredump;
> +
> return 0;
> }
>
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 2fa68bf..0ed1a2b 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -392,6 +392,7 @@ struct rproc_ops {
> int (*load)(struct rproc *rproc, const struct firmware *fw);
> int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
> u64 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
> + void (*coredump)(struct rproc *rproc);
> unsigned long (*panic)(struct rproc *rproc);
> };
>
> --
> 1.9.1
>