[Qemu-devel] [PULL 0/9] usb patch queue

[Qemu-devel] [PULL 0/9] usb patch queue

[Gerd Hoffmann]

  Hi,

Here comes the most recent usb patch queue, featuring a collection of
little bug fixes all over the place.  See individual patches for
details.

please pull,
  Gerd

The following changes since commit 84988cf910a6881f2180fdcec516b60f8f0dc8c4:

  bitops.h: Add functions to extract and deposit bitfields (2012-07-07 09:07:01 +0000)

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

Gerd Hoffmann (6):
      ehci: fix ehci_qh_do_overlay
      ehci: fix td writeback
      ehci: don't flush cache on doorbell rings.
      usb: split endpoint init and reset
      usb: fix interface initialization
      usb-host: add trace events for iso xfers

Hans de Goede (3):
      usb-ehci: Fix an assert whenever isoc transfers are used
      ehci: Kick async schedule on wakeup in the non companion case
      usb-redir: Correctly handle the usb_redir_babble usbredir status

 hw/usb.h            |    3 ++
 hw/usb/core.c       |   17 ++++++++--
 hw/usb/hcd-ehci.c   |   84 +++++++++++++++++++++++++++++++++-----------------
 hw/usb/host-linux.c |   15 +++++----
 hw/usb/redirect.c   |    2 +
 trace-events        |    6 ++-
 6 files changed, 86 insertions(+), 41 deletions(-)

[Qemu-devel] [PATCH 1/9] ehci: fix ehci_qh_do_overlay

[Gerd Hoffmann]

Use ehci_flush_qh to make sure we touch inly the fields the hc is
allowed to touch.

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

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 1582c2c..7de47e5 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1246,6 +1246,23 @@ static inline int put_dwords(EHCIState *ehci, uint32_t addr,
     return 1;
 }
 
+/*
+ *  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 dwords 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(q->ehci, addr + 3 * sizeof(uint32_t), qh + 3, dwords - 3);
+}
+
 // 4.10.2
 
 static int ehci_qh_do_overlay(EHCIQueue *q)
@@ -1293,8 +1310,7 @@ static int ehci_qh_do_overlay(EHCIQueue *q)
     q->qh.bufptr[1] &= ~BUFPTR_CPROGMASK_MASK;
     q->qh.bufptr[2] &= ~BUFPTR_FRAMETAG_MASK;
 
-    put_dwords(q->ehci, NLPTR_GET(q->qhaddr), (uint32_t *) &q->qh,
-               sizeof(EHCIqh) >> 2);
+    ehci_flush_qh(q);
 
     return 0;
 }
@@ -1600,23 +1616,6 @@ static int ehci_process_itd(EHCIState *ehci,
 }
 
 
-/*
- *  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 dwords 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(q->ehci, addr + 3 * sizeof(uint32_t), qh + 3, dwords - 3);
-}
-
 /*  This state is the entry point for asynchronous schedule
  *  processing.  Entry here consitutes a EHCI start event state (4.8.5)
  */
-- 
1.7.1

[Qemu-devel] [PATCH] usb: split endpoint init and reset

[Gerd Hoffmann]

Create a new usb_ep_reset() function to reset endpoint state, without
re-initialiting the queues, so we don't unlink in-flight packets just
because usb-host has to re-parse the descriptor tables.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb.h            |    1 +
 hw/usb/core.c       |   13 +++++++++++--
 hw/usb/host-linux.c |    5 +++--
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/usb.h b/hw/usb.h
index a5623d3..9cd2f89 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -363,6 +363,7 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p);
 void usb_cancel_packet(USBPacket * p);
 
 void usb_ep_init(USBDevice *dev);
+void usb_ep_reset(USBDevice *dev);
 void usb_ep_dump(USBDevice *dev);
 struct USBEndpoint *usb_ep_get(USBDevice *dev, int pid, int ep);
 uint8_t usb_ep_get_type(USBDevice *dev, int pid, int ep);
diff --git a/hw/usb/core.c b/hw/usb/core.c
index 0e02da7..fe15be0 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -550,7 +550,7 @@ void usb_packet_cleanup(USBPacket *p)
     qemu_iovec_destroy(&p->iov);
 }
 
-void usb_ep_init(USBDevice *dev)
+void usb_ep_reset(USBDevice *dev)
 {
     int ep;
 
@@ -559,7 +559,6 @@ void usb_ep_init(USBDevice *dev)
     dev->ep_ctl.ifnum = 0;
     dev->ep_ctl.dev = dev;
     dev->ep_ctl.pipeline = false;
-    QTAILQ_INIT(&dev->ep_ctl.queue);
     for (ep = 0; ep < USB_MAX_ENDPOINTS; ep++) {
         dev->ep_in[ep].nr = ep + 1;
         dev->ep_out[ep].nr = ep + 1;
@@ -573,6 +572,16 @@ void usb_ep_init(USBDevice *dev)
         dev->ep_out[ep].dev = dev;
         dev->ep_in[ep].pipeline = false;
         dev->ep_out[ep].pipeline = false;
+    }
+}
+
+void usb_ep_init(USBDevice *dev)
+{
+    int ep;
+
+    usb_ep_reset(dev);
+    QTAILQ_INIT(&dev->ep_ctl.queue);
+    for (ep = 0; ep < USB_MAX_ENDPOINTS; ep++) {
         QTAILQ_INIT(&dev->ep_in[ep].queue);
         QTAILQ_INIT(&dev->ep_out[ep].queue);
     }
diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index 5479fb5..9ba8925 100644
--- a/hw/usb/host-linux.c
+++ b/hw/usb/host-linux.c
@@ -1136,7 +1136,7 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
     USBDescriptor *d;
     bool active = false;
 
-    usb_ep_init(&s->dev);
+    usb_ep_reset(&s->dev);
 
     for (i = 0;; i += d->bLength) {
         if (i+2 >= s->descr_len) {
@@ -1239,7 +1239,7 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
     return 0;
 
 error:
-    usb_ep_init(&s->dev);
+    usb_ep_reset(&s->dev);
     return 1;
 }
 
@@ -1326,6 +1326,7 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
         goto fail;
     }
 
+    usb_ep_init(&dev->dev);
     ret = usb_linux_update_endp_table(dev);
     if (ret) {
         goto fail;
-- 
1.7.1

[Qemu-devel] [PATCH 2/9] ehci: fix td writeback

[Gerd Hoffmann]

Only write back the dwords the hc is supposed to update.  Should not
make a difference in theory as the guest must not touch the td while
it is active to avoid races.  But it is still more correct.

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

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 7de47e5..1406b84 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2070,6 +2070,7 @@ out:
 static int ehci_state_writeback(EHCIQueue *q)
 {
     EHCIPacket *p = QTAILQ_FIRST(&q->packets);
+    uint32_t *qtd, addr;
     int again = 0;
 
     /*  Write back the QTD from the QH area */
@@ -2077,8 +2078,9 @@ static int ehci_state_writeback(EHCIQueue *q)
     assert(p->qtdaddr == q->qtdaddr);
 
     ehci_trace_qtd(q, NLPTR_GET(p->qtdaddr), (EHCIqtd *) &q->qh.next_qtd);
-    put_dwords(q->ehci, NLPTR_GET(p->qtdaddr), (uint32_t *) &q->qh.next_qtd,
-               sizeof(EHCIqtd) >> 2);
+    qtd = (uint32_t *) &q->qh.next_qtd;
+    addr = NLPTR_GET(p->qtdaddr);
+    put_dwords(q->ehci, addr + 2 * sizeof(uint32_t), qtd + 2, 2);
     ehci_free_packet(p);
 
     /*
-- 
1.7.1

[Qemu-devel] [PATCH 3/9] ehci: don't flush cache on doorbell rings.

[Gerd Hoffmann]

Commit 4be23939ab0d7019c7e59a37485b416fbbf0f073 makes ehci instantly
zap any unlinked queue heads when the guest rings the doorbell.

While hacking up uas support this turned out to be a problem.  The linux
kernel can unlink and instantly relink the very same queue head, thereby
killing any async packets in flight.  That alone isn't an issue yet, the
packet will canceled and resubmitted and everything is fine.  We'll run
into trouble though in case the async packet is completed already, so we
can't cancel it any more.  The transaction is simply lost then.

usb_ehci_qh_ptrs q (nil) - QH @ 39c4f000: next 39c4f122 qtds 00000000,00000001,39c50000
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95feba90a0 - QH @ 39c4f000: next 39c4f122 qtds 00000000,00000001,39c50000
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95fe515210 - QH @ 39c4f120: next 39c4f0c2 qtds 29dbce40,29dbc4e0,00000009
usb_ehci_qh_fields QH @ 39c4f120 - rl 4, mplen 512, eps 2, ep 1, dev 2
usb_ehci_packet_action q 0x7f95fe515210 p 0x7f95fdec32a0: alloc
usb_packet_state_change bus 0, port 2, ep 1, packet 0x7f95fdec32e0, state undef -> setup
usb_ehci_packet_action q 0x7f95fe515210 p 0x7f95fdec32a0: process
usb_uas_command dev 2, tag 0x2, lun 0, lun64 00000000-00000000
scsi_req_parsed target 0 lun 0 tag 2 command 42 dir 2 length 16384
scsi_req_parsed_lba target 0 lun 0 tag 2 command 42 lba 5933312
scsi_req_alloc target 0 lun 0 tag 2
scsi_req_continue target 0 lun 0 tag 2
scsi_req_data target 0 lun 0 tag 2 len 16384
usb_uas_scsi_data dev 2, tag 0x2, bytes 16384
usb_uas_write_ready dev 2, tag 0x2
usb_packet_state_change bus 0, port 2, ep 1, packet 0x7f95fdec32e0, state setup -> complete
usb_ehci_packet_action q 0x7f95fe515210 p 0x7f95fdec32a0: free
usb_ehci_qh_ptrs q 0x7f95fdec3210 - QH @ 39c4f0c0: next 39c4f002 qtds 29dbce40,00000001,00000009
usb_ehci_qh_fields QH @ 39c4f0c0 - rl 4, mplen 512, eps 2, ep 2, dev 2
usb_ehci_queue_action q 0x7f95fe5152a0: free
usb_packet_state_change bus 0, port 2, ep 2, packet 0x7f95feba9170, state async -> complete
^^^ async packets completes.
usb_ehci_packet_action q 0x7f95fdec3210 p 0x7f95feba9130: wakeup

usb_ehci_qh_ptrs q (nil) - QH @ 39c4f000: next 39c4f122 qtds 00000000,00000001,39c50000
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95feba90a0 - QH @ 39c4f000: next 39c4f122 qtds 00000000,00000001,39c50000
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95fe515210 - QH @ 39c4f120: next 39c4f002 qtds 29dbc4e0,29dbc8a0,00000009
usb_ehci_qh_fields QH @ 39c4f120 - rl 4, mplen 512, eps 2, ep 1, dev 2
usb_ehci_queue_action q 0x7f95fdec3210: free
usb_ehci_packet_action q 0x7f95fdec3210 p 0x7f95feba9130: free
^^^ endpoint #2 queue head removed from schedule, doorbell makes ehci zap the queue,
    the (completed) usb packet is freed too and gets lost.

usb_ehci_qh_ptrs q (nil) - QH @ 39c4f000: next 39c4f0c2 qtds 00000000,00000001,39c50000
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95feba90a0 - QH @ 39c4f000: next 39c4f0c2 qtds 00000000,00000001,39c50000
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_queue_action q 0x7f9600dff570: alloc
usb_ehci_qh_ptrs q 0x7f9600dff570 - QH @ 39c4f0c0: next 39c4f122 qtds 29dbce40,00000001,00000009
usb_ehci_qh_fields QH @ 39c4f0c0 - rl 4, mplen 512, eps 2, ep 2, dev 2
usb_ehci_packet_action q 0x7f9600dff570 p 0x7f95feba9130: alloc
usb_packet_state_change bus 0, port 2, ep 2, packet 0x7f95feba9170, state undef -> setup
usb_ehci_packet_action q 0x7f9600dff570 p 0x7f95feba9130: process
usb_packet_state_change bus 0, port 2, ep 2, packet 0x7f95feba9170, state setup -> async
usb_ehci_packet_action q 0x7f9600dff570 p 0x7f95feba9130: async
^^^ linux kernel relinked the queue head, ehci creates a new usb packet,
    but we should have delivered the completed one instead.
usb_ehci_qh_ptrs q 0x7f95fe515210 - QH @ 39c4f120: next 39c4f002 qtds 29dbc4e0,29dbc8a0,00000009
usb_ehci_qh_fields QH @ 39c4f120 - rl 4, mplen 512, eps 2, ep 1, dev 2

So instead of instantly zapping the queue we'll set a flag that the
queue needs revalidation in case we'll see it again in the schedule.
ehci then checks that the queue head fields addressing / describing the
endpoint and the qtd pointer match the cached content before reusing it.

Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ehci.c |   35 +++++++++++++++++++++++++++++------
 1 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 1406b84..0bdfbe8 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -365,6 +365,7 @@ struct EHCIQueue {
     uint32_t seen;
     uint64_t ts;
     int async;
+    int revalidate;
 
     /* cached data from guest - needs to be flushed
      * when guest removes an entry (doorbell, handshake sequence)
@@ -775,7 +776,18 @@ static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr,
     return NULL;
 }
 
-static void ehci_queues_rip_unused(EHCIState *ehci, int async, int flush)
+static void ehci_queues_tag_unused_async(EHCIState *ehci)
+{
+    EHCIQueue *q;
+
+    QTAILQ_FOREACH(q, &ehci->aqueues, next) {
+        if (!q->seen) {
+            q->revalidate = 1;
+        }
+    }
+}
+
+static void ehci_queues_rip_unused(EHCIState *ehci, int async)
 {
     EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
     uint64_t maxage = FRAME_TIMER_NS * ehci->maxframes * 4;
@@ -787,7 +799,7 @@ static void ehci_queues_rip_unused(EHCIState *ehci, int async, int flush)
             q->ts = ehci->last_run_ns;
             continue;
         }
-        if (!flush && ehci->last_run_ns < q->ts + maxage) {
+        if (ehci->last_run_ns < q->ts + maxage) {
             continue;
         }
         ehci_free_queue(q);
@@ -1631,7 +1643,7 @@ static int ehci_state_waitlisthead(EHCIState *ehci,  int async)
         ehci_set_usbsts(ehci, USBSTS_REC);
     }
 
-    ehci_queues_rip_unused(ehci, async, 0);
+    ehci_queues_rip_unused(ehci, async);
 
     /*  Find the head of the list (4.9.1.1) */
     for(i = 0; i < MAX_QH; i++) {
@@ -1716,6 +1728,7 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
     EHCIPacket *p;
     uint32_t entry, devaddr;
     EHCIQueue *q;
+    EHCIqh qh;
 
     entry = ehci_get_fetch_addr(ehci, async);
     q = ehci_find_queue_by_qh(ehci, entry, async);
@@ -1733,7 +1746,17 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
     }
 
     get_dwords(ehci, NLPTR_GET(q->qhaddr),
-               (uint32_t *) &q->qh, sizeof(EHCIqh) >> 2);
+               (uint32_t *) &qh, sizeof(EHCIqh) >> 2);
+    if (q->revalidate && (q->qh.epchar      != qh.epchar ||
+                          q->qh.epcap       != qh.epcap  ||
+                          q->qh.current_qtd != qh.current_qtd)) {
+        ehci_free_queue(q);
+        q = ehci_alloc_queue(ehci, entry, async);
+        q->seen++;
+        p = NULL;
+    }
+    q->qh = qh;
+    q->revalidate = 0;
     ehci_trace_qh(q, NLPTR_GET(q->qhaddr), &q->qh);
 
     devaddr = get_field(q->qh.epchar, QH_EPCHAR_DEVADDR);
@@ -2228,7 +2251,7 @@ static void ehci_advance_async_state(EHCIState *ehci)
          */
         if (ehci->usbcmd & USBCMD_IAAD) {
             /* Remove all unseen qhs from the async qhs queue */
-            ehci_queues_rip_unused(ehci, async, 1);
+            ehci_queues_tag_unused_async(ehci);
             DPRINTF("ASYNC: doorbell request acknowledged\n");
             ehci->usbcmd &= ~USBCMD_IAAD;
             ehci_set_interrupt(ehci, USBSTS_IAA);
@@ -2281,7 +2304,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
         ehci_set_fetch_addr(ehci, async,entry);
         ehci_set_state(ehci, async, EST_FETCHENTRY);
         ehci_advance_state(ehci, async);
-        ehci_queues_rip_unused(ehci, async, 0);
+        ehci_queues_rip_unused(ehci, async);
         break;
 
     default:
-- 
1.7.1

[Qemu-devel] [PATCH 4/9] usb-ehci: Fix an assert whenever isoc transfers are used

[Gerd Hoffmann]

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

hcd-ehci.c is missing an usb_packet_init() call for the ipacket UsbPacket
it uses for isoc transfers, triggering an assert (taking the entire vm down)
in usb_packet_setup as soon as any isoc transfers are done by a high speed
USB device.

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

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 0bdfbe8..f612610 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2581,6 +2581,7 @@ static int usb_ehci_initfn(PCIDevice *dev)
     s->async_bh = qemu_bh_new(ehci_async_bh, s);
     QTAILQ_INIT(&s->aqueues);
     QTAILQ_INIT(&s->pqueues);
+    usb_packet_init(&s->ipacket);
 
     qemu_register_reset(ehci_reset, s);
 
-- 
1.7.1

[Qemu-devel] [PATCH 5/9] ehci: Kick async schedule on wakeup in the non companion case

[Gerd Hoffmann]

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

Commit 0f588df8b3688b00e77aabaa32e26ece5f19bd39, added code
to ehci_wakeup to kick the async schedule on wakeup, but the else
was positioned wrong making it trigger for devices which are routed
to the companion rather then to the ehci controller itself.

This patch fixes this. Note that the "programming style" with using the
return at the end of the companion block matches how the companion case
is handled in the other ports ops, and is done this way for consistency.

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

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index f612610..080f62c 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -905,10 +905,11 @@ static void ehci_wakeup(USBPort *port)
         USBPort *companion = s->companion_ports[port->index];
         if (companion->ops->wakeup) {
             companion->ops->wakeup(companion);
-        } else {
-            qemu_bh_schedule(s->async_bh);
         }
+        return;
     }
+
+    qemu_bh_schedule(s->async_bh);
 }
 
 static int ehci_register_companion(USBBus *bus, USBPort *ports[],
-- 
1.7.1

[Qemu-devel] [PATCH 6/9] usb-redir: Correctly handle the usb_redir_babble usbredir status

[Gerd Hoffmann]

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

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

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index d949f04..10b4fbb 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1033,6 +1033,8 @@ static int usbredir_handle_status(USBRedirDevice *dev,
     case usb_redir_inval:
         WARNING("got invalid param error from usb-host?\n");
         return USB_RET_NAK;
+    case usb_redir_babble:
+        return USB_RET_BABBLE;
     case usb_redir_ioerror:
     case usb_redir_timeout:
     default:
-- 
1.7.1

[Qemu-devel] [PATCH 7/9] usb: split endpoint init and reset

[Gerd Hoffmann]

Create a new usb_ep_reset() function to reset endpoint state, without
re-initialiting the queues, so we don't unlink in-flight packets just
because usb-host has to re-parse the descriptor tables.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb.h            |    1 +
 hw/usb/core.c       |   13 +++++++++++--
 hw/usb/host-linux.c |    5 +++--
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/usb.h b/hw/usb.h
index a5623d3..9cd2f89 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -363,6 +363,7 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p);
 void usb_cancel_packet(USBPacket * p);
 
 void usb_ep_init(USBDevice *dev);
+void usb_ep_reset(USBDevice *dev);
 void usb_ep_dump(USBDevice *dev);
 struct USBEndpoint *usb_ep_get(USBDevice *dev, int pid, int ep);
 uint8_t usb_ep_get_type(USBDevice *dev, int pid, int ep);
diff --git a/hw/usb/core.c b/hw/usb/core.c
index 0e02da7..fe15be0 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -550,7 +550,7 @@ void usb_packet_cleanup(USBPacket *p)
     qemu_iovec_destroy(&p->iov);
 }
 
-void usb_ep_init(USBDevice *dev)
+void usb_ep_reset(USBDevice *dev)
 {
     int ep;
 
@@ -559,7 +559,6 @@ void usb_ep_init(USBDevice *dev)
     dev->ep_ctl.ifnum = 0;
     dev->ep_ctl.dev = dev;
     dev->ep_ctl.pipeline = false;
-    QTAILQ_INIT(&dev->ep_ctl.queue);
     for (ep = 0; ep < USB_MAX_ENDPOINTS; ep++) {
         dev->ep_in[ep].nr = ep + 1;
         dev->ep_out[ep].nr = ep + 1;
@@ -573,6 +572,16 @@ void usb_ep_init(USBDevice *dev)
         dev->ep_out[ep].dev = dev;
         dev->ep_in[ep].pipeline = false;
         dev->ep_out[ep].pipeline = false;
+    }
+}
+
+void usb_ep_init(USBDevice *dev)
+{
+    int ep;
+
+    usb_ep_reset(dev);
+    QTAILQ_INIT(&dev->ep_ctl.queue);
+    for (ep = 0; ep < USB_MAX_ENDPOINTS; ep++) {
         QTAILQ_INIT(&dev->ep_in[ep].queue);
         QTAILQ_INIT(&dev->ep_out[ep].queue);
     }
diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index 5479fb5..9ba8925 100644
--- a/hw/usb/host-linux.c
+++ b/hw/usb/host-linux.c
@@ -1136,7 +1136,7 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
     USBDescriptor *d;
     bool active = false;
 
-    usb_ep_init(&s->dev);
+    usb_ep_reset(&s->dev);
 
     for (i = 0;; i += d->bLength) {
         if (i+2 >= s->descr_len) {
@@ -1239,7 +1239,7 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
     return 0;
 
 error:
-    usb_ep_init(&s->dev);
+    usb_ep_reset(&s->dev);
     return 1;
 }
 
@@ -1326,6 +1326,7 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
         goto fail;
     }
 
+    usb_ep_init(&dev->dev);
     ret = usb_linux_update_endp_table(dev);
     if (ret) {
         goto fail;
-- 
1.7.1

[Qemu-devel] [PATCH 8/9] usb: fix interface initialization

[Gerd Hoffmann]

zero is a valid interface number, so don't use it when resetting the
endpoints.

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

diff --git a/hw/usb.h b/hw/usb.h
index 9cd2f89..7ed8fb8 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -145,6 +145,8 @@
 #define USB_ENDPOINT_XFER_INT		3
 #define USB_ENDPOINT_XFER_INVALID     255
 
+#define USB_INTERFACE_INVALID         255
+
 typedef struct USBBus USBBus;
 typedef struct USBBusOps USBBusOps;
 typedef struct USBPort USBPort;
diff --git a/hw/usb/core.c b/hw/usb/core.c
index fe15be0..0614f76 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -566,8 +566,8 @@ void usb_ep_reset(USBDevice *dev)
         dev->ep_out[ep].pid = USB_TOKEN_OUT;
         dev->ep_in[ep].type = USB_ENDPOINT_XFER_INVALID;
         dev->ep_out[ep].type = USB_ENDPOINT_XFER_INVALID;
-        dev->ep_in[ep].ifnum = 0;
-        dev->ep_out[ep].ifnum = 0;
+        dev->ep_in[ep].ifnum = USB_INTERFACE_INVALID;
+        dev->ep_out[ep].ifnum = USB_INTERFACE_INVALID;
         dev->ep_in[ep].dev = dev;
         dev->ep_out[ep].dev = dev;
         dev->ep_in[ep].pipeline = false;
-- 
1.7.1

[Qemu-devel] [PATCH 9/9] usb-host: add trace events for iso xfers

[Gerd Hoffmann]

Replace iso transfer fprintf's with trace points.  Also rename existing
tracepoints so they all match usb_host_iso_*.

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

diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index 9ba8925..d55be87 100644
--- a/hw/usb/host-linux.c
+++ b/hw/usb/host-linux.c
@@ -213,7 +213,7 @@ static int is_iso_started(USBHostDevice *s, int pid, int ep)
 
 static void clear_iso_started(USBHostDevice *s, int pid, int ep)
 {
-    trace_usb_host_ep_stop_iso(s->bus_num, s->addr, ep);
+    trace_usb_host_iso_stop(s->bus_num, s->addr, ep);
     get_endp(s, pid, ep)->iso_started = 0;
 }
 
@@ -221,7 +221,7 @@ static void set_iso_started(USBHostDevice *s, int pid, int ep)
 {
     struct endp_data *e = get_endp(s, pid, ep);
 
-    trace_usb_host_ep_start_iso(s->bus_num, s->addr, ep);
+    trace_usb_host_iso_start(s->bus_num, s->addr, ep);
     if (!e->iso_started) {
         e->iso_started = 1;
         e->inflight = 0;
@@ -319,7 +319,8 @@ static void async_complete(void *opaque)
         if (r < 0) {
             if (errno == EAGAIN) {
                 if (urbs > 2) {
-                    fprintf(stderr, "husb: %d iso urbs finished at once\n", urbs);
+                    /* indicates possible latency issues */
+                    trace_usb_host_iso_many_urbs(s->bus_num, s->addr, urbs);
                 }
                 return;
             }
@@ -352,7 +353,8 @@ static void async_complete(void *opaque)
             urbs++;
             inflight = change_iso_inflight(s, pid, ep, -1);
             if (inflight == 0 && is_iso_started(s, pid, ep)) {
-                fprintf(stderr, "husb: out of buffers for iso stream\n");
+                /* can be latency issues, or simply end of stream */
+                trace_usb_host_iso_out_of_bufs(s->bus_num, s->addr, ep);
             }
             continue;
         }
diff --git a/trace-events b/trace-events
index c935ba2..c33d58c 100644
--- a/trace-events
+++ b/trace-events
@@ -368,8 +368,10 @@ usb_host_urb_complete(int bus, int addr, void *aurb, int status, int length, int
 usb_host_urb_canceled(int bus, int addr, void *aurb) "dev %d:%d, aurb %p"
 usb_host_ep_set_halt(int bus, int addr, int ep) "dev %d:%d, ep %d"
 usb_host_ep_clear_halt(int bus, int addr, int ep) "dev %d:%d, ep %d"
-usb_host_ep_start_iso(int bus, int addr, int ep) "dev %d:%d, ep %d"
-usb_host_ep_stop_iso(int bus, int addr, int ep) "dev %d:%d, ep %d"
+usb_host_iso_start(int bus, int addr, int ep) "dev %d:%d, ep %d"
+usb_host_iso_stop(int bus, int addr, int ep) "dev %d:%d, ep %d"
+usb_host_iso_out_of_bufs(int bus, int addr, int ep) "dev %d:%d, ep %d"
+usb_host_iso_many_urbs(int bus, int addr, int count) "dev %d:%d, count %d"
 usb_host_reset(int bus, int addr) "dev %d:%d"
 usb_host_auto_scan_enabled(void)
 usb_host_auto_scan_disabled(void)
-- 
1.7.1

Re: [Qemu-devel] [PULL 0/9] usb patch queue

[Anthony Liguori]

On 07/09/2012 05:20 AM, Gerd Hoffmann wrote:
>    Hi,
>
> Here comes the most recent usb patch queue, featuring a collection of
> little bug fixes all over the place.  See individual patches for
> details.

Pulled.  Thanks.

Regards,

Anthony Liguori

> please pull,
>    Gerd
>
> The following changes since commit 84988cf910a6881f2180fdcec516b60f8f0dc8c4:
>
>    bitops.h: Add functions to extract and deposit bitfields (2012-07-07 09:07:01 +0000)
>
> are available in the git repository at:
>    git://git.kraxel.org/qemu usb.55
>
> Gerd Hoffmann (6):
>        ehci: fix ehci_qh_do_overlay
>        ehci: fix td writeback
>        ehci: don't flush cache on doorbell rings.
>        usb: split endpoint init and reset
>        usb: fix interface initialization
>        usb-host: add trace events for iso xfers
>
> Hans de Goede (3):
>        usb-ehci: Fix an assert whenever isoc transfers are used
>        ehci: Kick async schedule on wakeup in the non companion case
>        usb-redir: Correctly handle the usb_redir_babble usbredir status
>
>   hw/usb.h            |    3 ++
>   hw/usb/core.c       |   17 ++++++++--
>   hw/usb/hcd-ehci.c   |   84 +++++++++++++++++++++++++++++++++-----------------
>   hw/usb/host-linux.c |   15 +++++----
>   hw/usb/redirect.c   |    2 +
>   trace-events        |    6 ++-
>   6 files changed, 86 insertions(+), 41 deletions(-)
>
>