On Thu, Apr 25, 2013 at 03:24:23PM +0800, Jason Wang wrote: > Multiqueue patchset conditionally add control vq only when guest negotiate the > feature. Though the spec is not clear on this but it breaks the minix guest > since it will identify the ctrl vq even if it does not support it. Though this > behavior seems a violation on the spec "If the VIRTIO_NET_F_CTRL_VQ feature > bit > is negotiated, identify the control virtqueue.", to keep the backward > compatibility, always add the ctrl vq at end of the queues. > > Reported-by: Aurelien Jarno <aurel...@aurel32.net> > Cc: Aurelien Jarno <aurel...@aurel32.net> > Signed-off-by: Jason Wang <jasow...@redhat.com> > > --- > Changes from V1: > - Drop the unrelated cleanups > - Drop the meaningless ctrl parameter of virtio_net_set_multiqueue() > --- > hw/net/virtio-net.c | 15 ++++++++------- > 1 files changed, 8 insertions(+), 7 deletions(-)
Thanks a lot for this patch. It fixes the issue observed on Minix. For the record it also fixes virtio-net with NetBSD 6.0.1, before it was triggering an assertion in QEMU. Now that the problem is clear, I will try to work on fixing the problem on the OS side when I find some time. Tested-by: Aurelien Jarno <aurel...@aurel32.net> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 4d2cdd2..1662f46 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -309,7 +309,7 @@ static void virtio_net_set_queues(VirtIONet *n) > } > } > > -static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int > ctrl); > +static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue); > > static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t > features) > { > @@ -364,8 +364,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, > uint32_t features) > VirtIONet *n = VIRTIO_NET(vdev); > int i; > > - virtio_net_set_multiqueue(n, !!(features & (1 << VIRTIO_NET_F_MQ)), > - !!(features & (1 << VIRTIO_NET_F_CTRL_VQ))); > + virtio_net_set_multiqueue(n, !!(features & (1 << VIRTIO_NET_F_MQ))); > > virtio_net_set_mrg_rx_bufs(n, !!(features & (1 << > VIRTIO_NET_F_MRG_RXBUF))); > > @@ -1038,7 +1037,7 @@ static void virtio_net_tx_bh(void *opaque) > } > } > > -static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl) > +static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue) > { > VirtIODevice *vdev = VIRTIO_DEVICE(n); > int i, max = multiqueue ? n->max_queues : 1; > @@ -1067,9 +1066,11 @@ static void virtio_net_set_multiqueue(VirtIONet *n, > int multiqueue, int ctrl) > n->vqs[i].n = n; > } > > - if (ctrl) { > - n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl); > - } > + /* Note: Minux Guests (version 3.2.1) use ctrl vq but don't ack > + * VIRTIO_NET_F_CTRL_VQ. Create ctrl vq unconditionally to avoid > + * breaking them. > + */ > + n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl); > > virtio_net_set_queues(n); > } > -- > 1.7.1 > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net