[Qemu-devel] [PULL for-1.3 00/14] usb patch queue

[Qemu-devel] [PULL for-1.3 00/14] usb patch queue

[Gerd Hoffmann]

  Hi,

A bunch of usb bugfixes all over the place piled
up in the usb patch queue.  Here we go.

please pull,
  Gerd

The following changes since commit 6801038bc52d61f81ac8a25fbe392f1bad982887:

  target-mips: fix wrong microMIPS opcode encoding (2012-11-15 14:48:16 +0100)

are available in the git repository at:
  git://git.kraxel.org/qemu usb.71

David Gibson (1):
      usb: Fix (another) bug in usb_packet_map() for IOMMU handling

Gerd Hoffmann (6):
      fix live migration
      usb-host: scan for usb devices when the vm starts
      ehci: keep the frame timer running in case the guest asked for frame list rollover interrupts
      ehci: handle dma errors
      usb-host: update tracing
      usb-host: fix splitted transfers

Hans de Goede (6):
      ehci: Don't access packet after freeing it
      ehci: Fixup q->qtdaddr after cancelling an already completed packet
      ehci: Better detection for qtd-s linked in circles
      ehci: Don't verify the next pointer for periodic qh-s and qtd-s
      usb-redir: Only add actually in flight packets to the in flight queue
      usb-redir: Set default debug level to warning

Jan Kiszka (1):
      usb: host-linux: Ignore parsing errors of the device descriptors

 hw/pci.c              |    4 ++
 hw/usb/hcd-ehci-pci.c |   17 ++++++++
 hw/usb/hcd-ehci.c     |  101 +++++++++++++++++++++++++++++++++++--------------
 hw/usb/host-linux.c   |   69 +++++++++++++++++++--------------
 hw/usb/libhw.c        |    2 +-
 hw/usb/redirect.c     |    6 ++-
 trace-events          |    3 +-
 7 files changed, 139 insertions(+), 63 deletions(-)

[Qemu-devel] [PATCH 01/14] fix live migration

[Gerd Hoffmann]

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 <avi@redhat.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index dceda0b..9841e39 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;
 }
-- 
1.7.1

[Qemu-devel] [PATCH 02/14] usb: Fix (another) bug in usb_packet_map() for IOMMU handling

[Gerd Hoffmann]

From: David Gibson <david@gibson.dropbear.id.au>

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 <david@gibson.dropbear.id.au>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/libhw.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/usb/libhw.c b/hw/usb/libhw.c
index 703e2d2..24d3cad 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;
             }
-- 
1.7.1

[Qemu-devel] [PATCH 03/14] usb-host: scan for usb devices when the vm starts

[Gerd Hoffmann]

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 <kraxel@redhat.com>
---
 hw/usb/host-linux.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index ca3e24a..5bc77b2 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) {
-- 
1.7.1

[Qemu-devel] [PATCH 04/14] usb: host-linux: Ignore parsing errors of the device descriptors

[Gerd Hoffmann]

From: Jan Kiszka <jan.kiszka@siemens.com>

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 <jan.kiszka@siemens.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/host-linux.c |   31 +++++++++++--------------------
 1 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index 5bc77b2..b17e1dc 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;
-- 
1.7.1

[Qemu-devel] [PATCH 05/14] ehci: Don't access packet after freeing it

[Gerd Hoffmann]

From: Hans de Goede <hdegoede@redhat.com>

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 <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index ee6c9ae..a8b1a40 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;
     }
-- 
1.7.1

[Qemu-devel] [PATCH 06/14] ehci: Fixup q->qtdaddr after cancelling an already completed packet

[Gerd Hoffmann]

From: Hans de Goede <hdegoede@redhat.com>

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 <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index a8b1a40..5e3b4a8 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;
-- 
1.7.1

[Qemu-devel] [PATCH 07/14] ehci: Better detection for qtd-s linked in circles

[Gerd Hoffmann]

From: Hans de Goede <hdegoede@redhat.com>

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 <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 5e3b4a8..89b7520 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;
 }
-- 
1.7.1

[Qemu-devel] [PATCH 08/14] ehci: Don't verify the next pointer for periodic qh-s and qtd-s

[Gerd Hoffmann]

From: Hans de Goede <hdegoede@redhat.com>

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 <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 89b7520..287a066 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);
-- 
1.7.1

[Qemu-devel] [PATCH 09/14] ehci: keep the frame timer running in case the guest asked for frame list rollover interrupts

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 287a066..14269da 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 */
-- 
1.7.1

[Qemu-devel] [PATCH 10/14] ehci: handle dma errors

[Gerd Hoffmann]

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 <kraxel@redhat.com>
---
 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 fe45a1f..5887eab 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 14269da..7df8e21 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 e1a37cc..35308be 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 ==="
-- 
1.7.1

[Qemu-devel] [PATCH 11/14] usb-redir: Only add actually in flight packets to the in flight queue

[Gerd Hoffmann]

From: Hans de Goede <hdegoede@redhat.com>

Packets which are queued up, but not yet handed over to the device, are
*not* in flight.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/redirect.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index be9a232..32ae103 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);
+        }
     }
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 12/14] usb-redir: Set default debug level to warning

[Gerd Hoffmann]

From: Hans de Goede <hdegoede@redhat.com>

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 <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/redirect.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 32ae103..0c95e6b 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(),
-- 
1.7.1

[Qemu-devel] [PATCH 13/14] usb-host: update tracing

[Gerd Hoffmann]

Now that we have separate status and length fields in USBPacket
update the completion tracepoint to log both.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 b17e1dc..e3d394f 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 35308be..6c6cbf1 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"
-- 
1.7.1

[Qemu-devel] [PATCH 14/14] usb-host: fix splitted transfers

[Gerd Hoffmann]

USBPacket->actual_length wasn't updated correctly for USBPackets
splitted into multiple urbs.  Fix it.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/host-linux.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index e3d394f..aa77b77 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:
-- 
1.7.1