Re: [Qemu-devel] Bug in SeaBIOS virtio-ring handling bug with vhost-scsi-pci

On Mon, Apr 08, 2013 at 09:09:54PM -0700, Nicholas A. Bellinger wrote:

Ping.

Any ideas on this one..?  It seems like this bug would be effecting
non vhost-scsi-pci code as well..

Nicholas, where is the latest v3 code. Can you push it to your tree. 

--nab

On Thu, 2013-04-04 at 19:59 -0700, Nicholas A. Bellinger wrote:

quoted

Hi Paolo & Co,

So I've been running into another bug with SeaBIOS w/ virtio-scsi +
vhost-scsi-pci code..

During the last weeks debugging on the QEMU vhost memory re-mapping bug,
my seabios builds have been using a smaller target ID (16) for scanning
than the default hard coded value of 256 in init_virtio_scsi().

After bumping this value back to 256 recently, a bogus out_num value
from vhost_get_vq_desc() start appearing on the tcm_vhost side:

[17348.044031] Invalid buffer layout out: 0 in: 2

After adding more debug output to SeaBIOS, the vring->num value appears
to be reset from the expected MAX_QUEUE_NUM=128, then incremented each
time after vring_more_used() is called to poll for completion.. 

init virtio-scsi
found virtio-scsi at 0:4
vring_init Setting vr->num: 128
Calling virtio_scsi_scan_target for target: 0
Searching bootorder for: /pci@i0cf8/*@4/*@0/*@0,0
virtio_scsi_cmd, using vring_add_buf for out_num: 1 in_num: 2
virtio_scsi_cmd, using vring_add_buf vr: 0x000ed404 vr->num: 128
vring_add_buf, vr: 0x000ed404 vr->num: 128
vring_add_buf, head: 0, index: 0
vring_add_buf, setting desc[prev=2].flags
vring_add_buf, setting free_head: 3
vring_add_buf, setting q->vdata[head=0]
vring_add_buf, setting q->ring[av=0]
virtio_scsi_cmd, after vring_add_buf vr: 0x000ed404 vr->num: 128
virtio_scsi_cmd, after vring_kick vr: 0x000ed404 vr->num: 128
virtio_scsi_cmd, after vring_more_used #4 vr: 0x000ed404 vr->num: 1
                                          ^^^^^^^^^^^^^^^^^^^^^^^^^   

   Reset from 128 to 1 immediate after first vring_more_used()..?

100 more quoted lines

Ah, I did not see this before. If we disable SeaBIOS and only use guest
kernel to access tcm_vhost disk, do you see this.

quoted

I'm using the following to work-around this issue for the moment, and
would like some input from virtio / SeaBIOS folks..

WDYT..?

--nab

diff --git a/src/virtio-ring.c b/src/virtio-ring.c
index bddb090..e046141 100644
--- a/src/virtio-ring.c
+++ b/src/virtio-ring.c

@@ -35,10 +35,16 @@
 
 int vring_more_used(struct vring_virtqueue *vq)
 {
+    struct vring *vr = &vq->vring;
     struct vring_used *used = GET_LOWFLAT(vq->vring.used);
     int more = GET_LOWFLAT(vq->last_used_idx) != GET_LOWFLAT(used->idx);
     /* Make sure ring reads are done after idx read above. */
     smp_rmb();
+
+    /* Huh..?  vr->num is somehow changing from below us..? */

Pretty strange.

quoted

+    if (GET_LOWFLAT(vr->num) != MAX_QUEUE_NUM) {
+           SET_LOWFLAT(vr->num, MAX_QUEUE_NUM);
+    }
     return more;
 }

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

-- 
Asias

On Tue, 2013-04-09 at 12:24 +0800, Asias He wrote:

On Mon, Apr 08, 2013 at 09:09:54PM -0700, Nicholas A. Bellinger wrote:

quoted

Ping.

Any ideas on this one..?  It seems like this bug would be effecting
non vhost-scsi-pci code as well..

Nicholas, where is the latest v3 code. Can you push it to your tree. 

Sure.  Just pushed to:

http://git.kernel.org/cgit/virt/kvm/nab/qemu-kvm.git/log/?h=vhost-scsi-for-1.4

and should be appearing momentarily.

--nab

quoted

--nab

On Thu, 2013-04-04 at 19:59 -0700, Nicholas A. Bellinger wrote:

quoted

Hi Paolo & Co,

So I've been running into another bug with SeaBIOS w/ virtio-scsi +
vhost-scsi-pci code..

During the last weeks debugging on the QEMU vhost memory re-mapping bug,
my seabios builds have been using a smaller target ID (16) for scanning
than the default hard coded value of 256 in init_virtio_scsi().

After bumping this value back to 256 recently, a bogus out_num value
from vhost_get_vq_desc() start appearing on the tcm_vhost side:

[17348.044031] Invalid buffer layout out: 0 in: 2

After adding more debug output to SeaBIOS, the vring->num value appears
to be reset from the expected MAX_QUEUE_NUM=128, then incremented each
time after vring_more_used() is called to poll for completion.. 

init virtio-scsi
found virtio-scsi at 0:4
vring_init Setting vr->num: 128
Calling virtio_scsi_scan_target for target: 0
Searching bootorder for: /pci@i0cf8/*@4/*@0/*@0,0
virtio_scsi_cmd, using vring_add_buf for out_num: 1 in_num: 2
virtio_scsi_cmd, using vring_add_buf vr: 0x000ed404 vr->num: 128
vring_add_buf, vr: 0x000ed404 vr->num: 128
vring_add_buf, head: 0, index: 0
vring_add_buf, setting desc[prev=2].flags
vring_add_buf, setting free_head: 3
vring_add_buf, setting q->vdata[head=0]
vring_add_buf, setting q->ring[av=0]
virtio_scsi_cmd, after vring_add_buf vr: 0x000ed404 vr->num: 128
virtio_scsi_cmd, after vring_kick vr: 0x000ed404 vr->num: 128
virtio_scsi_cmd, after vring_more_used #4 vr: 0x000ed404 vr->num: 1
                                          ^^^^^^^^^^^^^^^^^^^^^^^^^   

   Reset from 128 to 1 immediate after first vring_more_used()..?

142 more quoted lines