2020-11-19 21:08:53

by Siddharth Gupta

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

Sometimes firmware sizes can be in tens 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.

Changelog:
v7 -> v8:
- Addressed all comments from Bjorn:
* Renamed set_section_name to elf_strtbl_add.
* Renamed rproc_minidump to rproc_coredump_using_sections.
* Removed qcom_minidump header and moved structures to qcom_common source files.
* Moved minidump specific functions to qcom_common source files.
* Other minor fixes.

v6 -> v7:
- The STR_TAB size is calculated dynamically now instead of a predefined size.
- Added comments to indicate details about the reserved null section header. More
details can be found at https://refspecs.linuxfoundation.org/elf/elf.pdf.

v5 -> v6:
- Removed priv_cleanup operation from rproc_ops. The dump_segments list is
updated and cleaned up each time minidump is invoked.
- Split patch #2 into 2 parts - one that adds the rproc_minidump function, and
the other that uses the new function in the qcom_q6v5_pas driver.
- Updated structs in qcom_minidump to explicitly indicate the endianness of the
data stored in SMEM, also updated member names.
- Read the global table of contents in SMEM each time adsp_minidump is invoked.

v4 -> v5:
- Fixed adsp_add_minidump_segments to read IO memory using appropriate functions.

v3 -> v4:
- Made adsp_priv_cleanup a static function.

v2 -> v3:
- Refactored code to remove dependency on Qualcomm configs.
- Renamed do_rproc_minidump to rproc_minidump and marked as exported
symbol.

v1 -> v2:
- 3 kernel test robot warnings have been resolved.
- Introduced priv_cleanup op in order to making the cleaning of
private elements used by the remoteproc more readable.
- Removed rproc_cleanup_priv as it is no longer needed.
- Switched to if/else format for rproc_alloc in order to keep
the static const decalaration of adsp_minidump_ops.

Siddharth Gupta (4):
remoteproc: core: Add ops to enable custom coredump functionality
remoteproc: coredump: Add minidump functionality
remoteproc: qcom: Add capability to collect minidumps
remoteproc: qcom: Add minidump id for sm8150 modem

drivers/remoteproc/qcom_common.c | 147 ++++++++++++++++++++++++++++
drivers/remoteproc/qcom_common.h | 2 +
drivers/remoteproc/qcom_q6v5_pas.c | 28 +++++-
drivers/remoteproc/remoteproc_core.c | 6 +-
drivers/remoteproc/remoteproc_coredump.c | 140 ++++++++++++++++++++++++++
drivers/remoteproc/remoteproc_elf_helpers.h | 26 +++++
include/linux/remoteproc.h | 3 +
7 files changed, 349 insertions(+), 3 deletions(-)

--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2020-11-19 21:10:23

by Siddharth Gupta

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

Add minidump id for modem in sm8150 chipset so that the regions to be
included in the coredump generated upon a crash is based on the minidump
tables in SMEM instead of those in the ELF header.

Signed-off-by: Siddharth Gupta <[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 ca05c2ef..e61ef88 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -630,6 +630,7 @@ static const struct adsp_data mpss_resource_init = {
.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*[]){
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2020-11-19 21:10:29

by Siddharth Gupta

[permalink] [raw]
Subject: [PATCH v8 1/4] remoteproc: core: Add ops to enable custom coredump functionality

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: Siddharth Gupta <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 6 +++++-
include/linux/remoteproc.h | 2 ++
2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index dab2c0f..eba7543 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1704,7 +1704,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);
@@ -2126,6 +2126,10 @@ static int rproc_alloc_ops(struct rproc *rproc, const struct rproc_ops *ops)
if (!rproc->ops)
return -ENOMEM;

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

diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 3fa3ba6..a419878 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -375,6 +375,7 @@ enum rsc_handling_status {
* @get_boot_addr: get boot address to entry point specified in firmware
* @panic: optional callback to react to system panic, core will delay
* panic at least the returned number of milliseconds
+ * @coredump: collect firmware dump after the subsystem is shutdown
*/
struct rproc_ops {
int (*prepare)(struct rproc *rproc);
@@ -393,6 +394,7 @@ struct rproc_ops {
int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
u64 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
unsigned long (*panic)(struct rproc *rproc);
+ void (*coredump)(struct rproc *rproc);
};

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

2020-11-19 21:10:48

by Siddharth Gupta

[permalink] [raw]
Subject: [PATCH v8 3/4] 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.

Co-developed-by: Rishabh Bhatnagar <[email protected]>
Signed-off-by: Rishabh Bhatnagar <[email protected]>
Co-developed-by: Gurbir Arora <[email protected]>
Signed-off-by: Gurbir Arora <[email protected]>
Signed-off-by: Siddharth Gupta <[email protected]>
---
drivers/remoteproc/qcom_common.c | 147 +++++++++++++++++++++++++++++++++++++
drivers/remoteproc/qcom_common.h | 2 +
drivers/remoteproc/qcom_q6v5_pas.c | 27 ++++++-
3 files changed, 174 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 085fd73..c41c3a5 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -17,6 +17,7 @@
#include <linux/rpmsg/qcom_smd.h>
#include <linux/slab.h>
#include <linux/soc/qcom/mdt_loader.h>
+#include <linux/soc/qcom/smem.h>

#include "remoteproc_internal.h"
#include "qcom_common.h"
@@ -25,6 +26,61 @@
#define to_smd_subdev(d) container_of(d, struct qcom_rproc_subdev, subdev)
#define to_ssr_subdev(d) container_of(d, struct qcom_rproc_ssr, subdev)

+#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)
+
+/**
+ * struct minidump_region - Minidump region
+ * @name : Name of the region to be dumped
+ * @seq_num: : Use to differentiate regions with same name.
+ * @valid : This entry to be dumped (if set to 1)
+ * @address : Physical address of region to be dumped
+ * @size : Size of the region
+ */
+struct minidump_region {
+ char name[MAX_REGION_NAME_LENGTH];
+ __le32 seq_num;
+ __le32 valid;
+ __le64 address;
+ __le64 size;
+};
+
+/**
+ * struct minidump_subsystem_toc: Subsystem's SMEM Table of content
+ * @status : Subsystem toc init status
+ * @enabled : 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
+ * @region_count : Number of regions added in this subsystem toc
+ * @regions_baseptr : regions base pointer of the subsystem
+ */
+struct minidump_subsystem {
+ __le32 status;
+ __le32 enabled;
+ __le32 encryption_status;
+ __le32 encryption_required;
+ __le32 region_count;
+ __le64 regions_baseptr;
+};
+
+/**
+ * struct minidump_global_toc: Global Table of Content
+ * @status : Global Minidump init status
+ * @md_revision : Minidump revision
+ * @enabled : Minidump enable status
+ * @subsystems : Array of subsystems toc
+ */
+struct minidump_global_toc {
+ __le32 status;
+ __le32 md_revision;
+ __le32 enabled;
+ struct minidump_subsystem subsystems[MAX_NUM_OF_SS];
+};
+
struct qcom_ssr_subsystem {
const char *name;
struct srcu_notifier_head notifier_list;
@@ -34,6 +90,97 @@ struct qcom_ssr_subsystem {
static LIST_HEAD(qcom_ssr_subsystem_list);
static DEFINE_MUTEX(qcom_ssr_subsys_lock);

+
+static void qcom_minidump_cleanup(struct rproc *rproc)
+{
+ struct rproc_dump_segment *entry, *tmp;
+
+ list_for_each_entry_safe(entry, tmp, &rproc->dump_segments, node) {
+ list_del(&entry->node);
+ kfree(entry->priv);
+ kfree(entry);
+ }
+}
+
+static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsystem *subsystem)
+{
+ struct minidump_region __iomem *ptr;
+ struct minidump_region region;
+ int seg_cnt, i;
+ dma_addr_t da;
+ size_t size;
+ char *name;
+
+ if (WARN_ON(!list_empty(&rproc->dump_segments))) {
+ dev_err(&rproc->dev, "dump segment list already populated\n");
+ return -EUCLEAN;
+ }
+
+ seg_cnt = le32_to_cpu(subsystem->region_count);
+ ptr = ioremap((unsigned long)le64_to_cpu(subsystem->regions_baseptr),
+ seg_cnt * sizeof(struct minidump_region));
+ if (!ptr)
+ return -EFAULT;
+
+ for (i = 0; i < seg_cnt; i++) {
+ memcpy_fromio(&region, ptr + i, sizeof(region));
+ if (region.valid == MD_REGION_VALID) {
+ name = kstrdup(region.name, GFP_KERNEL);
+ if (!name) {
+ iounmap(ptr);
+ return -ENOMEM;
+ }
+ da = le64_to_cpu(region.address);
+ size = le32_to_cpu(region.size);
+ rproc_coredump_add_custom_segment(rproc, da, size, NULL, name);
+ }
+ }
+
+ iounmap(ptr);
+ return 0;
+}
+
+void qcom_minidump(struct rproc *rproc, unsigned int minidump_id)
+{
+ int ret;
+ struct minidump_subsystem *subsystem;
+ struct minidump_global_toc *toc;
+
+ /* Get Global minidump ToC*/
+ toc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, NULL);
+
+ /* check if global table pointer exists and init is set */
+ if (IS_ERR(toc) || !toc->status) {
+ dev_err(&rproc->dev, "Minidump TOC not found in SMEM\n");
+ return;
+ }
+
+ /* Get subsystem table of contents using the minidump id */
+ subsystem = &toc->subsystems[minidump_id];
+
+ /**
+ * Collect minidump if SS ToC is valid and segment table
+ * is initialized in memory and encryption status is set.
+ */
+ if (subsystem->regions_baseptr == 0 ||
+ le32_to_cpu(subsystem->status) != 1 ||
+ le32_to_cpu(subsystem->enabled) != MD_SS_ENABLED ||
+ le32_to_cpu(subsystem->encryption_status) != MD_SS_ENCR_DONE) {
+ dev_err(&rproc->dev, "Minidump not ready, skipping\n");
+ return;
+ }
+
+ ret = qcom_add_minidump_segments(rproc, subsystem);
+ if (ret) {
+ dev_err(&rproc->dev, "Failed with error: %d while adding minidump entries\n", ret);
+ goto clean_minidump;
+ }
+ rproc_coredump_using_sections(rproc);
+clean_minidump:
+ qcom_minidump_cleanup(rproc);
+}
+EXPORT_SYMBOL_GPL(qcom_minidump);
+
static int glink_subdev_start(struct rproc_subdev *subdev)
{
struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
index dfc641c..a359f16 100644
--- a/drivers/remoteproc/qcom_common.h
+++ b/drivers/remoteproc/qcom_common.h
@@ -33,6 +33,8 @@ struct qcom_rproc_ssr {
struct qcom_ssr_subsystem *info;
};

+void qcom_minidump(struct rproc *rproc, unsigned int minidump_id);
+
void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink,
const char *ssr_name);
void qcom_remove_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink);
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 3837f23..ca05c2ef 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;
+ unsigned int minidump_id;
bool has_aggre2_clk;
bool auto_boot;

@@ -63,6 +64,7 @@ struct qcom_adsp {
int proxy_pd_count;

int pas_id;
+ unsigned int minidump_id;
int crash_reason_smem;
bool has_aggre2_clk;
const char *info_name;
@@ -81,6 +83,13 @@ struct qcom_adsp {
struct qcom_sysmon *sysmon;
};

+static void adsp_minidump(struct rproc *rproc)
+{
+ struct qcom_adsp *adsp = rproc->priv;
+
+ qcom_minidump(rproc, adsp->minidump_id);
+}
+
static int adsp_pds_enable(struct qcom_adsp *adsp, struct device **pds,
size_t pd_count)
{
@@ -258,6 +267,15 @@ static const struct rproc_ops adsp_ops = {
.panic = adsp_panic,
};

+static const struct rproc_ops adsp_minidump_ops = {
+ .start = adsp_start,
+ .stop = adsp_stop,
+ .da_to_va = adsp_da_to_va,
+ .load = adsp_load,
+ .panic = adsp_panic,
+ .coredump = adsp_minidump,
+};
+
static int adsp_init_clock(struct qcom_adsp *adsp)
{
int ret;
@@ -383,6 +401,7 @@ static int adsp_probe(struct platform_device *pdev)
struct qcom_adsp *adsp;
struct rproc *rproc;
const char *fw_name;
+ const struct rproc_ops *ops = &adsp_ops;
int ret;

desc = of_device_get_match_data(&pdev->dev);
@@ -398,8 +417,11 @@ 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)
+ ops = &adsp_minidump_ops;
+
+ rproc = rproc_alloc(&pdev->dev, pdev->name, ops, fw_name, sizeof(*adsp));
+
if (!rproc) {
dev_err(&pdev->dev, "unable to allocate remoteproc\n");
return -ENOMEM;
@@ -411,6 +433,7 @@ static int adsp_probe(struct platform_device *pdev)
adsp = (struct qcom_adsp *)rproc->priv;
adsp->dev = &pdev->dev;
adsp->rproc = rproc;
+ adsp->minidump_id = desc->minidump_id;
adsp->pas_id = desc->pas_id;
adsp->has_aggre2_clk = desc->has_aggre2_clk;
adsp->info_name = desc->sysmon_name;
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2020-11-21 04:06:23

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v8 1/4] remoteproc: core: Add ops to enable custom coredump functionality

On Thu 19 Nov 15:05 CST 2020, Siddharth Gupta 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.
>

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> Signed-off-by: Siddharth Gupta <[email protected]>
> ---
> drivers/remoteproc/remoteproc_core.c | 6 +++++-
> include/linux/remoteproc.h | 2 ++
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index dab2c0f..eba7543 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1704,7 +1704,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);
> @@ -2126,6 +2126,10 @@ static int rproc_alloc_ops(struct rproc *rproc, const struct rproc_ops *ops)
> if (!rproc->ops)
> return -ENOMEM;
>
> + /* Default to rproc_coredump if no coredump function is specified */
> + if (!rproc->ops->coredump)
> + rproc->ops->coredump = rproc_coredump;
> +
> if (rproc->ops->load)
> return 0;
>
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 3fa3ba6..a419878 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -375,6 +375,7 @@ enum rsc_handling_status {
> * @get_boot_addr: get boot address to entry point specified in firmware
> * @panic: optional callback to react to system panic, core will delay
> * panic at least the returned number of milliseconds
> + * @coredump: collect firmware dump after the subsystem is shutdown
> */
> struct rproc_ops {
> int (*prepare)(struct rproc *rproc);
> @@ -393,6 +394,7 @@ struct rproc_ops {
> int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
> u64 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
> unsigned long (*panic)(struct rproc *rproc);
> + void (*coredump)(struct rproc *rproc);
> };
>
> /**
> --
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

2020-11-21 04:07:08

by Bjorn Andersson

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

On Thu 19 Nov 15:05 CST 2020, Siddharth Gupta wrote:

> 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.
>
> Co-developed-by: Rishabh Bhatnagar <[email protected]>
> Signed-off-by: Rishabh Bhatnagar <[email protected]>
> Co-developed-by: Gurbir Arora <[email protected]>
> Signed-off-by: Gurbir Arora <[email protected]>
> Signed-off-by: Siddharth Gupta <[email protected]>

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> drivers/remoteproc/qcom_common.c | 147 +++++++++++++++++++++++++++++++++++++
> drivers/remoteproc/qcom_common.h | 2 +
> drivers/remoteproc/qcom_q6v5_pas.c | 27 ++++++-
> 3 files changed, 174 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 085fd73..c41c3a5 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -17,6 +17,7 @@
> #include <linux/rpmsg/qcom_smd.h>
> #include <linux/slab.h>
> #include <linux/soc/qcom/mdt_loader.h>
> +#include <linux/soc/qcom/smem.h>
>
> #include "remoteproc_internal.h"
> #include "qcom_common.h"
> @@ -25,6 +26,61 @@
> #define to_smd_subdev(d) container_of(d, struct qcom_rproc_subdev, subdev)
> #define to_ssr_subdev(d) container_of(d, struct qcom_rproc_ssr, subdev)
>
> +#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)
> +
> +/**
> + * struct minidump_region - Minidump region
> + * @name : Name of the region to be dumped
> + * @seq_num: : Use to differentiate regions with same name.
> + * @valid : This entry to be dumped (if set to 1)
> + * @address : Physical address of region to be dumped
> + * @size : Size of the region
> + */
> +struct minidump_region {
> + char name[MAX_REGION_NAME_LENGTH];
> + __le32 seq_num;
> + __le32 valid;
> + __le64 address;
> + __le64 size;
> +};
> +
> +/**
> + * struct minidump_subsystem_toc: Subsystem's SMEM Table of content
> + * @status : Subsystem toc init status
> + * @enabled : 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
> + * @region_count : Number of regions added in this subsystem toc
> + * @regions_baseptr : regions base pointer of the subsystem
> + */
> +struct minidump_subsystem {
> + __le32 status;
> + __le32 enabled;
> + __le32 encryption_status;
> + __le32 encryption_required;
> + __le32 region_count;
> + __le64 regions_baseptr;
> +};
> +
> +/**
> + * struct minidump_global_toc: Global Table of Content
> + * @status : Global Minidump init status
> + * @md_revision : Minidump revision
> + * @enabled : Minidump enable status
> + * @subsystems : Array of subsystems toc
> + */
> +struct minidump_global_toc {
> + __le32 status;
> + __le32 md_revision;
> + __le32 enabled;
> + struct minidump_subsystem subsystems[MAX_NUM_OF_SS];
> +};
> +
> struct qcom_ssr_subsystem {
> const char *name;
> struct srcu_notifier_head notifier_list;
> @@ -34,6 +90,97 @@ struct qcom_ssr_subsystem {
> static LIST_HEAD(qcom_ssr_subsystem_list);
> static DEFINE_MUTEX(qcom_ssr_subsys_lock);
>
> +
> +static void qcom_minidump_cleanup(struct rproc *rproc)
> +{
> + struct rproc_dump_segment *entry, *tmp;
> +
> + list_for_each_entry_safe(entry, tmp, &rproc->dump_segments, node) {
> + list_del(&entry->node);
> + kfree(entry->priv);
> + kfree(entry);
> + }
> +}
> +
> +static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsystem *subsystem)
> +{
> + struct minidump_region __iomem *ptr;
> + struct minidump_region region;
> + int seg_cnt, i;
> + dma_addr_t da;
> + size_t size;
> + char *name;
> +
> + if (WARN_ON(!list_empty(&rproc->dump_segments))) {
> + dev_err(&rproc->dev, "dump segment list already populated\n");
> + return -EUCLEAN;
> + }
> +
> + seg_cnt = le32_to_cpu(subsystem->region_count);
> + ptr = ioremap((unsigned long)le64_to_cpu(subsystem->regions_baseptr),
> + seg_cnt * sizeof(struct minidump_region));
> + if (!ptr)
> + return -EFAULT;
> +
> + for (i = 0; i < seg_cnt; i++) {
> + memcpy_fromio(&region, ptr + i, sizeof(region));
> + if (region.valid == MD_REGION_VALID) {
> + name = kstrdup(region.name, GFP_KERNEL);
> + if (!name) {
> + iounmap(ptr);
> + return -ENOMEM;
> + }
> + da = le64_to_cpu(region.address);
> + size = le32_to_cpu(region.size);
> + rproc_coredump_add_custom_segment(rproc, da, size, NULL, name);
> + }
> + }
> +
> + iounmap(ptr);
> + return 0;
> +}
> +
> +void qcom_minidump(struct rproc *rproc, unsigned int minidump_id)
> +{
> + int ret;
> + struct minidump_subsystem *subsystem;
> + struct minidump_global_toc *toc;
> +
> + /* Get Global minidump ToC*/
> + toc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, NULL);
> +
> + /* check if global table pointer exists and init is set */
> + if (IS_ERR(toc) || !toc->status) {
> + dev_err(&rproc->dev, "Minidump TOC not found in SMEM\n");
> + return;
> + }
> +
> + /* Get subsystem table of contents using the minidump id */
> + subsystem = &toc->subsystems[minidump_id];
> +
> + /**
> + * Collect minidump if SS ToC is valid and segment table
> + * is initialized in memory and encryption status is set.
> + */
> + if (subsystem->regions_baseptr == 0 ||
> + le32_to_cpu(subsystem->status) != 1 ||
> + le32_to_cpu(subsystem->enabled) != MD_SS_ENABLED ||
> + le32_to_cpu(subsystem->encryption_status) != MD_SS_ENCR_DONE) {
> + dev_err(&rproc->dev, "Minidump not ready, skipping\n");
> + return;
> + }
> +
> + ret = qcom_add_minidump_segments(rproc, subsystem);
> + if (ret) {
> + dev_err(&rproc->dev, "Failed with error: %d while adding minidump entries\n", ret);
> + goto clean_minidump;
> + }
> + rproc_coredump_using_sections(rproc);
> +clean_minidump:
> + qcom_minidump_cleanup(rproc);
> +}
> +EXPORT_SYMBOL_GPL(qcom_minidump);
> +
> static int glink_subdev_start(struct rproc_subdev *subdev)
> {
> struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
> diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
> index dfc641c..a359f16 100644
> --- a/drivers/remoteproc/qcom_common.h
> +++ b/drivers/remoteproc/qcom_common.h
> @@ -33,6 +33,8 @@ struct qcom_rproc_ssr {
> struct qcom_ssr_subsystem *info;
> };
>
> +void qcom_minidump(struct rproc *rproc, unsigned int minidump_id);
> +
> void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink,
> const char *ssr_name);
> void qcom_remove_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink);
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 3837f23..ca05c2ef 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;
> + unsigned int minidump_id;
> bool has_aggre2_clk;
> bool auto_boot;
>
> @@ -63,6 +64,7 @@ struct qcom_adsp {
> int proxy_pd_count;
>
> int pas_id;
> + unsigned int minidump_id;
> int crash_reason_smem;
> bool has_aggre2_clk;
> const char *info_name;
> @@ -81,6 +83,13 @@ struct qcom_adsp {
> struct qcom_sysmon *sysmon;
> };
>
> +static void adsp_minidump(struct rproc *rproc)
> +{
> + struct qcom_adsp *adsp = rproc->priv;
> +
> + qcom_minidump(rproc, adsp->minidump_id);
> +}
> +
> static int adsp_pds_enable(struct qcom_adsp *adsp, struct device **pds,
> size_t pd_count)
> {
> @@ -258,6 +267,15 @@ static const struct rproc_ops adsp_ops = {
> .panic = adsp_panic,
> };
>
> +static const struct rproc_ops adsp_minidump_ops = {
> + .start = adsp_start,
> + .stop = adsp_stop,
> + .da_to_va = adsp_da_to_va,
> + .load = adsp_load,
> + .panic = adsp_panic,
> + .coredump = adsp_minidump,
> +};
> +
> static int adsp_init_clock(struct qcom_adsp *adsp)
> {
> int ret;
> @@ -383,6 +401,7 @@ static int adsp_probe(struct platform_device *pdev)
> struct qcom_adsp *adsp;
> struct rproc *rproc;
> const char *fw_name;
> + const struct rproc_ops *ops = &adsp_ops;
> int ret;
>
> desc = of_device_get_match_data(&pdev->dev);
> @@ -398,8 +417,11 @@ 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)
> + ops = &adsp_minidump_ops;
> +
> + rproc = rproc_alloc(&pdev->dev, pdev->name, ops, fw_name, sizeof(*adsp));
> +
> if (!rproc) {
> dev_err(&pdev->dev, "unable to allocate remoteproc\n");
> return -ENOMEM;
> @@ -411,6 +433,7 @@ static int adsp_probe(struct platform_device *pdev)
> adsp = (struct qcom_adsp *)rproc->priv;
> adsp->dev = &pdev->dev;
> adsp->rproc = rproc;
> + adsp->minidump_id = desc->minidump_id;
> adsp->pas_id = desc->pas_id;
> adsp->has_aggre2_clk = desc->has_aggre2_clk;
> adsp->info_name = desc->sysmon_name;
> --
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

2020-11-21 04:09:47

by Bjorn Andersson

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

On Thu 19 Nov 15:05 CST 2020, Siddharth Gupta wrote:

> Add minidump id for modem in sm8150 chipset so that the regions to be
> included in the coredump generated upon a crash is based on the minidump
> tables in SMEM instead of those in the ELF header.
>
> Signed-off-by: Siddharth Gupta <[email protected]>

When reposting patches without modifications, please add any
Acked-by, Reviewed-by or Tested-by that you received previously.


Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> 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 ca05c2ef..e61ef88 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -630,6 +630,7 @@ static const struct adsp_data mpss_resource_init = {
> .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*[]){
> --
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>