[1/2] vhost: search the split vq desc and avail in RO areas

Message ID 20250605113552.918120-2-eperezma@redhat.com (mailing list archive)
State Awaiting Upstream
Delegated to: Maxime Coquelin
Headers
Series Search the split vq desc and avail in RO areas |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Eugenio Perez Martin June 5, 2025, 11:35 a.m. UTC
QEMU's shadow virtqueue and VDUSE exposes these areas as read-only. If
we don't change it, vhost_iova_to_vva do not consider them as valid and
returns that they're not found.

Fixes: eefac9536a90 ("vhost: postpone device creation until rings are mapped")
Cc: stable@dpdk.org

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 lib/vhost/vhost.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Maxime Coquelin June 12, 2025, 1:55 p.m. UTC | #1
On 6/5/25 1:35 PM, Eugenio Pérez wrote:
> QEMU's shadow virtqueue and VDUSE exposes these areas as read-only. If
> we don't change it, vhost_iova_to_vva do not consider them as valid and
> returns that they're not found.
> 
> Fixes: eefac9536a90 ("vhost: postpone device creation until rings are mapped")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>   lib/vhost/vhost.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
> index 0353a04dc8..95a99bace6 100644
> --- a/lib/vhost/vhost.c
> +++ b/lib/vhost/vhost.c
> @@ -497,7 +497,7 @@ vring_translate_split(struct virtio_net *dev, struct vhost_virtqueue *vq)
>   	size = req_size;
>   	vq->desc = (struct vring_desc *)(uintptr_t)vhost_iova_to_vva(dev, vq,
>   						vq->ring_addrs.desc_user_addr,
> -						&size, VHOST_ACCESS_RW);
> +						&size, VHOST_ACCESS_RO);
>   	if (!vq->desc || size != req_size)
>   		return -1;
>   
> @@ -508,7 +508,7 @@ vring_translate_split(struct virtio_net *dev, struct vhost_virtqueue *vq)
>   	size = req_size;
>   	vq->avail = (struct vring_avail *)(uintptr_t)vhost_iova_to_vva(dev, vq,
>   						vq->ring_addrs.avail_user_addr,
> -						&size, VHOST_ACCESS_RW);
> +						&size, VHOST_ACCESS_RO);
>   	if (!vq->avail || size != req_size)
>   		return -1;
>   

I propose below text for the commit message:

"
The virtqueues driver areas are read-only for the device.
While they are exposed as Read/Write to Vhost-user and regular VDUSE
backends, they are only exposed as Read-only when the control Virtqueue
is shadowed by QEMU with VDUSE backend.

This patch makes the backend to request these areas as Read-only, so 
that it can be accessed in all the configurations.
"

Also, as requested by David, patch 2 will be squashed into this one.

Eugenio, is that Ok for you?

With above suggested changes:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime
  
Eugenio Perez Martin June 12, 2025, 2:39 p.m. UTC | #2
On Thu, Jun 12, 2025 at 3:56 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
>
>
> On 6/5/25 1:35 PM, Eugenio Pérez wrote:
> > QEMU's shadow virtqueue and VDUSE exposes these areas as read-only. If
> > we don't change it, vhost_iova_to_vva do not consider them as valid and
> > returns that they're not found.
> >
> > Fixes: eefac9536a90 ("vhost: postpone device creation until rings are mapped")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >   lib/vhost/vhost.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
> > index 0353a04dc8..95a99bace6 100644
> > --- a/lib/vhost/vhost.c
> > +++ b/lib/vhost/vhost.c
> > @@ -497,7 +497,7 @@ vring_translate_split(struct virtio_net *dev, struct vhost_virtqueue *vq)
> >       size = req_size;
> >       vq->desc = (struct vring_desc *)(uintptr_t)vhost_iova_to_vva(dev, vq,
> >                                               vq->ring_addrs.desc_user_addr,
> > -                                             &size, VHOST_ACCESS_RW);
> > +                                             &size, VHOST_ACCESS_RO);
> >       if (!vq->desc || size != req_size)
> >               return -1;
> >
> > @@ -508,7 +508,7 @@ vring_translate_split(struct virtio_net *dev, struct vhost_virtqueue *vq)
> >       size = req_size;
> >       vq->avail = (struct vring_avail *)(uintptr_t)vhost_iova_to_vva(dev, vq,
> >                                               vq->ring_addrs.avail_user_addr,
> > -                                             &size, VHOST_ACCESS_RW);
> > +                                             &size, VHOST_ACCESS_RO);
> >       if (!vq->avail || size != req_size)
> >               return -1;
> >
>
> I propose below text for the commit message:
>
> "
> The virtqueues driver areas are read-only for the device.
> While they are exposed as Read/Write to Vhost-user and regular VDUSE
> backends, they are only exposed as Read-only when the control Virtqueue
> is shadowed by QEMU with VDUSE backend.
>
> This patch makes the backend to request these areas as Read-only, so
> that it can be accessed in all the configurations.
> "
>
> Also, as requested by David, patch 2 will be squashed into this one.
>
> Eugenio, is that Ok for you?
>

Sure, I'm ok with that.

Thanks!

> With above suggested changes:
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> Thanks,
> Maxime
>
  

Patch

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index 0353a04dc8..95a99bace6 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -497,7 +497,7 @@  vring_translate_split(struct virtio_net *dev, struct vhost_virtqueue *vq)
 	size = req_size;
 	vq->desc = (struct vring_desc *)(uintptr_t)vhost_iova_to_vva(dev, vq,
 						vq->ring_addrs.desc_user_addr,
-						&size, VHOST_ACCESS_RW);
+						&size, VHOST_ACCESS_RO);
 	if (!vq->desc || size != req_size)
 		return -1;
 
@@ -508,7 +508,7 @@  vring_translate_split(struct virtio_net *dev, struct vhost_virtqueue *vq)
 	size = req_size;
 	vq->avail = (struct vring_avail *)(uintptr_t)vhost_iova_to_vva(dev, vq,
 						vq->ring_addrs.avail_user_addr,
-						&size, VHOST_ACCESS_RW);
+						&size, VHOST_ACCESS_RO);
 	if (!vq->avail || size != req_size)
 		return -1;