| Message ID | 1648621997-22416-2-git-send-email-si-wei.liu@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vhost-vdpa multiqueue fixes | expand |
On Wed, Mar 30, 2022 at 2:33 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote: > > With MQ enabled vdpa device and non-MQ supporting guest e.g. > booting vdpa with mq=on over OVMF of single vqp, below assert > failure is seen: > > ../hw/virtio/vhost-vdpa.c:560: vhost_vdpa_get_vq_index: Assertion `idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs' failed. > > 0 0x00007f8ce3ff3387 in raise () at /lib64/libc.so.6 > 1 0x00007f8ce3ff4a78 in abort () at /lib64/libc.so.6 > 2 0x00007f8ce3fec1a6 in __assert_fail_base () at /lib64/libc.so.6 > 3 0x00007f8ce3fec252 in () at /lib64/libc.so.6 > 4 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:563 > 5 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:558 > 6 0x0000558f52d7329a in vhost_virtqueue_mask (hdev=0x558f55c01800, vdev=0x558f568f91f0, n=2, mask=<optimized out>) at ../hw/virtio/vhost.c:1557 > 7 0x0000558f52c6b89a in virtio_pci_set_guest_notifier (d=d@entry=0x558f568f0f60, n=n@entry=2, assign=assign@entry=true, with_irqfd=with_irqfd@entry=false) > at ../hw/virtio/virtio-pci.c:974 > 8 0x0000558f52c6c0d8 in virtio_pci_set_guest_notifiers (d=0x558f568f0f60, nvqs=3, assign=true) at ../hw/virtio/virtio-pci.c:1019 > 9 0x0000558f52bf091d in vhost_net_start (dev=dev@entry=0x558f568f91f0, ncs=0x558f56937cd0, data_queue_pairs=data_queue_pairs@entry=1, cvq=cvq@entry=1) > at ../hw/net/vhost_net.c:361 > 10 0x0000558f52d4e5e7 in virtio_net_set_status (status=<optimized out>, n=0x558f568f91f0) at ../hw/net/virtio-net.c:289 > 11 0x0000558f52d4e5e7 in virtio_net_set_status (vdev=0x558f568f91f0, status=15 '\017') at ../hw/net/virtio-net.c:370 > 12 0x0000558f52d6c4b2 in virtio_set_status (vdev=vdev@entry=0x558f568f91f0, val=val@entry=15 '\017') at ../hw/virtio/virtio.c:1945 > 13 0x0000558f52c69eff in virtio_pci_common_write (opaque=0x558f568f0f60, addr=<optimized out>, val=<optimized out>, size=<optimized out>) at ../hw/virtio/virtio-pci.c:1292 > 14 0x0000558f52d15d6e in memory_region_write_accessor (mr=0x558f568f19d0, addr=20, value=<optimized out>, size=1, shift=<optimized out>, mask=<optimized out>, attrs=...) > at ../softmmu/memory.c:492 > 15 0x0000558f52d127de in access_with_adjusted_size (addr=addr@entry=20, value=value@entry=0x7f8cdbffe748, size=size@entry=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x558f52d15cf0 <memory_region_write_accessor>, mr=0x558f568f19d0, attrs=...) at ../softmmu/memory.c:554 > 16 0x0000558f52d157ef in memory_region_dispatch_write (mr=mr@entry=0x558f568f19d0, addr=20, data=<optimized out>, op=<optimized out>, attrs=attrs@entry=...) > at ../softmmu/memory.c:1504 > 17 0x0000558f52d078e7 in flatview_write_continue (fv=fv@entry=0x7f8accbc3b90, addr=addr@entry=103079215124, attrs=..., ptr=ptr@entry=0x7f8ce6300028, len=len@entry=1, addr1=<optimized out>, l=<optimized out>, mr=0x558f568f19d0) at /home/opc/qemu-upstream/include/qemu/host-utils.h:165 > 18 0x0000558f52d07b06 in flatview_write (fv=0x7f8accbc3b90, addr=103079215124, attrs=..., buf=0x7f8ce6300028, len=1) at ../softmmu/physmem.c:2822 > 19 0x0000558f52d0b36b in address_space_write (as=<optimized out>, addr=<optimized out>, attrs=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>) > at ../softmmu/physmem.c:2914 > 20 0x0000558f52d0b3da in address_space_rw (as=<optimized out>, addr=<optimized out>, attrs=..., > attrs@entry=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>, is_write=<optimized out>) at ../softmmu/physmem.c:2924 > 21 0x0000558f52dced09 in kvm_cpu_exec (cpu=cpu@entry=0x558f55c2da60) at ../accel/kvm/kvm-all.c:2903 > 22 0x0000558f52dcfabd in kvm_vcpu_thread_fn (arg=arg@entry=0x558f55c2da60) at ../accel/kvm/kvm-accel-ops.c:49 > 23 0x0000558f52f9f04a in qemu_thread_start (args=<optimized out>) at ../util/qemu-thread-posix.c:556 > 24 0x00007f8ce4392ea5 in start_thread () at /lib64/libpthread.so.0 > 25 0x00007f8ce40bb9fd in clone () at /lib64/libc.so.6 > > The cause for the assert failure is due to that the vhost_dev index > for the ctrl vq was not aligned with actual one in use by the guest. > Upon multiqueue feature negotiation in virtio_net_set_multiqueue(), > if guest doesn't support multiqueue, the guest vq layout would shrink > to a single queue pair, consisting of 3 vqs in total (rx, tx and ctrl). > This results in ctrl_vq taking a different vhost_dev group index than > the default. We can map vq to the correct vhost_dev group by checking > if MQ is supported by guest and successfully negotiated. Since the > MQ feature is only present along with CTRL_VQ, we make sure the index > 2 is only meant for the control vq while MQ is not supported by guest. > > Be noted if QEMU or guest doesn't support control vq, there's no bother > exposing vhost_dev and guest notifier for the control vq. Since > vhost_net_start/stop implies DRIVER_OK is set in device status, feature > negotiation should be completed when reaching virtio_net_vhost_status(). > > Fixes: 22288fe ("virtio-net: vhost control virtqueue support") > Suggested-by: Jason Wang <jasowang@redhat.com> > Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com> > --- > hw/net/virtio-net.c | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 1067e72..484b215 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -245,7 +245,8 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) > VirtIODevice *vdev = VIRTIO_DEVICE(n); > NetClientState *nc = qemu_get_queue(n->nic); > int queue_pairs = n->multiqueue ? n->max_queue_pairs : 1; > - int cvq = n->max_ncs - n->max_queue_pairs; > + int cvq = virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ) ? > + n->max_ncs - n->max_queue_pairs : 0; Let's use a separate patch for this. > > if (!get_vhost_net(nc->peer)) { > return; > @@ -3170,8 +3171,14 @@ static NetClientInfo net_virtio_info = { > static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx) > { > VirtIONet *n = VIRTIO_NET(vdev); > - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); > + NetClientState *nc; > assert(n->vhost_started); > + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { > + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); This assert seems guest trigger-able. If yes, I would remove this or replace it with log_guest_error. > + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); > + } else { > + nc = qemu_get_subqueue(n->nic, vq2q(idx)); > + } > return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx); > } > > @@ -3179,8 +3186,14 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, > bool mask) > { > VirtIONet *n = VIRTIO_NET(vdev); > - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); > + NetClientState *nc; > assert(n->vhost_started); > + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { > + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); And this. Thanks > + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); > + } else { > + nc = qemu_get_subqueue(n->nic, vq2q(idx)); > + } > vhost_net_virtqueue_mask(get_vhost_net(nc->peer), > vdev, idx, mask); > } > -- > 1.8.3.1 >
On 3/30/2022 2:00 AM, Jason Wang wrote: > On Wed, Mar 30, 2022 at 2:33 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote: >> With MQ enabled vdpa device and non-MQ supporting guest e.g. >> booting vdpa with mq=on over OVMF of single vqp, below assert >> failure is seen: >> >> ../hw/virtio/vhost-vdpa.c:560: vhost_vdpa_get_vq_index: Assertion `idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs' failed. >> >> 0 0x00007f8ce3ff3387 in raise () at /lib64/libc.so.6 >> 1 0x00007f8ce3ff4a78 in abort () at /lib64/libc.so.6 >> 2 0x00007f8ce3fec1a6 in __assert_fail_base () at /lib64/libc.so.6 >> 3 0x00007f8ce3fec252 in () at /lib64/libc.so.6 >> 4 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:563 >> 5 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:558 >> 6 0x0000558f52d7329a in vhost_virtqueue_mask (hdev=0x558f55c01800, vdev=0x558f568f91f0, n=2, mask=<optimized out>) at ../hw/virtio/vhost.c:1557 >> 7 0x0000558f52c6b89a in virtio_pci_set_guest_notifier (d=d@entry=0x558f568f0f60, n=n@entry=2, assign=assign@entry=true, with_irqfd=with_irqfd@entry=false) >> at ../hw/virtio/virtio-pci.c:974 >> 8 0x0000558f52c6c0d8 in virtio_pci_set_guest_notifiers (d=0x558f568f0f60, nvqs=3, assign=true) at ../hw/virtio/virtio-pci.c:1019 >> 9 0x0000558f52bf091d in vhost_net_start (dev=dev@entry=0x558f568f91f0, ncs=0x558f56937cd0, data_queue_pairs=data_queue_pairs@entry=1, cvq=cvq@entry=1) >> at ../hw/net/vhost_net.c:361 >> 10 0x0000558f52d4e5e7 in virtio_net_set_status (status=<optimized out>, n=0x558f568f91f0) at ../hw/net/virtio-net.c:289 >> 11 0x0000558f52d4e5e7 in virtio_net_set_status (vdev=0x558f568f91f0, status=15 '\017') at ../hw/net/virtio-net.c:370 >> 12 0x0000558f52d6c4b2 in virtio_set_status (vdev=vdev@entry=0x558f568f91f0, val=val@entry=15 '\017') at ../hw/virtio/virtio.c:1945 >> 13 0x0000558f52c69eff in virtio_pci_common_write (opaque=0x558f568f0f60, addr=<optimized out>, val=<optimized out>, size=<optimized out>) at ../hw/virtio/virtio-pci.c:1292 >> 14 0x0000558f52d15d6e in memory_region_write_accessor (mr=0x558f568f19d0, addr=20, value=<optimized out>, size=1, shift=<optimized out>, mask=<optimized out>, attrs=...) >> at ../softmmu/memory.c:492 >> 15 0x0000558f52d127de in access_with_adjusted_size (addr=addr@entry=20, value=value@entry=0x7f8cdbffe748, size=size@entry=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x558f52d15cf0 <memory_region_write_accessor>, mr=0x558f568f19d0, attrs=...) at ../softmmu/memory.c:554 >> 16 0x0000558f52d157ef in memory_region_dispatch_write (mr=mr@entry=0x558f568f19d0, addr=20, data=<optimized out>, op=<optimized out>, attrs=attrs@entry=...) >> at ../softmmu/memory.c:1504 >> 17 0x0000558f52d078e7 in flatview_write_continue (fv=fv@entry=0x7f8accbc3b90, addr=addr@entry=103079215124, attrs=..., ptr=ptr@entry=0x7f8ce6300028, len=len@entry=1, addr1=<optimized out>, l=<optimized out>, mr=0x558f568f19d0) at /home/opc/qemu-upstream/include/qemu/host-utils.h:165 >> 18 0x0000558f52d07b06 in flatview_write (fv=0x7f8accbc3b90, addr=103079215124, attrs=..., buf=0x7f8ce6300028, len=1) at ../softmmu/physmem.c:2822 >> 19 0x0000558f52d0b36b in address_space_write (as=<optimized out>, addr=<optimized out>, attrs=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>) >> at ../softmmu/physmem.c:2914 >> 20 0x0000558f52d0b3da in address_space_rw (as=<optimized out>, addr=<optimized out>, attrs=..., >> attrs@entry=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>, is_write=<optimized out>) at ../softmmu/physmem.c:2924 >> 21 0x0000558f52dced09 in kvm_cpu_exec (cpu=cpu@entry=0x558f55c2da60) at ../accel/kvm/kvm-all.c:2903 >> 22 0x0000558f52dcfabd in kvm_vcpu_thread_fn (arg=arg@entry=0x558f55c2da60) at ../accel/kvm/kvm-accel-ops.c:49 >> 23 0x0000558f52f9f04a in qemu_thread_start (args=<optimized out>) at ../util/qemu-thread-posix.c:556 >> 24 0x00007f8ce4392ea5 in start_thread () at /lib64/libpthread.so.0 >> 25 0x00007f8ce40bb9fd in clone () at /lib64/libc.so.6 >> >> The cause for the assert failure is due to that the vhost_dev index >> for the ctrl vq was not aligned with actual one in use by the guest. >> Upon multiqueue feature negotiation in virtio_net_set_multiqueue(), >> if guest doesn't support multiqueue, the guest vq layout would shrink >> to a single queue pair, consisting of 3 vqs in total (rx, tx and ctrl). >> This results in ctrl_vq taking a different vhost_dev group index than >> the default. We can map vq to the correct vhost_dev group by checking >> if MQ is supported by guest and successfully negotiated. Since the >> MQ feature is only present along with CTRL_VQ, we make sure the index >> 2 is only meant for the control vq while MQ is not supported by guest. >> >> Be noted if QEMU or guest doesn't support control vq, there's no bother >> exposing vhost_dev and guest notifier for the control vq. Since >> vhost_net_start/stop implies DRIVER_OK is set in device status, feature >> negotiation should be completed when reaching virtio_net_vhost_status(). >> >> Fixes: 22288fe ("virtio-net: vhost control virtqueue support") >> Suggested-by: Jason Wang <jasowang@redhat.com> >> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com> >> --- >> hw/net/virtio-net.c | 19 ++++++++++++++++--- >> 1 file changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index 1067e72..484b215 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -245,7 +245,8 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) >> VirtIODevice *vdev = VIRTIO_DEVICE(n); >> NetClientState *nc = qemu_get_queue(n->nic); >> int queue_pairs = n->multiqueue ? n->max_queue_pairs : 1; >> - int cvq = n->max_ncs - n->max_queue_pairs; >> + int cvq = virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ) ? >> + n->max_ncs - n->max_queue_pairs : 0; > Let's use a separate patch for this. Yes, I can do that. Then the new patch will become a requisite for this patch. > >> if (!get_vhost_net(nc->peer)) { >> return; >> @@ -3170,8 +3171,14 @@ static NetClientInfo net_virtio_info = { >> static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx) >> { >> VirtIONet *n = VIRTIO_NET(vdev); >> - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); >> + NetClientState *nc; >> assert(n->vhost_started); >> + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { >> + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); > This assert seems guest trigger-able. If yes, I would remove this or > replace it with log_guest_error. This assert actually is relevant to the cvq change in virtio_net_vhost_status(). Since the same check on VIRTIO_NET_F_CTRL_VQ has been done earlier, it is assured that CTRL_VQ is negotiated when getting here. Noted the vhost_started is asserted in the same function, which in turn implies DRIVER_OK is set meaning feature negotiation is complete. I can't easily think of a scenario which guest may inadvertently or purposely trigger the assert? -Siwei > >> + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); >> + } else { >> + nc = qemu_get_subqueue(n->nic, vq2q(idx)); >> + } >> return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx); >> } >> >> @@ -3179,8 +3186,14 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, >> bool mask) >> { >> VirtIONet *n = VIRTIO_NET(vdev); >> - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); >> + NetClientState *nc; >> assert(n->vhost_started); >> + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { >> + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); > And this. > > Thanks > > >> + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); >> + } else { >> + nc = qemu_get_subqueue(n->nic, vq2q(idx)); >> + } >> vhost_net_virtqueue_mask(get_vhost_net(nc->peer), >> vdev, idx, mask); >> } >> -- >> 1.8.3.1 >>
On Wed, Mar 30, 2022 at 11:48 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote: > > > > On 3/30/2022 2:00 AM, Jason Wang wrote: > > On Wed, Mar 30, 2022 at 2:33 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote: > >> With MQ enabled vdpa device and non-MQ supporting guest e.g. > >> booting vdpa with mq=on over OVMF of single vqp, below assert > >> failure is seen: > >> > >> ../hw/virtio/vhost-vdpa.c:560: vhost_vdpa_get_vq_index: Assertion `idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs' failed. > >> > >> 0 0x00007f8ce3ff3387 in raise () at /lib64/libc.so.6 > >> 1 0x00007f8ce3ff4a78 in abort () at /lib64/libc.so.6 > >> 2 0x00007f8ce3fec1a6 in __assert_fail_base () at /lib64/libc.so.6 > >> 3 0x00007f8ce3fec252 in () at /lib64/libc.so.6 > >> 4 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:563 > >> 5 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:558 > >> 6 0x0000558f52d7329a in vhost_virtqueue_mask (hdev=0x558f55c01800, vdev=0x558f568f91f0, n=2, mask=<optimized out>) at ../hw/virtio/vhost.c:1557 > >> 7 0x0000558f52c6b89a in virtio_pci_set_guest_notifier (d=d@entry=0x558f568f0f60, n=n@entry=2, assign=assign@entry=true, with_irqfd=with_irqfd@entry=false) > >> at ../hw/virtio/virtio-pci.c:974 > >> 8 0x0000558f52c6c0d8 in virtio_pci_set_guest_notifiers (d=0x558f568f0f60, nvqs=3, assign=true) at ../hw/virtio/virtio-pci.c:1019 > >> 9 0x0000558f52bf091d in vhost_net_start (dev=dev@entry=0x558f568f91f0, ncs=0x558f56937cd0, data_queue_pairs=data_queue_pairs@entry=1, cvq=cvq@entry=1) > >> at ../hw/net/vhost_net.c:361 > >> 10 0x0000558f52d4e5e7 in virtio_net_set_status (status=<optimized out>, n=0x558f568f91f0) at ../hw/net/virtio-net.c:289 > >> 11 0x0000558f52d4e5e7 in virtio_net_set_status (vdev=0x558f568f91f0, status=15 '\017') at ../hw/net/virtio-net.c:370 > >> 12 0x0000558f52d6c4b2 in virtio_set_status (vdev=vdev@entry=0x558f568f91f0, val=val@entry=15 '\017') at ../hw/virtio/virtio.c:1945 > >> 13 0x0000558f52c69eff in virtio_pci_common_write (opaque=0x558f568f0f60, addr=<optimized out>, val=<optimized out>, size=<optimized out>) at ../hw/virtio/virtio-pci.c:1292 > >> 14 0x0000558f52d15d6e in memory_region_write_accessor (mr=0x558f568f19d0, addr=20, value=<optimized out>, size=1, shift=<optimized out>, mask=<optimized out>, attrs=...) > >> at ../softmmu/memory.c:492 > >> 15 0x0000558f52d127de in access_with_adjusted_size (addr=addr@entry=20, value=value@entry=0x7f8cdbffe748, size=size@entry=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x558f52d15cf0 <memory_region_write_accessor>, mr=0x558f568f19d0, attrs=...) at ../softmmu/memory.c:554 > >> 16 0x0000558f52d157ef in memory_region_dispatch_write (mr=mr@entry=0x558f568f19d0, addr=20, data=<optimized out>, op=<optimized out>, attrs=attrs@entry=...) > >> at ../softmmu/memory.c:1504 > >> 17 0x0000558f52d078e7 in flatview_write_continue (fv=fv@entry=0x7f8accbc3b90, addr=addr@entry=103079215124, attrs=..., ptr=ptr@entry=0x7f8ce6300028, len=len@entry=1, addr1=<optimized out>, l=<optimized out>, mr=0x558f568f19d0) at /home/opc/qemu-upstream/include/qemu/host-utils.h:165 > >> 18 0x0000558f52d07b06 in flatview_write (fv=0x7f8accbc3b90, addr=103079215124, attrs=..., buf=0x7f8ce6300028, len=1) at ../softmmu/physmem.c:2822 > >> 19 0x0000558f52d0b36b in address_space_write (as=<optimized out>, addr=<optimized out>, attrs=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>) > >> at ../softmmu/physmem.c:2914 > >> 20 0x0000558f52d0b3da in address_space_rw (as=<optimized out>, addr=<optimized out>, attrs=..., > >> attrs@entry=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>, is_write=<optimized out>) at ../softmmu/physmem.c:2924 > >> 21 0x0000558f52dced09 in kvm_cpu_exec (cpu=cpu@entry=0x558f55c2da60) at ../accel/kvm/kvm-all.c:2903 > >> 22 0x0000558f52dcfabd in kvm_vcpu_thread_fn (arg=arg@entry=0x558f55c2da60) at ../accel/kvm/kvm-accel-ops.c:49 > >> 23 0x0000558f52f9f04a in qemu_thread_start (args=<optimized out>) at ../util/qemu-thread-posix.c:556 > >> 24 0x00007f8ce4392ea5 in start_thread () at /lib64/libpthread.so.0 > >> 25 0x00007f8ce40bb9fd in clone () at /lib64/libc.so.6 > >> > >> The cause for the assert failure is due to that the vhost_dev index > >> for the ctrl vq was not aligned with actual one in use by the guest. > >> Upon multiqueue feature negotiation in virtio_net_set_multiqueue(), > >> if guest doesn't support multiqueue, the guest vq layout would shrink > >> to a single queue pair, consisting of 3 vqs in total (rx, tx and ctrl). > >> This results in ctrl_vq taking a different vhost_dev group index than > >> the default. We can map vq to the correct vhost_dev group by checking > >> if MQ is supported by guest and successfully negotiated. Since the > >> MQ feature is only present along with CTRL_VQ, we make sure the index > >> 2 is only meant for the control vq while MQ is not supported by guest. > >> > >> Be noted if QEMU or guest doesn't support control vq, there's no bother > >> exposing vhost_dev and guest notifier for the control vq. Since > >> vhost_net_start/stop implies DRIVER_OK is set in device status, feature > >> negotiation should be completed when reaching virtio_net_vhost_status(). > >> > >> Fixes: 22288fe ("virtio-net: vhost control virtqueue support") > >> Suggested-by: Jason Wang <jasowang@redhat.com> > >> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com> > >> --- > >> hw/net/virtio-net.c | 19 ++++++++++++++++--- > >> 1 file changed, 16 insertions(+), 3 deletions(-) > >> > >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > >> index 1067e72..484b215 100644 > >> --- a/hw/net/virtio-net.c > >> +++ b/hw/net/virtio-net.c > >> @@ -245,7 +245,8 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) > >> VirtIODevice *vdev = VIRTIO_DEVICE(n); > >> NetClientState *nc = qemu_get_queue(n->nic); > >> int queue_pairs = n->multiqueue ? n->max_queue_pairs : 1; > >> - int cvq = n->max_ncs - n->max_queue_pairs; > >> + int cvq = virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ) ? > >> + n->max_ncs - n->max_queue_pairs : 0; > > Let's use a separate patch for this. > Yes, I can do that. Then the new patch will become a requisite for this > patch. > > > > >> if (!get_vhost_net(nc->peer)) { > >> return; > >> @@ -3170,8 +3171,14 @@ static NetClientInfo net_virtio_info = { > >> static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx) > >> { > >> VirtIONet *n = VIRTIO_NET(vdev); > >> - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); > >> + NetClientState *nc; > >> assert(n->vhost_started); > >> + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { > >> + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); > > This assert seems guest trigger-able. If yes, I would remove this or > > replace it with log_guest_error. > This assert actually is relevant to the cvq change in > virtio_net_vhost_status(). Since the same check on VIRTIO_NET_F_CTRL_VQ > has been done earlier, it is assured that CTRL_VQ is negotiated when > getting here. > Noted the vhost_started is asserted in the same function, which in turn > implies DRIVER_OK is set meaning feature negotiation is complete. I > can't easily think of a scenario which guest may inadvertently or > purposely trigger the assert? So the code can be triggered like e.g unmasking: virtio_pci_vq_vector_unmask() k->guest_notifier_pending() Thanks > > -Siwei > > > > >> + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); > >> + } else { > >> + nc = qemu_get_subqueue(n->nic, vq2q(idx)); > >> + } > >> return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx); > >> } > >> > >> @@ -3179,8 +3186,14 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, > >> bool mask) > >> { > >> VirtIONet *n = VIRTIO_NET(vdev); > >> - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); > >> + NetClientState *nc; > >> assert(n->vhost_started); > >> + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { > >> + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); > > And this. > > > > Thanks > > > > > >> + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); > >> + } else { > >> + nc = qemu_get_subqueue(n->nic, vq2q(idx)); > >> + } > >> vhost_net_virtqueue_mask(get_vhost_net(nc->peer), > >> vdev, idx, mask); > >> } > >> -- > >> 1.8.3.1 > >> >
On 3/31/2022 1:39 AM, Jason Wang wrote: > On Wed, Mar 30, 2022 at 11:48 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote: >> >> >> On 3/30/2022 2:00 AM, Jason Wang wrote: >>> On Wed, Mar 30, 2022 at 2:33 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote: >>>> With MQ enabled vdpa device and non-MQ supporting guest e.g. >>>> booting vdpa with mq=on over OVMF of single vqp, below assert >>>> failure is seen: >>>> >>>> ../hw/virtio/vhost-vdpa.c:560: vhost_vdpa_get_vq_index: Assertion `idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs' failed. >>>> >>>> 0 0x00007f8ce3ff3387 in raise () at /lib64/libc.so.6 >>>> 1 0x00007f8ce3ff4a78 in abort () at /lib64/libc.so.6 >>>> 2 0x00007f8ce3fec1a6 in __assert_fail_base () at /lib64/libc.so.6 >>>> 3 0x00007f8ce3fec252 in () at /lib64/libc.so.6 >>>> 4 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:563 >>>> 5 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:558 >>>> 6 0x0000558f52d7329a in vhost_virtqueue_mask (hdev=0x558f55c01800, vdev=0x558f568f91f0, n=2, mask=<optimized out>) at ../hw/virtio/vhost.c:1557 >>>> 7 0x0000558f52c6b89a in virtio_pci_set_guest_notifier (d=d@entry=0x558f568f0f60, n=n@entry=2, assign=assign@entry=true, with_irqfd=with_irqfd@entry=false) >>>> at ../hw/virtio/virtio-pci.c:974 >>>> 8 0x0000558f52c6c0d8 in virtio_pci_set_guest_notifiers (d=0x558f568f0f60, nvqs=3, assign=true) at ../hw/virtio/virtio-pci.c:1019 >>>> 9 0x0000558f52bf091d in vhost_net_start (dev=dev@entry=0x558f568f91f0, ncs=0x558f56937cd0, data_queue_pairs=data_queue_pairs@entry=1, cvq=cvq@entry=1) >>>> at ../hw/net/vhost_net.c:361 >>>> 10 0x0000558f52d4e5e7 in virtio_net_set_status (status=<optimized out>, n=0x558f568f91f0) at ../hw/net/virtio-net.c:289 >>>> 11 0x0000558f52d4e5e7 in virtio_net_set_status (vdev=0x558f568f91f0, status=15 '\017') at ../hw/net/virtio-net.c:370 >>>> 12 0x0000558f52d6c4b2 in virtio_set_status (vdev=vdev@entry=0x558f568f91f0, val=val@entry=15 '\017') at ../hw/virtio/virtio.c:1945 >>>> 13 0x0000558f52c69eff in virtio_pci_common_write (opaque=0x558f568f0f60, addr=<optimized out>, val=<optimized out>, size=<optimized out>) at ../hw/virtio/virtio-pci.c:1292 >>>> 14 0x0000558f52d15d6e in memory_region_write_accessor (mr=0x558f568f19d0, addr=20, value=<optimized out>, size=1, shift=<optimized out>, mask=<optimized out>, attrs=...) >>>> at ../softmmu/memory.c:492 >>>> 15 0x0000558f52d127de in access_with_adjusted_size (addr=addr@entry=20, value=value@entry=0x7f8cdbffe748, size=size@entry=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x558f52d15cf0 <memory_region_write_accessor>, mr=0x558f568f19d0, attrs=...) at ../softmmu/memory.c:554 >>>> 16 0x0000558f52d157ef in memory_region_dispatch_write (mr=mr@entry=0x558f568f19d0, addr=20, data=<optimized out>, op=<optimized out>, attrs=attrs@entry=...) >>>> at ../softmmu/memory.c:1504 >>>> 17 0x0000558f52d078e7 in flatview_write_continue (fv=fv@entry=0x7f8accbc3b90, addr=addr@entry=103079215124, attrs=..., ptr=ptr@entry=0x7f8ce6300028, len=len@entry=1, addr1=<optimized out>, l=<optimized out>, mr=0x558f568f19d0) at /home/opc/qemu-upstream/include/qemu/host-utils.h:165 >>>> 18 0x0000558f52d07b06 in flatview_write (fv=0x7f8accbc3b90, addr=103079215124, attrs=..., buf=0x7f8ce6300028, len=1) at ../softmmu/physmem.c:2822 >>>> 19 0x0000558f52d0b36b in address_space_write (as=<optimized out>, addr=<optimized out>, attrs=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>) >>>> at ../softmmu/physmem.c:2914 >>>> 20 0x0000558f52d0b3da in address_space_rw (as=<optimized out>, addr=<optimized out>, attrs=..., >>>> attrs@entry=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>, is_write=<optimized out>) at ../softmmu/physmem.c:2924 >>>> 21 0x0000558f52dced09 in kvm_cpu_exec (cpu=cpu@entry=0x558f55c2da60) at ../accel/kvm/kvm-all.c:2903 >>>> 22 0x0000558f52dcfabd in kvm_vcpu_thread_fn (arg=arg@entry=0x558f55c2da60) at ../accel/kvm/kvm-accel-ops.c:49 >>>> 23 0x0000558f52f9f04a in qemu_thread_start (args=<optimized out>) at ../util/qemu-thread-posix.c:556 >>>> 24 0x00007f8ce4392ea5 in start_thread () at /lib64/libpthread.so.0 >>>> 25 0x00007f8ce40bb9fd in clone () at /lib64/libc.so.6 >>>> >>>> The cause for the assert failure is due to that the vhost_dev index >>>> for the ctrl vq was not aligned with actual one in use by the guest. >>>> Upon multiqueue feature negotiation in virtio_net_set_multiqueue(), >>>> if guest doesn't support multiqueue, the guest vq layout would shrink >>>> to a single queue pair, consisting of 3 vqs in total (rx, tx and ctrl). >>>> This results in ctrl_vq taking a different vhost_dev group index than >>>> the default. We can map vq to the correct vhost_dev group by checking >>>> if MQ is supported by guest and successfully negotiated. Since the >>>> MQ feature is only present along with CTRL_VQ, we make sure the index >>>> 2 is only meant for the control vq while MQ is not supported by guest. >>>> >>>> Be noted if QEMU or guest doesn't support control vq, there's no bother >>>> exposing vhost_dev and guest notifier for the control vq. Since >>>> vhost_net_start/stop implies DRIVER_OK is set in device status, feature >>>> negotiation should be completed when reaching virtio_net_vhost_status(). >>>> >>>> Fixes: 22288fe ("virtio-net: vhost control virtqueue support") >>>> Suggested-by: Jason Wang <jasowang@redhat.com> >>>> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com> >>>> --- >>>> hw/net/virtio-net.c | 19 ++++++++++++++++--- >>>> 1 file changed, 16 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>>> index 1067e72..484b215 100644 >>>> --- a/hw/net/virtio-net.c >>>> +++ b/hw/net/virtio-net.c >>>> @@ -245,7 +245,8 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) >>>> VirtIODevice *vdev = VIRTIO_DEVICE(n); >>>> NetClientState *nc = qemu_get_queue(n->nic); >>>> int queue_pairs = n->multiqueue ? n->max_queue_pairs : 1; >>>> - int cvq = n->max_ncs - n->max_queue_pairs; >>>> + int cvq = virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ) ? >>>> + n->max_ncs - n->max_queue_pairs : 0; >>> Let's use a separate patch for this. >> Yes, I can do that. Then the new patch will become a requisite for this >> patch. >> >>>> if (!get_vhost_net(nc->peer)) { >>>> return; >>>> @@ -3170,8 +3171,14 @@ static NetClientInfo net_virtio_info = { >>>> static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx) >>>> { >>>> VirtIONet *n = VIRTIO_NET(vdev); >>>> - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); >>>> + NetClientState *nc; >>>> assert(n->vhost_started); >>>> + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { >>>> + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); >>> This assert seems guest trigger-able. If yes, I would remove this or >>> replace it with log_guest_error. >> This assert actually is relevant to the cvq change in >> virtio_net_vhost_status(). Since the same check on VIRTIO_NET_F_CTRL_VQ >> has been done earlier, it is assured that CTRL_VQ is negotiated when >> getting here. >> Noted the vhost_started is asserted in the same function, which in turn >> implies DRIVER_OK is set meaning feature negotiation is complete. I >> can't easily think of a scenario which guest may inadvertently or >> purposely trigger the assert? > So the code can be triggered like e.g unmasking: > > virtio_pci_vq_vector_unmask() > k->guest_notifier_pending() Hmmm, are you concerned more about idx being invalid, or VIRTIO_NET_F_CTRL_VQ getting cleared? virtio_pci_vector_unmask() has validation through virtio_queue_get_num() that ensures the vq index is valid. While it doesn't seem possible for VIRTIO_NET_F_CTRL_VQ to be cleared without device reset first, during which the pending event left over on guest notifier eventfd should have been completed within virtio_pci_set_guest_notifiers(false) before vhost_net_stop() returns. If I am not missing something here, I guess we're probably fine? -Siwei > > Thanks > > >> -Siwei >> >>>> + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); >>>> + } else { >>>> + nc = qemu_get_subqueue(n->nic, vq2q(idx)); >>>> + } >>>> return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx); >>>> } >>>> >>>> @@ -3179,8 +3186,14 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, >>>> bool mask) >>>> { >>>> VirtIONet *n = VIRTIO_NET(vdev); >>>> - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); >>>> + NetClientState *nc; >>>> assert(n->vhost_started); >>>> + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { >>>> + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); >>> And this. >>> >>> Thanks >>> >>> >>>> + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); >>>> + } else { >>>> + nc = qemu_get_subqueue(n->nic, vq2q(idx)); >>>> + } >>>> vhost_net_virtqueue_mask(get_vhost_net(nc->peer), >>>> vdev, idx, mask); >>>> } >>>> -- >>>> 1.8.3.1 >>>>
On Sat, Apr 2, 2022 at 6:32 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote: > > > > On 3/31/2022 1:39 AM, Jason Wang wrote: > > On Wed, Mar 30, 2022 at 11:48 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote: > >> > >> > >> On 3/30/2022 2:00 AM, Jason Wang wrote: > >>> On Wed, Mar 30, 2022 at 2:33 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote: > >>>> With MQ enabled vdpa device and non-MQ supporting guest e.g. > >>>> booting vdpa with mq=on over OVMF of single vqp, below assert > >>>> failure is seen: > >>>> > >>>> ../hw/virtio/vhost-vdpa.c:560: vhost_vdpa_get_vq_index: Assertion `idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs' failed. > >>>> > >>>> 0 0x00007f8ce3ff3387 in raise () at /lib64/libc.so.6 > >>>> 1 0x00007f8ce3ff4a78 in abort () at /lib64/libc.so.6 > >>>> 2 0x00007f8ce3fec1a6 in __assert_fail_base () at /lib64/libc.so.6 > >>>> 3 0x00007f8ce3fec252 in () at /lib64/libc.so.6 > >>>> 4 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:563 > >>>> 5 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:558 > >>>> 6 0x0000558f52d7329a in vhost_virtqueue_mask (hdev=0x558f55c01800, vdev=0x558f568f91f0, n=2, mask=<optimized out>) at ../hw/virtio/vhost.c:1557 > >>>> 7 0x0000558f52c6b89a in virtio_pci_set_guest_notifier (d=d@entry=0x558f568f0f60, n=n@entry=2, assign=assign@entry=true, with_irqfd=with_irqfd@entry=false) > >>>> at ../hw/virtio/virtio-pci.c:974 > >>>> 8 0x0000558f52c6c0d8 in virtio_pci_set_guest_notifiers (d=0x558f568f0f60, nvqs=3, assign=true) at ../hw/virtio/virtio-pci.c:1019 > >>>> 9 0x0000558f52bf091d in vhost_net_start (dev=dev@entry=0x558f568f91f0, ncs=0x558f56937cd0, data_queue_pairs=data_queue_pairs@entry=1, cvq=cvq@entry=1) > >>>> at ../hw/net/vhost_net.c:361 > >>>> 10 0x0000558f52d4e5e7 in virtio_net_set_status (status=<optimized out>, n=0x558f568f91f0) at ../hw/net/virtio-net.c:289 > >>>> 11 0x0000558f52d4e5e7 in virtio_net_set_status (vdev=0x558f568f91f0, status=15 '\017') at ../hw/net/virtio-net.c:370 > >>>> 12 0x0000558f52d6c4b2 in virtio_set_status (vdev=vdev@entry=0x558f568f91f0, val=val@entry=15 '\017') at ../hw/virtio/virtio.c:1945 > >>>> 13 0x0000558f52c69eff in virtio_pci_common_write (opaque=0x558f568f0f60, addr=<optimized out>, val=<optimized out>, size=<optimized out>) at ../hw/virtio/virtio-pci.c:1292 > >>>> 14 0x0000558f52d15d6e in memory_region_write_accessor (mr=0x558f568f19d0, addr=20, value=<optimized out>, size=1, shift=<optimized out>, mask=<optimized out>, attrs=...) > >>>> at ../softmmu/memory.c:492 > >>>> 15 0x0000558f52d127de in access_with_adjusted_size (addr=addr@entry=20, value=value@entry=0x7f8cdbffe748, size=size@entry=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x558f52d15cf0 <memory_region_write_accessor>, mr=0x558f568f19d0, attrs=...) at ../softmmu/memory.c:554 > >>>> 16 0x0000558f52d157ef in memory_region_dispatch_write (mr=mr@entry=0x558f568f19d0, addr=20, data=<optimized out>, op=<optimized out>, attrs=attrs@entry=...) > >>>> at ../softmmu/memory.c:1504 > >>>> 17 0x0000558f52d078e7 in flatview_write_continue (fv=fv@entry=0x7f8accbc3b90, addr=addr@entry=103079215124, attrs=..., ptr=ptr@entry=0x7f8ce6300028, len=len@entry=1, addr1=<optimized out>, l=<optimized out>, mr=0x558f568f19d0) at /home/opc/qemu-upstream/include/qemu/host-utils.h:165 > >>>> 18 0x0000558f52d07b06 in flatview_write (fv=0x7f8accbc3b90, addr=103079215124, attrs=..., buf=0x7f8ce6300028, len=1) at ../softmmu/physmem.c:2822 > >>>> 19 0x0000558f52d0b36b in address_space_write (as=<optimized out>, addr=<optimized out>, attrs=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>) > >>>> at ../softmmu/physmem.c:2914 > >>>> 20 0x0000558f52d0b3da in address_space_rw (as=<optimized out>, addr=<optimized out>, attrs=..., > >>>> attrs@entry=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>, is_write=<optimized out>) at ../softmmu/physmem.c:2924 > >>>> 21 0x0000558f52dced09 in kvm_cpu_exec (cpu=cpu@entry=0x558f55c2da60) at ../accel/kvm/kvm-all.c:2903 > >>>> 22 0x0000558f52dcfabd in kvm_vcpu_thread_fn (arg=arg@entry=0x558f55c2da60) at ../accel/kvm/kvm-accel-ops.c:49 > >>>> 23 0x0000558f52f9f04a in qemu_thread_start (args=<optimized out>) at ../util/qemu-thread-posix.c:556 > >>>> 24 0x00007f8ce4392ea5 in start_thread () at /lib64/libpthread.so.0 > >>>> 25 0x00007f8ce40bb9fd in clone () at /lib64/libc.so.6 > >>>> > >>>> The cause for the assert failure is due to that the vhost_dev index > >>>> for the ctrl vq was not aligned with actual one in use by the guest. > >>>> Upon multiqueue feature negotiation in virtio_net_set_multiqueue(), > >>>> if guest doesn't support multiqueue, the guest vq layout would shrink > >>>> to a single queue pair, consisting of 3 vqs in total (rx, tx and ctrl). > >>>> This results in ctrl_vq taking a different vhost_dev group index than > >>>> the default. We can map vq to the correct vhost_dev group by checking > >>>> if MQ is supported by guest and successfully negotiated. Since the > >>>> MQ feature is only present along with CTRL_VQ, we make sure the index > >>>> 2 is only meant for the control vq while MQ is not supported by guest. > >>>> > >>>> Be noted if QEMU or guest doesn't support control vq, there's no bother > >>>> exposing vhost_dev and guest notifier for the control vq. Since > >>>> vhost_net_start/stop implies DRIVER_OK is set in device status, feature > >>>> negotiation should be completed when reaching virtio_net_vhost_status(). > >>>> > >>>> Fixes: 22288fe ("virtio-net: vhost control virtqueue support") > >>>> Suggested-by: Jason Wang <jasowang@redhat.com> > >>>> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com> > >>>> --- > >>>> hw/net/virtio-net.c | 19 ++++++++++++++++--- > >>>> 1 file changed, 16 insertions(+), 3 deletions(-) > >>>> > >>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > >>>> index 1067e72..484b215 100644 > >>>> --- a/hw/net/virtio-net.c > >>>> +++ b/hw/net/virtio-net.c > >>>> @@ -245,7 +245,8 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) > >>>> VirtIODevice *vdev = VIRTIO_DEVICE(n); > >>>> NetClientState *nc = qemu_get_queue(n->nic); > >>>> int queue_pairs = n->multiqueue ? n->max_queue_pairs : 1; > >>>> - int cvq = n->max_ncs - n->max_queue_pairs; > >>>> + int cvq = virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ) ? > >>>> + n->max_ncs - n->max_queue_pairs : 0; > >>> Let's use a separate patch for this. > >> Yes, I can do that. Then the new patch will become a requisite for this > >> patch. > >> > >>>> if (!get_vhost_net(nc->peer)) { > >>>> return; > >>>> @@ -3170,8 +3171,14 @@ static NetClientInfo net_virtio_info = { > >>>> static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx) > >>>> { > >>>> VirtIONet *n = VIRTIO_NET(vdev); > >>>> - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); > >>>> + NetClientState *nc; > >>>> assert(n->vhost_started); > >>>> + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { > >>>> + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); > >>> This assert seems guest trigger-able. If yes, I would remove this or > >>> replace it with log_guest_error. > >> This assert actually is relevant to the cvq change in > >> virtio_net_vhost_status(). Since the same check on VIRTIO_NET_F_CTRL_VQ > >> has been done earlier, it is assured that CTRL_VQ is negotiated when > >> getting here. > >> Noted the vhost_started is asserted in the same function, which in turn > >> implies DRIVER_OK is set meaning feature negotiation is complete. I > >> can't easily think of a scenario which guest may inadvertently or > >> purposely trigger the assert? > > So the code can be triggered like e.g unmasking: > > > > virtio_pci_vq_vector_unmask() > > k->guest_notifier_pending() > Hmmm, are you concerned more about idx being invalid, or > VIRTIO_NET_F_CTRL_VQ getting cleared? Something like this, we can't let a buggy driver crash into Qemu. > > virtio_pci_vector_unmask() has validation through virtio_queue_get_num() > that ensures the vq index is valid. Actually not, it just check whether the vq size is set: int virtio_queue_get_num(VirtIODevice *vdev, int n) { return vdev->vq[n].vring.num; } > While it doesn't seem possible for > VIRTIO_NET_F_CTRL_VQ to be cleared without device reset first, Probably, since we had a check in virtio_set_features(): /* * The driver must not attempt to set features after feature negotiation * has finished. */ if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) { return -EINVAL; } But another interesting part is that the guest_feautres come from the migration stream as well: static const VMStateDescription vmstate_virtio_64bit_features = { .name = "virtio/64bit_features", .version_id = 1, .minimum_version_id = 1, .needed = &virtio_64bit_features_needed, .fields = (VMStateField[]) { VMSTATE_UINT64(guest_features, VirtIODevice), VMSTATE_END_OF_LIST() } }; We should also be ready to let the buggy migration flow to crash us. >during > which the pending event left over on guest notifier eventfd should have > been completed within virtio_pci_set_guest_notifiers(false) before > vhost_net_stop() returns. If I am not missing something here, I guess > we're probably fine? I'm not sure I got here, but the mask/unmask is not necessarily related to vhost stop. E.g it can happen if guest want to change IRQ affinity. Thanks > > -Siwei > > > > > Thanks > > > > > >> -Siwei > >> > >>>> + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); > >>>> + } else { > >>>> + nc = qemu_get_subqueue(n->nic, vq2q(idx)); > >>>> + } > >>>> return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx); > >>>> } > >>>> > >>>> @@ -3179,8 +3186,14 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, > >>>> bool mask) > >>>> { > >>>> VirtIONet *n = VIRTIO_NET(vdev); > >>>> - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); > >>>> + NetClientState *nc; > >>>> assert(n->vhost_started); > >>>> + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { > >>>> + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); > >>> And this. > >>> > >>> Thanks > >>> > >>> > >>>> + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); > >>>> + } else { > >>>> + nc = qemu_get_subqueue(n->nic, vq2q(idx)); > >>>> + } > >>>> vhost_net_virtqueue_mask(get_vhost_net(nc->peer), > >>>> vdev, idx, mask); > >>>> } > >>>> -- > >>>> 1.8.3.1 > >>>> >
On 4/1/2022 7:10 PM, Jason Wang wrote: > On Sat, Apr 2, 2022 at 6:32 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote: >> >> >> On 3/31/2022 1:39 AM, Jason Wang wrote: >>> On Wed, Mar 30, 2022 at 11:48 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote: >>>> >>>> On 3/30/2022 2:00 AM, Jason Wang wrote: >>>>> On Wed, Mar 30, 2022 at 2:33 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote: >>>>>> With MQ enabled vdpa device and non-MQ supporting guest e.g. >>>>>> booting vdpa with mq=on over OVMF of single vqp, below assert >>>>>> failure is seen: >>>>>> >>>>>> ../hw/virtio/vhost-vdpa.c:560: vhost_vdpa_get_vq_index: Assertion `idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs' failed. >>>>>> >>>>>> 0 0x00007f8ce3ff3387 in raise () at /lib64/libc.so.6 >>>>>> 1 0x00007f8ce3ff4a78 in abort () at /lib64/libc.so.6 >>>>>> 2 0x00007f8ce3fec1a6 in __assert_fail_base () at /lib64/libc.so.6 >>>>>> 3 0x00007f8ce3fec252 in () at /lib64/libc.so.6 >>>>>> 4 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:563 >>>>>> 5 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:558 >>>>>> 6 0x0000558f52d7329a in vhost_virtqueue_mask (hdev=0x558f55c01800, vdev=0x558f568f91f0, n=2, mask=<optimized out>) at ../hw/virtio/vhost.c:1557 >>>>>> 7 0x0000558f52c6b89a in virtio_pci_set_guest_notifier (d=d@entry=0x558f568f0f60, n=n@entry=2, assign=assign@entry=true, with_irqfd=with_irqfd@entry=false) >>>>>> at ../hw/virtio/virtio-pci.c:974 >>>>>> 8 0x0000558f52c6c0d8 in virtio_pci_set_guest_notifiers (d=0x558f568f0f60, nvqs=3, assign=true) at ../hw/virtio/virtio-pci.c:1019 >>>>>> 9 0x0000558f52bf091d in vhost_net_start (dev=dev@entry=0x558f568f91f0, ncs=0x558f56937cd0, data_queue_pairs=data_queue_pairs@entry=1, cvq=cvq@entry=1) >>>>>> at ../hw/net/vhost_net.c:361 >>>>>> 10 0x0000558f52d4e5e7 in virtio_net_set_status (status=<optimized out>, n=0x558f568f91f0) at ../hw/net/virtio-net.c:289 >>>>>> 11 0x0000558f52d4e5e7 in virtio_net_set_status (vdev=0x558f568f91f0, status=15 '\017') at ../hw/net/virtio-net.c:370 >>>>>> 12 0x0000558f52d6c4b2 in virtio_set_status (vdev=vdev@entry=0x558f568f91f0, val=val@entry=15 '\017') at ../hw/virtio/virtio.c:1945 >>>>>> 13 0x0000558f52c69eff in virtio_pci_common_write (opaque=0x558f568f0f60, addr=<optimized out>, val=<optimized out>, size=<optimized out>) at ../hw/virtio/virtio-pci.c:1292 >>>>>> 14 0x0000558f52d15d6e in memory_region_write_accessor (mr=0x558f568f19d0, addr=20, value=<optimized out>, size=1, shift=<optimized out>, mask=<optimized out>, attrs=...) >>>>>> at ../softmmu/memory.c:492 >>>>>> 15 0x0000558f52d127de in access_with_adjusted_size (addr=addr@entry=20, value=value@entry=0x7f8cdbffe748, size=size@entry=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x558f52d15cf0 <memory_region_write_accessor>, mr=0x558f568f19d0, attrs=...) at ../softmmu/memory.c:554 >>>>>> 16 0x0000558f52d157ef in memory_region_dispatch_write (mr=mr@entry=0x558f568f19d0, addr=20, data=<optimized out>, op=<optimized out>, attrs=attrs@entry=...) >>>>>> at ../softmmu/memory.c:1504 >>>>>> 17 0x0000558f52d078e7 in flatview_write_continue (fv=fv@entry=0x7f8accbc3b90, addr=addr@entry=103079215124, attrs=..., ptr=ptr@entry=0x7f8ce6300028, len=len@entry=1, addr1=<optimized out>, l=<optimized out>, mr=0x558f568f19d0) at /home/opc/qemu-upstream/include/qemu/host-utils.h:165 >>>>>> 18 0x0000558f52d07b06 in flatview_write (fv=0x7f8accbc3b90, addr=103079215124, attrs=..., buf=0x7f8ce6300028, len=1) at ../softmmu/physmem.c:2822 >>>>>> 19 0x0000558f52d0b36b in address_space_write (as=<optimized out>, addr=<optimized out>, attrs=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>) >>>>>> at ../softmmu/physmem.c:2914 >>>>>> 20 0x0000558f52d0b3da in address_space_rw (as=<optimized out>, addr=<optimized out>, attrs=..., >>>>>> attrs@entry=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>, is_write=<optimized out>) at ../softmmu/physmem.c:2924 >>>>>> 21 0x0000558f52dced09 in kvm_cpu_exec (cpu=cpu@entry=0x558f55c2da60) at ../accel/kvm/kvm-all.c:2903 >>>>>> 22 0x0000558f52dcfabd in kvm_vcpu_thread_fn (arg=arg@entry=0x558f55c2da60) at ../accel/kvm/kvm-accel-ops.c:49 >>>>>> 23 0x0000558f52f9f04a in qemu_thread_start (args=<optimized out>) at ../util/qemu-thread-posix.c:556 >>>>>> 24 0x00007f8ce4392ea5 in start_thread () at /lib64/libpthread.so.0 >>>>>> 25 0x00007f8ce40bb9fd in clone () at /lib64/libc.so.6 >>>>>> >>>>>> The cause for the assert failure is due to that the vhost_dev index >>>>>> for the ctrl vq was not aligned with actual one in use by the guest. >>>>>> Upon multiqueue feature negotiation in virtio_net_set_multiqueue(), >>>>>> if guest doesn't support multiqueue, the guest vq layout would shrink >>>>>> to a single queue pair, consisting of 3 vqs in total (rx, tx and ctrl). >>>>>> This results in ctrl_vq taking a different vhost_dev group index than >>>>>> the default. We can map vq to the correct vhost_dev group by checking >>>>>> if MQ is supported by guest and successfully negotiated. Since the >>>>>> MQ feature is only present along with CTRL_VQ, we make sure the index >>>>>> 2 is only meant for the control vq while MQ is not supported by guest. >>>>>> >>>>>> Be noted if QEMU or guest doesn't support control vq, there's no bother >>>>>> exposing vhost_dev and guest notifier for the control vq. Since >>>>>> vhost_net_start/stop implies DRIVER_OK is set in device status, feature >>>>>> negotiation should be completed when reaching virtio_net_vhost_status(). >>>>>> >>>>>> Fixes: 22288fe ("virtio-net: vhost control virtqueue support") >>>>>> Suggested-by: Jason Wang <jasowang@redhat.com> >>>>>> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com> >>>>>> --- >>>>>> hw/net/virtio-net.c | 19 ++++++++++++++++--- >>>>>> 1 file changed, 16 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>>>>> index 1067e72..484b215 100644 >>>>>> --- a/hw/net/virtio-net.c >>>>>> +++ b/hw/net/virtio-net.c >>>>>> @@ -245,7 +245,8 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) >>>>>> VirtIODevice *vdev = VIRTIO_DEVICE(n); >>>>>> NetClientState *nc = qemu_get_queue(n->nic); >>>>>> int queue_pairs = n->multiqueue ? n->max_queue_pairs : 1; >>>>>> - int cvq = n->max_ncs - n->max_queue_pairs; >>>>>> + int cvq = virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ) ? >>>>>> + n->max_ncs - n->max_queue_pairs : 0; >>>>> Let's use a separate patch for this. >>>> Yes, I can do that. Then the new patch will become a requisite for this >>>> patch. >>>> >>>>>> if (!get_vhost_net(nc->peer)) { >>>>>> return; >>>>>> @@ -3170,8 +3171,14 @@ static NetClientInfo net_virtio_info = { >>>>>> static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx) >>>>>> { >>>>>> VirtIONet *n = VIRTIO_NET(vdev); >>>>>> - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); >>>>>> + NetClientState *nc; >>>>>> assert(n->vhost_started); >>>>>> + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { >>>>>> + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); >>>>> This assert seems guest trigger-able. If yes, I would remove this or >>>>> replace it with log_guest_error. >>>> This assert actually is relevant to the cvq change in >>>> virtio_net_vhost_status(). Since the same check on VIRTIO_NET_F_CTRL_VQ >>>> has been done earlier, it is assured that CTRL_VQ is negotiated when >>>> getting here. >>>> Noted the vhost_started is asserted in the same function, which in turn >>>> implies DRIVER_OK is set meaning feature negotiation is complete. I >>>> can't easily think of a scenario which guest may inadvertently or >>>> purposely trigger the assert? >>> So the code can be triggered like e.g unmasking: >>> >>> virtio_pci_vq_vector_unmask() >>> k->guest_notifier_pending() >> Hmmm, are you concerned more about idx being invalid, or >> VIRTIO_NET_F_CTRL_VQ getting cleared? > Something like this, we can't let a buggy driver crash into Qemu. > >> virtio_pci_vector_unmask() has validation through virtio_queue_get_num() >> that ensures the vq index is valid. > Actually not, it just check whether the vq size is set: > > int virtio_queue_get_num(VirtIODevice *vdev, int n) > { > return vdev->vq[n].vring.num; > } > >> While it doesn't seem possible for >> VIRTIO_NET_F_CTRL_VQ to be cleared without device reset first, > Probably, since we had a check in virtio_set_features(): > > /* > * The driver must not attempt to set features after feature negotiation > * has finished. > */ > if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) { > return -EINVAL; > } > > But another interesting part is that the guest_feautres come from the > migration stream as well: > > static const VMStateDescription vmstate_virtio_64bit_features = { > .name = "virtio/64bit_features", > .version_id = 1, > .minimum_version_id = 1, > .needed = &virtio_64bit_features_needed, > .fields = (VMStateField[]) { > VMSTATE_UINT64(guest_features, VirtIODevice), > VMSTATE_END_OF_LIST() > } > }; > > We should also be ready to let the buggy migration flow to crash us. Fair enough. Given the possibility of introduction through migration stream I think now it's needed to converting assert to error. Thanks for pointing it out. Thanks, -Siwei > >> during >> which the pending event left over on guest notifier eventfd should have >> been completed within virtio_pci_set_guest_notifiers(false) before >> vhost_net_stop() returns. If I am not missing something here, I guess >> we're probably fine? > I'm not sure I got here, but the mask/unmask is not necessarily > related to vhost stop. E.g it can happen if guest want to change IRQ > affinity. > > Thanks > >> -Siwei >> >>> Thanks >>> >>> >>>> -Siwei >>>> >>>>>> + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); >>>>>> + } else { >>>>>> + nc = qemu_get_subqueue(n->nic, vq2q(idx)); >>>>>> + } >>>>>> return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx); >>>>>> } >>>>>> >>>>>> @@ -3179,8 +3186,14 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, >>>>>> bool mask) >>>>>> { >>>>>> VirtIONet *n = VIRTIO_NET(vdev); >>>>>> - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); >>>>>> + NetClientState *nc; >>>>>> assert(n->vhost_started); >>>>>> + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { >>>>>> + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); >>>>> And this. >>>>> >>>>> Thanks >>>>> >>>>> >>>>>> + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); >>>>>> + } else { >>>>>> + nc = qemu_get_subqueue(n->nic, vq2q(idx)); >>>>>> + } >>>>>> vhost_net_virtqueue_mask(get_vhost_net(nc->peer), >>>>>> vdev, idx, mask); >>>>>> } >>>>>> -- >>>>>> 1.8.3.1 >>>>>> >
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 1067e72..484b215 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -245,7 +245,8 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) VirtIODevice *vdev = VIRTIO_DEVICE(n); NetClientState *nc = qemu_get_queue(n->nic); int queue_pairs = n->multiqueue ? n->max_queue_pairs : 1; - int cvq = n->max_ncs - n->max_queue_pairs; + int cvq = virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ) ? + n->max_ncs - n->max_queue_pairs : 0; if (!get_vhost_net(nc->peer)) { return; @@ -3170,8 +3171,14 @@ static NetClientInfo net_virtio_info = { static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx) { VirtIONet *n = VIRTIO_NET(vdev); - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); + NetClientState *nc; assert(n->vhost_started); + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); + } else { + nc = qemu_get_subqueue(n->nic, vq2q(idx)); + } return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx); } @@ -3179,8 +3186,14 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask) { VirtIONet *n = VIRTIO_NET(vdev); - NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx)); + NetClientState *nc; assert(n->vhost_started); + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) { + assert(virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)); + nc = qemu_get_subqueue(n->nic, n->max_queue_pairs); + } else { + nc = qemu_get_subqueue(n->nic, vq2q(idx)); + } vhost_net_virtqueue_mask(get_vhost_net(nc->peer), vdev, idx, mask); }
With MQ enabled vdpa device and non-MQ supporting guest e.g. booting vdpa with mq=on over OVMF of single vqp, below assert failure is seen: ../hw/virtio/vhost-vdpa.c:560: vhost_vdpa_get_vq_index: Assertion `idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs' failed. 0 0x00007f8ce3ff3387 in raise () at /lib64/libc.so.6 1 0x00007f8ce3ff4a78 in abort () at /lib64/libc.so.6 2 0x00007f8ce3fec1a6 in __assert_fail_base () at /lib64/libc.so.6 3 0x00007f8ce3fec252 in () at /lib64/libc.so.6 4 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:563 5 0x0000558f52d79421 in vhost_vdpa_get_vq_index (dev=<optimized out>, idx=<optimized out>) at ../hw/virtio/vhost-vdpa.c:558 6 0x0000558f52d7329a in vhost_virtqueue_mask (hdev=0x558f55c01800, vdev=0x558f568f91f0, n=2, mask=<optimized out>) at ../hw/virtio/vhost.c:1557 7 0x0000558f52c6b89a in virtio_pci_set_guest_notifier (d=d@entry=0x558f568f0f60, n=n@entry=2, assign=assign@entry=true, with_irqfd=with_irqfd@entry=false) at ../hw/virtio/virtio-pci.c:974 8 0x0000558f52c6c0d8 in virtio_pci_set_guest_notifiers (d=0x558f568f0f60, nvqs=3, assign=true) at ../hw/virtio/virtio-pci.c:1019 9 0x0000558f52bf091d in vhost_net_start (dev=dev@entry=0x558f568f91f0, ncs=0x558f56937cd0, data_queue_pairs=data_queue_pairs@entry=1, cvq=cvq@entry=1) at ../hw/net/vhost_net.c:361 10 0x0000558f52d4e5e7 in virtio_net_set_status (status=<optimized out>, n=0x558f568f91f0) at ../hw/net/virtio-net.c:289 11 0x0000558f52d4e5e7 in virtio_net_set_status (vdev=0x558f568f91f0, status=15 '\017') at ../hw/net/virtio-net.c:370 12 0x0000558f52d6c4b2 in virtio_set_status (vdev=vdev@entry=0x558f568f91f0, val=val@entry=15 '\017') at ../hw/virtio/virtio.c:1945 13 0x0000558f52c69eff in virtio_pci_common_write (opaque=0x558f568f0f60, addr=<optimized out>, val=<optimized out>, size=<optimized out>) at ../hw/virtio/virtio-pci.c:1292 14 0x0000558f52d15d6e in memory_region_write_accessor (mr=0x558f568f19d0, addr=20, value=<optimized out>, size=1, shift=<optimized out>, mask=<optimized out>, attrs=...) at ../softmmu/memory.c:492 15 0x0000558f52d127de in access_with_adjusted_size (addr=addr@entry=20, value=value@entry=0x7f8cdbffe748, size=size@entry=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x558f52d15cf0 <memory_region_write_accessor>, mr=0x558f568f19d0, attrs=...) at ../softmmu/memory.c:554 16 0x0000558f52d157ef in memory_region_dispatch_write (mr=mr@entry=0x558f568f19d0, addr=20, data=<optimized out>, op=<optimized out>, attrs=attrs@entry=...) at ../softmmu/memory.c:1504 17 0x0000558f52d078e7 in flatview_write_continue (fv=fv@entry=0x7f8accbc3b90, addr=addr@entry=103079215124, attrs=..., ptr=ptr@entry=0x7f8ce6300028, len=len@entry=1, addr1=<optimized out>, l=<optimized out>, mr=0x558f568f19d0) at /home/opc/qemu-upstream/include/qemu/host-utils.h:165 18 0x0000558f52d07b06 in flatview_write (fv=0x7f8accbc3b90, addr=103079215124, attrs=..., buf=0x7f8ce6300028, len=1) at ../softmmu/physmem.c:2822 19 0x0000558f52d0b36b in address_space_write (as=<optimized out>, addr=<optimized out>, attrs=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>) at ../softmmu/physmem.c:2914 20 0x0000558f52d0b3da in address_space_rw (as=<optimized out>, addr=<optimized out>, attrs=..., attrs@entry=..., buf=buf@entry=0x7f8ce6300028, len=<optimized out>, is_write=<optimized out>) at ../softmmu/physmem.c:2924 21 0x0000558f52dced09 in kvm_cpu_exec (cpu=cpu@entry=0x558f55c2da60) at ../accel/kvm/kvm-all.c:2903 22 0x0000558f52dcfabd in kvm_vcpu_thread_fn (arg=arg@entry=0x558f55c2da60) at ../accel/kvm/kvm-accel-ops.c:49 23 0x0000558f52f9f04a in qemu_thread_start (args=<optimized out>) at ../util/qemu-thread-posix.c:556 24 0x00007f8ce4392ea5 in start_thread () at /lib64/libpthread.so.0 25 0x00007f8ce40bb9fd in clone () at /lib64/libc.so.6 The cause for the assert failure is due to that the vhost_dev index for the ctrl vq was not aligned with actual one in use by the guest. Upon multiqueue feature negotiation in virtio_net_set_multiqueue(), if guest doesn't support multiqueue, the guest vq layout would shrink to a single queue pair, consisting of 3 vqs in total (rx, tx and ctrl). This results in ctrl_vq taking a different vhost_dev group index than the default. We can map vq to the correct vhost_dev group by checking if MQ is supported by guest and successfully negotiated. Since the MQ feature is only present along with CTRL_VQ, we make sure the index 2 is only meant for the control vq while MQ is not supported by guest. Be noted if QEMU or guest doesn't support control vq, there's no bother exposing vhost_dev and guest notifier for the control vq. Since vhost_net_start/stop implies DRIVER_OK is set in device status, feature negotiation should be completed when reaching virtio_net_vhost_status(). Fixes: 22288fe ("virtio-net: vhost control virtqueue support") Suggested-by: Jason Wang <jasowang@redhat.com> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com> --- hw/net/virtio-net.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)