Re: [PATCH] net/virtio-user: fix used ring address calculation

[Maxime Coquelin]

On 8/18/25 2:56 PM, David Marchand wrote:

On Tue, Aug 5, 2025 at 10:37 AM Maxime Coquelin
<maxime.coque...@redhat.com> wrote:

This patch fixes the used ring address calculation, to
avoid Vhost-vDPA backends (such as VDUSE) to fail while
trying to translate it.

Maybe update the patch title to reflect that this issue affects
vhost-vdpa backend?

Fixes: 666ef294ddf7 ("net/virtio-user: share descriptor IOVA to backend")
Cc: sta...@dpdk.org

Reported-by: Adrian Moreno <amore...@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com>
---
  drivers/net/virtio/virtio_user/virtio_user_dev.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c 
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 187f81b066..7789f337f6 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -149,7 +149,8 @@ virtio_user_kick_queue(struct virtio_user_dev *dev, 
uint32_t queue_sel)
         } else {
                 desc_addr = vring->desc_iova;
                 avail_addr = desc_addr + vring->num * sizeof(struct 
vring_desc);
-               used_addr = 
RTE_ALIGN_CEIL((uintptr_t)(&vring->avail->ring[vring->num]),
+               used_addr = RTE_ALIGN_CEIL(avail_addr + offsetof(struct 
vring_avail,
+                                               ring[vring->num]),
                                            VIRTIO_VRING_ALIGN);

The fix looks good to me, but I would go one step further (as a
followup maybe?).

I see no good reason for computing again the addresses in the kick
helper, while those addresses should be set once and for all at
"setup" time.
Only an offset needs to be applied to account for iova stuff.

What do you think of something like:

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 187f81b066..dcb702ace3 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -118,7 +118,7 @@ virtio_user_kick_queue(struct virtio_user_dev
*dev, uint32_t queue_sel)
         struct vhost_vring_state state;
         struct vring *vring = &dev->vrings.split[queue_sel];
         struct vring_packed *pq_vring = &dev->vrings.packed[queue_sel];
-       uint64_t desc_addr, avail_addr, used_addr;
+       uint64_t desc_addr, desc_iova_addr, avail_addr, used_addr;
         struct vhost_vring_addr addr = {
                 .index = queue_sel,
                 .log_guest_addr = 0,
@@ -138,25 +138,22 @@ virtio_user_kick_queue(struct virtio_user_dev
*dev, uint32_t queue_sel)
         }

         if (dev->features & (1ULL << VIRTIO_F_RING_PACKED)) {
-               desc_addr = pq_vring->desc_iova;
-               avail_addr = desc_addr + pq_vring->num * sizeof(struct
vring_packed_desc);
-               used_addr =  RTE_ALIGN_CEIL(avail_addr + sizeof(struct
vring_packed_desc_event),
-                                           VIRTIO_VRING_ALIGN);
-
-               addr.desc_user_addr = desc_addr;
-               addr.avail_user_addr = avail_addr;
-               addr.used_user_addr = used_addr;
+               desc_iova_addr = pq_vring->desc_iova;
+               desc_addr = (uint64_t)(uintptr_t)pq_vring->desc;
+               avail_addr = (uint64_t)(uintptr_t)pq_vring->driver;
+               used_addr = (uint64_t)(uintptr_t)pq_vring->device;
+
         } else {
-               desc_addr = vring->desc_iova;
-               avail_addr = desc_addr + vring->num * sizeof(struct vring_desc);
-               used_addr =
RTE_ALIGN_CEIL((uintptr_t)(&vring->avail->ring[vring->num]),
-                                          VIRTIO_VRING_ALIGN);
-
-               addr.desc_user_addr = desc_addr;
-               addr.avail_user_addr = avail_addr;
-               addr.used_user_addr = used_addr;
+               desc_iova_addr = vring->desc_iova;
+               desc_addr = (uint64_t)(uintptr_t)vring->desc;
+               avail_addr = (uint64_t)(uintptr_t)vring->avail;
+               used_addr = (uint64_t)(uintptr_t)vring->used;
         }

+       addr.desc_user_addr = desc_iova_addr;
+       addr.avail_user_addr = (desc_iova_addr - desc_addr) + avail_addr;
+       addr.used_user_addr = (desc_iova_addr - desc_addr) + used_addr;
+
         state.index = queue_sel;
         state.num = vring->num;
         ret = dev->ops->set_vring_num(dev, &state);

I agree with your suggestion, I will ost a v2 soon.

Thanks,
Maxime