From 5c244dfbfcc8b610eac30ffa39c009724843695b Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 5 Mar 2012 12:29:12 +0100 Subject: [PATCH 1/4] scsi: do not send MODE SENSE except to QEMU disks This is the simplest way to avoid breaking boot on USB sticks that stall when asked for the MODE SENSE page 4. Some old sticks do not support the MODE SENSE command at all and just return a "medium may have changed" unit attention condition when SeaBIOS sends it! Reported-by: Dave Frodin Signed-off-by: Paolo Bonzini (cherry-picked from commit cb721714570520c02ae48efc26d3c04b8548d973) --- src/blockcmd.c | 37 +++++++++++++++++++++++-------------- 1 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/blockcmd.c b/src/blockcmd.c index 9919acb..4f3131c 100644 --- a/src/blockcmd.c +++ b/src/blockcmd.c @@ -131,20 +131,29 @@ scsi_init_drive(struct drive_s *drive, const char *s, int *pdt, char **desc) dprintf(1, "%s blksize=%d sectors=%d\n" , s, drive->blksize, (unsigned)drive->sectors); - struct cdbres_mode_sense_geom geomdata; - ret = cdb_mode_sense_geom(&dop, &geomdata); - if (ret == 0) { - u32 cylinders; - cylinders = geomdata.cyl[0] << 16; - cylinders |= geomdata.cyl[1] << 8; - cylinders |= geomdata.cyl[2]; - if (cylinders && geomdata.heads && - drive->sectors <= 0xFFFFFFFFULL && - ((u32)drive->sectors % (geomdata.heads * cylinders) == 0)) { - drive->pchs.cylinders = cylinders; - drive->pchs.heads = geomdata.heads; - drive->pchs.spt = (u32)drive->sectors - / (geomdata.heads * cylinders); + // We do not recover from USB stalls, so try to be safe and avoid + // sending the command if the (obsolete, but still provided by QEMU) + // fixed disk geometry page may not be supported. + // + // We could also send the command only to small disks (e.g. <504MiB) + // but some old USB keys only support a very small subset of SCSI which + // does not even include the MODE SENSE command! + // + if (! CONFIG_COREBOOT && memcmp(vendor, "QEMU ", 8) == 0) { + struct cdbres_mode_sense_geom geomdata; + ret = cdb_mode_sense_geom(&dop, &geomdata); + if (ret == 0) { + u32 cylinders; + cylinders = geomdata.cyl[0] << 16; + cylinders |= geomdata.cyl[1] << 8; + cylinders |= geomdata.cyl[2]; + if (cylinders && geomdata.heads && + drive->sectors <= 0xFFFFFFFFULL && + ((u32)drive->sectors % (geomdata.heads * cylinders) == 0)) { + drive->pchs.cylinders = cylinders; + drive->pchs.heads = geomdata.heads; + drive->pchs.spt = (u32)drive->sectors / (geomdata.heads * cylinders); + } } } -- 1.7.9.1 From 04b3f8876a82eac4f06c8e89d6ca0e2c35f632a6 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 15 Mar 2012 16:10:53 +0100 Subject: [PATCH 2/4] Use OUT mode for all zero byte "scsi" transfers. Upstream status: posted at http://permalink.gmane.org/gmane.comp.bios.coreboot.seabios/3466 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 Signed-off-by: Paolo Bonzini (cherry-picked from commit 1fd9a89082b807a4bb4ab6ce1285df474cb75746) --- 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 4f3131c..f77f3af 100644 --- a/src/blockcmd.c +++ b/src/blockcmd.c @@ -31,6 +31,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 bace649..4442ae1 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 cde08ce..8210a15 100644 --- a/src/usb-msc.c +++ b/src/usb-msc.c @@ -74,13 +74,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 9437116..76c5f29 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.9.1 From ff5cb1672acf63e2a82a765f04f4044ce5c19d6f Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 19 Mar 2012 11:44:05 +0100 Subject: [PATCH 3/4] virtio-scsi: Fix virtio-scsi after cdb_is_read changes. The previous patch changes the way TEST_UNIT_READY is composed in the buffers and breaks virtio-scsi. Signed-off-by: Paolo Bonzini (cherry-picked from commit 8c313078917099a002d45f58d58ae2f4eb9a657f) --- src/virtio-scsi.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c index 76c5f29..0aa3388 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 +55,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); -- 1.7.9.1 From 9c70fa372c60e1195bc407fcd0064efc24f440a5 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 19 Mar 2012 11:37:30 +0100 Subject: [PATCH 4/4] ata: send TEST UNIT READY correctly The ATAPI driver does not need to support writes, but it does needs to avoid the PIO transfer and DRQ check when TEST UNIT READY is sent. Since TEST UNIT READY has no payload, checking for not busy is enough. This fixes a timeout when booting from CD/DVD, which fellaw@gmx.net reported to cause boot failures. Signed-off-by: Paolo Bonzini --- src/ata.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/ata.c b/src/ata.c index b33dcb6..752db4a 100644 --- a/src/ata.c +++ b/src/ata.c @@ -642,13 +642,15 @@ atapi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) ret = -2; goto fail; } - if (!(status & ATA_CB_STAT_DRQ)) { - dprintf(6, "send_atapi_cmd : DRQ not set (status %02x)\n", status); - ret = -3; - goto fail; - } + if (blocksize) { + if (!(status & ATA_CB_STAT_DRQ)) { + dprintf(6, "send_atapi_cmd : DRQ not set (status %02x)\n", status); + ret = -3; + goto fail; + } - ret = ata_pio_transfer(op, 0, blocksize); + ret = ata_pio_transfer(op, 0, blocksize); + } fail: // Enable interrupts -- 1.7.9.1