Re: [SeaBIOS] usb-msc.c: TEST_UNIT_READY needs USB_DIR_OUT?
On Wed, Mar 14, 2012 at 01:02:39PM -0600, Steve Goodrich wrote:
I've been working with coreboot and SeaBIOS lately to try to get a platform working, including having boot capability using an SD-to-USB adapter. I got stuck on this last part: the USB/SD adapter would start enumeration, but would fail out during TEST_UNIT_READY because the device came back with a STALL (this via a LeCroy/CACT USB analyzer). I compared the transactions to those done during USB enumeration/configuration in Linux, and I noticed that the Direction bit in bmCBWFlags is *cleared* for the TEST_UNIT_READY command in Linux, but *set* in coreboot.
If Linux always clears the direction bit for zero length transfers, then I think it should be safe to do that in SeaBIOS also. How about the patch below? -Kevin
From 1fd9a89082b807a4bb4ab6ce1285df474cb75746 Mon Sep 17 00:00:00 2001 From: Kevin O'Connor <kevin@koconnor.net> Date: Wed, 14 Mar 2012 21:27:45 -0400 Subject: [PATCH] Use OUT mode for all zero byte "scsi" transfers.
Some devices can get confused if asked to "read" data during a zero byte transfer, so consider these transfers as "writes". (Reported by Steve Goodrich.) Also, extract out the code to determine the transfer direction into cdb_is_read(). Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --- src/blockcmd.c | 7 +++++++ src/blockcmd.h | 1 + src/usb-msc.c | 4 ++-- src/virtio-scsi.c | 7 ++++--- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/blockcmd.c b/src/blockcmd.c index 8d881a5..2dda04d 100644 --- a/src/blockcmd.c +++ b/src/blockcmd.c @@ -35,6 +35,13 @@ cdb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) } } +// Determine if the command is a request to pull data from the device +int +cdb_is_read(u8 *cdbcmd, u16 blocksize) +{ + return blocksize && cdbcmd[0] != CDB_CMD_WRITE_10; +} + int scsi_is_ready(struct disk_op_s *op) { diff --git a/src/blockcmd.h b/src/blockcmd.h index ccfeeaf..8459d3e 100644 --- a/src/blockcmd.h +++ b/src/blockcmd.h @@ -100,6 +100,7 @@ struct cdbres_mode_sense_geom { } PACKED; // blockcmd.c +int cdb_is_read(u8 *cdbcmd, u16 blocksize); int cdb_get_inquiry(struct disk_op_s *op, struct cdbres_inquiry *data); int cdb_get_sense(struct disk_op_s *op, struct cdbres_request_sense *data); int cdb_test_unit_ready(struct disk_op_s *op); diff --git a/src/usb-msc.c b/src/usb-msc.c index dadb9ca..c53a1f5 100644 --- a/src/usb-msc.c +++ b/src/usb-msc.c @@ -77,13 +77,13 @@ usb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) cbw.dCBWSignature = CBW_SIGNATURE; cbw.dCBWTag = 999; // XXX cbw.dCBWDataTransferLength = bytes; - cbw.bmCBWFlags = (cbw.CBWCB[0] == CDB_CMD_WRITE_10) ? USB_DIR_OUT : USB_DIR_IN; + cbw.bmCBWFlags = cdb_is_read(cdbcmd, blocksize) ? USB_DIR_IN : USB_DIR_OUT; cbw.bCBWLUN = 0; // XXX cbw.bCBWCBLength = USB_CDB_SIZE; // Transfer cbw to device. int ret = usb_msc_send(udrive_g, USB_DIR_OUT - , MAKE_FLATPTR(GET_SEG(SS), &cbw), sizeof(cbw)); + , MAKE_FLATPTR(GET_SEG(SS), &cbw), sizeof(cbw)); if (ret) goto fail; diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c index 3c59438..50339d7 100644 --- a/src/virtio-scsi.c +++ b/src/virtio-scsi.c @@ -31,7 +31,7 @@ struct virtio_lun_s { static int virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, - void *cdbcmd, u16 target, u16 lun, u32 len) + void *cdbcmd, u16 target, u16 lun, u16 blocksize) { struct virtio_scsi_req_cmd req; struct virtio_scsi_resp_cmd resp; @@ -44,7 +44,8 @@ virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, req.lun[3] = (lun & 0xff); memcpy(req.cdb, cdbcmd, 16); - int datain = (req.cdb[0] != CDB_CMD_WRITE_10); + u32 len = op->count * blocksize; + int datain = cdb_is_read(cdbcmd, blocksize); int data_idx = (datain ? 2 : 1); int out_num = (datain ? 1 : 2); int in_num = (len ? 3 : 2) - out_num; @@ -89,7 +90,7 @@ virtio_scsi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) return virtio_scsi_cmd(GET_GLOBAL(vlun->ioaddr), GET_GLOBAL(vlun->vq), op, cdbcmd, GET_GLOBAL(vlun->target), GET_GLOBAL(vlun->lun), - blocksize * op->count); + blocksize); } static int -- 1.7.6.5
Kevin O'Connor wrote:
Subject: [PATCH] Use OUT mode for all zero byte "scsi" transfers. .. +++ b/src/blockcmd.c @@ -35,6 +35,13 @@ cdb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) } }
+// Determine if the command is a request to pull data from the device +int +cdb_is_read(u8 *cdbcmd, u16 blocksize) +{ + return blocksize && cdbcmd[0] != CDB_CMD_WRITE_10; +}
Suggest that this only evaluates commands, ..
diff --git a/src/usb-msc.c b/src/usb-msc.c index dadb9ca..c53a1f5 100644 --- a/src/usb-msc.c +++ b/src/usb-msc.c @@ -77,13 +77,13 @@ usb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) cbw.dCBWSignature = CBW_SIGNATURE; cbw.dCBWTag = 999; // XXX cbw.dCBWDataTransferLength = bytes; - cbw.bmCBWFlags = (cbw.CBWCB[0] == CDB_CMD_WRITE_10) ? USB_DIR_OUT : USB_DIR_IN; + cbw.bmCBWFlags = cdb_is_read(cdbcmd, blocksize) ? USB_DIR_IN : USB_DIR_OUT;
..and that blocksize is checked here: cbw.bmCBWFlags = cdb_is_read(&cbw) && blocksize ? USB_DIR_IN : USB_DIR_OUT;
cbw.bCBWLUN = 0; // XXX cbw.bCBWCBLength = USB_CDB_SIZE;
// Transfer cbw to device. int ret = usb_msc_send(udrive_g, USB_DIR_OUT - , MAKE_FLATPTR(GET_SEG(SS), &cbw), sizeof(cbw)); + , MAKE_FLATPTR(GET_SEG(SS), &cbw), sizeof(cbw));
Unrelated whitespace change?
diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c index 3c59438..50339d7 100644 --- a/src/virtio-scsi.c +++ b/src/virtio-scsi.c @@ -31,7 +31,7 @@ struct virtio_lun_s {
static int virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, - void *cdbcmd, u16 target, u16 lun, u32 len) + void *cdbcmd, u16 target, u16 lun, u16 blocksize) { struct virtio_scsi_req_cmd req; struct virtio_scsi_resp_cmd resp; @@ -44,7 +44,8 @@ virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, req.lun[3] = (lun & 0xff); memcpy(req.cdb, cdbcmd, 16);
- int datain = (req.cdb[0] != CDB_CMD_WRITE_10); + u32 len = op->count * blocksize; + int datain = cdb_is_read(cdbcmd, blocksize); int data_idx = (datain ? 2 : 1); int out_num = (datain ? 1 : 2); int in_num = (len ? 3 : 2) - out_num; @@ -89,7 +90,7 @@ virtio_scsi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) return virtio_scsi_cmd(GET_GLOBAL(vlun->ioaddr), GET_GLOBAL(vlun->vq), op, cdbcmd, GET_GLOBAL(vlun->target), GET_GLOBAL(vlun->lun), - blocksize * op->count); + blocksize); }
This also seems to be an unrelated change? //Peter
Il 15/03/2012 03:44, Peter Stuge ha scritto:
Kevin O'Connor wrote:
Subject: [PATCH] Use OUT mode for all zero byte "scsi" transfers. .. +++ b/src/blockcmd.c @@ -35,6 +35,13 @@ cdb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) } }
+// Determine if the command is a request to pull data from the device +int +cdb_is_read(u8 *cdbcmd, u16 blocksize) +{ + return blocksize && cdbcmd[0] != CDB_CMD_WRITE_10; +}
Suggest that this only evaluates commands, ..
blocksize == 0 identifies a command that has no DMA, so it's a good way to catch all the problematic commands, in case more are added in the future.
diff --git a/src/usb-msc.c b/src/usb-msc.c index dadb9ca..c53a1f5 100644 --- a/src/usb-msc.c +++ b/src/usb-msc.c @@ -77,13 +77,13 @@ usb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) cbw.dCBWSignature = CBW_SIGNATURE; cbw.dCBWTag = 999; // XXX cbw.dCBWDataTransferLength = bytes; - cbw.bmCBWFlags = (cbw.CBWCB[0] == CDB_CMD_WRITE_10) ? USB_DIR_OUT : USB_DIR_IN; + cbw.bmCBWFlags = cdb_is_read(cdbcmd, blocksize) ? USB_DIR_IN : USB_DIR_OUT;
..and that blocksize is checked here:
cbw.bmCBWFlags = cdb_is_read(&cbw) && blocksize ? USB_DIR_IN : USB_DIR_OUT;
That would duplicate the check in virtio-scsi too.
diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c index 3c59438..50339d7 100644 --- a/src/virtio-scsi.c +++ b/src/virtio-scsi.c @@ -31,7 +31,7 @@ struct virtio_lun_s {
static int virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, - void *cdbcmd, u16 target, u16 lun, u32 len) + void *cdbcmd, u16 target, u16 lun, u16 blocksize) { struct virtio_scsi_req_cmd req; struct virtio_scsi_resp_cmd resp; @@ -44,7 +44,8 @@ virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, req.lun[3] = (lun & 0xff); memcpy(req.cdb, cdbcmd, 16);
- int datain = (req.cdb[0] != CDB_CMD_WRITE_10); + u32 len = op->count * blocksize; + int datain = cdb_is_read(cdbcmd, blocksize); int data_idx = (datain ? 2 : 1); int out_num = (datain ? 1 : 2); int in_num = (len ? 3 : 2) - out_num; @@ -89,7 +90,7 @@ virtio_scsi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) return virtio_scsi_cmd(GET_GLOBAL(vlun->ioaddr), GET_GLOBAL(vlun->vq), op, cdbcmd, GET_GLOBAL(vlun->target), GET_GLOBAL(vlun->lun), - blocksize * op->count); + blocksize); }
This also seems to be an unrelated change?
virtio_scsi_cmd needs blocksize to pass it to cdb_is_read. So it's related. FWIW, Acked-by: Paolo Bonzini <pbonzini@redhat.com> Paolo
-----Original Message----- From: Kevin O'Connor [mailto:kevin@koconnor.net] Subject: Re: [SeaBIOS] usb-msc.c: TEST_UNIT_READY needs USB_DIR_OUT?
If Linux always clears the direction bit for zero length transfers, then I think it should be safe to do that in SeaBIOS also. How about the patch below?
I tried this patch on my system and it worked fine. -- Steve G.
Il 15/03/2012 02:31, Kevin O'Connor ha scritto:
On Wed, Mar 14, 2012 at 01:02:39PM -0600, Steve Goodrich wrote:
I've been working with coreboot and SeaBIOS lately to try to get a platform working, including having boot capability using an SD-to-USB adapter. I got stuck on this last part: the USB/SD adapter would start enumeration, but would fail out during TEST_UNIT_READY because the device came back with a STALL (this via a LeCroy/CACT USB analyzer). I compared the transactions to those done during USB enumeration/configuration in Linux, and I noticed that the Direction bit in bmCBWFlags is *cleared* for the TEST_UNIT_READY command in Linux, but *set* in coreboot.
If Linux always clears the direction bit for zero length transfers, then I think it should be safe to do that in SeaBIOS also. How about the patch below?
-Kevin
From 1fd9a89082b807a4bb4ab6ce1285df474cb75746 Mon Sep 17 00:00:00 2001 From: Kevin O'Connor <kevin@koconnor.net> Date: Wed, 14 Mar 2012 21:27:45 -0400 Subject: [PATCH] Use OUT mode for all zero byte "scsi" transfers.
Some devices can get confused if asked to "read" data during a zero byte transfer, so consider these transfers as "writes". (Reported by Steve Goodrich.)
Also, extract out the code to determine the transfer direction into cdb_is_read().
Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --- src/blockcmd.c | 7 +++++++ src/blockcmd.h | 1 + src/usb-msc.c | 4 ++-- src/virtio-scsi.c | 7 ++++--- 4 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/src/blockcmd.c b/src/blockcmd.c index 8d881a5..2dda04d 100644 --- a/src/blockcmd.c +++ b/src/blockcmd.c @@ -35,6 +35,13 @@ cdb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) } }
+// Determine if the command is a request to pull data from the device +int +cdb_is_read(u8 *cdbcmd, u16 blocksize) +{ + return blocksize && cdbcmd[0] != CDB_CMD_WRITE_10; +} + int scsi_is_ready(struct disk_op_s *op) { diff --git a/src/blockcmd.h b/src/blockcmd.h index ccfeeaf..8459d3e 100644 --- a/src/blockcmd.h +++ b/src/blockcmd.h @@ -100,6 +100,7 @@ struct cdbres_mode_sense_geom { } PACKED;
// blockcmd.c +int cdb_is_read(u8 *cdbcmd, u16 blocksize); int cdb_get_inquiry(struct disk_op_s *op, struct cdbres_inquiry *data); int cdb_get_sense(struct disk_op_s *op, struct cdbres_request_sense *data); int cdb_test_unit_ready(struct disk_op_s *op); diff --git a/src/usb-msc.c b/src/usb-msc.c index dadb9ca..c53a1f5 100644 --- a/src/usb-msc.c +++ b/src/usb-msc.c @@ -77,13 +77,13 @@ usb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) cbw.dCBWSignature = CBW_SIGNATURE; cbw.dCBWTag = 999; // XXX cbw.dCBWDataTransferLength = bytes; - cbw.bmCBWFlags = (cbw.CBWCB[0] == CDB_CMD_WRITE_10) ? USB_DIR_OUT : USB_DIR_IN; + cbw.bmCBWFlags = cdb_is_read(cdbcmd, blocksize) ? USB_DIR_IN : USB_DIR_OUT; cbw.bCBWLUN = 0; // XXX cbw.bCBWCBLength = USB_CDB_SIZE;
// Transfer cbw to device. int ret = usb_msc_send(udrive_g, USB_DIR_OUT - , MAKE_FLATPTR(GET_SEG(SS), &cbw), sizeof(cbw)); + , MAKE_FLATPTR(GET_SEG(SS), &cbw), sizeof(cbw)); if (ret) goto fail;
diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c index 3c59438..50339d7 100644 --- a/src/virtio-scsi.c +++ b/src/virtio-scsi.c @@ -31,7 +31,7 @@ struct virtio_lun_s {
static int virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, - void *cdbcmd, u16 target, u16 lun, u32 len) + void *cdbcmd, u16 target, u16 lun, u16 blocksize) { struct virtio_scsi_req_cmd req; struct virtio_scsi_resp_cmd resp; @@ -44,7 +44,8 @@ virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, req.lun[3] = (lun & 0xff); memcpy(req.cdb, cdbcmd, 16);
- int datain = (req.cdb[0] != CDB_CMD_WRITE_10); + u32 len = op->count * blocksize; + int datain = cdb_is_read(cdbcmd, blocksize);
The patch changes the way TEST_UNIT_READY is composed in the buffers and breaks virtio-scsi. Now I remember why I did it this way. :) You can squash this in: diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c index 76c5f29..d03a562 100644 --- a/src/virtio-scsi.c +++ b/src/virtio-scsi.c @@ -46,9 +46,8 @@ virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, u32 len = op->count * blocksize; int datain = cdb_is_read(cdbcmd, blocksize); - int data_idx = (datain ? 2 : 1); - int out_num = (datain ? 1 : 2); - int in_num = (len ? 3 : 2) - out_num; + int in_num = (datain ? 2 : 1); + int out_num = (len ? 3 : 2) - in_num; sg[0].addr = MAKE_FLATPTR(GET_SEG(SS), &req); sg[0].length = sizeof(req); @@ -56,8 +56,11 @@ virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, sg[out_num].addr = MAKE_FLATPTR(GET_SEG(SS), &resp); sg[out_num].length = sizeof(resp); - sg[data_idx].addr = op->buf_fl; - sg[data_idx].length = len; + if (len) { + int data_idx = (datain ? 2 : 1); + sg[data_idx].addr = op->buf_fl; + sg[data_idx].length = len; + } /* Add to virtqueue and kick host */ vring_add_buf(vq, sg, out_num, in_num, 0, 0); and apply the patch. Thanks! Paolo
I tested this (the combined patches) with several of the different USB thumbdrives that I have and didn't have any problems. In fact it actually increased the pass rate of my failing Ubuntu booting problem from ~0% to ~50%, but I'll address that in that email stream. dave ----- Original Message -----
From: "Paolo Bonzini" <pbonzini@redhat.com> To: "Kevin O'Connor" <kevin@koconnor.net> Cc: seabios@seabios.org Sent: Friday, March 16, 2012 11:54:46 AM Subject: Re: [SeaBIOS] usb-msc.c: TEST_UNIT_READY needs USB_DIR_OUT?
Il 15/03/2012 02:31, Kevin O'Connor ha scritto:
On Wed, Mar 14, 2012 at 01:02:39PM -0600, Steve Goodrich wrote:
I've been working with coreboot and SeaBIOS lately to try to get a platform working, including having boot capability using an SD-to-USB adapter. I got stuck on this last part: the USB/SD adapter would start enumeration, but would fail out during TEST_UNIT_READY because the device came back with a STALL (this via a LeCroy/CACT USB analyzer). I compared the transactions to those done during USB enumeration/configuration in Linux, and I noticed that the Direction bit in bmCBWFlags is *cleared* for the TEST_UNIT_READY command in Linux, but *set* in coreboot.
If Linux always clears the direction bit for zero length transfers, then I think it should be safe to do that in SeaBIOS also. How about the patch below?
-Kevin
From 1fd9a89082b807a4bb4ab6ce1285df474cb75746 Mon Sep 17 00:00:00 2001 From: Kevin O'Connor <kevin@koconnor.net> Date: Wed, 14 Mar 2012 21:27:45 -0400 Subject: [PATCH] Use OUT mode for all zero byte "scsi" transfers.
Some devices can get confused if asked to "read" data during a zero byte transfer, so consider these transfers as "writes". (Reported by Steve Goodrich.)
Also, extract out the code to determine the transfer direction into cdb_is_read().
Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --- src/blockcmd.c | 7 +++++++ src/blockcmd.h | 1 + src/usb-msc.c | 4 ++-- src/virtio-scsi.c | 7 ++++--- 4 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/src/blockcmd.c b/src/blockcmd.c index 8d881a5..2dda04d 100644 --- a/src/blockcmd.c +++ b/src/blockcmd.c @@ -35,6 +35,13 @@ cdb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) } }
+// Determine if the command is a request to pull data from the device +int +cdb_is_read(u8 *cdbcmd, u16 blocksize) +{ + return blocksize && cdbcmd[0] != CDB_CMD_WRITE_10; +} + int scsi_is_ready(struct disk_op_s *op) { diff --git a/src/blockcmd.h b/src/blockcmd.h index ccfeeaf..8459d3e 100644 --- a/src/blockcmd.h +++ b/src/blockcmd.h @@ -100,6 +100,7 @@ struct cdbres_mode_sense_geom { } PACKED;
// blockcmd.c +int cdb_is_read(u8 *cdbcmd, u16 blocksize); int cdb_get_inquiry(struct disk_op_s *op, struct cdbres_inquiry *data); int cdb_get_sense(struct disk_op_s *op, struct cdbres_request_sense *data); int cdb_test_unit_ready(struct disk_op_s *op); diff --git a/src/usb-msc.c b/src/usb-msc.c index dadb9ca..c53a1f5 100644 --- a/src/usb-msc.c +++ b/src/usb-msc.c @@ -77,13 +77,13 @@ usb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) cbw.dCBWSignature = CBW_SIGNATURE; cbw.dCBWTag = 999; // XXX cbw.dCBWDataTransferLength = bytes; - cbw.bmCBWFlags = (cbw.CBWCB[0] == CDB_CMD_WRITE_10) ? USB_DIR_OUT : USB_DIR_IN; + cbw.bmCBWFlags = cdb_is_read(cdbcmd, blocksize) ? USB_DIR_IN : USB_DIR_OUT; cbw.bCBWLUN = 0; // XXX cbw.bCBWCBLength = USB_CDB_SIZE;
// Transfer cbw to device. int ret = usb_msc_send(udrive_g, USB_DIR_OUT - , MAKE_FLATPTR(GET_SEG(SS), &cbw), sizeof(cbw)); + , MAKE_FLATPTR(GET_SEG(SS), &cbw), sizeof(cbw)); if (ret) goto fail;
diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c index 3c59438..50339d7 100644 --- a/src/virtio-scsi.c +++ b/src/virtio-scsi.c @@ -31,7 +31,7 @@ struct virtio_lun_s {
static int virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, - void *cdbcmd, u16 target, u16 lun, u32 len) + void *cdbcmd, u16 target, u16 lun, u16 blocksize) { struct virtio_scsi_req_cmd req; struct virtio_scsi_resp_cmd resp; @@ -44,7 +44,8 @@ virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, req.lun[3] = (lun & 0xff); memcpy(req.cdb, cdbcmd, 16);
- int datain = (req.cdb[0] != CDB_CMD_WRITE_10); + u32 len = op->count * blocksize; + int datain = cdb_is_read(cdbcmd, blocksize);
The patch changes the way TEST_UNIT_READY is composed in the buffers and breaks virtio-scsi. Now I remember why I did it this way. :)
You can squash this in:
diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c index 76c5f29..d03a562 100644 --- a/src/virtio-scsi.c +++ b/src/virtio-scsi.c @@ -46,9 +46,8 @@ virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op,
u32 len = op->count * blocksize; int datain = cdb_is_read(cdbcmd, blocksize); - int data_idx = (datain ? 2 : 1); - int out_num = (datain ? 1 : 2); - int in_num = (len ? 3 : 2) - out_num; + int in_num = (datain ? 2 : 1); + int out_num = (len ? 3 : 2) - in_num;
sg[0].addr = MAKE_FLATPTR(GET_SEG(SS), &req); sg[0].length = sizeof(req); @@ -56,8 +56,11 @@ virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, sg[out_num].addr = MAKE_FLATPTR(GET_SEG(SS), &resp); sg[out_num].length = sizeof(resp);
- sg[data_idx].addr = op->buf_fl; - sg[data_idx].length = len; + if (len) { + int data_idx = (datain ? 2 : 1); + sg[data_idx].addr = op->buf_fl; + sg[data_idx].length = len; + }
/* Add to virtqueue and kick host */ vring_add_buf(vq, sg, out_num, in_num, 0, 0);
and apply the patch.
Thanks!
Paolo
_______________________________________________ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
On Fri, Mar 16, 2012 at 06:54:46PM +0100, Paolo Bonzini wrote:
The patch changes the way TEST_UNIT_READY is composed in the buffers and breaks virtio-scsi. Now I remember why I did it this way. :)
Thanks. I commited the original patch and your patch on top of it. -Kevin
participants (5)
-
Dave Frodin -
Kevin O'Connor -
Paolo Bonzini -
Peter Stuge -
Steve Goodrich