[Qemu-devel] [PATCH 0/8] xhci: stream transfer fixes, cleanups

[Qemu-devel] [PATCH 0/8] xhci: stream transfer fixes, cleanups

[Gerd Hoffmann]

  Hi,

Our xhci emulation has trouble handling devices with many xhci streams,
which is fixed by patch #4.  Patch #1 can be used to reproduce the bug
with the qemu uas emulation.  It's there for testing convinience only,
I do not intend to merge it.

The other patches are cleanups.

please review,
  Gerd

Gerd Hoffmann (8):
  [debug] uas: use 32 streams
  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

 hw/usb/dev-uas.c  |   2 +-
 hw/usb/hcd-xhci.c | 219 +++++++++++++++++++++++++++++-------------------------
 2 files changed, 119 insertions(+), 102 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/8] [debug] uas: use 32 streams

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-uas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 3a8ff18..2a5fcbc 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -101,7 +101,7 @@ typedef struct {
 
 /* --------------------------------------------------------------------- */
 
-#define UAS_STREAM_BM_ATTR  4
+#define UAS_STREAM_BM_ATTR  5
 #define UAS_MAX_STREAMS     (1 << UAS_STREAM_BM_ATTR)
 
 typedef struct UASDevice UASDevice;
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/8] 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>
---
 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 726435c..15cac56 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] [PATCH 3/8] xhci: drop unused comp_xfer field

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <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 15cac56..089dcbf 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -384,7 +384,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] [PATCH 4/8] 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>
---
 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 089dcbf..d8e4074 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
@@ -344,6 +343,7 @@ typedef struct XHCIPort {
 
 typedef struct XHCITransfer {
     XHCIState *xhci;
+    XHCIEPContext *epctx;
     USBPacket packet;
     QEMUSGList sgl;
     bool running_async;
@@ -359,7 +359,6 @@ typedef struct XHCITransfer {
     bool timed_xfer;
 
     unsigned int trb_count;
-    unsigned int trb_alloced;
     XHCITRB *trbs;
 
     TRBCCode status;
@@ -369,6 +368,8 @@ typedef struct XHCITransfer {
     unsigned int cur_pkt;
 
     uint64_t mfindex_kick;
+
+    QTAILQ_ENTRY(XHCITransfer) next;
 } XHCITransfer;
 
 struct XHCIStreamContext {
@@ -383,8 +384,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;
@@ -1360,19 +1361,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;
@@ -1433,6 +1428,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;
@@ -1459,7 +1489,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;
 }
@@ -1469,7 +1499,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);
@@ -1484,14 +1515,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);
@@ -1506,7 +1539,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);
@@ -1527,10 +1559,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);
@@ -2094,6 +2122,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;
@@ -2158,6 +2187,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;
     }
 
@@ -2183,27 +2213,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;
@@ -2213,25 +2230,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;
@@ -3470,6 +3481,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] [PATCH 5/8] xhci: drop XHCITransfer->xhci

[Gerd Hoffmann]

Use XHCITransfer->epctx->xhci instead.

Signed-off-by: Gerd Hoffmann <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 d8e4074..da249f7 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -342,7 +342,6 @@ typedef struct XHCIPort {
 } XHCIPort;
 
 typedef struct XHCITransfer {
-    XHCIState *xhci;
     XHCIEPContext *epctx;
     USBPacket packet;
     QEMUSGList sgl;
@@ -1439,7 +1438,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;
@@ -1478,10 +1476,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;
@@ -1711,7 +1708,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;
@@ -1770,7 +1767,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;
@@ -1844,9 +1841,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;
 
@@ -1870,7 +1866,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;
 
@@ -3480,7 +3476,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] [PATCH 6/8] 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>
---
 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 da249f7..4e557c2 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -507,6 +507,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);
@@ -1352,7 +1353,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,
@@ -1998,7 +1999,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;
 }
@@ -2102,7 +2103,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;
 }
@@ -2116,16 +2117,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);
 
@@ -2140,11 +2133,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;
     }
 
@@ -2225,7 +2234,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);
@@ -2245,7 +2254,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);
     }
@@ -3476,7 +3485,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] [PATCH 7/8] xhci: drop XHCITransfer->{slotid,epid}

[Gerd Hoffmann]

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

Signed-off-by: Gerd Hoffmann <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 4e557c2..108d185 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -350,8 +350,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;
@@ -1440,8 +1438,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);
@@ -1806,8 +1802,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;
@@ -1876,7 +1872,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);
@@ -1956,7 +1952,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) {
@@ -2110,7 +2107,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] [PATCH 8/8] 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>
---
 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 108d185..5176405 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -511,8 +511,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",
@@ -1190,7 +1189,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;
         }
@@ -1521,7 +1520,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);
     }
@@ -1863,7 +1862,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;
 
@@ -1872,7 +1870,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);
@@ -2252,7 +2250,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);
     }
@@ -3517,17 +3515,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