All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes.
@ 2011-05-26 10:44 Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 01/12] usb-linux: catch NODEV in more places Gerd Hoffmann
                   ` (11 more replies)
  0 siblings, 12 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

  Hi,

This patch series brings a number of improvements for the EHCI host
adapter.

First it makes EHCI use the tracing infrastructure.  Conversion is done
in a bunch of steps to ease review and to improve bisectability in case
regressions show up.  Not all debug messages are converted (yet).

Second EHCI can keep track of multiple queues now and supports multiple
async requests being in flight at the same time.

And as usual a few bug fixes are included too.

please review,
  Gerd

Gerd Hoffmann (11):
  usb-linux: catch NODEV in more places.
  usb-ehci: trace mmio and usbsts
  usb-ehci: trace state machine changes
  usb-ehci: trace port state
  usb-ehci: improve mmio tracing
  usb-ehci: trace buffer copy
  usb-ehci: add queue data struct
  usb-ehci: multiqueue support
  usb-ehci: fix offset writeback in ehci_buffer_rw
  usb-ehci: fix error handling.
  usb: cancel async packets on unplug

Hans de Goede (1):
  ehci: fix a number of unused-but-set-variable warnings (new with
    gcc-4.6)

 hw/usb-bus.c  |    5 +-
 hw/usb-ehci.c | 1073 +++++++++++++++++++++++++++++++++-----------------------
 hw/usb-musb.c |   23 ++-
 hw/usb-ohci.c |   16 +-
 hw/usb-uhci.c |   26 ++-
 hw/usb.h      |    8 +-
 trace-events  |   16 +
 usb-linux.c   |   27 +-
 8 files changed, 741 insertions(+), 453 deletions(-)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 01/12] usb-linux: catch NODEV in more places.
  2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
@ 2011-05-26 10:44 ` Gerd Hoffmann
  2011-05-26 18:20   ` Markus Armbruster
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 02/12] usb-ehci: trace mmio and usbsts Gerd Hoffmann
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Factor out disconnect code (called when a device disappears) to a
separate function.  Add a check for NODEV errno to a few more places to
make sure we notice disconnects.

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

diff --git a/usb-linux.c b/usb-linux.c
index baa6574..b195e38 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -268,6 +268,14 @@ static void async_free(AsyncURB *aurb)
     qemu_free(aurb);
 }
 
+static void do_disconnect(USBHostDevice *s)
+{
+    printf("husb: device %d.%d disconnected\n",
+           s->bus_num, s->addr);
+    usb_host_close(s);
+    usb_host_auto_check(NULL);
+}
+
 static void async_complete(void *opaque)
 {
     USBHostDevice *s = opaque;
@@ -282,10 +290,7 @@ static void async_complete(void *opaque)
                 return;
             }
             if (errno == ENODEV && !s->closing) {
-                printf("husb: device %d.%d disconnected\n",
-                       s->bus_num, s->addr);
-                usb_host_close(s);
-                usb_host_auto_check(NULL);
+                do_disconnect(s);
                 return;
             }
 
@@ -359,6 +364,7 @@ static void usb_host_async_cancel(USBDevice *dev, USBPacket *p)
 
 static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
 {
+    const char *op = NULL;
     int dev_descr_len, config_descr_len;
     int interface, nb_interfaces;
     int ret, i;
@@ -411,9 +417,9 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
             ctrl.ioctl_code = USBDEVFS_DISCONNECT;
             ctrl.ifno = interface;
             ctrl.data = 0;
+            op = "USBDEVFS_DISCONNECT";
             ret = ioctl(dev->fd, USBDEVFS_IOCTL, &ctrl);
             if (ret < 0 && errno != ENODATA) {
-                perror("USBDEVFS_DISCONNECT");
                 goto fail;
             }
         }
@@ -422,6 +428,7 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
 
     /* XXX: only grab if all interfaces are free */
     for (interface = 0; interface < nb_interfaces; interface++) {
+        op = "USBDEVFS_CLAIMINTERFACE";
         ret = ioctl(dev->fd, USBDEVFS_CLAIMINTERFACE, &interface);
         if (ret < 0) {
             if (errno == EBUSY) {
@@ -429,8 +436,7 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
             } else {
                 perror("husb: failed to claim interface");
             }
-        fail:
-            return 0;
+            goto fail;
         }
     }
 
@@ -440,6 +446,13 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
     dev->ninterfaces   = nb_interfaces;
     dev->configuration = configuration;
     return 1;
+
+fail:
+    if (errno == ENODEV) {
+        do_disconnect(dev);
+    }
+    perror(op);
+    return 0;
 }
 
 static int usb_host_release_interfaces(USBHostDevice *s)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 02/12] usb-ehci: trace mmio and usbsts
  2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 01/12] usb-linux: catch NODEV in more places Gerd Hoffmann
@ 2011-05-26 10:44 ` Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 03/12] usb-ehci: trace state machine changes Gerd Hoffmann
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

This patch starts adding trace support to ehci.  It traces
updates of the status register (USBSTS), mmio access and
controller reset.

It also adds functions to set and clear status register bits
and puts them in use everywhere.

Some DPRINTF's are dropped in favor of the new tracepoints.

No change in behavior.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-ehci.c |  156 ++++++++++++++++++++++++++++++---------------------------
 trace-events  |    6 ++
 2 files changed, 89 insertions(+), 73 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index f63519e..16fbca6 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -30,6 +30,7 @@
 #include "usb.h"
 #include "pci.h"
 #include "monitor.h"
+#include "trace.h"
 
 #define EHCI_DEBUG   0
 #define STATE_DEBUG  0       /* state transitions  */
@@ -417,24 +418,23 @@ typedef struct {
     } while(0)
 
 
-#if EHCI_DEBUG
-static const char *addr2str(unsigned addr)
+static const char *addr2str(target_phys_addr_t addr)
 {
-    const char *r            = "   unknown";
+    const char *r            = "unknown";
     const char *n[] = {
-        [ CAPLENGTH ]        = " CAPLENGTH",
+        [ CAPLENGTH ]        = "CAPLENGTH",
         [ HCIVERSION ]       = "HCIVERSION",
-        [ HCSPARAMS ]        = " HCSPARAMS",
-        [ HCCPARAMS ]        = " HCCPARAMS",
-        [ USBCMD ]           = "   COMMAND",
-        [ USBSTS ]           = "    STATUS",
-        [ USBINTR ]          = " INTERRUPT",
-        [ FRINDEX ]          = " FRAME IDX",
+        [ HCSPARAMS ]        = "HCSPARAMS",
+        [ HCCPARAMS ]        = "HCCPARAMS",
+        [ USBCMD ]           = "USBCMD",
+        [ USBSTS ]           = "USBSTS",
+        [ USBINTR ]          = "USBINTR",
+        [ FRINDEX ]          = "FRINDEX",
         [ PERIODICLISTBASE ] = "P-LIST BASE",
         [ ASYNCLISTADDR ]    = "A-LIST ADDR",
         [ PORTSC_BEGIN ...
-          PORTSC_END ]       = "PORT STATUS",
-        [ CONFIGFLAG ]       = "CONFIG FLAG",
+          PORTSC_END ]       = "PORTSC",
+        [ CONFIGFLAG ]       = "CONFIGFLAG",
     };
 
     if (addr < ARRAY_SIZE(n) && n[addr] != NULL) {
@@ -443,8 +443,61 @@ static const char *addr2str(unsigned addr)
         return r;
     }
 }
-#endif
 
+static void ehci_trace_usbsts(uint32_t mask, int state)
+{
+    /* interrupts */
+    if (mask & USBSTS_INT) {
+        trace_usb_ehci_usbsts("INT", state);
+    }
+    if (mask & USBSTS_ERRINT) {
+        trace_usb_ehci_usbsts("ERRINT", state);
+    }
+    if (mask & USBSTS_PCD) {
+        trace_usb_ehci_usbsts("PCD", state);
+    }
+    if (mask & USBSTS_FLR) {
+        trace_usb_ehci_usbsts("FLR", state);
+    }
+    if (mask & USBSTS_HSE) {
+        trace_usb_ehci_usbsts("HSE", state);
+    }
+    if (mask & USBSTS_IAA) {
+        trace_usb_ehci_usbsts("IAA", state);
+    }
+
+    /* status */
+    if (mask & USBSTS_HALT) {
+        trace_usb_ehci_usbsts("HALT", state);
+    }
+    if (mask & USBSTS_REC) {
+        trace_usb_ehci_usbsts("REC", state);
+    }
+    if (mask & USBSTS_PSS) {
+        trace_usb_ehci_usbsts("PSS", state);
+    }
+    if (mask & USBSTS_ASS) {
+        trace_usb_ehci_usbsts("ASS", state);
+    }
+}
+
+static inline void ehci_set_usbsts(EHCIState *s, int mask)
+{
+    if ((s->usbsts & mask) == mask) {
+        return;
+    }
+    ehci_trace_usbsts(mask, 1);
+    s->usbsts |= mask;
+}
+
+static inline void ehci_clear_usbsts(EHCIState *s, int mask)
+{
+    if ((s->usbsts & mask) == 0) {
+        return;
+    }
+    ehci_trace_usbsts(mask, 0);
+    s->usbsts &= ~mask;
+}
 
 static inline void ehci_set_interrupt(EHCIState *s, int intr)
 {
@@ -452,7 +505,7 @@ static inline void ehci_set_interrupt(EHCIState *s, int intr)
 
     // TODO honour interrupt threshold requests
 
-    s->usbsts |= intr;
+    ehci_set_usbsts(s, intr);
 
     if ((s->usbsts & USBINTR_MASK) & s->usbintr) {
         level = 1;
@@ -526,6 +579,7 @@ static void ehci_reset(void *opaque)
     uint8_t *pci_conf;
     int i;
 
+    trace_usb_ehci_reset();
     pci_conf = s->dev.config;
 
     memset(&s->mmio[OPREGBASE], 0x00, MMIO_SIZE - OPREGBASE);
@@ -576,6 +630,7 @@ static uint32_t ehci_mem_readl(void *ptr, target_phys_addr_t addr)
     val = s->mmio[addr] | (s->mmio[addr+1] << 8) |
           (s->mmio[addr+2] << 16) | (s->mmio[addr+3] << 24);
 
+    trace_usb_ehci_mmio_readl(addr, addr2str(addr), val);
     return val;
 }
 
@@ -645,9 +700,9 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
 {
     EHCIState *s = ptr;
     int i;
-#if EHCI_DEBUG
-    const char *str;
-#endif
+
+    trace_usb_ehci_mmio_writel(addr, addr2str(addr), val,
+                               *(uint32_t *)(&s->mmio[addr]));
 
     /* Only aligned reads are allowed on OHCI */
     if (addr & 3) {
@@ -669,30 +724,21 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
 
 
     /* Do any register specific pre-write processing here.  */
-#if EHCI_DEBUG
-    str = addr2str((unsigned) addr);
-#endif
     switch(addr) {
     case USBCMD:
-        DPRINTF("ehci_mem_writel: USBCMD val=0x%08X, current cmd=0x%08X\n",
-                val, s->usbcmd);
-
         if ((val & USBCMD_RUNSTOP) && !(s->usbcmd & USBCMD_RUNSTOP)) {
-            DPRINTF("ehci_mem_writel: %s run, clear halt\n", str);
             qemu_mod_timer(s->frame_timer, qemu_get_clock_ns(vm_clock));
             SET_LAST_RUN_CLOCK(s);
-            s->usbsts &= ~USBSTS_HALT;
+            ehci_clear_usbsts(s, USBSTS_HALT);
         }
 
         if (!(val & USBCMD_RUNSTOP) && (s->usbcmd & USBCMD_RUNSTOP)) {
-            DPRINTF("                         ** STOP **\n");
             qemu_del_timer(s->frame_timer);
             // TODO - should finish out some stuff before setting halt
-            s->usbsts |= USBSTS_HALT;
+            ehci_set_usbsts(s, USBSTS_HALT);
         }
 
         if (val & USBCMD_HCRESET) {
-            DPRINTF("ehci_mem_writel: %s run, resetting\n", str);
             ehci_reset(s);
             val &= ~USBCMD_HCRESET;
         }
@@ -703,56 +749,24 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
                     val & USBCMD_FLS);
             val &= ~USBCMD_FLS;
         }
-#if EHCI_DEBUG
-        if ((val & USBCMD_PSE) && !(s->usbcmd & USBCMD_PSE)) {
-            DPRINTF("periodic scheduling enabled\n");
-        }
-        if (!(val & USBCMD_PSE) && (s->usbcmd & USBCMD_PSE)) {
-            DPRINTF("periodic scheduling disabled\n");
-        }
-        if ((val & USBCMD_ASE) && !(s->usbcmd & USBCMD_ASE)) {
-            DPRINTF("asynchronous scheduling enabled\n");
-        }
-        if (!(val & USBCMD_ASE) && (s->usbcmd & USBCMD_ASE)) {
-            DPRINTF("asynchronous scheduling disabled\n");
-        }
-        if ((val & USBCMD_IAAD) && !(s->usbcmd & USBCMD_IAAD)) {
-            DPRINTF("doorbell request received\n");
-        }
-        if ((val & USBCMD_LHCR) && !(s->usbcmd & USBCMD_LHCR)) {
-            DPRINTF("light host controller reset received\n");
-        }
-        if ((val & USBCMD_ITC) != (s->usbcmd & USBCMD_ITC)) {
-            DPRINTF("interrupt threshold control set to %x\n",
-                    (val & USBCMD_ITC)>>USBCMD_ITC_SH);
-        }
-#endif
         break;
 
-
     case USBSTS:
         val &= USBSTS_RO_MASK;              // bits 6 thru 31 are RO
-        DPRINTF("ehci_mem_writel: %s RWC set to 0x%08X\n", str, val);
-
-        val = (s->usbsts &= ~val);         // bits 0 thru 5 are R/WC
-
-        DPRINTF("ehci_mem_writel: %s updating interrupt condition\n", str);
+        ehci_clear_usbsts(s, val);          // bits 0 thru 5 are R/WC
+        val = s->usbsts;
         ehci_set_interrupt(s, 0);
         break;
 
-
     case USBINTR:
         val &= USBINTR_MASK;
-        DPRINTF("ehci_mem_writel: %s set to 0x%08X\n", str, val);
         break;
 
     case FRINDEX:
         s->sofv = val >> 3;
-        DPRINTF("ehci_mem_writel: %s set to 0x%08X\n", str, val);
         break;
 
     case CONFIGFLAG:
-        DPRINTF("ehci_mem_writel: %s set to 0x%08X\n", str, val);
         val &= 0x1;
         if (val) {
             for(i = 0; i < NB_PORTS; i++)
@@ -766,7 +780,6 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
               "ehci: PERIODIC list base register set while periodic schedule\n"
               "      is enabled and HC is enabled\n");
         }
-        DPRINTF("ehci_mem_writel: P-LIST BASE set to 0x%08X\n", val);
         break;
 
     case ASYNCLISTADDR:
@@ -775,7 +788,6 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
               "ehci: ASYNC list address register set while async schedule\n"
               "      is enabled and HC is enabled\n");
         }
-        DPRINTF("ehci_mem_writel: A-LIST ADDR set to 0x%08X\n", val);
         break;
     }
 
@@ -1227,7 +1239,7 @@ static int ehci_state_waitlisthead(EHCIState *ehci,  int async, int *state)
 
     /* set reclamation flag at start event (4.8.6) */
     if (async) {
-        ehci->usbsts |= USBSTS_REC;
+        ehci_set_usbsts(ehci, USBSTS_REC);
     }
 
     /*  Find the head of the list (4.9.1.1) */
@@ -1334,7 +1346,7 @@ static int ehci_state_fetchqh(EHCIState *ehci, int async, int *state)
 
         /*  EHCI spec version 1.0 Section 4.8.3 & 4.10.1 */
         if (ehci->usbsts & USBSTS_REC) {
-            ehci->usbsts &= ~USBSTS_REC;
+            ehci_clear_usbsts(ehci, USBSTS_REC);
         } else {
             DPRINTF("FETCHQH:  QH 0x%08x. H-bit set, reclamation status reset"
                        " - done processing\n", ehci->qhaddr);
@@ -1534,7 +1546,7 @@ static int ehci_state_execute(EHCIState *ehci, int async, int *state)
     }
 
     if (async) {
-        ehci->usbsts |= USBSTS_REC;
+        ehci_set_usbsts(ehci, USBSTS_REC);
     }
 
     ehci->exec_status = ehci_execute(ehci, qh);
@@ -1734,13 +1746,13 @@ static void ehci_advance_async_state(EHCIState *ehci)
         if (!(ehci->usbcmd & USBCMD_ASE)) {
             break;
         }
-        ehci->usbsts |= USBSTS_ASS;
+        ehci_set_usbsts(ehci, USBSTS_ASS);
         ehci->astate = EST_ACTIVE;
         // No break, fall through to ACTIVE
 
     case EST_ACTIVE:
         if ( !(ehci->usbcmd & USBCMD_ASE)) {
-            ehci->usbsts &= ~USBSTS_ASS;
+            ehci_clear_usbsts(ehci, USBSTS_ASS);
             ehci->astate = EST_INACTIVE;
             break;
         }
@@ -1800,8 +1812,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
     switch(ehci->pstate) {
     case EST_INACTIVE:
         if ( !(ehci->frindex & 7) && (ehci->usbcmd & USBCMD_PSE)) {
-            DPRINTF("PERIODIC going active\n");
-            ehci->usbsts |= USBSTS_PSS;
+            ehci_set_usbsts(ehci, USBSTS_PSS);
             ehci->pstate = EST_ACTIVE;
             // No break, fall through to ACTIVE
         } else
@@ -1809,8 +1820,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
 
     case EST_ACTIVE:
         if ( !(ehci->frindex & 7) && !(ehci->usbcmd & USBCMD_PSE)) {
-            DPRINTF("PERIODIC going inactive\n");
-            ehci->usbsts &= ~USBSTS_PSS;
+            ehci_clear_usbsts(ehci, USBSTS_PSS);
             ehci->pstate = EST_INACTIVE;
             break;
         }
diff --git a/trace-events b/trace-events
index 385cb00..c63e6f9 100644
--- a/trace-events
+++ b/trace-events
@@ -194,6 +194,12 @@ disable sun4m_iommu_page_get_flags(uint64_t pa, uint64_t iopte, uint32_t ret) "g
 disable sun4m_iommu_translate_pa(uint64_t addr, uint64_t pa, uint32_t iopte) "xlate dva %"PRIx64" => pa %"PRIx64" iopte = %x"
 disable sun4m_iommu_bad_addr(uint64_t addr) "bad addr %"PRIx64""
 
+# hw/usb-ehci.c
+disable usb_ehci_reset(void) "=== RESET ==="
+disable usb_ehci_mmio_readl(uint32_t addr, const char *str, uint32_t val) "rd mmio %04x [%s] = %x"
+disable usb_ehci_mmio_writel(uint32_t addr, const char *str, uint32_t val, uint32_t oldval) "wr mmio %04x [%s] = %x (old: %x)"
+disable usb_ehci_usbsts(const char *sts, int state) "usbsts %s %d"
+
 # hw/usb-desc.c
 disable usb_desc_device(int addr, int len, int ret) "dev %d query device, len %d, ret %d"
 disable usb_desc_device_qualifier(int addr, int len, int ret) "dev %d query device qualifier, len %d, ret %d"
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 03/12] usb-ehci: trace state machine changes
  2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 01/12] usb-linux: catch NODEV in more places Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 02/12] usb-ehci: trace mmio and usbsts Gerd Hoffmann
@ 2011-05-26 10:44 ` Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 04/12] usb-ehci: trace port state Gerd Hoffmann
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Add functions to get and set the current state of the state machine,
add tracepoints there to trace state transitions.  Add support for
traceing the queue heads and transfer descriptors as we look at them.

Drop a few DPRINTFs and all DPRINTF_ST lines, they are obsolete now.

No change in behavior.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-ehci.c |  307 +++++++++++++++++++++++++++++++-------------------------
 trace-events  |    4 +
 2 files changed, 174 insertions(+), 137 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 16fbca6..40a447b 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -33,20 +33,13 @@
 #include "trace.h"
 
 #define EHCI_DEBUG   0
-#define STATE_DEBUG  0       /* state transitions  */
 
-#if EHCI_DEBUG || STATE_DEBUG
+#if EHCI_DEBUG
 #define DPRINTF printf
 #else
 #define DPRINTF(...)
 #endif
 
-#if STATE_DEBUG
-#define DPRINTF_ST DPRINTF
-#else
-#define DPRINTF_ST(...)
-#endif
-
 /* internal processing - reset HC to try and recover */
 #define USB_RET_PROCERR   (-99)
 
@@ -417,33 +410,59 @@ typedef struct {
     *data = val; \
     } while(0)
 
+static const char *ehci_state_names[] = {
+    [ EST_INACTIVE ]     = "INACTIVE",
+    [ EST_ACTIVE ]       = "ACTIVE",
+    [ EST_EXECUTING ]    = "EXECUTING",
+    [ EST_SLEEPING ]     = "SLEEPING",
+    [ EST_WAITLISTHEAD ] = "WAITLISTHEAD",
+    [ EST_FETCHENTRY ]   = "FETCH ENTRY",
+    [ EST_FETCHQH ]      = "FETCH QH",
+    [ EST_FETCHITD ]     = "FETCH ITD",
+    [ EST_ADVANCEQUEUE ] = "ADVANCEQUEUE",
+    [ EST_FETCHQTD ]     = "FETCH QTD",
+    [ EST_EXECUTE ]      = "EXECUTE",
+    [ EST_WRITEBACK ]    = "WRITEBACK",
+    [ EST_HORIZONTALQH ] = "HORIZONTALQH",
+};
+
+static const char *ehci_mmio_names[] = {
+    [ CAPLENGTH ]        = "CAPLENGTH",
+    [ HCIVERSION ]       = "HCIVERSION",
+    [ HCSPARAMS ]        = "HCSPARAMS",
+    [ HCCPARAMS ]        = "HCCPARAMS",
+    [ USBCMD ]           = "USBCMD",
+    [ USBSTS ]           = "USBSTS",
+    [ USBINTR ]          = "USBINTR",
+    [ FRINDEX ]          = "FRINDEX",
+    [ PERIODICLISTBASE ] = "P-LIST BASE",
+    [ ASYNCLISTADDR ]    = "A-LIST ADDR",
+    [ PORTSC_BEGIN ]     = "PORTSC #0",
+    [ PORTSC_BEGIN + 4]  = "PORTSC #1",
+    [ PORTSC_BEGIN + 8]  = "PORTSC #2",
+    [ PORTSC_BEGIN + 12] = "PORTSC #3",
+    [ CONFIGFLAG ]       = "CONFIGFLAG",
+};
 
-static const char *addr2str(target_phys_addr_t addr)
+static const char *nr2str(const char **n, size_t len, uint32_t nr)
 {
-    const char *r            = "unknown";
-    const char *n[] = {
-        [ CAPLENGTH ]        = "CAPLENGTH",
-        [ HCIVERSION ]       = "HCIVERSION",
-        [ HCSPARAMS ]        = "HCSPARAMS",
-        [ HCCPARAMS ]        = "HCCPARAMS",
-        [ USBCMD ]           = "USBCMD",
-        [ USBSTS ]           = "USBSTS",
-        [ USBINTR ]          = "USBINTR",
-        [ FRINDEX ]          = "FRINDEX",
-        [ PERIODICLISTBASE ] = "P-LIST BASE",
-        [ ASYNCLISTADDR ]    = "A-LIST ADDR",
-        [ PORTSC_BEGIN ...
-          PORTSC_END ]       = "PORTSC",
-        [ CONFIGFLAG ]       = "CONFIGFLAG",
-    };
-
-    if (addr < ARRAY_SIZE(n) && n[addr] != NULL) {
-        return n[addr];
+    if (nr < len && n[nr] != NULL) {
+        return n[nr];
     } else {
-        return r;
+        return "unknown";
     }
 }
 
+static const char *state2str(uint32_t state)
+{
+    return nr2str(ehci_state_names, ARRAY_SIZE(ehci_state_names), state);
+}
+
+static const char *addr2str(target_phys_addr_t addr)
+{
+    return nr2str(ehci_mmio_names, ARRAY_SIZE(ehci_mmio_names), addr);
+}
+
 static void ehci_trace_usbsts(uint32_t mask, int state)
 {
     /* interrupts */
@@ -528,6 +547,56 @@ static inline void ehci_commit_interrupt(EHCIState *s)
     s->usbsts_pending = 0;
 }
 
+static void ehci_set_state(EHCIState *s, int async, int state)
+{
+    if (async) {
+        trace_usb_ehci_state("async", state2str(state));
+        s->astate = state;
+    } else {
+        trace_usb_ehci_state("periodic", state2str(state));
+        s->pstate = state;
+    }
+}
+
+static int ehci_get_state(EHCIState *s, int async)
+{
+    return async ? s->astate : s->pstate;
+}
+
+static void ehci_trace_qh(EHCIState *s, target_phys_addr_t addr, EHCIqh *qh)
+{
+    trace_usb_ehci_qh(addr, qh->next,
+                      qh->current_qtd, qh->next_qtd, qh->altnext_qtd,
+                      get_field(qh->epchar, QH_EPCHAR_RL),
+                      get_field(qh->epchar, QH_EPCHAR_MPLEN),
+                      get_field(qh->epchar, QH_EPCHAR_EPS),
+                      get_field(qh->epchar, QH_EPCHAR_EP),
+                      get_field(qh->epchar, QH_EPCHAR_DEVADDR),
+                      (bool)(qh->epchar & QH_EPCHAR_C),
+                      (bool)(qh->epchar & QH_EPCHAR_H),
+                      (bool)(qh->epchar & QH_EPCHAR_DTC),
+                      (bool)(qh->epchar & QH_EPCHAR_I));
+}
+
+static void ehci_trace_qtd(EHCIState *s, target_phys_addr_t addr, EHCIqtd *qtd)
+{
+    trace_usb_ehci_qtd(addr, qtd->next, qtd->altnext,
+                       get_field(qtd->token, QTD_TOKEN_TBYTES),
+                       get_field(qtd->token, QTD_TOKEN_CPAGE),
+                       get_field(qtd->token, QTD_TOKEN_CERR),
+                       get_field(qtd->token, QTD_TOKEN_PID),
+                       (bool)(qtd->token & QTD_TOKEN_IOC),
+                       (bool)(qtd->token & QTD_TOKEN_ACTIVE),
+                       (bool)(qtd->token & QTD_TOKEN_HALT),
+                       (bool)(qtd->token & QTD_TOKEN_BABBLE),
+                       (bool)(qtd->token & QTD_TOKEN_XACTERR));
+}
+
+static void ehci_trace_itd(EHCIState *s, target_phys_addr_t addr, EHCIitd *itd)
+{
+    trace_usb_ehci_itd(addr, itd->next);
+}
+
 /* Attach or detach a device on root hub */
 
 static void ehci_attach(USBPort *port)
@@ -1230,7 +1299,7 @@ static int ehci_process_itd(EHCIState *ehci,
 /*  This state is the entry point for asynchronous schedule
  *  processing.  Entry here consitutes a EHCI start event state (4.8.5)
  */
-static int ehci_state_waitlisthead(EHCIState *ehci,  int async, int *state)
+static int ehci_state_waitlisthead(EHCIState *ehci,  int async)
 {
     EHCIqh *qh = &ehci->qh;
     int i = 0;
@@ -1245,32 +1314,28 @@ static int ehci_state_waitlisthead(EHCIState *ehci,  int async, int *state)
     /*  Find the head of the list (4.9.1.1) */
     for(i = 0; i < MAX_QH; i++) {
         get_dwords(NLPTR_GET(entry), (uint32_t *) qh, sizeof(EHCIqh) >> 2);
+        ehci_trace_qh(ehci, NLPTR_GET(entry), qh);
 
         if (qh->epchar & QH_EPCHAR_H) {
-            DPRINTF_ST("WAITLISTHEAD: QH %08X is the HEAD of the list\n",
-                       entry);
             if (async) {
                 entry |= (NLPTR_TYPE_QH << 1);
             }
 
             ehci->fetch_addr = entry;
-            *state = EST_FETCHENTRY;
+            ehci_set_state(ehci, async, EST_FETCHENTRY);
             again = 1;
             goto out;
         }
 
-        DPRINTF_ST("WAITLISTHEAD: QH %08X is NOT the HEAD of the list\n",
-                   entry);
         entry = qh->next;
         if (entry == ehci->asynclistaddr) {
-            DPRINTF("WAITLISTHEAD: reached beginning of QH list\n");
             break;
         }
     }
 
     /* no head found for list. */
 
-    *state = EST_ACTIVE;
+    ehci_set_state(ehci, async, EST_ACTIVE);
 
 out:
     return again;
@@ -1280,7 +1345,7 @@ out:
 /*  This state is the entry point for periodic schedule processing as
  *  well as being a continuation state for async processing.
  */
-static int ehci_state_fetchentry(EHCIState *ehci, int async, int *state)
+static int ehci_state_fetchentry(EHCIState *ehci, int async)
 {
     int again = 0;
     uint32_t entry = ehci->fetch_addr;
@@ -1298,7 +1363,7 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async, int *state)
 #endif
     if (entry < 0x1000) {
         DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry);
-        *state = EST_ACTIVE;
+        ehci_set_state(ehci, async, EST_ACTIVE);
         goto out;
     }
 
@@ -1310,15 +1375,13 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async, int *state)
 
     switch (NLPTR_TYPE_GET(entry)) {
     case NLPTR_TYPE_QH:
-        DPRINTF_ST("FETCHENTRY: entry %X is a Queue Head\n", entry);
-        *state = EST_FETCHQH;
+        ehci_set_state(ehci, async, EST_FETCHQH);
         ehci->qhaddr = entry;
         again = 1;
         break;
 
     case NLPTR_TYPE_ITD:
-        DPRINTF_ST("FETCHENTRY: entry %X is an ITD\n", entry);
-        *state = EST_FETCHITD;
+        ehci_set_state(ehci, async, EST_FETCHITD);
         ehci->itdaddr = entry;
         again = 1;
         break;
@@ -1334,13 +1397,14 @@ out:
     return again;
 }
 
-static int ehci_state_fetchqh(EHCIState *ehci, int async, int *state)
+static int ehci_state_fetchqh(EHCIState *ehci, int async)
 {
     EHCIqh *qh = &ehci->qh;
     int reload;
     int again = 0;
 
     get_dwords(NLPTR_GET(ehci->qhaddr), (uint32_t *) qh, sizeof(EHCIqh) >> 2);
+    ehci_trace_qh(ehci, NLPTR_GET(ehci->qhaddr), qh);
 
     if (async && (qh->epchar & QH_EPCHAR_H)) {
 
@@ -1350,7 +1414,7 @@ static int ehci_state_fetchqh(EHCIState *ehci, int async, int *state)
         } else {
             DPRINTF("FETCHQH:  QH 0x%08x. H-bit set, reclamation status reset"
                        " - done processing\n", ehci->qhaddr);
-            *state = EST_ACTIVE;
+            ehci_set_state(ehci, async, EST_ACTIVE);
             goto out;
         }
     }
@@ -1368,25 +1432,21 @@ static int ehci_state_fetchqh(EHCIState *ehci, int async, int *state)
 
     reload = get_field(qh->epchar, QH_EPCHAR_RL);
     if (reload) {
-        DPRINTF_ST("FETCHQH: reloading nakcnt to %d\n", reload);
         set_field(&qh->altnext_qtd, reload, QH_ALTNEXT_NAKCNT);
     }
 
     if (qh->token & QTD_TOKEN_HALT) {
-        DPRINTF_ST("FETCHQH: QH Halted, go horizontal\n");
-        *state = EST_HORIZONTALQH;
+        ehci_set_state(ehci, async, EST_HORIZONTALQH);
         again = 1;
 
     } else if ((qh->token & QTD_TOKEN_ACTIVE) && (qh->current_qtd > 0x1000)) {
-        DPRINTF_ST("FETCHQH: Active, !Halt, execute - fetch qTD\n");
         ehci->qtdaddr = qh->current_qtd;
-        *state = EST_FETCHQTD;
+        ehci_set_state(ehci, async, EST_FETCHQTD);
         again = 1;
 
     } else {
         /*  EHCI spec version 1.0 Section 4.10.2 */
-        DPRINTF_ST("FETCHQH: !Active, !Halt, advance queue\n");
-        *state = EST_ADVANCEQUEUE;
+        ehci_set_state(ehci, async, EST_ADVANCEQUEUE);
         again = 1;
     }
 
@@ -1394,14 +1454,13 @@ out:
     return again;
 }
 
-static int ehci_state_fetchitd(EHCIState *ehci, int async, int *state)
+static int ehci_state_fetchitd(EHCIState *ehci, int async)
 {
     EHCIitd itd;
 
     get_dwords(NLPTR_GET(ehci->itdaddr),(uint32_t *) &itd,
                sizeof(EHCIitd) >> 2);
-    DPRINTF_ST("FETCHITD: Fetched ITD at address %08X " "(next is %08X)\n",
-               ehci->itdaddr, itd.next);
+    ehci_trace_itd(ehci, ehci->itdaddr, &itd);
 
     if (ehci_process_itd(ehci, &itd) != 0) {
         return -1;
@@ -1410,13 +1469,13 @@ static int ehci_state_fetchitd(EHCIState *ehci, int async, int *state)
     put_dwords(NLPTR_GET(ehci->itdaddr), (uint32_t *) &itd,
                 sizeof(EHCIitd) >> 2);
     ehci->fetch_addr = itd.next;
-    *state = EST_FETCHENTRY;
+    ehci_set_state(ehci, async, EST_FETCHENTRY);
 
     return 1;
 }
 
 /* Section 4.10.2 - paragraph 3 */
-static int ehci_state_advqueue(EHCIState *ehci, int async, int *state)
+static int ehci_state_advqueue(EHCIState *ehci, int async)
 {
 #if 0
     /* TO-DO: 4.10.2 - paragraph 2
@@ -1424,7 +1483,7 @@ static int ehci_state_advqueue(EHCIState *ehci, int async, int *state)
      * go to horizontal QH
      */
     if (I-bit set) {
-        *state = EST_HORIZONTALQH;
+        ehci_set_state(ehci, async, EST_HORIZONTALQH);
         goto out;
     }
 #endif
@@ -1435,71 +1494,63 @@ static int ehci_state_advqueue(EHCIState *ehci, int async, int *state)
     if (((ehci->qh.token & QTD_TOKEN_TBYTES_MASK) != 0) &&
         (ehci->qh.altnext_qtd > 0x1000) &&
         (NLPTR_TBIT(ehci->qh.altnext_qtd) == 0)) {
-        DPRINTF_ST("ADVQUEUE: goto alt next qTD. "
-                   "curr 0x%08x next 0x%08x alt 0x%08x (next qh %x)\n",
-                   ehci->qh.current_qtd, ehci->qh.altnext_qtd,
-                   ehci->qh.next_qtd, ehci->qh.next);
         ehci->qtdaddr = ehci->qh.altnext_qtd;
-        *state = EST_FETCHQTD;
+        ehci_set_state(ehci, async, EST_FETCHQTD);
 
     /*
      *  next qTD is valid
      */
     } else if ((ehci->qh.next_qtd > 0x1000) &&
                (NLPTR_TBIT(ehci->qh.next_qtd) == 0)) {
-        DPRINTF_ST("ADVQUEUE: next qTD. "
-                   "curr 0x%08x next 0x%08x alt 0x%08x (next qh %x)\n",
-                   ehci->qh.current_qtd, ehci->qh.altnext_qtd,
-                   ehci->qh.next_qtd, ehci->qh.next);
         ehci->qtdaddr = ehci->qh.next_qtd;
-        *state = EST_FETCHQTD;
+        ehci_set_state(ehci, async, EST_FETCHQTD);
 
     /*
      *  no valid qTD, try next QH
      */
     } else {
-        DPRINTF_ST("ADVQUEUE: go to horizontal QH\n");
-        *state = EST_HORIZONTALQH;
+        ehci_set_state(ehci, async, EST_HORIZONTALQH);
     }
 
     return 1;
 }
 
 /* Section 4.10.2 - paragraph 4 */
-static int ehci_state_fetchqtd(EHCIState *ehci, int async, int *state)
+static int ehci_state_fetchqtd(EHCIState *ehci, int async)
 {
     EHCIqtd *qtd = &ehci->qtd;
     int again = 0;
 
     get_dwords(NLPTR_GET(ehci->qtdaddr),(uint32_t *) qtd, sizeof(EHCIqtd) >> 2);
+    ehci_trace_qtd(ehci, NLPTR_GET(ehci->qtdaddr), qtd);
 
     if (qtd->token & QTD_TOKEN_ACTIVE) {
-        *state = EST_EXECUTE;
+        ehci_set_state(ehci, async, EST_EXECUTE);
         again = 1;
     } else {
-        *state = EST_HORIZONTALQH;
+        ehci_set_state(ehci, async, EST_HORIZONTALQH);
         again = 1;
     }
 
     return again;
 }
 
-static int ehci_state_horizqh(EHCIState *ehci, int async, int *state)
+static int ehci_state_horizqh(EHCIState *ehci, int async)
 {
     int again = 0;
 
     if (ehci->fetch_addr != ehci->qh.next) {
         ehci->fetch_addr = ehci->qh.next;
-        *state = EST_FETCHENTRY;
+        ehci_set_state(ehci, async, EST_FETCHENTRY);
         again = 1;
     } else {
-        *state = EST_ACTIVE;
+        ehci_set_state(ehci, async, EST_ACTIVE);
     }
 
     return again;
 }
 
-static int ehci_state_execute(EHCIState *ehci, int async, int *state)
+static int ehci_state_execute(EHCIState *ehci, int async)
 {
     EHCIqh *qh = &ehci->qh;
     EHCIqtd *qtd = &ehci->qtd;
@@ -1507,13 +1558,6 @@ static int ehci_state_execute(EHCIState *ehci, int async, int *state)
     int reload, nakcnt;
     int smask;
 
-    if (async) {
-        DPRINTF_ST(">>>>> ASYNC STATE MACHINE execute QH 0x%08x, QTD 0x%08x\n",
-                  ehci->qhaddr, ehci->qtdaddr);
-    } else {
-        DPRINTF_ST(">>>>> PERIODIC STATE MACHINE execute\n");
-    }
-
     if (ehci_qh_do_overlay(ehci, qh, qtd) != 0) {
         return -1;
     }
@@ -1524,8 +1568,7 @@ static int ehci_state_execute(EHCIState *ehci, int async, int *state)
         reload = get_field(qh->epchar, QH_EPCHAR_RL);
         nakcnt = get_field(qh->altnext_qtd, QH_ALTNEXT_NAKCNT);
         if (reload && !nakcnt) {
-            DPRINTF_ST("EXECUTE: RL != 0 but NakCnt == 0 -- no execute\n");
-            *state = EST_HORIZONTALQH;
+            ehci_set_state(ehci, async, EST_HORIZONTALQH);
             again = 1;
             goto out;
         }
@@ -1538,8 +1581,7 @@ static int ehci_state_execute(EHCIState *ehci, int async, int *state)
     if (!async) {
         int transactCtr = get_field(qh->epcap, QH_EPCAP_MULT);
         if (!transactCtr) {
-            DPRINTF("ZERO transactctr for int qh, go HORIZ\n");
-            *state = EST_HORIZONTALQH;
+            ehci_set_state(ehci, async, EST_HORIZONTALQH);
             again = 1;
             goto out;
         }
@@ -1554,7 +1596,7 @@ static int ehci_state_execute(EHCIState *ehci, int async, int *state)
         again = -1;
         goto out;
     }
-    *state = EST_EXECUTING;
+    ehci_set_state(ehci, async, EST_EXECUTING);
 
     if (ehci->exec_status != USB_RET_ASYNC) {
         again = 1;
@@ -1564,7 +1606,7 @@ out:
     return again;
 }
 
-static int ehci_state_executing(EHCIState *ehci, int async, int *state)
+static int ehci_state_executing(EHCIState *ehci, int async)
 {
     EHCIqh *qh = &ehci->qh;
     int again = 0;
@@ -1596,12 +1638,8 @@ static int ehci_state_executing(EHCIState *ehci, int async, int *state)
             if (nakcnt) {
                 nakcnt--;
             }
-            DPRINTF_ST("EXECUTING: Nak occured and RL != 0, dec NakCnt to %d\n",
-                    nakcnt);
         } else {
             nakcnt = reload;
-            DPRINTF_ST("EXECUTING: Nak didn't occur, reloading to %d\n",
-                       nakcnt);
         }
         set_field(&qh->altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);
     }
@@ -1611,16 +1649,13 @@ static int ehci_state_executing(EHCIState *ehci, int async, int *state)
      *  in the EHCI spec but we need to do it since we don't share
      *  physical memory with our guest VM.
      */
-
-    DPRINTF("EXECUTING: write QH to VM memory: qhaddr 0x%x, next 0x%x\n",
-              ehci->qhaddr, qh->next);
     put_dwords(NLPTR_GET(ehci->qhaddr), (uint32_t *) qh, sizeof(EHCIqh) >> 2);
 
     /* 4.10.5 */
     if ((ehci->exec_status == USB_RET_NAK) || (qh->token & QTD_TOKEN_ACTIVE)) {
-        *state = EST_HORIZONTALQH;
+        ehci_set_state(ehci, async, EST_HORIZONTALQH);
     } else {
-        *state = EST_WRITEBACK;
+        ehci_set_state(ehci, async, EST_WRITEBACK);
     }
 
     again = 1;
@@ -1630,13 +1665,13 @@ out:
 }
 
 
-static int ehci_state_writeback(EHCIState *ehci, int async, int *state)
+static int ehci_state_writeback(EHCIState *ehci, int async)
 {
     EHCIqh *qh = &ehci->qh;
     int again = 0;
 
     /*  Write back the QTD from the QH area */
-    DPRINTF_ST("WRITEBACK: write QTD to VM memory\n");
+    ehci_trace_qtd(ehci, NLPTR_GET(ehci->qtdaddr), (EHCIqtd*) &qh->next_qtd);
     put_dwords(NLPTR_GET(ehci->qtdaddr),(uint32_t *) &qh->next_qtd,
                 sizeof(EHCIqtd) >> 2);
 
@@ -1644,10 +1679,10 @@ static int ehci_state_writeback(EHCIState *ehci, int async, int *state)
      * but stop after one qtd if periodic
      */
     //if (async) {
-        *state = EST_ADVANCEQUEUE;
+        ehci_set_state(ehci, async, EST_ADVANCEQUEUE);
         again = 1;
     //} else {
-    //    *state = EST_ACTIVE;
+    //    ehci_set_state(ehci, async, EST_ACTIVE);
     //}
     return again;
 }
@@ -1656,15 +1691,14 @@ static int ehci_state_writeback(EHCIState *ehci, int async, int *state)
  * This is the state machine that is common to both async and periodic
  */
 
-static int ehci_advance_state(EHCIState *ehci,
-                              int async,
-                              int state)
+static void ehci_advance_state(EHCIState *ehci,
+                               int async)
 {
     int again;
     int iter = 0;
 
     do {
-        if (state == EST_FETCHQH) {
+        if (ehci_get_state(ehci, async) == EST_FETCHQH) {
             iter++;
             /* if we are roaming a lot of QH without executing a qTD
              * something is wrong with the linked list. TO-DO: why is
@@ -1672,50 +1706,50 @@ static int ehci_advance_state(EHCIState *ehci,
              */
             if (iter > MAX_ITERATIONS) {
                 DPRINTF("\n*** advance_state: bailing on MAX ITERATIONS***\n");
-                state = EST_ACTIVE;
+                ehci_set_state(ehci, async, EST_ACTIVE);
                 break;
             }
         }
-        switch(state) {
+        switch(ehci_get_state(ehci, async)) {
         case EST_WAITLISTHEAD:
-            again = ehci_state_waitlisthead(ehci, async, &state);
+            again = ehci_state_waitlisthead(ehci, async);
             break;
 
         case EST_FETCHENTRY:
-            again = ehci_state_fetchentry(ehci, async, &state);
+            again = ehci_state_fetchentry(ehci, async);
             break;
 
         case EST_FETCHQH:
-            again = ehci_state_fetchqh(ehci, async, &state);
+            again = ehci_state_fetchqh(ehci, async);
             break;
 
         case EST_FETCHITD:
-            again = ehci_state_fetchitd(ehci, async, &state);
+            again = ehci_state_fetchitd(ehci, async);
             break;
 
         case EST_ADVANCEQUEUE:
-            again = ehci_state_advqueue(ehci, async, &state);
+            again = ehci_state_advqueue(ehci, async);
             break;
 
         case EST_FETCHQTD:
-            again = ehci_state_fetchqtd(ehci, async, &state);
+            again = ehci_state_fetchqtd(ehci, async);
             break;
 
         case EST_HORIZONTALQH:
-            again = ehci_state_horizqh(ehci, async, &state);
+            again = ehci_state_horizqh(ehci, async);
             break;
 
         case EST_EXECUTE:
             iter = 0;
-            again = ehci_state_execute(ehci, async, &state);
+            again = ehci_state_execute(ehci, async);
             break;
 
         case EST_EXECUTING:
-            again = ehci_state_executing(ehci, async, &state);
+            again = ehci_state_executing(ehci, async);
             break;
 
         case EST_WRITEBACK:
-            again = ehci_state_writeback(ehci, async, &state);
+            again = ehci_state_writeback(ehci, async);
             break;
 
         default:
@@ -1733,27 +1767,26 @@ static int ehci_advance_state(EHCIState *ehci,
     while (again);
 
     ehci_commit_interrupt(ehci);
-    return state;
 }
 
 static void ehci_advance_async_state(EHCIState *ehci)
 {
     EHCIqh qh;
-    int state = ehci->astate;
+    int async = 1;
 
-    switch(state) {
+    switch(ehci_get_state(ehci, async)) {
     case EST_INACTIVE:
         if (!(ehci->usbcmd & USBCMD_ASE)) {
             break;
         }
         ehci_set_usbsts(ehci, USBSTS_ASS);
-        ehci->astate = EST_ACTIVE;
+        ehci_set_state(ehci, async, EST_ACTIVE);
         // No break, fall through to ACTIVE
 
     case EST_ACTIVE:
         if ( !(ehci->usbcmd & USBCMD_ASE)) {
             ehci_clear_usbsts(ehci, USBSTS_ASS);
-            ehci->astate = EST_INACTIVE;
+            ehci_set_state(ehci, async, EST_INACTIVE);
             break;
         }
 
@@ -1775,14 +1808,12 @@ static void ehci_advance_async_state(EHCIState *ehci)
             break;
         }
 
-        DPRINTF_ST("ASYNC: waiting for listhead, starting at %08x\n",
-                ehci->asynclistaddr);
         /* check that address register has been set */
         if (ehci->asynclistaddr == 0) {
             break;
         }
 
-        state = EST_WAITLISTHEAD;
+        ehci_set_state(ehci, async, EST_WAITLISTHEAD);
         /* fall through */
 
     case EST_FETCHENTRY:
@@ -1791,14 +1822,14 @@ static void ehci_advance_async_state(EHCIState *ehci)
     case EST_EXECUTING:
         get_dwords(NLPTR_GET(ehci->qhaddr), (uint32_t *) &qh,
                    sizeof(EHCIqh) >> 2);
-        ehci->astate = ehci_advance_state(ehci, 1, state);
+        ehci_advance_state(ehci, async);
         break;
 
     default:
         /* this should only be due to a developer mistake */
         fprintf(stderr, "ehci: Bad asynchronous state %d. "
                 "Resetting to active\n", ehci->astate);
-        ehci->astate = EST_ACTIVE;
+        ehci_set_state(ehci, async, EST_ACTIVE);
     }
 }
 
@@ -1806,14 +1837,15 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
 {
     uint32_t entry;
     uint32_t list;
+    int async = 0;
 
     // 4.6
 
-    switch(ehci->pstate) {
+    switch(ehci_get_state(ehci, async)) {
     case EST_INACTIVE:
         if ( !(ehci->frindex & 7) && (ehci->usbcmd & USBCMD_PSE)) {
             ehci_set_usbsts(ehci, USBSTS_PSS);
-            ehci->pstate = EST_ACTIVE;
+            ehci_set_state(ehci, async, EST_ACTIVE);
             // No break, fall through to ACTIVE
         } else
             break;
@@ -1821,7 +1853,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
     case EST_ACTIVE:
         if ( !(ehci->frindex & 7) && !(ehci->usbcmd & USBCMD_PSE)) {
             ehci_clear_usbsts(ehci, USBSTS_PSS);
-            ehci->pstate = EST_INACTIVE;
+            ehci_set_state(ehci, async, EST_INACTIVE);
             break;
         }
 
@@ -1838,19 +1870,20 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
         DPRINTF("PERIODIC state adv fr=%d.  [%08X] -> %08X\n",
                 ehci->frindex / 8, list, entry);
         ehci->fetch_addr = entry;
-        ehci->pstate = ehci_advance_state(ehci, 0, EST_FETCHENTRY);
+        ehci_set_state(ehci, async, EST_FETCHENTRY);
+        ehci_advance_state(ehci, async);
         break;
 
     case EST_EXECUTING:
         DPRINTF("PERIODIC state adv for executing\n");
-        ehci->pstate = ehci_advance_state(ehci, 0, EST_EXECUTING);
+        ehci_advance_state(ehci, async);
         break;
 
     default:
         /* this should only be due to a developer mistake */
         fprintf(stderr, "ehci: Bad periodic state %d. "
                 "Resetting to active\n", ehci->pstate);
-        ehci->pstate = EST_ACTIVE;
+        ehci_set_state(ehci, async, EST_ACTIVE);
     }
 }
 
@@ -1896,7 +1929,7 @@ static void ehci_frame_timer(void *opaque)
         } else {
             // TODO could this cause periodic frames to get skipped if async
             // active?
-            if (ehci->astate != EST_EXECUTING) {
+            if (ehci_get_state(ehci, 1) != EST_EXECUTING) {
                 ehci_advance_periodic_state(ehci);
             }
         }
@@ -1913,7 +1946,7 @@ static void ehci_frame_timer(void *opaque)
     /*  Async is not inside loop since it executes everything it can once
      *  called
      */
-    if (ehci->pstate != EST_EXECUTING) {
+    if (ehci_get_state(ehci, 0) != EST_EXECUTING) {
         ehci_advance_async_state(ehci);
     }
 
diff --git a/trace-events b/trace-events
index c63e6f9..33adbab 100644
--- a/trace-events
+++ b/trace-events
@@ -199,6 +199,10 @@ disable usb_ehci_reset(void) "=== RESET ==="
 disable usb_ehci_mmio_readl(uint32_t addr, const char *str, uint32_t val) "rd mmio %04x [%s] = %x"
 disable usb_ehci_mmio_writel(uint32_t addr, const char *str, uint32_t val, uint32_t oldval) "wr mmio %04x [%s] = %x (old: %x)"
 disable usb_ehci_usbsts(const char *sts, int state) "usbsts %s %d"
+disable usb_ehci_state(const char *schedule, const char *state) "%s schedule %s"
+disable usb_ehci_qh(uint32_t addr, uint32_t next, uint32_t c_qtd, uint32_t n_qtd, uint32_t a_qtd, int rl, int mplen, int eps, int ep, int devaddr, int c, int h, int dtc, int i) "QH @ %08x: next %08x qtds %08x,%08x,%08x - rl %d, mplen %d, eps %d, ep %d, dev %d, c %d, h %d, dtc %d, i %d"
+disable usb_ehci_qtd(uint32_t addr, uint32_t next, uint32_t altnext, int tbytes, int cpage, int cerr, int pid, int ioc, int active, int halt, int babble, int xacterr) "QH @ %08x: next %08x altnext %08x - tbytes %d, cpage %d, cerr %d, pid %d, ioc %d, active %d, halt %d, babble %d, xacterr %d"
+disable usb_ehci_itd(uint32_t addr, uint32_t next) "ITD @ %08x: next %08x"
 
 # hw/usb-desc.c
 disable usb_desc_device(int addr, int len, int ret) "dev %d query device, len %d, ret %d"
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 04/12] usb-ehci: trace port state
  2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 03/12] usb-ehci: trace state machine changes Gerd Hoffmann
@ 2011-05-26 10:44 ` Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 05/12] usb-ehci: improve mmio tracing Gerd Hoffmann
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Trace usb port operations (attach, detach, reset),
drop a few obsolete DPRINTF's.

No change in behavior.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-ehci.c |   10 ++++------
 trace-events  |    3 +++
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 40a447b..85e5ed9 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -604,8 +604,7 @@ static void ehci_attach(USBPort *port)
     EHCIState *s = port->opaque;
     uint32_t *portsc = &s->portsc[port->index];
 
-    DPRINTF("ehci_attach invoked for index %d, portsc 0x%x, desc %s\n",
-           port->index, *portsc, port->dev->product_desc);
+    trace_usb_ehci_port_attach(port->index, port->dev->product_desc);
 
     *portsc |= PORTSC_CONNECT;
     *portsc |= PORTSC_CSC;
@@ -625,8 +624,7 @@ static void ehci_detach(USBPort *port)
     EHCIState *s = port->opaque;
     uint32_t *portsc = &s->portsc[port->index];
 
-    DPRINTF("ehci_attach invoked for index %d, portsc 0x%x\n",
-           port->index, *portsc);
+    trace_usb_ehci_port_detach(port->index);
 
     *portsc &= ~PORTSC_CONNECT;
     *portsc |= PORTSC_CSC;
@@ -733,11 +731,11 @@ static void handle_port_status_write(EHCIState *s, int port, uint32_t val)
     *portsc &= ~rwc;
 
     if ((val & PORTSC_PRESET) && !(*portsc & PORTSC_PRESET)) {
-        DPRINTF("port_status_write: USBTRAN Port %d reset begin\n", port);
+        trace_usb_ehci_port_reset(port, 1);
     }
 
     if (!(val & PORTSC_PRESET) &&(*portsc & PORTSC_PRESET)) {
-        DPRINTF("port_status_write: USBTRAN Port %d reset done\n", port);
+        trace_usb_ehci_port_reset(port, 0);
         usb_attach(&s->ports[port], dev);
 
         // TODO how to handle reset of ports with no device
diff --git a/trace-events b/trace-events
index 33adbab..22f93c7 100644
--- a/trace-events
+++ b/trace-events
@@ -203,6 +203,9 @@ disable usb_ehci_state(const char *schedule, const char *state) "%s schedule %s"
 disable usb_ehci_qh(uint32_t addr, uint32_t next, uint32_t c_qtd, uint32_t n_qtd, uint32_t a_qtd, int rl, int mplen, int eps, int ep, int devaddr, int c, int h, int dtc, int i) "QH @ %08x: next %08x qtds %08x,%08x,%08x - rl %d, mplen %d, eps %d, ep %d, dev %d, c %d, h %d, dtc %d, i %d"
 disable usb_ehci_qtd(uint32_t addr, uint32_t next, uint32_t altnext, int tbytes, int cpage, int cerr, int pid, int ioc, int active, int halt, int babble, int xacterr) "QH @ %08x: next %08x altnext %08x - tbytes %d, cpage %d, cerr %d, pid %d, ioc %d, active %d, halt %d, babble %d, xacterr %d"
 disable usb_ehci_itd(uint32_t addr, uint32_t next) "ITD @ %08x: next %08x"
+disable usb_ehci_port_attach(uint32_t port, const char *device) "attach port #%d - %s"
+disable usb_ehci_port_detach(uint32_t port) "detach port #%d"
+disable usb_ehci_port_reset(uint32_t port, int enable) "reset port #%d - %d"
 
 # hw/usb-desc.c
 disable usb_desc_device(int addr, int len, int ret) "dev %d query device, len %d, ret %d"
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 05/12] usb-ehci: improve mmio tracing
  2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
                   ` (3 preceding siblings ...)
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 04/12] usb-ehci: trace port state Gerd Hoffmann
@ 2011-05-26 10:44 ` Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 06/12] usb-ehci: trace buffer copy Gerd Hoffmann
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Add a separate tracepoint to log how register values change in response
to a mmio write.  Especially useful for registers which have read-only
or clear-on-write bits in them.

No change in behavior.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-ehci.c |   16 ++++++----------
 trace-events  |    3 ++-
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 85e5ed9..b9204ab 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -719,10 +719,6 @@ static void handle_port_status_write(EHCIState *s, int port, uint32_t val)
     int rwc;
     USBDevice *dev = s->ports[port].dev;
 
-    DPRINTF("port_status_write: "
-            "PORTSC (port %d) curr %08X new %08X rw-clear %08X rw %08X\n",
-            port, *portsc, val, (val & PORTSC_RWC_MASK), val & PORTSC_RO_MASK);
-
     rwc = val & PORTSC_RWC_MASK;
     val &= PORTSC_RO_MASK;
 
@@ -744,8 +740,6 @@ static void handle_port_status_write(EHCIState *s, int port, uint32_t val)
         }
 
         if (s->ports[port].dev) {
-            DPRINTF("port_status_write: "
-                    "Device was connected before reset, clearing CSC bit\n");
             *portsc &= ~PORTSC_CSC;
         }
 
@@ -760,16 +754,16 @@ static void handle_port_status_write(EHCIState *s, int port, uint32_t val)
 
     *portsc &= ~PORTSC_RO_MASK;
     *portsc |= val;
-    DPRINTF("port_status_write: Port %d status set to 0x%08x\n", port, *portsc);
 }
 
 static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
 {
     EHCIState *s = ptr;
+    uint32_t *mmio = (uint32_t *)(&s->mmio[addr]);
+    uint32_t old = *mmio;
     int i;
 
-    trace_usb_ehci_mmio_writel(addr, addr2str(addr), val,
-                               *(uint32_t *)(&s->mmio[addr]));
+    trace_usb_ehci_mmio_writel(addr, addr2str(addr), val);
 
     /* Only aligned reads are allowed on OHCI */
     if (addr & 3) {
@@ -780,6 +774,7 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
 
     if (addr >= PORTSC && addr < PORTSC + 4 * NB_PORTS) {
         handle_port_status_write(s, (addr-PORTSC)/4, val);
+        trace_usb_ehci_mmio_change(addr, addr2str(addr), *mmio, old);
         return;
     }
 
@@ -858,7 +853,8 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
         break;
     }
 
-    *(uint32_t *)(&s->mmio[addr]) = val;
+    *mmio = val;
+    trace_usb_ehci_mmio_change(addr, addr2str(addr), *mmio, old);
 }
 
 
diff --git a/trace-events b/trace-events
index 22f93c7..b2ccd07 100644
--- a/trace-events
+++ b/trace-events
@@ -197,7 +197,8 @@ disable sun4m_iommu_bad_addr(uint64_t addr) "bad addr %"PRIx64""
 # hw/usb-ehci.c
 disable usb_ehci_reset(void) "=== RESET ==="
 disable usb_ehci_mmio_readl(uint32_t addr, const char *str, uint32_t val) "rd mmio %04x [%s] = %x"
-disable usb_ehci_mmio_writel(uint32_t addr, const char *str, uint32_t val, uint32_t oldval) "wr mmio %04x [%s] = %x (old: %x)"
+disable usb_ehci_mmio_writel(uint32_t addr, const char *str, uint32_t val) "wr mmio %04x [%s] = %x"
+disable usb_ehci_mmio_change(uint32_t addr, const char *str, uint32_t new, uint32_t old) "ch mmio %04x [%s] = %x (old: %x)"
 disable usb_ehci_usbsts(const char *sts, int state) "usbsts %s %d"
 disable usb_ehci_state(const char *schedule, const char *state) "%s schedule %s"
 disable usb_ehci_qh(uint32_t addr, uint32_t next, uint32_t c_qtd, uint32_t n_qtd, uint32_t a_qtd, int rl, int mplen, int eps, int ep, int devaddr, int c, int h, int dtc, int i) "QH @ %08x: next %08x qtds %08x,%08x,%08x - rl %d, mplen %d, eps %d, ep %d, dev %d, c %d, h %d, dtc %d, i %d"
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 06/12] usb-ehci: trace buffer copy
  2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
                   ` (4 preceding siblings ...)
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 05/12] usb-ehci: improve mmio tracing Gerd Hoffmann
@ 2011-05-26 10:44 ` Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 07/12] usb-ehci: add queue data struct Gerd Hoffmann
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Add a trace point for buffer copies and drop the DPRINTF's.

No change in behavior.

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

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index b9204ab..d89cb28 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -901,8 +901,6 @@ static int ehci_qh_do_overlay(EHCIState *ehci, EHCIqh *qh, EHCIqtd *qtd)
     dtoggle = qh->token & QTD_TOKEN_DTOGGLE;
     ping    = qh->token & QTD_TOKEN_PING;
 
-    DPRINTF("setting qh.current from %08X to 0x%08X\n", qh->current_qtd,
-            ehci->qtdaddr);
     qh->current_qtd = ehci->qtdaddr;
     qh->next_qtd    = qtd->next;
     qh->altnext_qtd = qtd->altnext;
@@ -955,8 +953,6 @@ static int ehci_buffer_rw(uint8_t *buffer, EHCIqh *qh, int bytes, int rw)
     }
 
     offset = qh->bufptr[0] & ~QTD_BUFPTR_MASK;
-    DPRINTF("ehci_buffer_rw: %sing %d bytes %08x cpage %d offset %d\n",
-           rw ? "writ" : "read", bytes, qh->bufptr[0], cpage, offset);
 
     do {
         /* start and end of this page */
@@ -969,9 +965,7 @@ static int ehci_buffer_rw(uint8_t *buffer, EHCIqh *qh, int bytes, int rw)
             tail = head + bytes;
         }
 
-        DPRINTF("DATA %s cpage:%d head:%08X tail:%08X target:%08X\n",
-                rw ? "WRITE" : "READ ", cpage, head, tail, bufpos);
-
+        trace_usb_ehci_data(rw, cpage, offset, head, tail-head, bufpos);
         cpu_physical_memory_rw(head, &buffer[bufpos], tail - head, rw);
 
         bufpos += (tail - head);
diff --git a/trace-events b/trace-events
index b2ccd07..47a3260 100644
--- a/trace-events
+++ b/trace-events
@@ -207,6 +207,7 @@ disable usb_ehci_itd(uint32_t addr, uint32_t next) "ITD @ %08x: next %08x"
 disable usb_ehci_port_attach(uint32_t port, const char *device) "attach port #%d - %s"
 disable usb_ehci_port_detach(uint32_t port) "detach port #%d"
 disable usb_ehci_port_reset(uint32_t port, int enable) "reset port #%d - %d"
+disable usb_ehci_data(int rw, uint32_t cpage, uint32_t offset, uint32_t addr, uint32_t len, uint32_t bufpos) "write %d, cpage %d, offset 0x%03x, addr 0x%08x, len %d, bufpos %d"
 
 # hw/usb-desc.c
 disable usb_desc_device(int addr, int len, int ret) "dev %d query device, len %d, ret %d"
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 07/12] usb-ehci: add queue data struct
  2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
                   ` (5 preceding siblings ...)
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 06/12] usb-ehci: trace buffer copy Gerd Hoffmann
@ 2011-05-26 10:44 ` Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 08/12] usb-ehci: multiqueue support Gerd Hoffmann
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Add EHCIQueue struct, move the fields needed to track the queue state
into that struct.  Pass the new struct instead of ehci state down to
functions which handle the queue state.  Lot of variable references have
changed due to that without an actual functional change.

Replace fetch_addr with two variables, one for async and one for
periodic schedule.  Add functions to get and set the fetch address.

Use EHCIQueue->usb_status (old name: EHCIState->exec_status) directly in
ehci_execute_complete instead of passing around the status using a
parameters and the return value.

ehci_state_fetchqh returns a EHCIQueue struct now.

No change in behavior.

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

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index d89cb28..3656a35 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -334,8 +334,37 @@ typedef struct EHCIfstn {
     uint32_t backptr;                 // Standard next link pointer
 } EHCIfstn;
 
-typedef struct {
+typedef struct EHCIQueue EHCIQueue;
+typedef struct EHCIState EHCIState;
+
+enum async_state {
+    EHCI_ASYNC_NONE = 0,
+    EHCI_ASYNC_INFLIGHT,
+    EHCI_ASYNC_FINISHED,
+};
+
+struct EHCIQueue {
+    EHCIState *ehci;
+
+    /* cached data from guest - needs to be flushed
+     * when guest removes an entry (doorbell, handshake sequence)
+     */
+    EHCIqh qh;             // copy of current QH (being worked on)
+    uint32_t qhaddr;       // address QH read from
+    EHCIqtd qtd;           // copy of current QTD (being worked on)
+    uint32_t qtdaddr;      // address QTD read from
+
+    USBPacket packet;
+    uint8_t buffer[BUFF_SIZE];
+    int pid;
+    uint32_t tbytes;
+    enum async_state async;
+    int usb_status;
+};
+
+struct EHCIState {
     PCIDevice dev;
+    USBBus bus;
     qemu_irq irq;
     target_phys_addr_t mem_base;
     int mem;
@@ -360,6 +389,7 @@ typedef struct {
             uint32_t portsc[NB_PORTS];
         };
     };
+
     /*
      *  Internal states, shadow registers, etc
      */
@@ -369,32 +399,19 @@ typedef struct {
     int astate;                        // Current state in asynchronous schedule
     int pstate;                        // Current state in periodic schedule
     USBPort ports[NB_PORTS];
-    uint8_t buffer[BUFF_SIZE];
     uint32_t usbsts_pending;
+    EHCIQueue queue;
 
-    /* cached data from guest - needs to be flushed
-     * when guest removes an entry (doorbell, handshake sequence)
-     */
-    EHCIqh qh;             // copy of current QH (being worked on)
-    uint32_t qhaddr;       // address QH read from
-
-    EHCIqtd qtd;           // copy of current QTD (being worked on)
-    uint32_t qtdaddr;      // address QTD read from
-
-    uint32_t itdaddr;      // current ITD
-
-    uint32_t fetch_addr;   // which address to look at next
+    uint32_t a_fetch_addr;   // which address to look at next
+    uint32_t p_fetch_addr;   // which address to look at next
 
-    USBBus bus;
-    USBPacket usb_packet;
-    int async_complete;
-    uint32_t tbytes;
-    int pid;
-    int exec_status;
+    USBPacket ipacket;
+    uint8_t ibuffer[BUFF_SIZE];
     int isoch_pause;
+
     uint32_t last_run_usec;
     uint32_t frame_end_usec;
-} EHCIState;
+};
 
 #define SET_LAST_RUN_CLOCK(s) \
     (s)->last_run_usec = qemu_get_clock_ns(vm_clock) / 1000;
@@ -563,6 +580,20 @@ static int ehci_get_state(EHCIState *s, int async)
     return async ? s->astate : s->pstate;
 }
 
+static void ehci_set_fetch_addr(EHCIState *s, int async, uint32_t addr)
+{
+    if (async) {
+        s->a_fetch_addr = addr;
+    } else {
+        s->p_fetch_addr = addr;
+    }
+}
+
+static int ehci_get_fetch_addr(EHCIState *s, int async)
+{
+    return async ? s->a_fetch_addr : s->p_fetch_addr;
+}
+
 static void ehci_trace_qh(EHCIState *s, target_phys_addr_t addr, EHCIqh *qh)
 {
     trace_usb_ehci_qh(addr, qh->next,
@@ -656,7 +687,6 @@ static void ehci_reset(void *opaque)
 
     s->astate = EST_INACTIVE;
     s->pstate = EST_INACTIVE;
-    s->async_complete = 0;
     s->isoch_pause = -1;
     s->attach_poll_counter = 0;
 
@@ -888,7 +918,7 @@ static inline int put_dwords(uint32_t addr, uint32_t *buf, int num)
 
 // 4.10.2
 
-static int ehci_qh_do_overlay(EHCIState *ehci, EHCIqh *qh, EHCIqtd *qtd)
+static int ehci_qh_do_overlay(EHCIQueue *q)
 {
     int i;
     int dtoggle;
@@ -898,43 +928,43 @@ static int ehci_qh_do_overlay(EHCIState *ehci, EHCIqh *qh, EHCIqtd *qtd)
 
     // remember values in fields to preserve in qh after overlay
 
-    dtoggle = qh->token & QTD_TOKEN_DTOGGLE;
-    ping    = qh->token & QTD_TOKEN_PING;
+    dtoggle = q->qh.token & QTD_TOKEN_DTOGGLE;
+    ping    = q->qh.token & QTD_TOKEN_PING;
 
-    qh->current_qtd = ehci->qtdaddr;
-    qh->next_qtd    = qtd->next;
-    qh->altnext_qtd = qtd->altnext;
-    qh->token       = qtd->token;
+    q->qh.current_qtd = q->qtdaddr;
+    q->qh.next_qtd    = q->qtd.next;
+    q->qh.altnext_qtd = q->qtd.altnext;
+    q->qh.token       = q->qtd.token;
 
 
-    eps = get_field(qh->epchar, QH_EPCHAR_EPS);
+    eps = get_field(q->qh.epchar, QH_EPCHAR_EPS);
     if (eps == EHCI_QH_EPS_HIGH) {
-        qh->token &= ~QTD_TOKEN_PING;
-        qh->token |= ping;
+        q->qh.token &= ~QTD_TOKEN_PING;
+        q->qh.token |= ping;
     }
 
-    reload = get_field(qh->epchar, QH_EPCHAR_RL);
-    set_field(&qh->altnext_qtd, reload, QH_ALTNEXT_NAKCNT);
+    reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
+    set_field(&q->qh.altnext_qtd, reload, QH_ALTNEXT_NAKCNT);
 
     for (i = 0; i < 5; i++) {
-        qh->bufptr[i] = qtd->bufptr[i];
+        q->qh.bufptr[i] = q->qtd.bufptr[i];
     }
 
-    if (!(qh->epchar & QH_EPCHAR_DTC)) {
+    if (!(q->qh.epchar & QH_EPCHAR_DTC)) {
         // preserve QH DT bit
-        qh->token &= ~QTD_TOKEN_DTOGGLE;
-        qh->token |= dtoggle;
+        q->qh.token &= ~QTD_TOKEN_DTOGGLE;
+        q->qh.token |= dtoggle;
     }
 
-    qh->bufptr[1] &= ~BUFPTR_CPROGMASK_MASK;
-    qh->bufptr[2] &= ~BUFPTR_FRAMETAG_MASK;
+    q->qh.bufptr[1] &= ~BUFPTR_CPROGMASK_MASK;
+    q->qh.bufptr[2] &= ~BUFPTR_FRAMETAG_MASK;
 
-    put_dwords(NLPTR_GET(ehci->qhaddr), (uint32_t *) qh, sizeof(EHCIqh) >> 2);
+    put_dwords(NLPTR_GET(q->qhaddr), (uint32_t *) &q->qh, sizeof(EHCIqh) >> 2);
 
     return 0;
 }
 
-static int ehci_buffer_rw(uint8_t *buffer, EHCIqh *qh, int bytes, int rw)
+static int ehci_buffer_rw(EHCIQueue *q, int bytes, int rw)
 {
     int bufpos = 0;
     int cpage, offset;
@@ -946,17 +976,17 @@ static int ehci_buffer_rw(uint8_t *buffer, EHCIqh *qh, int bytes, int rw)
         return 0;
     }
 
-    cpage = get_field(qh->token, QTD_TOKEN_CPAGE);
+    cpage = get_field(q->qh.token, QTD_TOKEN_CPAGE);
     if (cpage > 4) {
         fprintf(stderr, "cpage out of range (%d)\n", cpage);
         return USB_RET_PROCERR;
     }
 
-    offset = qh->bufptr[0] & ~QTD_BUFPTR_MASK;
+    offset = q->qh.bufptr[0] & ~QTD_BUFPTR_MASK;
 
     do {
         /* start and end of this page */
-        head = qh->bufptr[cpage] & QTD_BUFPTR_MASK;
+        head = q->qh.bufptr[cpage] & QTD_BUFPTR_MASK;
         tail = head + ~QTD_BUFPTR_MASK + 1;
         /* add offset into page */
         head |= offset;
@@ -966,7 +996,7 @@ static int ehci_buffer_rw(uint8_t *buffer, EHCIqh *qh, int bytes, int rw)
         }
 
         trace_usb_ehci_data(rw, cpage, offset, head, tail-head, bufpos);
-        cpu_physical_memory_rw(head, &buffer[bufpos], tail - head, rw);
+        cpu_physical_memory_rw(head, q->buffer + bufpos, tail - head, rw);
 
         bufpos += (tail - head);
         bytes -= (tail - head);
@@ -978,112 +1008,111 @@ static int ehci_buffer_rw(uint8_t *buffer, EHCIqh *qh, int bytes, int rw)
     } while (bytes > 0);
 
     /* save cpage */
-    set_field(&qh->token, cpage, QTD_TOKEN_CPAGE);
+    set_field(&q->qh.token, cpage, QTD_TOKEN_CPAGE);
 
     /* save offset into cpage */
     offset = tail - head;
-    qh->bufptr[0] &= ~QTD_BUFPTR_MASK;
-    qh->bufptr[0] |= offset;
+    q->qh.bufptr[0] &= ~QTD_BUFPTR_MASK;
+    q->qh.bufptr[0] |= offset;
 
     return 0;
 }
 
 static void ehci_async_complete_packet(USBDevice *dev, USBPacket *packet)
 {
-    EHCIState *ehci = container_of(packet, EHCIState, usb_packet);
+    EHCIQueue *q = container_of(packet, EHCIQueue, packet);
 
     DPRINTF("Async packet complete\n");
-    ehci->async_complete = 1;
-    ehci->exec_status = packet->len;
+    assert(q->async == EHIC_ASYNC_INFLIGHT);
+    q->async = EHCI_ASYNC_FINISHED;
+    q->usb_status = packet->len;
 }
 
-static int ehci_execute_complete(EHCIState *ehci, EHCIqh *qh, int ret)
+static void ehci_execute_complete(EHCIQueue *q)
 {
     int c_err, reload;
 
-    if (ret == USB_RET_ASYNC && !ehci->async_complete) {
+    if (q->async == EHCI_ASYNC_INFLIGHT) {
         DPRINTF("not done yet\n");
-        return ret;
+        return;
     }
-
-    ehci->async_complete = 0;
+    q->async = EHCI_ASYNC_NONE;
 
     DPRINTF("execute_complete: qhaddr 0x%x, next %x, qtdaddr 0x%x, status %d\n",
-            ehci->qhaddr, qh->next, ehci->qtdaddr, ret);
+            q->qhaddr, q->qh.next, q->qtdaddr, q->usb_status);
 
-    if (ret < 0) {
+    if (q->usb_status < 0) {
 err:
         /* TO-DO: put this is in a function that can be invoked below as well */
-        c_err = get_field(qh->token, QTD_TOKEN_CERR);
+        c_err = get_field(q->qh.token, QTD_TOKEN_CERR);
         c_err--;
-        set_field(&qh->token, c_err, QTD_TOKEN_CERR);
+        set_field(&q->qh.token, c_err, QTD_TOKEN_CERR);
 
-        switch(ret) {
+        switch(q->usb_status) {
         case USB_RET_NODEV:
             fprintf(stderr, "USB no device\n");
             break;
         case USB_RET_STALL:
             fprintf(stderr, "USB stall\n");
-            qh->token |= QTD_TOKEN_HALT;
-            ehci_record_interrupt(ehci, USBSTS_ERRINT);
+            q->qh.token |= QTD_TOKEN_HALT;
+            ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
             break;
         case USB_RET_NAK:
             /* 4.10.3 */
-            reload = get_field(qh->epchar, QH_EPCHAR_RL);
-            if ((ehci->pid == USB_TOKEN_IN) && reload) {
-                int nakcnt = get_field(qh->altnext_qtd, QH_ALTNEXT_NAKCNT);
+            reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
+            if ((q->pid == USB_TOKEN_IN) && reload) {
+                int nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);
                 nakcnt--;
-                set_field(&qh->altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);
+                set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);
             } else if (!reload) {
-                return USB_RET_NAK;
+                return;
             }
             break;
         case USB_RET_BABBLE:
             fprintf(stderr, "USB babble TODO\n");
-            qh->token |= QTD_TOKEN_BABBLE;
-            ehci_record_interrupt(ehci, USBSTS_ERRINT);
+            q->qh.token |= QTD_TOKEN_BABBLE;
+            ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
             break;
         default:
-            fprintf(stderr, "USB invalid response %d to handle\n", ret);
-            /* TO-DO: transaction error */
-            ret = USB_RET_PROCERR;
+            /* should not be triggerable */
+            fprintf(stderr, "USB invalid response %d to handle\n", q->usb_status);
+            assert(0);
             break;
         }
     } else {
         // DPRINTF("Short packet condition\n");
         // TODO check 4.12 for splits
 
-        if ((ret > ehci->tbytes) && (ehci->pid == USB_TOKEN_IN)) {
-            ret = USB_RET_BABBLE;
+        if ((q->usb_status > q->tbytes) && (q->pid == USB_TOKEN_IN)) {
+            q->usb_status = USB_RET_BABBLE;
             goto err;
         }
 
-        if (ehci->tbytes && ehci->pid == USB_TOKEN_IN) {
-            if (ehci_buffer_rw(ehci->buffer, qh, ret, 1) != 0) {
-                return USB_RET_PROCERR;
+        if (q->tbytes && q->pid == USB_TOKEN_IN) {
+            if (ehci_buffer_rw(q, q->usb_status, 1) != 0) {
+                q->usb_status = USB_RET_PROCERR;
+                return;
             }
-            ehci->tbytes -= ret;
+            q->tbytes -= q->usb_status;
         } else {
-            ehci->tbytes = 0;
+            q->tbytes = 0;
         }
 
-        DPRINTF("updating tbytes to %d\n", ehci->tbytes);
-        set_field(&qh->token, ehci->tbytes, QTD_TOKEN_TBYTES);
+        DPRINTF("updating tbytes to %d\n", q->tbytes);
+        set_field(&q->qh.token, q->tbytes, QTD_TOKEN_TBYTES);
     }
 
-    qh->token ^= QTD_TOKEN_DTOGGLE;
-    qh->token &= ~QTD_TOKEN_ACTIVE;
+    q->qh.token ^= QTD_TOKEN_DTOGGLE;
+    q->qh.token &= ~QTD_TOKEN_ACTIVE;
 
-    if ((ret >= 0) && (qh->token & QTD_TOKEN_IOC)) {
-        ehci_record_interrupt(ehci, USBSTS_INT);
+    if ((q->usb_status >= 0) && (q->qh.token & QTD_TOKEN_IOC)) {
+        ehci_record_interrupt(q->ehci, USBSTS_INT);
     }
-
-    return ret;
 }
 
 // 4.10.3
 
-static int ehci_execute(EHCIState *ehci, EHCIqh *qh)
+static int ehci_execute(EHCIQueue *q)
 {
     USBPort *port;
     USBDevice *dev;
@@ -1092,59 +1121,59 @@ static int ehci_execute(EHCIState *ehci, EHCIqh *qh)
     int endp;
     int devadr;
 
-    if ( !(qh->token & QTD_TOKEN_ACTIVE)) {
+    if ( !(q->qh.token & QTD_TOKEN_ACTIVE)) {
         fprintf(stderr, "Attempting to execute inactive QH\n");
         return USB_RET_PROCERR;
     }
 
-    ehci->tbytes = (qh->token & QTD_TOKEN_TBYTES_MASK) >> QTD_TOKEN_TBYTES_SH;
-    if (ehci->tbytes > BUFF_SIZE) {
+    q->tbytes = (q->qh.token & QTD_TOKEN_TBYTES_MASK) >> QTD_TOKEN_TBYTES_SH;
+    if (q->tbytes > BUFF_SIZE) {
         fprintf(stderr, "Request for more bytes than allowed\n");
         return USB_RET_PROCERR;
     }
 
-    ehci->pid = (qh->token & QTD_TOKEN_PID_MASK) >> QTD_TOKEN_PID_SH;
-    switch(ehci->pid) {
-        case 0: ehci->pid = USB_TOKEN_OUT; break;
-        case 1: ehci->pid = USB_TOKEN_IN; break;
-        case 2: ehci->pid = USB_TOKEN_SETUP; break;
+    q->pid = (q->qh.token & QTD_TOKEN_PID_MASK) >> QTD_TOKEN_PID_SH;
+    switch(q->pid) {
+        case 0: q->pid = USB_TOKEN_OUT; break;
+        case 1: q->pid = USB_TOKEN_IN; break;
+        case 2: q->pid = USB_TOKEN_SETUP; break;
         default: fprintf(stderr, "bad token\n"); break;
     }
 
-    if ((ehci->tbytes && ehci->pid != USB_TOKEN_IN) &&
-        (ehci_buffer_rw(ehci->buffer, qh, ehci->tbytes, 0) != 0)) {
+    if ((q->tbytes && q->pid != USB_TOKEN_IN) &&
+        (ehci_buffer_rw(q, q->tbytes, 0) != 0)) {
         return USB_RET_PROCERR;
     }
 
-    endp = get_field(qh->epchar, QH_EPCHAR_EP);
-    devadr = get_field(qh->epchar, QH_EPCHAR_DEVADDR);
+    endp = get_field(q->qh.epchar, QH_EPCHAR_EP);
+    devadr = get_field(q->qh.epchar, QH_EPCHAR_DEVADDR);
 
     ret = USB_RET_NODEV;
 
     // TO-DO: associating device with ehci port
     for(i = 0; i < NB_PORTS; i++) {
-        port = &ehci->ports[i];
+        port = &q->ehci->ports[i];
         dev = port->dev;
 
         // TODO sometime we will also need to check if we are the port owner
 
-        if (!(ehci->portsc[i] &(PORTSC_CONNECT))) {
+        if (!(q->ehci->portsc[i] &(PORTSC_CONNECT))) {
             DPRINTF("Port %d, no exec, not connected(%08X)\n",
-                    i, ehci->portsc[i]);
+                    i, q->ehci->portsc[i]);
             continue;
         }
 
-        ehci->usb_packet.pid = ehci->pid;
-        ehci->usb_packet.devaddr = devadr;
-        ehci->usb_packet.devep = endp;
-        ehci->usb_packet.data = ehci->buffer;
-        ehci->usb_packet.len = ehci->tbytes;
+        q->packet.pid = q->pid;
+        q->packet.devaddr = devadr;
+        q->packet.devep = endp;
+        q->packet.data = q->buffer;
+        q->packet.len = q->tbytes;
 
-        ret = usb_handle_packet(dev, &ehci->usb_packet);
+        ret = usb_handle_packet(dev, &q->packet);
 
         DPRINTF("submit: qh %x next %x qtd %x pid %x len %d (total %d) endp %x ret %d\n",
-                ehci->qhaddr, qh->next, ehci->qtdaddr, ehci->pid,
-                ehci->usb_packet.len, ehci->tbytes, endp, ret);
+                q->qhaddr, q->qh.next, q->qtdaddr, q->pid,
+                q->packet.len, q->tbytes, endp, ret);
 
         if (ret != USB_RET_NODEV) {
             break;
@@ -1157,7 +1186,7 @@ static int ehci_execute(EHCIState *ehci, EHCIqh *qh)
     }
 
     if (ret == USB_RET_ASYNC) {
-        ehci->async_complete = 0;
+        q->async = EHCI_ASYNC_INFLIGHT;
     }
 
     return ret;
@@ -1204,7 +1233,7 @@ static int ehci_process_itd(EHCIState *ehci,
             DPRINTF("ISOCH: buffer %08X len %d\n", ptr, len);
 
             if (!dir) {
-                cpu_physical_memory_rw(ptr, &ehci->buffer[0], len, 0);
+                cpu_physical_memory_rw(ptr, &ehci->ibuffer[0], len, 0);
                 pid = USB_TOKEN_OUT;
             } else
                 pid = USB_TOKEN_IN;
@@ -1223,14 +1252,14 @@ static int ehci_process_itd(EHCIState *ehci,
                     continue;
                 }
 
-                ehci->usb_packet.pid = ehci->pid;
-                ehci->usb_packet.devaddr = devadr;
-                ehci->usb_packet.devep = endp;
-                ehci->usb_packet.data = ehci->buffer;
-                ehci->usb_packet.len = len;
+                ehci->ipacket.pid = pid;
+                ehci->ipacket.devaddr = devadr;
+                ehci->ipacket.devep = endp;
+                ehci->ipacket.data = ehci->ibuffer;
+                ehci->ipacket.len = len;
 
                 DPRINTF("calling usb_handle_packet\n");
-                ret = usb_handle_packet(dev, &ehci->usb_packet);
+                ret = usb_handle_packet(dev, &ehci->ipacket);
 
                 if (ret != USB_RET_NODEV) {
                     break;
@@ -1271,7 +1300,7 @@ static int ehci_process_itd(EHCIState *ehci,
             }
 
             if (ret >= 0 && dir) {
-                cpu_physical_memory_rw(ptr, &ehci->buffer[0], len, 1);
+                cpu_physical_memory_rw(ptr, &ehci->ibuffer[0], len, 1);
 
                 if (ret != len) {
                     DPRINTF("ISOCH IN expected %d, got %d\n",
@@ -1289,7 +1318,7 @@ static int ehci_process_itd(EHCIState *ehci,
  */
 static int ehci_state_waitlisthead(EHCIState *ehci,  int async)
 {
-    EHCIqh *qh = &ehci->qh;
+    EHCIqh qh;
     int i = 0;
     int again = 0;
     uint32_t entry = ehci->asynclistaddr;
@@ -1301,21 +1330,21 @@ 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(NLPTR_GET(entry), (uint32_t *) qh, sizeof(EHCIqh) >> 2);
-        ehci_trace_qh(ehci, NLPTR_GET(entry), qh);
+        get_dwords(NLPTR_GET(entry), (uint32_t *) &qh, sizeof(EHCIqh) >> 2);
+        ehci_trace_qh(ehci, NLPTR_GET(entry), &qh);
 
-        if (qh->epchar & QH_EPCHAR_H) {
+        if (qh.epchar & QH_EPCHAR_H) {
             if (async) {
                 entry |= (NLPTR_TYPE_QH << 1);
             }
 
-            ehci->fetch_addr = entry;
+            ehci_set_fetch_addr(ehci, async, entry);
             ehci_set_state(ehci, async, EST_FETCHENTRY);
             again = 1;
             goto out;
         }
 
-        entry = qh->next;
+        entry = qh.next;
         if (entry == ehci->asynclistaddr) {
             break;
         }
@@ -1336,7 +1365,7 @@ out:
 static int ehci_state_fetchentry(EHCIState *ehci, int async)
 {
     int again = 0;
-    uint32_t entry = ehci->fetch_addr;
+    uint32_t entry = ehci_get_fetch_addr(ehci, async);
 
 #if EHCI_DEBUG == 0
     if (qemu_get_clock_ns(vm_clock) / 1000 >= ehci->frame_end_usec) {
@@ -1364,13 +1393,11 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async)
     switch (NLPTR_TYPE_GET(entry)) {
     case NLPTR_TYPE_QH:
         ehci_set_state(ehci, async, EST_FETCHQH);
-        ehci->qhaddr = entry;
         again = 1;
         break;
 
     case NLPTR_TYPE_ITD:
         ehci_set_state(ehci, async, EST_FETCHITD);
-        ehci->itdaddr = entry;
         again = 1;
         break;
 
@@ -1385,85 +1412,91 @@ out:
     return again;
 }
 
-static int ehci_state_fetchqh(EHCIState *ehci, int async)
+static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
 {
-    EHCIqh *qh = &ehci->qh;
+    uint32_t entry;
+    EHCIQueue *q;
     int reload;
-    int again = 0;
 
-    get_dwords(NLPTR_GET(ehci->qhaddr), (uint32_t *) qh, sizeof(EHCIqh) >> 2);
-    ehci_trace_qh(ehci, NLPTR_GET(ehci->qhaddr), qh);
+    entry = ehci_get_fetch_addr(ehci, async);
+    q = &ehci->queue; /* temporary */
+    q->qhaddr = entry;
 
-    if (async && (qh->epchar & QH_EPCHAR_H)) {
+    get_dwords(NLPTR_GET(q->qhaddr), (uint32_t *) &q->qh, sizeof(EHCIqh) >> 2);
+    ehci_trace_qh(ehci, NLPTR_GET(q->qhaddr), &q->qh);
+
+    if (async && (q->qh.epchar & QH_EPCHAR_H)) {
 
         /*  EHCI spec version 1.0 Section 4.8.3 & 4.10.1 */
         if (ehci->usbsts & USBSTS_REC) {
             ehci_clear_usbsts(ehci, USBSTS_REC);
         } else {
             DPRINTF("FETCHQH:  QH 0x%08x. H-bit set, reclamation status reset"
-                       " - done processing\n", ehci->qhaddr);
+                       " - done processing\n", q->qhaddr);
             ehci_set_state(ehci, async, EST_ACTIVE);
+            q = NULL;
             goto out;
         }
     }
 
 #if EHCI_DEBUG
-    if (ehci->qhaddr != qh->next) {
+    if (q->qhaddr != q->qh.next) {
     DPRINTF("FETCHQH:  QH 0x%08x (h %x halt %x active %x) next 0x%08x\n",
-               ehci->qhaddr,
-               qh->epchar & QH_EPCHAR_H,
-               qh->token & QTD_TOKEN_HALT,
-               qh->token & QTD_TOKEN_ACTIVE,
-               qh->next);
+               q->qhaddr,
+               q->qh.epchar & QH_EPCHAR_H,
+               q->qh.token & QTD_TOKEN_HALT,
+               q->qh.token & QTD_TOKEN_ACTIVE,
+               q->qh.next);
     }
 #endif
 
-    reload = get_field(qh->epchar, QH_EPCHAR_RL);
+    reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
     if (reload) {
-        set_field(&qh->altnext_qtd, reload, QH_ALTNEXT_NAKCNT);
+        set_field(&q->qh.altnext_qtd, reload, QH_ALTNEXT_NAKCNT);
     }
 
-    if (qh->token & QTD_TOKEN_HALT) {
+    if (q->qh.token & QTD_TOKEN_HALT) {
         ehci_set_state(ehci, async, EST_HORIZONTALQH);
-        again = 1;
 
-    } else if ((qh->token & QTD_TOKEN_ACTIVE) && (qh->current_qtd > 0x1000)) {
-        ehci->qtdaddr = qh->current_qtd;
+    } else if ((q->qh.token & QTD_TOKEN_ACTIVE) && (q->qh.current_qtd > 0x1000)) {
+        q->qtdaddr = q->qh.current_qtd;
         ehci_set_state(ehci, async, EST_FETCHQTD);
-        again = 1;
 
     } else {
         /*  EHCI spec version 1.0 Section 4.10.2 */
         ehci_set_state(ehci, async, EST_ADVANCEQUEUE);
-        again = 1;
     }
 
 out:
-    return again;
+    return q;
 }
 
 static int ehci_state_fetchitd(EHCIState *ehci, int async)
 {
+    uint32_t entry;
     EHCIitd itd;
 
-    get_dwords(NLPTR_GET(ehci->itdaddr),(uint32_t *) &itd,
+    assert(!async);
+    entry = ehci_get_fetch_addr(ehci, async);
+
+    get_dwords(NLPTR_GET(entry),(uint32_t *) &itd,
                sizeof(EHCIitd) >> 2);
-    ehci_trace_itd(ehci, ehci->itdaddr, &itd);
+    ehci_trace_itd(ehci, entry, &itd);
 
     if (ehci_process_itd(ehci, &itd) != 0) {
         return -1;
     }
 
-    put_dwords(NLPTR_GET(ehci->itdaddr), (uint32_t *) &itd,
+    put_dwords(NLPTR_GET(entry), (uint32_t *) &itd,
                 sizeof(EHCIitd) >> 2);
-    ehci->fetch_addr = itd.next;
+    ehci_set_fetch_addr(ehci, async, itd.next);
     ehci_set_state(ehci, async, EST_FETCHENTRY);
 
     return 1;
 }
 
 /* Section 4.10.2 - paragraph 3 */
-static int ehci_state_advqueue(EHCIState *ehci, int async)
+static int ehci_state_advqueue(EHCIQueue *q, int async)
 {
 #if 0
     /* TO-DO: 4.10.2 - paragraph 2
@@ -1479,84 +1512,81 @@ static int ehci_state_advqueue(EHCIState *ehci, int async)
     /*
      * want data and alt-next qTD is valid
      */
-    if (((ehci->qh.token & QTD_TOKEN_TBYTES_MASK) != 0) &&
-        (ehci->qh.altnext_qtd > 0x1000) &&
-        (NLPTR_TBIT(ehci->qh.altnext_qtd) == 0)) {
-        ehci->qtdaddr = ehci->qh.altnext_qtd;
-        ehci_set_state(ehci, async, EST_FETCHQTD);
+    if (((q->qh.token & QTD_TOKEN_TBYTES_MASK) != 0) &&
+        (q->qh.altnext_qtd > 0x1000) &&
+        (NLPTR_TBIT(q->qh.altnext_qtd) == 0)) {
+        q->qtdaddr = q->qh.altnext_qtd;
+        ehci_set_state(q->ehci, async, EST_FETCHQTD);
 
     /*
      *  next qTD is valid
      */
-    } else if ((ehci->qh.next_qtd > 0x1000) &&
-               (NLPTR_TBIT(ehci->qh.next_qtd) == 0)) {
-        ehci->qtdaddr = ehci->qh.next_qtd;
-        ehci_set_state(ehci, async, EST_FETCHQTD);
+    } else if ((q->qh.next_qtd > 0x1000) &&
+               (NLPTR_TBIT(q->qh.next_qtd) == 0)) {
+        q->qtdaddr = q->qh.next_qtd;
+        ehci_set_state(q->ehci, async, EST_FETCHQTD);
 
     /*
      *  no valid qTD, try next QH
      */
     } else {
-        ehci_set_state(ehci, async, EST_HORIZONTALQH);
+        ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
     }
 
     return 1;
 }
 
 /* Section 4.10.2 - paragraph 4 */
-static int ehci_state_fetchqtd(EHCIState *ehci, int async)
+static int ehci_state_fetchqtd(EHCIQueue *q, int async)
 {
-    EHCIqtd *qtd = &ehci->qtd;
     int again = 0;
 
-    get_dwords(NLPTR_GET(ehci->qtdaddr),(uint32_t *) qtd, sizeof(EHCIqtd) >> 2);
-    ehci_trace_qtd(ehci, NLPTR_GET(ehci->qtdaddr), qtd);
+    get_dwords(NLPTR_GET(q->qtdaddr),(uint32_t *) &q->qtd, sizeof(EHCIqtd) >> 2);
+    ehci_trace_qtd(q->ehci, NLPTR_GET(q->qtdaddr), &q->qtd);
 
-    if (qtd->token & QTD_TOKEN_ACTIVE) {
-        ehci_set_state(ehci, async, EST_EXECUTE);
+    if (q->qtd.token & QTD_TOKEN_ACTIVE) {
+        ehci_set_state(q->ehci, async, EST_EXECUTE);
         again = 1;
     } else {
-        ehci_set_state(ehci, async, EST_HORIZONTALQH);
+        ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
         again = 1;
     }
 
     return again;
 }
 
-static int ehci_state_horizqh(EHCIState *ehci, int async)
+static int ehci_state_horizqh(EHCIQueue *q, int async)
 {
     int again = 0;
 
-    if (ehci->fetch_addr != ehci->qh.next) {
-        ehci->fetch_addr = ehci->qh.next;
-        ehci_set_state(ehci, async, EST_FETCHENTRY);
+    if (ehci_get_fetch_addr(q->ehci, async) != q->qh.next) {
+        ehci_set_fetch_addr(q->ehci, async, q->qh.next);
+        ehci_set_state(q->ehci, async, EST_FETCHENTRY);
         again = 1;
     } else {
-        ehci_set_state(ehci, async, EST_ACTIVE);
+        ehci_set_state(q->ehci, async, EST_ACTIVE);
     }
 
     return again;
 }
 
-static int ehci_state_execute(EHCIState *ehci, int async)
+static int ehci_state_execute(EHCIQueue *q, int async)
 {
-    EHCIqh *qh = &ehci->qh;
-    EHCIqtd *qtd = &ehci->qtd;
     int again = 0;
     int reload, nakcnt;
     int smask;
 
-    if (ehci_qh_do_overlay(ehci, qh, qtd) != 0) {
+    if (ehci_qh_do_overlay(q) != 0) {
         return -1;
     }
 
-    smask = get_field(qh->epcap, QH_EPCAP_SMASK);
+    smask = get_field(q->qh.epcap, QH_EPCAP_SMASK);
 
     if (!smask) {
-        reload = get_field(qh->epchar, QH_EPCHAR_RL);
-        nakcnt = get_field(qh->altnext_qtd, QH_ALTNEXT_NAKCNT);
+        reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
+        nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);
         if (reload && !nakcnt) {
-            ehci_set_state(ehci, async, EST_HORIZONTALQH);
+            ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
             again = 1;
             goto out;
         }
@@ -1567,26 +1597,26 @@ static int ehci_state_execute(EHCIState *ehci, int async)
     // TODO Windows does not seem to ever set the MULT field
 
     if (!async) {
-        int transactCtr = get_field(qh->epcap, QH_EPCAP_MULT);
+        int transactCtr = get_field(q->qh.epcap, QH_EPCAP_MULT);
         if (!transactCtr) {
-            ehci_set_state(ehci, async, EST_HORIZONTALQH);
+            ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
             again = 1;
             goto out;
         }
     }
 
     if (async) {
-        ehci_set_usbsts(ehci, USBSTS_REC);
+        ehci_set_usbsts(q->ehci, USBSTS_REC);
     }
 
-    ehci->exec_status = ehci_execute(ehci, qh);
-    if (ehci->exec_status == USB_RET_PROCERR) {
+    q->usb_status = ehci_execute(q);
+    if (q->usb_status == USB_RET_PROCERR) {
         again = -1;
         goto out;
     }
-    ehci_set_state(ehci, async, EST_EXECUTING);
+    ehci_set_state(q->ehci, async, EST_EXECUTING);
 
-    if (ehci->exec_status != USB_RET_ASYNC) {
+    if (q->usb_status != USB_RET_ASYNC) {
         again = 1;
     }
 
@@ -1594,42 +1624,40 @@ out:
     return again;
 }
 
-static int ehci_state_executing(EHCIState *ehci, int async)
+static int ehci_state_executing(EHCIQueue *q, int async)
 {
-    EHCIqh *qh = &ehci->qh;
     int again = 0;
     int reload, nakcnt;
 
-    ehci->exec_status = ehci_execute_complete(ehci, qh, ehci->exec_status);
-    if (ehci->exec_status == USB_RET_ASYNC) {
+    ehci_execute_complete(q);
+    if (q->usb_status == USB_RET_ASYNC) {
         goto out;
     }
-    if (ehci->exec_status == USB_RET_PROCERR) {
+    if (q->usb_status == USB_RET_PROCERR) {
         again = -1;
         goto out;
     }
 
     // 4.10.3
     if (!async) {
-        int transactCtr = get_field(qh->epcap, QH_EPCAP_MULT);
+        int transactCtr = get_field(q->qh.epcap, QH_EPCAP_MULT);
         transactCtr--;
-        set_field(&qh->epcap, transactCtr, QH_EPCAP_MULT);
+        set_field(&q->qh.epcap, transactCtr, QH_EPCAP_MULT);
         // 4.10.3, bottom of page 82, should exit this state when transaction
         // counter decrements to 0
     }
 
-
-    reload = get_field(qh->epchar, QH_EPCHAR_RL);
+    reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
     if (reload) {
-        nakcnt = get_field(qh->altnext_qtd, QH_ALTNEXT_NAKCNT);
-        if (ehci->exec_status == USB_RET_NAK) {
+        nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);
+        if (q->usb_status == USB_RET_NAK) {
             if (nakcnt) {
                 nakcnt--;
             }
         } else {
             nakcnt = reload;
         }
-        set_field(&qh->altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);
+        set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);
     }
 
     /*
@@ -1637,13 +1665,13 @@ static int ehci_state_executing(EHCIState *ehci, int async)
      *  in the EHCI spec but we need to do it since we don't share
      *  physical memory with our guest VM.
      */
-    put_dwords(NLPTR_GET(ehci->qhaddr), (uint32_t *) qh, sizeof(EHCIqh) >> 2);
+    put_dwords(NLPTR_GET(q->qhaddr), (uint32_t *) &q->qh, sizeof(EHCIqh) >> 2);
 
     /* 4.10.5 */
-    if ((ehci->exec_status == USB_RET_NAK) || (qh->token & QTD_TOKEN_ACTIVE)) {
-        ehci_set_state(ehci, async, EST_HORIZONTALQH);
+    if ((q->usb_status == USB_RET_NAK) || (q->qh.token & QTD_TOKEN_ACTIVE)) {
+        ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
     } else {
-        ehci_set_state(ehci, async, EST_WRITEBACK);
+        ehci_set_state(q->ehci, async, EST_WRITEBACK);
     }
 
     again = 1;
@@ -1653,21 +1681,20 @@ out:
 }
 
 
-static int ehci_state_writeback(EHCIState *ehci, int async)
+static int ehci_state_writeback(EHCIQueue *q, int async)
 {
-    EHCIqh *qh = &ehci->qh;
     int again = 0;
 
     /*  Write back the QTD from the QH area */
-    ehci_trace_qtd(ehci, NLPTR_GET(ehci->qtdaddr), (EHCIqtd*) &qh->next_qtd);
-    put_dwords(NLPTR_GET(ehci->qtdaddr),(uint32_t *) &qh->next_qtd,
+    ehci_trace_qtd(q->ehci, NLPTR_GET(q->qtdaddr), (EHCIqtd*) &q->qh.next_qtd);
+    put_dwords(NLPTR_GET(q->qtdaddr),(uint32_t *) &q->qh.next_qtd,
                 sizeof(EHCIqtd) >> 2);
 
     /* TODO confirm next state.  For now, keep going if async
      * but stop after one qtd if periodic
      */
     //if (async) {
-        ehci_set_state(ehci, async, EST_ADVANCEQUEUE);
+        ehci_set_state(q->ehci, async, EST_ADVANCEQUEUE);
         again = 1;
     //} else {
     //    ehci_set_state(ehci, async, EST_ACTIVE);
@@ -1682,6 +1709,7 @@ static int ehci_state_writeback(EHCIState *ehci, int async)
 static void ehci_advance_state(EHCIState *ehci,
                                int async)
 {
+    EHCIQueue *q = NULL;
     int again;
     int iter = 0;
 
@@ -1708,7 +1736,8 @@ static void ehci_advance_state(EHCIState *ehci,
             break;
 
         case EST_FETCHQH:
-            again = ehci_state_fetchqh(ehci, async);
+            q = ehci_state_fetchqh(ehci, async);
+            again = q ? 1 : 0;
             break;
 
         case EST_FETCHITD:
@@ -1716,28 +1745,29 @@ static void ehci_advance_state(EHCIState *ehci,
             break;
 
         case EST_ADVANCEQUEUE:
-            again = ehci_state_advqueue(ehci, async);
+            again = ehci_state_advqueue(q, async);
             break;
 
         case EST_FETCHQTD:
-            again = ehci_state_fetchqtd(ehci, async);
+            again = ehci_state_fetchqtd(q, async);
             break;
 
         case EST_HORIZONTALQH:
-            again = ehci_state_horizqh(ehci, async);
+            again = ehci_state_horizqh(q, async);
             break;
 
         case EST_EXECUTE:
             iter = 0;
-            again = ehci_state_execute(ehci, async);
+            again = ehci_state_execute(q, async);
             break;
 
         case EST_EXECUTING:
-            again = ehci_state_executing(ehci, async);
+            q = &ehci->queue; /* temporary */
+            again = ehci_state_executing(q, async);
             break;
 
         case EST_WRITEBACK:
-            again = ehci_state_writeback(ehci, async);
+            again = ehci_state_writeback(q, async);
             break;
 
         default:
@@ -1759,7 +1789,6 @@ static void ehci_advance_state(EHCIState *ehci,
 
 static void ehci_advance_async_state(EHCIState *ehci)
 {
-    EHCIqh qh;
     int async = 1;
 
     switch(ehci_get_state(ehci, async)) {
@@ -1808,8 +1837,6 @@ static void ehci_advance_async_state(EHCIState *ehci)
         /* fall through */
 
     case EST_EXECUTING:
-        get_dwords(NLPTR_GET(ehci->qhaddr), (uint32_t *) &qh,
-                   sizeof(EHCIqh) >> 2);
         ehci_advance_state(ehci, async);
         break;
 
@@ -1817,7 +1844,7 @@ static void ehci_advance_async_state(EHCIState *ehci)
         /* this should only be due to a developer mistake */
         fprintf(stderr, "ehci: Bad asynchronous state %d. "
                 "Resetting to active\n", ehci->astate);
-        ehci_set_state(ehci, async, EST_ACTIVE);
+        assert(0);
     }
 }
 
@@ -1857,7 +1884,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
 
         DPRINTF("PERIODIC state adv fr=%d.  [%08X] -> %08X\n",
                 ehci->frindex / 8, list, entry);
-        ehci->fetch_addr = entry;
+        ehci_set_fetch_addr(ehci, async,entry);
         ehci_set_state(ehci, async, EST_FETCHENTRY);
         ehci_advance_state(ehci, async);
         break;
@@ -1871,7 +1898,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
         /* this should only be due to a developer mistake */
         fprintf(stderr, "ehci: Bad periodic state %d. "
                 "Resetting to active\n", ehci->pstate);
-        ehci_set_state(ehci, async, EST_ACTIVE);
+        assert(0);
     }
 }
 
@@ -2043,6 +2070,7 @@ static int usb_ehci_initfn(PCIDevice *dev)
     }
 
     s->frame_timer = qemu_new_timer_ns(vm_clock, ehci_frame_timer, s);
+    s->queue.ehci = s;
 
     qemu_register_reset(ehci_reset, s);
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 08/12] usb-ehci: multiqueue support
  2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
                   ` (6 preceding siblings ...)
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 07/12] usb-ehci: add queue data struct Gerd Hoffmann
@ 2011-05-26 10:44 ` Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 09/12] usb-ehci: fix offset writeback in ehci_buffer_rw Gerd Hoffmann
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

This patch adds support for keeping multiple queues going at the same
time.  One slow device will not affect other devices any more.

The patch adds code to manage EHCIQueue structs.  It also does a number
of changes to the state machine:

 * The state machine will never ever stop in EXECUTING any more.
   Instead it will continue with the next queue (aka HORIZONTALQH) when
   the usb device returns USB_RET_ASYNC.
 * The state machine will stop processing when it figures it walks in
   circles (easy to figure now that we have a EHCIQueue struct for each
   QH we've processed).  The bailout logic should not be needed any
   more.  For now it is still in, but will assert() in case it triggers.
 * The state machine will just skip queues with a async USBPacket in
   flight.
 * The state machine will resume processing as soon as the async
   USBPacket is finished.

The patch also takes care to flush the QH struct back to guest memory
when needed, so we don't get stale data when (re-)loading it from guest
memory in FETCHQH state.

It also makes the writeback code to not touch the first three dwords of
the QH struct as the EHCI must not write them.  This actually fixes a
bug where QH chaining changes (next ptr) by the linux ehci driver where
overwritten by the emulated EHCI.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-ehci.c |  171 ++++++++++++++++++++++++++++++++++++++++++++++-----------
 trace-events  |    5 +-
 2 files changed, 142 insertions(+), 34 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 3656a35..5cbb675 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -345,6 +345,9 @@ enum async_state {
 
 struct EHCIQueue {
     EHCIState *ehci;
+    QTAILQ_ENTRY(EHCIQueue) next;
+    bool async_schedule;
+    uint32_t seen, ts;
 
     /* cached data from guest - needs to be flushed
      * when guest removes an entry (doorbell, handshake sequence)
@@ -400,7 +403,7 @@ struct EHCIState {
     int pstate;                        // Current state in periodic schedule
     USBPort ports[NB_PORTS];
     uint32_t usbsts_pending;
-    EHCIQueue queue;
+    QTAILQ_HEAD(, EHCIQueue) queues;
 
     uint32_t a_fetch_addr;   // which address to look at next
     uint32_t p_fetch_addr;   // which address to look at next
@@ -594,9 +597,9 @@ static int ehci_get_fetch_addr(EHCIState *s, int async)
     return async ? s->a_fetch_addr : s->p_fetch_addr;
 }
 
-static void ehci_trace_qh(EHCIState *s, target_phys_addr_t addr, EHCIqh *qh)
+static void ehci_trace_qh(EHCIQueue *q, target_phys_addr_t addr, EHCIqh *qh)
 {
-    trace_usb_ehci_qh(addr, qh->next,
+    trace_usb_ehci_qh(q, addr, qh->next,
                       qh->current_qtd, qh->next_qtd, qh->altnext_qtd,
                       get_field(qh->epchar, QH_EPCHAR_RL),
                       get_field(qh->epchar, QH_EPCHAR_MPLEN),
@@ -609,9 +612,9 @@ static void ehci_trace_qh(EHCIState *s, target_phys_addr_t addr, EHCIqh *qh)
                       (bool)(qh->epchar & QH_EPCHAR_I));
 }
 
-static void ehci_trace_qtd(EHCIState *s, target_phys_addr_t addr, EHCIqtd *qtd)
+static void ehci_trace_qtd(EHCIQueue *q, target_phys_addr_t addr, EHCIqtd *qtd)
 {
-    trace_usb_ehci_qtd(addr, qtd->next, qtd->altnext,
+    trace_usb_ehci_qtd(q, addr, qtd->next, qtd->altnext,
                        get_field(qtd->token, QTD_TOKEN_TBYTES),
                        get_field(qtd->token, QTD_TOKEN_CPAGE),
                        get_field(qtd->token, QTD_TOKEN_CERR),
@@ -628,6 +631,69 @@ static void ehci_trace_itd(EHCIState *s, target_phys_addr_t addr, EHCIitd *itd)
     trace_usb_ehci_itd(addr, itd->next);
 }
 
+/* queue management */
+
+static EHCIQueue *ehci_alloc_queue(EHCIState *ehci, int async)
+{
+    EHCIQueue *q;
+
+    q = qemu_mallocz(sizeof(*q));
+    q->ehci = ehci;
+    q->async_schedule = async;
+    QTAILQ_INSERT_HEAD(&ehci->queues, q, next);
+    trace_usb_ehci_queue_action(q, "alloc");
+    return q;
+}
+
+static void ehci_free_queue(EHCIQueue *q)
+{
+    trace_usb_ehci_queue_action(q, "free");
+    if (q->async == EHCI_ASYNC_INFLIGHT) {
+        usb_cancel_packet(&q->packet);
+    }
+    QTAILQ_REMOVE(&q->ehci->queues, q, next);
+    qemu_free(q);
+}
+
+static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr)
+{
+    EHCIQueue *q;
+
+    QTAILQ_FOREACH(q, &ehci->queues, next) {
+        if (addr == q->qhaddr) {
+            return q;
+        }
+    }
+    return NULL;
+}
+
+static void ehci_queues_rip_unused(EHCIState *ehci)
+{
+    EHCIQueue *q, *tmp;
+
+    QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
+        if (q->seen) {
+            q->seen = 0;
+            q->ts = ehci->last_run_usec;
+            continue;
+        }
+        if (ehci->last_run_usec < q->ts + 250000) {
+            /* allow 0.25 sec idle */
+            continue;
+        }
+        ehci_free_queue(q);
+    }
+}
+
+static void ehci_queues_rip_all(EHCIState *ehci)
+{
+    EHCIQueue *q, *tmp;
+
+    QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
+        ehci_free_queue(q);
+    }
+}
+
 /* Attach or detach a device on root hub */
 
 static void ehci_attach(USBPort *port)
@@ -697,6 +763,7 @@ static void ehci_reset(void *opaque)
             usb_attach(&s->ports[i], s->ports[i].dev);
         }
     }
+    ehci_queues_rip_all(s);
 }
 
 static uint32_t ehci_mem_readb(void *ptr, target_phys_addr_t addr)
@@ -1022,8 +1089,8 @@ static void ehci_async_complete_packet(USBDevice *dev, USBPacket *packet)
 {
     EHCIQueue *q = container_of(packet, EHCIQueue, packet);
 
-    DPRINTF("Async packet complete\n");
-    assert(q->async == EHIC_ASYNC_INFLIGHT);
+    trace_usb_ehci_queue_action(q, "wakeup");
+    assert(q->async == EHCI_ASYNC_INFLIGHT);
     q->async = EHCI_ASYNC_FINISHED;
     q->usb_status = packet->len;
 }
@@ -1032,10 +1099,7 @@ static void ehci_execute_complete(EHCIQueue *q)
 {
     int c_err, reload;
 
-    if (q->async == EHCI_ASYNC_INFLIGHT) {
-        DPRINTF("not done yet\n");
-        return;
-    }
+    assert(q->async != EHCI_ASYNC_INFLIGHT);
     q->async = EHCI_ASYNC_NONE;
 
     DPRINTF("execute_complete: qhaddr 0x%x, next %x, qtdaddr 0x%x, status %d\n",
@@ -1185,10 +1249,6 @@ static int ehci_execute(EHCIQueue *q)
         return USB_RET_PROCERR;
     }
 
-    if (ret == USB_RET_ASYNC) {
-        q->async = EHCI_ASYNC_INFLIGHT;
-    }
-
     return ret;
 }
 
@@ -1328,10 +1388,12 @@ static int ehci_state_waitlisthead(EHCIState *ehci,  int async)
         ehci_set_usbsts(ehci, USBSTS_REC);
     }
 
+    ehci_queues_rip_unused(ehci);
+
     /*  Find the head of the list (4.9.1.1) */
     for(i = 0; i < MAX_QH; i++) {
         get_dwords(NLPTR_GET(entry), (uint32_t *) &qh, sizeof(EHCIqh) >> 2);
-        ehci_trace_qh(ehci, NLPTR_GET(entry), &qh);
+        ehci_trace_qh(NULL, NLPTR_GET(entry), &qh);
 
         if (qh.epchar & QH_EPCHAR_H) {
             if (async) {
@@ -1419,11 +1481,34 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
     int reload;
 
     entry = ehci_get_fetch_addr(ehci, async);
-    q = &ehci->queue; /* temporary */
+    q = ehci_find_queue_by_qh(ehci, entry);
+    if (NULL == q) {
+        q = ehci_alloc_queue(ehci, async);
+    }
     q->qhaddr = entry;
+    q->seen++;
+
+    if (q->seen > 1) {
+        /* we are going in circles -- stop processing */
+        ehci_set_state(ehci, async, EST_ACTIVE);
+        q = NULL;
+        goto out;
+    }
 
     get_dwords(NLPTR_GET(q->qhaddr), (uint32_t *) &q->qh, sizeof(EHCIqh) >> 2);
-    ehci_trace_qh(ehci, NLPTR_GET(q->qhaddr), &q->qh);
+    ehci_trace_qh(q, NLPTR_GET(q->qhaddr), &q->qh);
+
+    if (q->async == EHCI_ASYNC_INFLIGHT) {
+        /* I/O still in progress -- skip queue */
+        ehci_set_state(ehci, async, EST_HORIZONTALQH);
+        goto out;
+    }
+    if (q->async == EHCI_ASYNC_FINISHED) {
+        /* I/O finished -- continue processing queue */
+        trace_usb_ehci_queue_action(q, "resume");
+        ehci_set_state(ehci, async, EST_EXECUTING);
+        goto out;
+    }
 
     if (async && (q->qh.epchar & QH_EPCHAR_H)) {
 
@@ -1542,7 +1627,7 @@ static int ehci_state_fetchqtd(EHCIQueue *q, int async)
     int again = 0;
 
     get_dwords(NLPTR_GET(q->qtdaddr),(uint32_t *) &q->qtd, sizeof(EHCIqtd) >> 2);
-    ehci_trace_qtd(q->ehci, NLPTR_GET(q->qtdaddr), &q->qtd);
+    ehci_trace_qtd(q, NLPTR_GET(q->qtdaddr), &q->qtd);
 
     if (q->qtd.token & QTD_TOKEN_ACTIVE) {
         ehci_set_state(q->ehci, async, EST_EXECUTE);
@@ -1570,6 +1655,23 @@ static int ehci_state_horizqh(EHCIQueue *q, int async)
     return again;
 }
 
+/*
+ *  Write the qh back to guest physical memory.  This step isn't
+ *  in the EHCI spec but we need to do it since we don't share
+ *  physical memory with our guest VM.
+ *
+ *  The first three bytes are read-only for the EHCI, so skip them
+ *  when writing back the qh.
+ */
+static void ehci_flush_qh(EHCIQueue *q)
+{
+    uint32_t *qh = (uint32_t *) &q->qh;
+    uint32_t dwords = sizeof(EHCIqh) >> 2;
+    uint32_t addr = NLPTR_GET(q->qhaddr);
+
+    put_dwords(addr + 3 * sizeof(uint32_t), qh + 3, dwords - 3);
+}
+
 static int ehci_state_execute(EHCIQueue *q, int async)
 {
     int again = 0;
@@ -1614,12 +1716,18 @@ static int ehci_state_execute(EHCIQueue *q, int async)
         again = -1;
         goto out;
     }
-    ehci_set_state(q->ehci, async, EST_EXECUTING);
-
-    if (q->usb_status != USB_RET_ASYNC) {
+    if (q->usb_status == USB_RET_ASYNC) {
+        ehci_flush_qh(q);
+        trace_usb_ehci_queue_action(q, "suspend");
+        q->async = EHCI_ASYNC_INFLIGHT;
+        ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
         again = 1;
+        goto out;
     }
 
+    ehci_set_state(q->ehci, async, EST_EXECUTING);
+    again = 1;
+
 out:
     return again;
 }
@@ -1660,13 +1768,6 @@ static int ehci_state_executing(EHCIQueue *q, int async)
         set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);
     }
 
-    /*
-     *  Write the qh back to guest physical memory.  This step isn't
-     *  in the EHCI spec but we need to do it since we don't share
-     *  physical memory with our guest VM.
-     */
-    put_dwords(NLPTR_GET(q->qhaddr), (uint32_t *) &q->qh, sizeof(EHCIqh) >> 2);
-
     /* 4.10.5 */
     if ((q->usb_status == USB_RET_NAK) || (q->qh.token & QTD_TOKEN_ACTIVE)) {
         ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
@@ -1677,6 +1778,7 @@ static int ehci_state_executing(EHCIQueue *q, int async)
     again = 1;
 
 out:
+    ehci_flush_qh(q);
     return again;
 }
 
@@ -1686,7 +1788,7 @@ static int ehci_state_writeback(EHCIQueue *q, int async)
     int again = 0;
 
     /*  Write back the QTD from the QH area */
-    ehci_trace_qtd(q->ehci, NLPTR_GET(q->qtdaddr), (EHCIqtd*) &q->qh.next_qtd);
+    ehci_trace_qtd(q, NLPTR_GET(q->qtdaddr), (EHCIqtd*) &q->qh.next_qtd);
     put_dwords(NLPTR_GET(q->qtdaddr),(uint32_t *) &q->qh.next_qtd,
                 sizeof(EHCIqtd) >> 2);
 
@@ -1720,11 +1822,14 @@ static void ehci_advance_state(EHCIState *ehci,
              * something is wrong with the linked list. TO-DO: why is
              * this hack needed?
              */
+            assert(iter < MAX_ITERATIONS);
+#if 0
             if (iter > MAX_ITERATIONS) {
                 DPRINTF("\n*** advance_state: bailing on MAX ITERATIONS***\n");
                 ehci_set_state(ehci, async, EST_ACTIVE);
                 break;
             }
+#endif
         }
         switch(ehci_get_state(ehci, async)) {
         case EST_WAITLISTHEAD:
@@ -1762,7 +1867,7 @@ static void ehci_advance_state(EHCIState *ehci,
             break;
 
         case EST_EXECUTING:
-            q = &ehci->queue; /* temporary */
+            assert(q != NULL);
             again = ehci_state_executing(q, async);
             break;
 
@@ -1773,6 +1878,7 @@ static void ehci_advance_state(EHCIState *ehci,
         default:
             fprintf(stderr, "Bad state!\n");
             again = -1;
+            assert(0);
             break;
         }
 
@@ -1780,6 +1886,7 @@ static void ehci_advance_state(EHCIState *ehci,
             fprintf(stderr, "processing error - resetting ehci HC\n");
             ehci_reset(ehci);
             again = 0;
+            assert(0);
         }
     }
     while (again);
@@ -2070,7 +2177,7 @@ static int usb_ehci_initfn(PCIDevice *dev)
     }
 
     s->frame_timer = qemu_new_timer_ns(vm_clock, ehci_frame_timer, s);
-    s->queue.ehci = s;
+    QTAILQ_INIT(&s->queues);
 
     qemu_register_reset(ehci_reset, s);
 
diff --git a/trace-events b/trace-events
index 47a3260..41ad222 100644
--- a/trace-events
+++ b/trace-events
@@ -201,13 +201,14 @@ disable usb_ehci_mmio_writel(uint32_t addr, const char *str, uint32_t val) "wr m
 disable usb_ehci_mmio_change(uint32_t addr, const char *str, uint32_t new, uint32_t old) "ch mmio %04x [%s] = %x (old: %x)"
 disable usb_ehci_usbsts(const char *sts, int state) "usbsts %s %d"
 disable usb_ehci_state(const char *schedule, const char *state) "%s schedule %s"
-disable usb_ehci_qh(uint32_t addr, uint32_t next, uint32_t c_qtd, uint32_t n_qtd, uint32_t a_qtd, int rl, int mplen, int eps, int ep, int devaddr, int c, int h, int dtc, int i) "QH @ %08x: next %08x qtds %08x,%08x,%08x - rl %d, mplen %d, eps %d, ep %d, dev %d, c %d, h %d, dtc %d, i %d"
-disable usb_ehci_qtd(uint32_t addr, uint32_t next, uint32_t altnext, int tbytes, int cpage, int cerr, int pid, int ioc, int active, int halt, int babble, int xacterr) "QH @ %08x: next %08x altnext %08x - tbytes %d, cpage %d, cerr %d, pid %d, ioc %d, active %d, halt %d, babble %d, xacterr %d"
+disable usb_ehci_qh(void *q, uint32_t addr, uint32_t next, uint32_t c_qtd, uint32_t n_qtd, uint32_t a_qtd, int rl, int mplen, int eps, int ep, int devaddr, int c, int h, int dtc, int i) "q %p - QH @ %08x: next %08x qtds %08x,%08x,%08x - rl %d, mplen %d, eps %d, ep %d, dev %d, c %d, h %d, dtc %d, i %d"
+disable usb_ehci_qtd(void *q, uint32_t addr, uint32_t next, uint32_t altnext, int tbytes, int cpage, int cerr, int pid, int ioc, int active, int halt, int babble, int xacterr) "q %p - QTD @ %08x: next %08x altnext %08x - tbytes %d, cpage %d, cerr %d, pid %d, ioc %d, active %d, halt %d, babble %d, xacterr %d"
 disable usb_ehci_itd(uint32_t addr, uint32_t next) "ITD @ %08x: next %08x"
 disable usb_ehci_port_attach(uint32_t port, const char *device) "attach port #%d - %s"
 disable usb_ehci_port_detach(uint32_t port) "detach port #%d"
 disable usb_ehci_port_reset(uint32_t port, int enable) "reset port #%d - %d"
 disable usb_ehci_data(int rw, uint32_t cpage, uint32_t offset, uint32_t addr, uint32_t len, uint32_t bufpos) "write %d, cpage %d, offset 0x%03x, addr 0x%08x, len %d, bufpos %d"
+disable usb_ehci_queue_action(void *q, const char *action) "q %p: %s"
 
 # hw/usb-desc.c
 disable usb_desc_device(int addr, int len, int ret) "dev %d query device, len %d, ret %d"
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 09/12] usb-ehci: fix offset writeback in ehci_buffer_rw
  2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
                   ` (7 preceding siblings ...)
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 08/12] usb-ehci: multiqueue support Gerd Hoffmann
@ 2011-05-26 10:44 ` Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 10/12] usb-ehci: fix error handling Gerd Hoffmann
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Two bugs at once:

First the mask is backwards, so the it used to keeps the offset and
clears the page address, which is not what we need when we update the
offset.

Second the offset calculation is wrong in case head isn't page aligned.

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

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 5cbb675..cf10dfc 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1066,6 +1066,7 @@ static int ehci_buffer_rw(EHCIQueue *q, int bytes, int rw)
         cpu_physical_memory_rw(head, q->buffer + bufpos, tail - head, rw);
 
         bufpos += (tail - head);
+        offset += (tail - head);
         bytes -= (tail - head);
 
         if (bytes > 0) {
@@ -1078,8 +1079,7 @@ static int ehci_buffer_rw(EHCIQueue *q, int bytes, int rw)
     set_field(&q->qh.token, cpage, QTD_TOKEN_CPAGE);
 
     /* save offset into cpage */
-    offset = tail - head;
-    q->qh.bufptr[0] &= ~QTD_BUFPTR_MASK;
+    q->qh.bufptr[0] &= QTD_BUFPTR_MASK;
     q->qh.bufptr[0] |= offset;
 
     return 0;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 10/12] usb-ehci: fix error handling.
  2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
                   ` (8 preceding siblings ...)
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 09/12] usb-ehci: fix offset writeback in ehci_buffer_rw Gerd Hoffmann
@ 2011-05-26 10:44 ` Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 11/12] ehci: fix a number of unused-but-set-variable warnings (new with gcc-4.6) Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 12/12] usb: cancel async packets on unplug Gerd Hoffmann
  11 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Set the correct bits for nodev, stall and babble errors.
Raise errint irq.  Fix state transition from WRITEBACK
to the next state.

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

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index cf10dfc..2bbb5e4 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1114,10 +1114,10 @@ err:
 
         switch(q->usb_status) {
         case USB_RET_NODEV:
-            fprintf(stderr, "USB no device\n");
+            q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_XACTERR);
+            ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
             break;
         case USB_RET_STALL:
-            fprintf(stderr, "USB stall\n");
             q->qh.token |= QTD_TOKEN_HALT;
             ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
             break;
@@ -1133,8 +1133,7 @@ err:
             }
             break;
         case USB_RET_BABBLE:
-            fprintf(stderr, "USB babble TODO\n");
-            q->qh.token |= QTD_TOKEN_BABBLE;
+            q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_BABBLE);
             ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
             break;
         default:
@@ -1792,15 +1791,21 @@ static int ehci_state_writeback(EHCIQueue *q, int async)
     put_dwords(NLPTR_GET(q->qtdaddr),(uint32_t *) &q->qh.next_qtd,
                 sizeof(EHCIqtd) >> 2);
 
-    /* TODO confirm next state.  For now, keep going if async
-     * but stop after one qtd if periodic
+    /*
+     * EHCI specs say go horizontal here.
+     *
+     * We can also advance the queue here for performance reasons.  We
+     * need to take care to only take that shortcut in case we've
+     * processed the qtd just written back without errors, i.e. halt
+     * bit is clear.
      */
-    //if (async) {
+    if (q->qh.token & QTD_TOKEN_HALT) {
+        ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
+        again = 1;
+    } else {
         ehci_set_state(q->ehci, async, EST_ADVANCEQUEUE);
         again = 1;
-    //} else {
-    //    ehci_set_state(ehci, async, EST_ACTIVE);
-    //}
+    }
     return again;
 }
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 11/12] ehci: fix a number of unused-but-set-variable warnings (new with gcc-4.6)
  2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
                   ` (9 preceding siblings ...)
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 10/12] usb-ehci: fix error handling Gerd Hoffmann
@ 2011-05-26 10:44 ` Gerd Hoffmann
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 12/12] usb: cancel async packets on unplug Gerd Hoffmann
  11 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hans de Goede

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

---
 hw/usb-ehci.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 2bbb5e4..e368395 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -740,11 +740,9 @@ static void ehci_detach(USBPort *port)
 static void ehci_reset(void *opaque)
 {
     EHCIState *s = opaque;
-    uint8_t *pci_conf;
     int i;
 
     trace_usb_ehci_reset();
-    pci_conf = s->dev.config;
 
     memset(&s->mmio[OPREGBASE], 0x00, MMIO_SIZE - OPREGBASE);
 
@@ -1268,12 +1266,11 @@ static int ehci_process_itd(EHCIState *ehci,
     int dir;
     int devadr;
     int endp;
-    int maxpkt;
 
     dir =(itd->bufptr[1] & ITD_BUFPTR_DIRECTION);
     devadr = get_field(itd->bufptr[0], ITD_BUFPTR_DEVADDR);
     endp = get_field(itd->bufptr[0], ITD_BUFPTR_EP);
-    maxpkt = get_field(itd->bufptr[1], ITD_BUFPTR_MAXPKT);
+    /* maxpkt = get_field(itd->bufptr[1], ITD_BUFPTR_MAXPKT); */
 
     for(i = 0; i < 8; i++) {
         if (itd->transact[i] & ITD_XACT_ACTIVE) {
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 12/12] usb: cancel async packets on unplug
  2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
                   ` (10 preceding siblings ...)
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 11/12] ehci: fix a number of unused-but-set-variable warnings (new with gcc-4.6) Gerd Hoffmann
@ 2011-05-26 10:44 ` Gerd Hoffmann
  11 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-26 10:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

This patch adds USBBusOps struct with (for now) only a single callback
which is called when a device is about to be destroyed.  The USB Host
adapters are implementing this callback and use it to cancel any async
requests which might be in flight before the device actually goes away.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-bus.c  |    5 ++++-
 hw/usb-ehci.c |   25 ++++++++++++++++++++++++-
 hw/usb-musb.c |   23 ++++++++++++++++++++++-
 hw/usb-ohci.c |   16 +++++++++++++++-
 hw/usb-uhci.c |   26 +++++++++++++++++++++++++-
 hw/usb.h      |    8 +++++++-
 6 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index abc7e61..874c253 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -39,9 +39,10 @@ const VMStateDescription vmstate_usb_device = {
     }
 };
 
-void usb_bus_new(USBBus *bus, DeviceState *host)
+void usb_bus_new(USBBus *bus, USBBusOps *ops, DeviceState *host)
 {
     qbus_create_inplace(&bus->qbus, &usb_bus_info, host, NULL);
+    bus->ops = ops;
     bus->busnr = next_usb_bus++;
     bus->qbus.allow_hotplug = 1; /* Yes, we can */
     QTAILQ_INIT(&bus->free);
@@ -81,8 +82,10 @@ static int usb_qdev_init(DeviceState *qdev, DeviceInfo *base)
 static int usb_qdev_exit(DeviceState *qdev)
 {
     USBDevice *dev = DO_UPCAST(USBDevice, qdev, qdev);
+    USBBus *bus = usb_bus_from_device(dev);
 
     usb_device_detach(dev);
+    bus->ops->device_destroy(bus, dev);
     if (dev->info->handle_destroy) {
         dev->info->handle_destroy(dev);
     }
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index e368395..e345a1c 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -685,6 +685,18 @@ static void ehci_queues_rip_unused(EHCIState *ehci)
     }
 }
 
+static void ehci_queues_rip_device(EHCIState *ehci, USBDevice *dev)
+{
+    EHCIQueue *q, *tmp;
+
+    QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
+        if (q->packet.owner != dev) {
+            continue;
+        }
+        ehci_free_queue(q);
+    }
+}
+
 static void ehci_queues_rip_all(EHCIState *ehci)
 {
     EHCIQueue *q, *tmp;
@@ -2100,6 +2112,13 @@ static void ehci_map(PCIDevice *pci_dev, int region_num,
     cpu_register_physical_memory(addr, size, s->mem);
 }
 
+static void ehci_device_destroy(USBBus *bus, USBDevice *dev)
+{
+    EHCIState *s = container_of(bus, EHCIState, bus);
+
+    ehci_queues_rip_device(s, dev);
+}
+
 static int usb_ehci_initfn(PCIDevice *dev);
 
 static USBPortOps ehci_port_ops = {
@@ -2108,6 +2127,10 @@ static USBPortOps ehci_port_ops = {
     .complete = ehci_async_complete_packet,
 };
 
+static USBBusOps ehci_bus_ops = {
+    .device_destroy = ehci_device_destroy,
+};
+
 static PCIDeviceInfo ehci_info = {
     .qdev.name    = "usb-ehci",
     .qdev.size    = sizeof(EHCIState),
@@ -2170,7 +2193,7 @@ static int usb_ehci_initfn(PCIDevice *dev)
 
     s->irq = s->dev.irq[3];
 
-    usb_bus_new(&s->bus, &s->dev.qdev);
+    usb_bus_new(&s->bus, &ehci_bus_ops, &s->dev.qdev);
     for(i = 0; i < NB_PORTS; i++) {
         usb_register_port(&s->bus, &s->ports[i], s, i, &ehci_port_ops,
                           USB_SPEED_MASK_HIGH);
diff --git a/hw/usb-musb.c b/hw/usb-musb.c
index 6037193..21f35af 100644
--- a/hw/usb-musb.c
+++ b/hw/usb-musb.c
@@ -262,6 +262,7 @@
 static void musb_attach(USBPort *port);
 static void musb_detach(USBPort *port);
 static void musb_schedule_cb(USBDevice *dev, USBPacket *p);
+static void musb_device_destroy(USBBus *bus, USBDevice *dev);
 
 static USBPortOps musb_port_ops = {
     .attach = musb_attach,
@@ -269,6 +270,10 @@ static USBPortOps musb_port_ops = {
     .complete = musb_schedule_cb,
 };
 
+static USBBusOps musb_bus_ops = {
+    .device_destroy = musb_device_destroy,
+};
+
 typedef struct MUSBPacket MUSBPacket;
 typedef struct MUSBEndPoint MUSBEndPoint;
 
@@ -361,7 +366,7 @@ struct MUSBState *musb_init(qemu_irq *irqs)
         s->ep[i].epnum = i;
     }
 
-    usb_bus_new(&s->bus, NULL /* FIXME */);
+    usb_bus_new(&s->bus, &musb_bus_ops, NULL /* FIXME */);
     usb_register_port(&s->bus, &s->port, s, 0, &musb_port_ops,
                       USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL);
     usb_port_location(&s->port, NULL, 1);
@@ -778,6 +783,22 @@ static void musb_rx_packet_complete(USBPacket *packey, void *opaque)
     musb_rx_intr_set(s, epnum, 1);
 }
 
+static void musb_device_destroy(USBBus *bus, USBDevice *dev)
+{
+    MUSBState *s = container_of(bus, MUSBState, bus);
+    int ep, dir;
+
+    for (ep = 0; ep < 16; ep++) {
+        for (dir = 0; dir < 2; dir++) {
+            if (s->ep[ep].packey[dir].p.owner != dev) {
+                continue;
+            }
+            usb_cancel_packet(&s->ep[ep].packey[dir].p);
+            /* status updates needed here? */
+        }
+    }
+}
+
 static void musb_tx_rdy(MUSBState *s, int epnum)
 {
     MUSBEndPoint *ep = s->ep + epnum;
diff --git a/hw/usb-ohci.c b/hw/usb-ohci.c
index 8b966f7..401045a 100644
--- a/hw/usb-ohci.c
+++ b/hw/usb-ohci.c
@@ -1644,6 +1644,16 @@ static void ohci_mem_write(void *ptr, target_phys_addr_t addr, uint32_t val)
     }
 }
 
+static void ohci_device_destroy(USBBus *bus, USBDevice *dev)
+{
+    OHCIState *ohci = container_of(bus, OHCIState, bus);
+
+    if (ohci->async_td && ohci->usb_packet.owner == dev) {
+        usb_cancel_packet(&ohci->usb_packet);
+        ohci->async_td = 0;
+    }
+}
+
 /* Only dword reads are defined on OHCI register space */
 static CPUReadMemoryFunc * const ohci_readfn[3]={
     ohci_mem_read,
@@ -1664,6 +1674,10 @@ static USBPortOps ohci_port_ops = {
     .complete = ohci_async_complete_packet,
 };
 
+static USBBusOps ohci_bus_ops = {
+    .device_destroy = ohci_device_destroy,
+};
+
 static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
                           int num_ports, uint32_t localmem_base)
 {
@@ -1691,7 +1705,7 @@ static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
 
     ohci->name = dev->info->name;
 
-    usb_bus_new(&ohci->bus, dev);
+    usb_bus_new(&ohci->bus, &ohci_bus_ops, dev);
     ohci->num_ports = num_ports;
     for (i = 0; i < num_ports; i++) {
         usb_register_port(&ohci->bus, &ohci->rhport[i].port, ohci, i, &ohci_port_ops,
diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index c0de05b..8f504d1 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -234,6 +234,19 @@ static void uhci_async_validate_end(UHCIState *s)
     }
 }
 
+static void uhci_async_cancel_device(UHCIState *s, USBDevice *dev)
+{
+    UHCIAsync *curr, *n;
+
+    QTAILQ_FOREACH_SAFE(curr, &s->async_pending, next, n) {
+        if (curr->packet.owner != dev) {
+            continue;
+        }
+        uhci_async_unlink(s, curr);
+        uhci_async_cancel(s, curr);
+    }
+}
+
 static void uhci_async_cancel_all(UHCIState *s)
 {
     UHCIAsync *curr, *n;
@@ -1081,6 +1094,13 @@ static void uhci_map(PCIDevice *pci_dev, int region_num,
     register_ioport_read(addr, 32, 1, uhci_ioport_readb, s);
 }
 
+static void uhci_device_destroy(USBBus *bus, USBDevice *dev)
+{
+    UHCIState *s = container_of(bus, UHCIState, bus);
+
+    uhci_async_cancel_device(s, dev);
+}
+
 static USBPortOps uhci_port_ops = {
     .attach = uhci_attach,
     .detach = uhci_detach,
@@ -1088,6 +1108,10 @@ static USBPortOps uhci_port_ops = {
     .complete = uhci_async_complete,
 };
 
+static USBBusOps uhci_bus_ops = {
+    .device_destroy = uhci_device_destroy,
+};
+
 static int usb_uhci_common_initfn(UHCIState *s)
 {
     uint8_t *pci_conf = s->dev.config;
@@ -1100,7 +1124,7 @@ static int usb_uhci_common_initfn(UHCIState *s)
     pci_conf[PCI_INTERRUPT_PIN] = 4; // interrupt pin 3
     pci_conf[0x60] = 0x10; // release number
 
-    usb_bus_new(&s->bus, &s->dev.qdev);
+    usb_bus_new(&s->bus, &uhci_bus_ops, &s->dev.qdev);
     for(i = 0; i < NB_PORTS; i++) {
         usb_register_port(&s->bus, &s->ports[i].port, s, i, &uhci_port_ops,
                           USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL);
diff --git a/hw/usb.h b/hw/usb.h
index 9882400..6097208 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -132,6 +132,7 @@
 #define USB_ENDPOINT_XFER_INT		3
 
 typedef struct USBBus USBBus;
+typedef struct USBBusOps USBBusOps;
 typedef struct USBPort USBPort;
 typedef struct USBDevice USBDevice;
 typedef struct USBDeviceInfo USBDeviceInfo;
@@ -323,6 +324,7 @@ void musb_set_size(MUSBState *s, int epnum, int size, int is_tx);
 
 struct USBBus {
     BusState qbus;
+    USBBusOps *ops;
     int busnr;
     int nfree;
     int nused;
@@ -331,7 +333,11 @@ struct USBBus {
     QTAILQ_ENTRY(USBBus) next;
 };
 
-void usb_bus_new(USBBus *bus, DeviceState *host);
+struct USBBusOps {
+    void (*device_destroy)(USBBus *bus, USBDevice *dev);
+};
+
+void usb_bus_new(USBBus *bus, USBBusOps *ops, DeviceState *host);
 USBBus *usb_bus_find(int busnr);
 void usb_qdev_register(USBDeviceInfo *info);
 void usb_qdev_register_many(USBDeviceInfo *info);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 01/12] usb-linux: catch NODEV in more places.
  2011-05-26 10:44 ` [Qemu-devel] [PATCH 01/12] usb-linux: catch NODEV in more places Gerd Hoffmann
@ 2011-05-26 18:20   ` Markus Armbruster
  2011-05-27  5:58     ` Gerd Hoffmann
  0 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2011-05-26 18:20 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

Gerd Hoffmann <kraxel@redhat.com> writes:

> Factor out disconnect code (called when a device disappears) to a
> separate function.  Add a check for NODEV errno to a few more places to
> make sure we notice disconnects.

Ah, you mean ENODEV!  Suddenly the subject makes sense.  Fix it up?

[...]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 01/12] usb-linux: catch NODEV in more places.
  2011-05-26 18:20   ` Markus Armbruster
@ 2011-05-27  5:58     ` Gerd Hoffmann
  0 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2011-05-27  5:58 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel

On 05/26/11 20:20, Markus Armbruster wrote:
> Gerd Hoffmann<kraxel@redhat.com>  writes:
>
>> Factor out disconnect code (called when a device disappears) to a
>> separate function.  Add a check for NODEV errno to a few more places to
>> make sure we notice disconnects.
>
> Ah, you mean ENODEV!  Suddenly the subject makes sense.  Fix it up?

Done.

thanks,
   Gerd

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2011-05-27  5:58 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-26 10:44 [Qemu-devel] [PATCH 00/12] ehci: tracing, multiqueue, bugfixes Gerd Hoffmann
2011-05-26 10:44 ` [Qemu-devel] [PATCH 01/12] usb-linux: catch NODEV in more places Gerd Hoffmann
2011-05-26 18:20   ` Markus Armbruster
2011-05-27  5:58     ` Gerd Hoffmann
2011-05-26 10:44 ` [Qemu-devel] [PATCH 02/12] usb-ehci: trace mmio and usbsts Gerd Hoffmann
2011-05-26 10:44 ` [Qemu-devel] [PATCH 03/12] usb-ehci: trace state machine changes Gerd Hoffmann
2011-05-26 10:44 ` [Qemu-devel] [PATCH 04/12] usb-ehci: trace port state Gerd Hoffmann
2011-05-26 10:44 ` [Qemu-devel] [PATCH 05/12] usb-ehci: improve mmio tracing Gerd Hoffmann
2011-05-26 10:44 ` [Qemu-devel] [PATCH 06/12] usb-ehci: trace buffer copy Gerd Hoffmann
2011-05-26 10:44 ` [Qemu-devel] [PATCH 07/12] usb-ehci: add queue data struct Gerd Hoffmann
2011-05-26 10:44 ` [Qemu-devel] [PATCH 08/12] usb-ehci: multiqueue support Gerd Hoffmann
2011-05-26 10:44 ` [Qemu-devel] [PATCH 09/12] usb-ehci: fix offset writeback in ehci_buffer_rw Gerd Hoffmann
2011-05-26 10:44 ` [Qemu-devel] [PATCH 10/12] usb-ehci: fix error handling Gerd Hoffmann
2011-05-26 10:44 ` [Qemu-devel] [PATCH 11/12] ehci: fix a number of unused-but-set-variable warnings (new with gcc-4.6) Gerd Hoffmann
2011-05-26 10:44 ` [Qemu-devel] [PATCH 12/12] usb: cancel async packets on unplug Gerd Hoffmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.