2019-01-10 13:55:53

by Loic Pallardy

[permalink] [raw]
Subject: [PATCH v2 0/3] remoteproc: complete fixed memory region support

This new series corresponds to the remaining patches from [1]
addressing fixed memory region support for remote processor.

The three remaining patches allow to:
- assign a specific memory region to vdev sub device. As all virtio based
services are using virtio device parent for memory allocation, I propose to
keep a dedicated vdev device per virtio link for memory allocation. A second
step will be to used virtio device itself.
- migrate ST driver on new memory carveout management
- allocate virtio rpmsg buffer from parent device

Regards,
Loic


[1] https://lkml.org/lkml/2018/7/27/612

---
Changes from V1:
- rebase on kernel v5.0-rc0
- correct error management for device registering

Loic Pallardy (3):
remoteproc: create vdev subdevice with specific dma memory pool
remoteproc: st: add reserved memory support
rpmsg: virtio: allocate buffer from parent

drivers/remoteproc/remoteproc_core.c | 47 +++++++++++++++--
drivers/remoteproc/remoteproc_internal.h | 1 +
drivers/remoteproc/remoteproc_virtio.c | 42 ++++++++++++++-
drivers/remoteproc/st_remoteproc.c | 91 ++++++++++++++++++++++++++++----
drivers/rpmsg/virtio_rpmsg_bus.c | 6 +--
include/linux/remoteproc.h | 1 +
6 files changed, 170 insertions(+), 18 deletions(-)

--
2.7.4



2019-01-10 13:56:12

by Loic Pallardy

[permalink] [raw]
Subject: [PATCH v2 1/3] remoteproc: create vdev subdevice with specific dma memory pool

This patch creates a dedicated vdev subdevice for each vdev declared
in firmware resource table and associates carveout named "vdev%dbuffer"
(with %d vdev index in resource table) if any as dma coherent memory pool.

Then vdev subdevice is used as parent for virtio device.

Signed-off-by: Loic Pallardy <[email protected]>

---
Changes from v1:
- rebase on v5.0-rc1 (dma_declare_coherent_memory() API change)
- call put_device() in case of error returned by device_register()
---
drivers/remoteproc/remoteproc_core.c | 47 ++++++++++++++++++++++++++++++--
drivers/remoteproc/remoteproc_internal.h | 1 +
drivers/remoteproc/remoteproc_virtio.c | 42 +++++++++++++++++++++++++++-
include/linux/remoteproc.h | 1 +
4 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 54ec38fc5dca..821dbedef18e 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -39,9 +39,11 @@
#include <linux/idr.h>
#include <linux/elf.h>
#include <linux/crc32.h>
+#include <linux/of_reserved_mem.h>
#include <linux/virtio_ids.h>
#include <linux/virtio_ring.h>
#include <asm/byteorder.h>
+#include <linux/platform_device.h>

#include "remoteproc_internal.h"

@@ -145,7 +147,7 @@ static void rproc_disable_iommu(struct rproc *rproc)
iommu_domain_free(domain);
}

-static phys_addr_t rproc_va_to_pa(void *cpu_addr)
+phys_addr_t rproc_va_to_pa(void *cpu_addr)
{
/*
* Return physical address according to virtual address location
@@ -160,6 +162,7 @@ static phys_addr_t rproc_va_to_pa(void *cpu_addr)
WARN_ON(!virt_addr_valid(cpu_addr));
return virt_to_phys(cpu_addr);
}
+EXPORT_SYMBOL(rproc_va_to_pa);

/**
* rproc_da_to_va() - lookup the kernel virtual address for a remoteproc address
@@ -423,6 +426,20 @@ static void rproc_vdev_do_stop(struct rproc_subdev *subdev, bool crashed)
}

/**
+ * rproc_rvdev_release() - release the existence of a rvdev
+ *
+ * @dev: the subdevice's dev
+ */
+static void rproc_rvdev_release(struct device *dev)
+{
+ struct rproc_vdev *rvdev = container_of(dev, struct rproc_vdev, dev);
+
+ of_reserved_mem_device_release(dev);
+
+ kfree(rvdev);
+}
+
+/**
* rproc_handle_vdev() - handle a vdev fw resource
* @rproc: the remote processor
* @rsc: the vring resource descriptor
@@ -455,6 +472,7 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
struct device *dev = &rproc->dev;
struct rproc_vdev *rvdev;
int i, ret;
+ char name[16];

/* make sure resource isn't truncated */
if (sizeof(*rsc) + rsc->num_of_vrings * sizeof(struct fw_rsc_vdev_vring)
@@ -488,6 +506,29 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
rvdev->rproc = rproc;
rvdev->index = rproc->nb_vdev++;

+ /* Initialise vdev subdevice */
+ snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index);
+ rvdev->dev.parent = rproc->dev.parent;
+ rvdev->dev.release = rproc_rvdev_release;
+ dev_set_name(&rvdev->dev, "%s#%s", dev_name(rvdev->dev.parent), name);
+ dev_set_drvdata(&rvdev->dev, rvdev);
+
+ ret = device_register(&rvdev->dev);
+ if (ret) {
+ put_device(&rvdev->dev);
+ return ret;
+ }
+ /* Make device dma capable by inheriting from parent's capabilities */
+ set_dma_ops(&rvdev->dev, get_dma_ops(rproc->dev.parent));
+
+ ret = dma_coerce_mask_and_coherent(&rvdev->dev,
+ dma_get_mask(rproc->dev.parent));
+ if (ret) {
+ dev_warn(dev,
+ "Failed to set DMA mask %llx. Trying to continue... %x\n",
+ dma_get_mask(rproc->dev.parent), ret);
+ }
+
/* parse the vrings */
for (i = 0; i < rsc->num_of_vrings; i++) {
ret = rproc_parse_vring(rvdev, rsc, i);
@@ -518,7 +559,7 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc,
for (i--; i >= 0; i--)
rproc_free_vring(&rvdev->vring[i]);
free_rvdev:
- kfree(rvdev);
+ device_unregister(&rvdev->dev);
return ret;
}

@@ -536,7 +577,7 @@ void rproc_vdev_release(struct kref *ref)

rproc_remove_subdev(rproc, &rvdev->subdev);
list_del(&rvdev->node);
- kfree(rvdev);
+ device_unregister(&rvdev->dev);
}

/**
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index f6cad243d7ca..bfeacfd40947 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -52,6 +52,7 @@ void rproc_free_vring(struct rproc_vring *rvring);
int rproc_alloc_vring(struct rproc_vdev *rvdev, int i);

void *rproc_da_to_va(struct rproc *rproc, u64 da, int len);
+phys_addr_t rproc_va_to_pa(void *cpu_addr);
int rproc_trigger_recovery(struct rproc *rproc);

int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw);
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 183fc42a510a..d08b2cfd875b 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -17,7 +17,9 @@
* GNU General Public License for more details.
*/

+#include <linux/dma-mapping.h>
#include <linux/export.h>
+#include <linux/of_reserved_mem.h>
#include <linux/remoteproc.h>
#include <linux/virtio.h>
#include <linux/virtio_config.h>
@@ -328,10 +330,48 @@ static void rproc_virtio_dev_release(struct device *dev)
int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
{
struct rproc *rproc = rvdev->rproc;
- struct device *dev = &rproc->dev;
+ struct device *dev = &rvdev->dev;
struct virtio_device *vdev = &rvdev->vdev;
+ struct rproc_mem_entry *mem;
int ret;

+ /* Try to find dedicated vdev buffer carveout */
+ mem = rproc_find_carveout_by_name(rproc, "vdev%dbuffer", rvdev->index);
+ if (mem) {
+ phys_addr_t pa;
+
+ if (mem->of_resm_idx != -1) {
+ struct device_node *np = rproc->dev.parent->of_node;
+
+ /* Associate reserved memory to vdev device */
+ ret = of_reserved_mem_device_init_by_idx(dev, np,
+ mem->of_resm_idx);
+ if (ret) {
+ dev_err(dev, "Can't associate reserved memory\n");
+ goto out;
+ }
+ } else {
+ if (mem->va) {
+ dev_warn(dev, "vdev %d buffer already mapped\n",
+ rvdev->index);
+ pa = rproc_va_to_pa(mem->va);
+ } else {
+ /* Use dma address as carveout no memmapped yet */
+ pa = (phys_addr_t)mem->dma;
+ }
+
+ /* Associate vdev buffer memory pool to vdev subdev */
+ ret = dma_declare_coherent_memory(dev, pa,
+ mem->da,
+ mem->len,
+ DMA_MEMORY_EXCLUSIVE);
+ if (ret < 0) {
+ dev_err(dev, "Failed to associate buffer\n");
+ goto out;
+ }
+ }
+ }
+
vdev->id.device = id,
vdev->config = &rproc_virtio_config_ops,
vdev->dev.parent = dev;
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 507a2b524208..beecd8cf2eb5 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -554,6 +554,7 @@ struct rproc_vdev {
struct kref refcount;

struct rproc_subdev subdev;
+ struct device dev;

unsigned int id;
struct list_head node;
--
2.7.4


2019-01-10 16:10:51

by Loic Pallardy

[permalink] [raw]
Subject: [PATCH v2 2/3] remoteproc: st: add reserved memory support

ST remote processor needs some specified memory regions for
firmware and IPC.
Memory regions are defined as reserved memory and should
be registered in remoteproc core thanks to rproc_add_carveout
function before rproc_start. For this, st rproc driver implements
prepare ops.

Signed-off-by: Loic Pallardy <[email protected]>
---
drivers/remoteproc/st_remoteproc.c | 91 +++++++++++++++++++++++++++++++++-----
1 file changed, 80 insertions(+), 11 deletions(-)

diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c
index aacef0ea3b90..51049d17b1e5 100644
--- a/drivers/remoteproc/st_remoteproc.c
+++ b/drivers/remoteproc/st_remoteproc.c
@@ -19,6 +19,7 @@
#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_address.h>
#include <linux/of_device.h>
#include <linux/of_reserved_mem.h>
#include <linux/platform_device.h>
@@ -91,6 +92,77 @@ static void st_rproc_kick(struct rproc *rproc, int vqid)
dev_err(dev, "failed to send message via mbox: %d\n", ret);
}

+static int st_rproc_mem_alloc(struct rproc *rproc,
+ struct rproc_mem_entry *mem)
+{
+ struct device *dev = rproc->dev.parent;
+ void *va;
+
+ va = ioremap_wc(mem->dma, mem->len);
+ if (!va) {
+ dev_err(dev, "Unable to map memory region: %pa+%zx\n",
+ &mem->dma, mem->len);
+ return -ENOMEM;
+ }
+
+ /* Update memory entry va */
+ mem->va = va;
+
+ return 0;
+}
+
+static int st_rproc_mem_release(struct rproc *rproc,
+ struct rproc_mem_entry *mem)
+{
+ iounmap(mem->va);
+
+ return 0;
+}
+
+static int st_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
+{
+ struct device *dev = rproc->dev.parent;
+ struct device_node *np = dev->of_node;
+ struct rproc_mem_entry *mem;
+ struct reserved_mem *rmem;
+ struct of_phandle_iterator it;
+ int index = 0;
+
+ of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
+ while (of_phandle_iterator_next(&it) == 0) {
+ rmem = of_reserved_mem_lookup(it.node);
+ if (!rmem) {
+ dev_err(dev, "unable to acquire memory-region\n");
+ return -EINVAL;
+ }
+
+ /* No need to map vdev buffer */
+ if (strcmp(it.node->name, "vdev0buffer")) {
+ /* Register memory region */
+ mem = rproc_mem_entry_init(dev, NULL,
+ (dma_addr_t)rmem->base,
+ rmem->size, rmem->base,
+ st_rproc_mem_alloc,
+ st_rproc_mem_release,
+ it.node->name);
+ } else {
+ /* Register reserved memory for vdev buffer allocation */
+ mem = rproc_of_resm_mem_entry_init(dev, index,
+ rmem->size,
+ rmem->base,
+ it.node->name);
+ }
+
+ if (!mem)
+ return -ENOMEM;
+
+ rproc_add_carveout(rproc, mem);
+ index++;
+ }
+
+ return rproc_elf_load_rsc_table(rproc, fw);
+}
+
static int st_rproc_start(struct rproc *rproc)
{
struct st_rproc *ddata = rproc->priv;
@@ -158,9 +230,14 @@ static int st_rproc_stop(struct rproc *rproc)
}

static const struct rproc_ops st_rproc_ops = {
- .kick = st_rproc_kick,
- .start = st_rproc_start,
- .stop = st_rproc_stop,
+ .kick = st_rproc_kick,
+ .start = st_rproc_start,
+ .stop = st_rproc_stop,
+ .parse_fw = st_rproc_parse_fw,
+ .load = rproc_elf_load_segments,
+ .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
+ .sanity_check = rproc_elf_sanity_check,
+ .get_boot_addr = rproc_elf_get_boot_addr,
};

/*
@@ -254,12 +331,6 @@ static int st_rproc_parse_dt(struct platform_device *pdev)
return -EINVAL;
}

- err = of_reserved_mem_device_init(dev);
- if (err) {
- dev_err(dev, "Failed to obtain shared memory\n");
- return err;
- }
-
err = clk_prepare(ddata->clk);
if (err)
dev_err(dev, "failed to get clock\n");
@@ -387,8 +458,6 @@ static int st_rproc_remove(struct platform_device *pdev)

clk_disable_unprepare(ddata->clk);

- of_reserved_mem_device_release(&pdev->dev);
-
for (i = 0; i < ST_RPROC_MAX_VRING * MBOX_MAX; i++)
mbox_free_channel(ddata->mbox_chan[i]);

--
2.7.4


2019-01-10 19:33:45

by Loic Pallardy

[permalink] [raw]
Subject: [PATCH v2 3/3] rpmsg: virtio: allocate buffer from parent

Remoteproc is now capable to create one specific sub-device per
virtio link to associate a dedicated memory pool.
This implies to change device used by virtio_rpmsg for
buffer allocation from grand-parent to parent.

Signed-off-by: Loic Pallardy <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Tested-by: Anup Patel <[email protected]>
---
drivers/rpmsg/virtio_rpmsg_bus.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 664f957012cd..5c892019ce89 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -912,7 +912,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
total_buf_space = vrp->num_bufs * vrp->buf_size;

/* allocate coherent memory for the buffers */
- bufs_va = dma_alloc_coherent(vdev->dev.parent->parent,
+ bufs_va = dma_alloc_coherent(vdev->dev.parent,
total_buf_space, &vrp->bufs_dma,
GFP_KERNEL);
if (!bufs_va) {
@@ -980,7 +980,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
return 0;

free_coherent:
- dma_free_coherent(vdev->dev.parent->parent, total_buf_space,
+ dma_free_coherent(vdev->dev.parent, total_buf_space,
bufs_va, vrp->bufs_dma);
vqs_del:
vdev->config->del_vqs(vrp->vdev);
@@ -1015,7 +1015,7 @@ static void rpmsg_remove(struct virtio_device *vdev)

vdev->config->del_vqs(vrp->vdev);

- dma_free_coherent(vdev->dev.parent->parent, total_buf_space,
+ dma_free_coherent(vdev->dev.parent, total_buf_space,
vrp->rbufs, vrp->bufs_dma);

kfree(vrp);
--
2.7.4