[Qemu-devel] [PULL 00/36] usb patch queue

[Qemu-devel] [PULL 00/36] usb patch queue

[Gerd Hoffmann]

  Hi,

Here comes the usb patch queue.  Nothing big standing out.  Tons of
cleanups and small bug fixes.  Some performance improvements too.
Some patches preparing the usb core for the upcoming input pipelining
bits.

please pull,
  Gerd

The following changes since commit a8170e5e97ad17ca169c64ba87ae2f53850dab4c:

  Rename target_phys_addr_t to hwaddr (2012-10-23 08:58:25 -0500)

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

Gerd Hoffmann (5):
      xhci: fix function name in error message
      xhci: flush endpoint context unconditinally
      xhci: allow disabling interrupters
      xhci: make number of interrupters and slots configurable
      xhci: fix usb name in caps

Hans de Goede (31):
      uhci: Properly unmap packets on cancel / invalid pid
      uhci: Move checks to continue queuing to uhci_fill_queue()
      ehci: Get rid of packet tbytes field
      ehci: Set int flag on a short input packet
      ehci: Improve latency of interrupt delivery and async schedule scanning
      ehci: Speed up the timer of raising int from the async schedule
      ehci: Detect going in circles when filling the queue
      ehci: Retry to fill the queue while waiting for td completion
      xhci: Add a xhci_ep_nuke_one_xfer helper function
      usb: Rename __usb_packet_complete to usb_packet_complete_one
      usb: Add USB_RET_ADD_TO_QUEUE packet result code
      usb: Move clearing of queue on halt to the core
      usb: Move short-not-ok handling to the core
      usb: Add an int_req flag to USBPacket
      usb: Enforce iso endpoints never returing USB_RET_ASYNC
      uhci: No need to handle async completion of isoc packets
      uhci: cleanup: Add an unlink call to uhci_async_cancel()
      uhci: Don't retry on error
      uhci: Drop unnecessary forward declaration of some static functions
      uhci: Move emptying of the queue's asyncs' queue to uhci_queue_free
      uhci: Rename UHCIAsync->td to UHCIAsync->td_addr
      uhci: Add uhci_read_td() helper function
      uhci: Make uhci_fill_queue() actually operate on an UHCIQueue
      uhci: Store ep in UHCIQueue
      uhci: Immediately free queues on device disconnect
      uhci: Verify queue has not been changed by guest
      uhci: Detect guest td re-use
      uhci: When the guest marks a pending td non-active, cancel the queue
      uhci: Always mark a queue valid when we encounter it
      uhci: Retry to fill the queue while waiting for td completion
      uhci: Use only one queue for ctrl endpoints

 hw/usb.h            |   28 +++-
 hw/usb/bus.c        |    8 +
 hw/usb/core.c       |   28 +++-
 hw/usb/hcd-ehci.c   |  105 ++++++++------
 hw/usb/hcd-musb.c   |    3 +-
 hw/usb/hcd-ohci.c   |    9 +-
 hw/usb/hcd-uhci.c   |  380 +++++++++++++++++++++++++--------------------------
 hw/usb/hcd-xhci.c   |  169 +++++++++++++++--------
 hw/usb/host-linux.c |    3 +-
 hw/usb/redirect.c   |   18 ++-
 trace-events        |    2 +-
 11 files changed, 425 insertions(+), 328 deletions(-)

[Qemu-devel] [PATCH 01/36] uhci: Properly unmap packets on cancel / invalid pid

[Gerd Hoffmann]

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

Packets with an invalid pid, or which were cancelled have
usb_packet_map() called on them on init, but not usb_packet_unmap()
before being freed.

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index c2f08e3..671c712 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -236,6 +236,7 @@ static void uhci_async_cancel(UHCIAsync *async)
     trace_usb_uhci_packet_cancel(async->queue->token, async->td, async->done);
     if (!async->done)
         usb_cancel_packet(&async->packet);
+    usb_packet_unmap(&async->packet, &async->sgl);
     uhci_async_free(async);
 }
 
@@ -887,6 +888,7 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
 
     default:
         /* invalid pid : frame interrupted */
+        usb_packet_unmap(&async->packet, &async->sgl);
         uhci_async_free(async);
         s->status |= UHCI_STS_HCPERR;
         uhci_update_irq(s);
-- 
1.7.1

[Qemu-devel] [PATCH 02/36] uhci: Move checks to continue queuing to uhci_fill_queue()

[Gerd Hoffmann]

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

Rather then having a special check to start queuing after the first packet,
and then another check for the other packets in uhci_fill_queue(), simply
check the previous packet beforehand in uhci_fill_queue()

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 671c712..600d095 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -991,7 +991,8 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td)
     UHCI_TD ptd;
     int ret;
 
-    while (is_valid(plink)) {
+    ptd.ctrl = td->ctrl;
+    while (is_valid(plink) && !(ptd.ctrl & TD_CTRL_SPD)) {
         pci_dma_read(&s->dev, plink & ~0xf, &ptd, sizeof(ptd));
         le32_to_cpus(&ptd.link);
         le32_to_cpus(&ptd.ctrl);
@@ -1010,9 +1011,6 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td)
         }
         assert(ret == TD_RESULT_ASYNC_START);
         assert(int_mask == 0);
-        if (ptd.ctrl & TD_CTRL_SPD) {
-            break;
-        }
         plink = ptd.link;
     }
 }
@@ -1110,9 +1108,7 @@ static void uhci_process_frame(UHCIState *s)
 
         case TD_RESULT_ASYNC_START:
             trace_usb_uhci_td_async(curr_qh & ~0xf, link & ~0xf);
-            if (is_valid(td.link) && !(td.ctrl & TD_CTRL_SPD)) {
-                uhci_fill_queue(s, &td);
-            }
+            uhci_fill_queue(s, &td);
             link = curr_qh ? qh.link : td.link;
             continue;
 
-- 
1.7.1

[Qemu-devel] [PATCH 03/36] ehci: Get rid of packet tbytes field

[Gerd Hoffmann]

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

This field is used in some places to track the tbytes field of the token, but
in other places the field is used directly, use it directly everywhere for
consistency.

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

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 6c65a73..82f4dbd 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -362,7 +362,6 @@ struct EHCIPacket {
     USBPacket packet;
     QEMUSGList sgl;
     int pid;
-    uint32_t tbytes;
     enum async_state async;
     int usb_status;
 };
@@ -1505,15 +1504,16 @@ static void ehci_execute_complete(EHCIQueue *q)
         }
     } else {
         // TODO check 4.12 for splits
+        uint32_t tbytes = get_field(q->qh.token, QTD_TOKEN_TBYTES);
 
-        if (p->tbytes && p->pid == USB_TOKEN_IN) {
-            p->tbytes -= p->usb_status;
+        if (tbytes && p->pid == USB_TOKEN_IN) {
+            tbytes -= p->usb_status;
         } else {
-            p->tbytes = 0;
+            tbytes = 0;
         }
 
-        DPRINTF("updating tbytes to %d\n", p->tbytes);
-        set_field(&q->qh.token, p->tbytes, QTD_TOKEN_TBYTES);
+        DPRINTF("updating tbytes to %d\n", tbytes);
+        set_field(&q->qh.token, tbytes, QTD_TOKEN_TBYTES);
     }
     ehci_finish_transfer(q, p->usb_status);
     usb_packet_unmap(&p->packet, &p->sgl);
@@ -1544,8 +1544,7 @@ static int ehci_execute(EHCIPacket *p, const char *action)
         return USB_RET_PROCERR;
     }
 
-    p->tbytes = (p->qtd.token & QTD_TOKEN_TBYTES_MASK) >> QTD_TOKEN_TBYTES_SH;
-    if (p->tbytes > BUFF_SIZE) {
+    if (get_field(p->qtd.token, QTD_TOKEN_TBYTES) > BUFF_SIZE) {
         ehci_trace_guest_bug(p->queue->ehci,
                              "guest requested more bytes than allowed");
         return USB_RET_PROCERR;
@@ -1582,10 +1581,9 @@ static int ehci_execute(EHCIPacket *p, const char *action)
 
     trace_usb_ehci_packet_action(p->queue, p, action);
     ret = usb_handle_packet(p->queue->dev, &p->packet);
-    DPRINTF("submit: qh %x next %x qtd %x pid %x len %zd "
-            "(total %d) endp %x ret %d\n",
+    DPRINTF("submit: qh %x next %x qtd %x pid %x len %zd endp %x ret %d\n",
             q->qhaddr, q->qh.next, q->qtdaddr, q->pid,
-            q->packet.iov.size, q->tbytes, endp, ret);
+            q->packet.iov.size, endp, ret);
 
     if (ret > BUFF_SIZE) {
         fprintf(stderr, "ret from usb_handle_packet > BUFF_SIZE\n");
-- 
1.7.1

[Qemu-devel] [PATCH 04/36] ehci: Set int flag on a short input packet

[Gerd Hoffmann]

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

According to 4.15.1.2 an interrupt must be raised when a short packet
is received. If we don't do this it may take a significant time for
the guest to notice a short trasnfer has completed, since only the last td
will have its IOC flag set, and a short transfer may complete in an earlier
packet.

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

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 82f4dbd..8b4e3c8 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1508,6 +1508,10 @@ static void ehci_execute_complete(EHCIQueue *q)
 
         if (tbytes && p->pid == USB_TOKEN_IN) {
             tbytes -= p->usb_status;
+            if (tbytes) {
+                /* 4.15.1.2 must raise int on a short input packet */
+                ehci_raise_irq(q->ehci, USBSTS_INT);
+            }
         } else {
             tbytes = 0;
         }
-- 
1.7.1

[Qemu-devel] [PATCH 05/36] ehci: Improve latency of interrupt delivery and async schedule scanning

[Gerd Hoffmann]

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

While doing various performance tests of reading from USB mass storage devices
I noticed the following::
1) When an async handled packet completes, we don't immediately report an
   interrupt to the guest, instead we wait for the frame-timer to run and
   report it from there
2) If 1) has been fixed and an async handled packet takes a while to complete,
   then async_stepdown will become a high value, which means that there
   will be a large latency before any new packets queued by the guest in
   response to the interrupt get seen

1) was done deliberately as part of commit f0ad01f92:
http://www.kraxel.org/cgit/qemu/commit/?h=usb.57&id=f0ad01f92ca02eee7cadbfd225c5de753ebd5fce
Since setting the interrupt immediately on async packet completion was causing
issues with Linux guests, I believe this recently fixed Linux bug explains
why this is happening:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=361aabf395e4a23cf554cf4ec0c0c6963b8beb01

Note that we can *not* count on this fix being present in all Linux guests!

I was hoping that the recently added support for Interrupt Threshold Control
would fix the issues with Linux guests, but adding a simple ehci_commit_irq()
call to ehci_async_bh() still caused problems with Linux guests.

The problem is, that when doing ehci_commit_irq() from ehci_async_bh(),
the "old" frindex value is used to calculate usbsts_frindex, and when
the frame-timer then runs possibly very shortly after ehci_async_bh(),
it increases the frame-timer, and thus any interrupts raised from that
frame-timer run, will also get reported to the guest immediately, rather
then being delayed to the next frame-timer run.

Luckily the solution for this is simple, this means that we need to
increase frindex before calling ehci_commit_irq() from ehci_async_bh(),
which in the end boils down to simple calling ehci_frame_timer() instead
of ehci_async_bh() from the bh.

This may seem like it causes a lot of extra work to be done, but this
is not true. Any work done from the frame-timer processing the periodic
schedule is work which then does not need to be done the next time the
frame timer runs, also the frame-timer will re-arm itself at (possibly)
a later time then it was armed for saving a vmexit at that time.

As an additional advantage moving to simply calling the frame-timer also
fixes 2) as the packet completion will set async_stepdown to 0, and the
re-arming of the timer with an async_stepdown of 0 ensures that any
newly queued up packets get seen in a reasonable amount of time.

This improves the speed (MB/s) of a Linux guest reading from a USB mass
storage device by a factor of 1.5 - 1.7 with input pipelining disabled,
and by a factor of 1.8 with input pipelining enabled.

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

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 8b4e3c8..f9ae05e 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1244,7 +1244,7 @@ static void ehci_opreg_write(void *ptr, hwaddr addr,
             s->usbcmd = val; /* Set usbcmd for ehci_update_halt() */
             ehci_update_halt(s);
             s->async_stepdown = 0;
-            qemu_mod_timer(s->frame_timer, qemu_get_clock_ns(vm_clock));
+            qemu_bh_schedule(s->async_bh);
         }
         break;
 
@@ -2510,12 +2510,6 @@ static void ehci_frame_timer(void *opaque)
     }
 }
 
-static void ehci_async_bh(void *opaque)
-{
-    EHCIState *ehci = opaque;
-    ehci_advance_async_state(ehci);
-}
-
 static const MemoryRegionOps ehci_mmio_caps_ops = {
     .read = ehci_caps_read,
     .valid.min_access_size = 1,
@@ -2744,7 +2738,7 @@ static int usb_ehci_initfn(PCIDevice *dev)
     }
 
     s->frame_timer = qemu_new_timer_ns(vm_clock, ehci_frame_timer, s);
-    s->async_bh = qemu_bh_new(ehci_async_bh, s);
+    s->async_bh = qemu_bh_new(ehci_frame_timer, s);
     QTAILQ_INIT(&s->aqueues);
     QTAILQ_INIT(&s->pqueues);
     usb_packet_init(&s->ipacket);
-- 
1.7.1

[Qemu-devel] [PATCH 06/36] ehci: Speed up the timer of raising int from the async schedule

[Gerd Hoffmann]

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

Often the guest will queue up new packets in response to a packet, in the
async schedule with its IOC flag set, completing. By speeding up the
frame-timer, we notice these new packets earlier. This increases the
speed (MB/s) of a Linux guest reading from a USB mass storage device by a
factor of 1.15 on top of the "Improve latency of interrupt delivery"
speed-ups, both with and without input pipelining enabled.

I've not tested the speed-up of this patch without the
"Improve latency of interrupt delivery" patch.

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

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index f9ae05e..d600f08 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -443,6 +443,7 @@ struct EHCIState {
 
     uint64_t last_run_ns;
     uint32_t async_stepdown;
+    bool int_req_by_async;
 };
 
 #define SET_LAST_RUN_CLOCK(s) \
@@ -1529,6 +1530,9 @@ static void ehci_execute_complete(EHCIQueue *q)
 
     if (q->qh.token & QTD_TOKEN_IOC) {
         ehci_raise_irq(q->ehci, USBSTS_INT);
+        if (q->async) {
+            q->ehci->int_req_by_async = true;
+        }
     }
 }
 
@@ -2504,8 +2508,15 @@ static void ehci_frame_timer(void *opaque)
     }
 
     if (need_timer) {
-        expire_time = t_now + (get_ticks_per_sec()
+        /* If we've raised int, we speed up the timer, so that we quickly
+         * notice any new packets queued up in response */
+        if (ehci->int_req_by_async && (ehci->usbsts & USBSTS_INT)) {
+            expire_time = t_now + get_ticks_per_sec() / (FRAME_TIMER_FREQ * 2);
+            ehci->int_req_by_async = false;
+        } else {
+            expire_time = t_now + (get_ticks_per_sec()
                                * (ehci->async_stepdown+1) / FRAME_TIMER_FREQ);
+        }
         qemu_mod_timer(ehci->frame_timer, expire_time);
     }
 }
-- 
1.7.1

[Qemu-devel] [PATCH 07/36] ehci: Detect going in circles when filling the queue

[Gerd Hoffmann]

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

For ctrl endpoints Windows (atleast Win7) creates circular td lists, so far
these were not a problem because we would stop filling the queue if altnext
was set. Since further patches in this patchset remove the altnext check this
does become a problem and we need detection for going in circles.

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

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index d600f08..8bd87c7 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2073,7 +2073,7 @@ static int ehci_fill_queue(EHCIPacket *p)
 {
     EHCIQueue *q = p->queue;
     EHCIqtd qtd = p->qtd;
-    uint32_t qtdaddr;
+    uint32_t qtdaddr, start_addr = p->qtdaddr;
 
     for (;;) {
         if (NLPTR_TBIT(qtd.altnext) == 0) {
@@ -2083,6 +2083,13 @@ static int ehci_fill_queue(EHCIPacket *p)
             break;
         }
         qtdaddr = qtd.next;
+        /*
+         * Detect circular td lists, Windows creates these, counting on the
+         * active bit going low after execution to make the queue stop.
+         */
+        if (qtdaddr == start_addr) {
+            break;
+        }
         get_dwords(q->ehci, NLPTR_GET(qtdaddr),
                    (uint32_t *) &qtd, sizeof(EHCIqtd) >> 2);
         ehci_trace_qtd(q, NLPTR_GET(qtdaddr), &qtd);
-- 
1.7.1

[Qemu-devel] [PATCH 08/36] ehci: Retry to fill the queue while waiting for td completion

[Gerd Hoffmann]

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

If the guest is using multiple transfers to try and keep the usb bus busy /
used at maximum efficiency, currently we would see / do the following:

1) submit transfer 1 to the device
2) submit transfer 2 to the device
3) report transfer 1 completion to guest
4) report transfer 2 completion to guest
5) submit transfer 1 to the device
6) report transfer 1 completion to guest
7) submit transfer 2 to the device
8) report transfer 2 completion to guest
etc.

So after the initial submission we would effectively only have 1 transfer
in flight, rather then 2. This is caused by us not checking the queue for
addition of new transfers by the guest (ie the resubmission of a recently
finished transfer), while waiting for a pending transfer to complete.
This patch does add a check for this, changing the sequence to:

1) submit transfer 1 to the device
2) submit transfer 2 to the device
3) report transfer 1 completion to guest
4) submit transfer 1 to the device
5) report transfer 2 completion to guest
6) submit transfer 2 to the device
etc.

Thus keeping 2 transfers in flight (most of the time, and always 1),
as intended by the guest.

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

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 8bd87c7..a02fe96 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -381,7 +381,7 @@ struct EHCIQueue {
     uint32_t qhaddr;       /* address QH read from                 */
     uint32_t qtdaddr;      /* address QTD read from                */
     USBDevice *dev;
-    QTAILQ_HEAD(, EHCIPacket) packets;
+    QTAILQ_HEAD(pkts_head, EHCIPacket) packets;
 };
 
 typedef QTAILQ_HEAD(EHCIQueueHead, EHCIQueue) EHCIQueueHead;
@@ -488,6 +488,7 @@ static const char *ehci_mmio_names[] = {
 
 static int ehci_state_executing(EHCIQueue *q);
 static int ehci_state_writeback(EHCIQueue *q);
+static int ehci_fill_queue(EHCIPacket *p);
 
 static const char *nr2str(const char **n, size_t len, uint32_t nr)
 {
@@ -1994,7 +1995,7 @@ static int ehci_state_fetchqtd(EHCIQueue *q)
 {
     EHCIqtd qtd;
     EHCIPacket *p;
-    int again = 0;
+    int again = 1;
 
     get_dwords(q->ehci, NLPTR_GET(q->qtdaddr), (uint32_t *) &qtd,
                sizeof(EHCIqtd) >> 2);
@@ -2022,7 +2023,6 @@ static int ehci_state_fetchqtd(EHCIQueue *q)
             p = NULL;
         }
         ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
-        again = 1;
     } else if (p != NULL) {
         switch (p->async) {
         case EHCI_ASYNC_NONE:
@@ -2031,6 +2031,9 @@ static int ehci_state_fetchqtd(EHCIQueue *q)
             ehci_set_state(q->ehci, q->async, EST_EXECUTE);
             break;
         case EHCI_ASYNC_INFLIGHT:
+            /* Check if the guest has added new tds to the queue */
+            again = (ehci_fill_queue(QTAILQ_LAST(&q->packets, pkts_head)) ==
+                     USB_RET_PROCERR) ? -1 : 1;
             /* Unfinished async handled packet, go horizontal */
             ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
             break;
@@ -2042,13 +2045,11 @@ static int ehci_state_fetchqtd(EHCIQueue *q)
             ehci_set_state(q->ehci, q->async, EST_EXECUTING);
             break;
         }
-        again = 1;
     } else {
         p = ehci_alloc_packet(q);
         p->qtdaddr = q->qtdaddr;
         p->qtd = qtd;
         ehci_set_state(q->ehci, q->async, EST_EXECUTE);
-        again = 1;
     }
 
     return again;
-- 
1.7.1

[Qemu-devel] [PATCH 09/36] xhci: Add a xhci_ep_nuke_one_xfer helper function

[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/hcd-xhci.c |   49 ++++++++++++++++++++++++++++++-------------------
 1 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 37b3dbb..47d5702 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1082,6 +1082,35 @@ static TRBCCode xhci_enable_ep(XHCIState *xhci, unsigned int slotid,
     return CC_SUCCESS;
 }
 
+static int xhci_ep_nuke_one_xfer(XHCITransfer *t)
+{
+    int killed = 0;
+
+    if (t->running_async) {
+        usb_cancel_packet(&t->packet);
+        t->running_async = 0;
+        t->cancelled = 1;
+        DPRINTF("xhci: cancelling transfer, waiting for it to complete\n");
+        killed = 1;
+    }
+    if (t->running_retry) {
+        XHCIEPContext *epctx = t->xhci->slots[t->slotid-1].eps[t->epid-1];
+        if (epctx) {
+            epctx->retry = NULL;
+            qemu_del_timer(epctx->kick_timer);
+        }
+        t->running_retry = 0;
+    }
+    if (t->trbs) {
+        g_free(t->trbs);
+    }
+
+    t->trbs = NULL;
+    t->trb_count = t->trb_alloced = 0;
+
+    return killed;
+}
+
 static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid,
                                unsigned int epid)
 {
@@ -1103,25 +1132,7 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid,
 
     xferi = epctx->next_xfer;
     for (i = 0; i < TD_QUEUE; i++) {
-        XHCITransfer *t = &epctx->transfers[xferi];
-        if (t->running_async) {
-            usb_cancel_packet(&t->packet);
-            t->running_async = 0;
-            t->cancelled = 1;
-            DPRINTF("xhci: cancelling transfer %d, waiting for it to complete...\n", i);
-            killed++;
-        }
-        if (t->running_retry) {
-            t->running_retry = 0;
-            epctx->retry = NULL;
-            qemu_del_timer(epctx->kick_timer);
-        }
-        if (t->trbs) {
-            g_free(t->trbs);
-        }
-
-        t->trbs = NULL;
-        t->trb_count = t->trb_alloced = 0;
+        killed += xhci_ep_nuke_one_xfer(&epctx->transfers[xferi]);
         xferi = (xferi + 1) % TD_QUEUE;
     }
     return killed;
-- 
1.7.1

[Qemu-devel] [PATCH 10/36] usb: Rename __usb_packet_complete to usb_packet_complete_one

[Gerd Hoffmann]

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

And make it available for use outside of core.c

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

diff --git a/hw/usb.h b/hw/usb.h
index 48c8926..01dd423 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -370,6 +370,7 @@ USBDevice *usb_find_device(USBPort *port, uint8_t addr);
 
 int usb_handle_packet(USBDevice *dev, USBPacket *p);
 void usb_packet_complete(USBDevice *dev, USBPacket *p);
+void usb_packet_complete_one(USBDevice *dev, USBPacket *p);
 void usb_cancel_packet(USBPacket * p);
 
 void usb_ep_init(USBDevice *dev);
diff --git a/hw/usb/core.c b/hw/usb/core.c
index b9f1f7a..e2e31ca 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -412,10 +412,11 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
     return ret;
 }
 
-static void __usb_packet_complete(USBDevice *dev, USBPacket *p)
+void usb_packet_complete_one(USBDevice *dev, USBPacket *p)
 {
     USBEndpoint *ep = p->ep;
 
+    assert(QTAILQ_FIRST(&ep->queue) == p);
     assert(p->result != USB_RET_ASYNC && p->result != USB_RET_NAK);
 
     if (p->result < 0) {
@@ -435,8 +436,7 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p)
     int ret;
 
     usb_packet_check_state(p, USB_PACKET_ASYNC);
-    assert(QTAILQ_FIRST(&ep->queue) == p);
-    __usb_packet_complete(dev, p);
+    usb_packet_complete_one(dev, p);
 
     while (!ep->halted && !QTAILQ_EMPTY(&ep->queue)) {
         p = QTAILQ_FIRST(&ep->queue);
@@ -450,7 +450,7 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p)
             break;
         }
         p->result = ret;
-        __usb_packet_complete(ep->dev, p);
+        usb_packet_complete_one(ep->dev, p);
     }
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 11/36] usb: Add USB_RET_ADD_TO_QUEUE packet result code

[Gerd Hoffmann]

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

This can be used by usb-device code which wishes to process an entire endpoint
queue at once, to do this the usb-device code returns USB_RET_ADD_TO_QUEUE
from its handle_data class method and defines a flush_ep_queue class method
to call when the hcd is done queuing up packets.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb.h          |   21 +++++++++++++++------
 hw/usb/bus.c      |    8 ++++++++
 hw/usb/core.c     |    4 ++++
 hw/usb/hcd-ehci.c |    4 ++++
 hw/usb/hcd-musb.c |    1 +
 hw/usb/hcd-ohci.c |    2 ++
 hw/usb/hcd-uhci.c |   16 +++++++++++-----
 hw/usb/hcd-xhci.c |    6 ++++++
 8 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/hw/usb.h b/hw/usb.h
index 01dd423..435cd42 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -38,12 +38,13 @@
 #define USB_TOKEN_IN    0x69 /* device -> host */
 #define USB_TOKEN_OUT   0xe1 /* host -> device */
 
-#define USB_RET_NODEV   (-1)
-#define USB_RET_NAK     (-2)
-#define USB_RET_STALL   (-3)
-#define USB_RET_BABBLE  (-4)
-#define USB_RET_IOERROR (-5)
-#define USB_RET_ASYNC   (-6)
+#define USB_RET_NODEV             (-1)
+#define USB_RET_NAK               (-2)
+#define USB_RET_STALL             (-3)
+#define USB_RET_BABBLE            (-4)
+#define USB_RET_IOERROR           (-5)
+#define USB_RET_ASYNC             (-6)
+#define USB_RET_ADD_TO_QUEUE      (-7)
 
 #define USB_SPEED_LOW   0
 #define USB_SPEED_FULL  1
@@ -293,6 +294,12 @@ typedef struct USBDeviceClass {
     void (*set_interface)(USBDevice *dev, int interface,
                           int alt_old, int alt_new);
 
+    /*
+     * Called when the hcd is done queuing packets for an endpoint, only
+     * necessary for devices which can return USB_RET_ADD_TO_QUEUE.
+     */
+    void (*flush_ep_queue)(USBDevice *dev, USBEndpoint *ep);
+
     const char *product_desc;
     const USBDesc *usb_desc;
 } USBDeviceClass;
@@ -507,6 +514,8 @@ int usb_device_handle_data(USBDevice *dev, USBPacket *p);
 void usb_device_set_interface(USBDevice *dev, int interface,
                               int alt_old, int alt_new);
 
+void usb_device_flush_ep_queue(USBDevice *dev, USBEndpoint *ep);
+
 const char *usb_device_get_product_desc(USBDevice *dev);
 
 const USBDesc *usb_device_get_usb_desc(USBDevice *dev);
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index b649360..8066291 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -181,6 +181,14 @@ void usb_device_set_interface(USBDevice *dev, int interface,
     }
 }
 
+void usb_device_flush_ep_queue(USBDevice *dev, USBEndpoint *ep)
+{
+    USBDeviceClass *klass = USB_DEVICE_GET_CLASS(dev);
+    if (klass->flush_ep_queue) {
+        klass->flush_ep_queue(dev, ep);
+    }
+}
+
 static int usb_qdev_init(DeviceState *qdev)
 {
     USBDevice *dev = USB_DEVICE(qdev);
diff --git a/hw/usb/core.c b/hw/usb/core.c
index e2e31ca..014e3ac 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -393,6 +393,10 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
         if (ret == USB_RET_ASYNC) {
             usb_packet_set_state(p, USB_PACKET_ASYNC);
             QTAILQ_INSERT_TAIL(&p->ep->queue, p, queue);
+        } else if (ret == USB_RET_ADD_TO_QUEUE) {
+            usb_packet_set_state(p, USB_PACKET_QUEUED);
+            QTAILQ_INSERT_TAIL(&p->ep->queue, p, queue);
+            ret = USB_RET_ASYNC;
         } else {
             /*
              * When pipelining is enabled usb-devices must always return async,
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index a02fe96..d11311e 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2072,6 +2072,7 @@ static int ehci_state_horizqh(EHCIQueue *q)
 
 static int ehci_fill_queue(EHCIPacket *p)
 {
+    USBEndpoint *ep = p->packet.ep;
     EHCIQueue *q = p->queue;
     EHCIqtd qtd = p->qtd;
     uint32_t qtdaddr, start_addr = p->qtdaddr;
@@ -2107,6 +2108,9 @@ static int ehci_fill_queue(EHCIPacket *p)
         assert(p->usb_status == USB_RET_ASYNC);
         p->async = EHCI_ASYNC_INFLIGHT;
     }
+    if (p->usb_status != USB_RET_PROCERR) {
+        usb_device_flush_ep_queue(ep->dev, ep);
+    }
     return p->usb_status;
 }
 
diff --git a/hw/usb/hcd-musb.c b/hw/usb/hcd-musb.c
index dc114fe..212dd12 100644
--- a/hw/usb/hcd-musb.c
+++ b/hw/usb/hcd-musb.c
@@ -635,6 +635,7 @@ static void musb_packet(MUSBState *s, MUSBEndPoint *ep,
     ret = usb_handle_packet(dev, &ep->packey[dir].p);
 
     if (ret == USB_RET_ASYNC) {
+        usb_device_flush_ep_queue(dev, uep);
         ep->status[dir] = len;
         return;
     }
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 0cc1e5d..31dcfbb 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -816,6 +816,7 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
         usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, len);
         ret = usb_handle_packet(dev, &ohci->usb_packet);
         if (ret == USB_RET_ASYNC) {
+            usb_device_flush_ep_queue(dev, ep);
             return 1;
         }
     }
@@ -1018,6 +1019,7 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
         DPRINTF("ret=%d\n", ret);
 #endif
         if (ret == USB_RET_ASYNC) {
+            usb_device_flush_ep_queue(dev, ep);
             ohci->async_td = addr;
             return 1;
         }
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 600d095..46e544b 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -818,7 +818,8 @@ out:
 }
 
 static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
-                          uint32_t *int_mask, bool queuing)
+                          uint32_t *int_mask, bool queuing,
+                          struct USBEndpoint **ep_ret)
 {
     UHCIAsync *async;
     int len = 0, max_len;
@@ -870,6 +871,9 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
 
     dev = uhci_find_device(s, (td->token >> 8) & 0x7f);
     ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf);
+    if (ep_ret) {
+        *ep_ret = ep;
+    }
     usb_packet_setup(&async->packet, pid, ep, addr);
     qemu_sglist_add(&async->sgl, td->buffer, max_len);
     usb_packet_map(&async->packet, &async->sgl);
@@ -983,7 +987,7 @@ static int qhdb_insert(QhDb *db, uint32_t addr)
     return 0;
 }
 
-static void uhci_fill_queue(UHCIState *s, UHCI_TD *td)
+static void uhci_fill_queue(UHCIState *s, UHCI_TD *td, struct USBEndpoint *ep)
 {
     uint32_t int_mask = 0;
     uint32_t plink = td->link;
@@ -1005,7 +1009,7 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td)
             break;
         }
         trace_usb_uhci_td_queue(plink & ~0xf, ptd.ctrl, ptd.token);
-        ret = uhci_handle_td(s, plink, &ptd, &int_mask, true);
+        ret = uhci_handle_td(s, plink, &ptd, &int_mask, true, NULL);
         if (ret == TD_RESULT_ASYNC_CONT) {
             break;
         }
@@ -1013,12 +1017,14 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td)
         assert(int_mask == 0);
         plink = ptd.link;
     }
+    usb_device_flush_ep_queue(ep->dev, ep);
 }
 
 static void uhci_process_frame(UHCIState *s)
 {
     uint32_t frame_addr, link, old_td_ctrl, val, int_mask;
     uint32_t curr_qh, td_count = 0;
+    struct USBEndpoint *curr_ep;
     int cnt, ret;
     UHCI_TD td;
     UHCI_QH qh;
@@ -1089,7 +1095,7 @@ static void uhci_process_frame(UHCIState *s)
         trace_usb_uhci_td_load(curr_qh & ~0xf, link & ~0xf, td.ctrl, td.token);
 
         old_td_ctrl = td.ctrl;
-        ret = uhci_handle_td(s, link, &td, &int_mask, false);
+        ret = uhci_handle_td(s, link, &td, &int_mask, false, &curr_ep);
         if (old_td_ctrl != td.ctrl) {
             /* update the status bits of the TD */
             val = cpu_to_le32(td.ctrl);
@@ -1108,7 +1114,7 @@ static void uhci_process_frame(UHCIState *s)
 
         case TD_RESULT_ASYNC_START:
             trace_usb_uhci_td_async(curr_qh & ~0xf, link & ~0xf);
-            uhci_fill_queue(s, &td);
+            uhci_fill_queue(s, &td, curr_ep);
             link = curr_qh ? qh.link : td.link;
             continue;
 
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 47d5702..e8929a0 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1652,6 +1652,7 @@ static int xhci_fire_transfer(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext
 static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid)
 {
     XHCIEPContext *epctx;
+    USBEndpoint *ep = NULL;
     uint64_t mfindex;
     int length;
     int i;
@@ -1745,12 +1746,14 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid
         if (epid == 1) {
             if (xhci_fire_ctl_transfer(xhci, xfer) >= 0) {
                 epctx->next_xfer = (epctx->next_xfer + 1) % TD_QUEUE;
+                ep = xfer->packet.ep;
             } else {
                 fprintf(stderr, "xhci: error firing CTL transfer\n");
             }
         } else {
             if (xhci_fire_transfer(xhci, xfer, epctx) >= 0) {
                 epctx->next_xfer = (epctx->next_xfer + 1) % TD_QUEUE;
+                ep = xfer->packet.ep;
             } else {
                 if (!xfer->iso_xfer) {
                     fprintf(stderr, "xhci: error firing data transfer\n");
@@ -1767,6 +1770,9 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid
             break;
         }
     }
+    if (ep) {
+        usb_device_flush_ep_queue(ep->dev, ep);
+    }
 }
 
 static TRBCCode xhci_enable_slot(XHCIState *xhci, unsigned int slotid)
-- 
1.7.1

[Qemu-devel] [PATCH 12/36] usb: Move clearing of queue on halt to the core

[Gerd Hoffmann]

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

hcds which queue up more then one packet at once (uhci, ehci and xhci),
must clear the queue after an error which has caused the queue to halt.

Currently this is handled as a special case inside the hcd code, this
patch instead adds an USB_RET_REMOVE_FROM_QUEUE packet result code, teaches
the 3 hcds about this and moves the clearing of the queue on a halt into
the USB core.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb.h          |    1 +
 hw/usb/core.c     |    8 +++++++-
 hw/usb/hcd-ehci.c |   22 ++++++++--------------
 hw/usb/hcd-uhci.c |   22 ++++++----------------
 hw/usb/hcd-xhci.c |    4 ++++
 5 files changed, 26 insertions(+), 31 deletions(-)

diff --git a/hw/usb.h b/hw/usb.h
index 435cd42..ead03c9 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -45,6 +45,7 @@
 #define USB_RET_IOERROR           (-5)
 #define USB_RET_ASYNC             (-6)
 #define USB_RET_ADD_TO_QUEUE      (-7)
+#define USB_RET_REMOVE_FROM_QUEUE (-8)
 
 #define USB_SPEED_LOW   0
 #define USB_SPEED_FULL  1
diff --git a/hw/usb/core.c b/hw/usb/core.c
index 014e3ac..5a97a0e 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -442,8 +442,14 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p)
     usb_packet_check_state(p, USB_PACKET_ASYNC);
     usb_packet_complete_one(dev, p);
 
-    while (!ep->halted && !QTAILQ_EMPTY(&ep->queue)) {
+    while (!QTAILQ_EMPTY(&ep->queue)) {
         p = QTAILQ_FIRST(&ep->queue);
+        if (ep->halted) {
+            /* Empty the queue on a halt */
+            p->result = USB_RET_REMOVE_FROM_QUEUE;
+            dev->port->ops->complete(dev->port, p);
+            continue;
+        }
         if (p->state == USB_PACKET_ASYNC) {
             break;
         }
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index d11311e..74a2587 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1457,8 +1457,15 @@ static void ehci_async_complete_packet(USBPort *port, USBPacket *packet)
     }
 
     p = container_of(packet, EHCIPacket, packet);
-    trace_usb_ehci_packet_action(p->queue, p, "wakeup");
     assert(p->async == EHCI_ASYNC_INFLIGHT);
+
+    if (packet->result == USB_RET_REMOVE_FROM_QUEUE) {
+        trace_usb_ehci_packet_action(p->queue, p, "remove");
+        ehci_free_packet(p);
+        return;
+    }
+
+    trace_usb_ehci_packet_action(p->queue, p, "wakeup");
     p->async = EHCI_ASYNC_FINISHED;
     p->usb_status = packet->result;
 
@@ -2216,19 +2223,6 @@ static int ehci_state_writeback(EHCIQueue *q)
      * bit is clear.
      */
     if (q->qh.token & QTD_TOKEN_HALT) {
-        /*
-         * We should not do any further processing on a halted queue!
-         * This is esp. important for bulk endpoints with pipelining enabled
-         * (redirection to a real USB device), where we must cancel all the
-         * transfers after this one so that:
-         * 1) If they've completed already, they are not processed further
-         *    causing more stalls, originating from the same failed transfer
-         * 2) If still in flight, they are cancelled before the guest does
-         *    a clear stall, otherwise the guest and device can loose sync!
-         */
-        while ((p = QTAILQ_FIRST(&q->packets)) != NULL) {
-            ehci_free_packet(p);
-        }
         ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
         again = 1;
     } else {
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 46e544b..00dc9d5 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -744,22 +744,6 @@ static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_
     return TD_RESULT_COMPLETE;
 
 out:
-    /*
-     * We should not do any further processing on a queue with errors!
-     * This is esp. important for bulk endpoints with pipelining enabled
-     * (redirection to a real USB device), where we must cancel all the
-     * transfers after this one so that:
-     * 1) If they've completed already, they are not processed further
-     *    causing more stalls, originating from the same failed transfer
-     * 2) If still in flight, they are cancelled before the guest does
-     *    a clear stall, otherwise the guest and device can loose sync!
-     */
-    while (!QTAILQ_EMPTY(&async->queue->asyncs)) {
-        UHCIAsync *as = QTAILQ_FIRST(&async->queue->asyncs);
-        uhci_async_unlink(as);
-        uhci_async_cancel(as);
-    }
-
     switch(ret) {
     case USB_RET_STALL:
         td->ctrl |= TD_CTRL_STALL;
@@ -918,6 +902,12 @@ static void uhci_async_complete(USBPort *port, USBPacket *packet)
     UHCIAsync *async = container_of(packet, UHCIAsync, packet);
     UHCIState *s = async->queue->uhci;
 
+    if (packet->result == USB_RET_REMOVE_FROM_QUEUE) {
+        uhci_async_unlink(async);
+        uhci_async_cancel(async);
+        return;
+    }
+
     if (async->isoc) {
         UHCI_TD td;
         uint32_t link = async->td;
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index e8929a0..1f437cc 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2839,6 +2839,10 @@ static void xhci_complete(USBPort *port, USBPacket *packet)
 {
     XHCITransfer *xfer = container_of(packet, XHCITransfer, packet);
 
+    if (packet->result == USB_RET_REMOVE_FROM_QUEUE) {
+        xhci_ep_nuke_one_xfer(xfer);
+        return;
+    }
     xhci_complete_packet(xfer, packet->result);
     xhci_kick_ep(xfer->xhci, xfer->slotid, xfer->epid);
 }
-- 
1.7.1

[Qemu-devel] [PATCH 13/36] usb: Move short-not-ok handling to the core

[Gerd Hoffmann]

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

After a short-not-ok packet ending short, we should not advance the queue.
Move enforcing this to the core, rather then handling it in the hcd code.

This may result in the queue now actually containing multiple input packets
(which would not happen before), and this requires special handling in
combination with pipelining, so disable pipleining for input endpoints
(for now).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb.h            |    4 +++-
 hw/usb/core.c       |    6 ++++--
 hw/usb/hcd-ehci.c   |    9 ++++-----
 hw/usb/hcd-musb.c   |    2 +-
 hw/usb/hcd-ohci.c   |    4 ++--
 hw/usb/hcd-uhci.c   |    7 ++++---
 hw/usb/hcd-xhci.c   |    2 +-
 hw/usb/host-linux.c |    3 ++-
 hw/usb/redirect.c   |   18 ++++++++++++------
 9 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/hw/usb.h b/hw/usb.h
index ead03c9..1fcf79c 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -351,6 +351,7 @@ struct USBPacket {
     USBEndpoint *ep;
     QEMUIOVector iov;
     uint64_t parameter; /* control transfers */
+    bool short_not_ok;
     int result; /* transfer length or USB_RET_* status code */
     /* Internal use by the USB layer.  */
     USBPacketState state;
@@ -360,7 +361,8 @@ struct USBPacket {
 void usb_packet_init(USBPacket *p);
 void usb_packet_set_state(USBPacket *p, USBPacketState state);
 void usb_packet_check_state(USBPacket *p, USBPacketState expected);
-void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id);
+void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id,
+                      bool short_not_ok);
 void usb_packet_addbuf(USBPacket *p, void *ptr, size_t len);
 int usb_packet_map(USBPacket *p, QEMUSGList *sgl);
 void usb_packet_unmap(USBPacket *p, QEMUSGList *sgl);
diff --git a/hw/usb/core.c b/hw/usb/core.c
index 5a97a0e..f4a5ad2 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -423,7 +423,7 @@ void usb_packet_complete_one(USBDevice *dev, USBPacket *p)
     assert(QTAILQ_FIRST(&ep->queue) == p);
     assert(p->result != USB_RET_ASYNC && p->result != USB_RET_NAK);
 
-    if (p->result < 0) {
+    if (p->result < 0 || (p->short_not_ok && (p->result < p->iov.size))) {
         ep->halted = true;
     }
     usb_packet_set_state(p, USB_PACKET_COMPLETE);
@@ -532,7 +532,8 @@ void usb_packet_set_state(USBPacket *p, USBPacketState state)
     p->state = state;
 }
 
-void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id)
+void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id,
+                      bool short_not_ok)
 {
     assert(!usb_packet_is_inflight(p));
     assert(p->iov.iov != NULL);
@@ -541,6 +542,7 @@ void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id)
     p->ep = ep;
     p->result = 0;
     p->parameter = 0;
+    p->short_not_ok = short_not_ok;
     qemu_iovec_reset(&p->iov);
     usb_packet_set_state(p, USB_PACKET_SETUP);
 }
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 74a2587..8e8ec6b 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1551,6 +1551,7 @@ static int ehci_execute(EHCIPacket *p, const char *action)
     USBEndpoint *ep;
     int ret;
     int endp;
+    bool spd;
 
     assert(p->async == EHCI_ASYNC_NONE ||
            p->async == EHCI_ASYNC_INITIALIZED);
@@ -1590,7 +1591,8 @@ static int ehci_execute(EHCIPacket *p, const char *action)
             return USB_RET_PROCERR;
         }
 
-        usb_packet_setup(&p->packet, p->pid, ep, p->qtdaddr);
+        spd = (p->pid == USB_TOKEN_IN && NLPTR_TBIT(p->qtd.altnext) == 0);
+        usb_packet_setup(&p->packet, p->pid, ep, p->qtdaddr, spd);
         usb_packet_map(&p->packet, &p->sgl);
         p->async = EHCI_ASYNC_INITIALIZED;
     }
@@ -1660,7 +1662,7 @@ static int ehci_process_itd(EHCIState *ehci,
             dev = ehci_find_device(ehci, devaddr);
             ep = usb_ep_get(dev, pid, endp);
             if (ep && ep->type == USB_ENDPOINT_XFER_ISOC) {
-                usb_packet_setup(&ehci->ipacket, pid, ep, addr);
+                usb_packet_setup(&ehci->ipacket, pid, ep, addr, false);
                 usb_packet_map(&ehci->ipacket, &ehci->isgl);
                 ret = usb_handle_packet(dev, &ehci->ipacket);
                 assert(ret != USB_RET_ASYNC);
@@ -2085,9 +2087,6 @@ static int ehci_fill_queue(EHCIPacket *p)
     uint32_t qtdaddr, start_addr = p->qtdaddr;
 
     for (;;) {
-        if (NLPTR_TBIT(qtd.altnext) == 0) {
-            break;
-        }
         if (NLPTR_TBIT(qtd.next) != 0) {
             break;
         }
diff --git a/hw/usb/hcd-musb.c b/hw/usb/hcd-musb.c
index 212dd12..c4309a4 100644
--- a/hw/usb/hcd-musb.c
+++ b/hw/usb/hcd-musb.c
@@ -627,7 +627,7 @@ static void musb_packet(MUSBState *s, MUSBEndPoint *ep,
     dev = usb_find_device(&s->port, ep->faddr[idx]);
     uep = usb_ep_get(dev, pid, ep->type[idx] & 0xf);
     usb_packet_setup(&ep->packey[dir].p, pid, uep,
-                     (dev->addr << 16) | (uep->nr << 8) | pid);
+                     (dev->addr << 16) | (uep->nr << 8) | pid, false);
     usb_packet_addbuf(&ep->packey[dir].p, ep->buf[idx], len);
     ep->packey[dir].ep = ep;
     ep->packey[dir].dir = dir;
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 31dcfbb..00e2e1a 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -812,7 +812,7 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
     } else {
         dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA));
         ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN));
-        usb_packet_setup(&ohci->usb_packet, pid, ep, addr);
+        usb_packet_setup(&ohci->usb_packet, pid, ep, addr, false);
         usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, len);
         ret = usb_handle_packet(dev, &ohci->usb_packet);
         if (ret == USB_RET_ASYNC) {
@@ -1012,7 +1012,7 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
         }
         dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA));
         ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN));
-        usb_packet_setup(&ohci->usb_packet, pid, ep, addr);
+        usb_packet_setup(&ohci->usb_packet, pid, ep, addr, !flag_r);
         usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, pktlen);
         ret = usb_handle_packet(dev, &ohci->usb_packet);
 #ifdef DEBUG_PACKET
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 00dc9d5..953897b 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -808,6 +808,7 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
     UHCIAsync *async;
     int len = 0, max_len;
     uint8_t pid;
+    bool spd;
     USBDevice *dev;
     USBEndpoint *ep;
 
@@ -852,13 +853,14 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
 
     max_len = ((td->token >> 21) + 1) & 0x7ff;
     pid = td->token & 0xff;
+    spd = (pid == USB_TOKEN_IN && (td->ctrl & TD_CTRL_SPD) != 0);
 
     dev = uhci_find_device(s, (td->token >> 8) & 0x7f);
     ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf);
     if (ep_ret) {
         *ep_ret = ep;
     }
-    usb_packet_setup(&async->packet, pid, ep, addr);
+    usb_packet_setup(&async->packet, pid, ep, addr, spd);
     qemu_sglist_add(&async->sgl, td->buffer, max_len);
     usb_packet_map(&async->packet, &async->sgl);
 
@@ -985,8 +987,7 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td, struct USBEndpoint *ep)
     UHCI_TD ptd;
     int ret;
 
-    ptd.ctrl = td->ctrl;
-    while (is_valid(plink) && !(ptd.ctrl & TD_CTRL_SPD)) {
+    while (is_valid(plink)) {
         pci_dma_read(&s->dev, plink & ~0xf, &ptd, sizeof(ptd));
         le32_to_cpus(&ptd.link);
         le32_to_cpus(&ptd.ctrl);
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 1f437cc..ebfc5b8 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1446,7 +1446,7 @@ static int xhci_setup_packet(XHCITransfer *xfer)
         ep = usb_ep_get(dev, dir, xfer->epid >> 1);
     }
 
-    usb_packet_setup(&xfer->packet, dir, ep, xfer->trbs[0].addr);
+    usb_packet_setup(&xfer->packet, dir, ep, xfer->trbs[0].addr, false);
     xhci_xfer_map(xfer);
     DPRINTF("xhci: setup packet pid 0x%x addr %d ep %d\n",
             xfer->packet.pid, dev->addr, ep->nr);
diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index 44f1a64..3a258b4 100644
--- a/hw/usb/host-linux.c
+++ b/hw/usb/host-linux.c
@@ -1224,7 +1224,8 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
                 usb_ep_set_type(&s->dev, pid, ep, type);
                 usb_ep_set_ifnum(&s->dev, pid, ep, interface);
                 if ((s->options & (1 << USB_HOST_OPT_PIPELINE)) &&
-                    (type == USB_ENDPOINT_XFER_BULK)) {
+                    (type == USB_ENDPOINT_XFER_BULK) &&
+                    (pid == USB_TOKEN_OUT)) {
                     usb_ep_set_pipeline(&s->dev, pid, ep, true);
                 }
 
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 2283565..22f671b 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1270,6 +1270,16 @@ static void usbredir_interface_info(void *priv,
     }
 }
 
+static void usbredir_set_pipeline(USBRedirDevice *dev, struct USBEndpoint *uep)
+{
+    if (uep->type != USB_ENDPOINT_XFER_BULK) {
+        return;
+    }
+    if (uep->pid == USB_TOKEN_OUT) {
+        uep->pipeline = true;
+    }
+}
+
 static void usbredir_ep_info(void *priv,
     struct usb_redir_ep_info_header *ep_info)
 {
@@ -1311,9 +1321,7 @@ static void usbredir_ep_info(void *priv,
             dev->endpoint[i].max_packet_size =
                 usb_ep->max_packet_size = ep_info->max_packet_size[i];
         }
-        if (ep_info->type[i] == usb_redir_type_bulk) {
-            usb_ep->pipeline = true;
-        }
+        usbredir_set_pipeline(dev, usb_ep);
     }
 }
 
@@ -1574,9 +1582,7 @@ static int usbredir_post_load(void *priv, int version_id)
         usb_ep->type = dev->endpoint[i].type;
         usb_ep->ifnum = dev->endpoint[i].interface;
         usb_ep->max_packet_size = dev->endpoint[i].max_packet_size;
-        if (dev->endpoint[i].type == usb_redir_type_bulk) {
-            usb_ep->pipeline = true;
-        }
+        usbredir_set_pipeline(dev, usb_ep);
     }
     return 0;
 }
-- 
1.7.1

[Qemu-devel] [PATCH 14/36] usb: Add an int_req flag to USBPacket

[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.h          |    3 ++-
 hw/usb/core.c     |    3 ++-
 hw/usb/hcd-ehci.c |    6 ++++--
 hw/usb/hcd-musb.c |    2 +-
 hw/usb/hcd-ohci.c |    7 +++++--
 hw/usb/hcd-uhci.c |    3 ++-
 hw/usb/hcd-xhci.c |   16 +++++++++++-----
 7 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/hw/usb.h b/hw/usb.h
index 1fcf79c..3a6cc84 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -352,6 +352,7 @@ struct USBPacket {
     QEMUIOVector iov;
     uint64_t parameter; /* control transfers */
     bool short_not_ok;
+    bool int_req;
     int result; /* transfer length or USB_RET_* status code */
     /* Internal use by the USB layer.  */
     USBPacketState state;
@@ -362,7 +363,7 @@ void usb_packet_init(USBPacket *p);
 void usb_packet_set_state(USBPacket *p, USBPacketState state);
 void usb_packet_check_state(USBPacket *p, USBPacketState expected);
 void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id,
-                      bool short_not_ok);
+                      bool short_not_ok, bool int_req);
 void usb_packet_addbuf(USBPacket *p, void *ptr, size_t len);
 int usb_packet_map(USBPacket *p, QEMUSGList *sgl);
 void usb_packet_unmap(USBPacket *p, QEMUSGList *sgl);
diff --git a/hw/usb/core.c b/hw/usb/core.c
index f4a5ad2..87a513f 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -533,7 +533,7 @@ void usb_packet_set_state(USBPacket *p, USBPacketState state)
 }
 
 void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id,
-                      bool short_not_ok)
+                      bool short_not_ok, bool int_req)
 {
     assert(!usb_packet_is_inflight(p));
     assert(p->iov.iov != NULL);
@@ -543,6 +543,7 @@ void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep, uint64_t id,
     p->result = 0;
     p->parameter = 0;
     p->short_not_ok = short_not_ok;
+    p->int_req = int_req;
     qemu_iovec_reset(&p->iov);
     usb_packet_set_state(p, USB_PACKET_SETUP);
 }
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 8e8ec6b..8ac78ad 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1592,7 +1592,8 @@ static int ehci_execute(EHCIPacket *p, const char *action)
         }
 
         spd = (p->pid == USB_TOKEN_IN && NLPTR_TBIT(p->qtd.altnext) == 0);
-        usb_packet_setup(&p->packet, p->pid, ep, p->qtdaddr, spd);
+        usb_packet_setup(&p->packet, p->pid, ep, p->qtdaddr, spd,
+                         (p->qtd.token & QTD_TOKEN_IOC) != 0);
         usb_packet_map(&p->packet, &p->sgl);
         p->async = EHCI_ASYNC_INITIALIZED;
     }
@@ -1662,7 +1663,8 @@ static int ehci_process_itd(EHCIState *ehci,
             dev = ehci_find_device(ehci, devaddr);
             ep = usb_ep_get(dev, pid, endp);
             if (ep && ep->type == USB_ENDPOINT_XFER_ISOC) {
-                usb_packet_setup(&ehci->ipacket, pid, ep, addr, false);
+                usb_packet_setup(&ehci->ipacket, pid, ep, addr, false,
+                                 (itd->transact[i] & ITD_XACT_IOC) != 0);
                 usb_packet_map(&ehci->ipacket, &ehci->isgl);
                 ret = usb_handle_packet(dev, &ehci->ipacket);
                 assert(ret != USB_RET_ASYNC);
diff --git a/hw/usb/hcd-musb.c b/hw/usb/hcd-musb.c
index c4309a4..4f55390 100644
--- a/hw/usb/hcd-musb.c
+++ b/hw/usb/hcd-musb.c
@@ -627,7 +627,7 @@ static void musb_packet(MUSBState *s, MUSBEndPoint *ep,
     dev = usb_find_device(&s->port, ep->faddr[idx]);
     uep = usb_ep_get(dev, pid, ep->type[idx] & 0xf);
     usb_packet_setup(&ep->packey[dir].p, pid, uep,
-                     (dev->addr << 16) | (uep->nr << 8) | pid, false);
+                     (dev->addr << 16) | (uep->nr << 8) | pid, false, true);
     usb_packet_addbuf(&ep->packey[dir].p, ep->buf[idx], len);
     ep->packey[dir].ep = ep;
     ep->packey[dir].dir = dir;
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 00e2e1a..7571e9e 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -810,9 +810,11 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
     if (completion) {
         ret = ohci->usb_packet.result;
     } else {
+        bool int_req = relative_frame_number == frame_count &&
+                       OHCI_BM(iso_td.flags, TD_DI) == 0;
         dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA));
         ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN));
-        usb_packet_setup(&ohci->usb_packet, pid, ep, addr, false);
+        usb_packet_setup(&ohci->usb_packet, pid, ep, addr, false, int_req);
         usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, len);
         ret = usb_handle_packet(dev, &ohci->usb_packet);
         if (ret == USB_RET_ASYNC) {
@@ -1012,7 +1014,8 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
         }
         dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA));
         ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN));
-        usb_packet_setup(&ohci->usb_packet, pid, ep, addr, !flag_r);
+        usb_packet_setup(&ohci->usb_packet, pid, ep, addr, !flag_r,
+                         OHCI_BM(td.flags, TD_DI) == 0);
         usb_packet_addbuf(&ohci->usb_packet, ohci->usb_buf, pktlen);
         ret = usb_handle_packet(dev, &ohci->usb_packet);
 #ifdef DEBUG_PACKET
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 953897b..4a1ea6b 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -860,7 +860,8 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
     if (ep_ret) {
         *ep_ret = ep;
     }
-    usb_packet_setup(&async->packet, pid, ep, addr, spd);
+    usb_packet_setup(&async->packet, pid, ep, addr, spd,
+                     (td->ctrl & TD_CTRL_IOC) != 0);
     qemu_sglist_add(&async->sgl, td->buffer, max_len);
     usb_packet_map(&async->packet, &async->sgl);
 
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index ebfc5b8..caa5f3e 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -322,6 +322,7 @@ typedef struct XHCITransfer {
     bool running_retry;
     bool cancelled;
     bool complete;
+    bool int_req;
     unsigned int iso_pkts;
     unsigned int slotid;
     unsigned int epid;
@@ -1292,18 +1293,22 @@ static TRBCCode xhci_set_ep_dequeue(XHCIState *xhci, unsigned int slotid,
     return CC_SUCCESS;
 }
 
-static int xhci_xfer_map(XHCITransfer *xfer)
+static int xhci_xfer_create_sgl(XHCITransfer *xfer, int in_xfer)
 {
-    int in_xfer = (xfer->packet.pid == USB_TOKEN_IN);
     XHCIState *xhci = xfer->xhci;
     int i;
 
+    xfer->int_req = false;
     pci_dma_sglist_init(&xfer->sgl, &xhci->pci_dev, xfer->trb_count);
     for (i = 0; i < xfer->trb_count; i++) {
         XHCITRB *trb = &xfer->trbs[i];
         dma_addr_t addr;
         unsigned int chunk = 0;
 
+        if (trb->control & TRB_TR_IOC) {
+            xfer->int_req = true;
+        }
+
         switch (TRB_TYPE(*trb)) {
         case TR_DATA:
             if ((!(trb->control & TRB_TR_DIR)) != (!in_xfer)) {
@@ -1328,7 +1333,6 @@ static int xhci_xfer_map(XHCITransfer *xfer)
         }
     }
 
-    usb_packet_map(&xfer->packet, &xfer->sgl);
     return 0;
 
 err:
@@ -1446,8 +1450,10 @@ static int xhci_setup_packet(XHCITransfer *xfer)
         ep = usb_ep_get(dev, dir, xfer->epid >> 1);
     }
 
-    usb_packet_setup(&xfer->packet, dir, ep, xfer->trbs[0].addr, false);
-    xhci_xfer_map(xfer);
+    xhci_xfer_create_sgl(xfer, dir == USB_TOKEN_IN); /* Also sets int_req */
+    usb_packet_setup(&xfer->packet, dir, ep, xfer->trbs[0].addr, false,
+                     xfer->int_req);
+    usb_packet_map(&xfer->packet, &xfer->sgl);
     DPRINTF("xhci: setup packet pid 0x%x addr %d ep %d\n",
             xfer->packet.pid, dev->addr, ep->nr);
     return 0;
-- 
1.7.1

[Qemu-devel] [PATCH 15/36] usb: Enforce iso endpoints never returing USB_RET_ASYNC

[Gerd Hoffmann]

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

ehci was already testing for this, and we depend in various places
on no devices doing this, so lets move the check for this to the
usb core.

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

diff --git a/hw/usb/core.c b/hw/usb/core.c
index 87a513f..632a8ef 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -391,6 +391,7 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
     if (QTAILQ_EMPTY(&p->ep->queue) || p->ep->pipeline) {
         ret = usb_process_one(p);
         if (ret == USB_RET_ASYNC) {
+            assert(p->ep->type != USB_ENDPOINT_XFER_ISOC);
             usb_packet_set_state(p, USB_PACKET_ASYNC);
             QTAILQ_INSERT_TAIL(&p->ep->queue, p, queue);
         } else if (ret == USB_RET_ADD_TO_QUEUE) {
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 8ac78ad..f14f9d7 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1667,7 +1667,6 @@ static int ehci_process_itd(EHCIState *ehci,
                                  (itd->transact[i] & ITD_XACT_IOC) != 0);
                 usb_packet_map(&ehci->ipacket, &ehci->isgl);
                 ret = usb_handle_packet(dev, &ehci->ipacket);
-                assert(ret != USB_RET_ASYNC);
                 usb_packet_unmap(&ehci->ipacket, &ehci->isgl);
             } else {
                 DPRINTF("ISOCH: attempt to addess non-iso endpoint\n");
-- 
1.7.1

[Qemu-devel] [PATCH 16/36] uhci: No need to handle async completion of isoc packets

[Gerd Hoffmann]

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

No devices ever return async for isoc endpoints and the core
already enforces this.

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 4a1ea6b..129792b 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -101,7 +101,6 @@ struct UHCIAsync {
     UHCIQueue *queue;
     QTAILQ_ENTRY(UHCIAsync) next;
     uint32_t  td;
-    uint8_t   isoc;
     uint8_t   done;
 };
 
@@ -849,7 +848,6 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
      * for initial isochronous requests
      */
     async->queue->valid = 32;
-    async->isoc = td->ctrl & TD_CTRL_IOS;
 
     max_len = ((td->token >> 21) + 1) & 0x7ff;
     pid = td->token & 0xff;
@@ -911,30 +909,9 @@ static void uhci_async_complete(USBPort *port, USBPacket *packet)
         return;
     }
 
-    if (async->isoc) {
-        UHCI_TD td;
-        uint32_t link = async->td;
-        uint32_t int_mask = 0, val;
-
-        pci_dma_read(&s->dev, link & ~0xf, &td, sizeof(td));
-        le32_to_cpus(&td.link);
-        le32_to_cpus(&td.ctrl);
-        le32_to_cpus(&td.token);
-        le32_to_cpus(&td.buffer);
-
-        uhci_async_unlink(async);
-        uhci_complete_td(s, &td, async, &int_mask);
-        s->pending_int_mask |= int_mask;
-
-        /* update the status bits of the TD */
-        val = cpu_to_le32(td.ctrl);
-        pci_dma_write(&s->dev, (link & ~0xf) + 4, &val, sizeof(val));
-        uhci_async_free(async);
-    } else {
-        async->done = 1;
-        if (s->frame_bytes < s->frame_bandwidth) {
-            qemu_bh_schedule(s->bh);
-        }
+    async->done = 1;
+    if (s->frame_bytes < s->frame_bandwidth) {
+        qemu_bh_schedule(s->bh);
     }
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 17/36] uhci: cleanup: Add an unlink call to uhci_async_cancel()

[Gerd Hoffmann]

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

All callers of uhci_async_cancel() call uhci_async_unlink() first, so
lets move the unlink call to uhci_async_cancel()

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 129792b..82dd5c2 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -232,6 +232,7 @@ static void uhci_async_unlink(UHCIAsync *async)
 
 static void uhci_async_cancel(UHCIAsync *async)
 {
+    uhci_async_unlink(async);
     trace_usb_uhci_packet_cancel(async->queue->token, async->td, async->done);
     if (!async->done)
         usb_cancel_packet(&async->packet);
@@ -266,7 +267,6 @@ static void uhci_async_validate_end(UHCIState *s)
         }
         while (!QTAILQ_EMPTY(&queue->asyncs)) {
             async = QTAILQ_FIRST(&queue->asyncs);
-            uhci_async_unlink(async);
             uhci_async_cancel(async);
         }
         uhci_queue_free(queue);
@@ -284,7 +284,6 @@ static void uhci_async_cancel_device(UHCIState *s, USBDevice *dev)
                 curr->packet.ep->dev != dev) {
                 continue;
             }
-            uhci_async_unlink(curr);
             uhci_async_cancel(curr);
         }
     }
@@ -297,7 +296,6 @@ static void uhci_async_cancel_all(UHCIState *s)
 
     QTAILQ_FOREACH_SAFE(queue, &s->queues, next, nq) {
         QTAILQ_FOREACH_SAFE(curr, &queue->asyncs, next, n) {
-            uhci_async_unlink(curr);
             uhci_async_cancel(curr);
         }
         uhci_queue_free(queue);
-- 
1.7.1

[Qemu-devel] [PATCH 18/36] uhci: Don't retry on error

[Gerd Hoffmann]

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

Since we are either dealing with emulated devices, where retrying is
not going to help, or with redirected devices where the host OS will
have already retried, don't bother retrying on failed transfers.

Also move some common/indentical code out of all the error cases
into the generic error path.

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 82dd5c2..eecd291 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -696,10 +696,6 @@ static USBDevice *uhci_find_device(UHCIState *s, uint8_t addr)
 static void uhci_async_complete(USBPort *port, USBPacket *packet);
 static void uhci_process_frame(UHCIState *s);
 
-/* return -1 if fatal error (frame must be stopped)
-          0 if TD successful
-          1 if TD unsuccessful or inactive
-*/
 static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_t *int_mask)
 {
     int len = 0, max_len, err, ret;
@@ -742,60 +738,40 @@ static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_
 
 out:
     switch(ret) {
+    case USB_RET_NAK:
+        td->ctrl |= TD_CTRL_NAK;
+        return TD_RESULT_NEXT_QH;
+
     case USB_RET_STALL:
         td->ctrl |= TD_CTRL_STALL;
-        td->ctrl &= ~TD_CTRL_ACTIVE;
-        s->status |= UHCI_STS_USBERR;
-        if (td->ctrl & TD_CTRL_IOC) {
-            *int_mask |= 0x01;
-        }
-        uhci_update_irq(s);
         trace_usb_uhci_packet_complete_stall(async->queue->token, async->td);
-        return TD_RESULT_NEXT_QH;
+        err = TD_RESULT_NEXT_QH;
+        break;
 
     case USB_RET_BABBLE:
         td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
-        td->ctrl &= ~TD_CTRL_ACTIVE;
-        s->status |= UHCI_STS_USBERR;
-        if (td->ctrl & TD_CTRL_IOC) {
-            *int_mask |= 0x01;
-        }
-        uhci_update_irq(s);
         /* frame interrupted */
         trace_usb_uhci_packet_complete_babble(async->queue->token, async->td);
-        return TD_RESULT_STOP_FRAME;
-
-    case USB_RET_NAK:
-        td->ctrl |= TD_CTRL_NAK;
-        if (pid == USB_TOKEN_SETUP)
-            break;
-        return TD_RESULT_NEXT_QH;
+        err = TD_RESULT_STOP_FRAME;
+        break;
 
     case USB_RET_IOERROR:
     case USB_RET_NODEV:
     default:
-	break;
+        td->ctrl |= TD_CTRL_TIMEOUT;
+        td->ctrl &= ~(3 << TD_CTRL_ERROR_SHIFT);
+        trace_usb_uhci_packet_complete_error(async->queue->token, async->td);
+        err = TD_RESULT_NEXT_QH;
+        break;
     }
 
-    /* Retry the TD if error count is not zero */
-
-    td->ctrl |= TD_CTRL_TIMEOUT;
-    err = (td->ctrl >> TD_CTRL_ERROR_SHIFT) & 3;
-    if (err != 0) {
-        err--;
-        if (err == 0) {
-            td->ctrl &= ~TD_CTRL_ACTIVE;
-            s->status |= UHCI_STS_USBERR;
-            if (td->ctrl & TD_CTRL_IOC)
-                *int_mask |= 0x01;
-            uhci_update_irq(s);
-            trace_usb_uhci_packet_complete_error(async->queue->token,
-                                                 async->td);
-        }
+    td->ctrl &= ~TD_CTRL_ACTIVE;
+    s->status |= UHCI_STS_USBERR;
+    if (td->ctrl & TD_CTRL_IOC) {
+        *int_mask |= 0x01;
     }
-    td->ctrl = (td->ctrl & ~(3 << TD_CTRL_ERROR_SHIFT)) |
-        (err << TD_CTRL_ERROR_SHIFT);
-    return TD_RESULT_NEXT_QH;
+    uhci_update_irq(s);
+    return err;
 }
 
 static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
-- 
1.7.1

[Qemu-devel] [PATCH 19/36] uhci: Drop unnecessary forward declaration of some static functions

[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/hcd-uhci.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index eecd291..7dfedef 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -693,9 +693,6 @@ static USBDevice *uhci_find_device(UHCIState *s, uint8_t addr)
     return NULL;
 }
 
-static void uhci_async_complete(USBPort *port, USBPacket *packet);
-static void uhci_process_frame(UHCIState *s);
-
 static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_t *int_mask)
 {
     int len = 0, max_len, err, ret;
-- 
1.7.1

[Qemu-devel] [PATCH 20/36] uhci: Move emptying of the queue's asyncs' queue to uhci_queue_free

[Gerd Hoffmann]

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

Cleanup: all callers of uhci_queue_free first unconditionally cancel
all remaining asyncs in the queue, so lets move this to uhci_queue_free().

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 7dfedef..0335f3c 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -160,6 +160,8 @@ typedef struct UHCI_QH {
     uint32_t el_link;
 } UHCI_QH;
 
+static void uhci_async_cancel(UHCIAsync *async);
+
 static inline int32_t uhci_queue_token(UHCI_TD *td)
 {
     /* covers ep, dev, pid -> identifies the endpoint */
@@ -189,6 +191,12 @@ static UHCIQueue *uhci_queue_get(UHCIState *s, UHCI_TD *td)
 static void uhci_queue_free(UHCIQueue *queue)
 {
     UHCIState *s = queue->uhci;
+    UHCIAsync *async;
+
+    while (!QTAILQ_EMPTY(&queue->asyncs)) {
+        async = QTAILQ_FIRST(&queue->asyncs);
+        uhci_async_cancel(async);
+    }
 
     trace_usb_uhci_queue_del(queue->token);
     QTAILQ_REMOVE(&s->queues, queue, next);
@@ -259,17 +267,11 @@ static void uhci_async_validate_begin(UHCIState *s)
 static void uhci_async_validate_end(UHCIState *s)
 {
     UHCIQueue *queue, *n;
-    UHCIAsync *async;
 
     QTAILQ_FOREACH_SAFE(queue, &s->queues, next, n) {
-        if (queue->valid > 0) {
-            continue;
+        if (!queue->valid) {
+            uhci_queue_free(queue);
         }
-        while (!QTAILQ_EMPTY(&queue->asyncs)) {
-            async = QTAILQ_FIRST(&queue->asyncs);
-            uhci_async_cancel(async);
-        }
-        uhci_queue_free(queue);
     }
 }
 
@@ -292,12 +294,8 @@ static void uhci_async_cancel_device(UHCIState *s, USBDevice *dev)
 static void uhci_async_cancel_all(UHCIState *s)
 {
     UHCIQueue *queue, *nq;
-    UHCIAsync *curr, *n;
 
     QTAILQ_FOREACH_SAFE(queue, &s->queues, next, nq) {
-        QTAILQ_FOREACH_SAFE(curr, &queue->asyncs, next, n) {
-            uhci_async_cancel(curr);
-        }
         uhci_queue_free(queue);
     }
 }
-- 
1.7.1

[Qemu-devel] [PATCH 21/36] uhci: Rename UHCIAsync->td to UHCIAsync->td_addr

[Gerd Hoffmann]

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

We use the name td both to refer to a UHCI_TD read from guest memory as
well as to refer to the guest address where a td is stored, switch over
to always use td_addr in the second case for consistency.

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 0335f3c..7589a5b 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -100,7 +100,7 @@ struct UHCIAsync {
     QEMUSGList sgl;
     UHCIQueue *queue;
     QTAILQ_ENTRY(UHCIAsync) next;
-    uint32_t  td;
+    uint32_t  td_addr;
     uint8_t   done;
 };
 
@@ -203,22 +203,22 @@ static void uhci_queue_free(UHCIQueue *queue)
     g_free(queue);
 }
 
-static UHCIAsync *uhci_async_alloc(UHCIQueue *queue, uint32_t addr)
+static UHCIAsync *uhci_async_alloc(UHCIQueue *queue, uint32_t td_addr)
 {
     UHCIAsync *async = g_new0(UHCIAsync, 1);
 
     async->queue = queue;
-    async->td = addr;
+    async->td_addr = td_addr;
     usb_packet_init(&async->packet);
     pci_dma_sglist_init(&async->sgl, &queue->uhci->dev, 1);
-    trace_usb_uhci_packet_add(async->queue->token, async->td);
+    trace_usb_uhci_packet_add(async->queue->token, async->td_addr);
 
     return async;
 }
 
 static void uhci_async_free(UHCIAsync *async)
 {
-    trace_usb_uhci_packet_del(async->queue->token, async->td);
+    trace_usb_uhci_packet_del(async->queue->token, async->td_addr);
     usb_packet_cleanup(&async->packet);
     qemu_sglist_destroy(&async->sgl);
     g_free(async);
@@ -228,20 +228,21 @@ static void uhci_async_link(UHCIAsync *async)
 {
     UHCIQueue *queue = async->queue;
     QTAILQ_INSERT_TAIL(&queue->asyncs, async, next);
-    trace_usb_uhci_packet_link_async(async->queue->token, async->td);
+    trace_usb_uhci_packet_link_async(async->queue->token, async->td_addr);
 }
 
 static void uhci_async_unlink(UHCIAsync *async)
 {
     UHCIQueue *queue = async->queue;
     QTAILQ_REMOVE(&queue->asyncs, async, next);
-    trace_usb_uhci_packet_unlink_async(async->queue->token, async->td);
+    trace_usb_uhci_packet_unlink_async(async->queue->token, async->td_addr);
 }
 
 static void uhci_async_cancel(UHCIAsync *async)
 {
     uhci_async_unlink(async);
-    trace_usb_uhci_packet_cancel(async->queue->token, async->td, async->done);
+    trace_usb_uhci_packet_cancel(async->queue->token, async->td_addr,
+                                 async->done);
     if (!async->done)
         usb_cancel_packet(&async->packet);
     usb_packet_unmap(&async->packet, &async->sgl);
@@ -300,7 +301,8 @@ static void uhci_async_cancel_all(UHCIState *s)
     }
 }
 
-static UHCIAsync *uhci_async_find_td(UHCIState *s, uint32_t addr, UHCI_TD *td)
+static UHCIAsync *uhci_async_find_td(UHCIState *s, uint32_t td_addr,
+                                     UHCI_TD *td)
 {
     uint32_t token = uhci_queue_token(td);
     UHCIQueue *queue;
@@ -316,7 +318,7 @@ static UHCIAsync *uhci_async_find_td(UHCIState *s, uint32_t addr, UHCI_TD *td)
     }
 
     QTAILQ_FOREACH(async, &queue->asyncs, next) {
-        if (async->td == addr) {
+        if (async->td_addr == td_addr) {
             return async;
         }
     }
@@ -722,13 +724,14 @@ static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_
             *int_mask |= 0x02;
             /* short packet: do not update QH */
             trace_usb_uhci_packet_complete_shortxfer(async->queue->token,
-                                                    async->td);
+                                                     async->td_addr);
             return TD_RESULT_NEXT_QH;
         }
     }
 
     /* success */
-    trace_usb_uhci_packet_complete_success(async->queue->token, async->td);
+    trace_usb_uhci_packet_complete_success(async->queue->token,
+                                           async->td_addr);
     return TD_RESULT_COMPLETE;
 
 out:
@@ -739,14 +742,16 @@ out:
 
     case USB_RET_STALL:
         td->ctrl |= TD_CTRL_STALL;
-        trace_usb_uhci_packet_complete_stall(async->queue->token, async->td);
+        trace_usb_uhci_packet_complete_stall(async->queue->token,
+                                             async->td_addr);
         err = TD_RESULT_NEXT_QH;
         break;
 
     case USB_RET_BABBLE:
         td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
         /* frame interrupted */
-        trace_usb_uhci_packet_complete_babble(async->queue->token, async->td);
+        trace_usb_uhci_packet_complete_babble(async->queue->token,
+                                              async->td_addr);
         err = TD_RESULT_STOP_FRAME;
         break;
 
@@ -755,7 +760,8 @@ out:
     default:
         td->ctrl |= TD_CTRL_TIMEOUT;
         td->ctrl &= ~(3 << TD_CTRL_ERROR_SHIFT);
-        trace_usb_uhci_packet_complete_error(async->queue->token, async->td);
+        trace_usb_uhci_packet_complete_error(async->queue->token,
+                                             async->td_addr);
         err = TD_RESULT_NEXT_QH;
         break;
     }
@@ -769,7 +775,7 @@ out:
     return err;
 }
 
-static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
+static int uhci_handle_td(UHCIState *s, uint32_t td_addr, UHCI_TD *td,
                           uint32_t *int_mask, bool queuing,
                           struct USBEndpoint **ep_ret)
 {
@@ -792,7 +798,7 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
         return TD_RESULT_NEXT_QH;
     }
 
-    async = uhci_async_find_td(s, addr, td);
+    async = uhci_async_find_td(s, td_addr, td);
     if (async) {
         /* Already submitted */
         async->queue->valid = 32;
@@ -811,7 +817,7 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
     }
 
     /* Allocate new packet */
-    async = uhci_async_alloc(uhci_queue_get(s, td), addr);
+    async = uhci_async_alloc(uhci_queue_get(s, td), td_addr);
 
     /* valid needs to be large enough to handle 10 frame delay
      * for initial isochronous requests
@@ -827,7 +833,7 @@ static int uhci_handle_td(UHCIState *s, uint32_t addr, UHCI_TD *td,
     if (ep_ret) {
         *ep_ret = ep;
     }
-    usb_packet_setup(&async->packet, pid, ep, addr, spd,
+    usb_packet_setup(&async->packet, pid, ep, td_addr, spd,
                      (td->ctrl & TD_CTRL_IOC) != 0);
     qemu_sglist_add(&async->sgl, td->buffer, max_len);
     usb_packet_map(&async->packet, &async->sgl);
-- 
1.7.1

[Qemu-devel] [PATCH 22/36] uhci: Add uhci_read_td() helper function

[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/hcd-uhci.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 7589a5b..6d2db7f 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -693,6 +693,15 @@ static USBDevice *uhci_find_device(UHCIState *s, uint8_t addr)
     return NULL;
 }
 
+static void uhci_read_td(UHCIState *s, UHCI_TD *td, uint32_t link)
+{
+    pci_dma_read(&s->dev, link & ~0xf, td, sizeof(*td));
+    le32_to_cpus(&td->link);
+    le32_to_cpus(&td->ctrl);
+    le32_to_cpus(&td->token);
+    le32_to_cpus(&td->buffer);
+}
+
 static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_t *int_mask)
 {
     int len = 0, max_len, err, ret;
@@ -941,11 +950,7 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td, struct USBEndpoint *ep)
     int ret;
 
     while (is_valid(plink)) {
-        pci_dma_read(&s->dev, plink & ~0xf, &ptd, sizeof(ptd));
-        le32_to_cpus(&ptd.link);
-        le32_to_cpus(&ptd.ctrl);
-        le32_to_cpus(&ptd.token);
-        le32_to_cpus(&ptd.buffer);
+        uhci_read_td(s, &ptd, plink);
         if (!(ptd.ctrl & TD_CTRL_ACTIVE)) {
             break;
         }
@@ -1031,11 +1036,7 @@ static void uhci_process_frame(UHCIState *s)
         }
 
         /* TD */
-        pci_dma_read(&s->dev, link & ~0xf, &td, sizeof(td));
-        le32_to_cpus(&td.link);
-        le32_to_cpus(&td.ctrl);
-        le32_to_cpus(&td.token);
-        le32_to_cpus(&td.buffer);
+        uhci_read_td(s, &td, link);
         trace_usb_uhci_td_load(curr_qh & ~0xf, link & ~0xf, td.ctrl, td.token);
 
         old_td_ctrl = td.ctrl;
-- 
1.7.1

[Qemu-devel] [PATCH 23/36] uhci: Make uhci_fill_queue() actually operate on an UHCIQueue

[Gerd Hoffmann]

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

And move its calling point to handle_td, this removes the ep_ret ugliness,
and prepates the way for further cleanups in the follow-up patches in this
patch-set.

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 6d2db7f..2bbc6fb 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -161,6 +161,7 @@ typedef struct UHCI_QH {
 } UHCI_QH;
 
 static void uhci_async_cancel(UHCIAsync *async);
+static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td, struct USBEndpoint *ep);
 
 static inline int32_t uhci_queue_token(UHCI_TD *td)
 {
@@ -784,9 +785,8 @@ out:
     return err;
 }
 
-static int uhci_handle_td(UHCIState *s, uint32_t td_addr, UHCI_TD *td,
-                          uint32_t *int_mask, bool queuing,
-                          struct USBEndpoint **ep_ret)
+static int uhci_handle_td(UHCIState *s, UHCIQueue *q,
+                          UHCI_TD *td, uint32_t td_addr, uint32_t *int_mask)
 {
     UHCIAsync *async;
     int len = 0, max_len;
@@ -794,6 +794,7 @@ static int uhci_handle_td(UHCIState *s, uint32_t td_addr, UHCI_TD *td,
     bool spd;
     USBDevice *dev;
     USBEndpoint *ep;
+    bool queuing = (q != NULL);
 
     /* Is active ? */
     if (!(td->ctrl & TD_CTRL_ACTIVE)) {
@@ -826,7 +827,10 @@ static int uhci_handle_td(UHCIState *s, uint32_t td_addr, UHCI_TD *td,
     }
 
     /* Allocate new packet */
-    async = uhci_async_alloc(uhci_queue_get(s, td), td_addr);
+    if (q == NULL) {
+        q = uhci_queue_get(s, td);
+    }
+    async = uhci_async_alloc(q, td_addr);
 
     /* valid needs to be large enough to handle 10 frame delay
      * for initial isochronous requests
@@ -839,9 +843,6 @@ static int uhci_handle_td(UHCIState *s, uint32_t td_addr, UHCI_TD *td,
 
     dev = uhci_find_device(s, (td->token >> 8) & 0x7f);
     ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf);
-    if (ep_ret) {
-        *ep_ret = ep;
-    }
     usb_packet_setup(&async->packet, pid, ep, td_addr, spd,
                      (td->ctrl & TD_CTRL_IOC) != 0);
     qemu_sglist_add(&async->sgl, td->buffer, max_len);
@@ -870,6 +871,9 @@ static int uhci_handle_td(UHCIState *s, uint32_t td_addr, UHCI_TD *td,
  
     if (len == USB_RET_ASYNC) {
         uhci_async_link(async);
+        if (!queuing) {
+            uhci_queue_fill(q, td, ep);
+        }
         return TD_RESULT_ASYNC_START;
     }
 
@@ -941,24 +945,23 @@ static int qhdb_insert(QhDb *db, uint32_t addr)
     return 0;
 }
 
-static void uhci_fill_queue(UHCIState *s, UHCI_TD *td, struct USBEndpoint *ep)
+static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td, struct USBEndpoint *ep)
 {
     uint32_t int_mask = 0;
     uint32_t plink = td->link;
-    uint32_t token = uhci_queue_token(td);
     UHCI_TD ptd;
     int ret;
 
     while (is_valid(plink)) {
-        uhci_read_td(s, &ptd, plink);
+        uhci_read_td(q->uhci, &ptd, plink);
         if (!(ptd.ctrl & TD_CTRL_ACTIVE)) {
             break;
         }
-        if (uhci_queue_token(&ptd) != token) {
+        if (uhci_queue_token(&ptd) != q->token) {
             break;
         }
         trace_usb_uhci_td_queue(plink & ~0xf, ptd.ctrl, ptd.token);
-        ret = uhci_handle_td(s, plink, &ptd, &int_mask, true, NULL);
+        ret = uhci_handle_td(q->uhci, q, &ptd, plink, &int_mask);
         if (ret == TD_RESULT_ASYNC_CONT) {
             break;
         }
@@ -973,7 +976,6 @@ static void uhci_process_frame(UHCIState *s)
 {
     uint32_t frame_addr, link, old_td_ctrl, val, int_mask;
     uint32_t curr_qh, td_count = 0;
-    struct USBEndpoint *curr_ep;
     int cnt, ret;
     UHCI_TD td;
     UHCI_QH qh;
@@ -1040,7 +1042,7 @@ static void uhci_process_frame(UHCIState *s)
         trace_usb_uhci_td_load(curr_qh & ~0xf, link & ~0xf, td.ctrl, td.token);
 
         old_td_ctrl = td.ctrl;
-        ret = uhci_handle_td(s, link, &td, &int_mask, false, &curr_ep);
+        ret = uhci_handle_td(s, NULL, &td, link, &int_mask);
         if (old_td_ctrl != td.ctrl) {
             /* update the status bits of the TD */
             val = cpu_to_le32(td.ctrl);
@@ -1059,7 +1061,6 @@ static void uhci_process_frame(UHCIState *s)
 
         case TD_RESULT_ASYNC_START:
             trace_usb_uhci_td_async(curr_qh & ~0xf, link & ~0xf);
-            uhci_fill_queue(s, &td, curr_ep);
             link = curr_qh ? qh.link : td.link;
             continue;
 
-- 
1.7.1

[Qemu-devel] [PATCH 24/36] uhci: Store ep in UHCIQueue

[Gerd Hoffmann]

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

This avoids the need to repeatedly lookup the device, and ep.

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 2bbc6fb..8214a21 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -107,6 +107,7 @@ struct UHCIAsync {
 struct UHCIQueue {
     uint32_t  token;
     UHCIState *uhci;
+    USBEndpoint *ep;
     QTAILQ_ENTRY(UHCIQueue) next;
     QTAILQ_HEAD(, UHCIAsync) asyncs;
     int8_t    valid;
@@ -161,7 +162,7 @@ typedef struct UHCI_QH {
 } UHCI_QH;
 
 static void uhci_async_cancel(UHCIAsync *async);
-static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td, struct USBEndpoint *ep);
+static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td);
 
 static inline int32_t uhci_queue_token(UHCI_TD *td)
 {
@@ -169,7 +170,7 @@ static inline int32_t uhci_queue_token(UHCI_TD *td)
     return td->token & 0x7ffff;
 }
 
-static UHCIQueue *uhci_queue_get(UHCIState *s, UHCI_TD *td)
+static UHCIQueue *uhci_queue_get(UHCIState *s, UHCI_TD *td, USBEndpoint *ep)
 {
     uint32_t token = uhci_queue_token(td);
     UHCIQueue *queue;
@@ -183,6 +184,7 @@ static UHCIQueue *uhci_queue_get(UHCIState *s, UHCI_TD *td)
     queue = g_new0(UHCIQueue, 1);
     queue->uhci = s;
     queue->token = token;
+    queue->ep = ep;
     QTAILQ_INIT(&queue->asyncs);
     QTAILQ_INSERT_HEAD(&s->queues, queue, next);
     trace_usb_uhci_queue_add(queue->token);
@@ -790,11 +792,9 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q,
 {
     UHCIAsync *async;
     int len = 0, max_len;
-    uint8_t pid;
     bool spd;
-    USBDevice *dev;
-    USBEndpoint *ep;
     bool queuing = (q != NULL);
+    uint8_t pid = td->token & 0xff;
 
     /* Is active ? */
     if (!(td->ctrl & TD_CTRL_ACTIVE)) {
@@ -828,7 +828,9 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q,
 
     /* Allocate new packet */
     if (q == NULL) {
-        q = uhci_queue_get(s, td);
+        USBDevice *dev = uhci_find_device(s, (td->token >> 8) & 0x7f);
+        USBEndpoint *ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf);
+        q = uhci_queue_get(s, td, ep);
     }
     async = uhci_async_alloc(q, td_addr);
 
@@ -838,12 +840,8 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q,
     async->queue->valid = 32;
 
     max_len = ((td->token >> 21) + 1) & 0x7ff;
-    pid = td->token & 0xff;
     spd = (pid == USB_TOKEN_IN && (td->ctrl & TD_CTRL_SPD) != 0);
-
-    dev = uhci_find_device(s, (td->token >> 8) & 0x7f);
-    ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf);
-    usb_packet_setup(&async->packet, pid, ep, td_addr, spd,
+    usb_packet_setup(&async->packet, pid, q->ep, td_addr, spd,
                      (td->ctrl & TD_CTRL_IOC) != 0);
     qemu_sglist_add(&async->sgl, td->buffer, max_len);
     usb_packet_map(&async->packet, &async->sgl);
@@ -851,13 +849,13 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q,
     switch(pid) {
     case USB_TOKEN_OUT:
     case USB_TOKEN_SETUP:
-        len = usb_handle_packet(dev, &async->packet);
+        len = usb_handle_packet(q->ep->dev, &async->packet);
         if (len >= 0)
             len = max_len;
         break;
 
     case USB_TOKEN_IN:
-        len = usb_handle_packet(dev, &async->packet);
+        len = usb_handle_packet(q->ep->dev, &async->packet);
         break;
 
     default:
@@ -872,7 +870,7 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q,
     if (len == USB_RET_ASYNC) {
         uhci_async_link(async);
         if (!queuing) {
-            uhci_queue_fill(q, td, ep);
+            uhci_queue_fill(q, td);
         }
         return TD_RESULT_ASYNC_START;
     }
@@ -945,7 +943,7 @@ static int qhdb_insert(QhDb *db, uint32_t addr)
     return 0;
 }
 
-static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td, struct USBEndpoint *ep)
+static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td)
 {
     uint32_t int_mask = 0;
     uint32_t plink = td->link;
@@ -969,7 +967,7 @@ static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td, struct USBEndpoint *ep)
         assert(int_mask == 0);
         plink = ptd.link;
     }
-    usb_device_flush_ep_queue(ep->dev, ep);
+    usb_device_flush_ep_queue(q->ep->dev, q->ep);
 }
 
 static void uhci_process_frame(UHCIState *s)
-- 
1.7.1

[Qemu-devel] [PATCH 25/36] uhci: Immediately free queues on device disconnect

[Gerd Hoffmann]

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

There is no need to just cancel any in-flight packets, and then wait
for validate-end to clean things up, we can simply clean things up
immediately on device removal.

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 8214a21..a8b74bd 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -281,16 +281,11 @@ static void uhci_async_validate_end(UHCIState *s)
 
 static void uhci_async_cancel_device(UHCIState *s, USBDevice *dev)
 {
-    UHCIQueue *queue;
-    UHCIAsync *curr, *n;
+    UHCIQueue *queue, *n;
 
-    QTAILQ_FOREACH(queue, &s->queues, next) {
-        QTAILQ_FOREACH_SAFE(curr, &queue->asyncs, next, n) {
-            if (!usb_packet_is_inflight(&curr->packet) ||
-                curr->packet.ep->dev != dev) {
-                continue;
-            }
-            uhci_async_cancel(curr);
+    QTAILQ_FOREACH_SAFE(queue, &s->queues, next, n) {
+        if (queue->ep->dev == dev) {
+            uhci_queue_free(queue, "cancel-device");
         }
     }
 }
-- 
1.7.1

[Qemu-devel] [PATCH 26/36] uhci: Verify queue has not been changed by guest

[Gerd Hoffmann]

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

According to the spec a guest can unlink a qh, and then as soon as frindex
has changed by 1 since the unlink, assume it is idle and re-use it. However
for various reasons, we cannot simply consider a qh as unlinked if we've not
seen it for 1 frame. This means that it is possible for a guest to re-use /
restart the queue while we still see its old state. This patch adds a safety
check for this, and "early" retires queues when they were changed by the guest.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-uhci.c |   62 ++++++++++++++++++++++++++++++++++++++--------------
 trace-events      |    2 +-
 2 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index a8b74bd..0984bee 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -105,6 +105,7 @@ struct UHCIAsync {
 };
 
 struct UHCIQueue {
+    uint32_t  qh_addr;
     uint32_t  token;
     UHCIState *uhci;
     USBEndpoint *ep;
@@ -170,20 +171,15 @@ static inline int32_t uhci_queue_token(UHCI_TD *td)
     return td->token & 0x7ffff;
 }
 
-static UHCIQueue *uhci_queue_get(UHCIState *s, UHCI_TD *td, USBEndpoint *ep)
+static UHCIQueue *uhci_queue_new(UHCIState *s, uint32_t qh_addr, UHCI_TD *td,
+                                 USBEndpoint *ep)
 {
-    uint32_t token = uhci_queue_token(td);
     UHCIQueue *queue;
 
-    QTAILQ_FOREACH(queue, &s->queues, next) {
-        if (queue->token == token) {
-            return queue;
-        }
-    }
-
     queue = g_new0(UHCIQueue, 1);
     queue->uhci = s;
-    queue->token = token;
+    queue->qh_addr = qh_addr;
+    queue->token = uhci_queue_token(td);
     queue->ep = ep;
     QTAILQ_INIT(&queue->asyncs);
     QTAILQ_INSERT_HEAD(&s->queues, queue, next);
@@ -191,7 +187,7 @@ static UHCIQueue *uhci_queue_get(UHCIState *s, UHCI_TD *td, USBEndpoint *ep)
     return queue;
 }
 
-static void uhci_queue_free(UHCIQueue *queue)
+static void uhci_queue_free(UHCIQueue *queue, const char *reason)
 {
     UHCIState *s = queue->uhci;
     UHCIAsync *async;
@@ -201,11 +197,35 @@ static void uhci_queue_free(UHCIQueue *queue)
         uhci_async_cancel(async);
     }
 
-    trace_usb_uhci_queue_del(queue->token);
+    trace_usb_uhci_queue_del(queue->token, reason);
     QTAILQ_REMOVE(&s->queues, queue, next);
     g_free(queue);
 }
 
+static UHCIQueue *uhci_queue_find(UHCIState *s, UHCI_TD *td)
+{
+    uint32_t token = uhci_queue_token(td);
+    UHCIQueue *queue;
+
+    QTAILQ_FOREACH(queue, &s->queues, next) {
+        if (queue->token == token) {
+            return queue;
+        }
+    }
+    return NULL;
+}
+
+static bool uhci_queue_verify(UHCIQueue *queue, uint32_t qh_addr, UHCI_TD *td,
+                              uint32_t td_addr, bool queuing)
+{
+    UHCIAsync *first = QTAILQ_FIRST(&queue->asyncs);
+
+    return queue->qh_addr == qh_addr &&
+           queue->token == uhci_queue_token(td) &&
+           (queuing || !(td->ctrl & TD_CTRL_ACTIVE) || first == NULL ||
+            first->td_addr == td_addr);
+}
+
 static UHCIAsync *uhci_async_alloc(UHCIQueue *queue, uint32_t td_addr)
 {
     UHCIAsync *async = g_new0(UHCIAsync, 1);
@@ -274,7 +294,7 @@ static void uhci_async_validate_end(UHCIState *s)
 
     QTAILQ_FOREACH_SAFE(queue, &s->queues, next, n) {
         if (!queue->valid) {
-            uhci_queue_free(queue);
+            uhci_queue_free(queue, "validate-end");
         }
     }
 }
@@ -295,7 +315,7 @@ static void uhci_async_cancel_all(UHCIState *s)
     UHCIQueue *queue, *nq;
 
     QTAILQ_FOREACH_SAFE(queue, &s->queues, next, nq) {
-        uhci_queue_free(queue);
+        uhci_queue_free(queue, "cancel-all");
     }
 }
 
@@ -782,7 +802,7 @@ out:
     return err;
 }
 
-static int uhci_handle_td(UHCIState *s, UHCIQueue *q,
+static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
                           UHCI_TD *td, uint32_t td_addr, uint32_t *int_mask)
 {
     UHCIAsync *async;
@@ -791,6 +811,14 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q,
     bool queuing = (q != NULL);
     uint8_t pid = td->token & 0xff;
 
+    if (q == NULL) {
+        q = uhci_queue_find(s, td);
+        if (q && !uhci_queue_verify(q, qh_addr, td, td_addr, queuing)) {
+            uhci_queue_free(q, "guest re-used qh");
+            q = NULL;
+        }
+    }
+
     /* Is active ? */
     if (!(td->ctrl & TD_CTRL_ACTIVE)) {
         /*
@@ -825,7 +853,7 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q,
     if (q == NULL) {
         USBDevice *dev = uhci_find_device(s, (td->token >> 8) & 0x7f);
         USBEndpoint *ep = usb_ep_get(dev, pid, (td->token >> 15) & 0xf);
-        q = uhci_queue_get(s, td, ep);
+        q = uhci_queue_new(s, qh_addr, td, ep);
     }
     async = uhci_async_alloc(q, td_addr);
 
@@ -954,7 +982,7 @@ static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td)
             break;
         }
         trace_usb_uhci_td_queue(plink & ~0xf, ptd.ctrl, ptd.token);
-        ret = uhci_handle_td(q->uhci, q, &ptd, plink, &int_mask);
+        ret = uhci_handle_td(q->uhci, q, q->qh_addr, &ptd, plink, &int_mask);
         if (ret == TD_RESULT_ASYNC_CONT) {
             break;
         }
@@ -1035,7 +1063,7 @@ static void uhci_process_frame(UHCIState *s)
         trace_usb_uhci_td_load(curr_qh & ~0xf, link & ~0xf, td.ctrl, td.token);
 
         old_td_ctrl = td.ctrl;
-        ret = uhci_handle_td(s, NULL, &td, link, &int_mask);
+        ret = uhci_handle_td(s, NULL, curr_qh, &td, link, &int_mask);
         if (old_td_ctrl != td.ctrl) {
             /* update the status bits of the TD */
             val = cpu_to_le32(td.ctrl);
diff --git a/trace-events b/trace-events
index e2d4580..ed136a5 100644
--- a/trace-events
+++ b/trace-events
@@ -287,7 +287,7 @@ usb_uhci_mmio_writew(uint32_t addr, uint32_t val) "addr 0x%04x, val 0x%04x"
 usb_uhci_mmio_readl(uint32_t addr, uint32_t val) "addr 0x%04x, ret 0x%08x"
 usb_uhci_mmio_writel(uint32_t addr, uint32_t val) "addr 0x%04x, val 0x%08x"
 usb_uhci_queue_add(uint32_t token) "token 0x%x"
-usb_uhci_queue_del(uint32_t token) "token 0x%x"
+usb_uhci_queue_del(uint32_t token, const char *reason) "token 0x%x: %s"
 usb_uhci_packet_add(uint32_t token, uint32_t addr) "token 0x%x, td 0x%x"
 usb_uhci_packet_link_async(uint32_t token, uint32_t addr) "token 0x%x, td 0x%x"
 usb_uhci_packet_unlink_async(uint32_t token, uint32_t addr) "token 0x%x, td 0x%x"
-- 
1.7.1

[Qemu-devel] [PATCH 27/36] uhci: Detect guest td re-use

[Gerd Hoffmann]

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

A td can be reused by the guest in a different queue, before we notice
the original queue has been unlinked. So search for tds by addr only, detect
guest td reuse, and cancel the original queue, this is necessary to keep our
packet ids unique.

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 0984bee..c4f2f98 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -319,28 +319,18 @@ static void uhci_async_cancel_all(UHCIState *s)
     }
 }
 
-static UHCIAsync *uhci_async_find_td(UHCIState *s, uint32_t td_addr,
-                                     UHCI_TD *td)
+static UHCIAsync *uhci_async_find_td(UHCIState *s, uint32_t td_addr)
 {
-    uint32_t token = uhci_queue_token(td);
     UHCIQueue *queue;
     UHCIAsync *async;
 
     QTAILQ_FOREACH(queue, &s->queues, next) {
-        if (queue->token == token) {
-            break;
-        }
-    }
-    if (queue == NULL) {
-        return NULL;
-    }
-
-    QTAILQ_FOREACH(async, &queue->asyncs, next) {
-        if (async->td_addr == td_addr) {
-            return async;
+        QTAILQ_FOREACH(async, &queue->asyncs, next) {
+            if (async->td_addr == td_addr) {
+                return async;
+            }
         }
     }
-
     return NULL;
 }
 
@@ -805,11 +795,21 @@ out:
 static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
                           UHCI_TD *td, uint32_t td_addr, uint32_t *int_mask)
 {
-    UHCIAsync *async;
     int len = 0, max_len;
     bool spd;
     bool queuing = (q != NULL);
     uint8_t pid = td->token & 0xff;
+    UHCIAsync *async = uhci_async_find_td(s, td_addr);
+
+    if (async) {
+        if (uhci_queue_verify(async->queue, qh_addr, td, td_addr, queuing)) {
+            assert(q == NULL || q == async->queue);
+            q = async->queue;
+        } else {
+            uhci_queue_free(async->queue, "guest re-used pending td");
+            async = NULL;
+        }
+    }
 
     if (q == NULL) {
         q = uhci_queue_find(s, td);
@@ -831,7 +831,6 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
         return TD_RESULT_NEXT_QH;
     }
 
-    async = uhci_async_find_td(s, td_addr, td);
     if (async) {
         /* Already submitted */
         async->queue->valid = 32;
-- 
1.7.1

[Qemu-devel] [PATCH 28/36] uhci: When the guest marks a pending td non-active, cancel the queue

[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/hcd-uhci.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index c4f2f98..592ad8d 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -821,6 +821,10 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
 
     /* Is active ? */
     if (!(td->ctrl & TD_CTRL_ACTIVE)) {
+        if (async) {
+            /* Guest marked a pending td non-active, cancel the queue */
+            uhci_queue_free(async->queue, "pending td non-active");
+        }
         /*
          * ehci11d spec page 22: "Even if the Active bit in the TD is already
          * cleared when the TD is fetched ... an IOC interrupt is generated"
-- 
1.7.1

[Qemu-devel] [PATCH 29/36] uhci: Always mark a queue valid when we encounter it

[Gerd Hoffmann]

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

Before this patch we would not mark a queue valid when its head was a
non-active td. This causes us to misbehave in the following scenario:

1) queue with multiple input transfers queued
2) We hit some latency issue, causing qemu to get behind processing frames
3) When qemu gets to run again, it notices the first transfer ends short,
   marking the head td non-active
4) It now processes 32+ frames in a row without giving the guest a chance
   to run since it is behind
5) valid is decreased to 0, causing the queue to get cancelled also cancelling
   already queued up further input transfers
6) guest gets to run, notices the inactive td, cleanups up further tds
   from the short transfer, and lets the queue continue at the first td of
   the next input transfer
7) we re-start the queue, issuing the second input transfer for the *second*
   time, and any data read by the first time we issued it has been lost

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 592ad8d..beeb3fd 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -183,6 +183,9 @@ static UHCIQueue *uhci_queue_new(UHCIState *s, uint32_t qh_addr, UHCI_TD *td,
     queue->ep = ep;
     QTAILQ_INIT(&queue->asyncs);
     QTAILQ_INSERT_HEAD(&s->queues, queue, next);
+    /* valid needs to be large enough to handle 10 frame delay
+     * for initial isochronous requests */
+    queue->valid = 32;
     trace_usb_uhci_queue_add(queue->token);
     return queue;
 }
@@ -819,6 +822,10 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
         }
     }
 
+    if (q) {
+        q->valid = 32;
+    }
+
     /* Is active ? */
     if (!(td->ctrl & TD_CTRL_ACTIVE)) {
         if (async) {
@@ -836,9 +843,6 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
     }
 
     if (async) {
-        /* Already submitted */
-        async->queue->valid = 32;
-
         if (!async->done)
             return TD_RESULT_ASYNC_CONT;
         if (queuing) {
@@ -860,11 +864,6 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
     }
     async = uhci_async_alloc(q, td_addr);
 
-    /* valid needs to be large enough to handle 10 frame delay
-     * for initial isochronous requests
-     */
-    async->queue->valid = 32;
-
     max_len = ((td->token >> 21) + 1) & 0x7ff;
     spd = (pid == USB_TOKEN_IN && (td->ctrl & TD_CTRL_SPD) != 0);
     usb_packet_setup(&async->packet, pid, q->ep, td_addr, spd,
-- 
1.7.1

[Qemu-devel] [PATCH 30/36] uhci: Retry to fill the queue while waiting for td completion

[Gerd Hoffmann]

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

If the guest is using multiple transfers to try and keep the usb bus busy /
used at maximum efficiency, currently we would see / do the following:

1) submit transfer 1 to the device
2) submit transfer 2 to the device
3) report transfer 1 completion to guest
4) report transfer 2 completion to guest
5) submit transfer 1 to the device
6) report transfer 1 completion to guest
7) submit transfer 2 to the device
8) report transfer 2 completion to guest
etc.

So after the initial submission we would effectively only have 1 transfer
in flight, rather then 2. This is caused by us not checking the queue for
addition of new transfers by the guest (ie the resubmission of a recently
finished transfer), while waiting for a pending transfer to complete.
This patch does add a check for this, changing the sequence to:

1) submit transfer 1 to the device
2) submit transfer 2 to the device
3) report transfer 1 completion to guest
4) submit transfer 1 to the device
5) report transfer 2 completion to guest
6) submit transfer 2 to the device
etc.

Thus keeping 2 transfers in flight (most of the time, and always 1),
as intended by the guest.

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index beeb3fd..a9e06ef 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -110,7 +110,7 @@ struct UHCIQueue {
     UHCIState *uhci;
     USBEndpoint *ep;
     QTAILQ_ENTRY(UHCIQueue) next;
-    QTAILQ_HEAD(, UHCIAsync) asyncs;
+    QTAILQ_HEAD(asyncs_head, UHCIAsync) asyncs;
     int8_t    valid;
 };
 
@@ -843,15 +843,25 @@ static int uhci_handle_td(UHCIState *s, UHCIQueue *q, uint32_t qh_addr,
     }
 
     if (async) {
-        if (!async->done)
-            return TD_RESULT_ASYNC_CONT;
         if (queuing) {
             /* we are busy filling the queue, we are not prepared
                to consume completed packages then, just leave them
                in async state */
             return TD_RESULT_ASYNC_CONT;
         }
+        if (!async->done) {
+            UHCI_TD last_td;
+            UHCIAsync *last = QTAILQ_LAST(&async->queue->asyncs, asyncs_head);
+            /*
+             * While we are waiting for the current td to complete, the guest
+             * may have added more tds to the queue. Note we re-read the td
+             * rather then caching it, as we want to see guest made changes!
+             */
+            uhci_read_td(s, &last_td, last->td_addr);
+            uhci_queue_fill(async->queue, &last_td);
 
+            return TD_RESULT_ASYNC_CONT;
+        }
         uhci_async_unlink(async);
         goto done;
     }
-- 
1.7.1

[Qemu-devel] [PATCH 31/36] uhci: Use only one queue for ctrl endpoints

[Gerd Hoffmann]

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

ctrl endpoints use different pids for different phases of a control
transfer, this patch makes us use only one queue for a ctrl ep, rather
then 3.

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

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index a9e06ef..b6b972f 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -167,8 +167,13 @@ static void uhci_queue_fill(UHCIQueue *q, UHCI_TD *td);
 
 static inline int32_t uhci_queue_token(UHCI_TD *td)
 {
-    /* covers ep, dev, pid -> identifies the endpoint */
-    return td->token & 0x7ffff;
+    if ((td->token & (0xf << 15)) == 0) {
+        /* ctrl ep, cover ep and dev, not pid! */
+        return td->token & 0x7ff00;
+    } else {
+        /* covers ep, dev, pid -> identifies the endpoint */
+        return td->token & 0x7ffff;
+    }
 }
 
 static UHCIQueue *uhci_queue_new(UHCIState *s, uint32_t qh_addr, UHCI_TD *td,
-- 
1.7.1

[Qemu-devel] [PATCH 32/36] xhci: fix function name in error message

[Gerd Hoffmann]

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

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index caa5f3e..8345fa3 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2676,7 +2676,7 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
     trace_usb_xhci_runtime_write(reg, val);
 
     if (reg < 0x20) {
-        fprintf(stderr, "xhci_oper_write: reg 0x%x unimplemented\n", (int)reg);
+        fprintf(stderr, "%s: reg 0x%x unimplemented\n", __func__, (int)reg);
         return;
     }
 
-- 
1.7.1

[Qemu-devel] [PATCH 33/36] xhci: flush endpoint context unconditinally

[Gerd Hoffmann]

Not updating the endpoint context in case the state didn't change is
wrong.  Other context fields might have changed, for example the
dequeue pointer in response to a CR_SET_TR_DEQUEUE command.

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

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 8345fa3..d8d1226 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1009,9 +1009,6 @@ static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx,
                               uint32_t state)
 {
     uint32_t ctx[5];
-    if (epctx->state == state) {
-        return;
-    }
 
     pci_dma_read(&xhci->pci_dev, epctx->pctx, ctx, sizeof(ctx));
     ctx[0] &= ~EP_STATE_MASK;
-- 
1.7.1

[Qemu-devel] [PATCH 34/36] xhci: allow disabling interrupters

[Gerd Hoffmann]

For secondary interrupters this is explicitly allowed in the specs.
For the primary interrupter behavior is undefined, lets be friendly
and allow disabling too.

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

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index d8d1226..bd8d4a5 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -964,6 +964,12 @@ static void xhci_er_reset(XHCIState *xhci, int v)
     XHCIInterrupter *intr = &xhci->intr[v];
     XHCIEvRingSeg seg;
 
+    if (intr->erstsz == 0) {
+        /* disabled */
+        intr->er_start = 0;
+        intr->er_size = 0;
+        return;
+    }
     /* cache the (sole) event ring segment location */
     if (intr->erstsz != 1) {
         fprintf(stderr, "xhci: invalid value for ERSTSZ: %d\n", intr->erstsz);
-- 
1.7.1

[Qemu-devel] [PATCH 35/36] xhci: make number of interrupters and slots configurable

[Gerd Hoffmann]

Add properties to tweak the numbers of available interrupters and slots.

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

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index bd8d4a5..25b04cd 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -417,6 +417,8 @@ struct XHCIState {
     /* properties */
     uint32_t numports_2;
     uint32_t numports_3;
+    uint32_t numintrs;
+    uint32_t numslots;
     uint32_t flags;
 
     /* Operational Registers */
@@ -816,8 +818,8 @@ static void xhci_event(XHCIState *xhci, XHCIEvent *event, int v)
     dma_addr_t erdp;
     unsigned int dp_idx;
 
-    if (v >= MAXINTRS) {
-        DPRINTF("intr nr out of range (%d >= %d)\n", v, MAXINTRS);
+    if (v >= xhci->numintrs) {
+        DPRINTF("intr nr out of range (%d >= %d)\n", v, xhci->numintrs);
         return;
     }
     intr = &xhci->intr[v];
@@ -1043,7 +1045,7 @@ static TRBCCode xhci_enable_ep(XHCIState *xhci, unsigned int slotid,
     int i;
 
     trace_usb_xhci_ep_enable(slotid, epid);
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
     assert(epid >= 1 && epid <= 31);
 
     slot = &xhci->slots[slotid-1];
@@ -1121,7 +1123,7 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid,
     XHCISlot *slot;
     XHCIEPContext *epctx;
     int i, xferi, killed = 0;
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
     assert(epid >= 1 && epid <= 31);
 
     DPRINTF("xhci_ep_nuke_xfers(%d, %d)\n", slotid, epid);
@@ -1149,7 +1151,7 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid,
     XHCIEPContext *epctx;
 
     trace_usb_xhci_ep_disable(slotid, epid);
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
     assert(epid >= 1 && epid <= 31);
 
     slot = &xhci->slots[slotid-1];
@@ -1179,7 +1181,7 @@ static TRBCCode xhci_stop_ep(XHCIState *xhci, unsigned int slotid,
     XHCIEPContext *epctx;
 
     trace_usb_xhci_ep_stop(slotid, epid);
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
 
     if (epid < 1 || epid > 31) {
         fprintf(stderr, "xhci: bad ep %d\n", epid);
@@ -1213,7 +1215,7 @@ static TRBCCode xhci_reset_ep(XHCIState *xhci, unsigned int slotid,
     USBDevice *dev;
 
     trace_usb_xhci_ep_reset(slotid, epid);
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
 
     if (epid < 1 || epid > 31) {
         fprintf(stderr, "xhci: bad ep %d\n", epid);
@@ -1263,7 +1265,7 @@ static TRBCCode xhci_set_ep_dequeue(XHCIState *xhci, unsigned int slotid,
     XHCIEPContext *epctx;
     dma_addr_t dequeue;
 
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
 
     if (epid < 1 || epid > 31) {
         fprintf(stderr, "xhci: bad ep %d\n", epid);
@@ -1667,7 +1669,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid
     int i;
 
     trace_usb_xhci_ep_kick(slotid, epid);
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
     assert(epid >= 1 && epid <= 31);
 
     if (!xhci->slots[slotid-1].enabled) {
@@ -1787,7 +1789,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid
 static TRBCCode xhci_enable_slot(XHCIState *xhci, unsigned int slotid)
 {
     trace_usb_xhci_slot_enable(slotid);
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
     xhci->slots[slotid-1].enabled = 1;
     xhci->slots[slotid-1].uport = NULL;
     memset(xhci->slots[slotid-1].eps, 0, sizeof(XHCIEPContext*)*31);
@@ -1800,7 +1802,7 @@ static TRBCCode xhci_disable_slot(XHCIState *xhci, unsigned int slotid)
     int i;
 
     trace_usb_xhci_slot_disable(slotid);
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
 
     for (i = 1; i <= 31; i++) {
         if (xhci->slots[slotid-1].eps[i-1]) {
@@ -1852,7 +1854,7 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid,
     TRBCCode res;
 
     trace_usb_xhci_slot_address(slotid);
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
 
     dcbaap = xhci_addr64(xhci->dcbaap_low, xhci->dcbaap_high);
     pci_dma_read(&xhci->pci_dev, dcbaap + 8*slotid, &poctx, sizeof(poctx));
@@ -1891,7 +1893,7 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid,
         return CC_USB_TRANSACTION_ERROR;
     }
 
-    for (i = 0; i < MAXSLOTS; i++) {
+    for (i = 0; i < xhci->numslots; i++) {
         if (xhci->slots[i].uport == uport) {
             fprintf(stderr, "xhci: port %s already assigned to slot %d\n",
                     uport->path, i+1);
@@ -1940,7 +1942,7 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid,
     TRBCCode res;
 
     trace_usb_xhci_slot_configure(slotid);
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
 
     ictx = xhci_mask64(pictx);
     octx = xhci->slots[slotid-1].ctx;
@@ -2028,7 +2030,7 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid,
     uint32_t slot_ctx[4];
 
     trace_usb_xhci_slot_evaluate(slotid);
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
 
     ictx = xhci_mask64(pictx);
     octx = xhci->slots[slotid-1].ctx;
@@ -2091,7 +2093,7 @@ static TRBCCode xhci_reset_slot(XHCIState *xhci, unsigned int slotid)
     int i;
 
     trace_usb_xhci_slot_reset(slotid);
-    assert(slotid >= 1 && slotid <= MAXSLOTS);
+    assert(slotid >= 1 && slotid <= xhci->numslots);
 
     octx = xhci->slots[slotid-1].ctx;
 
@@ -2117,7 +2119,7 @@ static unsigned int xhci_get_slot(XHCIState *xhci, XHCIEvent *event, XHCITRB *tr
 {
     unsigned int slotid;
     slotid = (trb->control >> TRB_CR_SLOTID_SHIFT) & TRB_CR_SLOTID_MASK;
-    if (slotid < 1 || slotid > MAXSLOTS) {
+    if (slotid < 1 || slotid > xhci->numslots) {
         fprintf(stderr, "xhci: bad slot id %d\n", slotid);
         event->ccode = CC_TRB_ERROR;
         return 0;
@@ -2209,12 +2211,12 @@ static void xhci_process_commands(XHCIState *xhci)
         event.ptr = addr;
         switch (type) {
         case CR_ENABLE_SLOT:
-            for (i = 0; i < MAXSLOTS; i++) {
+            for (i = 0; i < xhci->numslots; i++) {
                 if (!xhci->slots[i].enabled) {
                     break;
                 }
             }
-            if (i >= MAXSLOTS) {
+            if (i >= xhci->numslots) {
                 fprintf(stderr, "xhci: no device slots available\n");
                 event.ccode = CC_NO_SLOTS_ERROR;
             } else {
@@ -2361,7 +2363,7 @@ static void xhci_reset(DeviceState *dev)
     xhci->config = 0;
     xhci->devaddr = 2;
 
-    for (i = 0; i < MAXSLOTS; i++) {
+    for (i = 0; i < xhci->numslots; i++) {
         xhci_disable_slot(xhci, i+1);
     }
 
@@ -2369,7 +2371,7 @@ static void xhci_reset(DeviceState *dev)
         xhci_update_port(xhci, xhci->ports + i, 0);
     }
 
-    for (i = 0; i < MAXINTRS; i++) {
+    for (i = 0; i < xhci->numintrs; i++) {
         xhci->intr[i].iman = 0;
         xhci->intr[i].imod = 0;
         xhci->intr[i].erstsz = 0;
@@ -2401,7 +2403,7 @@ static uint64_t xhci_cap_read(void *ptr, hwaddr reg, unsigned size)
         break;
     case 0x04: /* HCSPARAMS 1 */
         ret = ((xhci->numports_2+xhci->numports_3)<<24)
-            | (MAXINTRS<<8) | MAXSLOTS;
+            | (xhci->numintrs<<8) | xhci->numslots;
         break;
     case 0x08: /* HCSPARAMS 2 */
         ret = 0x0000000f;
@@ -2756,7 +2758,7 @@ static void xhci_doorbell_write(void *ptr, hwaddr reg,
                     (uint32_t)val);
         }
     } else {
-        if (reg > MAXSLOTS) {
+        if (reg > xhci->numslots) {
             fprintf(stderr, "xhci: bad doorbell %d\n", (int)reg);
         } else if (val > 31) {
             fprintf(stderr, "xhci: bad doorbell %d write: 0x%x\n",
@@ -2862,7 +2864,7 @@ static void xhci_child_detach(USBPort *uport, USBDevice *child)
     XHCIState *xhci = container_of(bus, XHCIState, bus);
     int i;
 
-    for (i = 0; i < MAXSLOTS; i++) {
+    for (i = 0; i < xhci->numslots; i++) {
         if (xhci->slots[i].uport == uport) {
             xhci->slots[i].uport = NULL;
         }
@@ -2882,7 +2884,7 @@ static int xhci_find_slotid(XHCIState *xhci, USBDevice *dev)
     XHCISlot *slot;
     int slotid;
 
-    for (slotid = 1; slotid <= MAXSLOTS; slotid++) {
+    for (slotid = 1; slotid <= xhci->numslots; slotid++) {
         slot = &xhci->slots[slotid-1];
         if (slot->devaddr == dev->addr) {
             return slotid;
@@ -2978,6 +2980,19 @@ static int usb_xhci_initfn(struct PCIDevice *dev)
 
     usb_xhci_init(xhci, &dev->qdev);
 
+    if (xhci->numintrs > MAXINTRS) {
+        xhci->numintrs = MAXINTRS;
+    }
+    if (xhci->numintrs < 1) {
+        xhci->numintrs = 1;
+    }
+    if (xhci->numslots > MAXSLOTS) {
+        xhci->numslots = MAXSLOTS;
+    }
+    if (xhci->numslots < 1) {
+        xhci->numslots = 1;
+    }
+
     xhci->mfwrap_timer = qemu_new_timer_ns(vm_clock, xhci_mfwrap_timer, xhci);
 
     xhci->irq = xhci->pci_dev.irq[0];
@@ -3014,10 +3029,10 @@ static int usb_xhci_initfn(struct PCIDevice *dev)
     assert(ret >= 0);
 
     if (xhci->flags & (1 << XHCI_FLAG_USE_MSI)) {
-        msi_init(&xhci->pci_dev, 0x70, MAXINTRS, true, false);
+        msi_init(&xhci->pci_dev, 0x70, xhci->numintrs, true, false);
     }
     if (xhci->flags & (1 << XHCI_FLAG_USE_MSI_X)) {
-        msix_init(&xhci->pci_dev, MAXINTRS,
+        msix_init(&xhci->pci_dev, xhci->numintrs,
                   &xhci->mem, 0, OFF_MSIX_TABLE,
                   &xhci->mem, 0, OFF_MSIX_PBA,
                   0x90);
@@ -3032,10 +3047,12 @@ static const VMStateDescription vmstate_xhci = {
 };
 
 static Property xhci_properties[] = {
-    DEFINE_PROP_BIT("msi",    XHCIState, flags, XHCI_FLAG_USE_MSI, true),
-    DEFINE_PROP_BIT("msix",   XHCIState, flags, XHCI_FLAG_USE_MSI_X, true),
-    DEFINE_PROP_UINT32("p2",  XHCIState, numports_2, 4),
-    DEFINE_PROP_UINT32("p3",  XHCIState, numports_3, 4),
+    DEFINE_PROP_BIT("msi",      XHCIState, flags, XHCI_FLAG_USE_MSI, true),
+    DEFINE_PROP_BIT("msix",     XHCIState, flags, XHCI_FLAG_USE_MSI_X, true),
+    DEFINE_PROP_UINT32("intrs", XHCIState, numintrs, MAXINTRS),
+    DEFINE_PROP_UINT32("slots", XHCIState, numslots, MAXSLOTS),
+    DEFINE_PROP_UINT32("p2",    XHCIState, numports_2, 4),
+    DEFINE_PROP_UINT32("p3",    XHCIState, numports_3, 4),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.7.1

[Qemu-devel] [PATCH 36/36] xhci: fix usb name in caps

[Gerd Hoffmann]

Used to be "UTB" not "USB".

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

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 25b04cd..7b65741 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2430,7 +2430,7 @@ static uint64_t xhci_cap_read(void *ptr, hwaddr reg, unsigned size)
         ret = 0x02000402; /* USB 2.0 */
         break;
     case 0x24: /* Supported Protocol:04 */
-        ret = 0x20425455; /* "USB " */
+        ret = 0x20425355; /* "USB " */
         break;
     case 0x28: /* Supported Protocol:08 */
         ret = 0x00000001 | (xhci->numports_2<<8);
@@ -2442,7 +2442,7 @@ static uint64_t xhci_cap_read(void *ptr, hwaddr reg, unsigned size)
         ret = 0x03000002; /* USB 3.0 */
         break;
     case 0x34: /* Supported Protocol:04 */
-        ret = 0x20425455; /* "USB " */
+        ret = 0x20425355; /* "USB " */
         break;
     case 0x38: /* Supported Protocol:08 */
         ret = 0x00000000 | (xhci->numports_2+1) | (xhci->numports_3<<8);
-- 
1.7.1

Re: [Qemu-devel] [PULL 00/36] usb patch queue

[Anthony Liguori]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
> Here comes the usb patch queue.  Nothing big standing out.  Tons of
> cleanups and small bug fixes.  Some performance improvements too.
> Some patches preparing the usb core for the upcoming input pipelining
> bits.
>
> please pull,
>   Gerd
>

Pulled. Thanks.

Regards,

Anthony Liguori

> The following changes since commit a8170e5e97ad17ca169c64ba87ae2f53850dab4c:
>
>   Rename target_phys_addr_t to hwaddr (2012-10-23 08:58:25 -0500)
>
> are available in the git repository at:
>   git://git.kraxel.org/qemu usb.68
>
> Gerd Hoffmann (5):
>       xhci: fix function name in error message
>       xhci: flush endpoint context unconditinally
>       xhci: allow disabling interrupters
>       xhci: make number of interrupters and slots configurable
>       xhci: fix usb name in caps
>
> Hans de Goede (31):
>       uhci: Properly unmap packets on cancel / invalid pid
>       uhci: Move checks to continue queuing to uhci_fill_queue()
>       ehci: Get rid of packet tbytes field
>       ehci: Set int flag on a short input packet
>       ehci: Improve latency of interrupt delivery and async schedule scanning
>       ehci: Speed up the timer of raising int from the async schedule
>       ehci: Detect going in circles when filling the queue
>       ehci: Retry to fill the queue while waiting for td completion
>       xhci: Add a xhci_ep_nuke_one_xfer helper function
>       usb: Rename __usb_packet_complete to usb_packet_complete_one
>       usb: Add USB_RET_ADD_TO_QUEUE packet result code
>       usb: Move clearing of queue on halt to the core
>       usb: Move short-not-ok handling to the core
>       usb: Add an int_req flag to USBPacket
>       usb: Enforce iso endpoints never returing USB_RET_ASYNC
>       uhci: No need to handle async completion of isoc packets
>       uhci: cleanup: Add an unlink call to uhci_async_cancel()
>       uhci: Don't retry on error
>       uhci: Drop unnecessary forward declaration of some static functions
>       uhci: Move emptying of the queue's asyncs' queue to uhci_queue_free
>       uhci: Rename UHCIAsync->td to UHCIAsync->td_addr
>       uhci: Add uhci_read_td() helper function
>       uhci: Make uhci_fill_queue() actually operate on an UHCIQueue
>       uhci: Store ep in UHCIQueue
>       uhci: Immediately free queues on device disconnect
>       uhci: Verify queue has not been changed by guest
>       uhci: Detect guest td re-use
>       uhci: When the guest marks a pending td non-active, cancel the queue
>       uhci: Always mark a queue valid when we encounter it
>       uhci: Retry to fill the queue while waiting for td completion
>       uhci: Use only one queue for ctrl endpoints
>
>  hw/usb.h            |   28 +++-
>  hw/usb/bus.c        |    8 +
>  hw/usb/core.c       |   28 +++-
>  hw/usb/hcd-ehci.c   |  105 ++++++++------
>  hw/usb/hcd-musb.c   |    3 +-
>  hw/usb/hcd-ohci.c   |    9 +-
>  hw/usb/hcd-uhci.c   |  380 +++++++++++++++++++++++++--------------------------
>  hw/usb/hcd-xhci.c   |  169 +++++++++++++++--------
>  hw/usb/host-linux.c |    3 +-
>  hw/usb/redirect.c   |   18 ++-
>  trace-events        |    2 +-
>  11 files changed, 425 insertions(+), 328 deletions(-)