Picking up Sarangdhar's original patches for adding core dump support to
remoteproc.
Based on the recently proposed "load resources" hook this registers segments of
the Qualcomm MDT file as dump segments which are used to build an ELF file
representing the various memory segments in the case of a crash of the remote
processor.
Bjorn Andersson (2):
remoteproc: Rename "load_rsc_table" to "parse_fw"
soc: qcom: mdt-loader: Return relocation base
Sarangdhar Joshi (2):
remoteproc: Add remote processor coredump support
remoteproc: qcom: Register segments for core dump
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 +-
drivers/media/platform/qcom/venus/firmware.c | 2 +-
drivers/remoteproc/Kconfig | 1 +
drivers/remoteproc/qcom_adsp_pil.c | 5 +-
drivers/remoteproc/qcom_common.c | 44 +++++++++
drivers/remoteproc/qcom_common.h | 2 +
drivers/remoteproc/qcom_wcnss.c | 4 +-
drivers/remoteproc/remoteproc_core.c | 134 ++++++++++++++++++++++++++-
drivers/remoteproc/remoteproc_internal.h | 7 +-
drivers/soc/qcom/mdt_loader.c | 7 +-
include/linux/remoteproc.h | 20 +++-
include/linux/soc/qcom/mdt_loader.h | 3 +-
12 files changed, 218 insertions(+), 15 deletions(-)
--
2.15.0
In order to implement support for grabbing core dumps in remoteproc it's
necessary to know the relocated base of the image, as the offsets from
the virtual memory base might not be based on the physical address.
Return the adjusted physical base address to the caller.
Signed-off-by: Bjorn Andersson <[email protected]>
---
Changes since v1:
- New patch
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++--
drivers/media/platform/qcom/venus/firmware.c | 2 +-
drivers/remoteproc/qcom_adsp_pil.c | 4 +++-
drivers/remoteproc/qcom_wcnss.c | 3 ++-
drivers/soc/qcom/mdt_loader.c | 7 ++++++-
include/linux/soc/qcom/mdt_loader.h | 3 ++-
6 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index a1f4eeeb73e2..3cab1c74df84 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -87,14 +87,14 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname)
*/
if (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY) {
ret = qcom_mdt_load(dev, fw, fwname, GPU_PAS_ID,
- mem_region, mem_phys, mem_size);
+ mem_region, mem_phys, mem_size, NULL);
} else {
char newname[strlen("qcom/") + strlen(fwname) + 1];
sprintf(newname, "qcom/%s", fwname);
ret = qcom_mdt_load(dev, fw, newname, GPU_PAS_ID,
- mem_region, mem_phys, mem_size);
+ mem_region, mem_phys, mem_size, NULL);
}
if (ret)
goto out;
diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
index 521d4b36c090..c4a577848dd7 100644
--- a/drivers/media/platform/qcom/venus/firmware.c
+++ b/drivers/media/platform/qcom/venus/firmware.c
@@ -76,7 +76,7 @@ int venus_boot(struct device *dev, const char *fwname)
}
ret = qcom_mdt_load(dev, mdt, fwname, VENUS_PAS_ID, mem_va, mem_phys,
- mem_size);
+ mem_size, NULL);
release_firmware(mdt);
diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
index 373c167892d7..833763aa3f2a 100644
--- a/drivers/remoteproc/qcom_adsp_pil.c
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -82,7 +82,9 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
return qcom_mdt_load(adsp->dev, fw, rproc->firmware, adsp->pas_id,
- adsp->mem_region, adsp->mem_phys, adsp->mem_size);
+ adsp->mem_region, adsp->mem_phys, adsp->mem_size,
+ &adsp->mem_reloc);
+
}
static int adsp_start(struct rproc *rproc)
diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index 3f0609236a76..599c1aa73b7f 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -153,7 +153,8 @@ static int wcnss_load(struct rproc *rproc, const struct firmware *fw)
struct qcom_wcnss *wcnss = (struct qcom_wcnss *)rproc->priv;
return qcom_mdt_load(wcnss->dev, fw, rproc->firmware, WCNSS_PAS_ID,
- wcnss->mem_region, wcnss->mem_phys, wcnss->mem_size);
+ wcnss->mem_region, wcnss->mem_phys,
+ wcnss->mem_size, &wcnss->mem_reloc);
}
static void wcnss_indicate_nv_download(struct qcom_wcnss *wcnss)
diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
index 08bd8549242a..17b314d9a148 100644
--- a/drivers/soc/qcom/mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -83,12 +83,14 @@ EXPORT_SYMBOL_GPL(qcom_mdt_get_size);
* @mem_region: allocated memory region to load firmware into
* @mem_phys: physical address of allocated memory region
* @mem_size: size of the allocated memory region
+ * @reloc_base: adjusted physical address after relocation
*
* Returns 0 on success, negative errno otherwise.
*/
int qcom_mdt_load(struct device *dev, const struct firmware *fw,
const char *firmware, int pas_id, void *mem_region,
- phys_addr_t mem_phys, size_t mem_size)
+ phys_addr_t mem_phys, size_t mem_size,
+ phys_addr_t *reloc_base)
{
const struct elf32_phdr *phdrs;
const struct elf32_phdr *phdr;
@@ -192,6 +194,9 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
memset(ptr + phdr->p_filesz, 0, phdr->p_memsz - phdr->p_filesz);
}
+ if (reloc_base)
+ *reloc_base = mem_reloc;
+
out:
kfree(fw_name);
diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
index bd8e0864b059..5b98bbdabc25 100644
--- a/include/linux/soc/qcom/mdt_loader.h
+++ b/include/linux/soc/qcom/mdt_loader.h
@@ -14,6 +14,7 @@ struct firmware;
ssize_t qcom_mdt_get_size(const struct firmware *fw);
int qcom_mdt_load(struct device *dev, const struct firmware *fw,
const char *fw_name, int pas_id, void *mem_region,
- phys_addr_t mem_phys, size_t mem_size);
+ phys_addr_t mem_phys, size_t mem_size,
+ phys_addr_t *reloc_base);
#endif
--
2.15.0
From: Sarangdhar Joshi <[email protected]>
Register MDT segments with the remoteproc core dump functionality in
order to include them in a core dump, in case of a recovery of the remote
processor.
Signed-off-by: Sarangdhar Joshi <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/remoteproc/qcom_adsp_pil.c | 1 +
drivers/remoteproc/qcom_common.c | 44 ++++++++++++++++++++++++++++++++++++++
drivers/remoteproc/qcom_common.h | 2 ++
drivers/remoteproc/qcom_wcnss.c | 1 +
4 files changed, 48 insertions(+)
diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
index 833763aa3f2a..af12cdd45b71 100644
--- a/drivers/remoteproc/qcom_adsp_pil.c
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -179,6 +179,7 @@ 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,
};
diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 6eb9161f80b3..f9a0701c94e7 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -22,6 +22,7 @@
#include <linux/remoteproc.h>
#include <linux/rpmsg/qcom_glink.h>
#include <linux/rpmsg/qcom_smd.h>
+#include <linux/soc/qcom/mdt_loader.h>
#include "remoteproc_internal.h"
#include "qcom_common.h"
@@ -79,6 +80,49 @@ void qcom_remove_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glin
}
EXPORT_SYMBOL_GPL(qcom_remove_glink_subdev);
+/**
+ * qcom_register_dump_segments() - register segments for coredump
+ * @rproc: remoteproc handle
+ * @fw: firmware header
+ *
+ * Register all segments of the ELF in the remoteproc coredump segment list
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int qcom_register_dump_segments(struct rproc *rproc,
+ const struct firmware *fw)
+{
+ const struct elf32_phdr *phdrs;
+ const struct elf32_phdr *phdr;
+ const struct elf32_hdr *ehdr;
+ int ret;
+ int i;
+
+ ehdr = (struct elf32_hdr *)fw->data;
+ phdrs = (struct elf32_phdr *)(ehdr + 1);
+
+ for (i = 0; i < ehdr->e_phnum; i++) {
+ phdr = &phdrs[i];
+
+ if (phdr->p_type != PT_LOAD)
+ continue;
+
+ if ((phdr->p_flags & QCOM_MDT_TYPE_MASK) == QCOM_MDT_TYPE_HASH)
+ continue;
+
+ if (!phdr->p_memsz)
+ continue;
+
+ ret = rproc_coredump_add_segment(rproc, phdr->p_paddr,
+ phdr->p_memsz);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_register_dump_segments);
+
static int smd_subdev_probe(struct rproc_subdev *subdev)
{
struct qcom_rproc_subdev *smd = to_smd_subdev(subdev);
diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
index 728be9834d8b..7e614520fb69 100644
--- a/drivers/remoteproc/qcom_common.h
+++ b/drivers/remoteproc/qcom_common.h
@@ -30,6 +30,8 @@ struct qcom_rproc_ssr {
void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink);
void qcom_remove_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink);
+int qcom_register_dump_segments(struct rproc *rproc, const struct firmware *fw);
+
void qcom_add_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd);
void qcom_remove_smd_subdev(struct rproc *rproc, struct qcom_rproc_subdev *smd);
diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index 599c1aa73b7f..e9f9ba4c584d 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -309,6 +309,7 @@ static const struct rproc_ops wcnss_ops = {
.start = wcnss_start,
.stop = wcnss_stop,
.da_to_va = wcnss_da_to_va,
+ .parse_fw = qcom_register_dump_segments,
.load = wcnss_load,
};
--
2.15.0
The resource table is just one possible source of information that can
be extracted from the firmware file. Generalize this interface to allow
drivers to override this with parsers of other types of information.
Signed-off-by: Bjorn Andersson <[email protected]>
---
Changes since v1:
- New patch
drivers/remoteproc/remoteproc_core.c | 6 +++---
drivers/remoteproc/remoteproc_internal.h | 7 +++----
include/linux/remoteproc.h | 2 +-
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 5af7547b9d8d..6a72daa94673 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -944,8 +944,8 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
rproc->bootaddr = rproc_get_boot_addr(rproc, fw);
- /* load resource table */
- ret = rproc_load_rsc_table(rproc, fw);
+ /* parse firmware resources */
+ ret = rproc_parse_fw(rproc, fw);
if (ret)
goto disable_iommu;
@@ -1555,7 +1555,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
/* Default to ELF loader if no load function is specified */
if (!rproc->ops->load) {
rproc->ops->load = rproc_elf_load_segments;
- rproc->ops->load_rsc_table = rproc_elf_load_rsc_table;
+ rproc->ops->parse_fw = rproc_elf_load_rsc_table;
rproc->ops->find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table;
rproc->ops->sanity_check = rproc_elf_sanity_check;
rproc->ops->get_boot_addr = rproc_elf_get_boot_addr;
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 55a2950c5cb7..7570beb035b5 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -88,11 +88,10 @@ int rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
return -EINVAL;
}
-static inline int rproc_load_rsc_table(struct rproc *rproc,
- const struct firmware *fw)
+static inline int rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
{
- if (rproc->ops->load_rsc_table)
- return rproc->ops->load_rsc_table(rproc, fw);
+ if (rproc->ops->parse_fw)
+ return rproc->ops->parse_fw(rproc, fw);
return 0;
}
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index de6e20a3f061..dc93ac3d1692 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -343,7 +343,7 @@ struct rproc_ops {
int (*stop)(struct rproc *rproc);
void (*kick)(struct rproc *rproc, int vqid);
void * (*da_to_va)(struct rproc *rproc, u64 da, int len);
- int (*load_rsc_table)(struct rproc *rproc, const struct firmware *fw);
+ int (*parse_fw)(struct rproc *rproc, const struct firmware *fw);
struct resource_table *(*find_loaded_rsc_table)(
struct rproc *rproc, const struct firmware *fw);
int (*load)(struct rproc *rproc, const struct firmware *fw);
--
2.15.0
From: Sarangdhar Joshi <[email protected]>
As the remoteproc framework restarts the remote processor after a fatal
event, it's useful to be able to acquire a coredump of the remote
processor's state, for post mortem debugging.
This patch introduces a mechanism for extracting the memory contents
after the remote has stopped and before the restart sequence has begun
in the recovery path. The remoteproc framework builds the core dump in
memory and use devcoredump to expose this to user space.
Signed-off-by: Sarangdhar Joshi <[email protected]>
[bjorn: Use vmalloc instead of composing the ELF on the fly]
Signed-off-by: Bjorn Andersson <[email protected]>
---
Changes since v1:
- vmalloc() memory to build ELF file before registering devcoredump
- life cycle of segment list is from boot to shutdown (not start/shutdown)
- add WANT_DEV_COREDUMP select
- use dma_addr_t to denote device addresses
drivers/remoteproc/Kconfig | 1 +
drivers/remoteproc/remoteproc_core.c | 128 +++++++++++++++++++++++++++++++++++
include/linux/remoteproc.h | 18 +++++
3 files changed, 147 insertions(+)
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index b609e1d3654b..3e4bca77188d 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -6,6 +6,7 @@ config REMOTEPROC
select CRC32
select FW_LOADER
select VIRTIO
+ select WANT_DEV_COREDUMP
help
Support for remote processors (such as DSP coprocessors). These
are mainly used on embedded systems.
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 4170dfbd93bd..5af7547b9d8d 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -33,6 +33,7 @@
#include <linux/firmware.h>
#include <linux/string.h>
#include <linux/debugfs.h>
+#include <linux/devcoredump.h>
#include <linux/remoteproc.h>
#include <linux/iommu.h>
#include <linux/idr.h>
@@ -801,6 +802,20 @@ static void rproc_remove_subdevices(struct rproc *rproc)
subdev->remove(subdev);
}
+/**
+ * rproc_coredump_cleanup() - clean up dump_segments list
+ * @rproc: the remote processor handle
+ */
+static void rproc_coredump_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);
+ }
+}
+
/**
* rproc_resource_cleanup() - clean up and free all acquired resources
* @rproc: rproc handle
@@ -848,6 +863,8 @@ static void rproc_resource_cleanup(struct rproc *rproc)
/* clean up remote vdev entries */
list_for_each_entry_safe(rvdev, rvtmp, &rproc->rvdevs, node)
kref_put(&rvdev->refcount, rproc_vdev_release);
+
+ rproc_coredump_cleanup(rproc);
}
static int rproc_start(struct rproc *rproc, const struct firmware *fw)
@@ -1017,6 +1034,113 @@ static int rproc_stop(struct rproc *rproc)
return 0;
}
+/**
+ * rproc_coredump_add_segment() - add segment of device memory to coredump
+ * @rproc: handle of a remote processor
+ * @da: device address
+ * @size: size of segment
+ *
+ * Add device memory to the list of segments to be included in a coredump for
+ * the remoteproc.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int rproc_coredump_add_segment(struct rproc *rproc, dma_addr_t da, size_t size)
+{
+ struct rproc_dump_segment *segment;
+
+ segment = kzalloc(sizeof(*segment), GFP_KERNEL);
+ if (!segment)
+ return -ENOMEM;
+
+ segment->da = da;
+ segment->size = size;
+
+ list_add_tail(&segment->node, &rproc->dump_segments);
+
+ return 0;
+}
+EXPORT_SYMBOL(rproc_coredump_add_segment);
+
+/**
+ * rproc_coredump() - perform coredump
+ * @rproc: rproc handle
+ *
+ * This function will generate an ELF header for the registered segments
+ * and create a devcoredump device associated with rproc.
+ */
+static void rproc_coredump(struct rproc *rproc)
+{
+ struct rproc_dump_segment *segment;
+ struct elf32_phdr *phdr;
+ struct elf32_hdr *ehdr;
+ size_t data_size;
+ size_t offset;
+ void *data;
+ void *ptr;
+ int phnum = 0;
+
+ if (list_empty(&rproc->dump_segments))
+ return;
+
+ data_size = sizeof(*ehdr);
+ list_for_each_entry(segment, &rproc->dump_segments, node) {
+ data_size += sizeof(*phdr) + segment->size;
+
+ phnum++;
+ }
+
+ data = vmalloc(data_size);
+ if (!data)
+ return;
+
+ ehdr = data;
+
+ memset(ehdr, 0, sizeof(*ehdr));
+ memcpy(ehdr->e_ident, ELFMAG, SELFMAG);
+ ehdr->e_ident[EI_CLASS] = ELFCLASS32;
+ ehdr->e_ident[EI_DATA] = ELFDATA2LSB;
+ ehdr->e_ident[EI_VERSION] = EV_CURRENT;
+ ehdr->e_ident[EI_OSABI] = ELFOSABI_NONE;
+ ehdr->e_type = ET_CORE;
+ ehdr->e_machine = EM_NONE;
+ ehdr->e_version = EV_CURRENT;
+ ehdr->e_entry = rproc->bootaddr;
+ ehdr->e_phoff = sizeof(*ehdr);
+ ehdr->e_ehsize = sizeof(*ehdr);
+ ehdr->e_phentsize = sizeof(*phdr);
+ ehdr->e_phnum = phnum;
+
+ phdr = data + ehdr->e_phoff;
+ offset = ehdr->e_phoff + sizeof(*phdr) * ehdr->e_phnum;
+ list_for_each_entry(segment, &rproc->dump_segments, node) {
+ memset(phdr, 0, sizeof(*phdr));
+ phdr->p_type = PT_LOAD;
+ phdr->p_offset = offset;
+ phdr->p_vaddr = segment->da;
+ phdr->p_paddr = segment->da;
+ phdr->p_filesz = segment->size;
+ phdr->p_memsz = segment->size;
+ phdr->p_flags = PF_R | PF_W | PF_X;
+ phdr->p_align = 0;
+
+ ptr = rproc_da_to_va(rproc, segment->da, segment->size);
+ if (!ptr) {
+ dev_err(&rproc->dev,
+ "invalid coredump segment (%pad, %zu)\n",
+ &segment->da, segment->size);
+ memset(data + offset, 0xff, segment->size);
+ } else {
+ memcpy(data + offset, ptr, segment->size);
+ }
+
+ offset += phdr->p_filesz;
+ phdr++;
+ }
+
+ dev_coredumpv(&rproc->dev, data, data_size, GFP_KERNEL);
+}
+
/**
* rproc_trigger_recovery() - recover a remoteproc
* @rproc: the remote processor
@@ -1043,6 +1167,9 @@ int rproc_trigger_recovery(struct rproc *rproc)
if (ret)
goto unlock_mutex;
+ /* generate coredump */
+ rproc_coredump(rproc);
+
/* load firmware */
ret = request_firmware(&firmware_p, rproc->firmware, dev);
if (ret < 0) {
@@ -1443,6 +1570,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
INIT_LIST_HEAD(&rproc->traces);
INIT_LIST_HEAD(&rproc->rvdevs);
INIT_LIST_HEAD(&rproc->subdevs);
+ INIT_LIST_HEAD(&rproc->dump_segments);
INIT_WORK(&rproc->crash_handler, rproc_crash_handler_work);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 32a3bf351919..de6e20a3f061 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -393,6 +393,21 @@ enum rproc_crash_type {
RPROC_FATAL_ERROR,
};
+/**
+ * struct rproc_dump_segment - segment info from ELF header
+ * @node: list node related to the rproc segment list
+ * @da: device address of the segment
+ * @size: size of the segment
+ */
+struct rproc_dump_segment {
+ struct list_head node;
+
+ dma_addr_t da;
+ size_t size;
+
+ loff_t offset;
+};
+
/**
* struct rproc - represents a physical remote processor device
* @node: list node of this rproc object
@@ -423,6 +438,7 @@ enum rproc_crash_type {
* @cached_table: copy of the resource table
* @table_sz: size of @cached_table
* @has_iommu: flag to indicate if remote processor is behind an MMU
+ * @dump_segments: list of segments in the firmware
*/
struct rproc {
struct list_head node;
@@ -454,6 +470,7 @@ struct rproc {
size_t table_sz;
bool has_iommu;
bool auto_boot;
+ struct list_head dump_segments;
};
/**
@@ -533,6 +550,7 @@ void rproc_free(struct rproc *rproc);
int rproc_boot(struct rproc *rproc);
void rproc_shutdown(struct rproc *rproc);
void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type);
+int rproc_coredump_add_segment(struct rproc *rproc, dma_addr_t da, size_t size);
static inline struct rproc_vdev *vdev_to_rvdev(struct virtio_device *vdev)
{
--
2.15.0
> -----Original Message-----
> From: [email protected] [mailto:linux-remoteproc-
> [email protected]] On Behalf Of Bjorn Andersson
> Sent: Tuesday, December 26, 2017 9:39 PM
> To: Ohad Ben-Cohen <[email protected]>; Bjorn Andersson
> <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Suman Anna <s-
> [email protected]>; Avaneesh Kumar Dwivedi <[email protected]>
> Subject: [PATCH v2 2/4] remoteproc: Rename "load_rsc_table" to
> "parse_fw"
>
> The resource table is just one possible source of information that can
> be extracted from the firmware file. Generalize this interface to allow
> drivers to override this with parsers of other types of information.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
>
> Changes since v1:
> - New patch
>
> drivers/remoteproc/remoteproc_core.c | 6 +++---
> drivers/remoteproc/remoteproc_internal.h | 7 +++----
> include/linux/remoteproc.h | 2 +-
> 3 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> index 5af7547b9d8d..6a72daa94673 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -944,8 +944,8 @@ static int rproc_fw_boot(struct rproc *rproc, const
> struct firmware *fw)
>
> rproc->bootaddr = rproc_get_boot_addr(rproc, fw);
>
> - /* load resource table */
> - ret = rproc_load_rsc_table(rproc, fw);
> + /* parse firmware resources */
> + ret = rproc_parse_fw(rproc, fw);
Hi Bjorn,
I think it will be good to keep resource (aka rsc) in function name. only "parse_fw" is not enough explicit and we don't know why rproc should parse firmware.
Regards,
Loic
> if (ret)
> goto disable_iommu;
>
> @@ -1555,7 +1555,7 @@ struct rproc *rproc_alloc(struct device *dev, const
> char *name,
> /* Default to ELF loader if no load function is specified */
> if (!rproc->ops->load) {
> rproc->ops->load = rproc_elf_load_segments;
> - rproc->ops->load_rsc_table = rproc_elf_load_rsc_table;
> + rproc->ops->parse_fw = rproc_elf_load_rsc_table;
> rproc->ops->find_loaded_rsc_table =
> rproc_elf_find_loaded_rsc_table;
> rproc->ops->sanity_check = rproc_elf_sanity_check;
> rproc->ops->get_boot_addr = rproc_elf_get_boot_addr;
> diff --git a/drivers/remoteproc/remoteproc_internal.h
> b/drivers/remoteproc/remoteproc_internal.h
> index 55a2950c5cb7..7570beb035b5 100644
> --- a/drivers/remoteproc/remoteproc_internal.h
> +++ b/drivers/remoteproc/remoteproc_internal.h
> @@ -88,11 +88,10 @@ int rproc_load_segments(struct rproc *rproc, const
> struct firmware *fw)
> return -EINVAL;
> }
>
> -static inline int rproc_load_rsc_table(struct rproc *rproc,
> - const struct firmware *fw)
> +static inline int rproc_parse_fw(struct rproc *rproc, const struct firmware
> *fw)
> {
> - if (rproc->ops->load_rsc_table)
> - return rproc->ops->load_rsc_table(rproc, fw);
> + if (rproc->ops->parse_fw)
> + return rproc->ops->parse_fw(rproc, fw);
>
> return 0;
> }
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index de6e20a3f061..dc93ac3d1692 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -343,7 +343,7 @@ struct rproc_ops {
> int (*stop)(struct rproc *rproc);
> void (*kick)(struct rproc *rproc, int vqid);
> void * (*da_to_va)(struct rproc *rproc, u64 da, int len);
> - int (*load_rsc_table)(struct rproc *rproc, const struct firmware *fw);
> + int (*parse_fw)(struct rproc *rproc, const struct firmware *fw);
> struct resource_table *(*find_loaded_rsc_table)(
> struct rproc *rproc, const struct firmware
> *fw);
> int (*load)(struct rproc *rproc, const struct firmware *fw);
> --
> 2.15.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message-----
> From: [email protected] [mailto:linux-remoteproc-
> [email protected]] On Behalf Of Loic PALLARDY
> Sent: Wednesday, January 03, 2018 11:27 AM
> To: Bjorn Andersson <[email protected]>; Ohad Ben-Cohen
> <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Suman Anna <s-
> [email protected]>; Avaneesh Kumar Dwivedi <[email protected]>
> Subject: RE: [PATCH v2 2/4] remoteproc: Rename "load_rsc_table" to
> "parse_fw"
>
>
>
> > -----Original Message-----
> > From: [email protected] [mailto:linux-
> remoteproc-
> > [email protected]] On Behalf Of Bjorn Andersson
> > Sent: Tuesday, December 26, 2017 9:39 PM
> > To: Ohad Ben-Cohen <[email protected]>; Bjorn Andersson
> > <[email protected]>
> > Cc: [email protected]; [email protected];
> linux-
> > [email protected]; [email protected]; Suman Anna <s-
> > [email protected]>; Avaneesh Kumar Dwivedi <[email protected]>
> > Subject: [PATCH v2 2/4] remoteproc: Rename "load_rsc_table" to
> > "parse_fw"
> >
> > The resource table is just one possible source of information that can
> > be extracted from the firmware file. Generalize this interface to allow
> > drivers to override this with parsers of other types of information.
> >
> > Signed-off-by: Bjorn Andersson <[email protected]>
> > ---
> >
> > Changes since v1:
> > - New patch
> >
> > drivers/remoteproc/remoteproc_core.c | 6 +++---
> > drivers/remoteproc/remoteproc_internal.h | 7 +++----
> > include/linux/remoteproc.h | 2 +-
> > 3 files changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/remoteproc/remoteproc_core.c
> > b/drivers/remoteproc/remoteproc_core.c
> > index 5af7547b9d8d..6a72daa94673 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -944,8 +944,8 @@ static int rproc_fw_boot(struct rproc *rproc, const
> > struct firmware *fw)
> >
> > rproc->bootaddr = rproc_get_boot_addr(rproc, fw);
> >
> > - /* load resource table */
> > - ret = rproc_load_rsc_table(rproc, fw);
> > + /* parse firmware resources */
> > + ret = rproc_parse_fw(rproc, fw);
> Hi Bjorn,
>
> I think it will be good to keep resource (aka rsc) in function name. only
> "parse_fw" is not enough explicit and we don't know why rproc should parse
> firmware.
>
> Regards,
> Loic
Forgot my previous remark, better understanding thanks to the rest of the series.
Anyway, will be nice to have a comment here as it is not only parsing the firmware, you collect some information like copy of the resource table, list of elf segment to dump...
I think it is important to be clear about resource table management as it is a key element of the remoteproc core, where it is loaded, where it is copied back in memory...
Regards,
Loic
>
> > if (ret)
> > goto disable_iommu;
> >
> > @@ -1555,7 +1555,7 @@ struct rproc *rproc_alloc(struct device *dev,
> const
> > char *name,
> > /* Default to ELF loader if no load function is specified */
> > if (!rproc->ops->load) {
> > rproc->ops->load = rproc_elf_load_segments;
> > - rproc->ops->load_rsc_table = rproc_elf_load_rsc_table;
> > + rproc->ops->parse_fw = rproc_elf_load_rsc_table;
> > rproc->ops->find_loaded_rsc_table =
> > rproc_elf_find_loaded_rsc_table;
> > rproc->ops->sanity_check = rproc_elf_sanity_check;
> > rproc->ops->get_boot_addr = rproc_elf_get_boot_addr;
> > diff --git a/drivers/remoteproc/remoteproc_internal.h
> > b/drivers/remoteproc/remoteproc_internal.h
> > index 55a2950c5cb7..7570beb035b5 100644
> > --- a/drivers/remoteproc/remoteproc_internal.h
> > +++ b/drivers/remoteproc/remoteproc_internal.h
> > @@ -88,11 +88,10 @@ int rproc_load_segments(struct rproc *rproc, const
> > struct firmware *fw)
> > return -EINVAL;
> > }
> >
> > -static inline int rproc_load_rsc_table(struct rproc *rproc,
> > - const struct firmware *fw)
> > +static inline int rproc_parse_fw(struct rproc *rproc, const struct firmware
> > *fw)
> > {
> > - if (rproc->ops->load_rsc_table)
> > - return rproc->ops->load_rsc_table(rproc, fw);
> > + if (rproc->ops->parse_fw)
> > + return rproc->ops->parse_fw(rproc, fw);
> >
> > return 0;
> > }
> > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> > index de6e20a3f061..dc93ac3d1692 100644
> > --- a/include/linux/remoteproc.h
> > +++ b/include/linux/remoteproc.h
> > @@ -343,7 +343,7 @@ struct rproc_ops {
> > int (*stop)(struct rproc *rproc);
> > void (*kick)(struct rproc *rproc, int vqid);
> > void * (*da_to_va)(struct rproc *rproc, u64 da, int len);
> > - int (*load_rsc_table)(struct rproc *rproc, const struct firmware *fw);
> > + int (*parse_fw)(struct rproc *rproc, const struct firmware *fw);
> > struct resource_table *(*find_loaded_rsc_table)(
> > struct rproc *rproc, const struct firmware
> > *fw);
> > int (*load)(struct rproc *rproc, const struct firmware *fw);
> > --
> > 2.15.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-remoteproc"
> in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed 03 Jan 05:15 PST 2018, Loic PALLARDY wrote:
> > -----Original Message-----
> > From: [email protected] [mailto:linux-remoteproc-
> > [email protected]] On Behalf Of Loic PALLARDY
> > Sent: Wednesday, January 03, 2018 11:27 AM
> > To: Bjorn Andersson <[email protected]>; Ohad Ben-Cohen
> > <[email protected]>
> > Cc: [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; Suman Anna <s-
> > [email protected]>; Avaneesh Kumar Dwivedi <[email protected]>
> > Subject: RE: [PATCH v2 2/4] remoteproc: Rename "load_rsc_table" to
> > "parse_fw"
> > > -----Original Message-----
> > > From: [email protected] [mailto:linux-
> > remoteproc-
> > > [email protected]] On Behalf Of Bjorn Andersson
[..]
> > > - /* load resource table */
> > > - ret = rproc_load_rsc_table(rproc, fw);
> > > + /* parse firmware resources */
> > > + ret = rproc_parse_fw(rproc, fw);
> > Hi Bjorn,
> >
> > I think it will be good to keep resource (aka rsc) in function name. only
> > "parse_fw" is not enough explicit and we don't know why rproc should parse
> > firmware.
> >
> > Regards,
> > Loic
> Forgot my previous remark, better understanding thanks to the rest of
> the series.
> Anyway, will be nice to have a comment here as it is not only parsing
> the firmware, you collect some information like copy of the resource
> table, list of elf segment to dump...
> I think it is important to be clear about resource table management as
> it is a key element of the remoteproc core, where it is loaded, where
> it is copied back in memory...
I didn't manage to come up with a better name, but adding a comment to
capture this makes a lot of sense. I will respin this patch!
Thanks for reviewing this!
Regards,
Bjorn