From f1a12821d7df2e4d21be4f2206f84b4640533e53 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Thu, 30 Aug 2012 17:28:40 -0700 Subject: [PATCH 1/7] iSCSI: We need to support SG_IO also from iscsi_ioctl() We need to support SG_IO from the synchronous iscsi_ioctl() since scsi-block uses this to do an INQ to the device to discover its properties This patch makes scsi-block work with iscsi. Signed-off-by: Ronnie Sahlberg Signed-off-by: Paolo Bonzini --- block/iscsi.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/block/iscsi.c b/block/iscsi.c index 0b96165ec2..ea1660948d 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -628,9 +628,17 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, return &acb->common; } + +static void ioctl_cb(void *opaque, int status) +{ + int *p_status = opaque; + *p_status = status; +} + static int iscsi_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) { IscsiLun *iscsilun = bs->opaque; + int status; switch (req) { case SG_GET_VERSION_NUM: @@ -639,6 +647,15 @@ static int iscsi_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) case SG_GET_SCSI_ID: ((struct sg_scsi_id *)buf)->scsi_type = iscsilun->type; break; + case SG_IO: + status = -EINPROGRESS; + iscsi_aio_ioctl(bs, req, buf, ioctl_cb, &status); + + while (status == -EINPROGRESS) { + qemu_aio_wait(); + } + + return 0; default: return -1; } From 40a13ca8d28c21062e35b10d9b80e76b92405bdf Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Thu, 30 Aug 2012 16:56:36 -0700 Subject: [PATCH 2/7] iSCSI: We dont need to explicitely call qemu_notify_event() any more We no longer need to explicitely call qemu_notify_event() any more since this is now done automatically any time the filehandles we listen to change. Signed-off-by: Ronnie Sahlberg Signed-off-by: Paolo Bonzini --- block/iscsi.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index ea1660948d..fb001b955c 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -167,12 +167,6 @@ iscsi_set_events(IscsiLun *iscsilun) } - /* If we just added an event, the callback might be delayed - * unless we call qemu_notify_event(). - */ - if (ev & ~iscsilun->events) { - qemu_notify_event(); - } iscsilun->events = ev; } From 444bc908611ccaf4512dc37c33ac3b54d873a62b Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 5 Sep 2012 17:46:18 +0200 Subject: [PATCH 3/7] scsi-disk: introduce check_lba_range Abstract the test for an out-of-range (starting block, block count) pair. Signed-off-by: Paolo Bonzini --- hw/scsi-disk.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 1585683bce..3959603b0f 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1449,6 +1449,18 @@ invalid_field: return; } +static inline bool check_lba_range(SCSIDiskState *s, + uint64_t sector_num, uint32_t nb_sectors) +{ + /* + * The first line tests that no overflow happens when computing the last + * sector. The second line tests that the last accessed sector is in + * range. + */ + return (sector_num <= sector_num + nb_sectors && + sector_num + nb_sectors - 1 <= s->qdev.max_lba); +} + typedef struct UnmapCBData { SCSIDiskReq *r; uint8_t *inbuf; @@ -1473,8 +1485,7 @@ static void scsi_unmap_complete(void *opaque, int ret) if (data->count > 0 && !r->req.io_canceled) { sector_num = ldq_be_p(&data->inbuf[0]); nb_sectors = ldl_be_p(&data->inbuf[8]) & 0xffffffffULL; - if (sector_num > sector_num + nb_sectors || - sector_num + nb_sectors - 1 > s->qdev.max_lba) { + if (!check_lba_range(s, sector_num, nb_sectors)) { scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE)); goto done; } @@ -1802,8 +1813,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf) scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED)); return 0; } - if (r->req.cmd.lba > r->req.cmd.lba + nb_sectors || - r->req.cmd.lba + nb_sectors - 1 > s->qdev.max_lba) { + if (!check_lba_range(s, r->req.cmd.lba, nb_sectors)) { goto illegal_lba; } @@ -1878,8 +1888,7 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf) if (r->req.cmd.buf[1] & 0xe0) { goto illegal_request; } - if (r->req.cmd.lba > r->req.cmd.lba + len || - r->req.cmd.lba + len - 1 > s->qdev.max_lba) { + if (!check_lba_range(s, r->req.cmd.lba, len)) { goto illegal_lba; } r->sector = r->req.cmd.lba * (s->qdev.blocksize / 512); @@ -1907,8 +1916,7 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf) if (r->req.cmd.buf[1] & 0xe0) { goto illegal_request; } - if (r->req.cmd.lba > r->req.cmd.lba + len || - r->req.cmd.lba + len - 1 > s->qdev.max_lba) { + if (!check_lba_range(s, r->req.cmd.lba, len)) { goto illegal_lba; } r->sector = r->req.cmd.lba * (s->qdev.blocksize / 512); From 12ca76fc48081b3a0ad1a70546abfcf198aedfc4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 5 Sep 2012 17:54:36 +0200 Subject: [PATCH 4/7] scsi-disk: fix check for out-of-range LBA This fix is needed to correctly handle 0-block read and writes. Without it, a 0-block access at LBA 0 would underflow. Signed-off-by: Paolo Bonzini --- hw/scsi-disk.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 3959603b0f..d621852857 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1456,9 +1456,13 @@ static inline bool check_lba_range(SCSIDiskState *s, * The first line tests that no overflow happens when computing the last * sector. The second line tests that the last accessed sector is in * range. + * + * Careful, the computations should not underflow for nb_sectors == 0, + * and a 0-block read to the first LBA beyond the end of device is + * valid. */ return (sector_num <= sector_num + nb_sectors && - sector_num + nb_sectors - 1 <= s->qdev.max_lba); + sector_num + nb_sectors <= s->qdev.max_lba + 1); } typedef struct UnmapCBData { From bb729f758195a36db1dd0d5c01ec983e466729eb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 5 Sep 2012 17:57:19 +0200 Subject: [PATCH 5/7] scsi: introduce scsi_cdb_length and scsi_data_cdb_length Signed-off-by: Paolo Bonzini --- hw/scsi-bus.c | 23 ++++++++++++++++++----- hw/scsi.h | 2 ++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index 4981a02436..058d3b237f 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -801,26 +801,39 @@ static int ata_passthrough_16_xfer_size(SCSIDevice *dev, uint8_t *buf) return xfer * unit; } -static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) +uint32_t scsi_data_cdb_length(uint8_t *buf) +{ + if ((buf[0] >> 5) == 0 && buf[4] == 0) { + return 256; + } else { + return scsi_cdb_length(buf); + } +} + +uint32_t scsi_cdb_length(uint8_t *buf) { switch (buf[0] >> 5) { case 0: - cmd->xfer = buf[4]; + return buf[4]; break; case 1: case 2: - cmd->xfer = lduw_be_p(&buf[7]); + return lduw_be_p(&buf[7]); break; case 4: - cmd->xfer = ldl_be_p(&buf[10]) & 0xffffffffULL; + return ldl_be_p(&buf[10]) & 0xffffffffULL; break; case 5: - cmd->xfer = ldl_be_p(&buf[6]) & 0xffffffffULL; + return ldl_be_p(&buf[6]) & 0xffffffffULL; break; default: return -1; } +} +static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) +{ + cmd->xfer = scsi_cdb_length(buf); switch (buf[0]) { case TEST_UNIT_READY: case REWIND: diff --git a/hw/scsi.h b/hw/scsi.h index 1aeee4659c..b8f73577d3 100644 --- a/hw/scsi.h +++ b/hw/scsi.h @@ -218,6 +218,8 @@ extern const struct SCSISense sense_code_WRITE_PROTECTED; #define SENSE_CODE(x) sense_code_ ## x +uint32_t scsi_data_cdb_length(uint8_t *buf); +uint32_t scsi_cdb_length(uint8_t *buf); int scsi_sense_valid(SCSISense sense); int scsi_build_sense(uint8_t *in_buf, int in_len, uint8_t *buf, int len, bool fixed); From e93176d55f1eb4be1a366b51afeaf4f4c8c31d75 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 5 Sep 2012 18:00:57 +0200 Subject: [PATCH 6/7] scsi-disk: use scsi_data_cdb_length This simplifies and unifies the parsing of READ, WRITE and WRITE SAME commands. Signed-off-by: Paolo Bonzini --- hw/scsi-disk.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index d621852857..4ffca7aae5 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1808,11 +1808,8 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf) DPRINTF("Unmap (len %lu)\n", (long)r->req.cmd.xfer); break; case WRITE_SAME_10: - nb_sectors = lduw_be_p(&req->cmd.buf[7]); - goto write_same; case WRITE_SAME_16: - nb_sectors = ldl_be_p(&req->cmd.buf[10]) & 0xffffffffULL; - write_same: + nb_sectors = scsi_data_cdb_length(r->req.cmd.buf); if (bdrv_is_read_only(s->qdev.conf.bs)) { scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED)); return 0; @@ -1872,7 +1869,7 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf) { SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req); SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev); - int32_t len; + uint32_t len; uint8_t command; command = buf[0]; @@ -1882,13 +1879,13 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf) return 0; } + len = scsi_data_cdb_length(r->req.cmd.buf); switch (command) { case READ_6: case READ_10: case READ_12: case READ_16: - len = r->req.cmd.xfer / s->qdev.blocksize; - DPRINTF("Read (sector %" PRId64 ", count %d)\n", r->req.cmd.lba, len); + DPRINTF("Read (sector %" PRId64 ", count %u)\n", r->req.cmd.lba, len); if (r->req.cmd.buf[1] & 0xe0) { goto illegal_request; } @@ -1913,8 +1910,7 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf) case VERIFY_10: case VERIFY_12: case VERIFY_16: - len = r->req.cmd.xfer / s->qdev.blocksize; - DPRINTF("Write %s(sector %" PRId64 ", count %d)\n", + DPRINTF("Write %s(sector %" PRId64 ", count %u)\n", (command & 0xe) == 0xe ? "And Verify " : "", r->req.cmd.lba, len); if (r->req.cmd.buf[1] & 0xe0) { From 1109c894052751df99962c009fd7dbae397721f5 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Fri, 14 Sep 2012 18:13:29 -0700 Subject: [PATCH 7/7] SCSI: Standard INQUIRY data should report HiSup flag as set. QEMU as far as I know only reports LUN numbers using the modes that are described in SAM4. As such, since all LUN numbers generated by the SCSI emulation in QEMU follow SAM4, we should set the HiSup bit in the standard INQUIRY data to indicate such. From SAM4: 4.6.3 LUNs overview All LUN formats described in this standard are hierarchical in structure even when only a single level in that hierarchy is used. The HISUP bit shall be set to one in the standard INQUIRY data (see SPC-4) when any LUN format described in this standard is used. Non-hierarchical formats are outside the scope of this standard. Signed-off-by: Ronnie Sahlberg --- hw/scsi-disk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 4ffca7aae5..95e91585e6 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -678,7 +678,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf) * is actually implemented, but we're good enough. */ outbuf[2] = 5; - outbuf[3] = 2; /* Format 2 */ + outbuf[3] = 2 | 0x10; /* Format 2, HiSup */ if (buflen > 36) { outbuf[4] = buflen - 5; /* Additional Length = (Len - 1) - 4 */