Re: [PATCH v4 13/25] virtio_console: enable VQs early

2014-10-20 Thread Cornelia Huck
On Mon, 20 Oct 2014 13:07:50 +0100
Thomas Graf tg...@suug.ch 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 m...@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

I think we need to set this in the probe function instead, otherwise we
fail for multiqueue (which also wants to use the control queue early).

Completely untested patch below; I can send this with proper s-o-b if
it helps.

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index bfa6400..cf7a561 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1449,8 +1449,6 @@ 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) {
@@ -2026,6 +2024,8 @@ static int virtcons_probe(struct virtio_device *vdev)
spin_lock_init(portdev-ports_lock);
INIT_LIST_HEAD(portdev-ports);
 
+   virtio_device_ready(portdev-vdev);
+
if (multiport) {
unsigned int nr_added_bufs;
 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v4 13/25] virtio_console: enable VQs early

2014-10-20 Thread Michael S. Tsirkin
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 m...@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:  [#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: 8800364bc840 ti: 88007849 task.ti: 
 88007849
 [1.840169] RIP: 0010:[a01d92c6]  [a01d92c6] 
 virtio_device_ready.part.12+0x4/0x6 [virtio_console]
 [1.840169] RSP: 0018:880078493c78  EFLAGS: 00010202
 [1.840169] RAX: 0007 RBX: 880036406200 RCX: 
 6e39
 [1.840169] RDX: c0b2 RSI:  RDI: 
 0001c0b2
 [1.840169] RBP: 880078493c78 R08: 81d2c6f8 R09: 
 
 [1.840169] R10: 0001 R11: 8800364bd000 R12: 
 880036618400
 [1.840169] R13: 0001 R14: 8800368c6800 R15: 
 880036406220
 [1.840169] FS:  () GS:88007fd0() 
 knlGS:
 [1.840169] CS:  0010 DS:  ES:  CR0: 8005003b
 [1.840169] CR2: 7f1c31c9 CR3: 01c14000 CR4: 
 06e0
 [1.840169] Stack:
 [1.840169]  880078493ce8 a01d886a 8801 
 810e20cd
 [1.840169]  880078493cb8 0282  
 87f90194
 [1.840169]  880078493ce8 88007ab1d4e0 880036618498 
 880036618410
 [1.840169] Call Trace:
 [1.840169]  [a01d886a] add_port+0x40a/0x440 [virtio_console]
 [1.840169]  [810e20cd] ? trace_hardirqs_on+0xd/0x10
 [1.840169]  [a01d8c6a] control_work_handler+0x3ca/0x420 
 [virtio_console]
 [1.840169]  [810b0e7b] ? process_one_work+0x15b/0x530
 [1.840169]  [810b0ef4] process_one_work+0x1d4/0x530
 [1.840169]  [810b0e7b] ? process_one_work+0x15b/0x530
 [1.840169]  [810b136b] worker_thread+0x11b/0x490
 [1.840169]  [810b1250] ? process_one_work+0x530/0x530
 [1.840169]  [810b67c3] kthread+0xf3/0x110
 [1.840169]  [81788f00] ? _raw_spin_unlock_irq+0x30/0x50
 [1.840169]  [810b66d0] ? kthread_create_on_node+0x240/0x240
 [1.840169]  [81789a7c] ret_from_fork+0x7c/0xb0
 [1.840169]  [810b66d0] ? 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  [a01d92c6] virtio_device_ready.part.12+0x4/0x6 
 [virtio_console]
 [1.840169]  RSP 880078493c78
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v4 13/25] virtio_console: enable VQs early

2014-10-20 Thread Michael S. Tsirkin
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 m...@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.

-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v4 13/25] virtio_console: enable VQs early

2014-10-20 Thread Michael S. Tsirkin
On Mon, Oct 20, 2014 at 02:42:23PM +0200, Cornelia Huck wrote:
 On Mon, 20 Oct 2014 13:07:50 +0100
 Thomas Graf tg...@suug.ch 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 m...@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
 
 I think we need to set this in the probe function instead, otherwise we
 fail for multiqueue (which also wants to use the control queue early).
 
 Completely untested patch below; I can send this with proper s-o-b if
 it helps.
 
 diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
 index bfa6400..cf7a561 100644
 --- a/drivers/char/virtio_console.c
 +++ b/drivers/char/virtio_console.c
 @@ -1449,8 +1449,6 @@ 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) {
 @@ -2026,6 +2024,8 @@ static int virtcons_probe(struct virtio_device *vdev)
   spin_lock_init(portdev-ports_lock);
   INIT_LIST_HEAD(portdev-ports);
  
 + virtio_device_ready(portdev-vdev);
 +
   if (multiport) {
   unsigned int nr_added_bufs;
  

I wanted to set DRIVER_OK as late as possible, to both reduce
the chance it can fail after DRIVER_OK and to reduce  the risk of
introducing a regression since old qemu might only start sending
interrupts after DRIVER_OK is set.

So I wanted everything completely initialized before DRIVER_OK.

You patch makes sense to me since nothing can fail,
and we won't get interrupts before we add inbufs.

Reviewed-by: Michael S. Tsirkin m...@redhat.com

Testig will report shortly, pls send with sob line.

-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v4 13/25] virtio_console: enable VQs early

2014-10-20 Thread Michael S. Tsirkin
On Mon, Oct 20, 2014 at 04:35:55PM +0300, Michael S. Tsirkin wrote:
 On Mon, Oct 20, 2014 at 02:42:23PM +0200, Cornelia Huck wrote:
  On Mon, 20 Oct 2014 13:07:50 +0100
  Thomas Graf tg...@suug.ch 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 m...@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
  
  I think we need to set this in the probe function instead, otherwise we
  fail for multiqueue (which also wants to use the control queue early).
  
  Completely untested patch below; I can send this with proper s-o-b if
  it helps.
  
  diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
  index bfa6400..cf7a561 100644
  --- a/drivers/char/virtio_console.c
  +++ b/drivers/char/virtio_console.c
  @@ -1449,8 +1449,6 @@ 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) {
  @@ -2026,6 +2024,8 @@ static int virtcons_probe(struct virtio_device *vdev)
  spin_lock_init(portdev-ports_lock);
  INIT_LIST_HEAD(portdev-ports);
   
  +   virtio_device_ready(portdev-vdev);
  +
  if (multiport) {
  unsigned int nr_added_bufs;
   
 
 I wanted to set DRIVER_OK as late as possible, to both reduce
 the chance it can fail after DRIVER_OK and to reduce  the risk of
 introducing a regression since old qemu might only start sending
 interrupts after DRIVER_OK is set.
 
 So I wanted everything completely initialized before DRIVER_OK.
 
 You patch makes sense to me since nothing can fail,
 and we won't get interrupts before we add inbufs.
 
 Reviewed-by: Michael S. Tsirkin m...@redhat.com
 
 Testig will report shortly, pls send with sob line.

Sure enough, this helps:

Tested-by: Michael S. Tsirkin m...@redhat.com
Acked-by: Michael S. Tsirkin m...@redhat.com

Pls repost as a top-level patch.

 -- 
 MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v4 13/25] virtio_console: enable VQs early

2014-10-20 Thread Thomas Graf
On 10/20/14 at 05:04pm, Michael S. Tsirkin wrote:
 Sure enough, this helps:
 
 Tested-by: Michael S. Tsirkin m...@redhat.com
 Acked-by: Michael S. Tsirkin m...@redhat.com
 
 Pls repost as a top-level patch.

Thanks for fixing this so promptly.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v4 13/25] virtio_console: enable VQs early

2014-10-20 Thread Thomas Graf
On 10/20/14 at 02:42pm, Cornelia Huck wrote:
 On Mon, 20 Oct 2014 13:07:50 +0100
 Thomas Graf tg...@suug.ch 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 m...@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
 
 I think we need to set this in the probe function instead, otherwise we
 fail for multiqueue (which also wants to use the control queue early).
 
 Completely untested patch below; I can send this with proper s-o-b if
 it helps.

virtio_dev_probe() already sets DRIVER_OK if -probe() returned
without an error. I assume Michael added it to add_port() because
probing doesn't always occur first. Can we just remove the BUG_ON()?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v4 13/25] virtio_console: enable VQs early

2014-10-20 Thread Thomas Graf
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 m...@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:  [#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: 8800364bc840 ti: 88007849 task.ti: 
88007849
[1.840169] RIP: 0010:[a01d92c6]  [a01d92c6] 
virtio_device_ready.part.12+0x4/0x6 [virtio_console]
[1.840169] RSP: 0018:880078493c78  EFLAGS: 00010202
[1.840169] RAX: 0007 RBX: 880036406200 RCX: 6e39
[1.840169] RDX: c0b2 RSI:  RDI: 0001c0b2
[1.840169] RBP: 880078493c78 R08: 81d2c6f8 R09: 
[1.840169] R10: 0001 R11: 8800364bd000 R12: 880036618400
[1.840169] R13: 0001 R14: 8800368c6800 R15: 880036406220
[1.840169] FS:  () GS:88007fd0() 
knlGS:
[1.840169] CS:  0010 DS:  ES:  CR0: 8005003b
[1.840169] CR2: 7f1c31c9 CR3: 01c14000 CR4: 06e0
[1.840169] Stack:
[1.840169]  880078493ce8 a01d886a 8801 
810e20cd
[1.840169]  880078493cb8 0282  
87f90194
[1.840169]  880078493ce8 88007ab1d4e0 880036618498 
880036618410
[1.840169] Call Trace:
[1.840169]  [a01d886a] add_port+0x40a/0x440 [virtio_console]
[1.840169]  [810e20cd] ? trace_hardirqs_on+0xd/0x10
[1.840169]  [a01d8c6a] control_work_handler+0x3ca/0x420 
[virtio_console]
[1.840169]  [810b0e7b] ? process_one_work+0x15b/0x530
[1.840169]  [810b0ef4] process_one_work+0x1d4/0x530
[1.840169]  [810b0e7b] ? process_one_work+0x15b/0x530
[1.840169]  [810b136b] worker_thread+0x11b/0x490
[1.840169]  [810b1250] ? process_one_work+0x530/0x530
[1.840169]  [810b67c3] kthread+0xf3/0x110
[1.840169]  [81788f00] ? _raw_spin_unlock_irq+0x30/0x50
[1.840169]  [810b66d0] ? kthread_create_on_node+0x240/0x240
[1.840169]  [81789a7c] ret_from_fork+0x7c/0xb0
[1.840169]  [810b66d0] ? 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  [a01d92c6] virtio_device_ready.part.12+0x4/0x6 
[virtio_console]
[1.840169]  RSP 880078493c78

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v4 13/25] virtio_console: enable VQs early

2014-10-20 Thread Thomas Graf
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 m...@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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH v4 13/25] virtio_console: enable VQs early

2014-10-13 Thread Michael S. Tsirkin
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 m...@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) {
-- 
MST


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization