diff mbox

[v2,2/3] net: vhost stop updates virtio queue state

Message ID 1478704922-3400-3-git-send-email-yuri.benditovich@daynix.com
State New
Headers show

Commit Message

Yuri Benditovich Nov. 9, 2016, 3:22 p.m. UTC
From: Yuri Benditovich <yuri.benditovich@daynix.com>

Make virtio queue suitable for push operation from qemu
after vhost was stopped.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 hw/virtio/vhost.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Paolo Bonzini Nov. 9, 2016, 5:07 p.m. UTC | #1
On 09/11/2016 16:22, yuri.benditovich@daynix.com wrote:
> From: Yuri Benditovich <yuri.benditovich@daynix.com>
> 
> Make virtio queue suitable for push operation from qemu
> after vhost was stopped.
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>  hw/virtio/vhost.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index bd051ab..2e990d0 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -963,6 +963,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
>          virtio_queue_set_last_avail_idx(vdev, idx, state.num);
>      }
>      virtio_queue_invalidate_signalled_used(vdev, idx);
> +    virtio_queue_update_used_idx(vdev, idx);

All three functions virtio_queue_set_last_avail_idx,
virtio_queue_invalidate_signalled_used, virtio_queue_update_used_idx are
only used here.

Plus, virtio_queue_set_last_avail_idx is always called in practice,
since the only failure modes for VHOST_GET_VRING_BASE are EFAULT and
out-of-range virtqueue number.  In both cases QEMU can ignore the other
two steps.

So perhaps we should have a single function virtio_queue_set_vring_base,
taking a vhost_vring_state struct?  It can do all three.

Paolo

>  
>      /* In the cross-endian case, we need to reset the vring endianness to
>       * native as legacy devices expect so by default.
>
Michael S. Tsirkin Nov. 9, 2016, 8:12 p.m. UTC | #2
On Wed, Nov 09, 2016 at 06:07:29PM +0100, Paolo Bonzini wrote:
> 
> 
> On 09/11/2016 16:22, yuri.benditovich@daynix.com wrote:
> > From: Yuri Benditovich <yuri.benditovich@daynix.com>
> > 
> > Make virtio queue suitable for push operation from qemu
> > after vhost was stopped.
> > 
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >  hw/virtio/vhost.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index bd051ab..2e990d0 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -963,6 +963,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
> >          virtio_queue_set_last_avail_idx(vdev, idx, state.num);
> >      }
> >      virtio_queue_invalidate_signalled_used(vdev, idx);
> > +    virtio_queue_update_used_idx(vdev, idx);
> 
> All three functions virtio_queue_set_last_avail_idx,
> virtio_queue_invalidate_signalled_used, virtio_queue_update_used_idx are
> only used here.
> 
> Plus, virtio_queue_set_last_avail_idx is always called in practice,
> since the only failure modes for VHOST_GET_VRING_BASE are EFAULT and
> out-of-range virtqueue number.  In both cases QEMU can ignore the other
> two steps.
> 
> So perhaps we should have a single function virtio_queue_set_vring_base,
> taking a vhost_vring_state struct?  It can do all three.
> 
> Paolo

I don't object but I think the bugfix is helpful in this QEMU
version, and this change is minimally intrusive.

> >  
> >      /* In the cross-endian case, we need to reset the vring endianness to
> >       * native as legacy devices expect so by default.
> >
diff mbox

Patch

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index bd051ab..2e990d0 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -963,6 +963,7 @@  static void vhost_virtqueue_stop(struct vhost_dev *dev,
         virtio_queue_set_last_avail_idx(vdev, idx, state.num);
     }
     virtio_queue_invalidate_signalled_used(vdev, idx);
+    virtio_queue_update_used_idx(vdev, idx);
 
     /* In the cross-endian case, we need to reset the vring endianness to
      * native as legacy devices expect so by default.