On Mon, Apr 15, 2013 at 01:29:01PM +0800, Jason Wang wrote: > On 04/13/2013 09:17 PM, Aurelien Jarno wrote: > > On Fri, Jan 25, 2013 at 06:35:41PM +0800, Jason Wang wrote: > >> This patch implements both userspace and vhost support for multiple queue > >> virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of > >> VirtIONetQueue to VirtIONet. > >> > >> Signed-off-by: Jason Wang <jasow...@redhat.com> > >> --- > >> hw/virtio-net.c | 317 > >> +++++++++++++++++++++++++++++++++++++++++++------------ > >> hw/virtio-net.h | 28 +++++- > >> 2 files changed, 275 insertions(+), 70 deletions(-) > > This patch breaks virtio-net in Minix, even with multiqueue disable. I > > don't know virtio enough to know if it is a Minix or a QEMU problem. > > However I have been able to identify the part of the commit causing the > > failure: > > Hi Aurelien: > > Thanks for the work. > > > >> diff --git a/hw/virtio-net.c b/hw/virtio-net.c > >> index ef522d5..cec91a7 100644 > >> --- a/hw/virtio-net.c > >> +++ b/hw/virtio-net.c > > ... > > > >> +static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int > >> ctrl) > >> +{ > >> + VirtIODevice *vdev = &n->vdev; > >> + int i, max = multiqueue ? n->max_queues : 1; > >> + > >> + n->multiqueue = multiqueue; > >> + > >> + for (i = 2; i <= n->max_queues * 2 + 1; i++) { > >> + virtio_del_queue(vdev, i); > >> + } > >> + > > The for loop above is something which is new, even with multiqueue > > disabled. Even with max_queues=1 it calls virtio_del_queue with i = 2 > > and i = 3. Disabling this loop makes the code to work as before. > > Looks like a bug here, need to change n->max_queues * 2 + 1 to > n->max_queues * 2. The reason we need to del queue 2 each time because > vq 2 has different meaning is multiqueue and single queue. In single > queue, vq 2 maybe ctrl vq, but in multiqueue mode it was rx1. > > Let's see whether this small change works.
Unfortunately it doesn't fix the issue. I don't know a lot about virtio, but would it be possible to only delete the queue that have been enabled? > > > > On the Minix side it triggers the following assertion: > > > > | virtio.c:370: assert "q->vaddr != NULL" failed, function > > "free_phys_queue" > > > > | virtio_net(73141): panic: assert failed > > > > This correspond to this function in lib/libvirtio/virtio.c: > > > > | static void > > | free_phys_queue(struct virtio_queue *q) > > | { > > | assert(q != NULL); > > | assert(q->vaddr != NULL); > > | > > | free_contig(q->vaddr, q->ring_size); > > | q->vaddr = NULL; > > | q->paddr = 0; > > | q->num = 0; > > | free_contig(q->data, sizeof(q->data[0])); > > | q->data = NULL; > > | } > > > > Do you have an idea if the problem is on the Minix side or on the QEMU > > side? > > > > Haven't figured out the relationship between virtqueue dynamic del/add > and q->vaddr here. If the above changes does not work, I guess this Unfortunately I don't really know the minix code either. I happen to found the issue when testing other changes, and looked at the code to try to understand the problem. > problem happens only for ctrl vq (vq2)? And when does this happen? I guess so given removing the call to virtio_del_queue() for vq2 fixes or workarounds the issue. > Rebooting? It happens at the initial boot. Thanks, Aurelien -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net