Re: [PATCH v4 10/25] virtio: add API to enable VQs early

2014-11-10 Thread Andy Grover

On 10/13/2014 12:50 AM, Michael S. Tsirkin wrote:

virtio spec 0.9.X requires DRIVER_OK to be set before
VQs are used, but some drivers use VQs before probe
function returns.
Since DRIVER_OK is set after probe, this violates the spec.

Even though under virtio 1.0 transitional devices support this
behaviour, we want to make it possible for those early callers to become
spec compliant and eventually support non-transitional devices.

Add API for drivers to call before using VQs.

Sets DRIVER_OK internally.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
  include/linux/virtio_config.h | 17 +
  1 file changed, 17 insertions(+)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index e8f8f71..e36403b 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -109,6 +109,23 @@ struct virtqueue *virtio_find_single_vq(struct 
virtio_device *vdev,
return vq;
  }

+/**
+ * virtio_device_ready - enable vq use in probe function
+ * @vdev: the device
+ *
+ * Driver must call this to use vqs in the probe function.
+ *
+ * Note: vqs are enabled automatically after probe returns.
+ */
+static inline
+void virtio_device_ready(struct virtio_device *dev)
+{
+   unsigned status = dev-config-get_status(dev);
+
+   BUG_ON(status  VIRTIO_CONFIG_S_DRIVER_OK);
+   dev-config-set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
+}


Getting a BUG when booting via KVM, host Fedora 20, guest Fedora 20.

my config is at:

https://fedorapeople.org/~grover/config-20141110



[0.828494] [ cut here ]
[0.829039] kernel BUG at 
/home/agrover/git/kernel/include/linux/virtio_config.h:125!

[0.831266] invalid opcode:  [#1] SMP DEBUG_PAGEALLOC
[0.831266] Modules linked in:
[0.831266] CPU: 1 PID: 30 Comm: kworker/1:1 Not tainted 3.18.0-rc4 #120
[0.831266] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[0.831266] Workqueue: events control_work_handler
[0.831266] task: 88003cd98000 ti: 88003cd94000 task.ti: 
88003cd94000
[0.831266] RIP: 0010:[81445004]  [81445004] 
add_port+0x264/0x410

[0.831266] RSP: :88003cd97c78  EFLAGS: 00010202
[0.831266] RAX: 0007 RBX: 88003c58c400 RCX: 
0001
[0.831266] RDX: c132 RSI: 81a955e9 RDI: 
0001c132
[0.831266] RBP: 88003cd97cc8 R08:  R09: 

[0.831266] R10: 0001 R11:  R12: 
88003c58be00
[0.831266] R13: 0001 R14: 8800395ca800 R15: 
88003c58c420
[0.831266] FS:  () GS:88003fa0() 
knlGS:

[0.831266] CS:  0010 DS:  ES:  CR0: 8005003b
[0.831266] CR2:  CR3: 01c11000 CR4: 
06e0
[0.831266] DR0:  DR1:  DR2: 

[0.831266] DR3:  DR6: fffe0ff0 DR7: 
0400

[0.831266] Stack:
[0.831266]  8801 0292  
0001
[0.831266]  88003cd97cc8 88003dfa8a20 88003c58beb8 
88003c58be10
[0.831266]  8800395a2000  88003cd97d38 
8144531a

[0.831266] Call Trace:
[0.831266]  [8144531a] control_work_handler+0x16a/0x3c0
[0.831266]  [8108b0c8] ? process_one_work+0x208/0x500
[0.831266]  [8108b16c] process_one_work+0x2ac/0x500
[0.831266]  [8108b0c8] ? process_one_work+0x208/0x500
[0.831266]  [8108b68e] worker_thread+0x2ce/0x4e0
[0.831266]  [8108b3c0] ? process_one_work+0x500/0x500
[0.831266]  [81090b28] kthread+0xf8/0x100
[0.831266]  [810bad7d] ? trace_hardirqs_on+0xd/0x10
[0.831266]  [81090a30] ? kthread_stop+0x140/0x140
[0.831266]  [816ea92c] ret_from_fork+0x7c/0xb0
[0.831266]  [81090a30] ? kthread_stop+0x140/0x140
[0.831266] Code: c7 c2 48 31 01 83 48 c7 c6 e9 55 a9 81 e8 55 b4 c6 
ff 4d 8b b4 24 58 01 00 00 49 8b 86 e8 04 00 00 4c 89 f7 ff 50 10 a8 04 
74 0c 0f 0b 66 2e 0f 1f 84 00 00 00 00 00 49 8b 96 e8 04 00 00 83 c8

[0.831266] RIP  [81445004] add_port+0x264/0x410
[0.831266]  RSP 88003cd97c78
[0.878202] ---[ end trace f98fbb172cc7bbf4 ]---

Thanks -- Andy

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 10/25] virtio: add API to enable VQs early

2014-11-10 Thread Michael S. Tsirkin
On Mon, Nov 10, 2014 at 04:45:09PM -0800, Andy Grover wrote:
 On 10/13/2014 12:50 AM, Michael S. Tsirkin wrote:
 virtio spec 0.9.X requires DRIVER_OK to be set before
 VQs are used, but some drivers use VQs before probe
 function returns.
 Since DRIVER_OK is set after probe, this violates the spec.
 
 Even though under virtio 1.0 transitional devices support this
 behaviour, we want to make it possible for those early callers to become
 spec compliant and eventually support non-transitional devices.
 
 Add API for drivers to call before using VQs.
 
 Sets DRIVER_OK internally.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
 ---
   include/linux/virtio_config.h | 17 +
   1 file changed, 17 insertions(+)
 
 diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
 index e8f8f71..e36403b 100644
 --- a/include/linux/virtio_config.h
 +++ b/include/linux/virtio_config.h
 @@ -109,6 +109,23 @@ struct virtqueue *virtio_find_single_vq(struct 
 virtio_device *vdev,
  return vq;
   }
 
 +/**
 + * virtio_device_ready - enable vq use in probe function
 + * @vdev: the device
 + *
 + * Driver must call this to use vqs in the probe function.
 + *
 + * Note: vqs are enabled automatically after probe returns.
 + */
 +static inline
 +void virtio_device_ready(struct virtio_device *dev)
 +{
 +unsigned status = dev-config-get_status(dev);
 +
 +BUG_ON(status  VIRTIO_CONFIG_S_DRIVER_OK);
 +dev-config-set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
 +}
 
 Getting a BUG when booting via KVM, host Fedora 20, guest Fedora 20.
 
 my config is at:
 
 https://fedorapeople.org/~grover/config-20141110
 

The fix is here:
http://article.gmane.org/gmane.linux.kernel.virtualization/23324/raw

I'm surprised it's not merged yet.

Rusty, could you pick it up please?


 
 [0.828494] [ cut here ]
 [0.829039] kernel BUG at
 /home/agrover/git/kernel/include/linux/virtio_config.h:125!
 [0.831266] invalid opcode:  [#1] SMP DEBUG_PAGEALLOC
 [0.831266] Modules linked in:
 [0.831266] CPU: 1 PID: 30 Comm: kworker/1:1 Not tainted 3.18.0-rc4 #120
 [0.831266] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
 [0.831266] Workqueue: events control_work_handler
 [0.831266] task: 88003cd98000 ti: 88003cd94000 task.ti:
 88003cd94000
 [0.831266] RIP: 0010:[81445004]  [81445004]
 add_port+0x264/0x410
 [0.831266] RSP: :88003cd97c78  EFLAGS: 00010202
 [0.831266] RAX: 0007 RBX: 88003c58c400 RCX:
 0001
 [0.831266] RDX: c132 RSI: 81a955e9 RDI:
 0001c132
 [0.831266] RBP: 88003cd97cc8 R08:  R09:
 
 [0.831266] R10: 0001 R11:  R12:
 88003c58be00
 [0.831266] R13: 0001 R14: 8800395ca800 R15:
 88003c58c420
 [0.831266] FS:  () GS:88003fa0()
 knlGS:
 [0.831266] CS:  0010 DS:  ES:  CR0: 8005003b
 [0.831266] CR2:  CR3: 01c11000 CR4:
 06e0
 [0.831266] DR0:  DR1:  DR2:
 
 [0.831266] DR3:  DR6: fffe0ff0 DR7:
 0400
 [0.831266] Stack:
 [0.831266]  8801 0292 
 0001
 [0.831266]  88003cd97cc8 88003dfa8a20 88003c58beb8
 88003c58be10
 [0.831266]  8800395a2000  88003cd97d38
 8144531a
 [0.831266] Call Trace:
 [0.831266]  [8144531a] control_work_handler+0x16a/0x3c0
 [0.831266]  [8108b0c8] ? process_one_work+0x208/0x500
 [0.831266]  [8108b16c] process_one_work+0x2ac/0x500
 [0.831266]  [8108b0c8] ? process_one_work+0x208/0x500
 [0.831266]  [8108b68e] worker_thread+0x2ce/0x4e0
 [0.831266]  [8108b3c0] ? process_one_work+0x500/0x500
 [0.831266]  [81090b28] kthread+0xf8/0x100
 [0.831266]  [810bad7d] ? trace_hardirqs_on+0xd/0x10
 [0.831266]  [81090a30] ? kthread_stop+0x140/0x140
 [0.831266]  [816ea92c] ret_from_fork+0x7c/0xb0
 [0.831266]  [81090a30] ? kthread_stop+0x140/0x140
 [0.831266] Code: c7 c2 48 31 01 83 48 c7 c6 e9 55 a9 81 e8 55 b4 c6 ff
 4d 8b b4 24 58 01 00 00 49 8b 86 e8 04 00 00 4c 89 f7 ff 50 10 a8 04 74 0c
 0f 0b 66 2e 0f 1f 84 00 00 00 00 00 49 8b 96 e8 04 00 00 83 c8
 [0.831266] RIP  [81445004] add_port+0x264/0x410
 [0.831266]  RSP 88003cd97c78
 [0.878202] ---[ end trace f98fbb172cc7bbf4 ]---
 
 Thanks -- Andy
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html