I’m happy with factoring out these helpers. My only concern is the one
around vq->{used, avail, desc} being nulled out in patch 11/33.On Wed, Aug 13, 2025 at 12:51 PM Vladimir Sementsov-Ogievskiy <[email protected]> wrote: > > Introduce helper functions vhost_vrings_map() and > vhost_vrings_unmap() and use them. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> > --- > 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 c76e2dbb4e..f6ee59425f 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -498,6 +498,53 @@ static void vhost_memory_unmap(struct vhost_dev *dev, > void *buffer, > } > } > > +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, > @@ -1282,30 +1329,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); > @@ -1367,9 +1393,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; > } > > @@ -1416,9 +1440,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 > >
