Hi Amjad,
On Tue, 18 Jan 2022 at 03:13, Amjad Ouled-Ameur
<aouledam...@baylibre.com> wrote:
>
> From: Keerthy <j-keer...@ti.com>
>
> Add remoteproc resource handling helpers. These functions
> are primarily to parse the resource table and to handle
> different types of resources. Carveout, devmem, trace &
> vring resources are handled.
>
> Signed-off-by: Keerthy <j-keer...@ti.com>
> [Amjad: fix redefinition of "struct resource_table" and compile warnings ]
> Signed-off-by: Amjad Ouled-Ameur <aouledam...@baylibre.com>
>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Add useful checks and remove redundant code.
>
> drivers/remoteproc/rproc-uclass.c | 534 ++++++++++++++++++++++++++++++
> include/remoteproc.h | 384 ++++++++++++++++++++-
> 2 files changed, 917 insertions(+), 1 deletion(-)
You need to expand test/dm/remoteproc.c to cover the new functionality.
I notice this is 32-bit...does it work on 64-bit machines?
>
> diff --git a/drivers/remoteproc/rproc-uclass.c
> b/drivers/remoteproc/rproc-uclass.c
> index 87e1ec7ad7f0..50bcc9030e98 100644
> --- a/drivers/remoteproc/rproc-uclass.c
> +++ b/drivers/remoteproc/rproc-uclass.c
> @@ -8,15 +8,31 @@
>
> #define pr_fmt(fmt) "%s: " fmt, __func__
> #include <common.h>
> +#include <elf.h>
> #include <errno.h>
> #include <log.h>
> #include <malloc.h>
> +#include <virtio_ring.h>
> #include <remoteproc.h>
> #include <asm/io.h>
> #include <dm/device-internal.h>
> #include <dm.h>
> #include <dm/uclass.h>
> #include <dm/uclass-internal.h>
> +#include <linux/compat.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct resource_table {
> + u32 ver;
> + u32 num;
> + u32 reserved[2];
> + u32 offset[0];
> +} __packed;
needs comments. Why is it packed?
> +
> +typedef int (*handle_resource_t) (struct udevice *, void *, int offset, int
> avail);
please add arg names and a comment
> +
> +static struct resource_table *rsc_table;
can this be attached to the uclass priv data? We avoid BSS vars in
drivers so they can be used in SPL, etc.
>
> /**
> * for_each_remoteproc_device() - iterate through the list of rproc devices
> @@ -196,6 +212,80 @@ static int rproc_post_probe(struct udevice *dev)
> return 0;
> }
>
> +/**
> + * rproc_add_res() - After parsing the resource table add the mappings
> + * @dev: device we finished probing
> + * @mapping: rproc_mem_entry for the resource
> + *
> + * Return: if the remote proc driver has a add_res routine, invokes it and
> + * hands over the return value. overall, 0 if all went well, else appropriate
> + * error value.
> + */
> +static int rproc_add_res(struct udevice *dev, struct rproc_mem_entry
> *mapping)
> +{
> + const struct dm_rproc_ops *ops = rproc_get_ops(dev);
> +
> + if (!ops->add_res)
> + return -ENOSYS;
> +
> + return ops->add_res(dev, mapping);
> +}
> +
> +/**
> + * rproc_alloc_mem() - After parsing the resource table allocat mem
allocate
> + * @dev: device we finished probing
> + * @len: rproc_mem_entry for the resource
> + * @align: alignment for the resource
> + *
> + * Return: if the remote proc driver has a add_res routine, invokes it and
> + * hands over the return value. overall, 0 if all went well, else appropriate
> + * error value.
> + */
> +static void *rproc_alloc_mem(struct udevice *dev, unsigned long len,
> + unsigned long align)
This should return an error code, otherwise you don't know what happened
> +{
> + const struct dm_rproc_ops *ops;
> +
> + ops = rproc_get_ops(dev);
> + if (!ops) {
> + debug("%s driver has no ops?\n", dev->name);
log_debug() is more modern. But you should not check this. The ops are required.
> + return NULL;
> + }
> +
> + if (ops->alloc_mem)
> + return ops->alloc_mem(dev, len, align);
Should return -ENOSYS if no op
> +
> + return NULL;
> +}
> +
> +/**
> + * rproc_config_pagetable() - Configure page table for remote processor
> + * @dev: device we finished probing
> + * @virt: Virtual address of the resource
> + * @phys: Physical address the resource
> + * @len: length the resource
> + *
> + * Return: if the remote proc driver has a add_res routine, invokes it and
> + * hands over the return value. overall, 0 if all went well, else appropriate
> + * error value.
> + */
> +static int rproc_config_pagetable(struct udevice *dev, unsigned int virt,
> + unsigned int phys, unsigned int len)
> +{
> + const struct dm_rproc_ops *ops;
> +
> + ops = rproc_get_ops(dev);
> + if (!ops) {
> + debug("%s driver has no ops?\n", dev->name);
> + return -EINVAL;
> + }
See above and please fix globally
> +
> + if (ops->config_pagetable)
> + return ops->config_pagetable(dev, virt, phys, len);
Return -ENOSYS if no op
> +
> + return 0;
> +}
> +
> UCLASS_DRIVER(rproc) = {
> .id = UCLASS_REMOTEPROC,
> .name = "remoteproc",
> @@ -426,3 +516,447 @@ int rproc_is_running(int id)
> {
> return _rproc_ops_wrapper(id, RPROC_RUNNING);
> };
> +
> +
> +static int handle_trace(struct udevice *dev, struct fw_rsc_trace *rsc,
> + int offset, int avail)
> +{
> + if (sizeof(*rsc) > avail) {
> + debug("trace rsc is truncated\n");
consider using log_debug()
> + return -EINVAL;
> + }
> +
> + /*
> + * make sure reserved bytes are zeroes
> + */
single-line comment style. Please fix globally
> + if (rsc->reserved) {
> + debug("trace rsc has non zero reserved bytes\n");
> + return -EINVAL;
> + }
> +
> + debug("trace rsc: da 0x%x, len 0x%x\n", rsc->da, rsc->len);
> +
> + return 0;
> +}
> +
> +static int handle_devmem(struct udevice *dev, struct fw_rsc_devmem *rsc,
> + int offset, int avail)
> +{
> + struct rproc_mem_entry *mapping;
> +
> + if (sizeof(*rsc) > avail) {
> + debug("devmem rsc is truncated\n");
> + return -EINVAL;
-ENOSPC ?
> + }
> +
> + /*
> + * make sure reserved bytes are zeroes
> + */
> + if (rsc->reserved) {
> + debug("devmem rsc has non zero reserved bytes\n");
> + return -EINVAL;
> + }
> +
> + debug("devmem rsc: pa 0x%x, da 0x%x, len 0x%x\n",
> + rsc->pa, rsc->da, rsc->len);
> +
> + rproc_config_pagetable(dev, rsc->da, rsc->pa, rsc->len);
> +
> + mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> + if (!mapping)
> + return -ENOMEM;
> +
> + /*
> + * We'll need this info later when we'll want to unmap everything
> + * (e.g. on shutdown).
> + *
> + * We can't trust the remote processor not to change the resource
> + * table, so we must maintain this info independently.
> + */
> + mapping->dma = rsc->pa;
> + mapping->da = rsc->da;
> + mapping->len = rsc->len;
> + rproc_add_res(dev, mapping);
> +
> + debug("mapped devmem pa 0x%x, da 0x%x, len 0x%x\n",
> + rsc->pa, rsc->da, rsc->len);
> +
> + return 0;
> +}
> +
> +static int handle_carveout(struct udevice *dev, struct fw_rsc_carveout *rsc,
> + int offset, int avail)
> +{
> + struct rproc_mem_entry *mapping;
> +
> + if (sizeof(*rsc) > avail) {
> + debug("carveout rsc is truncated\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * make sure reserved bytes are zeroes
> + */
> + if (rsc->reserved) {
> + debug("carveout rsc has non zero reserved bytes\n");
> + return -EINVAL;
> + }
> +
> + debug("carveout rsc: da %x, pa %x, len %x, flags %x\n",
> + rsc->da, rsc->pa, rsc->len, rsc->flags);
> +
> + rsc->pa = (uintptr_t)rproc_alloc_mem(dev, rsc->len, 8);
If you want a physical address, why not make that the interface of
proc_alloc_mem()? Don't just case a pointer to an address as this does
not work in sandbox.
> + if (!rsc->pa) {
> + debug
> + ("failed to allocate carveout rsc: da %x, pa %x, len %x,
> flags %x\n",
> + rsc->da, rsc->pa, rsc->len, rsc->flags);
> + return -ENOMEM;
> + }
> + rproc_config_pagetable(dev, rsc->da, rsc->pa, rsc->len);
> +
> + /*
> + * Ok, this is non-standard.
> + *
> + * Sometimes we can't rely on the generic iommu-based DMA API
> + * to dynamically allocate the device address and then set the IOMMU
> + * tables accordingly, because some remote processors might
> + * _require_ us to use hard coded device addresses that their
> + * firmware was compiled with.
> + *
> + * In this case, we must use the IOMMU API directly and map
> + * the memory to the device address as expected by the remote
> + * processor.
> + *
> + * Obviously such remote processor devices should not be configured
> + * to use the iommu-based DMA API: we expect 'dma' to contain the
> + * physical address in this case.
> + */
> + mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> + if (!mapping)
> + return -ENOMEM;
> +
> + /*
> + * We'll need this info later when we'll want to unmap
> + * everything (e.g. on shutdown).
> + *
> + * We can't trust the remote processor not to change the
> + * resource table, so we must maintain this info independently.
> + */
> + mapping->dma = rsc->pa;
> + mapping->da = rsc->da;
> + mapping->len = rsc->len;
> + rproc_add_res(dev, mapping);
> +
> + debug("carveout mapped 0x%x to 0x%x\n", rsc->da, rsc->pa);
> +
> + return 0;
> +}
> +
> +#define RPROC_PAGE_SHIFT 12
> +#define RPROC_PAGE_SIZE BIT(RPROC_PAGE_SHIFT)
> +#define RPROC_PAGE_ALIGN(x) (((x) + (RPROC_PAGE_SIZE - 1)) &
> ~(RPROC_PAGE_SIZE - 1))
Can you put these at the top of the file?
Also, can you use ALIGN() ?
> +
> +static int alloc_vring(struct udevice *dev, struct fw_rsc_vdev *rsc, int i)
> +{
> + struct fw_rsc_vdev_vring *vring = &rsc->vring[i];
> + int size;
> + int order;
> + void *pa;
> +
> + debug("vdev rsc: vring%d: da %x, qsz %d, align %d\n",
> + i, vring->da, vring->num, vring->align);
> +
> + /*
> + * verify queue size and vring alignment are sane
> + */
> + if (!vring->num || !vring->align) {
> + debug("invalid qsz (%d) or alignment (%d)\n", vring->num,
> + vring->align);
> + return -EINVAL;
You use -EINVAL for a lot of things. Consider branching out.
> + }
> +
> + /*
> + * actual size of vring (in bytes)
> + */
> + size = RPROC_PAGE_ALIGN(vring_size(vring->num, vring->align));
> + order = vring->align >> RPROC_PAGE_SHIFT;
> +
> + pa = rproc_alloc_mem(dev, size, order);
here you use pa (which I assume means physical address) to hold a
pointer. Please rename the var. Please fix globally.
> + if (!pa) {
> + debug("failed to allocate vring rsc\n");
> + return -ENOMEM;
> + }
> + debug("alloc_mem(%#x, %d): %p\n", size, order, pa);
> + vring->da = (uintptr_t)pa;
cast
> +
> + return !pa;
> +}
> +
> +static int handle_vdev(struct udevice *dev, struct fw_rsc_vdev *rsc,
> + int offset, int avail)
> +{
> + int i, ret;
> + void *pa;
> +
> + /*
> + * make sure resource isn't truncated
> + */
> + if (sizeof(*rsc) + rsc->num_of_vrings * sizeof(struct
> fw_rsc_vdev_vring)
> + + rsc->config_len > avail) {
> + debug("vdev rsc is truncated\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * make sure reserved bytes are zeroes
> + */
> + if (rsc->reserved[0] || rsc->reserved[1]) {
> + debug("vdev rsc has non zero reserved bytes\n");
> + return -EINVAL;
> + }
> +
> + debug("vdev rsc: id %d, dfeatures %x, cfg len %d, %d vrings\n",
> + rsc->id, rsc->dfeatures, rsc->config_len, rsc->num_of_vrings);
> +
> + /*
> + * we currently support only two vrings per rvdev
> + */
> + if (rsc->num_of_vrings > 2) {
> + debug("too many vrings: %d\n", rsc->num_of_vrings);
> + return -EINVAL;
> + }
> +
> + /*
> + * allocate the vrings
> + */
> + for (i = 0; i < rsc->num_of_vrings; i++) {
> + ret = alloc_vring(dev, rsc, i);
> + if (ret)
> + goto alloc_error;
> + }
> +
> + pa = rproc_alloc_mem(dev, RPMSG_TOTAL_BUF_SPACE, 6);
> + if (!pa) {
> + debug("failed to allocate vdev rsc\n");
> + return -ENOMEM;
> + }
> + debug("vring buffer alloc_mem(%#x, 6): %p\n", RPMSG_TOTAL_BUF_SPACE,
> + pa);
> +
> + return 0;
> +
> + alloc_error:
> + return ret;
memory leak here, right?
> +}
> +
> +/*
> + * A lookup table for resource handlers. The indices are defined in
> + * enum fw_resource_type.
> + */
> +static handle_resource_t loading_handlers[RSC_LAST] = {
> + [RSC_CARVEOUT] = (handle_resource_t)handle_carveout,
> + [RSC_DEVMEM] = (handle_resource_t)handle_devmem,
> + [RSC_TRACE] = (handle_resource_t)handle_trace,
> + [RSC_VDEV] = (handle_resource_t)handle_vdev,
> +};
> +
> +/*
> + * handle firmware resource entries before booting the remote processor
handle in what sense? What does this actually do?
> + */
> +static int handle_resources(struct udevice *dev, int len,
> + handle_resource_t handlers[RSC_LAST])
> +{
> + handle_resource_t handler;
> + int ret = 0, i;
> +
> + for (i = 0; i < rsc_table->num; i++) {
> + int offset = rsc_table->offset[i];
> + struct fw_rsc_hdr *hdr = (void *)rsc_table + offset;
> + int avail = len - offset - sizeof(*hdr);
> + void *rsc = (void *)hdr + sizeof(*hdr);
> +
> + /*
> + * make sure table isn't truncated
> + */
> + if (avail < 0) {
> + debug("rsc table is truncated\n");
> + return -EINVAL;
> + }
> +
> + debug("rsc: type %d\n", hdr->type);
> +
> + if (hdr->type >= RSC_LAST) {
> + debug("unsupported resource %d\n", hdr->type);
> + continue;
> + }
> +
> + handler = handlers[hdr->type];
> + if (!handler)
> + continue;
> +
> + ret = handler(dev, rsc, offset + sizeof(*hdr), avail);
> + if (ret)
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static int
> +handle_intmem_to_l3_mapping(struct udevice *dev,
> + struct rproc_intmem_to_l3_mapping *l3_mapping)
comment?
> +{
> + u32 i = 0;
> +
> + for (i = 0; i < l3_mapping->num_entries; i++) {
> + struct l3_map *curr_map = &l3_mapping->mappings[i];
> + struct rproc_mem_entry *mapping;
> +
> + mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> + if (!mapping)
> + return -ENOMEM;
> +
> + mapping->dma = curr_map->l3_addr;
> + mapping->da = curr_map->priv_addr;
> + mapping->len = curr_map->len;
> + rproc_add_res(dev, mapping);
> + }
> +
> + return 0;
> +}
> +
> +static Elf32_Shdr *rproc_find_table(unsigned int addr)
> +{
> + Elf32_Ehdr *ehdr; /* Elf header structure pointer */
> + Elf32_Shdr *shdr; /* Section header structure pointer */
> + Elf32_Shdr sectionheader;
> + int i;
> + u8 *elf_data;
> + char *name_table;
> + struct resource_table *ptable;
> +
> + ehdr = (Elf32_Ehdr *)(uintptr_t)addr;
> + elf_data = (u8 *)ehdr;
> + shdr = (Elf32_Shdr *)(elf_data + ehdr->e_shoff);
> + memcpy(§ionheader, &shdr[ehdr->e_shstrndx],
> sizeof(sectionheader));
> + name_table = (char *)(elf_data + sectionheader.sh_offset);
> +
> + for (i = 0; i < ehdr->e_shnum; i++, shdr++) {
> + memcpy(§ionheader, shdr, sizeof(sectionheader));
> + u32 size = sectionheader.sh_size;
> + u32 offset = sectionheader.sh_offset;
> +
> + if (strcmp
> + (name_table + sectionheader.sh_name, ".resource_table"))
> + continue;
> +
> + ptable = (struct resource_table *)(elf_data + offset);
> +
> + /*
> + * make sure table has at least the header
> + */
> + if (sizeof(struct resource_table) > size) {
> + debug("header-less resource table\n");
> + return NULL;
How about returning a few different error codes?
> + }
> +
> + /*
> + * we don't support any version beyond the first
> + */
> + if (ptable->ver != 1) {
> + debug("unsupported fw ver: %d\n", ptable->ver);
> + return NULL;
> + }
> +
> + /*
> + * make sure reserved bytes are zeroes
> + */
> + if (ptable->reserved[0] || ptable->reserved[1]) {
> + debug("non zero reserved bytes\n");
> + return NULL;
> + }
> +
> + /*
> + * make sure the offsets array isn't truncated
> + */
> + if (ptable->num * sizeof(ptable->offset[0]) +
> + sizeof(struct resource_table) > size) {
> + debug("resource table incomplete\n");
> + return NULL;
> + }
> +
> + return shdr;
> + }
> +
> + return NULL;
> +}
> +
> +struct resource_table *rproc_find_resource_table(struct udevice *dev,
> + unsigned int addr,
> + int *tablesz)
> +{
> + Elf32_Shdr *shdr;
> + Elf32_Shdr sectionheader;
> + struct resource_table *ptable;
> + u8 *elf_data = (u8 *)(uintptr_t)addr;
> +
> + shdr = rproc_find_table(addr);
> + if (!shdr) {
> + debug("%s: failed to get resource section header\n",
> __func__);
> + return NULL;
> + }
> +
> + memcpy(§ionheader, shdr, sizeof(sectionheader));
> + ptable = (struct resource_table *)(elf_data +
> sectionheader.sh_offset);
> + if (tablesz)
> + *tablesz = sectionheader.sh_size;
> +
> + return ptable;
> +}
> +
> +unsigned long rproc_parse_resource_table(struct udevice *dev, struct rproc
> *cfg)
> +{
> + struct resource_table *ptable = NULL;
> + int tablesz;
> + int ret;
> + unsigned long addr;
ulong if you like
> +
> + addr = cfg->load_addr;
> +
> + ptable = rproc_find_resource_table(dev, addr, &tablesz);
> + if (!ptable) {
> + debug("%s : failed to find resource table\n", __func__);
> + return 0;
why 0? Isn't this an error?
> + }
> +
> + debug("%s : found resource table\n", __func__);
> + rsc_table = kzalloc(tablesz, GFP_KERNEL);
> + if (!rsc_table) {
> + debug("resource table alloc failed!\n");
> + return 0;
> + }
> +
> + /*
> + * Copy the resource table into a local buffer before handling the
> + * resource table.
> + */
> + memcpy(rsc_table, ptable, tablesz);
> + if (cfg->intmem_to_l3_mapping)
> + handle_intmem_to_l3_mapping(dev, cfg->intmem_to_l3_mapping);
> + ret = handle_resources(dev, tablesz, loading_handlers);
> + if (ret) {
> + debug("handle_resources failed: %d\n", ret);
> + return 0;
> + }
> +
> + /*
> + * Instead of trying to mimic the kernel flow of copying the
> + * processed resource table into its post ELF load location in DDR
> + * copying it into its original location.
> + */
> + memcpy(ptable, rsc_table, tablesz);
> + free(rsc_table);
> + rsc_table = NULL;
> +
> + return 1;
> +}
> diff --git a/include/remoteproc.h b/include/remoteproc.h
> index 089131f65fde..1a970bc23c52 100644
> --- a/include/remoteproc.h
> +++ b/include/remoteproc.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0+ */
> +/* SPDX-License-Identifier: GPL-2.0 */
> /*
> * (C) Copyright 2015
> * Texas Instruments Incorporated - http://www.ti.com/
> @@ -15,6 +15,375 @@
> */
> #include <dm/platdata.h> /* For platform data support - non dt world */
>
> +/**
> + * struct fw_rsc_hdr - firmware resource entry header
> + * @type: resource type
> + * @data: resource data
> + *
> + * Every resource entry begins with a 'struct fw_rsc_hdr' header providing
> + * its @type. The content of the entry itself will immediately follow
> + * this header, and it should be parsed according to the resource type.
> + */
> +struct fw_rsc_hdr {
> + u32 type;
> + u8 data[0];
> +};
> +
> +/**
> + * enum fw_resource_type - types of resource entries
> + *
> + * @RSC_CARVEOUT: request for allocation of a physically contiguous
> + * memory region.
> + * @RSC_DEVMEM: request to iommu_map a memory-based peripheral.
> + * @RSC_TRACE: announces the availability of a trace buffer into which
> + * the remote processor will be writing logs.
> + * @RSC_VDEV: declare support for a virtio device, and serve as its
> + * virtio header.
> + * @RSC_PRELOAD_VENDOR: a vendor resource type that needs to be handled by
> + * remoteproc implementations before loading
> + * @RSC_POSTLOAD_VENDOR: a vendor resource type that needs to be handled by
> + * remoteproc implementations after loading
> + * @RSC_LAST: just keep this one at the end
> + *
> + * For more details regarding a specific resource type, please see its
> + * dedicated structure below.
> + *
> + * Please note that these values are used as indices to the rproc_handle_rsc
> + * lookup table, so please keep them sane. Moreover, @RSC_LAST is used to
> + * check the validity of an index before the lookup table is accessed, so
> + * please update it as needed.
> + */
> +enum fw_resource_type {
> + RSC_CARVEOUT = 0,
> + RSC_DEVMEM = 1,
> + RSC_TRACE = 2,
> + RSC_VDEV = 3,
> + RSC_PRELOAD_VENDOR = 4,
> + RSC_POSTLOAD_VENDOR = 5,
> + RSC_LAST = 6,
> +};
> +
> +#define FW_RSC_ADDR_ANY (-1)
> +
> +/**
> + * struct fw_rsc_carveout - physically contiguous memory request
> + * @da: device address
> + * @pa: physical address
> + * @len: length (in bytes)
> + * @flags: iommu protection flags
> + * @reserved: reserved (must be zero)
> + * @name: human-readable name of the requested memory region
> + *
> + * This resource entry requests the host to allocate a physically contiguous
> + * memory region.
> + *
> + * These request entries should precede other firmware resource entries,
> + * as other entries might request placing other data objects inside
> + * these memory regions (e.g. data/code segments, trace resource entries,
> ...).
> + *
> + * Allocating memory this way helps utilizing the reserved physical memory
> + * (e.g. CMA) more efficiently, and also minimizes the number of TLB entries
> + * needed to map it (in case @rproc is using an IOMMU). Reducing the TLB
> + * pressure is important; it may have a substantial impact on performance.
> + *
> + * If the firmware is compiled with static addresses, then @da should specify
> + * the expected device address of this memory region. If @da is set to
> + * FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and then
> + * overwrite @da with the dynamically allocated address.
> + *
> + * We will always use @da to negotiate the device addresses, even if it
> + * isn't using an iommu. In that case, though, it will obviously contain
> + * physical addresses.
> + *
> + * Some remote processors needs to know the allocated physical address
> + * even if they do use an iommu. This is needed, e.g., if they control
> + * hardware accelerators which access the physical memory directly (this
> + * is the case with OMAP4 for instance). In that case, the host will
> + * overwrite @pa with the dynamically allocated physical address.
> + * Generally we don't want to expose physical addresses if we don't have to
> + * (remote processors are generally _not_ trusted), so we might want to
> + * change this to happen _only_ when explicitly required by the hardware.
> + *
> + * @flags is used to provide IOMMU protection flags, and @name should
> + * (optionally) contain a human readable name of this carveout region
> + * (mainly for debugging purposes).
> + */
> +struct fw_rsc_carveout {
> + u32 da;
> + u32 pa;
> + u32 len;
> + u32 flags;
> + u32 reserved;
> + u8 name[32];
> +};
> +
> +/**
> + * struct fw_rsc_devmem - iommu mapping request
> + * @da: device address
> + * @pa: physical address
> + * @len: length (in bytes)
> + * @flags: iommu protection flags
> + * @reserved: reserved (must be zero)
> + * @name: human-readable name of the requested region to be mapped
> + *
> + * This resource entry requests the host to iommu map a physically contiguous
> + * memory region. This is needed in case the remote processor requires
> + * access to certain memory-based peripherals; _never_ use it to access
> + * regular memory.
> + *
> + * This is obviously only needed if the remote processor is accessing memory
> + * via an iommu.
> + *
> + * @da should specify the required device address, @pa should specify
> + * the physical address we want to map, @len should specify the size of
> + * the mapping and @flags is the IOMMU protection flags. As always, @name may
> + * (optionally) contain a human readable name of this mapping (mainly for
> + * debugging purposes).
> + *
> + * Note: at this point we just "trust" those devmem entries to contain valid
> + * physical addresses, but this isn't safe and will be changed: eventually we
> + * want remoteproc implementations to provide us ranges of physical addresses
> + * the firmware is allowed to request, and not allow firmwares to request
> + * access to physical addresses that are outside those ranges.
> + */
> +struct fw_rsc_devmem {
> + u32 da;
> + u32 pa;
> + u32 len;
> + u32 flags;
> + u32 reserved;
> + u8 name[32];
> +};
> +
> +/**
> + * struct fw_rsc_trace - trace buffer declaration
> + * @da: device address
> + * @len: length (in bytes)
> + * @reserved: reserved (must be zero)
> + * @name: human-readable name of the trace buffer
> + *
> + * This resource entry provides the host information about a trace buffer
> + * into which the remote processor will write log messages.
> + *
> + * @da specifies the device address of the buffer, @len specifies
> + * its size, and @name may contain a human readable name of the trace buffer.
> + *
> + * After booting the remote processor, the trace buffers are exposed to the
> + * user via debugfs entries (called trace0, trace1, etc..).
> + */
> +struct fw_rsc_trace {
> + u32 da;
> + u32 len;
> + u32 reserved;
> + u8 name[32];
> +};
> +
> +/**
> + * struct fw_rsc_vdev_vring - vring descriptor entry
> + * @da: device address
> + * @align: the alignment between the consumer and producer parts of the vring
> + * @num: num of buffers supported by this vring (must be power of two)
> + * @notifyid is a unique rproc-wide notify index for this vring. This notify
> + * index is used when kicking a remote processor, to let it know that this
> + * vring is triggered.
> + * @pa: physical address
> + *
> + * This descriptor is not a resource entry by itself; it is part of the
> + * vdev resource type (see below).
> + *
> + * Note that @da should either contain the device address where
> + * the remote processor is expecting the vring, or indicate that
> + * dynamically allocation of the vring's device address is supported.
> + */
> +struct fw_rsc_vdev_vring {
> + u32 da;
> + u32 align;
> + u32 num;
> + u32 notifyid;
> + u32 pa;
> +};
> +
> +/**
> + * struct fw_rsc_vdev - virtio device header
> + * @id: virtio device id (as in virtio_ids.h)
> + * @notifyid is a unique rproc-wide notify index for this vdev. This notify
> + * index is used when kicking a remote processor, to let it know that the
> + * status/features of this vdev have changes.
> + * @dfeatures specifies the virtio device features supported by the firmware
> + * @gfeatures is a place holder used by the host to write back the
> + * negotiated features that are supported by both sides.
> + * @config_len is the size of the virtio config space of this vdev. The
> config
> + * space lies in the resource table immediate after this vdev header.
> + * @status is a place holder where the host will indicate its virtio
> progress.
> + * @num_of_vrings indicates how many vrings are described in this vdev header
> + * @reserved: reserved (must be zero)
> + * @vring is an array of @num_of_vrings entries of 'struct
> fw_rsc_vdev_vring'.
> + *
> + * This resource is a virtio device header: it provides information about
> + * the vdev, and is then used by the host and its peer remote processors
> + * to negotiate and share certain virtio properties.
> + *
> + * By providing this resource entry, the firmware essentially asks remoteproc
> + * to statically allocate a vdev upon registration of the rproc (dynamic vdev
> + * allocation is not yet supported).
> + *
> + * Note: unlike virtualization systems, the term 'host' here means
> + * the Linux side which is running remoteproc to control the remote
> + * processors. We use the name 'gfeatures' to comply with virtio's terms,
> + * though there isn't really any virtualized guest OS here: it's the host
> + * which is responsible for negotiating the final features.
> + * Yeah, it's a bit confusing.
> + *
> + * Note: immediately following this structure is the virtio config space for
> + * this vdev (which is specific to the vdev; for more info, read the virtio
> + * spec). the size of the config space is specified by @config_len.
> + */
> +struct fw_rsc_vdev {
> + u32 id;
> + u32 notifyid;
> + u32 dfeatures;
> + u32 gfeatures;
> + u32 config_len;
> + u8 status;
> + u8 num_of_vrings;
> + u8 reserved[2];
> + struct fw_rsc_vdev_vring vring[0];
> +};
> +
> +/**
> + * struct rproc_mem_entry - memory entry descriptor
> + * @va: virtual address
> + * @dma: dma address
> + * @len: length, in bytes
> + * @da: device address
> + * @priv: associated data
> + * @name: associated memory region name (optional)
> + * @node: list node
> + */
> +struct rproc_mem_entry {
> + void *va;
> + dma_addr_t dma;
> + int len;
> + u32 da;
> + void *priv;
> + char name[32];
> + struct list_head node;
> +};
> +
> +struct rproc;
> +
> +typedef u32(*init_func_proto) (u32 core_id, struct rproc *cfg);
what is this?
> +
> +struct l3_map {
> + u32 priv_addr;
> + u32 l3_addr;
> + u32 len;
> +};
> +
> +struct rproc_intmem_to_l3_mapping {
> + u32 num_entries;
> + struct l3_map mappings[16];
> +};
> +
> +/**
> + * enum rproc_crash_type - remote processor crash types
> + * @RPROC_MMUFAULT: iommu fault
> + * @RPROC_WATCHDOG: watchdog bite
> + * @RPROC_FATAL_ERROR fatal error
> + *
> + * Each element of the enum is used as an array index. So that, the value of
> + * the elements should be always something sane.
> + *
> + * Feel free to add more types when needed.
> + */
> +enum rproc_crash_type {
> + RPROC_MMUFAULT,
> + RPROC_WATCHDOG,
> + RPROC_FATAL_ERROR,
> +};
> +
> +/* we currently support only two vrings per rvdev */
> +#define RVDEV_NUM_VRINGS 2
> +
> +#define RPMSG_NUM_BUFS (512)
drop brackets
> +#define RPMSG_BUF_SIZE (512)
> +#define RPMSG_TOTAL_BUF_SPACE (RPMSG_NUM_BUFS * RPMSG_BUF_SIZE)
> +
> +/**
> + * struct rproc_vring - remoteproc vring state
> + * @va: virtual address
> + * @dma: dma address
> + * @len: length, in bytes
> + * @da: device address
> + * @align: vring alignment
> + * @notifyid: rproc-specific unique vring index
> + * @rvdev: remote vdev
> + * @vq: the virtqueue of this vring
> + */
> +struct rproc_vring {
> + void *va;
> + dma_addr_t dma;
> + int len;
> + u32 da;
> + u32 align;
> + int notifyid;
> + struct rproc_vdev *rvdev;
> + struct virtqueue *vq;
> +};
> +
> +/** struct rproc - structure with all processor specific information for
> + * loading remotecore from boot loader.
> + *
> + * @num_iommus: Number of IOMMUs for this remote core. Zero indicates that
> the
> + * processor does not have an IOMMU.
> + *
> + * @cma_base: Base address of the carveout for this remotecore.
> + *
> + * @cma_size: Length of the carveout in bytes.
> + *
> + * @page_table_addr: array with the physical address of the page table. We
> are
> + * using the same page table for both IOMMU's. There is currently no strong
> + * usecase for maintaining different page tables for different MMU's
> servicing
> + * the same CPU.
> + *
> + * @mmu_base_addr: base address of the MMU
> + *
> + * @entry_point: address that is the entry point for the remote core. This
> + * address is in the memory view of the remotecore.
> + *
> + * @load_addr: Address to which the bootloader loads the firmware from
> + * persistent storage before invoking the ELF loader. Keeping this address
> + * configurable allows future optimizations such as loading the firmware from
> + * storage for remotecore2 via EDMA while the CPU is processing the ELF image
> + * of remotecore1. This address is in the memory view of the A15.
> + *
> + * @firmware_name: Name of the file that is expected to contain the ELF
> image.
> + *
> + * @has_rsc_table: Flag populated after parsing the ELF binary on target.
Can you add the rest of the members
> + */
> +
> +struct rproc {
> + u32 num_iommus;
> + unsigned long cma_base;
ulong is shorter
> + u32 cma_size;
> + unsigned long page_table_addr;
> + unsigned long mmu_base_addr[2];
> + unsigned long load_addr;
> + unsigned long entry_point;
> + char *core_name;
> + char *firmware_name;
> + char *ptn;
> + init_func_proto start_clocks;
> + init_func_proto config_mmu;
> + init_func_proto config_peripherals;
> + init_func_proto start_core;
> + u32 has_rsc_table;
> + struct rproc_intmem_to_l3_mapping *intmem_to_l3_mapping;
> + u32 trace_pa;
> + u32 trace_len;
> +};
> +
> +extern struct rproc *rproc_cfg_arr[2];
> /**
> * enum rproc_mem_type - What type of memory model does the rproc use
> * @RPROC_INTERNAL_MEMORY_MAPPED: Remote processor uses own memory and is
> memory
> @@ -126,6 +495,12 @@ struct dm_rproc_ops {
> * @return virtual address.
> */
> void * (*device_to_virt)(struct udevice *dev, ulong da, ulong size);
> + int (*add_res)(struct udevice *dev,
> + struct rproc_mem_entry *mapping);
> + void * (*alloc_mem)(struct udevice *dev, unsigned long len,
> + unsigned long align);
I think this should return an error code
> + unsigned int (*config_pagetable)(struct udevice *dev, unsigned int
> virt,
> + unsigned int phys, unsigned int len);
> };
>
> /* Accessor */
> @@ -322,6 +697,13 @@ int rproc_elf64_load_rsc_table(struct udevice *dev,
> ulong fw_addr,
> */
> int rproc_elf_load_rsc_table(struct udevice *dev, ulong fw_addr,
> ulong fw_size, ulong *rsc_addr, ulong *rsc_size);
> +
> +unsigned long rproc_parse_resource_table(struct udevice *dev,
> + struct rproc *cfg);
> +
> +struct resource_table *rproc_find_resource_table(struct udevice *dev,
> + unsigned int addr,
> + int *tablesz);
> #else
> static inline int rproc_init(void) { return -ENOSYS; }
> static inline int rproc_dev_init(int id) { return -ENOSYS; }
> --
> 2.25.1
>
Regards,
Simon
