From e04da7c3d15d746be86898b4ce059b365d61dbc3 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 21 Nov 2011 11:17:59 +0100 Subject: usb-storage: move status debug message to usb_msd_send_status. usb_msd_send_status can be called from different code paths, move the debug message into the function to make sure it is printed unconditionally. Signed-off-by: Gerd Hoffmann --- hw/usb-msd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/usb-msd.c b/hw/usb-msd.c index b734177..8dde421 100644 --- a/hw/usb-msd.c +++ b/hw/usb-msd.c @@ -194,6 +194,8 @@ static void usb_msd_send_status(MSDState *s, USBPacket *p) struct usb_msd_csw csw; int len; + DPRINTF("Command status %d tag 0x%x, len %zd\n", + s->result, s->tag, p->iov.size); csw.sig = cpu_to_le32(0x53425355); csw.tag = cpu_to_le32(s->tag); csw.residue = s->residue; @@ -432,8 +434,6 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p) break; case USB_MSDM_CSW: - DPRINTF("Command status %d tag 0x%x, len %zd\n", - s->result, s->tag, p->iov.size); if (p->iov.size < 13) { goto fail; } -- cgit v1.1 From 92a114f6f8487b8b256340cac5a5a10498e220c1 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 21 Nov 2011 11:29:27 +0100 Subject: usb-storage: fill status in complete callback. Put status word into device state, fill it in command_complete, have usb_msd_send_status just send it out. Signed-off-by: Gerd Hoffmann --- hw/usb-msd.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) (limited to 'hw') diff --git a/hw/usb-msd.c b/hw/usb-msd.c index 8dde421..5dc4afc 100644 --- a/hw/usb-msd.c +++ b/hw/usb-msd.c @@ -38,6 +38,13 @@ enum USBMSDMode { USB_MSDM_CSW /* Command Status. */ }; +struct usb_msd_csw { + uint32_t sig; + uint32_t tag; + uint32_t residue; + uint8_t status; +}; + typedef struct { USBDevice dev; enum USBMSDMode mode; @@ -46,6 +53,7 @@ typedef struct { uint32_t data_len; uint32_t residue; uint32_t tag; + struct usb_msd_csw csw; SCSIRequest *req; SCSIBus bus; BlockConf conf; @@ -67,13 +75,6 @@ struct usb_msd_cbw { uint8_t cmd[16]; }; -struct usb_msd_csw { - uint32_t sig; - uint32_t tag; - uint32_t residue; - uint8_t status; -}; - enum { STR_MANUFACTURER = 1, STR_PRODUCT, @@ -191,19 +192,15 @@ static void usb_msd_copy_data(MSDState *s, USBPacket *p) static void usb_msd_send_status(MSDState *s, USBPacket *p) { - struct usb_msd_csw csw; int len; DPRINTF("Command status %d tag 0x%x, len %zd\n", - s->result, s->tag, p->iov.size); - csw.sig = cpu_to_le32(0x53425355); - csw.tag = cpu_to_le32(s->tag); - csw.residue = s->residue; - csw.status = s->result; - - len = MIN(sizeof(csw), p->iov.size); - usb_packet_copy(p, &csw, len); - p->result = len; + s->csw.status, s->csw.tag, p->iov.size); + + assert(s->csw.sig == 0x53425355); + len = MIN(sizeof(s->csw), p->iov.size); + usb_packet_copy(p, &s->csw, len); + memset(&s->csw, 0, sizeof(s->csw)); } static void usb_msd_transfer_data(SCSIRequest *req, uint32_t len) @@ -236,6 +233,12 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status) DPRINTF("Command complete %d\n", status); s->residue = s->data_len; s->result = status != 0; + + s->csw.sig = cpu_to_le32(0x53425355); + s->csw.tag = cpu_to_le32(s->tag); + s->csw.residue = s->residue; + s->csw.status = s->result; + if (s->packet) { if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) { /* A deferred packet with no write data remaining must be @@ -257,6 +260,7 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status) } else if (s->data_len == 0) { s->mode = USB_MSDM_CSW; } + scsi_req_unref(req); s->req = NULL; } -- cgit v1.1 From 7b863f41de0c0089c2735e3caa8952bb3b0a5a7c Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 21 Nov 2011 11:36:17 +0100 Subject: usb-storage: drop tag from device state. scsi keeps track of the tag in SCSIRequest, no need to store a separate copy. Signed-off-by: Gerd Hoffmann --- hw/usb-msd.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'hw') diff --git a/hw/usb-msd.c b/hw/usb-msd.c index 5dc4afc..089c23c 100644 --- a/hw/usb-msd.c +++ b/hw/usb-msd.c @@ -52,7 +52,6 @@ typedef struct { uint8_t *scsi_buf; uint32_t data_len; uint32_t residue; - uint32_t tag; struct usb_msd_csw csw; SCSIRequest *req; SCSIBus bus; @@ -230,12 +229,12 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status) MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent); USBPacket *p = s->packet; - DPRINTF("Command complete %d\n", status); + DPRINTF("Command complete %d tag 0x%x\n", status, req->tag); s->residue = s->data_len; s->result = status != 0; s->csw.sig = cpu_to_le32(0x53425355); - s->csw.tag = cpu_to_le32(s->tag); + s->csw.tag = cpu_to_le32(req->tag); s->csw.residue = s->residue; s->csw.status = s->result; @@ -260,7 +259,6 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status) } else if (s->data_len == 0) { s->mode = USB_MSDM_CSW; } - scsi_req_unref(req); s->req = NULL; } @@ -340,6 +338,7 @@ static void usb_msd_cancel_io(USBDevice *dev, USBPacket *p) static int usb_msd_handle_data(USBDevice *dev, USBPacket *p) { MSDState *s = (MSDState *)dev; + uint32_t tag; int ret = 0; struct usb_msd_cbw cbw; uint8_t devep = p->devep; @@ -366,7 +365,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p) fprintf(stderr, "usb-msd: Bad LUN %d\n", cbw.lun); goto fail; } - s->tag = le32_to_cpu(cbw.tag); + tag = le32_to_cpu(cbw.tag); s->data_len = le32_to_cpu(cbw.data_len); if (s->data_len == 0) { s->mode = USB_MSDM_CSW; @@ -376,10 +375,10 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p) s->mode = USB_MSDM_DATAOUT; } DPRINTF("Command tag 0x%x flags %08x len %d data %d\n", - s->tag, cbw.flags, cbw.cmd_len, s->data_len); + tag, cbw.flags, cbw.cmd_len, s->data_len); s->residue = 0; s->scsi_len = 0; - s->req = scsi_req_new(s->scsi_dev, s->tag, 0, cbw.cmd, NULL); + s->req = scsi_req_new(s->scsi_dev, tag, 0, cbw.cmd, NULL); scsi_req_enqueue(s->req); /* ??? Should check that USB and SCSI data transfer directions match. */ -- cgit v1.1 From 414c460431036bdadea8120d056c7e493bb60fb9 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 21 Nov 2011 11:41:30 +0100 Subject: usb-storage: drop result from device state. Signed-off-by: Gerd Hoffmann --- hw/usb-msd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'hw') diff --git a/hw/usb-msd.c b/hw/usb-msd.c index 089c23c..6f32a0e 100644 --- a/hw/usb-msd.c +++ b/hw/usb-msd.c @@ -59,7 +59,6 @@ typedef struct { char *serial; SCSIDevice *scsi_dev; uint32_t removable; - int result; /* For async completion. */ USBPacket *packet; } MSDState; @@ -231,12 +230,11 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status) DPRINTF("Command complete %d tag 0x%x\n", status, req->tag); s->residue = s->data_len; - s->result = status != 0; s->csw.sig = cpu_to_le32(0x53425355); s->csw.tag = cpu_to_le32(req->tag); s->csw.residue = s->residue; - s->csw.status = s->result; + s->csw.status = status != 0; if (s->packet) { if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) { -- cgit v1.1 From 59310659073d85745854f2f10c4292555c5a1c51 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 21 Nov 2011 14:01:26 +0100 Subject: usb-storage: don't try to send the status early. Until recently all scsi commands sent to scsi-disk did either transfer data or finished instantly. The correct implementation of SYNCRONIZE_CACHE changed the picture though, and usb-storage needs a fix to handle that case correctly. --- hw/usb-msd.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'hw') diff --git a/hw/usb-msd.c b/hw/usb-msd.c index 6f32a0e..68e3756 100644 --- a/hw/usb-msd.c +++ b/hw/usb-msd.c @@ -378,9 +378,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p) s->scsi_len = 0; s->req = scsi_req_new(s->scsi_dev, tag, 0, cbw.cmd, NULL); scsi_req_enqueue(s->req); - /* ??? Should check that USB and SCSI data transfer - directions match. */ - if (s->mode != USB_MSDM_CSW && s->residue == 0) { + if (s->req && s->req->cmd.xfer != SCSI_XFER_NONE) { scsi_req_continue(s->req); } ret = p->result; @@ -439,9 +437,15 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p) goto fail; } - usb_msd_send_status(s, p); - s->mode = USB_MSDM_CBW; - ret = 13; + if (s->req) { + /* still in flight */ + s->packet = p; + ret = USB_RET_ASYNC; + } else { + usb_msd_send_status(s, p); + s->mode = USB_MSDM_CBW; + ret = 13; + } break; case USB_MSDM_DATAIN: -- cgit v1.1 From b246721614e316ce948d058dbe45702447998b5f Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 9 Nov 2011 12:20:20 +0100 Subject: ehci: add assert Coverity thinks q could be NULL there and warns. I believe it can't be NULL there. Add assert to prove it. Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c | 1 + 1 file changed, 1 insertion(+) (limited to 'hw') diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index cdd5aae..3eea94d 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -2046,6 +2046,7 @@ static void ehci_advance_state(EHCIState *ehci, break; case EST_WRITEBACK: + assert(q != NULL); again = ehci_state_writeback(q, async); break; -- cgit v1.1