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

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

[Gerd Hoffmann]

  Hi,

This is the usb patch queue, again in bugfixing mode.  xhci transfers
are allocated dynamically now, the static allocations (and limit) didn't
play nicely with stream endpoints.  Some xhci cleanups along the way.
Some bugfixes picked up from the list.

please pull,
  Gerd

The following changes since commit 6b39b06339ee59559b31f860d4af635b046322df:

  build: Work around SIZE_MAX bug in OSX headers (2016-10-11 19:22:20 +0100)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/pull-usb-20161012-1

for you to fetch changes up to d5c42857d6b0c35028897df8dfc3749eba6f6de3:

  usb-redir: allocate buffers before waking up the host adapter (2016-10-12 14:37:24 +0200)

----------------------------------------------------------------
various usb bugfixes
some xhci cleanups

----------------------------------------------------------------
Gerd Hoffmann (9):
      xhci: limit the number of link trbs we are willing to process
      xhci: decouple EV_QUEUE from TD_QUEUE
      xhci: drop unused comp_xfer field
      xhci: use linked list for transfers
      xhci: drop XHCITransfer->xhci
      xhci: add & use xhci_kick_epctx()
      xhci: drop XHCITransfer->{slotid,epid}
      xhci: make xhci_epid_to_usbep accept XHCIEPContext
      usb: fix serial generator

Hans de Goede (1):
      usb-redir: allocate buffers before waking up the host adapter

Vijay Kumar B (1):
      usb: Fix incorrect default DMA offset.

 hw/usb/desc.c     |  12 +--
 hw/usb/hcd-ohci.c |   2 +-
 hw/usb/hcd-xhci.c | 229 ++++++++++++++++++++++++++++++------------------------
 hw/usb/redirect.c |  10 ++-
 4 files changed, 142 insertions(+), 111 deletions(-)

[Qemu-devel] [PULL 01/11] xhci: limit the number of link trbs we are willing to process

[Gerd Hoffmann]

Needed to avoid we run in circles forever in case the guest builds
an endless loop with link trbs.

Reported-by: Li Qiang <liqiang6-s@360.cn>
Tested-by: P J P <ppandit@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1476096382-7981-1-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 726435c..ee4fa48 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -54,6 +54,8 @@
  * to the specs when it gets them */
 #define ER_FULL_HACK
 
+#define TRB_LINK_LIMIT  4
+
 #define LEN_CAP         0x40
 #define LEN_OPER        (0x400 + 0x10 * MAXPORTS)
 #define LEN_RUNTIME     ((MAXINTRS + 1) * 0x20)
@@ -1000,6 +1002,7 @@ static TRBType xhci_ring_fetch(XHCIState *xhci, XHCIRing *ring, XHCITRB *trb,
                                dma_addr_t *addr)
 {
     PCIDevice *pci_dev = PCI_DEVICE(xhci);
+    uint32_t link_cnt = 0;
 
     while (1) {
         TRBType type;
@@ -1026,6 +1029,9 @@ static TRBType xhci_ring_fetch(XHCIState *xhci, XHCIRing *ring, XHCITRB *trb,
             ring->dequeue += TRB_SIZE;
             return type;
         } else {
+            if (++link_cnt > TRB_LINK_LIMIT) {
+                return 0;
+            }
             ring->dequeue = xhci_mask64(trb->parameter);
             if (trb->control & TRB_LK_TC) {
                 ring->ccs = !ring->ccs;
@@ -1043,6 +1049,7 @@ static int xhci_ring_chain_length(XHCIState *xhci, const XHCIRing *ring)
     bool ccs = ring->ccs;
     /* hack to bundle together the two/three TDs that make a setup transfer */
     bool control_td_set = 0;
+    uint32_t link_cnt = 0;
 
     while (1) {
         TRBType type;
@@ -1058,6 +1065,9 @@ static int xhci_ring_chain_length(XHCIState *xhci, const XHCIRing *ring)
         type = TRB_TYPE(trb);
 
         if (type == TR_LINK) {
+            if (++link_cnt > TRB_LINK_LIMIT) {
+                return -length;
+            }
             dequeue = xhci_mask64(trb.parameter);
             if (trb.control & TRB_LK_TC) {
                 ccs = !ccs;
-- 
1.8.3.1

[Qemu-devel] [PULL 02/11] xhci: decouple EV_QUEUE from TD_QUEUE

[Gerd Hoffmann]

EV_QUEUE must not change because an array of that size is part of live
migration data.  Hard-code current value there, so we can touch TD_QUEUE
without breaking live migration.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1474965172-30321-3-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index ee4fa48..d9ac1b4 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -49,7 +49,7 @@
 #define TD_QUEUE 24
 
 /* Very pessimistic, let's hope it's enough for all cases */
-#define EV_QUEUE (((3*TD_QUEUE)+16)*MAXSLOTS)
+#define EV_QUEUE (((3 * 24) + 16) * MAXSLOTS)
 /* Do not deliver ER Full events. NEC's driver does some things not bound
  * to the specs when it gets them */
 #define ER_FULL_HACK
-- 
1.8.3.1

[Qemu-devel] [PULL 03/11] xhci: drop unused comp_xfer field

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1474965172-30321-4-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index d9ac1b4..3a035c8 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -386,7 +386,6 @@ struct XHCIEPContext {
 
     XHCIRing ring;
     unsigned int next_xfer;
-    unsigned int comp_xfer;
     XHCITransfer transfers[TD_QUEUE];
     XHCITransfer *retry;
     EPType type;
-- 
1.8.3.1

[Qemu-devel] [PULL 04/11] xhci: use linked list for transfers

[Gerd Hoffmann]

xhci has a fixed number of 24 (TD_QUEUE) XHCITransfer structs per
endpoint, which turns out to be a problem for usb3 devices with 32 (or
more) bulk streams.  xhci re-checks the trb rings on every finished
transfer to make sure it'll pick up any pending work.  But that scheme
breaks in case the first transfer of a ring can't be started because we
ran out of XHCITransfer structs already.

So remove static XHCITransfer array from XHCIEPContext.  Use a linked
list instead, and allocate/free XHCITransfer as needed.  Add helper
functions to allocate & initialize and to cleanup & release
XHCITransfer structs.  That also simplifies trb management, we never
have to realloc XHCITransfer->trbs because we don't reuse XHCITransfer
structs any more.

New dynamic limit for in-flight xhci transfers per endpoint is
number-of-streams + 16.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1474965172-30321-5-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 122 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 68 insertions(+), 54 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 3a035c8..e14c2a8 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "qemu/timer.h"
+#include "qemu/queue.h"
 #include "hw/usb.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/msi.h"
@@ -46,8 +47,6 @@
 #define MAXSLOTS 64
 #define MAXINTRS 16
 
-#define TD_QUEUE 24
-
 /* Very pessimistic, let's hope it's enough for all cases */
 #define EV_QUEUE (((3 * 24) + 16) * MAXSLOTS)
 /* Do not deliver ER Full events. NEC's driver does some things not bound
@@ -346,6 +345,7 @@ typedef struct XHCIPort {
 
 typedef struct XHCITransfer {
     XHCIState *xhci;
+    XHCIEPContext *epctx;
     USBPacket packet;
     QEMUSGList sgl;
     bool running_async;
@@ -361,7 +361,6 @@ typedef struct XHCITransfer {
     bool timed_xfer;
 
     unsigned int trb_count;
-    unsigned int trb_alloced;
     XHCITRB *trbs;
 
     TRBCCode status;
@@ -371,6 +370,8 @@ typedef struct XHCITransfer {
     unsigned int cur_pkt;
 
     uint64_t mfindex_kick;
+
+    QTAILQ_ENTRY(XHCITransfer) next;
 } XHCITransfer;
 
 struct XHCIStreamContext {
@@ -385,8 +386,8 @@ struct XHCIEPContext {
     unsigned int epid;
 
     XHCIRing ring;
-    unsigned int next_xfer;
-    XHCITransfer transfers[TD_QUEUE];
+    uint32_t xfer_count;
+    QTAILQ_HEAD(, XHCITransfer) transfers;
     XHCITransfer *retry;
     EPType type;
     dma_addr_t pctx;
@@ -1370,19 +1371,13 @@ static XHCIEPContext *xhci_alloc_epctx(XHCIState *xhci,
                                        unsigned int epid)
 {
     XHCIEPContext *epctx;
-    int i;
 
     epctx = g_new0(XHCIEPContext, 1);
     epctx->xhci = xhci;
     epctx->slotid = slotid;
     epctx->epid = epid;
 
-    for (i = 0; i < ARRAY_SIZE(epctx->transfers); i++) {
-        epctx->transfers[i].xhci = xhci;
-        epctx->transfers[i].slotid = slotid;
-        epctx->transfers[i].epid = epid;
-        usb_packet_init(&epctx->transfers[i].packet);
-    }
+    QTAILQ_INIT(&epctx->transfers);
     epctx->kick_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_ep_kick_timer, epctx);
 
     return epctx;
@@ -1443,6 +1438,41 @@ static TRBCCode xhci_enable_ep(XHCIState *xhci, unsigned int slotid,
     return CC_SUCCESS;
 }
 
+static XHCITransfer *xhci_ep_alloc_xfer(XHCIEPContext *epctx,
+                                        uint32_t length)
+{
+    uint32_t limit = epctx->nr_pstreams + 16;
+    XHCITransfer *xfer;
+
+    if (epctx->xfer_count >= limit) {
+        return NULL;
+    }
+
+    xfer = g_new0(XHCITransfer, 1);
+    xfer->xhci = epctx->xhci;
+    xfer->epctx = epctx;
+    xfer->slotid = epctx->slotid;
+    xfer->epid = epctx->epid;
+    xfer->trbs = g_new(XHCITRB, length);
+    xfer->trb_count = length;
+    usb_packet_init(&xfer->packet);
+
+    QTAILQ_INSERT_TAIL(&epctx->transfers, xfer, next);
+    epctx->xfer_count++;
+
+    return xfer;
+}
+
+static void xhci_ep_free_xfer(XHCITransfer *xfer)
+{
+    QTAILQ_REMOVE(&xfer->epctx->transfers, xfer, next);
+    xfer->epctx->xfer_count--;
+
+    usb_packet_cleanup(&xfer->packet);
+    g_free(xfer->trbs);
+    g_free(xfer);
+}
+
 static int xhci_ep_nuke_one_xfer(XHCITransfer *t, TRBCCode report)
 {
     int killed = 0;
@@ -1469,7 +1499,7 @@ static int xhci_ep_nuke_one_xfer(XHCITransfer *t, TRBCCode report)
     g_free(t->trbs);
 
     t->trbs = NULL;
-    t->trb_count = t->trb_alloced = 0;
+    t->trb_count = 0;
 
     return killed;
 }
@@ -1479,7 +1509,8 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid,
 {
     XHCISlot *slot;
     XHCIEPContext *epctx;
-    int i, xferi, killed = 0;
+    XHCITransfer *xfer;
+    int killed = 0;
     USBEndpoint *ep = NULL;
     assert(slotid >= 1 && slotid <= xhci->numslots);
     assert(epid >= 1 && epid <= 31);
@@ -1494,14 +1525,16 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid,
 
     epctx = slot->eps[epid-1];
 
-    xferi = epctx->next_xfer;
-    for (i = 0; i < TD_QUEUE; i++) {
-        killed += xhci_ep_nuke_one_xfer(&epctx->transfers[xferi], report);
+    for (;;) {
+        xfer = QTAILQ_FIRST(&epctx->transfers);
+        if (xfer == NULL) {
+            break;
+        }
+        killed += xhci_ep_nuke_one_xfer(xfer, report);
         if (killed) {
             report = 0; /* Only report once */
         }
-        epctx->transfers[xferi].packet.ep = NULL;
-        xferi = (xferi + 1) % TD_QUEUE;
+        xhci_ep_free_xfer(xfer);
     }
 
     ep = xhci_epid_to_usbep(xhci, slotid, epid);
@@ -1516,7 +1549,6 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid,
 {
     XHCISlot *slot;
     XHCIEPContext *epctx;
-    int i;
 
     trace_usb_xhci_ep_disable(slotid, epid);
     assert(slotid >= 1 && slotid <= xhci->numslots);
@@ -1537,10 +1569,6 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid,
         xhci_free_streams(epctx);
     }
 
-    for (i = 0; i < ARRAY_SIZE(epctx->transfers); i++) {
-        usb_packet_cleanup(&epctx->transfers[i].packet);
-    }
-
     /* only touch guest RAM if we're not resetting the HC */
     if (xhci->dcbaap_low || xhci->dcbaap_high) {
         xhci_set_ep_state(xhci, epctx, NULL, EP_DISABLED);
@@ -2104,6 +2132,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
 {
     XHCIStreamContext *stctx;
     XHCIEPContext *epctx;
+    XHCITransfer *xfer;
     XHCIRing *ring;
     USBEndpoint *ep = NULL;
     uint64_t mfindex;
@@ -2168,6 +2197,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
             xhci_complete_packet(xfer);
         }
         assert(!xfer->running_retry);
+        xhci_ep_free_xfer(epctx->retry);
         epctx->retry = NULL;
     }
 
@@ -2193,27 +2223,14 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
     assert(ring->dequeue != 0);
 
     while (1) {
-        XHCITransfer *xfer = &epctx->transfers[epctx->next_xfer];
-        if (xfer->running_async || xfer->running_retry) {
-            break;
-        }
         length = xhci_ring_chain_length(xhci, ring);
-        if (length < 0) {
+        if (length <= 0) {
             break;
-        } else if (length == 0) {
-            break;
-        }
-        if (xfer->trbs && xfer->trb_alloced < length) {
-            xfer->trb_count = 0;
-            xfer->trb_alloced = 0;
-            g_free(xfer->trbs);
-            xfer->trbs = NULL;
         }
-        if (!xfer->trbs) {
-            xfer->trbs = g_new(XHCITRB, length);
-            xfer->trb_alloced = length;
+        xfer = xhci_ep_alloc_xfer(epctx, length);
+        if (xfer == NULL) {
+            break;
         }
-        xfer->trb_count = length;
 
         for (i = 0; i < length; i++) {
             TRBType type;
@@ -2223,25 +2240,19 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
         xfer->streamid = streamid;
 
         if (epid == 1) {
-            if (xhci_fire_ctl_transfer(xhci, xfer) >= 0) {
-                epctx->next_xfer = (epctx->next_xfer + 1) % TD_QUEUE;
-            } else {
-                DPRINTF("xhci: error firing CTL transfer\n");
-            }
+            xhci_fire_ctl_transfer(xhci, xfer);
         } else {
-            if (xhci_fire_transfer(xhci, xfer, epctx) >= 0) {
-                epctx->next_xfer = (epctx->next_xfer + 1) % TD_QUEUE;
-            } else {
-                if (!xfer->timed_xfer) {
-                    DPRINTF("xhci: error firing data transfer\n");
-                }
-            }
+            xhci_fire_transfer(xhci, xfer, epctx);
+        }
+        if (xfer->complete) {
+            xhci_ep_free_xfer(xfer);
+            xfer = NULL;
         }
 
         if (epctx->state == EP_HALTED) {
             break;
         }
-        if (xfer->running_retry) {
+        if (xfer != NULL && xfer->running_retry) {
             DPRINTF("xhci: xfer nacked, stopping schedule\n");
             epctx->retry = xfer;
             break;
@@ -3480,6 +3491,9 @@ static void xhci_complete(USBPort *port, USBPacket *packet)
     }
     xhci_complete_packet(xfer);
     xhci_kick_ep(xfer->xhci, xfer->slotid, xfer->epid, xfer->streamid);
+    if (xfer->complete) {
+        xhci_ep_free_xfer(xfer);
+    }
 }
 
 static void xhci_child_detach(USBPort *uport, USBDevice *child)
-- 
1.8.3.1

[Qemu-devel] [PULL 05/11] xhci: drop XHCITransfer->xhci

[Gerd Hoffmann]

Use XHCITransfer->epctx->xhci instead.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1474965172-30321-6-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index e14c2a8..20f46c4 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -344,7 +344,6 @@ typedef struct XHCIPort {
 } XHCIPort;
 
 typedef struct XHCITransfer {
-    XHCIState *xhci;
     XHCIEPContext *epctx;
     USBPacket packet;
     QEMUSGList sgl;
@@ -1449,7 +1448,6 @@ static XHCITransfer *xhci_ep_alloc_xfer(XHCIEPContext *epctx,
     }
 
     xfer = g_new0(XHCITransfer, 1);
-    xfer->xhci = epctx->xhci;
     xfer->epctx = epctx;
     xfer->slotid = epctx->slotid;
     xfer->epid = epctx->epid;
@@ -1488,10 +1486,9 @@ static int xhci_ep_nuke_one_xfer(XHCITransfer *t, TRBCCode report)
         killed = 1;
     }
     if (t->running_retry) {
-        XHCIEPContext *epctx = t->xhci->slots[t->slotid-1].eps[t->epid-1];
-        if (epctx) {
-            epctx->retry = NULL;
-            timer_del(epctx->kick_timer);
+        if (t->epctx) {
+            t->epctx->retry = NULL;
+            timer_del(t->epctx->kick_timer);
         }
         t->running_retry = 0;
         killed = 1;
@@ -1721,7 +1718,7 @@ static TRBCCode xhci_set_ep_dequeue(XHCIState *xhci, unsigned int slotid,
 
 static int xhci_xfer_create_sgl(XHCITransfer *xfer, int in_xfer)
 {
-    XHCIState *xhci = xfer->xhci;
+    XHCIState *xhci = xfer->epctx->xhci;
     int i;
 
     xfer->int_req = false;
@@ -1780,7 +1777,7 @@ static void xhci_xfer_report(XHCITransfer *xfer)
     bool reported = 0;
     bool shortpkt = 0;
     XHCIEvent event = {ER_TRANSFER, CC_SUCCESS};
-    XHCIState *xhci = xfer->xhci;
+    XHCIState *xhci = xfer->epctx->xhci;
     int i;
 
     left = xfer->packet.actual_length;
@@ -1854,9 +1851,8 @@ static void xhci_xfer_report(XHCITransfer *xfer)
 
 static void xhci_stall_ep(XHCITransfer *xfer)
 {
-    XHCIState *xhci = xfer->xhci;
-    XHCISlot *slot = &xhci->slots[xfer->slotid-1];
-    XHCIEPContext *epctx = slot->eps[xfer->epid-1];
+    XHCIEPContext *epctx = xfer->epctx;
+    XHCIState *xhci = epctx->xhci;
     uint32_t err;
     XHCIStreamContext *sctx;
 
@@ -1880,7 +1876,7 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer,
 
 static int xhci_setup_packet(XHCITransfer *xfer)
 {
-    XHCIState *xhci = xfer->xhci;
+    XHCIState *xhci = xfer->epctx->xhci;
     USBEndpoint *ep;
     int dir;
 
@@ -3490,7 +3486,7 @@ static void xhci_complete(USBPort *port, USBPacket *packet)
         return;
     }
     xhci_complete_packet(xfer);
-    xhci_kick_ep(xfer->xhci, xfer->slotid, xfer->epid, xfer->streamid);
+    xhci_kick_ep(xfer->epctx->xhci, xfer->slotid, xfer->epid, xfer->streamid);
     if (xfer->complete) {
         xhci_ep_free_xfer(xfer);
     }
-- 
1.8.3.1

[Qemu-devel] [PULL 06/11] xhci: add & use xhci_kick_epctx()

[Gerd Hoffmann]

xhci_kick_epctx is a xhci_kick_ep variant which takes an XHCIEPContext
as input instead of slotid and epid.  So in case we have a XHCIEPContext
at hand at the callsite we can just pass it directly.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1474965172-30321-7-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 20f46c4..c82fe09 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -509,6 +509,7 @@ enum xhci_flags {
 
 static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
                          unsigned int epid, unsigned int streamid);
+static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid);
 static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid,
                                 unsigned int epid);
 static void xhci_xfer_report(XHCITransfer *xfer);
@@ -1362,7 +1363,7 @@ static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx,
 static void xhci_ep_kick_timer(void *opaque)
 {
     XHCIEPContext *epctx = opaque;
-    xhci_kick_ep(epctx->xhci, epctx->slotid, epctx->epid, 0);
+    xhci_kick_epctx(epctx, 0);
 }
 
 static XHCIEPContext *xhci_alloc_epctx(XHCIState *xhci,
@@ -2008,7 +2009,7 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer)
 
     xhci_complete_packet(xfer);
     if (!xfer->running_async && !xfer->running_retry) {
-        xhci_kick_ep(xhci, xfer->slotid, xfer->epid, 0);
+        xhci_kick_epctx(xfer->epctx, 0);
     }
     return 0;
 }
@@ -2112,7 +2113,7 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx
 
     xhci_complete_packet(xfer);
     if (!xfer->running_async && !xfer->running_retry) {
-        xhci_kick_ep(xhci, xfer->slotid, xfer->epid, xfer->streamid);
+        xhci_kick_epctx(xfer->epctx, xfer->streamid);
     }
     return 0;
 }
@@ -2126,16 +2127,8 @@ static int xhci_fire_transfer(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext
 static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
                          unsigned int epid, unsigned int streamid)
 {
-    XHCIStreamContext *stctx;
     XHCIEPContext *epctx;
-    XHCITransfer *xfer;
-    XHCIRing *ring;
-    USBEndpoint *ep = NULL;
-    uint64_t mfindex;
-    int length;
-    int i;
 
-    trace_usb_xhci_ep_kick(slotid, epid, streamid);
     assert(slotid >= 1 && slotid <= xhci->numslots);
     assert(epid >= 1 && epid <= 31);
 
@@ -2150,11 +2143,27 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
         return;
     }
 
+    xhci_kick_epctx(epctx, streamid);
+}
+
+static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
+{
+    XHCIState *xhci = epctx->xhci;
+    XHCIStreamContext *stctx;
+    XHCITransfer *xfer;
+    XHCIRing *ring;
+    USBEndpoint *ep = NULL;
+    uint64_t mfindex;
+    int length;
+    int i;
+
+    trace_usb_xhci_ep_kick(epctx->slotid, epctx->epid, streamid);
+
     /* If the device has been detached, but the guest has not noticed this
        yet the 2 above checks will succeed, but we must NOT continue */
-    if (!xhci->slots[slotid - 1].uport ||
-        !xhci->slots[slotid - 1].uport->dev ||
-        !xhci->slots[slotid - 1].uport->dev->attached) {
+    if (!xhci->slots[epctx->slotid - 1].uport ||
+        !xhci->slots[epctx->slotid - 1].uport->dev ||
+        !xhci->slots[epctx->slotid - 1].uport->dev->attached) {
         return;
     }
 
@@ -2235,7 +2244,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
         }
         xfer->streamid = streamid;
 
-        if (epid == 1) {
+        if (epctx->epid == 1) {
             xhci_fire_ctl_transfer(xhci, xfer);
         } else {
             xhci_fire_transfer(xhci, xfer, epctx);
@@ -2255,7 +2264,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
         }
     }
 
-    ep = xhci_epid_to_usbep(xhci, slotid, epid);
+    ep = xhci_epid_to_usbep(xhci, epctx->slotid, epctx->epid);
     if (ep) {
         usb_device_flush_ep_queue(ep->dev, ep);
     }
@@ -3486,7 +3495,7 @@ static void xhci_complete(USBPort *port, USBPacket *packet)
         return;
     }
     xhci_complete_packet(xfer);
-    xhci_kick_ep(xfer->epctx->xhci, xfer->slotid, xfer->epid, xfer->streamid);
+    xhci_kick_epctx(xfer->epctx, xfer->streamid);
     if (xfer->complete) {
         xhci_ep_free_xfer(xfer);
     }
-- 
1.8.3.1

[Qemu-devel] [PULL 07/11] xhci: drop XHCITransfer->{slotid,epid}

[Gerd Hoffmann]

We can use XHCITransfer->epctx->{slotid,epid} instead.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1474965172-30321-8-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index c82fe09..80840d3 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -352,8 +352,6 @@ typedef struct XHCITransfer {
     bool complete;
     bool int_req;
     unsigned int iso_pkts;
-    unsigned int slotid;
-    unsigned int epid;
     unsigned int streamid;
     bool in_xfer;
     bool iso_xfer;
@@ -1450,8 +1448,6 @@ static XHCITransfer *xhci_ep_alloc_xfer(XHCIEPContext *epctx,
 
     xfer = g_new0(XHCITransfer, 1);
     xfer->epctx = epctx;
-    xfer->slotid = epctx->slotid;
-    xfer->epid = epctx->epid;
     xfer->trbs = g_new(XHCITRB, length);
     xfer->trb_count = length;
     usb_packet_init(&xfer->packet);
@@ -1816,8 +1812,8 @@ static void xhci_xfer_report(XHCITransfer *xfer)
         if (!reported && ((trb->control & TRB_TR_IOC) ||
                           (shortpkt && (trb->control & TRB_TR_ISP)) ||
                           (xfer->status != CC_SUCCESS && left == 0))) {
-            event.slotid = xfer->slotid;
-            event.epid = xfer->epid;
+            event.slotid = xfer->epctx->slotid;
+            event.epid = xfer->epctx->epid;
             event.length = (trb->status & 0x1ffff) - chunk;
             event.flags = 0;
             event.ptr = trb->addr;
@@ -1886,7 +1882,7 @@ static int xhci_setup_packet(XHCITransfer *xfer)
     if (xfer->packet.ep) {
         ep = xfer->packet.ep;
     } else {
-        ep = xhci_epid_to_usbep(xhci, xfer->slotid, xfer->epid);
+        ep = xhci_epid_to_usbep(xhci, xfer->epctx->slotid, xfer->epctx->epid);
         if (!ep) {
             DPRINTF("xhci: slot %d has no device\n",
                     xfer->slotid);
@@ -1966,7 +1962,8 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer)
     trb_setup = &xfer->trbs[0];
     trb_status = &xfer->trbs[xfer->trb_count-1];
 
-    trace_usb_xhci_xfer_start(xfer, xfer->slotid, xfer->epid, xfer->streamid);
+    trace_usb_xhci_xfer_start(xfer, xfer->epctx->slotid,
+                              xfer->epctx->epid, xfer->streamid);
 
     /* at most one Event Data TRB allowed after STATUS */
     if (TRB_TYPE(*trb_status) == TR_EVDATA && xfer->trb_count > 2) {
@@ -2120,7 +2117,8 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx
 
 static int xhci_fire_transfer(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx)
 {
-    trace_usb_xhci_xfer_start(xfer, xfer->slotid, xfer->epid, xfer->streamid);
+    trace_usb_xhci_xfer_start(xfer, xfer->epctx->slotid,
+                              xfer->epctx->epid, xfer->streamid);
     return xhci_submit(xhci, xfer, epctx);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 08/11] xhci: make xhci_epid_to_usbep accept XHCIEPContext

[Gerd Hoffmann]

All callsites have a XHCIEPContext pointer anyway, so we can just pass
it directly instead of fiddeling with slotid and epid.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1474965172-30321-9-git-send-email-kraxel@redhat.com
---
 hw/usb/hcd-xhci.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 80840d3..4acf0c6 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -513,8 +513,7 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid,
 static void xhci_xfer_report(XHCITransfer *xfer);
 static void xhci_event(XHCIState *xhci, XHCIEvent *event, int v);
 static void xhci_write_event(XHCIState *xhci, XHCIEvent *event, int v);
-static USBEndpoint *xhci_epid_to_usbep(XHCIState *xhci,
-                                       unsigned int slotid, unsigned int epid);
+static USBEndpoint *xhci_epid_to_usbep(XHCIEPContext *epctx);
 
 static const char *TRBType_names[] = {
     [TRB_RESERVED]                     = "TRB_RESERVED",
@@ -1200,7 +1199,7 @@ static int xhci_epmask_to_eps_with_streams(XHCIState *xhci,
         }
 
         epctx = slot->eps[i - 1];
-        ep = xhci_epid_to_usbep(xhci, slotid, i);
+        ep = xhci_epid_to_usbep(epctx);
         if (!epctx || !epctx->nr_pstreams || !ep) {
             continue;
         }
@@ -1531,7 +1530,7 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid,
         xhci_ep_free_xfer(xfer);
     }
 
-    ep = xhci_epid_to_usbep(xhci, slotid, epid);
+    ep = xhci_epid_to_usbep(epctx);
     if (ep) {
         usb_device_ep_stopped(ep->dev, ep);
     }
@@ -1873,7 +1872,6 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer,
 
 static int xhci_setup_packet(XHCITransfer *xfer)
 {
-    XHCIState *xhci = xfer->epctx->xhci;
     USBEndpoint *ep;
     int dir;
 
@@ -1882,7 +1880,7 @@ static int xhci_setup_packet(XHCITransfer *xfer)
     if (xfer->packet.ep) {
         ep = xfer->packet.ep;
     } else {
-        ep = xhci_epid_to_usbep(xhci, xfer->epctx->slotid, xfer->epctx->epid);
+        ep = xhci_epid_to_usbep(xfer->epctx);
         if (!ep) {
             DPRINTF("xhci: slot %d has no device\n",
                     xfer->slotid);
@@ -2262,7 +2260,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
         }
     }
 
-    ep = xhci_epid_to_usbep(xhci, epctx->slotid, epctx->epid);
+    ep = xhci_epid_to_usbep(epctx);
     if (ep) {
         usb_device_flush_ep_queue(ep->dev, ep);
     }
@@ -3527,17 +3525,20 @@ static int xhci_find_epid(USBEndpoint *ep)
     }
 }
 
-static USBEndpoint *xhci_epid_to_usbep(XHCIState *xhci,
-                                       unsigned int slotid, unsigned int epid)
+static USBEndpoint *xhci_epid_to_usbep(XHCIEPContext *epctx)
 {
-    assert(slotid >= 1 && slotid <= xhci->numslots);
+    USBPort *uport;
+    uint32_t token;
 
-    if (!xhci->slots[slotid - 1].uport) {
+    if (!epctx) {
         return NULL;
     }
-
-    return usb_ep_get(xhci->slots[slotid - 1].uport->dev,
-                      (epid & 1) ? USB_TOKEN_IN : USB_TOKEN_OUT, epid >> 1);
+    uport = epctx->xhci->slots[epctx->slotid - 1].uport;
+    token = (epctx->epid & 1) ? USB_TOKEN_IN : USB_TOKEN_OUT;
+    if (!uport) {
+        return NULL;
+    }
+    return usb_ep_get(uport->dev, token, epctx->epid >> 1);
 }
 
 static void xhci_wakeup_endpoint(USBBus *bus, USBEndpoint *ep,
-- 
1.8.3.1

[Qemu-devel] [PULL 09/11] usb: fix serial generator

[Gerd Hoffmann]

snprintf return value is *not* the number of chars written into the
buffer, but the number of chars needed.  So in case the buffer is too
small you can go alloc a bigger one and try again.  But that also means
you can't simply use the return value for the next snprintf call
without checking beforehand that things did actually fit.

Problem is that usb_desc_create_serial didn't perform that check, so a
loooong path string (can happen with deep pci-bridge nesting) results in
the third snprintf call smashing the stack.

Fix this by throwing out all the snpintf calls and use g_strdup_printf
instead.

https://bugzilla.redhat.com/show_bug.cgi?id=1381630

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1475659998-22045-1-git-send-email-kraxel@redhat.com
---
 hw/usb/desc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index 5e0e1d1..7828e52 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -556,9 +556,7 @@ void usb_desc_create_serial(USBDevice *dev)
     DeviceState *hcd = dev->qdev.parent_bus->parent;
     const USBDesc *desc = usb_device_get_usb_desc(dev);
     int index = desc->id.iSerialNumber;
-    char serial[64];
-    char *path;
-    int dst;
+    char *path, *serial;
 
     if (dev->serial) {
         /* 'serial' usb bus property has priority if present */
@@ -567,14 +565,16 @@ void usb_desc_create_serial(USBDevice *dev)
     }
 
     assert(index != 0 && desc->str[index] != NULL);
-    dst = snprintf(serial, sizeof(serial), "%s", desc->str[index]);
     path = qdev_get_dev_path(hcd);
     if (path) {
-        dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", path);
+        serial = g_strdup_printf("%s-%s-%s", desc->str[index],
+                                 path, dev->port->path);
+    } else {
+        serial = g_strdup_printf("%s-%s", desc->str[index], dev->port->path);
     }
-    dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", dev->port->path);
     usb_desc_set_string(dev, index, serial);
     g_free(path);
+    g_free(serial);
 }
 
 const char *usb_desc_get_string(USBDevice *dev, uint8_t index)
-- 
1.8.3.1

[Qemu-devel] [PULL 10/11] usb: Fix incorrect default DMA offset.

[Gerd Hoffmann]

From: Vijay Kumar B <vijaykumar@zilogic.com>

The default DMA offset is set to 3. When the property is not set by
the consumer, the default causes DMA access to be shifted by 3
bytes. In PXA, this results in incorrect DMA access, leading to error
notification in the USB controller driver. A better default would be
0, so that there is no offset, when the consumer does not specify one.

Signed-off-by: Vijay Kumar B. <vijaykumar@zilogic.com>
Reviewed-by: Deepak S. <deepak@zilogic.com>
Message-id: 1475060958-7760-1-git-send-email-vijaykumar@zilogic.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ohci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index fa57038..c82a92f 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -2139,7 +2139,7 @@ static const TypeInfo ohci_pci_info = {
 
 static Property ohci_sysbus_properties[] = {
     DEFINE_PROP_UINT32("num-ports", OHCISysBusState, num_ports, 3),
-    DEFINE_PROP_DMAADDR("dma-offset", OHCISysBusState, dma_offset, 3),
+    DEFINE_PROP_DMAADDR("dma-offset", OHCISysBusState, dma_offset, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.8.3.1

[Qemu-devel] [PULL 11/11] usb-redir: allocate buffers before waking up the host adapter

[Gerd Hoffmann]

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

Needed to make sure usb redirection is prepared to actually handle the
callback from the usb host adapter.  Without this interrupt endpoints
don't work on xhci.

Note: On ehci the usb_wakeup() call only schedules a BH for the actual
work, which hides this bug because the allocation happens before ehci
calls back even without this patch.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Message-id: 1476096313-7730-1-git-send-email-kraxel@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/redirect.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 444672a..d4ca026 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -2036,18 +2036,22 @@ static void usbredir_interrupt_packet(void *priv, uint64_t id,
     }
 
     if (ep & USB_DIR_IN) {
+        bool q_was_empty;
+
         if (dev->endpoint[EP2I(ep)].interrupt_started == 0) {
             DPRINTF("received int packet while not started ep %02X\n", ep);
             free(data);
             return;
         }
 
-        if (QTAILQ_EMPTY(&dev->endpoint[EP2I(ep)].bufpq)) {
-            usb_wakeup(usb_ep_get(&dev->dev, USB_TOKEN_IN, ep & 0x0f), 0);
-        }
+        q_was_empty = QTAILQ_EMPTY(&dev->endpoint[EP2I(ep)].bufpq);
 
         /* bufp_alloc also adds the packet to the ep queue */
         bufp_alloc(dev, data, data_len, interrupt_packet->status, ep, data);
+
+        if (q_was_empty) {
+            usb_wakeup(usb_ep_get(&dev->dev, USB_TOKEN_IN, ep & 0x0f), 0);
+        }
     } else {
         /*
          * We report output interrupt packets as completed directly upon
-- 
1.8.3.1

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

[Peter Maydell]

On 12 October 2016 at 13:58, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> This is the usb patch queue, again in bugfixing mode.  xhci transfers
> are allocated dynamically now, the static allocations (and limit) didn't
> play nicely with stream endpoints.  Some xhci cleanups along the way.
> Some bugfixes picked up from the list.
>
> please pull,
>   Gerd
>
> The following changes since commit 6b39b06339ee59559b31f860d4af635b046322df:
>
>   build: Work around SIZE_MAX bug in OSX headers (2016-10-11 19:22:20 +0100)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/pull-usb-20161012-1
>
> for you to fetch changes up to d5c42857d6b0c35028897df8dfc3749eba6f6de3:
>
>   usb-redir: allocate buffers before waking up the host adapter (2016-10-12 14:37:24 +0200)
>
> ----------------------------------------------------------------
> various usb bugfixes
> some xhci cleanups

Applied, thanks.

-- PMM