qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/31] usb patch queue
@ 2011-06-06 12:38 Gerd Hoffmann
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 01/31] usb-linux: catch ENODEV in more places Gerd Hoffmann
                   ` (30 more replies)
  0 siblings, 31 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

  Hi,

Here is the current usb patch queue.  It brings a bunch of small fixes
and cleanups, especially for EHCI and usb-linux.  Additionally EHCI gets
trace points and support for multiple async requests at the same time.

please pull,
  Gerd

The following changes since commit d800040fb47fe4500d1f8bf604b9fd129bda9419:

  scsi: fix tracing of scsi requests with simple backend (2011-06-05 15:05:35 +0000)

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

Brad Hards (3):
      usb: Add defines for USB Serial Bus Release Number register
      usb: Use defines for serial bus release number register for UHCI
      usb: Use defines for serial bus release number register for EHCI

Gerd Hoffmann (17):
      usb-linux: catch ENODEV 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
      usb-ehci: drop EXECUTING checks.
      usb-ehci: itd handling fixes.
      usb-ehci: split trace calls to handle arg count limits
      usb: documentation update
      usb-linux: only cleanup in host_close when host_open was successful.
      usb: don't call usb_host_device_open from vl.c

Hans de Goede (9):
      ehci: fix a number of unused-but-set-variable warnings (new with gcc-4.6)
      usb-linux: Get speed from sysfs rather then from the connectinfo ioctl
      usb-linux: Teach about super speed
      usb-linux: Don't do perror when errno is not set
      usb-linux: Ensure devep != 0
      usb-linux: Don't try to open the same device twice
      usb-linux: Enlarge buffer for descriptors to 8192 bytes
      usb-bus: Add knowledge of USB_SPEED_SUPER to usb_speed helper
      usb-bus: Don't detach non attached devices on device exit

Kevin O'Connor (2):
      Fix USB mouse Set_Protocol behavior
      The USB tablet should not claim boot protocol support.

 docs/usb2.txt          |   85 ++++
 hw/milkymist-softusb.c |   10 +-
 hw/usb-bus.c           |   10 +-
 hw/usb-ehci.c          | 1198 ++++++++++++++++++++++++++++--------------------
 hw/usb-hid.c           |    5 +-
 hw/usb-musb.c          |   23 +-
 hw/usb-ohci.c          |   16 +-
 hw/usb-uhci.c          |   28 +-
 hw/usb.h               |   14 +-
 trace-events           |   20 +
 usb-linux.c            |   96 +++--
 vl.c                   |    6 +-
 12 files changed, 967 insertions(+), 544 deletions(-)

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

* [Qemu-devel] [PATCH 01/31] usb-linux: catch ENODEV in more places.
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
@ 2011-06-06 12:38 ` Gerd Hoffmann
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 02/31] usb-ehci: trace mmio and usbsts Gerd Hoffmann
                   ` (29 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:38 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 ENODEV 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] 38+ messages in thread

* [Qemu-devel] [PATCH 02/31] usb-ehci: trace mmio and usbsts
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 01/31] usb-linux: catch ENODEV in more places Gerd Hoffmann
@ 2011-06-06 12:38 ` Gerd Hoffmann
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 03/31] usb-ehci: trace state machine changes Gerd Hoffmann
                   ` (28 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:38 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 e0e9574..c87a86e 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] 38+ messages in thread

* [Qemu-devel] [PATCH 03/31] usb-ehci: trace state machine changes
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 01/31] usb-linux: catch ENODEV in more places Gerd Hoffmann
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 02/31] usb-ehci: trace mmio and usbsts Gerd Hoffmann
@ 2011-06-06 12:38 ` Gerd Hoffmann
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 04/31] usb-ehci: trace port state Gerd Hoffmann
                   ` (27 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:38 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 c87a86e..2b89d0f 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] 38+ messages in thread

* [Qemu-devel] [PATCH 04/31] usb-ehci: trace port state
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 03/31] usb-ehci: trace state machine changes Gerd Hoffmann
@ 2011-06-06 12:38 ` Gerd Hoffmann
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 05/31] usb-ehci: improve mmio tracing Gerd Hoffmann
                   ` (26 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:38 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 2b89d0f..38c8939 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] 38+ messages in thread

* [Qemu-devel] [PATCH 05/31] usb-ehci: improve mmio tracing
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (3 preceding siblings ...)
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 04/31] usb-ehci: trace port state Gerd Hoffmann
@ 2011-06-06 12:38 ` Gerd Hoffmann
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 06/31] usb-ehci: trace buffer copy Gerd Hoffmann
                   ` (25 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:38 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 38c8939..1ee711e 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] 38+ messages in thread

* [Qemu-devel] [PATCH 06/31] usb-ehci: trace buffer copy
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (4 preceding siblings ...)
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 05/31] usb-ehci: improve mmio tracing Gerd Hoffmann
@ 2011-06-06 12:38 ` Gerd Hoffmann
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 07/31] usb-ehci: add queue data struct Gerd Hoffmann
                   ` (24 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:38 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 1ee711e..3426e14 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] 38+ messages in thread

* [Qemu-devel] [PATCH 07/31] usb-ehci: add queue data struct
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (5 preceding siblings ...)
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 06/31] usb-ehci: trace buffer copy Gerd Hoffmann
@ 2011-06-06 12:38 ` Gerd Hoffmann
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 08/31] usb-ehci: multiqueue support Gerd Hoffmann
                   ` (23 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:38 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] 38+ messages in thread

* [Qemu-devel] [PATCH 08/31] usb-ehci: multiqueue support
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (6 preceding siblings ...)
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 07/31] usb-ehci: add queue data struct Gerd Hoffmann
@ 2011-06-06 12:38 ` Gerd Hoffmann
  2011-06-06 14:50   ` David Ahern
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 09/31] usb-ehci: fix offset writeback in ehci_buffer_rw Gerd Hoffmann
                   ` (22 subsequent siblings)
  30 siblings, 1 reply; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:38 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 3426e14..51e2e7c 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] 38+ messages in thread

* [Qemu-devel] [PATCH 09/31] usb-ehci: fix offset writeback in ehci_buffer_rw
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (7 preceding siblings ...)
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 08/31] usb-ehci: multiqueue support Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 10/31] usb-ehci: fix error handling Gerd Hoffmann
                   ` (21 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 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] 38+ messages in thread

* [Qemu-devel] [PATCH 10/31] usb-ehci: fix error handling.
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (8 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 09/31] usb-ehci: fix offset writeback in ehci_buffer_rw Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 11/31] ehci: fix a number of unused-but-set-variable warnings (new with gcc-4.6) Gerd Hoffmann
                   ` (20 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 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] 38+ messages in thread

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

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

Signed-off-by: Gerd Hoffmann <kraxel@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] 38+ messages in thread

* [Qemu-devel] [PATCH 12/31] usb: cancel async packets on unplug
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (10 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 11/31] ehci: fix a number of unused-but-set-variable warnings (new with gcc-4.6) Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks Gerd Hoffmann
                   ` (18 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 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/milkymist-softusb.c |   10 +++++++++-
 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 +++++++-
 7 files changed, 106 insertions(+), 7 deletions(-)

diff --git a/hw/milkymist-softusb.c b/hw/milkymist-softusb.c
index 1565260..028f3b7 100644
--- a/hw/milkymist-softusb.c
+++ b/hw/milkymist-softusb.c
@@ -247,10 +247,18 @@ static void softusb_attach(USBPort *port)
 {
 }
 
+static void softusb_device_destroy(USBBus *bus, USBDevice *dev)
+{
+}
+
 static USBPortOps softusb_ops = {
     .attach = softusb_attach,
 };
 
+static USBBusOps softusb_bus_ops = {
+    .device_destroy = softusb_device_destroy,
+};
+
 static void milkymist_softusb_reset(DeviceState *d)
 {
     MilkymistSoftUsbState *s =
@@ -294,7 +302,7 @@ static int milkymist_softusb_init(SysBusDevice *dev)
     qemu_add_mouse_event_handler(softusb_mouse_event, s, 0, "Milkymist Mouse");
 
     /* create our usb bus */
-    usb_bus_new(&s->usbbus, NULL);
+    usb_bus_new(&s->usbbus, &softusb_bus_ops, NULL);
 
     /* our two ports */
     usb_register_port(&s->usbbus, &s->usbport[0], NULL, 0, &softusb_ops,
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] 38+ messages in thread

* [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks.
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (11 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 12/31] usb: cancel async packets on unplug Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 13:34   ` David Ahern
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 14/31] Fix USB mouse Set_Protocol behavior Gerd Hoffmann
                   ` (17 subsequent siblings)
  30 siblings, 1 reply; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

The state machine doesn't stop in EXECUTING state any more when async
packets are in flight, so the checks are not needed any more and can
be dropped.

Also kick out the check for the frame timer.  As we don't stop & sleep
any more on async packets this is obsolete.

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

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index e345a1c..8b1ae3a 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1437,17 +1437,6 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async)
     int again = 0;
     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) {
-        if (async) {
-            DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state machine\n");
-            goto out;
-        } else {
-            DPRINTF("FETCHENTRY: WARNING "
-                    "- frame timer elapsed during periodic\n");
-        }
-    }
-#endif
     if (entry < 0x1000) {
         DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry);
         ehci_set_state(ehci, async, EST_ACTIVE);
@@ -1952,12 +1941,6 @@ static void ehci_advance_async_state(EHCIState *ehci)
         }
 
         ehci_set_state(ehci, async, EST_WAITLISTHEAD);
-        /* fall through */
-
-    case EST_FETCHENTRY:
-        /* fall through */
-
-    case EST_EXECUTING:
         ehci_advance_state(ehci, async);
         break;
 
@@ -2010,11 +1993,6 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
         ehci_advance_state(ehci, async);
         break;
 
-    case EST_EXECUTING:
-        DPRINTF("PERIODIC state adv for executing\n");
-        ehci_advance_state(ehci, async);
-        break;
-
     default:
         /* this should only be due to a developer mistake */
         fprintf(stderr, "ehci: Bad periodic state %d. "
@@ -2063,11 +2041,7 @@ static void ehci_frame_timer(void *opaque)
         if (frames - i > 10) {
             skipped_frames++;
         } else {
-            // TODO could this cause periodic frames to get skipped if async
-            // active?
-            if (ehci_get_state(ehci, 1) != EST_EXECUTING) {
-                ehci_advance_periodic_state(ehci);
-            }
+            ehci_advance_periodic_state(ehci);
         }
 
         ehci->last_run_usec += FRAME_TIMER_USEC;
@@ -2082,9 +2056,7 @@ static void ehci_frame_timer(void *opaque)
     /*  Async is not inside loop since it executes everything it can once
      *  called
      */
-    if (ehci_get_state(ehci, 0) != EST_EXECUTING) {
-        ehci_advance_async_state(ehci);
-    }
+    ehci_advance_async_state(ehci);
 
     qemu_mod_timer(ehci->frame_timer, expire_time);
 }
-- 
1.7.1

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

* [Qemu-devel] [PATCH 14/31] Fix USB mouse Set_Protocol behavior
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (12 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 15/31] The USB tablet should not claim boot protocol support Gerd Hoffmann
                   ` (16 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin O'Connor, Gerd Hoffmann

From: Kevin O'Connor <kevin@koconnor.net>

The QEMU USB mouse claims to support the "boot" protocol
(bInterfaceSubClass is 1).  However, the mouse rejects the
Set_Protocol command.

The qemu mouse does support the "boot" protocol specification, so a
simple fix is to enable the Set_Protocol request.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-hid.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index 53b261c..8197a86 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -782,13 +782,13 @@ static int usb_hid_handle_control(USBDevice *dev, USBPacket *p,
             goto fail;
         break;
     case GET_PROTOCOL:
-        if (s->kind != USB_KEYBOARD)
+        if (s->kind != USB_KEYBOARD && s->kind != USB_MOUSE)
             goto fail;
         ret = 1;
         data[0] = s->protocol;
         break;
     case SET_PROTOCOL:
-        if (s->kind != USB_KEYBOARD)
+        if (s->kind != USB_KEYBOARD && s->kind != USB_MOUSE)
             goto fail;
         ret = 0;
         s->protocol = value;
-- 
1.7.1

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

* [Qemu-devel] [PATCH 15/31] The USB tablet should not claim boot protocol support.
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (13 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 14/31] Fix USB mouse Set_Protocol behavior Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 16/31] usb-ehci: itd handling fixes Gerd Hoffmann
                   ` (15 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin O'Connor, Gerd Hoffmann

From: Kevin O'Connor <kevin@koconnor.net>

The USB tablet advertises that it supports the "boot" protocol.
However, its reports aren't "boot" protocol compatible.  So, it
shouldn't claim that.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-hid.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index 8197a86..d711b5c 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -142,7 +142,6 @@ static const USBDescIface desc_iface_tablet = {
     .bInterfaceNumber              = 0,
     .bNumEndpoints                 = 1,
     .bInterfaceClass               = USB_CLASS_HID,
-    .bInterfaceSubClass            = 0x01, /* boot */
     .bInterfaceProtocol            = 0x02,
     .ndesc                         = 1,
     .descs = (USBDescOther[]) {
-- 
1.7.1

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

* [Qemu-devel] [PATCH 16/31] usb-ehci: itd handling fixes.
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (14 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 15/31] The USB tablet should not claim boot protocol support Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 17/31] usb-ehci: split trace calls to handle arg count limits Gerd Hoffmann
                   ` (14 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

This patch fixes a bunch of issues in the itd descriptor handling.
Most important fix is to handle transfers which cross page borders
correctly by looking up the address of the next page.  Luckily the
linux uses physically contigous memory so the data used to hits the
correct location even with this bug instead of corrupting guest
memory.  Also the transfer length updates for outgoing transfers wasn't
correct.

While being at it DPRINTFs have been replaced by tracepoints.

The isoch_pause logic has been disabled.  Not clear to me which propose
this serves and I think it is incorrect too as we just skip processing
itds.  Even when no xfer happens we have to clear the active bit.

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

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 8b1ae3a..d807245 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -198,6 +198,7 @@ typedef struct EHCIitd {
 #define ITD_BUFPTR_MAXPKT_MASK   0x000007ff
 #define ITD_BUFPTR_MAXPKT_SH     0
 #define ITD_BUFPTR_MULT_MASK     0x00000003
+#define ITD_BUFPTR_MULT_SH       0
 } EHCIitd;
 
 /*  EHCI spec version 1.0 Section 3.4
@@ -628,7 +629,11 @@ static void ehci_trace_qtd(EHCIQueue *q, target_phys_addr_t addr, EHCIqtd *qtd)
 
 static void ehci_trace_itd(EHCIState *s, target_phys_addr_t addr, EHCIitd *itd)
 {
-    trace_usb_ehci_itd(addr, itd->next);
+    trace_usb_ehci_itd(addr, itd->next,
+                       get_field(itd->bufptr[1], ITD_BUFPTR_MAXPKT),
+                       get_field(itd->bufptr[2], ITD_BUFPTR_MULT),
+                       get_field(itd->bufptr[0], ITD_BUFPTR_EP),
+                       get_field(itd->bufptr[0], ITD_BUFPTR_DEVADDR));
 }
 
 /* queue management */
@@ -1270,41 +1275,51 @@ static int ehci_process_itd(EHCIState *ehci,
     USBPort *port;
     USBDevice *dev;
     int ret;
-    int i, j;
-    int ptr;
-    int pid;
-    int pg;
-    int len;
-    int dir;
-    int devadr;
-    int endp;
+    uint32_t i, j, len, len1, len2, pid, dir, devaddr, endp;
+    uint32_t pg, off, ptr1, ptr2, max, mult;
 
     dir =(itd->bufptr[1] & ITD_BUFPTR_DIRECTION);
-    devadr = get_field(itd->bufptr[0], ITD_BUFPTR_DEVADDR);
+    devaddr = 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); */
+    max = get_field(itd->bufptr[1], ITD_BUFPTR_MAXPKT);
+    mult = get_field(itd->bufptr[2], ITD_BUFPTR_MULT);
 
     for(i = 0; i < 8; i++) {
         if (itd->transact[i] & ITD_XACT_ACTIVE) {
-            DPRINTF("ISOCHRONOUS active for frame %d, interval %d\n",
-                    ehci->frindex >> 3, i);
-
-            pg = get_field(itd->transact[i], ITD_XACT_PGSEL);
-            ptr = (itd->bufptr[pg] & ITD_BUFPTR_MASK) |
-                (itd->transact[i] & ITD_XACT_OFFSET_MASK);
-            len = get_field(itd->transact[i], ITD_XACT_LENGTH);
+            pg   = get_field(itd->transact[i], ITD_XACT_PGSEL);
+            off  = itd->transact[i] & ITD_XACT_OFFSET_MASK;
+            ptr1 = (itd->bufptr[pg] & ITD_BUFPTR_MASK);
+            ptr2 = (itd->bufptr[pg+1] & ITD_BUFPTR_MASK);
+            len  = get_field(itd->transact[i], ITD_XACT_LENGTH);
+
+            if (len > max * mult) {
+                len = max * mult;
+            }
 
             if (len > BUFF_SIZE) {
                 return USB_RET_PROCERR;
             }
 
-            DPRINTF("ISOCH: buffer %08X len %d\n", ptr, len);
+            if (off + len > 4096) {
+                /* transfer crosses page border */
+                len2 = off + len - 4096;
+                len1 = len - len2;
+            } else {
+                len1 = len;
+                len2 = 0;
+            }
 
             if (!dir) {
-                cpu_physical_memory_rw(ptr, &ehci->ibuffer[0], len, 0);
                 pid = USB_TOKEN_OUT;
-            } else
+                trace_usb_ehci_data(0, pg, off, ptr1 + off, len1, 0);
+                cpu_physical_memory_rw(ptr1 + off, &ehci->ibuffer[0], len1, 0);
+                if (len2) {
+                    trace_usb_ehci_data(0, pg+1, 0, ptr2, len2, len1);
+                    cpu_physical_memory_rw(ptr2, &ehci->ibuffer[len1], len2, 0);
+                }
+            } else {
                 pid = USB_TOKEN_IN;
+            }
 
             ret = USB_RET_NODEV;
 
@@ -1315,18 +1330,15 @@ static int ehci_process_itd(EHCIState *ehci,
                 // TODO sometime we will also need to check if we are the port owner
 
                 if (!(ehci->portsc[j] &(PORTSC_CONNECT))) {
-                    DPRINTF("Port %d, no exec, not connected(%08X)\n",
-                            j, ehci->portsc[j]);
                     continue;
                 }
 
                 ehci->ipacket.pid = pid;
-                ehci->ipacket.devaddr = devadr;
+                ehci->ipacket.devaddr = devaddr;
                 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->ipacket);
 
                 if (ret != USB_RET_NODEV) {
@@ -1334,6 +1346,7 @@ static int ehci_process_itd(EHCIState *ehci,
                 }
             }
 
+#if 0
             /*  In isoch, there is no facility to indicate a NAK so let's
              *  instead just complete a zero-byte transaction.  Setting
              *  DBERR seems too draconian.
@@ -1358,24 +1371,40 @@ static int ehci_process_itd(EHCIState *ehci,
                 DPRINTF("ISOCH: received ACK, clearing pause\n");
                 ehci->isoch_pause = -1;
             }
+#else
+            if (ret == USB_RET_NAK) {
+                ret = 0;
+            }
+#endif
 
             if (ret >= 0) {
-                itd->transact[i] &= ~ITD_XACT_ACTIVE;
+                if (!dir) {
+                    /* OUT */
+                    set_field(&itd->transact[i], len - ret, ITD_XACT_LENGTH);
+                } else {
+                    /* IN */
+                    if (len1 > ret) {
+                        len1 = ret;
+                    }
+                    if (len2 > ret - len1) {
+                        len2 = ret - len1;
+                    }
+                    if (len1) {
+                        trace_usb_ehci_data(1, pg, off, ptr1 + off, len1, 0);
+                        cpu_physical_memory_rw(ptr1 + off, &ehci->ibuffer[0], len1, 1);
+                    }
+                    if (len2) {
+                        trace_usb_ehci_data(1, pg+1, 0, ptr2, len2, len1);
+                        cpu_physical_memory_rw(ptr2, &ehci->ibuffer[len1], len2, 1);
+                    }
+                    set_field(&itd->transact[i], ret, ITD_XACT_LENGTH);
+                }
 
                 if (itd->transact[i] & ITD_XACT_IOC) {
                     ehci_record_interrupt(ehci, USBSTS_INT);
                 }
             }
-
-            if (ret >= 0 && dir) {
-                cpu_physical_memory_rw(ptr, &ehci->ibuffer[0], len, 1);
-
-                if (ret != len) {
-                    DPRINTF("ISOCH IN expected %d, got %d\n",
-                            len, ret);
-                    set_field(&itd->transact[i], ret, ITD_XACT_LENGTH);
-                }
-            }
+            itd->transact[i] &= ~ITD_XACT_ACTIVE;
         }
     }
     return 0;
diff --git a/trace-events b/trace-events
index 51e2e7c..dd69702 100644
--- a/trace-events
+++ b/trace-events
@@ -203,7 +203,7 @@ 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(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_itd(uint32_t addr, uint32_t next, uint32_t mplen, uint32_t mult, uint32_t ep, uint32_t devaddr) "ITD @ %08x: next %08x - mplen %d, mult %d, ep %d, dev %d"
 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"
-- 
1.7.1

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

* [Qemu-devel] [PATCH 17/31] usb-ehci: split trace calls to handle arg count limits
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (15 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 16/31] usb-ehci: itd handling fixes Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 18/31] usb: documentation update Gerd Hoffmann
                   ` (13 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

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

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index d807245..b151b48 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -600,31 +600,37 @@ static int ehci_get_fetch_addr(EHCIState *s, int async)
 
 static void ehci_trace_qh(EHCIQueue *q, target_phys_addr_t addr, EHCIqh *qh)
 {
-    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),
-                      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));
+    /* need three here due to argument count limits */
+    trace_usb_ehci_qh_ptrs(q, addr, qh->next,
+                           qh->current_qtd, qh->next_qtd, qh->altnext_qtd);
+    trace_usb_ehci_qh_fields(addr,
+                             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));
+    trace_usb_ehci_qh_bits(addr,
+                           (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(EHCIQueue *q, target_phys_addr_t addr, EHCIqtd *qtd)
 {
-    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),
-                       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));
+    /* need three here due to argument count limits */
+    trace_usb_ehci_qtd_ptrs(q, addr, qtd->next, qtd->altnext);
+    trace_usb_ehci_qtd_fields(addr,
+                              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));
+    trace_usb_ehci_qtd_bits(addr,
+                            (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)
diff --git a/trace-events b/trace-events
index dd69702..f1230f1 100644
--- a/trace-events
+++ b/trace-events
@@ -201,8 +201,12 @@ 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(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_qh_ptrs(void *q, uint32_t addr, uint32_t next, uint32_t c_qtd, uint32_t n_qtd, uint32_t a_qtd) "q %p - QH @ %08x: next %08x qtds %08x,%08x,%08x"
+disable usb_ehci_qh_fields(uint32_t addr, int rl, int mplen, int eps, int ep, int devaddr) "QH @ %08x - rl %d, mplen %d, eps %d, ep %d, dev %d"
+disable usb_ehci_qh_bits(uint32_t addr, int c, int h, int dtc, int i) "QH @ %08x - c %d, h %d, dtc %d, i %d"
+disable usb_ehci_qtd_ptrs(void *q, uint32_t addr, uint32_t next, uint32_t altnext) "q %p - QTD @ %08x: next %08x altnext %08x"
+disable usb_ehci_qtd_fields(uint32_t addr, int tbytes, int cpage, int cerr, int pid) "QTD @ %08x - tbytes %d, cpage %d, cerr %d, pid %d"
+disable usb_ehci_qtd_bits(uint32_t addr, int ioc, int active, int halt, int babble, int xacterr) "QTD @ %08x - ioc %d, active %d, halt %d, babble %d, xacterr %d"
 disable usb_ehci_itd(uint32_t addr, uint32_t next, uint32_t mplen, uint32_t mult, uint32_t ep, uint32_t devaddr) "ITD @ %08x: next %08x - mplen %d, mult %d, ep %d, dev %d"
 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"
-- 
1.7.1

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

* [Qemu-devel] [PATCH 18/31] usb: documentation update
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (16 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 17/31] usb-ehci: split trace calls to handle arg count limits Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 19/31] usb-linux: Get speed from sysfs rather then from the connectinfo ioctl Gerd Hoffmann
                   ` (12 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Add some more informations to docs/usb2.txt about using usb2 (also usb1)
devices.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 docs/usb2.txt |   85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 85 insertions(+), 0 deletions(-)

diff --git a/docs/usb2.txt b/docs/usb2.txt
index b283c13..5950c71 100644
--- a/docs/usb2.txt
+++ b/docs/usb2.txt
@@ -31,6 +31,91 @@ a complete example:
 This attaches a usb tablet to the UHCI adapter and a usb mass storage
 device to the EHCI adapter.
 
+
+More USB tips & tricks
+======================
+
+Recently the usb pass through driver (also known as usb-host) and the
+qemu usb subsystem gained a few capabilities which are available only
+via qdev properties, i,e. when using '-device'.
+
+
+physical port addressing
+------------------------
+
+First you can (for all usb devices) specify the physical port where
+the device will show up in the guest.  This can be done using the
+"port" property.  UHCI has two root ports (1,2).  EHCI has four root
+ports (1-4), the emulated (1.1) USB hub has eight ports.
+
+Plugging a tablet into UHCI port 1 works like this:
+
+        -device usb-tablet,bus=usb.0,port=1
+
+Plugging a hub into UHCI port 2 works like this:
+
+        -device usb-hub,bus=usb.0,port=2
+
+Plugging a virtual usb stick into port 4 of the hub just plugged works
+this way:
+
+        -device usb-storage,bus=usb.0,port=2.4,drive=...
+
+You can do basically the same in the monitor using the device_add
+command.  If you want to unplug devices too you should specify some
+unique id which you can use to refer to the device ...
+
+        (qemu) device_add usb-tablet,bus=usb.0,port=1,id=my-tablet
+        (qemu) device_del my-tablet
+
+... when unplugging it with device_del.
+
+
+USB pass through hints
+----------------------
+
+The usb-host driver has a bunch of properties to specify the device
+which should be passed to the guest:
+
+  hostbus=<nr> -- Specifies the bus number the device must be attached
+  to.
+
+  hostaddr=<nr> -- Specifies the device address the device got
+  assigned by the guest os.
+
+  hostport=<str> -- Specifies the physical port the device is attached
+  to.
+
+  vendorid=<hexnr> -- Specifies the vendor ID of the device.
+  productid=<hexnr> -- Specifies the product ID of the device.
+
+In theory you can combine all these properties as you like.  In
+practice only a few combinations are useful:
+
+  (1) vendorid+productid -- match for a specific device, pass it to
+      the guest when it shows up somewhere in the host.
+
+  (2) hostbus+hostport -- match for a specific physical port in the
+      host, any device which is plugged in there gets passed to the
+      guest.
+
+  (3) hostbus+hostaddr -- most useful for ad-hoc pass through as the
+      hostaddr isn't stable, the next time you plug in the device it
+      gets a new one ...
+
+Note that USB 1.1 devices are handled by UHCI/OHCI and USB 2.0 by
+EHCI.  That means a device plugged into the very same physical port
+may show up on different busses depending on the speed.  The port I'm
+using for testing is bus 1 + port 1 for 2.0 devices and bus 3 + port 1
+for 1.1 devices.  Passing through any device plugged into that port
+and also assign them to the correct bus can be done this way:
+
+    qemu -M pc ${otheroptions}                           \
+        -usb                                             \
+        -device usb-ehci,id=ehci                         \
+        -device usb-host,bus=usb.0,hostbus=3,hostport=1  \
+        -device usb-host,bus=ehci.0,hostbus=1,hostport=1
+
 enjoy,
   Gerd
 
-- 
1.7.1

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

* [Qemu-devel] [PATCH 19/31] usb-linux: Get speed from sysfs rather then from the connectinfo ioctl
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (17 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 18/31] usb: documentation update Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 20/31] usb-linux: Teach about super speed Gerd Hoffmann
                   ` (11 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hans de Goede, Gerd Hoffmann

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

The connectinfo ioctl only differentiates between lo speed devices, and
all other speeds, where as we would like to know the real speed. The real
speed is available in sysfs so use that when available.

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

diff --git a/usb-linux.c b/usb-linux.c
index b195e38..32e6ecb 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1057,10 +1057,9 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
 }
 
 static int usb_host_open(USBHostDevice *dev, int bus_num,
-                         int addr, char *port, const char *prod_name)
+                        int addr, char *port, const char *prod_name, int speed)
 {
     int fd = -1, ret;
-    struct usbdevfs_connectinfo ci;
     char buf[1024];
 
     if (dev->fd != -1) {
@@ -1115,24 +1114,29 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
         goto fail;
     }
 
-    ret = ioctl(fd, USBDEVFS_CONNECTINFO, &ci);
-    if (ret < 0) {
-        perror("usb_host_device_open: USBDEVFS_CONNECTINFO");
-        goto fail;
-    }
-
-    printf("husb: grabbed usb device %d.%d\n", bus_num, addr);
-
     ret = usb_linux_update_endp_table(dev);
     if (ret) {
         goto fail;
     }
 
-    if (ci.slow) {
-        dev->dev.speed = USB_SPEED_LOW;
-    } else {
-        dev->dev.speed = USB_SPEED_HIGH;
+    if (speed == -1) {
+        struct usbdevfs_connectinfo ci;
+
+        ret = ioctl(fd, USBDEVFS_CONNECTINFO, &ci);
+        if (ret < 0) {
+            perror("usb_host_device_open: USBDEVFS_CONNECTINFO");
+            goto fail;
+        }
+
+        if (ci.slow) {
+            speed = USB_SPEED_LOW;
+        } else {
+            speed = USB_SPEED_HIGH;
+        }
     }
+    dev->dev.speed = speed;
+
+    printf("husb: grabbed usb device %d.%d\n", bus_num, addr);
 
     if (!prod_name || prod_name[0] == '\0') {
         snprintf(dev->dev.product_desc, sizeof(dev->dev.product_desc),
@@ -1346,7 +1350,8 @@ static int usb_host_scan_dev(void *opaque, USBScanFunc *func)
     }
 
     device_count = 0;
-    bus_num = addr = speed = class_id = product_id = vendor_id = 0;
+    bus_num = addr = class_id = product_id = vendor_id = 0;
+    speed = -1; /* Can't get the speed from /[proc|dev]/bus/usb/devices */
     for(;;) {
         if (fgets(line, sizeof(line), f) == NULL) {
             break;
@@ -1656,7 +1661,7 @@ static int usb_host_auto_scan(void *opaque, int bus_num, int addr, char *port,
         }
         DPRINTF("husb: auto open: bus_num %d addr %d\n", bus_num, addr);
 
-        usb_host_open(s, bus_num, addr, port, product_name);
+        usb_host_open(s, bus_num, addr, port, product_name, speed);
     }
 
     return 0;
-- 
1.7.1

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

* [Qemu-devel] [PATCH 20/31] usb-linux: Teach about super speed
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (18 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 19/31] usb-linux: Get speed from sysfs rather then from the connectinfo ioctl Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 21/31] usb-linux: Don't do perror when errno is not set Gerd Hoffmann
                   ` (10 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hans de Goede, Gerd Hoffmann

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

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

diff --git a/usb-linux.c b/usb-linux.c
index 32e6ecb..03e43c0 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1379,7 +1379,9 @@ static int usb_host_scan_dev(void *opaque, USBScanFunc *func)
             if (get_tag_value(buf, sizeof(buf), line, "Spd=", " ") < 0) {
                 goto fail;
             }
-            if (!strcmp(buf, "480")) {
+            if (!strcmp(buf, "5000")) {
+                speed = USB_SPEED_SUPER;
+            } else if (!strcmp(buf, "480")) {
                 speed = USB_SPEED_HIGH;
             } else if (!strcmp(buf, "1.5")) {
                 speed = USB_SPEED_LOW;
@@ -1523,7 +1525,9 @@ static int usb_host_scan_sys(void *opaque, USBScanFunc *func)
             if (!usb_host_read_file(line, sizeof(line), "speed", de->d_name)) {
                 goto the_end;
             }
-            if (!strcmp(line, "480\n")) {
+            if (!strcmp(line, "5000\n")) {
+                speed = USB_SPEED_SUPER;
+            } else if (!strcmp(line, "480\n")) {
                 speed = USB_SPEED_HIGH;
             } else if (!strcmp(line, "1.5\n")) {
                 speed = USB_SPEED_LOW;
@@ -1800,6 +1804,9 @@ static void usb_info_device(Monitor *mon, int bus_num, int addr, char *port,
     case USB_SPEED_HIGH:
         speed_str = "480";
         break;
+    case USB_SPEED_SUPER:
+        speed_str = "5000";
+        break;
     default:
         speed_str = "?";
         break;
-- 
1.7.1

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

* [Qemu-devel] [PATCH 21/31] usb-linux: Don't do perror when errno is not set
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (19 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 20/31] usb-linux: Teach about super speed Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 22/31] usb-linux: Ensure devep != 0 Gerd Hoffmann
                   ` (9 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hans de Goede, Gerd Hoffmann

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

Note that "op" also is not set, so before this change these error paths
would feed NULL to perror.

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

diff --git a/usb-linux.c b/usb-linux.c
index 03e43c0..4c6c284 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -377,7 +377,8 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
     i = 0;
     dev_descr_len = dev->descr[0];
     if (dev_descr_len > dev->descr_len) {
-        goto fail;
+        fprintf(stderr, "husb: update iface failed. descr too short\n");
+        return 0;
     }
 
     i += dev_descr_len;
@@ -405,7 +406,7 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
     if (i >= dev->descr_len) {
         fprintf(stderr,
                 "husb: update iface failed. no matching configuration\n");
-        goto fail;
+        return 0;
     }
     nb_interfaces = dev->descr[i + 4];
 
-- 
1.7.1

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

* [Qemu-devel] [PATCH 22/31] usb-linux: Ensure devep != 0
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (20 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 21/31] usb-linux: Don't do perror when errno is not set Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 23/31] usb-linux: Don't try to open the same device twice Gerd Hoffmann
                   ` (8 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hans de Goede, Gerd Hoffmann

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

So that we don't index endp_table with a negative index.

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

diff --git a/usb-linux.c b/usb-linux.c
index 4c6c284..82c1e7d 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1030,6 +1030,11 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
             }
 
             devep = descriptors[i + 2];
+            if ((devep & 0x0f) == 0) {
+                fprintf(stderr, "usb-linux: invalid ep descriptor, ep == 0\n");
+                return 1;
+            }
+
             switch (descriptors[i + 3] & 0x3) {
             case 0x00:
                 type = USBDEVFS_URB_TYPE_CONTROL;
-- 
1.7.1

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

* [Qemu-devel] [PATCH 23/31] usb-linux: Don't try to open the same device twice
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (21 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 22/31] usb-linux: Ensure devep != 0 Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 24/31] usb-linux: only cleanup in host_close when host_open was successful Gerd Hoffmann
                   ` (7 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hans de Goede, Gerd Hoffmann

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

If a user wants to redirect 2 identical usb sticks, in theory this is
possible by doing:
usb_add host:1234:5678
usb_add host:1234:5678

But this will lead to us trying to open the first stick twice, since we
don't break the loop after having found a match in our filter list, so the next'
filter list entry will result in us trying to open the same device again.

Fix this by adding the missing break.

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

diff --git a/usb-linux.c b/usb-linux.c
index 82c1e7d..1208a97 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1672,6 +1672,7 @@ static int usb_host_auto_scan(void *opaque, int bus_num, int addr, char *port,
         DPRINTF("husb: auto open: bus_num %d addr %d\n", bus_num, addr);
 
         usb_host_open(s, bus_num, addr, port, product_name, speed);
+        break;
     }
 
     return 0;
-- 
1.7.1

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

* [Qemu-devel] [PATCH 24/31] usb-linux: only cleanup in host_close when host_open was successful.
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (22 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 23/31] usb-linux: Don't try to open the same device twice Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 25/31] usb: don't call usb_host_device_open from vl.c Gerd Hoffmann
                   ` (6 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

---
 usb-linux.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index 1208a97..be667f0 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1159,9 +1159,9 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
     return 0;
 
 fail:
-    dev->fd = -1;
-    if (fd != -1) {
-        close(fd);
+    if (dev->fd != -1) {
+        close(dev->fd);
+        dev->fd = -1;
     }
     return -1;
 }
@@ -1170,7 +1170,7 @@ static int usb_host_close(USBHostDevice *dev)
 {
     int i;
 
-    if (dev->fd == -1) {
+    if (dev->fd == -1 || !dev->dev.attached) {
         return -1;
     }
 
-- 
1.7.1

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

* [Qemu-devel] [PATCH 25/31] usb: don't call usb_host_device_open from vl.c
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (23 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 24/31] usb-linux: only cleanup in host_close when host_open was successful Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 26/31] usb-linux: Enlarge buffer for descriptors to 8192 bytes Gerd Hoffmann
                   ` (5 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Not needed any more, usb-host is qdev-ified these days.
Well, at least the linux version ...

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 vl.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index b362871..b432421 100644
--- a/vl.c
+++ b/vl.c
@@ -923,9 +923,13 @@ static int usb_device_add(const char *devname)
         goto done;
 
     /* the other ones */
+#ifndef CONFIG_LINUX
+    /* only the linux version is qdev-ified, usb-bsd still needs this */
     if (strstart(devname, "host:", &p)) {
         dev = usb_host_device_open(p);
-    } else if (!strcmp(devname, "bt") || strstart(devname, "bt:", &p)) {
+    } else
+#endif
+    if (!strcmp(devname, "bt") || strstart(devname, "bt:", &p)) {
         dev = usb_bt_init(devname[2] ? hci_init(p) :
                         bt_new_hci(qemu_find_bt_vlan(0)));
     } else {
-- 
1.7.1

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

* [Qemu-devel] [PATCH 26/31] usb-linux: Enlarge buffer for descriptors to 8192 bytes
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (24 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 25/31] usb: don't call usb_host_device_open from vl.c Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 27/31] usb-bus: Add knowledge of USB_SPEED_SUPER to usb_speed helper Gerd Hoffmann
                   ` (4 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hans de Goede, Gerd Hoffmann

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

1024 bytes is way to small, one hd UVC webcam I have over here has so
many resolutions its descriptors take op close to 4k. Hopefully 8k will
be enough for all devices.

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

diff --git a/usb-linux.c b/usb-linux.c
index be667f0..600ec2d 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -116,7 +116,7 @@ typedef struct USBHostDevice {
     USBDevice dev;
     int       fd;
 
-    uint8_t   descr[1024];
+    uint8_t   descr[8192];
     int       descr_len;
     int       configuration;
     int       ninterfaces;
-- 
1.7.1

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

* [Qemu-devel] [PATCH 27/31] usb-bus: Add knowledge of USB_SPEED_SUPER to usb_speed helper
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (25 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 26/31] usb-linux: Enlarge buffer for descriptors to 8192 bytes Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 28/31] usb-bus: Don't detach non attached devices on device exit Gerd Hoffmann
                   ` (3 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hans de Goede, Gerd Hoffmann

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

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

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 874c253..91f2083 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -273,6 +273,7 @@ static const char *usb_speed(unsigned int speed)
         [ USB_SPEED_LOW  ] = "1.5",
         [ USB_SPEED_FULL ] = "12",
         [ USB_SPEED_HIGH ] = "480",
+        [ USB_SPEED_SUPER ] = "5000",
     };
     if (speed >= ARRAY_SIZE(txt))
         return "?";
-- 
1.7.1

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

* [Qemu-devel] [PATCH 28/31] usb-bus: Don't detach non attached devices on device exit
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (26 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 27/31] usb-bus: Add knowledge of USB_SPEED_SUPER to usb_speed helper Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 29/31] usb: Add defines for USB Serial Bus Release Number register Gerd Hoffmann
                   ` (2 subsequent siblings)
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hans de Goede, Gerd Hoffmann

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

This causes an "Error: tried to detach unattached usb device " to be printed,
this can happen when deleting ie a usb host qdev, which did not
get attached (because a device matching the filter never got plugged in).

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

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 91f2083..480956d 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -84,7 +84,9 @@ 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);
+    if (dev->attached) {
+        usb_device_detach(dev);
+    }
     bus->ops->device_destroy(bus, dev);
     if (dev->info->handle_destroy) {
         dev->info->handle_destroy(dev);
-- 
1.7.1

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

* [Qemu-devel] [PATCH 29/31] usb: Add defines for USB Serial Bus Release Number register
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (27 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 28/31] usb-bus: Don't detach non attached devices on device exit Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 30/31] usb: Use defines for serial bus release number register for UHCI Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 31/31] usb: Use defines for serial bus release number register for EHCI Gerd Hoffmann
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann, Brad Hards

From: Brad Hards <bradh@frogmouth.net>

Signed-off-by: Brad Hards <bradh@frogmouth.net>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/usb.h b/hw/usb.h
index 6097208..06ce058 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -26,6 +26,12 @@
 #include "qdev.h"
 #include "qemu-queue.h"
 
+/* Constants related to the USB / PCI interaction */
+#define USB_SBRN    0x60 /* Serial Bus Release Number Register */
+#define USB_RELEASE_1  0x10 /* USB 1.0 */
+#define USB_RELEASE_2  0x20 /* USB 2.0 */
+#define USB_RELEASE_3  0x30 /* USB 3.0 */
+
 #define USB_TOKEN_SETUP 0x2d
 #define USB_TOKEN_IN    0x69 /* device -> host */
 #define USB_TOKEN_OUT   0xe1 /* host -> device */
-- 
1.7.1

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

* [Qemu-devel] [PATCH 30/31] usb: Use defines for serial bus release number register for UHCI
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (28 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 29/31] usb: Add defines for USB Serial Bus Release Number register Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 31/31] usb: Use defines for serial bus release number register for EHCI Gerd Hoffmann
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann, Brad Hards

From: Brad Hards <bradh@frogmouth.net>

Signed-off-by: Brad Hards <bradh@frogmouth.net>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-uhci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 8f504d1..1503373 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -1122,7 +1122,7 @@ static int usb_uhci_common_initfn(UHCIState *s)
     pci_config_set_class(pci_conf, PCI_CLASS_SERIAL_USB);
     /* TODO: reset value should be 0. */
     pci_conf[PCI_INTERRUPT_PIN] = 4; // interrupt pin 3
-    pci_conf[0x60] = 0x10; // release number
+    pci_conf[USB_SBRN] = USB_RELEASE_1; // release number
 
     usb_bus_new(&s->bus, &uhci_bus_ops, &s->dev.qdev);
     for(i = 0; i < NB_PORTS; i++) {
-- 
1.7.1

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

* [Qemu-devel] [PATCH 31/31] usb: Use defines for serial bus release number register for EHCI
  2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
                   ` (29 preceding siblings ...)
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 30/31] usb: Use defines for serial bus release number register for UHCI Gerd Hoffmann
@ 2011-06-06 12:39 ` Gerd Hoffmann
  30 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann, Brad Hards

From: Brad Hards <bradh@frogmouth.net>

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

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index b151b48..36ba41e 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -2167,7 +2167,7 @@ static int usb_ehci_initfn(PCIDevice *dev)
 
     // pci_conf[0x50] = 0x01; // power management caps
 
-    pci_set_byte(&pci_conf[0x60], 0x20);  // spec release number (2.1.4)
+    pci_set_byte(&pci_conf[USB_SBRN], USB_RELEASE_2); // release number (2.1.4)
     pci_set_byte(&pci_conf[0x61], 0x20);  // frame length adjustment (2.1.5)
     pci_set_word(&pci_conf[0x62], 0x00);  // port wake up capability (2.1.6)
 
-- 
1.7.1

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

* Re: [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks.
  2011-06-06 12:39 ` [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks Gerd Hoffmann
@ 2011-06-06 13:34   ` David Ahern
  2011-06-06 13:57     ` David Ahern
  0 siblings, 1 reply; 38+ messages in thread
From: David Ahern @ 2011-06-06 13:34 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On 06/06/2011 06:39 AM, Gerd Hoffmann wrote:
> The state machine doesn't stop in EXECUTING state any more when async
> packets are in flight, so the checks are not needed any more and can
> be dropped.
> 
> Also kick out the check for the frame timer.  As we don't stop & sleep
> any more on async packets this is obsolete.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/usb-ehci.c |   32 ++------------------------------
>  1 files changed, 2 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
> index e345a1c..8b1ae3a 100644
> --- a/hw/usb-ehci.c
> +++ b/hw/usb-ehci.c
> @@ -1437,17 +1437,6 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async)
>      int again = 0;
>      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) {
> -        if (async) {
> -            DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state machine\n");
> -            goto out;
> -        } else {
> -            DPRINTF("FETCHENTRY: WARNING "
> -                    "- frame timer elapsed during periodic\n");
> -        }
> -    }
> -#endif
>      if (entry < 0x1000) {
>          DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry);
>          ehci_set_state(ehci, async, EST_ACTIVE);
> @@ -1952,12 +1941,6 @@ static void ehci_advance_async_state(EHCIState *ehci)
>          }
>  
>          ehci_set_state(ehci, async, EST_WAITLISTHEAD);
> -        /* fall through */
> -
> -    case EST_FETCHENTRY:
> -        /* fall through */

Why drop this case too? As I recall that is needed for proper execution.

David

> -
> -    case EST_EXECUTING:
>          ehci_advance_state(ehci, async);
>          break;
>  
> @@ -2010,11 +1993,6 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
>          ehci_advance_state(ehci, async);
>          break;
>  
> -    case EST_EXECUTING:
> -        DPRINTF("PERIODIC state adv for executing\n");
> -        ehci_advance_state(ehci, async);
> -        break;
> -
>      default:
>          /* this should only be due to a developer mistake */
>          fprintf(stderr, "ehci: Bad periodic state %d. "
> @@ -2063,11 +2041,7 @@ static void ehci_frame_timer(void *opaque)
>          if (frames - i > 10) {
>              skipped_frames++;
>          } else {
> -            // TODO could this cause periodic frames to get skipped if async
> -            // active?
> -            if (ehci_get_state(ehci, 1) != EST_EXECUTING) {
> -                ehci_advance_periodic_state(ehci);
> -            }
> +            ehci_advance_periodic_state(ehci);
>          }
>  
>          ehci->last_run_usec += FRAME_TIMER_USEC;
> @@ -2082,9 +2056,7 @@ static void ehci_frame_timer(void *opaque)
>      /*  Async is not inside loop since it executes everything it can once
>       *  called
>       */
> -    if (ehci_get_state(ehci, 0) != EST_EXECUTING) {
> -        ehci_advance_async_state(ehci);
> -    }
> +    ehci_advance_async_state(ehci);
>  
>      qemu_mod_timer(ehci->frame_timer, expire_time);
>  }

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

* Re: [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks.
  2011-06-06 13:34   ` David Ahern
@ 2011-06-06 13:57     ` David Ahern
  2011-06-06 14:25       ` Gerd Hoffmann
  0 siblings, 1 reply; 38+ messages in thread
From: David Ahern @ 2011-06-06 13:57 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel



On 06/06/2011 07:34 AM, David Ahern wrote:
> On 06/06/2011 06:39 AM, Gerd Hoffmann wrote:
>> The state machine doesn't stop in EXECUTING state any more when async
>> packets are in flight, so the checks are not needed any more and can
>> be dropped.
>>
>> Also kick out the check for the frame timer.  As we don't stop & sleep
>> any more on async packets this is obsolete.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>  hw/usb-ehci.c |   32 ++------------------------------
>>  1 files changed, 2 insertions(+), 30 deletions(-)
>>
>> diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
>> index e345a1c..8b1ae3a 100644
>> --- a/hw/usb-ehci.c
>> +++ b/hw/usb-ehci.c
>> @@ -1437,17 +1437,6 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async)
>>      int again = 0;
>>      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) {
>> -        if (async) {
>> -            DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state machine\n");
>> -            goto out;
>> -        } else {
>> -            DPRINTF("FETCHENTRY: WARNING "
>> -                    "- frame timer elapsed during periodic\n");
>> -        }
>> -    }
>> -#endif

This check was added to per Section 4 of the EHCI spec -- the HC should
not start transactions that will not be completed before the end of the
micro-frame. If you remove this what causes the EHCI model to take a
breather?

David


>>      if (entry < 0x1000) {
>>          DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry);
>>          ehci_set_state(ehci, async, EST_ACTIVE);
>> @@ -1952,12 +1941,6 @@ static void ehci_advance_async_state(EHCIState *ehci)
>>          }
>>  
>>          ehci_set_state(ehci, async, EST_WAITLISTHEAD);
>> -        /* fall through */
>> -
>> -    case EST_FETCHENTRY:
>> -        /* fall through */
> 
> Why drop this case too? As I recall that is needed for proper execution.
> 
> David
> 
>> -
>> -    case EST_EXECUTING:
>>          ehci_advance_state(ehci, async);
>>          break;
>>  
>> @@ -2010,11 +1993,6 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
>>          ehci_advance_state(ehci, async);
>>          break;
>>  
>> -    case EST_EXECUTING:
>> -        DPRINTF("PERIODIC state adv for executing\n");
>> -        ehci_advance_state(ehci, async);
>> -        break;
>> -
>>      default:
>>          /* this should only be due to a developer mistake */
>>          fprintf(stderr, "ehci: Bad periodic state %d. "
>> @@ -2063,11 +2041,7 @@ static void ehci_frame_timer(void *opaque)
>>          if (frames - i > 10) {
>>              skipped_frames++;
>>          } else {
>> -            // TODO could this cause periodic frames to get skipped if async
>> -            // active?
>> -            if (ehci_get_state(ehci, 1) != EST_EXECUTING) {
>> -                ehci_advance_periodic_state(ehci);
>> -            }
>> +            ehci_advance_periodic_state(ehci);
>>          }
>>  
>>          ehci->last_run_usec += FRAME_TIMER_USEC;
>> @@ -2082,9 +2056,7 @@ static void ehci_frame_timer(void *opaque)
>>      /*  Async is not inside loop since it executes everything it can once
>>       *  called
>>       */
>> -    if (ehci_get_state(ehci, 0) != EST_EXECUTING) {
>> -        ehci_advance_async_state(ehci);
>> -    }
>> +    ehci_advance_async_state(ehci);
>>  
>>      qemu_mod_timer(ehci->frame_timer, expire_time);
>>  }

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

* Re: [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks.
  2011-06-06 13:57     ` David Ahern
@ 2011-06-06 14:25       ` Gerd Hoffmann
  2011-06-06 14:51         ` David Ahern
  0 siblings, 1 reply; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-06 14:25 UTC (permalink / raw)
  To: David Ahern; +Cc: qemu-devel

   Hi,

>>> -#if EHCI_DEBUG == 0
>>> -    if (qemu_get_clock_ns(vm_clock) / 1000>= ehci->frame_end_usec) {
>>> -        if (async) {
>>> -            DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state machine\n");
>>> -            goto out;
>>> -        } else {
>>> -            DPRINTF("FETCHENTRY: WARNING "
>>> -                    "- frame timer elapsed during periodic\n");
>>> -        }
>>> -    }
>>> -#endif
>
> This check was added to per Section 4 of the EHCI spec -- the HC should
> not start transactions that will not be completed before the end of the
> micro-frame. If you remove this what causes the EHCI model to take a
> breather?

Look at patch #8.  That brings a number of state machine changes.  One 
of them is that the async schedule stops as soon as it notices it walks 
in circles (i.e. sees a QH the second time).

>>> -    case EST_FETCHENTRY:
>>> -        /* fall through */
>>
>> Why drop this case too? As I recall that is needed for proper execution.

The async schedule doesn't pause in fetchentry state any more (also done 
by patch #8).

cheers,
   Gerd

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

* Re: [Qemu-devel] [PATCH 08/31] usb-ehci: multiqueue support
  2011-06-06 12:38 ` [Qemu-devel] [PATCH 08/31] usb-ehci: multiqueue support Gerd Hoffmann
@ 2011-06-06 14:50   ` David Ahern
  2011-06-07  7:30     ` Gerd Hoffmann
  0 siblings, 1 reply; 38+ messages in thread
From: David Ahern @ 2011-06-06 14:50 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On 06/06/2011 06:38 AM, Gerd Hoffmann wrote:
> 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);
> +}

3 bytes or 3 words?  Comment above says skip 3 bytes.

David


> +
>  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 3426e14..51e2e7c 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"

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

* Re: [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks.
  2011-06-06 14:25       ` Gerd Hoffmann
@ 2011-06-06 14:51         ` David Ahern
  0 siblings, 0 replies; 38+ messages in thread
From: David Ahern @ 2011-06-06 14:51 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel



On 06/06/2011 08:25 AM, Gerd Hoffmann wrote:
> 
>   Hi,
> 
>>>> -#if EHCI_DEBUG == 0
>>>> -    if (qemu_get_clock_ns(vm_clock) / 1000>= ehci->frame_end_usec) {
>>>> -        if (async) {
>>>> -            DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state
>>>> machine\n");
>>>> -            goto out;
>>>> -        } else {
>>>> -            DPRINTF("FETCHENTRY: WARNING "
>>>> -                    "- frame timer elapsed during periodic\n");
>>>> -        }
>>>> -    }
>>>> -#endif
>>
>> This check was added to per Section 4 of the EHCI spec -- the HC should
>> not start transactions that will not be completed before the end of the
>> micro-frame. If you remove this what causes the EHCI model to take a
>> breather?
> 
> Look at patch #8.  That brings a number of state machine changes.  One
> of them is that the async schedule stops as soon as it notices it walks
> in circles (i.e. sees a QH the second time).
> 
>>>> -    case EST_FETCHENTRY:
>>>> -        /* fall through */
>>>
>>> Why drop this case too? As I recall that is needed for proper execution.
> 
> The async schedule doesn't pause in fetchentry state any more (also done
> by patch #8).

Ok. see that now.

David


> 
> cheers,
>   Gerd
> 

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

* Re: [Qemu-devel] [PATCH 08/31] usb-ehci: multiqueue support
  2011-06-06 14:50   ` David Ahern
@ 2011-06-07  7:30     ` Gerd Hoffmann
  0 siblings, 0 replies; 38+ messages in thread
From: Gerd Hoffmann @ 2011-06-07  7:30 UTC (permalink / raw)
  To: David Ahern; +Cc: qemu-devel

On 06/06/11 16:50, David Ahern wrote:
>>
>> >  +/*
>> >  + *  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);
>> >  +}
> 3 bytes or 3 words?  Comment above says skip 3 bytes.

3 dwords.  I'll fix the comment.

thanks,
   Gerd

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

end of thread, other threads:[~2011-06-07  7:30 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-06 12:38 [Qemu-devel] [PATCH 00/31] usb patch queue Gerd Hoffmann
2011-06-06 12:38 ` [Qemu-devel] [PATCH 01/31] usb-linux: catch ENODEV in more places Gerd Hoffmann
2011-06-06 12:38 ` [Qemu-devel] [PATCH 02/31] usb-ehci: trace mmio and usbsts Gerd Hoffmann
2011-06-06 12:38 ` [Qemu-devel] [PATCH 03/31] usb-ehci: trace state machine changes Gerd Hoffmann
2011-06-06 12:38 ` [Qemu-devel] [PATCH 04/31] usb-ehci: trace port state Gerd Hoffmann
2011-06-06 12:38 ` [Qemu-devel] [PATCH 05/31] usb-ehci: improve mmio tracing Gerd Hoffmann
2011-06-06 12:38 ` [Qemu-devel] [PATCH 06/31] usb-ehci: trace buffer copy Gerd Hoffmann
2011-06-06 12:38 ` [Qemu-devel] [PATCH 07/31] usb-ehci: add queue data struct Gerd Hoffmann
2011-06-06 12:38 ` [Qemu-devel] [PATCH 08/31] usb-ehci: multiqueue support Gerd Hoffmann
2011-06-06 14:50   ` David Ahern
2011-06-07  7:30     ` Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 09/31] usb-ehci: fix offset writeback in ehci_buffer_rw Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 10/31] usb-ehci: fix error handling Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 11/31] ehci: fix a number of unused-but-set-variable warnings (new with gcc-4.6) Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 12/31] usb: cancel async packets on unplug Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks Gerd Hoffmann
2011-06-06 13:34   ` David Ahern
2011-06-06 13:57     ` David Ahern
2011-06-06 14:25       ` Gerd Hoffmann
2011-06-06 14:51         ` David Ahern
2011-06-06 12:39 ` [Qemu-devel] [PATCH 14/31] Fix USB mouse Set_Protocol behavior Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 15/31] The USB tablet should not claim boot protocol support Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 16/31] usb-ehci: itd handling fixes Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 17/31] usb-ehci: split trace calls to handle arg count limits Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 18/31] usb: documentation update Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 19/31] usb-linux: Get speed from sysfs rather then from the connectinfo ioctl Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 20/31] usb-linux: Teach about super speed Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 21/31] usb-linux: Don't do perror when errno is not set Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 22/31] usb-linux: Ensure devep != 0 Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 23/31] usb-linux: Don't try to open the same device twice Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 24/31] usb-linux: only cleanup in host_close when host_open was successful Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 25/31] usb: don't call usb_host_device_open from vl.c Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 26/31] usb-linux: Enlarge buffer for descriptors to 8192 bytes Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 27/31] usb-bus: Add knowledge of USB_SPEED_SUPER to usb_speed helper Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 28/31] usb-bus: Don't detach non attached devices on device exit Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 29/31] usb: Add defines for USB Serial Bus Release Number register Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 30/31] usb: Use defines for serial bus release number register for UHCI Gerd Hoffmann
2011-06-06 12:39 ` [Qemu-devel] [PATCH 31/31] usb: Use defines for serial bus release number register for EHCI Gerd Hoffmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).