2019-08-07 05:41:50

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 0/9] remoteproc: qcom: post mortem debug support

The following series introduces two components that aids in post mortem
debugging of Qualcomm systems. The first part is used to store information
about loaded images in IMEM, for post mortem tools to know where the kernel
loaded the remoteproc firmware. The second part invokes a stop operation on the
remoteprocs during a kernel panic, in order to trigger them to flush caches
etc.

Bjorn Andersson (9):
remoteproc: qcom: Introduce driver to store pil info in IMEM
remoteproc: qcom: mss: Update IMEM PIL info on load
remoteproc: qcom: pas: Update IMEM PIL info on load
remoteproc: qcom: wcnss: Update IMEM PIL info on load
arm64: dts: qcom: qcs404: Add IMEM and PIL info region
arm64: dts: qcom: sdm845: Add IMEM and PIL info region
remoteproc: Introduce "panic" callback in ops
remoteproc: qcom: q6v5: Add common panic handler
remoteproc: qcom: Introduce panic handler for PAS and ADSP

arch/arm64/boot/dts/qcom/qcs404.dtsi | 10 ++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 10 ++
drivers/remoteproc/Kconfig | 6 ++
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/qcom_pil_info.c | 139 +++++++++++++++++++++++++++
drivers/remoteproc/qcom_pil_info.h | 6 ++
drivers/remoteproc/qcom_q6v5.c | 19 ++++
drivers/remoteproc/qcom_q6v5.h | 1 +
drivers/remoteproc/qcom_q6v5_adsp.c | 8 ++
drivers/remoteproc/qcom_q6v5_mss.c | 3 +
drivers/remoteproc/qcom_q6v5_pas.c | 23 ++++-
drivers/remoteproc/qcom_wcnss.c | 14 ++-
drivers/remoteproc/remoteproc_core.c | 16 +++
include/linux/remoteproc.h | 3 +
14 files changed, 253 insertions(+), 6 deletions(-)
create mode 100644 drivers/remoteproc/qcom_pil_info.c
create mode 100644 drivers/remoteproc/qcom_pil_info.h

--
2.18.0


2019-08-07 05:42:01

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 1/9] remoteproc: qcom: Introduce driver to store pil info in IMEM

A region in IMEM is used to communicate load addresses of remoteproc to
post mortem debug tools. Implement a driver that can be used to store
this information in order to enable these tools to process collected
ramdumps.

Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/remoteproc/Kconfig | 3 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/qcom_pil_info.c | 139 +++++++++++++++++++++++++++++
drivers/remoteproc/qcom_pil_info.h | 6 ++
4 files changed, 149 insertions(+)
create mode 100644 drivers/remoteproc/qcom_pil_info.c
create mode 100644 drivers/remoteproc/qcom_pil_info.h

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 28ed306982f7..3984bd16e670 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -85,6 +85,9 @@ config KEYSTONE_REMOTEPROC
It's safe to say N here if you're not interested in the Keystone
DSPs or just want to use a bare minimum kernel.

+config QCOM_PIL_INFO
+ tristate
+
config QCOM_RPROC_COMMON
tristate

diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 00f09e658cb3..c1b46e9033cb 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
obj-$(CONFIG_KEYSTONE_REMOTEPROC) += keystone_remoteproc.o
+obj-$(CONFIG_QCOM_PIL_INFO) += qcom_pil_info.o
obj-$(CONFIG_QCOM_RPROC_COMMON) += qcom_common.o
obj-$(CONFIG_QCOM_Q6V5_COMMON) += qcom_q6v5.o
obj-$(CONFIG_QCOM_Q6V5_ADSP) += qcom_q6v5_adsp.o
diff --git a/drivers/remoteproc/qcom_pil_info.c b/drivers/remoteproc/qcom_pil_info.c
new file mode 100644
index 000000000000..aa42732016f3
--- /dev/null
+++ b/drivers/remoteproc/qcom_pil_info.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2019 Linaro Ltd.
+ */
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/slab.h>
+
+struct pil_reloc_entry {
+ char name[8];
+ __le64 base;
+ __le32 size;
+} __packed;
+
+#define PIL_INFO_SIZE 200
+#define PIL_INFO_ENTRIES (PIL_INFO_SIZE / sizeof(struct pil_reloc_entry))
+
+struct pil_reloc {
+ struct device *dev;
+ struct regmap *map;
+ u32 offset;
+ int val_bytes;
+
+ struct pil_reloc_entry entries[PIL_INFO_ENTRIES];
+};
+
+static struct pil_reloc *_reloc;
+static DEFINE_MUTEX(reloc_mutex);
+
+/**
+ * qcom_pil_info_store() - store PIL information of image in IMEM
+ * @image: name of the image
+ * @base: base address of the loaded image
+ * @size: size of the loaded image
+ */
+void qcom_pil_info_store(const char *image, phys_addr_t base, size_t size)
+{
+ struct pil_reloc_entry *entry;
+ int idx = -1;
+ int i;
+
+ mutex_lock(&reloc_mutex);
+ if (!_reloc)
+ goto unlock;
+
+ for (i = 0; i < PIL_INFO_ENTRIES; i++) {
+ if (!_reloc->entries[i].name[0]) {
+ if (idx == -1)
+ idx = i;
+ continue;
+ }
+
+ if (!strncmp(_reloc->entries[i].name, image, 8)) {
+ idx = i;
+ goto found;
+ }
+ }
+
+ if (idx) {
+ dev_warn(_reloc->dev, "insufficient PIL info slots\n");
+ goto unlock;
+ }
+
+found:
+ entry = &_reloc->entries[idx];
+ stracpy(entry->name, image);
+ entry->base = base;
+ entry->size = size;
+
+ regmap_bulk_write(_reloc->map, _reloc->offset + idx * sizeof(*entry),
+ entry, sizeof(*entry) / _reloc->val_bytes);
+
+unlock:
+ mutex_unlock(&reloc_mutex);
+}
+EXPORT_SYMBOL_GPL(qcom_pil_info_store);
+
+static int pil_reloc_probe(struct platform_device *pdev)
+{
+ struct pil_reloc *reloc;
+
+ reloc = devm_kzalloc(&pdev->dev, sizeof(*reloc), GFP_KERNEL);
+ if (!reloc)
+ return -ENOMEM;
+
+ reloc->dev = &pdev->dev;
+ reloc->map = syscon_node_to_regmap(pdev->dev.parent->of_node);
+ if (IS_ERR(reloc->map))
+ return PTR_ERR(reloc->map);
+
+ if (of_property_read_u32(pdev->dev.of_node, "offset", &reloc->offset))
+ return -EINVAL;
+
+ reloc->val_bytes = regmap_get_val_bytes(reloc->map);
+ if (reloc->val_bytes < 0)
+ return -EINVAL;
+
+ regmap_bulk_write(reloc->map, reloc->offset, reloc->entries,
+ sizeof(reloc->entries) / reloc->val_bytes);
+
+ mutex_lock(&reloc_mutex);
+ _reloc = reloc;
+ mutex_unlock(&reloc_mutex);
+
+ return 0;
+}
+
+static int pil_reloc_remove(struct platform_device *pdev)
+{
+ mutex_lock(&reloc_mutex);
+ _reloc = NULL;
+ mutex_unlock(&reloc_mutex);
+
+ return 0;
+}
+
+static const struct of_device_id pil_reloc_of_match[] = {
+ { .compatible = "qcom,pil-reloc-info" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, pil_reloc_of_match);
+
+static struct platform_driver pil_reloc_driver = {
+ .probe = pil_reloc_probe,
+ .remove = pil_reloc_remove,
+ .driver = {
+ .name = "qcom-pil-reloc-info",
+ .of_match_table = pil_reloc_of_match,
+ },
+};
+module_platform_driver(pil_reloc_driver);
+
+MODULE_DESCRIPTION("Qualcomm PIL relocation info");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/remoteproc/qcom_pil_info.h b/drivers/remoteproc/qcom_pil_info.h
new file mode 100644
index 000000000000..c30c186b665d
--- /dev/null
+++ b/drivers/remoteproc/qcom_pil_info.h
@@ -0,0 +1,6 @@
+#ifndef __QCOM_PIL_INFO_H__
+#define __QCOM_PIL_INFO_H__
+
+void qcom_pil_info_store(const char *image, phys_addr_t base, size_t size);
+
+#endif
--
2.18.0

2019-08-07 05:42:04

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 9/9] remoteproc: qcom: Introduce panic handler for PAS and ADSP

Make the PAS and ADSP/CDSP remoteproc drivers implement the panic
handler that will invoke a stop to prepare the remoteprocs for post
mortem debugging.

Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/remoteproc/qcom_q6v5_adsp.c | 8 ++++++++
drivers/remoteproc/qcom_q6v5_pas.c | 8 ++++++++
2 files changed, 16 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index e953886b2eb7..3de1683903db 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -282,12 +282,20 @@ static void *adsp_da_to_va(struct rproc *rproc, u64 da, int len)
return adsp->mem_region + offset;
}

+static void adsp_panic(struct rproc *rproc)
+{
+ struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
+
+ qcom_q6v5_panic(&adsp->q6v5);
+}
+
static const struct rproc_ops adsp_ops = {
.start = adsp_start,
.stop = adsp_stop,
.da_to_va = adsp_da_to_va,
.parse_fw = qcom_register_dump_segments,
.load = adsp_load,
+ .panic = adsp_panic,
};

static int adsp_init_clock(struct qcom_adsp *adsp, const char **clk_ids)
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index bfb622d36cb3..31ff09bcd3ee 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -179,12 +179,20 @@ static void *adsp_da_to_va(struct rproc *rproc, u64 da, int len)
return adsp->mem_region + offset;
}

+static void adsp_panic(struct rproc *rproc)
+{
+ struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
+
+ qcom_q6v5_panic(&adsp->q6v5);
+}
+
static const struct rproc_ops adsp_ops = {
.start = adsp_start,
.stop = adsp_stop,
.da_to_va = adsp_da_to_va,
.parse_fw = qcom_register_dump_segments,
.load = adsp_load,
+ .panic = adsp_panic,
};

static int adsp_init_clock(struct qcom_adsp *adsp)
--
2.18.0

2019-08-07 05:42:40

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 3/9] remoteproc: qcom: pas: Update IMEM PIL info on load

Use the sysmon_name as identifier and store the relocated base address
and size of the memory region in the PIL reloation info structure in
IMEM.

Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/remoteproc/Kconfig | 1 +
drivers/remoteproc/qcom_q6v5_pas.c | 15 ++++++++++++---
2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index b88d74632d39..2aa0743fc05b 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -136,6 +136,7 @@ config QCOM_Q6V5_PAS
depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
depends on QCOM_SYSMON || QCOM_SYSMON=n
select MFD_SYSCON
+ select QCOM_PIL_INFO
select QCOM_MDT_LOADER
select QCOM_Q6V5_COMMON
select QCOM_RPROC_COMMON
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index db4b3c4bacd7..bfb622d36cb3 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -23,6 +23,7 @@
#include <linux/soc/qcom/smem_state.h>

#include "qcom_common.h"
+#include "qcom_pil_info.h"
#include "qcom_q6v5.h"
#include "remoteproc_internal.h"

@@ -52,6 +53,7 @@ struct qcom_adsp {
int pas_id;
int crash_reason_smem;
bool has_aggre2_clk;
+ const char *info_name;

struct completion start_done;
struct completion stop_done;
@@ -70,11 +72,17 @@ struct qcom_adsp {
static int adsp_load(struct rproc *rproc, const struct firmware *fw)
{
struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
+ int ret;

- return qcom_mdt_load(adsp->dev, fw, rproc->firmware, adsp->pas_id,
- adsp->mem_region, adsp->mem_phys, adsp->mem_size,
- &adsp->mem_reloc);
+ ret = qcom_mdt_load(adsp->dev, fw, rproc->firmware, adsp->pas_id,
+ adsp->mem_region, adsp->mem_phys, adsp->mem_size,
+ &adsp->mem_reloc);
+ if (ret)
+ return ret;

+ qcom_pil_info_store(adsp->info_name, adsp->mem_reloc, adsp->mem_size);
+
+ return 0;
}

static int adsp_start(struct rproc *rproc)
@@ -278,6 +286,7 @@ static int adsp_probe(struct platform_device *pdev)
adsp->rproc = rproc;
adsp->pas_id = desc->pas_id;
adsp->has_aggre2_clk = desc->has_aggre2_clk;
+ adsp->info_name = desc->sysmon_name;
platform_set_drvdata(pdev, adsp);

ret = adsp_alloc_memory_region(adsp);
--
2.18.0

2019-08-07 21:10:16

by Suman Anna

[permalink] [raw]
Subject: Re: [PATCH 1/9] remoteproc: qcom: Introduce driver to store pil info in IMEM

On 8/7/19 12:39 AM, Bjorn Andersson wrote:
> A region in IMEM is used to communicate load addresses of remoteproc to
> post mortem debug tools. Implement a driver that can be used to store
> this information in order to enable these tools to process collected
> ramdumps.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> drivers/remoteproc/Kconfig | 3 +
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/qcom_pil_info.c | 139 +++++++++++++++++++++++++++++
> drivers/remoteproc/qcom_pil_info.h | 6 ++
> 4 files changed, 149 insertions(+)
> create mode 100644 drivers/remoteproc/qcom_pil_info.c
> create mode 100644 drivers/remoteproc/qcom_pil_info.h
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 28ed306982f7..3984bd16e670 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -85,6 +85,9 @@ config KEYSTONE_REMOTEPROC
> It's safe to say N here if you're not interested in the Keystone
> DSPs or just want to use a bare minimum kernel.
>
> +config QCOM_PIL_INFO
> + tristate
> +
> config QCOM_RPROC_COMMON
> tristate
>
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 00f09e658cb3..c1b46e9033cb 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
> obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
> obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
> obj-$(CONFIG_KEYSTONE_REMOTEPROC) += keystone_remoteproc.o
> +obj-$(CONFIG_QCOM_PIL_INFO) += qcom_pil_info.o
> obj-$(CONFIG_QCOM_RPROC_COMMON) += qcom_common.o
> obj-$(CONFIG_QCOM_Q6V5_COMMON) += qcom_q6v5.o
> obj-$(CONFIG_QCOM_Q6V5_ADSP) += qcom_q6v5_adsp.o
> diff --git a/drivers/remoteproc/qcom_pil_info.c b/drivers/remoteproc/qcom_pil_info.c
> new file mode 100644
> index 000000000000..aa42732016f3
> --- /dev/null
> +++ b/drivers/remoteproc/qcom_pil_info.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2019 Linaro Ltd.
> + */
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/slab.h>
> +
> +struct pil_reloc_entry {
> + char name[8];
> + __le64 base;
> + __le32 size;
> +} __packed;
> +
> +#define PIL_INFO_SIZE 200
> +#define PIL_INFO_ENTRIES (PIL_INFO_SIZE / sizeof(struct pil_reloc_entry))
> +
> +struct pil_reloc {
> + struct device *dev;
> + struct regmap *map;
> + u32 offset;
> + int val_bytes;
> +
> + struct pil_reloc_entry entries[PIL_INFO_ENTRIES];
> +};
> +
> +static struct pil_reloc *_reloc;
> +static DEFINE_MUTEX(reloc_mutex);
> +
> +/**
> + * qcom_pil_info_store() - store PIL information of image in IMEM
> + * @image: name of the image
> + * @base: base address of the loaded image
> + * @size: size of the loaded image
> + */
> +void qcom_pil_info_store(const char *image, phys_addr_t base, size_t size)
> +{
> + struct pil_reloc_entry *entry;
> + int idx = -1;
> + int i;
> +
> + mutex_lock(&reloc_mutex);
> + if (!_reloc)
> + goto unlock;
> +
> + for (i = 0; i < PIL_INFO_ENTRIES; i++) {
> + if (!_reloc->entries[i].name[0]) {
> + if (idx == -1)
> + idx = i;
> + continue;
> + }
> +
> + if (!strncmp(_reloc->entries[i].name, image, 8)) {
> + idx = i;
> + goto found;
> + }
> + }
> +
> + if (idx) {
> + dev_warn(_reloc->dev, "insufficient PIL info slots\n");
> + goto unlock;
> + }
> +
> +found:
> + entry = &_reloc->entries[idx];
> + stracpy(entry->name, image);
> + entry->base = base;
> + entry->size = size;
> +
> + regmap_bulk_write(_reloc->map, _reloc->offset + idx * sizeof(*entry),
> + entry, sizeof(*entry) / _reloc->val_bytes);
> +
> +unlock:
> + mutex_unlock(&reloc_mutex);
> +}
> +EXPORT_SYMBOL_GPL(qcom_pil_info_store);
> +
> +static int pil_reloc_probe(struct platform_device *pdev)
> +{
> + struct pil_reloc *reloc;
> +
> + reloc = devm_kzalloc(&pdev->dev, sizeof(*reloc), GFP_KERNEL);
> + if (!reloc)
> + return -ENOMEM;
> +
> + reloc->dev = &pdev->dev;
> + reloc->map = syscon_node_to_regmap(pdev->dev.parent->of_node);
> + if (IS_ERR(reloc->map))
> + return PTR_ERR(reloc->map);
> +
> + if (of_property_read_u32(pdev->dev.of_node, "offset", &reloc->offset))
> + return -EINVAL;
> +
> + reloc->val_bytes = regmap_get_val_bytes(reloc->map);
> + if (reloc->val_bytes < 0)
> + return -EINVAL;
> +
> + regmap_bulk_write(reloc->map, reloc->offset, reloc->entries,
> + sizeof(reloc->entries) / reloc->val_bytes);
> +
> + mutex_lock(&reloc_mutex);
> + _reloc = reloc;
> + mutex_unlock(&reloc_mutex);
> +
> + return 0;
> +}
> +
> +static int pil_reloc_remove(struct platform_device *pdev)
> +{
> + mutex_lock(&reloc_mutex);
> + _reloc = NULL;
> + mutex_unlock(&reloc_mutex);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id pil_reloc_of_match[] = {
> + { .compatible = "qcom,pil-reloc-info" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, pil_reloc_of_match);
> +
> +static struct platform_driver pil_reloc_driver = {
> + .probe = pil_reloc_probe,
> + .remove = pil_reloc_remove,
> + .driver = {
> + .name = "qcom-pil-reloc-info",
> + .of_match_table = pil_reloc_of_match,
> + },
> +};
> +module_platform_driver(pil_reloc_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm PIL relocation info");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/remoteproc/qcom_pil_info.h b/drivers/remoteproc/qcom_pil_info.h
> new file mode 100644
> index 000000000000..c30c186b665d
> --- /dev/null
> +++ b/drivers/remoteproc/qcom_pil_info.h
> @@ -0,0 +1,6 @@

Please add a SPDX license identifier.

regards
Suman

> +#ifndef __QCOM_PIL_INFO_H__
> +#define __QCOM_PIL_INFO_H__
> +
> +void qcom_pil_info_store(const char *image, phys_addr_t base, size_t size);
> +
> +#endif
>