From 4ea375bf37d655bbf1696239ebea8575956bb226 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 8 Nov 2012 15:54:05 +0100 Subject: fix live migration Commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d breaks live migration. DMA stops working for ehci (and probably for any pci device) after restoring the guest because the bus master region never gets enabled. Add code doing that after loading the pci config space from vmstate. Cc: Avi Kivity Cc: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/pci.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/pci.c b/hw/pci.c index dceda0bdc5..9841e398a6 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -367,6 +367,10 @@ static int get_pci_config_device(QEMUFile *f, void *pv, size_t size) pci_update_mappings(s); + memory_region_set_enabled(&s->bus_master_enable_region, + pci_get_word(s->config + PCI_COMMAND) + & PCI_COMMAND_MASTER); + g_free(config); return 0; } -- cgit v1.2.1 From 155de06f2480219ea99916ddb9ad61b622b8f51f Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 14 Nov 2012 16:23:50 +1100 Subject: usb: Fix (another) bug in usb_packet_map() for IOMMU handling Elements in qemu SGLists can cross IOMMU page boundaries. So, in commit 39c138c8420f51a7da7b35233a8d7400a0b589ac "usb: Fix usb_packet_map() in the presence of IOMMUs", I changed usb_packet_map() to split up each SGList element on IOMMU page boundaries and each resulting piece of qemu's memory space separately to the iovec the usb code uses internally. That was correct in concept, but the patch has a bug. The 'base' variable correctly steps through the dma address of each piece, but then we call the dma_memory_map() function on the base address of the whole SGList element every time. This patch fixes at least one problem using XHCI on the pseries guest machine. It didn't affect OHCI because that doesn't use usb_packet_map(). In theory it also affects EHCI, but we haven't observed that in practice. I think the transfers were small enough on EHCI that they never crossed an IOMMU page boundary in practice. Signed-off-by: David Gibson Signed-off-by: Gerd Hoffmann --- hw/usb/libhw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/libhw.c b/hw/usb/libhw.c index 703e2d213b..24d3cad3a2 100644 --- a/hw/usb/libhw.c +++ b/hw/usb/libhw.c @@ -37,7 +37,7 @@ int usb_packet_map(USBPacket *p, QEMUSGList *sgl) while (len) { dma_addr_t xlen = len; - mem = dma_memory_map(sgl->dma, sgl->sg[i].base, &xlen, dir); + mem = dma_memory_map(sgl->dma, base, &xlen, dir); if (!mem) { goto err; } -- cgit v1.2.1 From c06c68c928edd36eb56baa0d2db065bbec28af27 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 14 Nov 2012 15:51:18 +0100 Subject: usb-host: scan for usb devices when the vm starts Commit a844ed842d9a9d929645c09ae0f52f753d7a02e0 leads to usb-host detecting devices not right after qemu startup because the guest isn't running yet. Instead they are found on the first of the regular usb device poll runs. Which is too late for seabios to see them, so booting from usb sticks fails. Fix this by adding a vm state change handler which triggers a device scan when the vm is started. Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index ca3e24a850..5bc77b2fec 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -1738,6 +1738,7 @@ static int usb_host_scan(void *opaque, USBScanFunc *func) } static QEMUTimer *usb_auto_timer; +static VMChangeStateEntry *usb_vmstate; static int usb_host_auto_scan(void *opaque, int bus_num, int addr, const char *port, @@ -1792,6 +1793,13 @@ static int usb_host_auto_scan(void *opaque, int bus_num, return 0; } +static void usb_host_vm_state(void *unused, int running, RunState state) +{ + if (running) { + usb_host_auto_check(unused); + } +} + static void usb_host_auto_check(void *unused) { struct USBHostDevice *s; @@ -1820,6 +1828,9 @@ static void usb_host_auto_check(void *unused) } } + if (!usb_vmstate) { + usb_vmstate = qemu_add_vm_change_state_handler(usb_host_vm_state, NULL); + } if (!usb_auto_timer) { usb_auto_timer = qemu_new_timer_ms(rt_clock, usb_host_auto_check, NULL); if (!usb_auto_timer) { -- cgit v1.2.1 From 537e8f1aa838677c8efd5e0966e89c4b5423dd18 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Thu, 15 Nov 2012 09:23:30 +0100 Subject: usb: host-linux: Ignore parsing errors of the device descriptors The Linux is more tolerant here as well: Just stop parsing the device descriptors when an error is detected but do not reset what was found so far. This allows to run buggy devices with partially invalid descriptors. Signed-off-by: Jan Kiszka Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index 5bc77b2fec..b17e1dcb7f 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -135,7 +135,7 @@ static int parse_filter(const char *spec, struct USBAutoFilter *f); static void usb_host_auto_check(void *unused); static int usb_host_read_file(char *line, size_t line_size, const char *device_file, const char *device_name); -static int usb_linux_update_endp_table(USBHostDevice *s); +static void usb_linux_update_endp_table(USBHostDevice *s); static int usb_host_usbfs_type(USBHostDevice *s, USBPacket *p) { @@ -1132,8 +1132,7 @@ static void usb_host_handle_control(USBDevice *dev, USBPacket *p, p->status = USB_RET_ASYNC; } -/* returns 1 on problem encountered or 0 for success */ -static int usb_linux_update_endp_table(USBHostDevice *s) +static void usb_linux_update_endp_table(USBHostDevice *s) { static const char *tname[] = { [USB_ENDPOINT_XFER_CONTROL] = "control", @@ -1159,23 +1158,23 @@ static int usb_linux_update_endp_table(USBHostDevice *s) if (d->bLength < 2) { trace_usb_host_parse_error(s->bus_num, s->addr, "descriptor too short"); - goto error; + return; } if (i + d->bLength > s->descr_len) { trace_usb_host_parse_error(s->bus_num, s->addr, "descriptor too long"); - goto error; + return; } switch (d->bDescriptorType) { case 0: trace_usb_host_parse_error(s->bus_num, s->addr, "invalid descriptor type"); - goto error; + return; case USB_DT_DEVICE: if (d->bLength < 0x12) { trace_usb_host_parse_error(s->bus_num, s->addr, "device descriptor too short"); - goto error; + return; } v = (d->u.device.idVendor_hi << 8) | d->u.device.idVendor_lo; p = (d->u.device.idProduct_hi << 8) | d->u.device.idProduct_lo; @@ -1185,7 +1184,7 @@ static int usb_linux_update_endp_table(USBHostDevice *s) if (d->bLength < 0x09) { trace_usb_host_parse_error(s->bus_num, s->addr, "config descriptor too short"); - goto error; + return; } configuration = d->u.config.bConfigurationValue; active = (configuration == s->dev.configuration); @@ -1196,7 +1195,7 @@ static int usb_linux_update_endp_table(USBHostDevice *s) if (d->bLength < 0x09) { trace_usb_host_parse_error(s->bus_num, s->addr, "interface descriptor too short"); - goto error; + return; } interface = d->u.interface.bInterfaceNumber; altsetting = d->u.interface.bAlternateSetting; @@ -1209,7 +1208,7 @@ static int usb_linux_update_endp_table(USBHostDevice *s) if (d->bLength < 0x07) { trace_usb_host_parse_error(s->bus_num, s->addr, "endpoint descriptor too short"); - goto error; + return; } devep = d->u.endpoint.bEndpointAddress; pid = (devep & USB_DIR_IN) ? USB_TOKEN_IN : USB_TOKEN_OUT; @@ -1217,7 +1216,7 @@ static int usb_linux_update_endp_table(USBHostDevice *s) if (ep == 0) { trace_usb_host_parse_error(s->bus_num, s->addr, "invalid endpoint address"); - goto error; + return; } type = d->u.endpoint.bmAttributes & 0x3; @@ -1250,11 +1249,6 @@ static int usb_linux_update_endp_table(USBHostDevice *s) break; } } - return 0; - -error: - usb_ep_reset(&s->dev); - return 1; } /* @@ -1341,10 +1335,7 @@ static int usb_host_open(USBHostDevice *dev, int bus_num, } usb_ep_init(&dev->dev); - ret = usb_linux_update_endp_table(dev); - if (ret) { - goto fail; - } + usb_linux_update_endp_table(dev); if (speed == -1) { struct usbdevfs_connectinfo ci; -- cgit v1.2.1 From 30d68cf6e156b97fc462e18f38ce83f44702cd7f Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 14 Nov 2012 17:21:36 +0100 Subject: ehci: Don't access packet after freeing it ehci_state_writeback() will free the packet, so we should not access the packet after calling ehci_state_writeback(). Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index ee6c9ae302..a8b1a40dea 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -453,12 +453,13 @@ static EHCIPacket *ehci_alloc_packet(EHCIQueue *q) static void ehci_free_packet(EHCIPacket *p) { if (p->async == EHCI_ASYNC_FINISHED) { - int state = ehci_get_state(p->queue->ehci, p->queue->async); + EHCIQueue *q = p->queue; + int state = ehci_get_state(q->ehci, q->async); /* This is a normal, but rare condition (cancel racing completion) */ fprintf(stderr, "EHCI: Warning packet completed but not processed\n"); - ehci_state_executing(p->queue); - ehci_state_writeback(p->queue); - ehci_set_state(p->queue->ehci, p->queue->async, state); + ehci_state_executing(q); + ehci_state_writeback(q); + ehci_set_state(q->ehci, q->async, state); /* state_writeback recurses into us with async == EHCI_ASYNC_NONE!! */ return; } -- cgit v1.2.1 From ff80ce599e0465cc6109a38bd3a8ca1890e88891 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 14 Nov 2012 17:21:37 +0100 Subject: ehci: Fixup q->qtdaddr after cancelling an already completed packet This avoids the q->qtdaddr == p->qtdaddr asserts we have triggering, when a queue contains multiple completed packages when we cancel the queue. I triggered this with windows7 + async interrupt endpoint handling (*) + not detecting circles in ehci_fill_queue() properly, which makes the qtd validation in ehci_fill_queue fail, causing cancellation of the queue on every mouse event ... *) Which is not going upstream as it will cause loss of interrupt events on migration. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index a8b1a40dea..5e3b4a8c4c 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -189,6 +189,7 @@ static const char *ehci_mmio_names[] = { static int ehci_state_executing(EHCIQueue *q); static int ehci_state_writeback(EHCIQueue *q); +static int ehci_state_advqueue(EHCIQueue *q); static int ehci_fill_queue(EHCIPacket *p); static const char *nr2str(const char **n, size_t len, uint32_t nr) @@ -459,6 +460,9 @@ static void ehci_free_packet(EHCIPacket *p) fprintf(stderr, "EHCI: Warning packet completed but not processed\n"); ehci_state_executing(q); ehci_state_writeback(q); + if (!(q->qh.token & QTD_TOKEN_HALT)) { + ehci_state_advqueue(q); + } ehci_set_state(q->ehci, q->async, state); /* state_writeback recurses into us with async == EHCI_ASYNC_NONE!! */ return; -- cgit v1.2.1 From 601a234731602df7c2bf6d01301c6eec4b1d1a5a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 14 Nov 2012 17:21:38 +0100 Subject: ehci: Better detection for qtd-s linked in circles Windows links interrupt qtd-s in circles, which means that when interrupt endpoints return USB_RET_ASYNC, combined with the recent "ehci: Retry to fill the queue while waiting for td completion" patch, we keep adding the tds to the queue over and over again, as we detect the circle from fill_queue, but we call it over and over again ... This patch fixes this by changing the circle detection to also detect circling into tds already queued up previously. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 5e3b4a8c4c..89b7520c07 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1790,7 +1790,7 @@ static int ehci_fill_queue(EHCIPacket *p) USBEndpoint *ep = p->packet.ep; EHCIQueue *q = p->queue; EHCIqtd qtd = p->qtd; - uint32_t qtdaddr, start_addr = p->qtdaddr; + uint32_t qtdaddr; for (;;) { if (NLPTR_TBIT(qtd.next) != 0) { @@ -1801,8 +1801,10 @@ static int ehci_fill_queue(EHCIPacket *p) * Detect circular td lists, Windows creates these, counting on the * active bit going low after execution to make the queue stop. */ - if (qtdaddr == start_addr) { - break; + QTAILQ_FOREACH(p, &q->packets, next) { + if (p->qtdaddr == qtdaddr) { + goto leave; + } } get_dwords(q->ehci, NLPTR_GET(qtdaddr), (uint32_t *) &qtd, sizeof(EHCIqtd) >> 2); @@ -1819,6 +1821,7 @@ static int ehci_fill_queue(EHCIPacket *p) assert(p->packet.status == USB_RET_ASYNC); p->async = EHCI_ASYNC_INFLIGHT; } +leave: usb_device_flush_ep_queue(ep->dev, ep); return 1; } -- cgit v1.2.1 From 2c7b15c1dec4b2ec17549c760df0b7a7cea3ed7d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 15 Nov 2012 14:14:58 +0100 Subject: ehci: Don't verify the next pointer for periodic qh-s and qtd-s While testing the move to async packet handling for interrupt endpoints I noticed that Windows-XP likes to play tricks with the next pointer for periodic qh-s, so we should not fail qh / qtd verification when it changes. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 89b7520c07..287a066d0a 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1550,8 +1550,10 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async) endp = get_field(qh.epchar, QH_EPCHAR_EP); if ((devaddr != get_field(q->qh.epchar, QH_EPCHAR_DEVADDR)) || (endp != get_field(q->qh.epchar, QH_EPCHAR_EP)) || - (memcmp(&qh.current_qtd, &q->qh.current_qtd, - 9 * sizeof(uint32_t)) != 0) || + (qh.current_qtd != q->qh.current_qtd) || + (q->async && qh.next_qtd != q->qh.next_qtd) || + (memcmp(&qh.altnext_qtd, &q->qh.altnext_qtd, + 7 * sizeof(uint32_t)) != 0) || (q->dev != NULL && q->dev->addr != devaddr)) { if (ehci_reset_queue(q) > 0) { ehci_trace_guest_bug(ehci, "guest updated active QH"); @@ -1719,7 +1721,8 @@ static int ehci_state_fetchqtd(EHCIQueue *q) p = QTAILQ_FIRST(&q->packets); if (p != NULL) { if (p->qtdaddr != q->qtdaddr || - (!NLPTR_TBIT(p->qtd.next) && (p->qtd.next != qtd.next)) || + (q->async && !NLPTR_TBIT(p->qtd.next) && + (p->qtd.next != qtd.next)) || (!NLPTR_TBIT(p->qtd.altnext) && (p->qtd.altnext != qtd.altnext)) || p->qtd.bufptr[0] != qtd.bufptr[0]) { ehci_cancel_queue(q); -- cgit v1.2.1 From 40862309a9d733cb0e878c79f477de003897b5d2 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 13 Nov 2012 17:20:05 +0100 Subject: ehci: keep the frame timer running in case the guest asked for frame list rollover interrupts Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 287a066d0a..14269dafed 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -964,6 +964,9 @@ static void ehci_opreg_write(void *ptr, hwaddr addr, case USBINTR: val &= USBINTR_MASK; + if (ehci_enabled(s) && (USBSTS_FLR & val)) { + qemu_bh_schedule(s->async_bh); + } break; case FRINDEX: @@ -2220,6 +2223,10 @@ static void ehci_frame_timer(void *opaque) ehci->async_stepdown = 0; } + if (ehci_enabled(ehci) && (ehci->usbintr & USBSTS_FLR)) { + need_timer++; + } + if (need_timer) { /* If we've raised int, we speed up the timer, so that we quickly * notice any new packets queued up in response */ -- cgit v1.2.1 From 55903f1d2d8abfa8d7610ab32a4046a1ed4fdbb8 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 15 Nov 2012 13:07:49 +0100 Subject: ehci: handle dma errors Starting with commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d dma transfers can actually fail. This patch makes ehci keep track of the busmaster bit in pci config space, by setting/clearing the dma_context pointer. Attempts to dma without context will result in raising HSE (Host System Error) interrupt and stopping the host controller. This patch fixes WinXP not booting with a usb stick attached to ehci. Root cause is seabios activating ehci so you can boot from the stick, and WinXP clearing the busmaster bit before resetting the host controller, leading to ehci actually trying dma while it is disabled. Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci-pci.c | 17 ++++++++++++++ hw/usb/hcd-ehci.c | 63 +++++++++++++++++++++++++++++++++++---------------- trace-events | 1 + 3 files changed, 62 insertions(+), 19 deletions(-) diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c index fe45a1fbba..5887eab197 100644 --- a/hw/usb/hcd-ehci-pci.c +++ b/hw/usb/hcd-ehci-pci.c @@ -17,6 +17,7 @@ #include "hw/usb/hcd-ehci.h" #include "hw/pci.h" +#include "range.h" typedef struct EHCIPCIState { PCIDevice pcidev; @@ -79,6 +80,21 @@ static int usb_ehci_pci_initfn(PCIDevice *dev) return 0; } +static void usb_ehci_pci_write_config(PCIDevice *dev, uint32_t addr, + uint32_t val, int l) +{ + EHCIPCIState *i = DO_UPCAST(EHCIPCIState, pcidev, dev); + bool busmaster; + + pci_default_write_config(dev, addr, val, l); + + if (!range_covers_byte(addr, l, PCI_COMMAND)) { + return; + } + busmaster = pci_get_word(dev->config + PCI_COMMAND) & PCI_COMMAND_MASTER; + i->ehci.dma = busmaster ? pci_dma_context(dev) : NULL; +} + static Property ehci_pci_properties[] = { DEFINE_PROP_UINT32("maxframes", EHCIPCIState, ehci.maxframes, 128), DEFINE_PROP_END_OF_LIST(), @@ -106,6 +122,7 @@ static void ehci_class_init(ObjectClass *klass, void *data) k->device_id = i->device_id; k->revision = i->revision; k->class_id = PCI_CLASS_SERIAL_USB; + k->config_write = usb_ehci_pci_write_config; dc->vmsd = &vmstate_ehci_pci; dc->props = ehci_pci_properties; } diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 14269dafed..7df8e21ecb 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1003,21 +1003,25 @@ static void ehci_opreg_write(void *ptr, hwaddr addr, *mmio, old); } - -// TODO : Put in common header file, duplication from usb-ohci.c - /* Get an array of dwords from main memory */ static inline int get_dwords(EHCIState *ehci, uint32_t addr, uint32_t *buf, int num) { int i; + if (!ehci->dma) { + ehci_raise_irq(ehci, USBSTS_HSE); + ehci->usbcmd &= ~USBCMD_RUNSTOP; + trace_usb_ehci_dma_error(); + return -1; + } + for(i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { dma_memory_read(ehci->dma, addr, buf, sizeof(*buf)); *buf = le32_to_cpu(*buf); } - return 1; + return num; } /* Put an array of dwords in to main memory */ @@ -1026,12 +1030,19 @@ static inline int put_dwords(EHCIState *ehci, uint32_t addr, { int i; + if (!ehci->dma) { + ehci_raise_irq(ehci, USBSTS_HSE); + ehci->usbcmd &= ~USBCMD_RUNSTOP; + trace_usb_ehci_dma_error(); + return -1; + } + for(i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { uint32_t tmp = cpu_to_le32(*buf); dma_memory_write(ehci->dma, addr, &tmp, sizeof(tmp)); } - return 1; + return num; } /* @@ -1443,8 +1454,10 @@ static int ehci_state_waitlisthead(EHCIState *ehci, int async) /* Find the head of the list (4.9.1.1) */ for(i = 0; i < MAX_QH; i++) { - get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &qh, - sizeof(EHCIqh) >> 2); + if (get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &qh, + sizeof(EHCIqh) >> 2) < 0) { + return 0; + } ehci_trace_qh(NULL, NLPTR_GET(entry), &qh); if (qh.epchar & QH_EPCHAR_H) { @@ -1541,8 +1554,11 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async) goto out; } - get_dwords(ehci, NLPTR_GET(q->qhaddr), - (uint32_t *) &qh, sizeof(EHCIqh) >> 2); + if (get_dwords(ehci, NLPTR_GET(q->qhaddr), + (uint32_t *) &qh, sizeof(EHCIqh) >> 2) < 0) { + q = NULL; + goto out; + } ehci_trace_qh(q, NLPTR_GET(q->qhaddr), &qh); /* @@ -1631,8 +1647,10 @@ static int ehci_state_fetchitd(EHCIState *ehci, int async) assert(!async); entry = ehci_get_fetch_addr(ehci, async); - get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &itd, - sizeof(EHCIitd) >> 2); + if (get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &itd, + sizeof(EHCIitd) >> 2) < 0) { + return -1; + } ehci_trace_itd(ehci, entry, &itd); if (ehci_process_itd(ehci, &itd, entry) != 0) { @@ -1655,8 +1673,10 @@ static int ehci_state_fetchsitd(EHCIState *ehci, int async) assert(!async); entry = ehci_get_fetch_addr(ehci, async); - get_dwords(ehci, NLPTR_GET(entry), (uint32_t *)&sitd, - sizeof(EHCIsitd) >> 2); + if (get_dwords(ehci, NLPTR_GET(entry), (uint32_t *)&sitd, + sizeof(EHCIsitd) >> 2) < 0) { + return 0; + } ehci_trace_sitd(ehci, entry, &sitd); if (!(sitd.results & SITD_RESULTS_ACTIVE)) { @@ -1717,8 +1737,10 @@ static int ehci_state_fetchqtd(EHCIQueue *q) EHCIPacket *p; int again = 1; - get_dwords(q->ehci, NLPTR_GET(q->qtdaddr), (uint32_t *) &qtd, - sizeof(EHCIqtd) >> 2); + if (get_dwords(q->ehci, NLPTR_GET(q->qtdaddr), (uint32_t *) &qtd, + sizeof(EHCIqtd) >> 2) < 0) { + return 0; + } ehci_trace_qtd(q, NLPTR_GET(q->qtdaddr), &qtd); p = QTAILQ_FIRST(&q->packets); @@ -1812,8 +1834,10 @@ static int ehci_fill_queue(EHCIPacket *p) goto leave; } } - get_dwords(q->ehci, NLPTR_GET(qtdaddr), - (uint32_t *) &qtd, sizeof(EHCIqtd) >> 2); + if (get_dwords(q->ehci, NLPTR_GET(qtdaddr), + (uint32_t *) &qtd, sizeof(EHCIqtd) >> 2) < 0) { + return -1; + } ehci_trace_qtd(q, NLPTR_GET(qtdaddr), &qtd); if (!(qtd.token & QTD_TOKEN_ACTIVE)) { break; @@ -2112,8 +2136,9 @@ static void ehci_advance_periodic_state(EHCIState *ehci) } list |= ((ehci->frindex & 0x1ff8) >> 1); - dma_memory_read(ehci->dma, list, &entry, sizeof entry); - entry = le32_to_cpu(entry); + if (get_dwords(ehci, list, &entry, 1) < 0) { + break; + } DPRINTF("PERIODIC state adv fr=%d. [%08X] -> %08X\n", ehci->frindex / 8, list, entry); diff --git a/trace-events b/trace-events index e1a37cc26f..35308be525 100644 --- a/trace-events +++ b/trace-events @@ -286,6 +286,7 @@ usb_ehci_irq(uint32_t level, uint32_t frindex, uint32_t sts, uint32_t mask) "lev usb_ehci_guest_bug(const char *reason) "%s" usb_ehci_doorbell_ring(void) "" usb_ehci_doorbell_ack(void) "" +usb_ehci_dma_error(void) "" # hw/usb/hcd-uhci.c usb_uhci_reset(void) "=== RESET ===" -- cgit v1.2.1 From 2cb343b442c98b45d1187f5691c45b3c114f3b04 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 15 Nov 2012 14:14:59 +0100 Subject: usb-redir: Only add actually in flight packets to the in flight queue Packets which are queued up, but not yet handed over to the device, are *not* in flight. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/redirect.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index be9a232059..32ae1034cc 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -342,7 +342,9 @@ static void usbredir_fill_already_in_flight_from_ep(USBRedirDevice *dev, if (p->combined && p != p->combined->first) { continue; } - packet_id_queue_add(&dev->already_in_flight, p->id); + if (p->state == USB_PACKET_ASYNC) { + packet_id_queue_add(&dev->already_in_flight, p->id); + } } } -- cgit v1.2.1 From 618fbc951d190734bac73d86abd1a77180a83050 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 15 Nov 2012 14:15:00 +0100 Subject: usb-redir: Set default debug level to warning The previous default of 0 means that even errors and warnings would not get printed, which is really not a good default. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/redirect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index 32ae1034cc..0c95e6b05e 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -1962,7 +1962,7 @@ static const VMStateDescription usbredir_vmstate = { static Property usbredir_properties[] = { DEFINE_PROP_CHR("chardev", USBRedirDevice, cs), - DEFINE_PROP_UINT8("debug", USBRedirDevice, debug, 0), + DEFINE_PROP_UINT8("debug", USBRedirDevice, debug, usbredirparser_warning), DEFINE_PROP_STRING("filter", USBRedirDevice, filter_str), DEFINE_PROP_INT32("bootindex", USBRedirDevice, bootindex, -1), DEFINE_PROP_END_OF_LIST(), -- cgit v1.2.1 From 8c908fca584dbf47094b63f132bb49b82eaa3e19 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 15 Nov 2012 16:11:20 +0100 Subject: usb-host: update tracing Now that we have separate status and length fields in USBPacket update the completion tracepoint to log both. Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c | 20 ++++++++++++-------- trace-events | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index b17e1dcb7f..e3d394fcef 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -385,10 +385,12 @@ static void async_complete(void *opaque) } if (aurb->urb.type == USBDEVFS_URB_TYPE_CONTROL) { - trace_usb_host_req_complete(s->bus_num, s->addr, p, p->status); + trace_usb_host_req_complete(s->bus_num, s->addr, p, + p->status, aurb->urb.actual_length); usb_generic_async_ctrl_complete(&s->dev, p); } else if (!aurb->more) { - trace_usb_host_req_complete(s->bus_num, s->addr, p, p->status); + trace_usb_host_req_complete(s->bus_num, s->addr, p, + p->status, aurb->urb.actual_length); usb_packet_complete(&s->dev, p); } } @@ -863,8 +865,9 @@ static void usb_host_handle_data(USBDevice *dev, USBPacket *p) p->ep->nr, p->iov.size); if (!is_valid(s, p->pid, p->ep->nr)) { - trace_usb_host_req_complete(s->bus_num, s->addr, p, USB_RET_NAK); p->status = USB_RET_NAK; + trace_usb_host_req_complete(s->bus_num, s->addr, p, + p->status, p->actual_length); return; } @@ -879,8 +882,9 @@ static void usb_host_handle_data(USBDevice *dev, USBPacket *p) ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &arg); if (ret < 0) { perror("USBDEVFS_CLEAR_HALT"); - trace_usb_host_req_complete(s->bus_num, s->addr, p, USB_RET_NAK); p->status = USB_RET_NAK; + trace_usb_host_req_complete(s->bus_num, s->addr, p, + p->status, p->actual_length); return; } clear_halt(s, p->pid, p->ep->nr); @@ -936,15 +940,15 @@ static void usb_host_handle_data(USBDevice *dev, USBPacket *p) switch(errno) { case ETIMEDOUT: - trace_usb_host_req_complete(s->bus_num, s->addr, p, - USB_RET_NAK); p->status = USB_RET_NAK; + trace_usb_host_req_complete(s->bus_num, s->addr, p, + p->status, p->actual_length); break; case EPIPE: default: - trace_usb_host_req_complete(s->bus_num, s->addr, p, - USB_RET_STALL); p->status = USB_RET_STALL; + trace_usb_host_req_complete(s->bus_num, s->addr, p, + p->status, p->actual_length); } return; } diff --git a/trace-events b/trace-events index 35308be525..6c6cbf10fd 100644 --- a/trace-events +++ b/trace-events @@ -409,7 +409,7 @@ usb_host_claim_interfaces(int bus, int addr, int config, int nif) "dev %d:%d, co usb_host_release_interfaces(int bus, int addr) "dev %d:%d" usb_host_req_control(int bus, int addr, void *p, int req, int value, int index) "dev %d:%d, packet %p, req 0x%x, value %d, index %d" usb_host_req_data(int bus, int addr, void *p, int in, int ep, int size) "dev %d:%d, packet %p, in %d, ep %d, size %d" -usb_host_req_complete(int bus, int addr, void *p, int status) "dev %d:%d, packet %p, status %d" +usb_host_req_complete(int bus, int addr, void *p, int status, int length) "dev %d:%d, packet %p, status %d, length %d" usb_host_req_emulated(int bus, int addr, void *p, int status) "dev %d:%d, packet %p, status %d" usb_host_req_canceled(int bus, int addr, void *p) "dev %d:%d, packet %p" usb_host_urb_submit(int bus, int addr, void *aurb, int length, int more) "dev %d:%d, aurb %p, length %d, more %d" -- cgit v1.2.1 From 71e0aa3930e7ac2e039b175ffad222e3dc5b1813 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 15 Nov 2012 16:11:49 +0100 Subject: usb-host: fix splitted transfers USBPacket->actual_length wasn't updated correctly for USBPackets splitted into multiple urbs. Fix it. Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index e3d394fcef..aa77b7704d 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -366,8 +366,11 @@ static void async_complete(void *opaque) if (p) { switch (aurb->urb.status) { case 0: - p->actual_length = aurb->urb.actual_length; - p->status = USB_RET_SUCCESS; /* Clear previous ASYNC status */ + p->actual_length += aurb->urb.actual_length; + if (!aurb->more) { + /* Clear previous ASYNC status */ + p->status = USB_RET_SUCCESS; + } break; case -EPIPE: -- cgit v1.2.1