From de9de157fbb9aa66380ab1973dd6ecf12fbd8b25 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 19 Nov 2013 14:36:59 +0100 Subject: xhci: Add a few missing checks for disconnected devices One of the reworks of qemu's usb core made changes to usb-port's disconnect handling. Now ports with a device will always have a non 0 dev member, but if the device is not attached (which is possible with usb redirection), dev->attached will be 0. So supplement all checks for dev to also check dev->attached, and add an extra check in a path where a device check was completely missing. This fixes various crashes (asserts triggering) I've been seeing when xhci attached usb devices get disconnected at the wrong time. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'hw/usb') diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 835f65ed81..c3377eebea 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -1495,7 +1495,8 @@ static TRBCCode xhci_reset_ep(XHCIState *xhci, unsigned int slotid, } if (!xhci->slots[slotid-1].uport || - !xhci->slots[slotid-1].uport->dev) { + !xhci->slots[slotid-1].uport->dev || + !xhci->slots[slotid-1].uport->dev->attached) { return CC_USB_TRANSACTION_ERROR; } @@ -1982,6 +1983,14 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, return; } + /* If the device has been detached, but the guest has not noticed this + yet the 2 above checks will succeed, but we must NOT continue */ + if (!xhci->slots[slotid - 1].uport || + !xhci->slots[slotid - 1].uport->dev || + !xhci->slots[slotid - 1].uport->dev->attached) { + return; + } + if (epctx->retry) { XHCITransfer *xfer = epctx->retry; @@ -2206,7 +2215,7 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid, trace_usb_xhci_slot_address(slotid, uport->path); dev = uport->dev; - if (!dev) { + if (!dev || !dev->attached) { fprintf(stderr, "xhci: port %s not connected\n", uport->path); return CC_USB_TRANSACTION_ERROR; } -- cgit v1.2.1 From f1f8bc218a422081f36f0b325b3de5e6a5078b74 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 8 Nov 2013 11:43:20 +0100 Subject: xhci: add support for suspend/resume The OS can ask the xhci controller to save and restore its internal state, which is used by the OS when the system is suspended and resumed. This patch handles writes to the save + restore bits in the command register. Only thing it does is updating the restore error bit in the status register to signal an error on restore. The guest OS should do a full reinitialization after resume then. This is the minimal patch which gets S3 going with xhci. Implementing full save/restore support is TBD. https://bugzilla.redhat.com/show_bug.cgi?id=1012365 Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'hw/usb') diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index c3377eebea..220e99dc35 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -3025,6 +3025,14 @@ static void xhci_oper_write(void *ptr, hwaddr reg, } else if (!(val & USBCMD_RS) && (xhci->usbcmd & USBCMD_RS)) { xhci_stop(xhci); } + if (val & USBCMD_CSS) { + /* save state */ + xhci->usbsts &= ~USBSTS_SRE; + } + if (val & USBCMD_CRS) { + /* restore state */ + xhci->usbsts |= USBSTS_SRE; + } xhci->usbcmd = val & 0xc0f; xhci_mfwrap_update(xhci); if (val & USBCMD_HCRST) { -- cgit v1.2.1 From d4bfc7b9f35da7dcc28ae87de503b28bd7ff56ee Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 24 Oct 2013 18:15:50 +0100 Subject: uas: Only use report iu-s for task_mgmt status reporting Regular scsi cmds should always report their status using a sense-iu, using the sense code to report any errors. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/dev-uas.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) (limited to 'hw/usb') diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c index 70ed2d1dbd..36a75b2f49 100644 --- a/hw/usb/dev-uas.c +++ b/hw/usb/dev-uas.c @@ -420,6 +420,24 @@ static void usb_uas_queue_sense(UASRequest *req, uint8_t status) usb_uas_queue_status(req->uas, st, len); } +static void usb_uas_queue_fake_sense(UASDevice *uas, uint16_t tag, + struct SCSISense sense) +{ + UASStatus *st = usb_uas_alloc_status(uas, UAS_UI_SENSE, tag); + int len, slen = 0; + + st->status.sense.status = CHECK_CONDITION; + st->status.sense.status_qualifier = cpu_to_be16(0); + st->status.sense.sense_data[0] = 0x70; + st->status.sense.sense_data[2] = sense.key; + st->status.sense.sense_data[7] = 10; + st->status.sense.sense_data[12] = sense.asc; + st->status.sense.sense_data[13] = sense.ascq; + slen = 18; + len = sizeof(uas_ui_sense) - sizeof(st->status.sense.sense_data) + slen; + usb_uas_queue_status(uas, st, len); +} + static void usb_uas_queue_read_ready(UASRequest *req) { UASStatus *st = usb_uas_alloc_status(req->uas, UAS_UI_READ_READY, @@ -672,8 +690,9 @@ static void usb_uas_command(UASDevice *uas, uas_ui *ui) { UASRequest *req; uint32_t len; + uint16_t tag = be16_to_cpu(ui->hdr.tag); - req = usb_uas_find_request(uas, be16_to_cpu(ui->hdr.tag)); + req = usb_uas_find_request(uas, tag); if (req) { goto overlapped_tag; } @@ -706,16 +725,11 @@ static void usb_uas_command(UASDevice *uas, uas_ui *ui) return; overlapped_tag: - usb_uas_queue_response(uas, req->tag, UAS_RC_OVERLAPPED_TAG, 0); + usb_uas_queue_fake_sense(uas, tag, sense_code_OVERLAPPED_COMMANDS); return; bad_target: - /* - * FIXME: Seems to upset linux, is this wrong? - * NOTE: Happens only with no scsi devices at the bus, not sure - * this is a valid UAS setup in the first place. - */ - usb_uas_queue_response(uas, req->tag, UAS_RC_INVALID_INFO_UNIT, 0); + usb_uas_queue_fake_sense(uas, tag, sense_code_LUN_NOT_SUPPORTED); g_free(req); } -- cgit v1.2.1 From 5eb6d9e3ef1fac096ab5b3f5c14e1f4079dd7367 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 24 Oct 2013 18:15:51 +0100 Subject: uas: Fix / cleanup usb_uas_task error handling -The correct error if we cannot find the dev is INCORRECT_LUN rather then INVALID_INFO_UNIT -Move the device not found check to the top so we only need to do it once -Remove the dev->lun != lun checks, dev is returned by scsi_device_find which searches by lun, so this will never trigger Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/dev-uas.c | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) (limited to 'hw/usb') diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c index 36a75b2f49..12d79ef743 100644 --- a/hw/usb/dev-uas.c +++ b/hw/usb/dev-uas.c @@ -746,17 +746,14 @@ static void usb_uas_task(UASDevice *uas, uas_ui *ui) if (req) { goto overlapped_tag; } + if (dev == NULL) { + goto incorrect_lun; + } switch (ui->task.function) { case UAS_TMF_ABORT_TASK: task_tag = be16_to_cpu(ui->task.task_tag); trace_usb_uas_tmf_abort_task(uas->dev.addr, tag, task_tag); - if (dev == NULL) { - goto bad_target; - } - if (dev->lun != lun) { - goto incorrect_lun; - } req = usb_uas_find_request(uas, task_tag); if (req && req->dev == dev) { scsi_req_cancel(req->req); @@ -766,12 +763,6 @@ static void usb_uas_task(UASDevice *uas, uas_ui *ui) case UAS_TMF_LOGICAL_UNIT_RESET: trace_usb_uas_tmf_logical_unit_reset(uas->dev.addr, tag, lun); - if (dev == NULL) { - goto bad_target; - } - if (dev->lun != lun) { - goto incorrect_lun; - } qdev_reset_all(&dev->qdev); usb_uas_queue_response(uas, tag, UAS_RC_TMF_COMPLETE, 0); break; @@ -787,11 +778,6 @@ overlapped_tag: usb_uas_queue_response(uas, req->tag, UAS_RC_OVERLAPPED_TAG, 0); return; -bad_target: - /* FIXME: correct? [see long comment in usb_uas_command()] */ - usb_uas_queue_response(uas, tag, UAS_RC_INVALID_INFO_UNIT, 0); - return; - incorrect_lun: usb_uas_queue_response(uas, tag, UAS_RC_INCORRECT_LUN, 0); } -- cgit v1.2.1 From 0478661ec5f949f16a70687b348c0fb2a56cc537 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 24 Oct 2013 18:15:52 +0100 Subject: uas: Streams are numbered 1-y, rather then 0-x It is easier to simply make the arrays one larger, rather then substracting one everywhere. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/dev-uas.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'hw/usb') diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c index 12d79ef743..70f41d390a 100644 --- a/hw/usb/dev-uas.c +++ b/hw/usb/dev-uas.c @@ -122,8 +122,8 @@ struct UASDevice { UASRequest *dataout2; /* usb 3.0 only */ - USBPacket *data3[UAS_MAX_STREAMS]; - USBPacket *status3[UAS_MAX_STREAMS]; + USBPacket *data3[UAS_MAX_STREAMS + 1]; + USBPacket *status3[UAS_MAX_STREAMS + 1]; }; struct UASRequest { @@ -666,7 +666,7 @@ static void usb_uas_cancel_io(USBDevice *dev, USBPacket *p) return; } if (uas_using_streams(uas)) { - for (i = 0; i < UAS_MAX_STREAMS; i++) { + for (i = 0; i <= UAS_MAX_STREAMS; i++) { if (uas->status3[i] == p) { uas->status3[i] = NULL; return; -- cgit v1.2.1 From 3453f9a0dfa58578e6dadf0905ff4528b428ec73 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 24 Oct 2013 18:15:53 +0100 Subject: uas: Bounds check tags when using streams Disallow the guest to cause us to address the data3 and status3 arrays out of bounds. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/dev-uas.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'hw/usb') diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c index 70f41d390a..5884035df3 100644 --- a/hw/usb/dev-uas.c +++ b/hw/usb/dev-uas.c @@ -692,6 +692,9 @@ static void usb_uas_command(UASDevice *uas, uas_ui *ui) uint32_t len; uint16_t tag = be16_to_cpu(ui->hdr.tag); + if (uas_using_streams(uas) && tag > UAS_MAX_STREAMS) { + goto invalid_tag; + } req = usb_uas_find_request(uas, tag); if (req) { goto overlapped_tag; @@ -724,6 +727,10 @@ static void usb_uas_command(UASDevice *uas, uas_ui *ui) } return; +invalid_tag: + usb_uas_queue_fake_sense(uas, tag, sense_code_INVALID_TAG); + return; + overlapped_tag: usb_uas_queue_fake_sense(uas, tag, sense_code_OVERLAPPED_COMMANDS); return; @@ -742,6 +749,9 @@ static void usb_uas_task(UASDevice *uas, uas_ui *ui) UASRequest *req; uint16_t task_tag; + if (uas_using_streams(uas) && tag > UAS_MAX_STREAMS) { + goto invalid_tag; + } req = usb_uas_find_request(uas, be16_to_cpu(ui->hdr.tag)); if (req) { goto overlapped_tag; @@ -774,6 +784,10 @@ static void usb_uas_task(UASDevice *uas, uas_ui *ui) } return; +invalid_tag: + usb_uas_queue_response(uas, tag, UAS_RC_INVALID_INFO_UNIT, 0); + return; + overlapped_tag: usb_uas_queue_response(uas, req->tag, UAS_RC_OVERLAPPED_TAG, 0); return; -- cgit v1.2.1 From 49cfa2fdc92be2cdd01b9fba846cd52aea1f7f63 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 31 Oct 2013 10:35:31 +0100 Subject: uas: Fix response iu struct definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch mirrors a patch to the Linux uas kernel driver which I've just submitted. It looks like the qemu uas struct definitions were taken from the Linux kernel driver, and have inherited the same mistake. Besides fixing the response iu struct, the patch also drops the add_info parameter from the usb_uas_queue_response() function, it is always 0 anyways, and expressing 3 zero-bytes as a function argument is a bit hard. Below is the long explanation for this change taken from the kernel commit: The response iu struct before this patch has a size of 7 bytes, which is weird since all other iu-s are explictly padded to a multiple of 4 bytes. Submitting a 7 byte bulk transfer to the status endpoint of a real uasp device when expecting a response iu results in an USB babble error, as the device actually sends 8 bytes. Up on closer reading of the UAS spec: http://www.t10.org/cgi-bin/ac.pl?t=f&f=uas2r00.pdf The reason for this becomes clear, the 2 entries in "Table 17 — RESPONSE IU" are numbered 4 and 6, looking at other iu definitions in the spec, esp. multi-byte fields, this indicates that the ADDITIONAL RESPONSE INFORMATION field is not a 2 byte field as one might assume at a first look, but is a multi-byte field containing 3 bytes. This also aligns with the SCSI Architecture Model 4 spec, which UAS is based on which states in paragraph "7.1 Task management function procedure calls" that the "Additional Response Information" output argument for a Task management function procedure call is 3 bytes. Last but not least I've verified this by sending a logical unit reset task management call with an invalid lun to an actual uasp device, and received back a response-iu with byte 6 being 0, and byte 7 being 9, which is the responce code for an invalid iu, which confirms that the response code is being reported in byte 7 of the response iu rather then in byte 6. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/dev-uas.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'hw/usb') diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c index 5884035df3..82a47beee5 100644 --- a/hw/usb/dev-uas.c +++ b/hw/usb/dev-uas.c @@ -76,7 +76,7 @@ typedef struct { } QEMU_PACKED uas_ui_sense; typedef struct { - uint16_t add_response_info; + uint8_t add_response_info[3]; uint8_t response_code; } QEMU_PACKED uas_ui_response; @@ -392,14 +392,12 @@ static void usb_uas_queue_status(UASDevice *uas, UASStatus *st, int length) } } -static void usb_uas_queue_response(UASDevice *uas, uint16_t tag, - uint8_t code, uint16_t add_info) +static void usb_uas_queue_response(UASDevice *uas, uint16_t tag, uint8_t code) { UASStatus *st = usb_uas_alloc_status(uas, UAS_UI_RESPONSE, tag); trace_usb_uas_response(uas->dev.addr, tag, code); st->status.response.response_code = code; - st->status.response.add_response_info = cpu_to_be16(add_info); usb_uas_queue_status(uas, st, sizeof(uas_ui_response)); } @@ -768,32 +766,32 @@ static void usb_uas_task(UASDevice *uas, uas_ui *ui) if (req && req->dev == dev) { scsi_req_cancel(req->req); } - usb_uas_queue_response(uas, tag, UAS_RC_TMF_COMPLETE, 0); + usb_uas_queue_response(uas, tag, UAS_RC_TMF_COMPLETE); break; case UAS_TMF_LOGICAL_UNIT_RESET: trace_usb_uas_tmf_logical_unit_reset(uas->dev.addr, tag, lun); qdev_reset_all(&dev->qdev); - usb_uas_queue_response(uas, tag, UAS_RC_TMF_COMPLETE, 0); + usb_uas_queue_response(uas, tag, UAS_RC_TMF_COMPLETE); break; default: trace_usb_uas_tmf_unsupported(uas->dev.addr, tag, ui->task.function); - usb_uas_queue_response(uas, tag, UAS_RC_TMF_NOT_SUPPORTED, 0); + usb_uas_queue_response(uas, tag, UAS_RC_TMF_NOT_SUPPORTED); break; } return; invalid_tag: - usb_uas_queue_response(uas, tag, UAS_RC_INVALID_INFO_UNIT, 0); + usb_uas_queue_response(uas, tag, UAS_RC_INVALID_INFO_UNIT); return; overlapped_tag: - usb_uas_queue_response(uas, req->tag, UAS_RC_OVERLAPPED_TAG, 0); + usb_uas_queue_response(uas, req->tag, UAS_RC_OVERLAPPED_TAG); return; incorrect_lun: - usb_uas_queue_response(uas, tag, UAS_RC_INCORRECT_LUN, 0); + usb_uas_queue_response(uas, tag, UAS_RC_INCORRECT_LUN); } static void usb_uas_handle_data(USBDevice *dev, USBPacket *p) -- cgit v1.2.1 From 5007c940a9c96ad974573915192424ba00b49932 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 19 Nov 2013 14:37:04 +0100 Subject: uas: s/ui/iu/ The various uas data structures are called IU-s, which is short for Information Unit, rather then UI-s. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/dev-uas.c | 76 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 38 insertions(+), 38 deletions(-) (limited to 'hw/usb') diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c index 82a47beee5..997b715952 100644 --- a/hw/usb/dev-uas.c +++ b/hw/usb/dev-uas.c @@ -55,7 +55,7 @@ typedef struct { uint8_t id; uint8_t reserved; uint16_t tag; -} QEMU_PACKED uas_ui_header; +} QEMU_PACKED uas_iu_header; typedef struct { uint8_t prio_taskattr; /* 6:3 priority, 2:0 task attribute */ @@ -65,7 +65,7 @@ typedef struct { uint64_t lun; uint8_t cdb[16]; uint8_t add_cdb[]; -} QEMU_PACKED uas_ui_command; +} QEMU_PACKED uas_iu_command; typedef struct { uint16_t status_qualifier; @@ -73,29 +73,29 @@ typedef struct { uint8_t reserved[7]; uint16_t sense_length; uint8_t sense_data[18]; -} QEMU_PACKED uas_ui_sense; +} QEMU_PACKED uas_iu_sense; typedef struct { uint8_t add_response_info[3]; uint8_t response_code; -} QEMU_PACKED uas_ui_response; +} QEMU_PACKED uas_iu_response; typedef struct { uint8_t function; uint8_t reserved; uint16_t task_tag; uint64_t lun; -} QEMU_PACKED uas_ui_task_mgmt; +} QEMU_PACKED uas_iu_task_mgmt; typedef struct { - uas_ui_header hdr; + uas_iu_header hdr; union { - uas_ui_command command; - uas_ui_sense sense; - uas_ui_task_mgmt task; - uas_ui_response response; + uas_iu_command command; + uas_iu_sense sense; + uas_iu_task_mgmt task; + uas_iu_response response; }; -} QEMU_PACKED uas_ui; +} QEMU_PACKED uas_iu; /* --------------------------------------------------------------------- */ @@ -145,7 +145,7 @@ struct UASRequest { struct UASStatus { uint32_t stream; - uas_ui status; + uas_iu status; uint32_t length; QTAILQ_ENTRY(UASStatus) next; }; @@ -338,7 +338,7 @@ static UASStatus *usb_uas_alloc_status(UASDevice *uas, uint8_t id, uint16_t tag) st->status.hdr.id = id; st->status.hdr.tag = cpu_to_be16(tag); - st->length = sizeof(uas_ui_header); + st->length = sizeof(uas_iu_header); if (uas_using_streams(uas)) { st->stream = tag; } @@ -398,7 +398,7 @@ static void usb_uas_queue_response(UASDevice *uas, uint16_t tag, uint8_t code) trace_usb_uas_response(uas->dev.addr, tag, code); st->status.response.response_code = code; - usb_uas_queue_status(uas, st, sizeof(uas_ui_response)); + usb_uas_queue_status(uas, st, sizeof(uas_iu_response)); } static void usb_uas_queue_sense(UASRequest *req, uint8_t status) @@ -414,7 +414,7 @@ static void usb_uas_queue_sense(UASRequest *req, uint8_t status) sizeof(st->status.sense.sense_data)); st->status.sense.sense_length = cpu_to_be16(slen); } - len = sizeof(uas_ui_sense) - sizeof(st->status.sense.sense_data) + slen; + len = sizeof(uas_iu_sense) - sizeof(st->status.sense.sense_data) + slen; usb_uas_queue_status(req->uas, st, len); } @@ -432,7 +432,7 @@ static void usb_uas_queue_fake_sense(UASDevice *uas, uint16_t tag, st->status.sense.sense_data[12] = sense.asc; st->status.sense.sense_data[13] = sense.ascq; slen = 18; - len = sizeof(uas_ui_sense) - sizeof(st->status.sense.sense_data) + slen; + len = sizeof(uas_iu_sense) - sizeof(st->status.sense.sense_data) + slen; usb_uas_queue_status(uas, st, len); } @@ -534,14 +534,14 @@ static void usb_uas_start_next_transfer(UASDevice *uas) } } -static UASRequest *usb_uas_alloc_request(UASDevice *uas, uas_ui *ui) +static UASRequest *usb_uas_alloc_request(UASDevice *uas, uas_iu *iu) { UASRequest *req; req = g_new0(UASRequest, 1); req->uas = uas; - req->tag = be16_to_cpu(ui->hdr.tag); - req->lun = be64_to_cpu(ui->command.lun); + req->tag = be16_to_cpu(iu->hdr.tag); + req->lun = be64_to_cpu(iu->command.lun); req->dev = usb_uas_get_dev(req->uas, req->lun); return req; } @@ -684,11 +684,11 @@ static void usb_uas_cancel_io(USBDevice *dev, USBPacket *p) assert(!"canceled usb packet not found"); } -static void usb_uas_command(UASDevice *uas, uas_ui *ui) +static void usb_uas_command(UASDevice *uas, uas_iu *iu) { UASRequest *req; uint32_t len; - uint16_t tag = be16_to_cpu(ui->hdr.tag); + uint16_t tag = be16_to_cpu(iu->hdr.tag); if (uas_using_streams(uas) && tag > UAS_MAX_STREAMS) { goto invalid_tag; @@ -697,7 +697,7 @@ static void usb_uas_command(UASDevice *uas, uas_ui *ui) if (req) { goto overlapped_tag; } - req = usb_uas_alloc_request(uas, ui); + req = usb_uas_alloc_request(uas, iu); if (req->dev == NULL) { goto bad_target; } @@ -714,7 +714,7 @@ static void usb_uas_command(UASDevice *uas, uas_ui *ui) req->req = scsi_req_new(req->dev, req->tag, usb_uas_get_lun(req->lun), - ui->command.cdb, req); + iu->command.cdb, req); if (uas->requestlog) { scsi_req_print(req->req); } @@ -738,10 +738,10 @@ bad_target: g_free(req); } -static void usb_uas_task(UASDevice *uas, uas_ui *ui) +static void usb_uas_task(UASDevice *uas, uas_iu *iu) { - uint16_t tag = be16_to_cpu(ui->hdr.tag); - uint64_t lun64 = be64_to_cpu(ui->task.lun); + uint16_t tag = be16_to_cpu(iu->hdr.tag); + uint64_t lun64 = be64_to_cpu(iu->task.lun); SCSIDevice *dev = usb_uas_get_dev(uas, lun64); int lun = usb_uas_get_lun(lun64); UASRequest *req; @@ -750,7 +750,7 @@ static void usb_uas_task(UASDevice *uas, uas_ui *ui) if (uas_using_streams(uas) && tag > UAS_MAX_STREAMS) { goto invalid_tag; } - req = usb_uas_find_request(uas, be16_to_cpu(ui->hdr.tag)); + req = usb_uas_find_request(uas, be16_to_cpu(iu->hdr.tag)); if (req) { goto overlapped_tag; } @@ -758,9 +758,9 @@ static void usb_uas_task(UASDevice *uas, uas_ui *ui) goto incorrect_lun; } - switch (ui->task.function) { + switch (iu->task.function) { case UAS_TMF_ABORT_TASK: - task_tag = be16_to_cpu(ui->task.task_tag); + task_tag = be16_to_cpu(iu->task.task_tag); trace_usb_uas_tmf_abort_task(uas->dev.addr, tag, task_tag); req = usb_uas_find_request(uas, task_tag); if (req && req->dev == dev) { @@ -776,7 +776,7 @@ static void usb_uas_task(UASDevice *uas, uas_ui *ui) break; default: - trace_usb_uas_tmf_unsupported(uas->dev.addr, tag, ui->task.function); + trace_usb_uas_tmf_unsupported(uas->dev.addr, tag, iu->task.function); usb_uas_queue_response(uas, tag, UAS_RC_TMF_NOT_SUPPORTED); break; } @@ -797,25 +797,25 @@ incorrect_lun: static void usb_uas_handle_data(USBDevice *dev, USBPacket *p) { UASDevice *uas = DO_UPCAST(UASDevice, dev, dev); - uas_ui ui; + uas_iu iu; UASStatus *st; UASRequest *req; int length; switch (p->ep->nr) { case UAS_PIPE_ID_COMMAND: - length = MIN(sizeof(ui), p->iov.size); - usb_packet_copy(p, &ui, length); - switch (ui.hdr.id) { + length = MIN(sizeof(iu), p->iov.size); + usb_packet_copy(p, &iu, length); + switch (iu.hdr.id) { case UAS_UI_COMMAND: - usb_uas_command(uas, &ui); + usb_uas_command(uas, &iu); break; case UAS_UI_TASK_MGMT: - usb_uas_task(uas, &ui); + usb_uas_task(uas, &iu); break; default: - fprintf(stderr, "%s: unknown command ui: id 0x%x\n", - __func__, ui.hdr.id); + fprintf(stderr, "%s: unknown command iu: id 0x%x\n", + __func__, iu.hdr.id); p->status = USB_RET_STALL; break; } -- cgit v1.2.1 From 04b300f85fafd2d6a92ec0a766f0035e9bc5835c Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 19 Nov 2013 14:36:56 +0100 Subject: usb: Add max_streams attribute to endpoint info Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/core.c | 22 ++++++++++++++++++++++ hw/usb/desc.c | 2 ++ 2 files changed, 24 insertions(+) (limited to 'hw/usb') diff --git a/hw/usb/core.c b/hw/usb/core.c index cf59a1abcf..67ba7d6018 100644 --- a/hw/usb/core.c +++ b/hw/usb/core.c @@ -623,6 +623,7 @@ void usb_ep_reset(USBDevice *dev) dev->ep_ctl.type = USB_ENDPOINT_XFER_CONTROL; dev->ep_ctl.ifnum = 0; dev->ep_ctl.max_packet_size = 64; + dev->ep_ctl.max_streams = 0; dev->ep_ctl.dev = dev; dev->ep_ctl.pipeline = false; for (ep = 0; ep < USB_MAX_ENDPOINTS; ep++) { @@ -636,6 +637,8 @@ void usb_ep_reset(USBDevice *dev) dev->ep_out[ep].ifnum = USB_INTERFACE_INVALID; dev->ep_in[ep].max_packet_size = 0; dev->ep_out[ep].max_packet_size = 0; + dev->ep_in[ep].max_streams = 0; + dev->ep_out[ep].max_streams = 0; dev->ep_in[ep].dev = dev; dev->ep_out[ep].dev = dev; dev->ep_in[ep].pipeline = false; @@ -764,6 +767,25 @@ int usb_ep_get_max_packet_size(USBDevice *dev, int pid, int ep) return uep->max_packet_size; } +void usb_ep_set_max_streams(USBDevice *dev, int pid, int ep, uint8_t raw) +{ + struct USBEndpoint *uep = usb_ep_get(dev, pid, ep); + int MaxStreams; + + MaxStreams = raw & 0x1f; + if (MaxStreams) { + uep->max_streams = 1 << MaxStreams; + } else { + uep->max_streams = 0; + } +} + +int usb_ep_get_max_streams(USBDevice *dev, int pid, int ep) +{ + struct USBEndpoint *uep = usb_ep_get(dev, pid, ep); + return uep->max_streams; +} + void usb_ep_set_pipeline(USBDevice *dev, int pid, int ep, bool enabled) { struct USBEndpoint *uep = usb_ep_get(dev, pid, ep); diff --git a/hw/usb/desc.c b/hw/usb/desc.c index bf6c522682..5dbe7545f5 100644 --- a/hw/usb/desc.c +++ b/hw/usb/desc.c @@ -385,6 +385,8 @@ static void usb_desc_ep_init(USBDevice *dev) usb_ep_set_ifnum(dev, pid, ep, iface->bInterfaceNumber); usb_ep_set_max_packet_size(dev, pid, ep, iface->eps[e].wMaxPacketSize); + usb_ep_set_max_streams(dev, pid, ep, + iface->eps[e].bmAttributes_super); } } } -- cgit v1.2.1 From 3b444eadf7726d976be4ac89e8e742cb7eb7a5ee Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 19 Nov 2013 14:36:57 +0100 Subject: usb: Add usb_device_alloc/free_streams Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/bus.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'hw/usb') diff --git a/hw/usb/bus.c b/hw/usb/bus.c index ca329bef29..09848c6320 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -203,6 +203,24 @@ void usb_device_ep_stopped(USBDevice *dev, USBEndpoint *ep) } } +int usb_device_alloc_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps, + int streams) +{ + USBDeviceClass *klass = USB_DEVICE_GET_CLASS(dev); + if (klass->alloc_streams) { + return klass->alloc_streams(dev, eps, nr_eps, streams); + } + return 0; +} + +void usb_device_free_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps) +{ + USBDeviceClass *klass = USB_DEVICE_GET_CLASS(dev); + if (klass->free_streams) { + klass->free_streams(dev, eps, nr_eps); + } +} + static int usb_qdev_init(DeviceState *qdev) { USBDevice *dev = USB_DEVICE(qdev); -- cgit v1.2.1 From 72391da50621c9f11bb8c57193ab2d1ad8bc5ad8 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 19 Nov 2013 14:36:58 +0100 Subject: xhci: Call usb_device_alloc/free_streams Note this code is not as KISS as I would like, the reason for this is that the Linux kernel interface wants streams on eps belonging to one interface to be allocated in one call. Things will also work if we do this one ep at a time (as long as all eps support the same amount of streams), but lets stick to the kernel API. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) (limited to 'hw/usb') diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 220e99dc35..bafe08590b 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -1150,6 +1150,111 @@ static void xhci_free_streams(XHCIEPContext *epctx) epctx->nr_pstreams = 0; } +static int xhci_epmask_to_eps_with_streams(XHCIState *xhci, + unsigned int slotid, + uint32_t epmask, + XHCIEPContext **epctxs, + USBEndpoint **eps) +{ + XHCISlot *slot; + XHCIEPContext *epctx; + USBEndpoint *ep; + int i, j; + + assert(slotid >= 1 && slotid <= xhci->numslots); + + slot = &xhci->slots[slotid - 1]; + + for (i = 2, j = 0; i <= 31; i++) { + if (!(epmask & (1 << i))) { + continue; + } + + epctx = slot->eps[i - 1]; + ep = xhci_epid_to_usbep(xhci, slotid, i); + if (!epctx || !epctx->nr_pstreams || !ep) { + continue; + } + + if (epctxs) { + epctxs[j] = epctx; + } + eps[j++] = ep; + } + return j; +} + +static void xhci_free_device_streams(XHCIState *xhci, unsigned int slotid, + uint32_t epmask) +{ + USBEndpoint *eps[30]; + int nr_eps; + + nr_eps = xhci_epmask_to_eps_with_streams(xhci, slotid, epmask, NULL, eps); + if (nr_eps) { + usb_device_free_streams(eps[0]->dev, eps, nr_eps); + } +} + +static TRBCCode xhci_alloc_device_streams(XHCIState *xhci, unsigned int slotid, + uint32_t epmask) +{ + XHCIEPContext *epctxs[30]; + USBEndpoint *eps[30]; + int i, r, nr_eps, req_nr_streams, dev_max_streams; + + nr_eps = xhci_epmask_to_eps_with_streams(xhci, slotid, epmask, epctxs, + eps); + if (nr_eps == 0) { + return CC_SUCCESS; + } + + req_nr_streams = epctxs[0]->nr_pstreams; + dev_max_streams = eps[0]->max_streams; + + for (i = 1; i < nr_eps; i++) { + /* + * HdG: I don't expect these to ever trigger, but if they do we need + * to come up with another solution, ie group identical endpoints + * together and make an usb_device_alloc_streams call per group. + */ + if (epctxs[i]->nr_pstreams != req_nr_streams) { + FIXME("guest streams config not identical for all eps"); + return CC_RESOURCE_ERROR; + } + if (eps[i]->max_streams != dev_max_streams) { + FIXME("device streams config not identical for all eps"); + return CC_RESOURCE_ERROR; + } + } + + /* + * max-streams in both the device descriptor and in the controller is a + * power of 2. But stream id 0 is reserved, so if a device can do up to 4 + * streams the guest will ask for 5 rounded up to the next power of 2 which + * becomes 8. For emulated devices usb_device_alloc_streams is a nop. + * + * For redirected devices however this is an issue, as there we must ask + * the real xhci controller to alloc streams, and the host driver for the + * real xhci controller will likely disallow allocating more streams then + * the device can handle. + * + * So we limit the requested nr_streams to the maximum number the device + * can handle. + */ + if (req_nr_streams > dev_max_streams) { + req_nr_streams = dev_max_streams; + } + + r = usb_device_alloc_streams(eps[0]->dev, eps, nr_eps, req_nr_streams); + if (r != 0) { + fprintf(stderr, "xhci: alloc streams failed\n"); + return CC_RESOURCE_ERROR; + } + + return CC_SUCCESS; +} + static XHCIStreamContext *xhci_find_stream(XHCIEPContext *epctx, unsigned int streamid, uint32_t *cc_error) @@ -2322,6 +2427,8 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid, return CC_CONTEXT_STATE_ERROR; } + xhci_free_device_streams(xhci, slotid, ictl_ctx[0] | ictl_ctx[1]); + for (i = 2; i <= 31; i++) { if (ictl_ctx[0] & (1< Date: Wed, 20 Nov 2013 14:10:19 +0100 Subject: ehci: implement port wakeup Update portsc register and raise irq in case a suspended port is woken up, so remote wakeup works on our ehci ports. Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) (limited to 'hw/usb') diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 22bdbf4a7d..8765f8f5a7 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -826,9 +826,9 @@ static void ehci_child_detach(USBPort *port, USBDevice *child) static void ehci_wakeup(USBPort *port) { EHCIState *s = port->opaque; - uint32_t portsc = s->portsc[port->index]; + uint32_t *portsc = &s->portsc[port->index]; - if (portsc & PORTSC_POWNER) { + if (*portsc & PORTSC_POWNER) { USBPort *companion = s->companion_ports[port->index]; if (companion->ops->wakeup) { companion->ops->wakeup(companion); @@ -836,6 +836,12 @@ static void ehci_wakeup(USBPort *port) return; } + if (*portsc & PORTSC_SUSPEND) { + trace_usb_ehci_port_wakeup(port->index); + *portsc |= PORTSC_FPRES; + ehci_raise_irq(s, USBSTS_PCD); + } + qemu_bh_schedule(s->async_bh); } @@ -1067,6 +1073,14 @@ static void ehci_port_write(void *ptr, hwaddr addr, } } + if ((val & PORTSC_SUSPEND) && !(*portsc & PORTSC_SUSPEND)) { + trace_usb_ehci_port_suspend(port); + } + if (!(val & PORTSC_FPRES) && (*portsc & PORTSC_FPRES)) { + trace_usb_ehci_port_resume(port); + val &= ~PORTSC_SUSPEND; + } + *portsc &= ~PORTSC_RO_MASK; *portsc |= val; trace_usb_ehci_portsc_change(addr + s->portscbase, addr >> 2, *portsc, old); -- cgit v1.2.1 From 690af06aebdfd043a6222de96a535dcba2144caf Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 20 Nov 2013 14:32:27 +0100 Subject: Revert "usb-tablet: Don't claim wakeup capability for USB-2 version" This reverts commit aa1c9e971e80d25b92908dce3dec7c38b49480ea. Signed-off-by: Gerd Hoffmann --- hw/usb/dev-hid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw/usb') diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c index 59567200ae..5e667f0199 100644 --- a/hw/usb/dev-hid.c +++ b/hw/usb/dev-hid.c @@ -236,7 +236,7 @@ static const USBDescDevice desc_device_tablet2 = { .bNumInterfaces = 1, .bConfigurationValue = 1, .iConfiguration = STR_CONFIG_TABLET, - .bmAttributes = 0x80, + .bmAttributes = 0xa0, .bMaxPower = 50, .nif = 1, .ifs = &desc_iface_tablet2, -- cgit v1.2.1 From 0b1fa34e1dd2764ee7ae2be849e1c5ba5e8724ca Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 20 Nov 2013 07:33:28 +0100 Subject: usb: move usb_{hi,lo} helpers to header file. Signed-off-by: Gerd Hoffmann --- hw/usb/desc.c | 10 ---------- hw/usb/desc.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 10 deletions(-) (limited to 'hw/usb') diff --git a/hw/usb/desc.c b/hw/usb/desc.c index 5dbe7545f5..f18a043500 100644 --- a/hw/usb/desc.c +++ b/hw/usb/desc.c @@ -6,16 +6,6 @@ /* ------------------------------------------------------------------ */ -static uint8_t usb_lo(uint16_t val) -{ - return val & 0xff; -} - -static uint8_t usb_hi(uint16_t val) -{ - return (val >> 8) & 0xff; -} - int usb_desc_device(const USBDescID *id, const USBDescDevice *dev, uint8_t *dest, size_t len) { diff --git a/hw/usb/desc.h b/hw/usb/desc.h index ddd3e7485c..81327b0e74 100644 --- a/hw/usb/desc.h +++ b/hw/usb/desc.h @@ -194,6 +194,17 @@ struct USBDesc { #define USB_DESC_FLAG_SUPER (1 << 1) +/* little helpers */ +static inline uint8_t usb_lo(uint16_t val) +{ + return val & 0xff; +} + +static inline uint8_t usb_hi(uint16_t val) +{ + return (val >> 8) & 0xff; +} + /* generate usb packages from structs */ int usb_desc_device(const USBDescID *id, const USBDescDevice *dev, uint8_t *dest, size_t len); -- cgit v1.2.1