From 5219042274fa2f993c25202680eeaea42193389d Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 27 Aug 2013 14:47:15 +0200 Subject: [PATCH 01/11] xhci: remove leftover debug printf Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index be6b86e2b..57c06e713 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -1164,8 +1164,6 @@ static XHCIStreamContext *xhci_find_stream(XHCIEPContext *epctx, if (sctx->sct == -1) { xhci_dma_read_u32s(epctx->xhci, sctx->pctx, ctx, sizeof(ctx)); - fprintf(stderr, "%s: init sctx #%d @ " DMA_ADDR_FMT ": %08x %08x\n", - __func__, streamid, sctx->pctx, ctx[0], ctx[1]); sct = (ctx[0] >> 1) & 0x07; if (epctx->lsa && sct != 1) { *cc_error = CC_INVALID_STREAM_TYPE_ERROR; From 1c82392a158471355aa6d1922df2d1545bb16b95 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 24 Apr 2013 15:01:25 +0200 Subject: [PATCH 02/11] xhci: add tracepoint for endpoint state changes Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 19 +++++++++++++++++++ trace-events | 1 + 2 files changed, 20 insertions(+) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 57c06e713..83161b9a3 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -586,6 +586,14 @@ static const char *TRBCCode_names[] = { [CC_SPLIT_TRANSACTION_ERROR] = "CC_SPLIT_TRANSACTION_ERROR", }; +static const char *ep_state_names[] = { + [EP_DISABLED] = "disabled", + [EP_RUNNING] = "running", + [EP_HALTED] = "halted", + [EP_STOPPED] = "stopped", + [EP_ERROR] = "error", +}; + static const char *lookup_name(uint32_t index, const char **list, uint32_t llen) { if (index >= llen || list[index] == NULL) { @@ -606,6 +614,12 @@ static const char *event_name(XHCIEvent *event) ARRAY_SIZE(TRBCCode_names)); } +static const char *ep_state_name(uint32_t state) +{ + return lookup_name(state, ep_state_names, + ARRAY_SIZE(ep_state_names)); +} + static uint64_t xhci_mfindex_get(XHCIState *xhci) { int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); @@ -1203,6 +1217,11 @@ static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx, } xhci_dma_write_u32s(xhci, epctx->pctx, ctx, sizeof(ctx)); + if (epctx->state != state) { + trace_usb_xhci_ep_state(epctx->slotid, epctx->epid, + ep_state_name(epctx->state), + ep_state_name(state)); + } epctx->state = state; } diff --git a/trace-events b/trace-events index 3856b5c20..eb8eaef29 100644 --- a/trace-events +++ b/trace-events @@ -381,6 +381,7 @@ usb_xhci_ep_set_dequeue(uint32_t slotid, uint32_t epid, uint32_t streamid, uint6 usb_xhci_ep_kick(uint32_t slotid, uint32_t epid, uint32_t streamid) "slotid %d, epid %d, streamid %d" usb_xhci_ep_stop(uint32_t slotid, uint32_t epid) "slotid %d, epid %d" usb_xhci_ep_reset(uint32_t slotid, uint32_t epid) "slotid %d, epid %d" +usb_xhci_ep_state(uint32_t slotid, uint32_t epid, const char *os, const char *ns) "slotid %d, epid %d, %s -> %s" usb_xhci_xfer_start(void *xfer, uint32_t slotid, uint32_t epid, uint32_t streamid) "%p: slotid %d, epid %d, streamid %d" usb_xhci_xfer_async(void *xfer) "%p" usb_xhci_xfer_nak(void *xfer) "%p" From 65d81ed402d3b78b6ffbade36a09ea53e41614d2 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 28 Aug 2013 11:46:45 +0200 Subject: [PATCH 03/11] xhci: add port to slot_address tracepoint Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 2 +- trace-events | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 83161b9a3..4d693bcf2 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -2135,7 +2135,6 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid, int i; TRBCCode res; - trace_usb_xhci_slot_address(slotid); assert(slotid >= 1 && slotid <= xhci->numslots); dcbaap = xhci_addr64(xhci->dcbaap_low, xhci->dcbaap_high); @@ -2168,6 +2167,7 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid, fprintf(stderr, "xhci: port not found\n"); return CC_TRB_ERROR; } + trace_usb_xhci_slot_address(slotid, uport->path); dev = uport->dev; if (!dev) { diff --git a/trace-events b/trace-events index eb8eaef29..f8498077f 100644 --- a/trace-events +++ b/trace-events @@ -371,7 +371,7 @@ usb_xhci_port_link(uint32_t port, uint32_t pls) "port %d, pls %d" usb_xhci_port_notify(uint32_t port, uint32_t pls) "port %d, bits %x" usb_xhci_slot_enable(uint32_t slotid) "slotid %d" usb_xhci_slot_disable(uint32_t slotid) "slotid %d" -usb_xhci_slot_address(uint32_t slotid) "slotid %d" +usb_xhci_slot_address(uint32_t slotid, const char *port) "slotid %d, port %s" usb_xhci_slot_configure(uint32_t slotid) "slotid %d" usb_xhci_slot_evaluate(uint32_t slotid) "slotid %d" usb_xhci_slot_reset(uint32_t slotid) "slotid %d" From ca7162782a293f525633e5816470498dd86a51cf Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 28 Aug 2013 11:39:02 +0200 Subject: [PATCH 04/11] xhci: fix endpoint interval calculation Cc: qemu-stable@nongnu.org Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 4d693bcf2..38269793b 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -1274,7 +1274,7 @@ static void xhci_init_epctx(XHCIEPContext *epctx, epctx->ring.ccs = ctx[2] & 1; } - epctx->interval = 1 << (ctx[0] >> 16) & 0xff; + epctx->interval = 1 << ((ctx[0] >> 16) & 0xff); } static TRBCCode xhci_enable_ep(XHCIState *xhci, unsigned int slotid, From 4d7a81c06f5f17e019a2d3a18300500bd64f6f40 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 28 Aug 2013 11:38:44 +0200 Subject: [PATCH 05/11] xhci: emulate intr endpoint intervals correctly Respect the interval for interrupt endpoints, so we don't finish transfers as fast as possible but at the rate configured by the guest. Fixes guest deadlocks triggered by interrupt storms. Cc: Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 44 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 38269793b..2e2eb55d0 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -355,6 +355,7 @@ typedef struct XHCITransfer { unsigned int streamid; bool in_xfer; bool iso_xfer; + bool timed_xfer; unsigned int trb_count; unsigned int trb_alloced; @@ -1820,6 +1821,7 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer) xfer->in_xfer = bmRequestType & USB_DIR_IN; xfer->iso_xfer = false; + xfer->timed_xfer = false; if (xhci_setup_packet(xfer) < 0) { return -1; @@ -1835,6 +1837,17 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer) return 0; } +static void xhci_calc_intr_kick(XHCIState *xhci, XHCITransfer *xfer, + XHCIEPContext *epctx, uint64_t mfindex) +{ + uint64_t asap = ((mfindex + epctx->interval - 1) & + ~(epctx->interval-1)); + uint64_t kick = epctx->mfindex_last + epctx->interval; + + assert(epctx->interval != 0); + xfer->mfindex_kick = MAX(asap, kick); +} + static void xhci_calc_iso_kick(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx, uint64_t mfindex) { @@ -1857,8 +1870,8 @@ static void xhci_calc_iso_kick(XHCIState *xhci, XHCITransfer *xfer, } } -static void xhci_check_iso_kick(XHCIState *xhci, XHCITransfer *xfer, - XHCIEPContext *epctx, uint64_t mfindex) +static void xhci_check_intr_iso_kick(XHCIState *xhci, XHCITransfer *xfer, + XHCIEPContext *epctx, uint64_t mfindex) { if (xfer->mfindex_kick > mfindex) { timer_mod(epctx->kick_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + @@ -1883,18 +1896,30 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx switch(epctx->type) { case ET_INTR_OUT: case ET_INTR_IN: + xfer->pkts = 0; + xfer->iso_xfer = false; + xfer->timed_xfer = true; + mfindex = xhci_mfindex_get(xhci); + xhci_calc_intr_kick(xhci, xfer, epctx, mfindex); + xhci_check_intr_iso_kick(xhci, xfer, epctx, mfindex); + if (xfer->running_retry) { + return -1; + } + break; case ET_BULK_OUT: case ET_BULK_IN: xfer->pkts = 0; xfer->iso_xfer = false; + xfer->timed_xfer = false; break; case ET_ISO_OUT: case ET_ISO_IN: xfer->pkts = 1; xfer->iso_xfer = true; + xfer->timed_xfer = true; mfindex = xhci_mfindex_get(xhci); xhci_calc_iso_kick(xhci, xfer, epctx, mfindex); - xhci_check_iso_kick(xhci, xfer, epctx, mfindex); + xhci_check_intr_iso_kick(xhci, xfer, epctx, mfindex); if (xfer->running_retry) { return -1; } @@ -1955,13 +1980,18 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, trace_usb_xhci_xfer_retry(xfer); assert(xfer->running_retry); - if (xfer->iso_xfer) { - /* retry delayed iso transfer */ + if (xfer->timed_xfer) { + /* time to kick the transfer? */ mfindex = xhci_mfindex_get(xhci); - xhci_check_iso_kick(xhci, xfer, epctx, mfindex); + xhci_check_intr_iso_kick(xhci, xfer, epctx, mfindex); if (xfer->running_retry) { return; } + xfer->timed_xfer = 0; + xfer->running_retry = 1; + } + if (xfer->iso_xfer) { + /* retry iso transfer */ if (xhci_setup_packet(xfer) < 0) { return; } @@ -2047,7 +2077,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, epctx->next_xfer = (epctx->next_xfer + 1) % TD_QUEUE; ep = xfer->packet.ep; } else { - if (!xfer->iso_xfer) { + if (!xfer->timed_xfer) { fprintf(stderr, "xhci: error firing data transfer\n"); } } From 5c67dd7b4884979a2613a4702ac1ab68b0e6a16e Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 28 Aug 2013 11:47:09 +0200 Subject: [PATCH 06/11] xhci: reset port when disabling slot Cc: qemu-stable@nongnu.org Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 2e2eb55d0..10c938a88 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -2123,6 +2123,7 @@ static TRBCCode xhci_disable_slot(XHCIState *xhci, unsigned int slotid) xhci->slots[slotid-1].enabled = 0; xhci->slots[slotid-1].addressed = 0; + xhci->slots[slotid-1].uport = NULL; return CC_SUCCESS; } From 1556a8fc38dbf4e950c50427192a3a37cdea3cba Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 27 Aug 2013 14:54:44 +0200 Subject: [PATCH 07/11] uas: add property for request logging Signed-off-by: Gerd Hoffmann --- hw/usb/dev-uas.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c index 63ad12ea6..87012925e 100644 --- a/hw/usb/dev-uas.c +++ b/hw/usb/dev-uas.c @@ -113,6 +113,9 @@ struct UASDevice { QTAILQ_HEAD(, UASStatus) results; QTAILQ_HEAD(, UASRequest) requests; + /* properties */ + uint32_t requestlog; + /* usb 2.0 only */ USBPacket *status2; UASRequest *datain2; @@ -692,9 +695,9 @@ 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); -#if 1 - scsi_req_print(req->req); -#endif + if (uas->requestlog) { + scsi_req_print(req->req); + } len = scsi_req_enqueue(req->req); if (len) { req->data_size = len; @@ -902,6 +905,11 @@ static const VMStateDescription vmstate_usb_uas = { } }; +static Property uas_properties[] = { + DEFINE_PROP_UINT32("log-scsi-req", UASDevice, requestlog, 0), + DEFINE_PROP_END_OF_LIST(), +}; + static void usb_uas_class_initfn(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -919,6 +927,7 @@ static void usb_uas_class_initfn(ObjectClass *klass, void *data) set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); dc->fw_name = "storage"; dc->vmsd = &vmstate_usb_uas; + dc->props = uas_properties; } static const TypeInfo uas_info = { From c96c41ed0d38d68a6c8b6f84751afebafeae31be Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 27 Aug 2013 15:25:24 +0200 Subject: [PATCH 08/11] usb: parallelize usb3 streams usb3 bulk endpoints with streams are implicitly pipelined now, so the requests will actually be processed in parallel. Also allow them to complete out-of-order. Fixes stalls in the uas driver. Cc: qemu-stable@nongnu.org Signed-off-by: Gerd Hoffmann --- hw/usb/core.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/usb/core.c b/hw/usb/core.c index 05948ca9a..31960c28a 100644 --- a/hw/usb/core.c +++ b/hw/usb/core.c @@ -403,7 +403,7 @@ void usb_handle_packet(USBDevice *dev, USBPacket *p) p->ep->halted = false; } - if (QTAILQ_EMPTY(&p->ep->queue) || p->ep->pipeline) { + if (QTAILQ_EMPTY(&p->ep->queue) || p->ep->pipeline || p->stream) { usb_process_one(p); if (p->status == USB_RET_ASYNC) { /* hcd drivers cannot handle async for isoc */ @@ -420,7 +420,8 @@ void usb_handle_packet(USBDevice *dev, USBPacket *p) * When pipelining is enabled usb-devices must always return async, * otherwise packets can complete out of order! */ - assert(!p->ep->pipeline || QTAILQ_EMPTY(&p->ep->queue)); + assert(p->stream || !p->ep->pipeline || + QTAILQ_EMPTY(&p->ep->queue)); if (p->status != USB_RET_NAK) { usb_packet_set_state(p, USB_PACKET_COMPLETE); } @@ -434,7 +435,7 @@ void usb_packet_complete_one(USBDevice *dev, USBPacket *p) { USBEndpoint *ep = p->ep; - assert(QTAILQ_FIRST(&ep->queue) == p); + assert(p->stream || QTAILQ_FIRST(&ep->queue) == p); assert(p->status != USB_RET_ASYNC && p->status != USB_RET_NAK); if (p->status != USB_RET_SUCCESS || From b8cbc1374acdc1d8081f1dc57ef1249d263cf389 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 27 Aug 2013 16:59:37 +0200 Subject: [PATCH 09/11] usb-hub: add tracepoint for status reports Signed-off-by: Gerd Hoffmann --- hw/usb/dev-hub.c | 1 + trace-events | 1 + 2 files changed, 2 insertions(+) diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c index e865a9875..54f63c0d4 100644 --- a/hw/usb/dev-hub.c +++ b/hw/usb/dev-hub.c @@ -475,6 +475,7 @@ static void usb_hub_handle_data(USBDevice *dev, USBPacket *p) port->wPortChange_reported = port->wPortChange; } if (status != 0) { + trace_usb_hub_status_report(s->dev.addr, status); for(i = 0; i < n; i++) { buf[i] = status >> (8 * i); } diff --git a/trace-events b/trace-events index f8498077f..754c28cc5 100644 --- a/trace-events +++ b/trace-events @@ -411,6 +411,7 @@ usb_hub_set_port_feature(int addr, int nr, const char *f) "dev %d, port %d, feat usb_hub_clear_port_feature(int addr, int nr, const char *f) "dev %d, port %d, feature %s" usb_hub_attach(int addr, int nr) "dev %d, port %d" usb_hub_detach(int addr, int nr) "dev %d, port %d" +usb_hub_status_report(int addr, int status) "dev %d, status 0x%x" # hw/usb/dev-uas.c usb_uas_reset(int addr) "dev %d" From bdebd6ee81f4d849aa8541c289203e3992450db0 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 27 Aug 2013 17:00:04 +0200 Subject: [PATCH 10/11] Revert "usb-hub: report status changes only once" This reverts commit a309ee6e0a256f690760abfba44fceaa52a7c2f3. This isn't in line with the usb specification and adds regressions, win7 fails to drive the usb hub for example. Was added because it "solved" the issue of hubs interacting badly with the xhci host controller. Now with the root cause being fixed in xhci (commit ) we can revert this one. Cc: qemu-stable@nongnu.org Signed-off-by: Gerd Hoffmann --- hw/usb/dev-hub.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c index 54f63c0d4..58647b485 100644 --- a/hw/usb/dev-hub.c +++ b/hw/usb/dev-hub.c @@ -33,7 +33,6 @@ typedef struct USBHubPort { USBPort port; uint16_t wPortStatus; uint16_t wPortChange; - uint16_t wPortChange_reported; } USBHubPort; typedef struct USBHubState { @@ -468,11 +467,8 @@ static void usb_hub_handle_data(USBDevice *dev, USBPacket *p) status = 0; for(i = 0; i < NUM_PORTS; i++) { port = &s->ports[i]; - if (port->wPortChange && - port->wPortChange_reported != port->wPortChange) { + if (port->wPortChange) status |= (1 << (i + 1)); - } - port->wPortChange_reported = port->wPortChange; } if (status != 0) { trace_usb_hub_status_report(s->dev.addr, status); From 31efd2e883018b4c079ad082105bc161fbb3fef8 Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Thu, 22 Aug 2013 20:11:36 +0300 Subject: [PATCH 11/11] usb/dev-hid: Modified usb-tablet category from Misc to Input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit usb-tablet device was wrongly assigned to Misc category Reported-by: Markus Armbruster Cc: qemu-stable@nongnu.org Signed-off-by: Marcel Apfelbaum Reviewed-by: Andreas Färber Signed-off-by: Gerd Hoffmann --- hw/usb/dev-hid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c index 66c63317d..59567200a 100644 --- a/hw/usb/dev-hid.c +++ b/hw/usb/dev-hid.c @@ -658,7 +658,7 @@ static void usb_tablet_class_initfn(ObjectClass *klass, void *data) uc->product_desc = "QEMU USB Tablet"; dc->vmsd = &vmstate_usb_ptr; dc->props = usb_tablet_properties; - set_bit(DEVICE_CATEGORY_MISC, dc->categories); + set_bit(DEVICE_CATEGORY_INPUT, dc->categories); } static const TypeInfo usb_tablet_info = {