mbox series

[0/9] remoteproc: qcom: post mortem debug support

Message ID 20190807053942.9836-1-bjorn.andersson@linaro.org
Headers show
Series remoteproc: qcom: post mortem debug support | expand

Message

Bjorn Andersson Aug. 7, 2019, 5:39 a.m. UTC
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

Comments

Suman Anna Aug. 7, 2019, 7:19 p.m. UTC | #1
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 <bjorn.andersson@linaro.org>

> ---

>  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

>
Suman Anna Aug. 7, 2019, 7:30 p.m. UTC | #2
On 8/7/19 12:39 AM, Bjorn Andersson wrote:
> Introduce a "panic" function in the remoteproc ops table, to allow

> remoteproc instances to perform operations needed in order to aid in

> post mortem system debugging, such as flushing caches etc, when the

> kernel panics.

> 

> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> ---

>  drivers/remoteproc/remoteproc_core.c | 16 ++++++++++++++++

>  include/linux/remoteproc.h           |  3 +++

>  2 files changed, 19 insertions(+)

> 

> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c

> index 3c5fbbbfb0f1..cc47797c6496 100644

> --- a/drivers/remoteproc/remoteproc_core.c

> +++ b/drivers/remoteproc/remoteproc_core.c

> @@ -1833,6 +1833,16 @@ void rproc_shutdown(struct rproc *rproc)

>  }

>  EXPORT_SYMBOL(rproc_shutdown);

>  

> +static int rproc_panic_handler(struct notifier_block *nb, unsigned long event,

> +			       void *ptr)

> +{

> +	struct rproc *rproc = container_of(nb, struct rproc, panic_nb);

> +

> +	rproc->ops->panic(rproc);

> +

> +	return NOTIFY_DONE;

> +}

> +

>  /**

>   * rproc_get_by_phandle() - find a remote processor by phandle

>   * @phandle: phandle to the rproc

> @@ -2058,6 +2068,12 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,

>  		rproc->ops->get_boot_addr = rproc_elf_get_boot_addr;

>  	}

>  

> +	/* Register panic notifier for remoteprocs with "panic" callback */

> +	if (rproc->ops->panic) {

> +		rproc->panic_nb.notifier_call = rproc_panic_handler;

> +		atomic_notifier_chain_register(&panic_notifier_list, &rproc->panic_nb);

> +	}

> +

>  	mutex_init(&rproc->lock);

>  

>  	idr_init(&rproc->notifyids);

> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h

> index 16ad66683ad0..33553f6d8ff0 100644

> --- a/include/linux/remoteproc.h

> +++ b/include/linux/remoteproc.h

> @@ -383,6 +383,7 @@ struct rproc_ops {

>  	int (*load)(struct rproc *rproc, const struct firmware *fw);

>  	int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);

>  	u32 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);

> +	void (*panic)(struct rproc *rproc);


Do add the kernel doc comment for the new callback op.

regards
Suman

>  };

>  

>  /**

> @@ -481,6 +482,7 @@ struct rproc_dump_segment {

>   * @auto_boot: flag to indicate if remote processor should be auto-started

>   * @dump_segments: list of segments in the firmware

>   * @nb_vdev: number of vdev currently handled by rproc

> + * @panic_nb: notifier_block for remoteproc's panic handler

>   */

>  struct rproc {

>  	struct list_head node;

> @@ -514,6 +516,7 @@ struct rproc {

>  	bool auto_boot;

>  	struct list_head dump_segments;

>  	int nb_vdev;

> +	struct notifier_block panic_nb;

>  };

>  

>  /**

>