From c8dcb531bcd37a4a81d2cc08a89fcd19c34348f9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 10 Oct 2012 12:18:03 +0200 Subject: [PATCH 1/8] scsi: do not return short responses for emulated commands The inquiry command, for the case of VPD=1, was returning short responses; the number of returned bytes was just the number of bytes in the request, without padding to the specified allocation length with zero bytes. This is usually harmless, but it is a violation of the SCSI specification. To fix this, always pad with zero bytes to r->cmd.xfer in scsi_disk_emulate_command, and return at most r->buflen bytes (the size of the buffer for command data) rather than at most buflen bytes (the number of bytes that was filled in). Before this patch, "strace sg_inq -p0x83 /dev/sda" would report a non-zero resid value. After this patch, it reports resid=0. Signed-off-by: Paolo Bonzini --- hw/scsi-disk.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 1b0afa6352..098558d372 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -652,7 +652,6 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf) if (buflen > SCSI_MAX_INQUIRY_LEN) { buflen = SCSI_MAX_INQUIRY_LEN; } - memset(outbuf, 0, buflen); outbuf[0] = s->qdev.type & 0x1f; outbuf[1] = (s->features & (1 << SCSI_DISK_F_REMOVABLE)) ? 0x80 : 0; @@ -1596,24 +1595,26 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf) break; } + /* + * FIXME: we shouldn't return anything bigger than 4k, but the code + * requires the buffer to be as big as req->cmd.xfer in several + * places. So, do not allow CDBs with a very large ALLOCATION + * LENGTH. The real fix would be to modify scsi_read_data and + * dma_buf_read, so that they return data beyond the buflen + * as all zeros. + */ + if (req->cmd.xfer > 65536) { + goto illegal_request; + } + r->buflen = MAX(4096, req->cmd.xfer); + if (!r->iov.iov_base) { - /* - * FIXME: we shouldn't return anything bigger than 4k, but the code - * requires the buffer to be as big as req->cmd.xfer in several - * places. So, do not allow CDBs with a very large ALLOCATION - * LENGTH. The real fix would be to modify scsi_read_data and - * dma_buf_read, so that they return data beyond the buflen - * as all zeros. - */ - if (req->cmd.xfer > 65536) { - goto illegal_request; - } - r->buflen = MAX(4096, req->cmd.xfer); r->iov.iov_base = qemu_blockalign(s->qdev.conf.bs, r->buflen); } buflen = req->cmd.xfer; outbuf = r->iov.iov_base; + memset(outbuf, 0, r->buflen); switch (req->cmd.buf[0]) { case TEST_UNIT_READY: assert(!s->tray_open && bdrv_is_inserted(s->qdev.conf.bs)); @@ -1694,12 +1695,14 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf) outbuf[5] = 0; outbuf[6] = s->qdev.blocksize >> 8; outbuf[7] = 0; - buflen = 8; break; case REQUEST_SENSE: /* Just return "NO SENSE". */ buflen = scsi_build_sense(NULL, 0, outbuf, r->buflen, (req->cmd.buf[1] & 1) == 0); + if (buflen < 0) { + goto illegal_request; + } break; case MECHANISM_STATUS: buflen = scsi_emulate_mechanism_status(s, outbuf); @@ -1770,7 +1773,6 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf) } /* Protection, exponent and lowest lba field left blank. */ - buflen = req->cmd.xfer; break; } DPRINTF("Unsupported Service Action In\n"); @@ -1827,7 +1829,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf) return 0; } assert(!r->req.aiocb); - r->iov.iov_len = MIN(buflen, req->cmd.xfer); + r->iov.iov_len = MIN(r->buflen, req->cmd.xfer); if (r->iov.iov_len == 0) { scsi_req_complete(&r->req, GOOD); } From cd41a671b370a3dd603963432d2b02f1e5990fb7 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 8 Oct 2012 16:50:51 +0200 Subject: [PATCH 2/8] virtio-scsi: factor checks for VIRTIO_SCSI_S_DRIVER_OK when reporting events Suggested by Laszlo Ersek. Signed-off-by: Paolo Bonzini --- hw/virtio-scsi.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c index b54c7895fc..30d3f8aca7 100644 --- a/hw/virtio-scsi.c +++ b/hw/virtio-scsi.c @@ -596,6 +596,10 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev, VirtIOSCSIEvent *evt; int in_size; + if (!(s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) { + return; + } + if (!req) { s->events_dropped = true; return; @@ -648,7 +652,6 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense) VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus); if (((s->vdev.guest_features >> VIRTIO_SCSI_F_CHANGE) & 1) && - (s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK) && dev->type != TYPE_ROM) { virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE, sense.asc | (sense.ascq << 8)); @@ -659,8 +662,7 @@ static void virtio_scsi_hotplug(SCSIBus *bus, SCSIDevice *dev) { VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus); - if (((s->vdev.guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) && - (s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) { + if ((s->vdev.guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) { virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_RESCAN); } From b5232e904fadac8af239306719be4a554f9e9263 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 8 Oct 2012 16:46:54 +0200 Subject: [PATCH 3/8] scsi: remove superfluous call to scsi_device_set_ua Suggested by Laszlo Ersek. Signed-off-by: Paolo Bonzini --- hw/scsi-disk.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 098558d372..d15f891974 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1964,7 +1964,6 @@ static void scsi_disk_resize_cb(void *opaque) * direct-access devices. */ if (s->qdev.type == TYPE_DISK) { - scsi_device_set_ua(&s->qdev, SENSE_CODE(CAPACITY_CHANGED)); scsi_device_report_change(&s->qdev, SENSE_CODE(CAPACITY_CHANGED)); } } From 346a3017ec8d5ac4a6961d823f1e576867dc35ef Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 28 Sep 2012 16:43:41 +0200 Subject: [PATCH 4/8] megasas: do not include block_int.h Signed-off-by: Paolo Bonzini --- hw/megasas.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/megasas.c b/hw/megasas.c index 7a2036eb76..b845ea7b48 100644 --- a/hw/megasas.c +++ b/hw/megasas.c @@ -25,7 +25,6 @@ #include "iov.h" #include "scsi.h" #include "scsi-defs.h" -#include "block_int.h" #include "trace.h" #include "mfi.h" From accfeb2dd32ece73350b06cee1b2403f47e86fe3 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 31 Oct 2012 17:14:41 +0100 Subject: [PATCH 5/8] scsi-disk: flush cache after disabling it SBC says that "if an application client changes the WCE bit from one to zero via a MODE SELECT command, then the device server shall write any data in volatile cache to non-volatile medium before completing the command". Signed-off-by: Paolo Bonzini --- hw/scsi-disk.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index d15f891974..49b5686a92 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1387,6 +1387,7 @@ invalid_param_len: static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf) { + SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev); uint8_t *p = inbuf; int cmd = r->req.cmd.buf[0]; int len = r->req.cmd.xfer; @@ -1423,6 +1424,14 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf) return; } } + if (!bdrv_enable_write_cache(s->qdev.conf.bs)) { + /* The request is used as the AIO opaque value, so add a ref. */ + scsi_req_ref(&r->req); + bdrv_acct_start(s->qdev.conf.bs, &r->acct, 0, BDRV_ACCT_FLUSH); + r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_aio_complete, r); + return; + } + scsi_req_complete(&r->req, GOOD); return; From 4003e24fce1df84a2e8c992376ed2c294816c33c Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Mon, 12 Nov 2012 15:42:42 +0100 Subject: [PATCH 6/8] megasas: Correct target/lun mapping The structure to reference a logical drive has an unused field, which can be used to carry the lun ID. This enabled seabios to establish the proper target/LUN mapping. Cc: Paolo Bonzini Cc: Gerd Hoffmann Signed-off-by: Hannes Reinecke Signed-off-by: Paolo Bonzini --- hw/megasas.c | 1 + hw/mfi.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/megasas.c b/hw/megasas.c index b845ea7b48..291ff40403 100644 --- a/hw/megasas.c +++ b/hw/megasas.c @@ -1079,6 +1079,7 @@ static int megasas_dcmd_ld_get_list(MegasasState *s, MegasasCmd *cmd) /* Logical device size is in blocks */ bdrv_get_geometry(conf->bs, &ld_size); info.ld_list[num_ld_disks].ld.v.target_id = sdev->id; + info.ld_list[num_ld_disks].ld.v.lun_id = sdev->lun; info.ld_list[num_ld_disks].state = MFI_LD_STATE_OPTIMAL; info.ld_list[num_ld_disks].size = cpu_to_le64(ld_size); num_ld_disks++; diff --git a/hw/mfi.h b/hw/mfi.h index 436b6906b1..cd8355badf 100644 --- a/hw/mfi.h +++ b/hw/mfi.h @@ -1085,7 +1085,7 @@ struct mfi_pd_list { union mfi_ld_ref { struct { uint8_t target_id; - uint8_t reserved; + uint8_t lun_id; uint16_t seq; } v; uint32_t ref; From 9e11908f12f92e31ea94dc2a4c962c836cba9f2a Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 29 Oct 2012 11:34:32 +1000 Subject: [PATCH 7/8] dma: Define dma_context_memory and use in sysbus-ohci Define a new global dma_context_memory which is a DMAContext corresponding to the global address_space_memory AddressSpace. This can be used by sysbus peripherals like sysbus-ohci which need to do DMA. In particular, use it in the sysbus-ohci device, which fixes a segfault when attempting to use that device. Signed-off-by: Peter Maydell Reviewed-by: Peter Crosthwaite --- dma.h | 5 +++++ exec.c | 5 +++++ hw/usb/hcd-ohci.c | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/dma.h b/dma.h index 91ccdb5eac..eedf878383 100644 --- a/dma.h +++ b/dma.h @@ -68,6 +68,11 @@ struct DMAContext { DMAUnmapFunc *unmap; }; +/* A global DMA context corresponding to the address_space_memory + * AddressSpace, for sysbus devices which do DMA. + */ +extern DMAContext dma_context_memory; + static inline void dma_barrier(DMAContext *dma, DMADirection dir) { /* diff --git a/exec.c b/exec.c index af94f9cd86..8435de0bd2 100644 --- a/exec.c +++ b/exec.c @@ -34,6 +34,7 @@ #include "hw/xen.h" #include "qemu-timer.h" #include "memory.h" +#include "dma.h" #include "exec-memory.h" #if defined(CONFIG_USER_ONLY) #include @@ -103,6 +104,7 @@ static MemoryRegion *system_io; AddressSpace address_space_io; AddressSpace address_space_memory; +DMAContext dma_context_memory; MemoryRegion io_mem_ram, io_mem_rom, io_mem_unassigned, io_mem_notdirty; static MemoryRegion io_mem_subpage_ram; @@ -3294,6 +3296,9 @@ static void memory_map_init(void) memory_listener_register(&core_memory_listener, &address_space_memory); memory_listener_register(&io_memory_listener, &address_space_io); memory_listener_register(&tcg_memory_listener, &address_space_memory); + + dma_context_init(&dma_context_memory, &address_space_memory, + NULL, NULL, NULL); } MemoryRegion *get_system_memory(void) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 7571e9e44a..efae0322ee 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -1851,7 +1851,7 @@ static int ohci_init_pxa(SysBusDevice *dev) /* Cannot fail as we pass NULL for masterbus */ usb_ohci_init(&s->ohci, &dev->qdev, s->num_ports, s->dma_offset, NULL, 0, - NULL); + &dma_context_memory); sysbus_init_irq(dev, &s->ohci.irq); sysbus_init_mmio(dev, &s->ohci.mem); From dd72fdd06268860a24f9f3828efade547ee2e2a9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 30 Oct 2012 17:31:43 +0100 Subject: [PATCH 8/8] virtio-scsi: use dma_context_memory Until address_space_rw was introduced, NULL was accepted as a placeholder for DMA with no IOMMU (to address_space_memory). This does not work anymore, and dma_context_memory needs to be specified explicitly. Signed-off-by: Paolo Bonzini --- hw/virtio-scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c index 30d3f8aca7..7d546f6ca7 100644 --- a/hw/virtio-scsi.c +++ b/hw/virtio-scsi.c @@ -204,7 +204,7 @@ static void virtio_scsi_bad_req(void) static void qemu_sgl_init_external(QEMUSGList *qsgl, struct iovec *sg, hwaddr *addr, int num) { - memset(qsgl, 0, sizeof(*qsgl)); + qemu_sglist_init(qsgl, num, &dma_context_memory); while (num--) { qemu_sglist_add(qsgl, *(addr++), (sg++)->iov_len); }