| Message ID | 1512060411-729-14-git-send-email-loic.pallardy@st.com |
|---|---|
| State | New |
| Headers | show |
| Series | remoteproc: add fixed memory region support | expand |
On Thu 30 Nov 08:46 PST 2017, Loic Pallardy wrote: > This patch parse existing carveout list to find a memory area > matching on "vdev<vdev_id>buffer" name. > If found, memory device will be used as parent for vdev creation, else > rproc platform device will be used as today. > > Signed-off-by: Loic Pallardy <loic.pallardy@st.com> > --- > drivers/remoteproc/remoteproc_core.c | 13 +++++++++++++ > drivers/remoteproc/remoteproc_virtio.c | 2 +- > include/linux/remoteproc.h | 1 + > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > index 6b5e2b2..9c12319 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -583,8 +583,11 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, > { > struct device *dev = &rproc->dev; > struct rproc_vdev *rvdev; > + struct device *memdev = dev->parent; > + struct rproc_mem_entry *carveout; > int i, ret; > static int index; > + char name[16]; > > /* make sure resource isn't truncated */ > if (sizeof(*rsc) + rsc->num_of_vrings * sizeof(struct fw_rsc_vdev_vring) > @@ -637,6 +640,16 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, > > list_add_tail(&rvdev->node, &rproc->rvdevs); > > + /* Find associated registered carveout. */ > + /* Try dedicated vdev buffer pool. */ > + snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index); > + carveout = rproc_find_carveout_by_name(rproc, name); > + > + if (carveout && carveout->memdev) > + memdev = &carveout->memdev->dev; > + > + rvdev->dev = memdev; > + > rproc_add_subdev(rproc, &rvdev->subdev, > rproc_vdev_do_probe, rproc_vdev_do_remove); > > diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c > index 2946348..1f7a444 100644 > --- a/drivers/remoteproc/remoteproc_virtio.c > +++ b/drivers/remoteproc/remoteproc_virtio.c > @@ -303,7 +303,7 @@ 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; This will cause the device structure to change shape based on there being a match of a carveout or not. I also think it's preferable to limit the life cycle of the allocations in this memory region to a single start->stop cycle, rather than boot->shutdown. So I think it makes more sense to use the vdev->dev and dmam_declare_coherent_memory on this. But as in the previous patch this can't be a carveout that has been remapped already. A somewhat unrelated topic to this is the ability to associate DT nodes to rpmsg devices (I do this for the Qualcomm children), in this case we would have a DT node per vdev under the remoteproc, perhaps it would make more sense to introduce that and put the memory-region in that node. Only thin that comes to mind is that we still need to match a carveout in the resource table, in order to communicate the buffer region to the remote side for your memory protection purposes. Regards, Bjorn
> -----Original Message----- > From: Bjorn Andersson [mailto:bjorn.andersson@linaro.org] > Sent: Thursday, December 14, 2017 6:33 AM > To: Loic PALLARDY <loic.pallardy@st.com> > Cc: ohad@wizery.com; linux-remoteproc@vger.kernel.org; linux- > kernel@vger.kernel.org; Arnaud POULIQUEN <arnaud.pouliquen@st.com>; > benjamin.gaignard@linaro.org > Subject: Re: [PATCH v2 13/16] remoteproc: look-up memory-device for virtio > device allocation > > On Thu 30 Nov 08:46 PST 2017, Loic Pallardy wrote: > > > This patch parse existing carveout list to find a memory area > > matching on "vdev<vdev_id>buffer" name. > > If found, memory device will be used as parent for vdev creation, else > > rproc platform device will be used as today. > > > > Signed-off-by: Loic Pallardy <loic.pallardy@st.com> > > --- > > drivers/remoteproc/remoteproc_core.c | 13 +++++++++++++ > > drivers/remoteproc/remoteproc_virtio.c | 2 +- > > include/linux/remoteproc.h | 1 + > > 3 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/remoteproc/remoteproc_core.c > b/drivers/remoteproc/remoteproc_core.c > > index 6b5e2b2..9c12319 100644 > > --- a/drivers/remoteproc/remoteproc_core.c > > +++ b/drivers/remoteproc/remoteproc_core.c > > @@ -583,8 +583,11 @@ static int rproc_handle_vdev(struct rproc *rproc, > struct fw_rsc_vdev *rsc, > > { > > struct device *dev = &rproc->dev; > > struct rproc_vdev *rvdev; > > + struct device *memdev = dev->parent; > > + struct rproc_mem_entry *carveout; > > int i, ret; > > static int index; > > + char name[16]; > > > > /* make sure resource isn't truncated */ > > if (sizeof(*rsc) + rsc->num_of_vrings * sizeof(struct > fw_rsc_vdev_vring) > > @@ -637,6 +640,16 @@ static int rproc_handle_vdev(struct rproc *rproc, > struct fw_rsc_vdev *rsc, > > > > list_add_tail(&rvdev->node, &rproc->rvdevs); > > > > + /* Find associated registered carveout. */ > > + /* Try dedicated vdev buffer pool. */ > > + snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index); > > + carveout = rproc_find_carveout_by_name(rproc, name); > > + > > + if (carveout && carveout->memdev) > > + memdev = &carveout->memdev->dev; > > + > > + rvdev->dev = memdev; > > + > > rproc_add_subdev(rproc, &rvdev->subdev, > > rproc_vdev_do_probe, rproc_vdev_do_remove); > > > > diff --git a/drivers/remoteproc/remoteproc_virtio.c > b/drivers/remoteproc/remoteproc_virtio.c > > index 2946348..1f7a444 100644 > > --- a/drivers/remoteproc/remoteproc_virtio.c > > +++ b/drivers/remoteproc/remoteproc_virtio.c > > @@ -303,7 +303,7 @@ 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; > > This will cause the device structure to change shape based on there > being a match of a carveout or not. > > > I also think it's preferable to limit the life cycle of the allocations > in this memory region to a single start->stop cycle, rather than > boot->shutdown. > > So I think it makes more sense to use the vdev->dev and > dmam_declare_coherent_memory on this. But as in the previous patch this > can't be a carveout that has been remapped already. > Yes it is the main issue. Rproc_handle_carveout function will process it before, except if we create a new status to not allocate a carveout and provide function to update carevout in resource table... > > A somewhat unrelated topic to this is the ability to associate DT nodes > to rpmsg devices (I do this for the Qualcomm children), in this case we > would have a DT node per vdev under the remoteproc, perhaps it would > make more sense to introduce that and put the memory-region in that > node. Only thin that comes to mind is that we still need to match a > carveout in the resource table, in order to communicate the buffer > region to the remote side for your memory protection purposes. > OK I'll have a look to Qualcom DT nodes. But as it is SW, is DT the right place? But it will help to decide if there is a dedicated memory region or not for virtio device. I'll investigate... Regards, Loic > Regards, > Bjorn
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 6b5e2b2..9c12319 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -583,8 +583,11 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, { struct device *dev = &rproc->dev; struct rproc_vdev *rvdev; + struct device *memdev = dev->parent; + struct rproc_mem_entry *carveout; int i, ret; static int index; + char name[16]; /* make sure resource isn't truncated */ if (sizeof(*rsc) + rsc->num_of_vrings * sizeof(struct fw_rsc_vdev_vring) @@ -637,6 +640,16 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, list_add_tail(&rvdev->node, &rproc->rvdevs); + /* Find associated registered carveout. */ + /* Try dedicated vdev buffer pool. */ + snprintf(name, sizeof(name), "vdev%dbuffer", rvdev->index); + carveout = rproc_find_carveout_by_name(rproc, name); + + if (carveout && carveout->memdev) + memdev = &carveout->memdev->dev; + + rvdev->dev = memdev; + rproc_add_subdev(rproc, &rvdev->subdev, rproc_vdev_do_probe, rproc_vdev_do_remove); diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index 2946348..1f7a444 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -303,7 +303,7 @@ 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; int ret; diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index fb293d3..b3d6656 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -506,6 +506,7 @@ struct rproc_vdev { struct kref refcount; struct rproc_subdev subdev; + struct device *dev; unsigned int id; unsigned int index;
This patch parse existing carveout list to find a memory area matching on "vdev<vdev_id>buffer" name. If found, memory device will be used as parent for vdev creation, else rproc platform device will be used as today. Signed-off-by: Loic Pallardy <loic.pallardy@st.com> --- drivers/remoteproc/remoteproc_core.c | 13 +++++++++++++ drivers/remoteproc/remoteproc_virtio.c | 2 +- include/linux/remoteproc.h | 1 + 3 files changed, 15 insertions(+), 1 deletion(-) -- 1.9.1