| Message ID | 1413114332-626-14-git-send-email-mst-v4@redhat.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On 10/13/14 at 10:50am, Michael S. Tsirkin wrote: > virtio spec requires drivers to set DRIVER_OK before using VQs. > This is set automatically after probe returns, virtio console violated this > rule by adding inbufs, which causes the VQ to be used directly within > probe. > > To fix, call virtio_device_ready before using VQs. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > drivers/char/virtio_console.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > index b585b47..6ebe8f6 100644 > --- a/drivers/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -1449,6 +1449,8 @@ static int add_port(struct ports_device *portdev, u32 id) > spin_lock_init(&port->outvq_lock); > init_waitqueue_head(&port->waitqueue); > > + virtio_device_ready(portdev->vdev); > + > /* Fill the in_vq with buffers so the host can send us data. */ > nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock); > if (!nr_added_bufs) { Seems like probe and add_port() now both set VIRTIO_CONFIG_S_DRIVER_OK 1.839658] kernel BUG at include/linux/virtio_config.h:125! [ 1.839995] invalid opcode: 0000 [#1] SMP [ 1.840169] Modules linked in: serio_raw virtio_balloon pcspkr virtio_net virtio_console soundcore parport_pc floppy pvpanic parport i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc qxl drm_kms_helper ttm drm virtio_blk i2c_core virtio_pci ata_generic virtio_ring virtio pata_acpi [ 1.840169] CPU: 2 PID: 266 Comm: kworker/2:2 Not tainted 3.17.0+ #1 [ 1.840169] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 [ 1.840169] Workqueue: events control_work_handler [virtio_console] [ 1.840169] task: ffff8800364bc840 ti: ffff880078490000 task.ti: ffff880078490000 [ 1.840169] RIP: 0010:[<ffffffffa01d92c6>] [<ffffffffa01d92c6>] virtio_device_ready.part.12+0x4/0x6 [virtio_console] [ 1.840169] RSP: 0018:ffff880078493c78 EFLAGS: 00010202 [ 1.840169] RAX: 0000000000000007 RBX: ffff880036406200 RCX: 0000000000006e39 [ 1.840169] RDX: 000000000000c0b2 RSI: 0000000000000000 RDI: 000000000001c0b2 [ 1.840169] RBP: ffff880078493c78 R08: ffffffff81d2c6f8 R09: 0000000000000000 [ 1.840169] R10: 0000000000000001 R11: ffff8800364bd000 R12: ffff880036618400 [ 1.840169] R13: 0000000000000001 R14: ffff8800368c6800 R15: ffff880036406220 [ 1.840169] FS: 0000000000000000(0000) GS:ffff88007fd00000(0000) knlGS:0000000000000000 [ 1.840169] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b [ 1.840169] CR2: 00007f1c31c90000 CR3: 0000000001c14000 CR4: 00000000000006e0 [ 1.840169] Stack: [ 1.840169] ffff880078493ce8 ffffffffa01d886a ffff880000000001 ffffffff810e20cd [ 1.840169] ffff880078493cb8 0000000000000282 0000000000000000 0000000087f90194 [ 1.840169] ffff880078493ce8 ffff88007ab1d4e0 ffff880036618498 ffff880036618410 [ 1.840169] Call Trace: [ 1.840169] [<ffffffffa01d886a>] add_port+0x40a/0x440 [virtio_console] [ 1.840169] [<ffffffff810e20cd>] ? trace_hardirqs_on+0xd/0x10 [ 1.840169] [<ffffffffa01d8c6a>] control_work_handler+0x3ca/0x420 [virtio_console] [ 1.840169] [<ffffffff810b0e7b>] ? process_one_work+0x15b/0x530 [ 1.840169] [<ffffffff810b0ef4>] process_one_work+0x1d4/0x530 [ 1.840169] [<ffffffff810b0e7b>] ? process_one_work+0x15b/0x530 [ 1.840169] [<ffffffff810b136b>] worker_thread+0x11b/0x490 [ 1.840169] [<ffffffff810b1250>] ? process_one_work+0x530/0x530 [ 1.840169] [<ffffffff810b67c3>] kthread+0xf3/0x110 [ 1.840169] [<ffffffff81788f00>] ? _raw_spin_unlock_irq+0x30/0x50 [ 1.840169] [<ffffffff810b66d0>] ? kthread_create_on_node+0x240/0x240 [ 1.840169] [<ffffffff81789a7c>] ret_from_fork+0x7c/0xb0 [ 1.840169] [<ffffffff810b66d0>] ? kthread_create_on_node+0x240/0x240 [ 1.840169] Code: ff 49 89 c4 4d 85 e4 0f 8f 25 ff ff ff eb ad 48 c7 c0 f4 ff ff ff e9 17 ff ff ff e8 f5 cd eb e0 90 55 48 89 e5 0f 0b 55 48 89 e5 <0f> 0b 55 48 89 e5 0f 0b 55 48 89 e5 e8 99 e2 ff ff 48 c7 c7 c0 [ 1.840169] RIP [<ffffffffa01d92c6>] virtio_device_ready.part.12+0x4/0x6 [virtio_console] [ 1.840169] RSP <ffff880078493c78> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 20, 2014 at 01:07:50PM +0100, Thomas Graf wrote: > On 10/13/14 at 10:50am, Michael S. Tsirkin wrote: > > virtio spec requires drivers to set DRIVER_OK before using VQs. > > This is set automatically after probe returns, virtio console violated this > > rule by adding inbufs, which causes the VQ to be used directly within > > probe. > > > > To fix, call virtio_device_ready before using VQs. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > drivers/char/virtio_console.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > > index b585b47..6ebe8f6 100644 > > --- a/drivers/char/virtio_console.c > > +++ b/drivers/char/virtio_console.c > > @@ -1449,6 +1449,8 @@ static int add_port(struct ports_device *portdev, u32 id) > > spin_lock_init(&port->outvq_lock); > > init_waitqueue_head(&port->waitqueue); > > > > + virtio_device_ready(portdev->vdev); > > + > > /* Fill the in_vq with buffers so the host can send us data. */ > > nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock); > > if (!nr_added_bufs) { I see Cornelia sent a patch already. I'd like to reproduce this though - could you send me the command line please? > Seems like probe and add_port() now both set VIRTIO_CONFIG_S_DRIVER_OK > > 1.839658] kernel BUG at include/linux/virtio_config.h:125! > [ 1.839995] invalid opcode: 0000 [#1] SMP > [ 1.840169] Modules linked in: serio_raw virtio_balloon pcspkr virtio_net virtio_console soundcore parport_pc floppy pvpanic parport i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc qxl drm_kms_helper ttm drm virtio_blk i2c_core virtio_pci ata_generic virtio_ring virtio pata_acpi > [ 1.840169] CPU: 2 PID: 266 Comm: kworker/2:2 Not tainted 3.17.0+ #1 > [ 1.840169] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 > [ 1.840169] Workqueue: events control_work_handler [virtio_console] > [ 1.840169] task: ffff8800364bc840 ti: ffff880078490000 task.ti: ffff880078490000 > [ 1.840169] RIP: 0010:[<ffffffffa01d92c6>] [<ffffffffa01d92c6>] virtio_device_ready.part.12+0x4/0x6 [virtio_console] > [ 1.840169] RSP: 0018:ffff880078493c78 EFLAGS: 00010202 > [ 1.840169] RAX: 0000000000000007 RBX: ffff880036406200 RCX: 0000000000006e39 > [ 1.840169] RDX: 000000000000c0b2 RSI: 0000000000000000 RDI: 000000000001c0b2 > [ 1.840169] RBP: ffff880078493c78 R08: ffffffff81d2c6f8 R09: 0000000000000000 > [ 1.840169] R10: 0000000000000001 R11: ffff8800364bd000 R12: ffff880036618400 > [ 1.840169] R13: 0000000000000001 R14: ffff8800368c6800 R15: ffff880036406220 > [ 1.840169] FS: 0000000000000000(0000) GS:ffff88007fd00000(0000) knlGS:0000000000000000 > [ 1.840169] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b > [ 1.840169] CR2: 00007f1c31c90000 CR3: 0000000001c14000 CR4: 00000000000006e0 > [ 1.840169] Stack: > [ 1.840169] ffff880078493ce8 ffffffffa01d886a ffff880000000001 ffffffff810e20cd > [ 1.840169] ffff880078493cb8 0000000000000282 0000000000000000 0000000087f90194 > [ 1.840169] ffff880078493ce8 ffff88007ab1d4e0 ffff880036618498 ffff880036618410 > [ 1.840169] Call Trace: > [ 1.840169] [<ffffffffa01d886a>] add_port+0x40a/0x440 [virtio_console] > [ 1.840169] [<ffffffff810e20cd>] ? trace_hardirqs_on+0xd/0x10 > [ 1.840169] [<ffffffffa01d8c6a>] control_work_handler+0x3ca/0x420 [virtio_console] > [ 1.840169] [<ffffffff810b0e7b>] ? process_one_work+0x15b/0x530 > [ 1.840169] [<ffffffff810b0ef4>] process_one_work+0x1d4/0x530 > [ 1.840169] [<ffffffff810b0e7b>] ? process_one_work+0x15b/0x530 > [ 1.840169] [<ffffffff810b136b>] worker_thread+0x11b/0x490 > [ 1.840169] [<ffffffff810b1250>] ? process_one_work+0x530/0x530 > [ 1.840169] [<ffffffff810b67c3>] kthread+0xf3/0x110 > [ 1.840169] [<ffffffff81788f00>] ? _raw_spin_unlock_irq+0x30/0x50 > [ 1.840169] [<ffffffff810b66d0>] ? kthread_create_on_node+0x240/0x240 > [ 1.840169] [<ffffffff81789a7c>] ret_from_fork+0x7c/0xb0 > [ 1.840169] [<ffffffff810b66d0>] ? kthread_create_on_node+0x240/0x240 > [ 1.840169] Code: ff 49 89 c4 4d 85 e4 0f 8f 25 ff ff ff eb ad 48 c7 c0 f4 ff ff ff e9 17 ff ff ff e8 f5 cd eb e0 90 55 48 89 e5 0f 0b 55 48 89 e5 <0f> 0b 55 48 89 e5 0f 0b 55 48 89 e5 e8 99 e2 ff ff 48 c7 c7 c0 > [ 1.840169] RIP [<ffffffffa01d92c6>] virtio_device_ready.part.12+0x4/0x6 [virtio_console] > [ 1.840169] RSP <ffff880078493c78> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/20/14 at 04:10pm, Michael S. Tsirkin wrote: > On Mon, Oct 20, 2014 at 01:07:50PM +0100, Thomas Graf wrote: > > On 10/13/14 at 10:50am, Michael S. Tsirkin wrote: > > > virtio spec requires drivers to set DRIVER_OK before using VQs. > > > This is set automatically after probe returns, virtio console violated this > > > rule by adding inbufs, which causes the VQ to be used directly within > > > probe. > > > > > > To fix, call virtio_device_ready before using VQs. > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > --- > > > drivers/char/virtio_console.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > > > index b585b47..6ebe8f6 100644 > > > --- a/drivers/char/virtio_console.c > > > +++ b/drivers/char/virtio_console.c > > > @@ -1449,6 +1449,8 @@ static int add_port(struct ports_device *portdev, u32 id) > > > spin_lock_init(&port->outvq_lock); > > > init_waitqueue_head(&port->waitqueue); > > > > > > + virtio_device_ready(portdev->vdev); > > > + > > > /* Fill the in_vq with buffers so the host can send us data. */ > > > nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock); > > > if (!nr_added_bufs) { > > I see Cornelia sent a patch already. > I'd like to reproduce this though - could you send me > the command line please? 1. Invoke qemu: /usr/bin/qemu-system-x86_64 -machine accel=kvm -name f20-2 -S -machine pc-i440fx-1.4,accel=kvm,usb=off -m 2048 -realtime mlock=off -smp 4,sockets=4,cores=1,threads=1 -uuid dd45ec4c-7c26-4b73-9b6b-81f2912c5235 -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/f20-2.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -drive file=/virt/f20n2-clone,if=none,id=drive-virtio-disk0,format=raw -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x6,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -netdev tap,fd=24,id=hostnet0,vhost=on,vhostfd=25 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:ef:72:4e,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0 -device usb-tablet,id=input0 -spice port=5900,addr=127.0.0.1,disable-ticketing,seamless-migration=on -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x2 -device intel-hda,id=sound0,bus=pci.0,addr=0x4 -device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x7 2. Attach console right away: virsh console f20-2 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 20, 2014 at 04:10:16PM +0300, Michael S. Tsirkin wrote: > On Mon, Oct 20, 2014 at 01:07:50PM +0100, Thomas Graf wrote: > > On 10/13/14 at 10:50am, Michael S. Tsirkin wrote: > > > virtio spec requires drivers to set DRIVER_OK before using VQs. > > > This is set automatically after probe returns, virtio console violated this > > > rule by adding inbufs, which causes the VQ to be used directly within > > > probe. > > > > > > To fix, call virtio_device_ready before using VQs. > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > --- > > > drivers/char/virtio_console.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > > > index b585b47..6ebe8f6 100644 > > > --- a/drivers/char/virtio_console.c > > > +++ b/drivers/char/virtio_console.c > > > @@ -1449,6 +1449,8 @@ static int add_port(struct ports_device *portdev, u32 id) > > > spin_lock_init(&port->outvq_lock); > > > init_waitqueue_head(&port->waitqueue); > > > > > > + virtio_device_ready(portdev->vdev); > > > + > > > /* Fill the in_vq with buffers so the host can send us data. */ > > > nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock); > > > if (!nr_added_bufs) { > > I see Cornelia sent a patch already. > I'd like to reproduce this though - could you send me > the command line please? Nevermind, the trick is to add a port it seems: -device virtio-serial -chardev socket,path=/tmp/c1,server,nowait,id=foo -device virtserialport,chardev=foo,name=org.fedoraproject.port.0 works fine without -device virtserialport.
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index b585b47..6ebe8f6 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1449,6 +1449,8 @@ static int add_port(struct ports_device *portdev, u32 id) spin_lock_init(&port->outvq_lock); init_waitqueue_head(&port->waitqueue); + virtio_device_ready(portdev->vdev); + /* Fill the in_vq with buffers so the host can send us data. */ nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock); if (!nr_added_bufs) {
virtio spec requires drivers to set DRIVER_OK before using VQs. This is set automatically after probe returns, virtio console violated this rule by adding inbufs, which causes the VQ to be used directly within probe. To fix, call virtio_device_ready before using VQs. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- drivers/char/virtio_console.c | 2 ++ 1 file changed, 2 insertions(+)