From 7d04c2b75562664a28612d7481f328ee4ec51dda Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 5 Apr 2013 13:03:08 +0200 Subject: [PATCH 1/4] xhci: remove XHCIRing->base (unused) Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-xhci.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index a26b78ec8..2c90e56c9 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -326,7 +326,6 @@ typedef enum EPType { } EPType; typedef struct XHCIRing { - dma_addr_t base; dma_addr_t dequeue; bool ccs; } XHCIRing; @@ -943,7 +942,6 @@ static void xhci_event(XHCIState *xhci, XHCIEvent *event, int v) static void xhci_ring_init(XHCIState *xhci, XHCIRing *ring, dma_addr_t base) { - ring->base = base; ring->dequeue = base; ring->ccs = 1; } @@ -1948,7 +1946,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, streamid = 0; xhci_set_ep_state(xhci, epctx, NULL, EP_RUNNING); } - assert(ring->base != 0); + assert(ring->dequeue != 0); while (1) { XHCITransfer *xfer = &epctx->transfers[epctx->next_xfer]; From e449f26bed42b1d8c6efefcd8dc768f23f19458f Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 9 Apr 2013 10:24:22 +0200 Subject: [PATCH 2/4] ehci_free_packet: Discard finished packets when the queue is halted With pipelining it is possible to encounter a finished packet when cleaning the queue due to a halt. This happens when a non stall error happens while talking to a real device. In this case the queue on the usb-host side will continue processing packets, and we can have completed packets waiting in the queue after an error condition packet causing a halt. There are 2 reasons to discard the completed packets at this point, rather then trying to writing them back to the guest: 1) The guest expect to be able to cancel and/or change packets after the packet with the error without doing an unlink, so writing them back may confuse the guest. 2) Since the queue does not advance when halted, the writing back of these packets will fail anyways since p->qtdaddr != q->qtdaddr, so the ehci_verify_qtd call in ehci_writeback_async_complete_packet will fail. Note that 2) means that then only functional change this patch introduces is the printing of a warning when this scenario happens. Note that discarding these packets means that the guest driver and the device will get out of sync! This is unfortunate, but should not be a problem since with a non stall error (iow an io-error) the 2 are out of sync already anyways. Still this patch adds a warning to signal this happening. Note that sofar this has only been seen with a DVB-T receiver, which gives of a MPEG-2 stream, which allows for recovering from lost packets, see: https://bugzilla.redhat.com/show_bug.cgi?id=890320 Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 5176251cb..0d3799d44 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -586,17 +586,23 @@ static EHCIPacket *ehci_alloc_packet(EHCIQueue *q) static void ehci_free_packet(EHCIPacket *p) { - if (p->async == EHCI_ASYNC_FINISHED) { + if (p->async == EHCI_ASYNC_FINISHED && + !(p->queue->qh.token & QTD_TOKEN_HALT)) { ehci_writeback_async_complete_packet(p); return; } trace_usb_ehci_packet_action(p->queue, p, "free"); - if (p->async == EHCI_ASYNC_INITIALIZED) { - usb_packet_unmap(&p->packet, &p->sgl); - qemu_sglist_destroy(&p->sgl); - } if (p->async == EHCI_ASYNC_INFLIGHT) { usb_cancel_packet(&p->packet); + } + if (p->async == EHCI_ASYNC_FINISHED && + p->packet.status == USB_RET_SUCCESS) { + fprintf(stderr, + "EHCI: Dropping completed packet from halted %s ep %02X\n", + (p->pid == USB_TOKEN_IN) ? "in" : "out", + get_field(p->queue->qh.epchar, QH_EPCHAR_EP)); + } + if (p->async != EHCI_ASYNC_NONE) { usb_packet_unmap(&p->packet, &p->sgl); qemu_sglist_destroy(&p->sgl); } From 3b7e759a4110690c9617b1ffa6a7c96a343a330d Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 18 Apr 2013 11:57:21 +0200 Subject: [PATCH 3/4] usb: better speed mismatch error reporting Report the supported speeds for device and port in the error message. Also add the speeds to the tracepoint. And while being at it drop the redundant error message in usb_desc_attach, usb_device_attach will report the error anyway. Signed-off-by: Gerd Hoffmann --- hw/usb/bus.c | 36 ++++++++++++++++++++++++++++++++---- hw/usb/desc.c | 2 -- trace-events | 2 +- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/hw/usb/bus.c b/hw/usb/bus.c index b10c290cf..d1827be10 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -417,19 +417,47 @@ void usb_release_port(USBDevice *dev) bus->nfree++; } +static void usb_mask_to_str(char *dest, size_t size, + unsigned int speedmask) +{ + static const struct { + unsigned int mask; + const char *name; + } speeds[] = { + { .mask = USB_SPEED_MASK_FULL, .name = "full" }, + { .mask = USB_SPEED_MASK_HIGH, .name = "high" }, + { .mask = USB_SPEED_MASK_SUPER, .name = "super" }, + }; + int i, pos = 0; + + for (i = 0; i < ARRAY_SIZE(speeds); i++) { + if (speeds[i].mask & speedmask) { + pos += snprintf(dest + pos, size - pos, "%s%s", + pos ? "+" : "", + speeds[i].name); + } + } +} + int usb_device_attach(USBDevice *dev) { USBBus *bus = usb_bus_from_device(dev); USBPort *port = dev->port; + char devspeed[32], portspeed[32]; assert(port != NULL); assert(!dev->attached); - trace_usb_port_attach(bus->busnr, port->path); + usb_mask_to_str(devspeed, sizeof(devspeed), dev->speedmask); + usb_mask_to_str(portspeed, sizeof(portspeed), port->speedmask); + trace_usb_port_attach(bus->busnr, port->path, + devspeed, portspeed); if (!(port->speedmask & dev->speedmask)) { - error_report("Warning: speed mismatch trying to attach " - "usb device %s to bus %s", - dev->product_desc, bus->qbus.name); + error_report("Warning: speed mismatch trying to attach" + " usb device \"%s\" (%s speed)" + " to bus \"%s\", port \"%s\" (%s speed)", + dev->product_desc, devspeed, + bus->qbus.name, port->path, portspeed); return -1; } diff --git a/hw/usb/desc.c b/hw/usb/desc.c index b38938132..fce303e9c 100644 --- a/hw/usb/desc.c +++ b/hw/usb/desc.c @@ -522,8 +522,6 @@ void usb_desc_attach(USBDevice *dev) } else if (desc->full && (dev->port->speedmask & USB_SPEED_MASK_FULL)) { dev->speed = USB_SPEED_FULL; } else { - fprintf(stderr, "usb: port/device speed mismatch for \"%s\"\n", - usb_device_get_product_desc(dev)); return; } usb_desc_setdefaults(dev); diff --git a/trace-events b/trace-events index e587487a3..ffaa3f472 100644 --- a/trace-events +++ b/trace-events @@ -277,7 +277,7 @@ usb_packet_state_fault(int bus, const char *port, int ep, void *p, const char *o # hw/usb/bus.c usb_port_claim(int bus, const char *port) "bus %d, port %s" -usb_port_attach(int bus, const char *port) "bus %d, port %s" +usb_port_attach(int bus, const char *port, const char *devspeed, const char *portspeed) "bus %d, port %s, devspeed %s, portspeed %s" usb_port_detach(int bus, const char *port) "bus %d, port %s" usb_port_release(int bus, const char *port) "bus %d, port %s" From 3f5cc97e2ba00b34fd20a5553ed9d2fecf32f7e3 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 18 Apr 2013 12:16:44 +0200 Subject: [PATCH 4/4] usb-host: raise libusbx minimum version to 1.0.13 Allows to remove one FIXME. Makes LIBUSB_LOG_LEVEL_WARNING build errors go away. And starting with that version libusb has a LIBUSBX_API_VERSION define which allows to easily #ifdef version dependencies should that need arrive in the future. Signed-off-by: Gerd Hoffmann --- configure | 2 +- hw/usb/host-libusb.c | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/configure b/configure index 51a6c5621..33d3354ea 100755 --- a/configure +++ b/configure @@ -3060,7 +3060,7 @@ fi # check for libusb if test "$libusb" != "no" ; then - if $pkg_config libusb-1.0 >/dev/null 2>&1 ; then + if $pkg_config --atleast-version=1.0.13 libusb-1.0 >/dev/null 2>&1 ; then libusb="yes" usb="libusb" libusb_cflags=$($pkg_config --cflags libusb-1.0 2>/dev/null) diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c index 29f35b392..d1186b8a9 100644 --- a/hw/usb/host-libusb.c +++ b/hw/usb/host-libusb.c @@ -236,8 +236,6 @@ static int usb_host_init(void) static int usb_host_get_port(libusb_device *dev, char *port, size_t len) { -#if defined(LIBUSBX_API_VERSION) && (LIBUSBX_API_VERSION >= 0x010000ff) - /* have libusb_get_port_path() */ uint8_t path[7]; size_t off; int rc, i; @@ -251,9 +249,6 @@ static int usb_host_get_port(libusb_device *dev, char *port, size_t len) off += snprintf(port+off, len-off, ".%d", path[i]); } return off; -#else - return snprintf(port, len, "FIXME"); -#endif } static void usb_host_libusb_error(const char *func, int rc)