Linux Remote Processor Subsystem development
 help / color / mirror / Atom feed
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Francesco Valla <francesco@valla.it>
Cc: Bjorn Andersson <andersson@kernel.org>,
	Kees Cook <kees@kernel.org>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH] remoteproc: virtio: support dynamic number of vrings
Date: Mon, 29 Jun 2026 10:43:35 -0600	[thread overview]
Message-ID: <akKgtwY5iGo0xsPD@p14s> (raw)
In-Reply-To: <aj2PQGRj4N_xWnyp@bywater>

On Thu, Jun 25, 2026 at 10:54:05PM +0200, Francesco Valla wrote:
> Hi Mathieu,
> 
> On Wed, Jun 24, 2026 at 11:55:00AM -0600, Mathieu Poirier wrote:
> > Hi Francesco,
> > 
> > On Sun, Jun 21, 2026 at 11:57:31PM +0200, Francesco Valla wrote:
> > > The number of vrings for each vdev has been fixed to 2 since the
> > > introduction of multi-vdev support [1]; this is completely fine for the
> > > rpmsg usecase, but can conflict with other virtio devices (CAN for
> > > example requires 3 virtqueues, entropy only 1, network a variable number
> > > and so on).
> > 
> > I suppose the remoteproc subsystem is involved because these other virtio
> > devices are behind a remote processor?  If so, how does virtio drivers for the
> > devices get called?
> > 
> 
> Yes, the idea is to have virtio devices provided by a remote processor.
> I am investingating the possibility after several years of custom
> solutions (especially for CAN buses, now covered by the recent virtio
> CAN device), as well as a brief discussion on the possibility of using
> virtio-gpu for screen sharing in mixed-criticality systems instead of
> rpmsg-based solutions (like the one presented recently by TI [1]).
> 
> AFAIU, the resource table format used by the remoteproc framework was
> designed to support any kind of virtio device, and in fact the
> rproc-virtio driver is not restricting the device type to a subset of
> allowed device IDs wehn calling register_virtio_device() [2].
> 
> In practice, only two virtio drivers are usable over the proc framework
> today, them being rpmsg and virtio console, since they are the only two
> that allocate their transfer buffers from the memory provided by the
> rproc framework (using dma_alloc_coherent()) and then properly translate
> the addresses used inside the scatter-gather lists used for vring
> transfers [3] [4].
> 
> I am still unsure on how to tackle the other drivers - I have a PoC for
> a modified virtio-rng driver, but other solutions are emerging, like the
> hypervisorless virtio effort [5] and virtio-msg [6].
>

My goal in asking for more details was to see if you had done your research on
this topic - and you have.  
 
> During this investigation, I stumbled on the limitation posed by the
> fixed number of vrings and decided to send a patch for it.
> 

Please resend this patch as part of the patchset that provides support for (one)
of the devices you are interested in and we'll take it from there.

> > Otherwise I'm good with the code.
> > 
> > Thanks,
> > Mathieu
> > 
> 
> Thank you!
> 
> Reagrds,
> Francesco
> 
> [1] https://cfp.embedded-recipes.org/media/er2026/submissions/PCYPJP/resources/rpmsg-kms-dispay-em_QToijxj.pdf
> [2] https://elixir.bootlin.com/linux/v7.1.1/source/drivers/remoteproc/remoteproc_virtio.c#L446
> [3] https://elixir.bootlin.com/linux/v7.1.1/source/drivers/rpmsg/virtio_rpmsg_bus.c#L864
> [4] https://elixir.bootlin.com/linux/v7.1.1/source/drivers/char/virtio_console.c#L426
> [5] https://openamp.readthedocs.io/en/latest/hypervisorless_virtio_zcu102/README_hypervisorless_virtio.html
> [6] https://lore.kernel.org/all/cover.1781514628.git.bertrand.marquis@arm.com/
> 
> > > 
> > > Remove the static vring allocation, transforming it to a flex array that
> > > is allocated at vdev probe time; for the existent usecases (i.e.: mainly
> > > rpmsg) this leads to no functional change, except the additional memory
> > > used for the counter associated to the new array.
> > > 
> > > The maximum number of virtqueues is limited to 256 due to the uint8_t
> > > value used inside the resource table to indicate the number of vring to
> > > allocate; for this reason, no additional plausibility check is performed
> > > on the number of vrings indicated by the resource table.
> > > 
> > > As a side effect, this also fixes the single virtqueue usecase, which
> > > was apparently supported also before but for which the remove action
> > > caused an error (because the remove action was trying to unmap also the
> > > second vring, which was in fact not mapped).
> > > 
> > > [1] https://lore.kernel.org/all/1330589497-4139-5-git-send-email-ohad@wizery.com/
> > > 
> > > Signed-off-by: Francesco Valla <francesco@valla.it>
> > > ---
> > >  drivers/remoteproc/remoteproc_core.c   |  7 -------
> > >  drivers/remoteproc/remoteproc_virtio.c | 21 +++++++++++++--------
> > >  include/linux/remoteproc.h             | 10 ++++------
> > >  3 files changed, 17 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> > > index f003be006b1b..88504d6eed93 100644
> > > --- a/drivers/remoteproc/remoteproc_core.c
> > > +++ b/drivers/remoteproc/remoteproc_core.c
> > > @@ -473,7 +473,6 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
> > >  {
> > >  	struct fw_rsc_vdev *rsc = ptr;
> > >  	struct device *dev = &rproc->dev;
> > > -	struct rproc_vdev *rvdev;
> > >  	size_t rsc_size;
> > >  	struct rproc_vdev_data rvdev_data;
> > >  	struct platform_device *pdev;
> > > @@ -494,12 +493,6 @@ static int rproc_handle_vdev(struct rproc *rproc, void *ptr,
> > >  	dev_dbg(dev, "vdev rsc: id %d, dfeatures 0x%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 > ARRAY_SIZE(rvdev->vring)) {
> > > -		dev_err(dev, "too many vrings: %d\n", rsc->num_of_vrings);
> > > -		return -EINVAL;
> > > -	}
> > > -
> > >  	rvdev_data.id = rsc->id;
> > >  	rvdev_data.index = rproc->nb_vdev++;
> > >  	rvdev_data.rsc_offset = offset;
> > > diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
> > > index d5e9ff045a28..96f13f828b8e 100644
> > > --- a/drivers/remoteproc/remoteproc_virtio.c
> > > +++ b/drivers/remoteproc/remoteproc_virtio.c
> > > @@ -115,8 +115,7 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev,
> > >  	void *addr;
> > >  	int num, size;
> > >  
> > > -	/* we're temporarily limited to two virtqueues per rvdev */
> > > -	if (id >= ARRAY_SIZE(rvdev->vring))
> > > +	if (id >= rvdev->num_vrings)
> > >  		return ERR_PTR(-EINVAL);
> > >  
> > >  	if (!name)
> > > @@ -503,17 +502,20 @@ static int rproc_virtio_probe(struct platform_device *pdev)
> > >  	if (!rvdev_data)
> > >  		return -EINVAL;
> > >  
> > > -	rvdev = devm_kzalloc(dev, sizeof(*rvdev), GFP_KERNEL);
> > > +	rsc = rvdev_data->rsc;
> > > +
> > > +	rvdev = kzalloc_flex(*rvdev, vring, rsc->num_of_vrings);
> > >  	if (!rvdev)
> > >  		return -ENOMEM;
> > >  
> > >  	rvdev->id = rvdev_data->id;
> > >  	rvdev->rproc = rproc;
> > >  	rvdev->index = rvdev_data->index;
> > > +	rvdev->num_vrings = rsc->num_of_vrings;
> > >  
> > >  	ret = copy_dma_range_map(dev, rproc->dev.parent);
> > >  	if (ret)
> > > -		return ret;
> > > +		goto free_rvdev;
> > >  
> > >  	/* Make device dma capable by inheriting from parent's capabilities */
> > >  	set_dma_ops(dev, get_dma_ops(rproc->dev.parent));
> > > @@ -527,13 +529,11 @@ static int rproc_virtio_probe(struct platform_device *pdev)
> > >  	platform_set_drvdata(pdev, rvdev);
> > >  	rvdev->pdev = pdev;
> > >  
> > > -	rsc = rvdev_data->rsc;
> > > -
> > >  	/* parse the vrings */
> > >  	for (i = 0; i < rsc->num_of_vrings; i++) {
> > >  		ret = rproc_parse_vring(rvdev, rsc, i);
> > >  		if (ret)
> > > -			return ret;
> > > +			goto free_rvdev;
> > >  	}
> > >  
> > >  	/* remember the resource offset*/
> > > @@ -569,6 +569,9 @@ static int rproc_virtio_probe(struct platform_device *pdev)
> > >  	for (i--; i >= 0; i--)
> > >  		rproc_free_vring(&rvdev->vring[i]);
> > >  
> > > +free_rvdev:
> > > +	kfree(rvdev);
> > > +
> > >  	return ret;
> > >  }
> > >  
> > > @@ -579,7 +582,7 @@ static void rproc_virtio_remove(struct platform_device *pdev)
> > >  	struct rproc_vring *rvring;
> > >  	int id;
> > >  
> > > -	for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
> > > +	for (id = 0; id < rvdev->num_vrings; id++) {
> > >  		rvring = &rvdev->vring[id];
> > >  		rproc_free_vring(rvring);
> > >  	}
> > > @@ -588,6 +591,8 @@ static void rproc_virtio_remove(struct platform_device *pdev)
> > >  	rproc_remove_rvdev(rvdev);
> > >  
> > >  	put_device(&rproc->dev);
> > > +
> > > +	kfree(rvdev);
> > >  }
> > >  
> > >  /* Platform driver */
> > > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> > > index 7c1546d48008..222e1a56cebc 100644
> > > --- a/include/linux/remoteproc.h
> > > +++ b/include/linux/remoteproc.h
> > > @@ -339,10 +339,6 @@ struct rproc_subdev {
> > >  	void (*unprepare)(struct rproc_subdev *subdev);
> > >  };
> > >  
> > > -/* we currently support only two vrings per rvdev */
> > > -
> > > -#define RVDEV_NUM_VRINGS 2
> > > -
> > >  /**
> > >   * struct rproc_vring - remoteproc vring state
> > >   * @va:	virtual address
> > > @@ -370,9 +366,10 @@ struct rproc_vring {
> > >   * @id: virtio device id (as in virtio_ids.h)
> > >   * @node: list node
> > >   * @rproc: the rproc handle
> > > - * @vring: the vrings for this vdev
> > >   * @rsc_offset: offset of the vdev's resource entry
> > >   * @index: vdev position versus other vdev declared in resource table
> > > + * @num_vrings: the number of vrings for this vdev
> > > + * @vring: the vrings for this vdev
> > >   */
> > >  struct rproc_vdev {
> > >  
> > > @@ -382,9 +379,10 @@ struct rproc_vdev {
> > >  	unsigned int id;
> > >  	struct list_head node;
> > >  	struct rproc *rproc;
> > > -	struct rproc_vring vring[RVDEV_NUM_VRINGS];
> > >  	u32 rsc_offset;
> > >  	u32 index;
> > > +	unsigned int num_vrings;
> > > +	struct rproc_vring vring[] __counted_by(num_vrings);
> > >  };
> > >  
> > >  struct rproc *rproc_get_by_phandle(phandle phandle);
> > > 
> > > ---
> > > base-commit: ef0c9f75a19532d7675384708fc8621e10850104
> > > change-id: 20260618-vring_flex-b4d23d1974ba
> > > 
> > > Best regards,
> > > --  
> > > Francesco Valla <francesco@valla.it>
> > > 

      reply	other threads:[~2026-06-29 16:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-21 21:57 [PATCH] remoteproc: virtio: support dynamic number of vrings Francesco Valla
2026-06-24 17:55 ` Mathieu Poirier
2026-06-25 20:54   ` Francesco Valla
2026-06-29 16:43     ` Mathieu Poirier [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=akKgtwY5iGo0xsPD@p14s \
    --to=mathieu.poirier@linaro.org \
    --cc=andersson@kernel.org \
    --cc=francesco@valla.it \
    --cc=gustavoars@kernel.org \
    --cc=kees@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox