[PATCH v2 15/23] vhost: final refactoring of vhost vrings map/unmap

Vladimir Sementsov-Ogievskiy posted 23 patches 1 month ago
Maintainers: "Gonglei (Arei)" <arei.gonglei@huawei.com>, Zhenwei Pi <pizhenwei@bytedance.com>, "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Raphael Norwitz <raphael@enfabrica.net>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Jason Wang <jasowang@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <fam@euphon.net>, "Alex Bennée" <alex.bennee@linaro.org>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH v2 15/23] vhost: final refactoring of vhost vrings map/unmap
Posted by Vladimir Sementsov-Ogievskiy 1 month ago
Introduce helper functions vhost_vrings_map() and
vhost_vrings_unmap() and use them.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 hw/virtio/vhost.c | 82 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 30 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index cc07450dcb..ebdea096aa 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -488,6 +488,53 @@ static void vhost_memory_unmap(struct vhost_dev *dev, void **buffer,
     *buffer = NULL;
 }
 
+static void vhost_vrings_unmap(struct vhost_dev *dev,
+                               struct vhost_virtqueue *vq, bool touched)
+{
+    vhost_memory_unmap(dev, &vq->used, vq->used_size, touched,
+                       touched ? vq->used_size : 0);
+    vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0,
+                       touched ? vq->avail_size : 0);
+    vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0,
+                       touched ? vq->desc_size : 0);
+}
+
+static int vhost_vrings_map(struct vhost_dev *dev,
+                            struct VirtIODevice *vdev,
+                            struct vhost_virtqueue *vq,
+                            unsigned idx)
+{
+    vq->desc_phys = virtio_queue_get_desc_addr(vdev, idx);
+    if (vq->desc_phys == 0) {
+        /* Queue might not be ready for start */
+        return 0;
+    }
+    vq->desc_size = virtio_queue_get_desc_size(vdev, idx);
+    vq->desc = vhost_memory_map(dev, vq->desc_phys, vq->desc_size, false);
+    if (!vq->desc) {
+        return -ENOMEM;
+    }
+
+    vq->avail_size = virtio_queue_get_avail_size(vdev, idx);
+    vq->avail_phys = virtio_queue_get_avail_addr(vdev, idx);
+    vq->avail = vhost_memory_map(dev, vq->avail_phys, vq->avail_size, false);
+    if (!vq->avail) {
+        goto fail;
+    }
+
+    vq->used_size = virtio_queue_get_used_size(vdev, idx);
+    vq->used_phys = virtio_queue_get_used_addr(vdev, idx);
+    vq->used = vhost_memory_map(dev, vq->used_phys, vq->used_size, true);
+    if (!vq->used) {
+        goto fail;
+    }
+
+    return 1;
+fail:
+    vhost_vrings_unmap(dev, vq, false);
+    return -ENOMEM;
+}
+
 static int vhost_verify_ring_part_mapping(void *ring_hva,
                                           uint64_t ring_gpa,
                                           uint64_t ring_size,
@@ -1286,30 +1333,9 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
     };
     struct VirtQueue *vvq = virtio_get_queue(vdev, idx);
 
-    vq->desc_phys = virtio_queue_get_desc_addr(vdev, idx);
-    if (vq->desc_phys == 0) {
-        /* Queue might not be ready for start */
-        return 0;
-    }
-    vq->desc_size = virtio_queue_get_desc_size(vdev, idx);
-    vq->desc = vhost_memory_map(dev, vq->desc_phys, vq->desc_size, false);
-    if (!vq->desc) {
-        r = -ENOMEM;
-        goto fail;
-    }
-    vq->avail_size = virtio_queue_get_avail_size(vdev, idx);
-    vq->avail_phys = virtio_queue_get_avail_addr(vdev, idx);
-    vq->avail = vhost_memory_map(dev, vq->avail_phys, vq->avail_size, false);
-    if (!vq->avail) {
-        r = -ENOMEM;
-        goto fail;
-    }
-    vq->used_size = virtio_queue_get_used_size(vdev, idx);
-    vq->used_phys = virtio_queue_get_used_addr(vdev, idx);
-    vq->used = vhost_memory_map(dev, vq->used_phys, vq->used_size, true);
-    if (!vq->used) {
-        r = -ENOMEM;
-        goto fail;
+    r = vhost_vrings_map(dev, vdev, vq, idx);
+    if (r <= 0) {
+        return r;
     }
 
     vq->num = state.num = virtio_queue_get_num(vdev, idx);
@@ -1371,9 +1397,7 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
     return 0;
 
 fail:
-    vhost_memory_unmap(dev, &vq->used, vq->used_size, 0, 0);
-    vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0, 0);
-    vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0, 0);
+    vhost_vrings_unmap(dev, vq, false);
     return r;
 }
 
@@ -1420,9 +1444,7 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
                                                 vhost_vq_index);
     }
 
-    vhost_memory_unmap(dev, &vq->used, vq->used_size, 1, vq->used_size);
-    vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0, vq->avail_size);
-    vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0, vq->desc_size);
+    vhost_vrings_unmap(dev, vq, true);
     return r;
 }
 
-- 
2.48.1
Re: [PATCH v2 15/23] vhost: final refactoring of vhost vrings map/unmap
Posted by Raphael Norwitz 1 month ago
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>

On Sat, Oct 11, 2025 at 7:24 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> Introduce helper functions vhost_vrings_map() and
> vhost_vrings_unmap() and use them.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  hw/virtio/vhost.c | 82 ++++++++++++++++++++++++++++++-----------------
>  1 file changed, 52 insertions(+), 30 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index cc07450dcb..ebdea096aa 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -488,6 +488,53 @@ static void vhost_memory_unmap(struct vhost_dev *dev, void **buffer,
>      *buffer = NULL;
>  }
>
> +static void vhost_vrings_unmap(struct vhost_dev *dev,
> +                               struct vhost_virtqueue *vq, bool touched)
> +{
> +    vhost_memory_unmap(dev, &vq->used, vq->used_size, touched,
> +                       touched ? vq->used_size : 0);
> +    vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0,
> +                       touched ? vq->avail_size : 0);
> +    vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0,
> +                       touched ? vq->desc_size : 0);
> +}
> +
> +static int vhost_vrings_map(struct vhost_dev *dev,
> +                            struct VirtIODevice *vdev,
> +                            struct vhost_virtqueue *vq,
> +                            unsigned idx)
> +{
> +    vq->desc_phys = virtio_queue_get_desc_addr(vdev, idx);
> +    if (vq->desc_phys == 0) {
> +        /* Queue might not be ready for start */
> +        return 0;
> +    }
> +    vq->desc_size = virtio_queue_get_desc_size(vdev, idx);
> +    vq->desc = vhost_memory_map(dev, vq->desc_phys, vq->desc_size, false);
> +    if (!vq->desc) {
> +        return -ENOMEM;
> +    }
> +
> +    vq->avail_size = virtio_queue_get_avail_size(vdev, idx);
> +    vq->avail_phys = virtio_queue_get_avail_addr(vdev, idx);
> +    vq->avail = vhost_memory_map(dev, vq->avail_phys, vq->avail_size, false);
> +    if (!vq->avail) {
> +        goto fail;
> +    }
> +
> +    vq->used_size = virtio_queue_get_used_size(vdev, idx);
> +    vq->used_phys = virtio_queue_get_used_addr(vdev, idx);
> +    vq->used = vhost_memory_map(dev, vq->used_phys, vq->used_size, true);
> +    if (!vq->used) {
> +        goto fail;
> +    }
> +
> +    return 1;
> +fail:
> +    vhost_vrings_unmap(dev, vq, false);
> +    return -ENOMEM;
> +}
> +
>  static int vhost_verify_ring_part_mapping(void *ring_hva,
>                                            uint64_t ring_gpa,
>                                            uint64_t ring_size,
> @@ -1286,30 +1333,9 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
>      };
>      struct VirtQueue *vvq = virtio_get_queue(vdev, idx);
>
> -    vq->desc_phys = virtio_queue_get_desc_addr(vdev, idx);
> -    if (vq->desc_phys == 0) {
> -        /* Queue might not be ready for start */
> -        return 0;
> -    }
> -    vq->desc_size = virtio_queue_get_desc_size(vdev, idx);
> -    vq->desc = vhost_memory_map(dev, vq->desc_phys, vq->desc_size, false);
> -    if (!vq->desc) {
> -        r = -ENOMEM;
> -        goto fail;
> -    }
> -    vq->avail_size = virtio_queue_get_avail_size(vdev, idx);
> -    vq->avail_phys = virtio_queue_get_avail_addr(vdev, idx);
> -    vq->avail = vhost_memory_map(dev, vq->avail_phys, vq->avail_size, false);
> -    if (!vq->avail) {
> -        r = -ENOMEM;
> -        goto fail;
> -    }
> -    vq->used_size = virtio_queue_get_used_size(vdev, idx);
> -    vq->used_phys = virtio_queue_get_used_addr(vdev, idx);
> -    vq->used = vhost_memory_map(dev, vq->used_phys, vq->used_size, true);
> -    if (!vq->used) {
> -        r = -ENOMEM;
> -        goto fail;
> +    r = vhost_vrings_map(dev, vdev, vq, idx);
> +    if (r <= 0) {
> +        return r;
>      }
>
>      vq->num = state.num = virtio_queue_get_num(vdev, idx);
> @@ -1371,9 +1397,7 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
>      return 0;
>
>  fail:
> -    vhost_memory_unmap(dev, &vq->used, vq->used_size, 0, 0);
> -    vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0, 0);
> -    vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0, 0);
> +    vhost_vrings_unmap(dev, vq, false);
>      return r;
>  }
>
> @@ -1420,9 +1444,7 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
>                                                  vhost_vq_index);
>      }
>
> -    vhost_memory_unmap(dev, &vq->used, vq->used_size, 1, vq->used_size);
> -    vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0, vq->avail_size);
> -    vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0, vq->desc_size);
> +    vhost_vrings_unmap(dev, vq, true);
>      return r;
>  }
>
> --
> 2.48.1
>