[Qemu-devel] [PATCH 00/13] usb: Add support for bulk streams to usb-host-libusb

[Qemu-devel] [PATCH 00/13] usb: Add support for bulk streams to usb-host-libusb

[Hans de Goede]

Hi Gerd et al,

After quite a bit of work I'm very happy to present this patch set which adds
full support for using USB-3 devices which use bulkstreams with qemu's usb
host redirection.

This has been tested under a Linux guest, with an uas usb-3 device, and
works well and stable.

There is a bunch of pre-requisites to be able to use this though (and for
it to be stable / reliable). The Linux host will need a kernel with a bunch
of xhci driver fixes as well as patches to extend the usbfs API with streams
support. I've submitted all these upstream and I hope they get merged into the
3.13 kernel.

Further more a version of libusbx with bulk stream support is necessary, since
the usbfs API is not yet in the upstream kernel (and thus not yet finalized),
I've not pushed the necessary libusbx patches upstream yet, for now you can
find them in my libusbx repo here:
https://github.com/jwrdegoede/libusbx/commits/master

I'm an upstream libusbx committer and I will push these upstream as soon as
the kernel API has been merged (which I expect to happen without any further
changes to it).

Note the last 2 patches in the set depend on the new libusbx APIs for this,
so if you're afraid the API may still change you could consider delaying
merging these, but I'm quite confident that there will be no further API
changes.

Regards,

Hans

[Qemu-devel] [PATCH 01/13] usb-host-libusb: Fix reset handling

[Hans de Goede]

The guest will issue an initial device reset when the device is attached, but
since the current usb-host-libusb code only actually does the reset when
udev->configuration != 0, and on attach the device is not yet configured,
the reset gets ignored. This means that the device gets passed to the guest
in an unknown state, which is not good.

The udev->configuration check is there because of the release / claim
interfaces done around the libusb_device_reset call, but these are not
necessary. If interfaces are claimed when libusb_device_reset gets called
libusb will release + reclaim them itself.

The usb_host_ep_update call also is not necessary. If the reset succeeds the
original config and interface alt settings will be restored.

Last if the reset fails, that means the device has either disconnected or
morphed into an another device and has been completely re-enumerated,
so it is treated by the host as a new device and our handle is invalid,
so on reset failure we need to call usb_host_nodev().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/host-libusb.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 128955d..428c7c5 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -1256,16 +1256,14 @@ static void usb_host_flush_ep_queue(USBDevice *dev, USBEndpoint *ep)
 static void usb_host_handle_reset(USBDevice *udev)
 {
     USBHostDevice *s = USB_HOST_DEVICE(udev);
+    int rc;
 
     trace_usb_host_reset(s->bus_num, s->addr);
 
-    if (udev->configuration == 0) {
-        return;
+    rc = libusb_reset_device(s->dh);
+    if (rc != 0) {
+        usb_host_nodev(s);
     }
-    usb_host_release_interfaces(s);
-    libusb_reset_device(s->dh);
-    usb_host_claim_interfaces(s, 0);
-    usb_host_ep_update(s);
 }
 
 /*
-- 
1.8.3.1

[Qemu-devel] [PATCH 02/13] usb-host-libusb: Configuration 0 may be a valid configuration

[Hans de Goede]

Quoting from: linux/Documentation/ABI/stable/sysfs-bus-usb:

	Note that some devices, in violation of the USB spec, have a
	configuration with a value equal to 0. Writing 0 to
	bConfigurationValue for these devices will install that
	configuration, rather then unconfigure the device.

So don't compare the configuration value against 0 to check for unconfigured
devices, instead check for a LIBUSB_ERROR_NOT_FOUND return from
libusb_get_active_config_descriptor().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/host-libusb.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 428c7c5..35bae55 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -992,15 +992,14 @@ static int usb_host_claim_interfaces(USBHostDevice *s, int configuration)
     udev->ninterfaces   = 0;
     udev->configuration = 0;
 
-    if (configuration == 0) {
-        /* address state - ignore */
-        return USB_RET_SUCCESS;
-    }
-
     usb_host_detach_kernel(s);
 
     rc = libusb_get_active_config_descriptor(s->dev, &conf);
     if (rc != 0) {
+        if (rc == LIBUSB_ERROR_NOT_FOUND) {
+            /* address state - ignore */
+            return USB_RET_SUCCESS;
+        }
         return USB_RET_STALL;
     }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 03/13] usb-host-libusb: Detach kernel drivers earlier

[Hans de Goede]

If we detach the kernel drivers on the first set_config, then they will
be still attached when the device gets its initial reset. Causing the drivers
to re-initialize the device after the reset, dirtying the device state.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/host-libusb.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 35bae55..fd320cd 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -137,6 +137,7 @@ static QTAILQ_HEAD(, USBHostDevice) hostdevs =
 static void usb_host_auto_check(void *unused);
 static void usb_host_release_interfaces(USBHostDevice *s);
 static void usb_host_nodev(USBHostDevice *s);
+static void usb_host_detach_kernel(USBHostDevice *s);
 static void usb_host_attach_kernel(USBHostDevice *s);
 
 /* ------------------------------------------------------------------------ */
@@ -787,10 +788,13 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev)
         goto fail;
     }
 
-    libusb_get_device_descriptor(dev, &s->ddesc);
     s->dev     = dev;
     s->bus_num = bus_num;
     s->addr    = addr;
+
+    usb_host_detach_kernel(s);
+
+    libusb_get_device_descriptor(dev, &s->ddesc);
     usb_host_get_port(s->dev, s->port, sizeof(s->port));
 
     usb_ep_init(udev);
@@ -1051,7 +1055,6 @@ static void usb_host_set_config(USBHostDevice *s, int config, USBPacket *p)
     trace_usb_host_set_config(s->bus_num, s->addr, config);
 
     usb_host_release_interfaces(s);
-    usb_host_detach_kernel(s);
     rc = libusb_set_configuration(s->dh, config);
     if (rc != 0) {
         usb_host_libusb_error("libusb_set_configuration", rc);
-- 
1.8.3.1

[Qemu-devel] [PATCH 04/13] usb-hcd-xhci: Remove unused sstreamsm member from XHCIStreamContext

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/hcd-xhci.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 469c24d..e078c50 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -374,7 +374,6 @@ struct XHCIStreamContext {
     dma_addr_t pctx;
     unsigned int sct;
     XHCIRing ring;
-    XHCIStreamContext *sstreams;
 };
 
 struct XHCIEPContext {
@@ -1133,7 +1132,6 @@ static void xhci_reset_streams(XHCIEPContext *epctx)
 
     for (i = 0; i < epctx->nr_pstreams; i++) {
         epctx->pstreams[i].sct = -1;
-        g_free(epctx->pstreams[i].sstreams);
     }
 }
 
@@ -1146,15 +1144,8 @@ static void xhci_alloc_streams(XHCIEPContext *epctx, dma_addr_t base)
 
 static void xhci_free_streams(XHCIEPContext *epctx)
 {
-    int i;
-
     assert(epctx->pstreams != NULL);
 
-    if (!epctx->lsa) {
-        for (i = 0; i < epctx->nr_pstreams; i++) {
-            g_free(epctx->pstreams[i].sstreams);
-        }
-    }
     g_free(epctx->pstreams);
     epctx->pstreams = NULL;
     epctx->nr_pstreams = 0;
-- 
1.8.3.1

[Qemu-devel] [PATCH 05/13] usb-hcd-xhci: Remove unused cancelled member from XHCITransfer

[Hans de Goede]

Since qemu's USB model is geared towards emulated devices cancellation
is instanteneous, so no need to wait for cancellation to complete, as
such there is no wait for cancellation code, and the cancelled bool
as well as the bogus comment about it can be removed.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/hcd-xhci.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index e078c50..7cf89ce 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -346,7 +346,6 @@ typedef struct XHCITransfer {
     QEMUSGList sgl;
     bool running_async;
     bool running_retry;
-    bool cancelled;
     bool complete;
     bool int_req;
     unsigned int iso_pkts;
@@ -1310,8 +1309,6 @@ static int xhci_ep_nuke_one_xfer(XHCITransfer *t)
     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) {
@@ -1728,14 +1725,12 @@ static int xhci_complete_packet(XHCITransfer *xfer)
         xfer->running_async = 1;
         xfer->running_retry = 0;
         xfer->complete = 0;
-        xfer->cancelled = 0;
         return 0;
     } else if (xfer->packet.status == USB_RET_NAK) {
         trace_usb_xhci_xfer_nak(xfer);
         xfer->running_async = 0;
         xfer->running_retry = 1;
         xfer->complete = 0;
-        xfer->cancelled = 0;
         return 0;
     } else {
         xfer->running_async = 0;
-- 
1.8.3.1

[Qemu-devel] [PATCH 06/13] usb-hcd-xhci: Report completion of active transfer with CC_STOPPED on ep stop

[Hans de Goede]

As we should per the XHCI spec "4.6.9 Stop Endpoint".

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/hcd-xhci.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 7cf89ce..0131151 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -505,6 +505,7 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
                          unsigned int epid, unsigned int streamid);
 static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid,
                                 unsigned int epid);
+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,
@@ -1302,10 +1303,15 @@ static TRBCCode xhci_enable_ep(XHCIState *xhci, unsigned int slotid,
     return CC_SUCCESS;
 }
 
-static int xhci_ep_nuke_one_xfer(XHCITransfer *t)
+static int xhci_ep_nuke_one_xfer(XHCITransfer *t, TRBCCode report)
 {
     int killed = 0;
 
+    if (report && (t->running_async || t->running_retry)) {
+        t->status = report;
+        xhci_xfer_report(t);
+    }
+
     if (t->running_async) {
         usb_cancel_packet(&t->packet);
         t->running_async = 0;
@@ -1318,6 +1324,7 @@ static int xhci_ep_nuke_one_xfer(XHCITransfer *t)
             timer_del(epctx->kick_timer);
         }
         t->running_retry = 0;
+        killed = 1;
     }
     if (t->trbs) {
         g_free(t->trbs);
@@ -1330,7 +1337,7 @@ static int xhci_ep_nuke_one_xfer(XHCITransfer *t)
 }
 
 static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid,
-                               unsigned int epid)
+                               unsigned int epid, TRBCCode report)
 {
     XHCISlot *slot;
     XHCIEPContext *epctx;
@@ -1351,7 +1358,10 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid,
 
     xferi = epctx->next_xfer;
     for (i = 0; i < TD_QUEUE; i++) {
-        killed += xhci_ep_nuke_one_xfer(&epctx->transfers[xferi]);
+        killed += xhci_ep_nuke_one_xfer(&epctx->transfers[xferi], report);
+        if (killed) {
+            report = 0; /* Only report once */
+        }
         epctx->transfers[xferi].packet.ep = NULL;
         xferi = (xferi + 1) % TD_QUEUE;
     }
@@ -1381,7 +1391,7 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid,
         return CC_SUCCESS;
     }
 
-    xhci_ep_nuke_xfers(xhci, slotid, epid);
+    xhci_ep_nuke_xfers(xhci, slotid, epid, 0);
 
     epctx = slot->eps[epid-1];
 
@@ -1423,7 +1433,7 @@ static TRBCCode xhci_stop_ep(XHCIState *xhci, unsigned int slotid,
         return CC_EP_NOT_ENABLED_ERROR;
     }
 
-    if (xhci_ep_nuke_xfers(xhci, slotid, epid) > 0) {
+    if (xhci_ep_nuke_xfers(xhci, slotid, epid, CC_STOPPED) > 0) {
         fprintf(stderr, "xhci: FIXME: endpoint stopped w/ xfers running, "
                 "data might be lost\n");
     }
@@ -1468,7 +1478,7 @@ static TRBCCode xhci_reset_ep(XHCIState *xhci, unsigned int slotid,
         return CC_CONTEXT_STATE_ERROR;
     }
 
-    if (xhci_ep_nuke_xfers(xhci, slotid, epid) > 0) {
+    if (xhci_ep_nuke_xfers(xhci, slotid, epid, 0) > 0) {
         fprintf(stderr, "xhci: FIXME: endpoint reset w/ xfers running, "
                 "data might be lost\n");
     }
@@ -2461,7 +2471,7 @@ static void xhci_detach_slot(XHCIState *xhci, USBPort *uport)
 
     for (ep = 0; ep < 31; ep++) {
         if (xhci->slots[slot].eps[ep]) {
-            xhci_ep_nuke_xfers(xhci, slot+1, ep+1);
+            xhci_ep_nuke_xfers(xhci, slot + 1, ep + 1, 0);
         }
     }
     xhci->slots[slot].uport = NULL;
@@ -3276,7 +3286,7 @@ static void xhci_complete(USBPort *port, USBPacket *packet)
     XHCITransfer *xfer = container_of(packet, XHCITransfer, packet);
 
     if (packet->status == USB_RET_REMOVE_FROM_QUEUE) {
-        xhci_ep_nuke_one_xfer(xfer);
+        xhci_ep_nuke_one_xfer(xfer, 0);
         return;
     }
     xhci_complete_packet(xfer);
-- 
1.8.3.1

[Qemu-devel] [PATCH 07/13] usb-hcd-xhci: Update endpoint context dequeue pointer for streams too

[Hans de Goede]

With streams the endpoint context dequeue pointer should point to the
dequeue value for the currently active stream.

At least Linux guests expect it to point to value set by an set_ep_dequeue
upon completion of the set_ep_dequeue (before kicking the ep).

Otherwise the Linux kernel will complain (and things won't work):

xhci_hcd 0000:00:05.0: Mismatch between completed Set TR Deq Ptr command & xHCI internal state.
xhci_hcd 0000:00:05.0: ep deq seg = ffff8800366f0880, deq ptr = ffff8800366ec010

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

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 0131151..fa27299 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1187,6 +1187,7 @@ static XHCIStreamContext *xhci_find_stream(XHCIEPContext *epctx,
 static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx,
                               XHCIStreamContext *sctx, uint32_t state)
 {
+    XHCIRing *ring = NULL;
     uint32_t ctx[5];
     uint32_t ctx2[2];
 
@@ -1197,6 +1198,7 @@ static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx,
     /* update ring dequeue ptr */
     if (epctx->nr_pstreams) {
         if (sctx != NULL) {
+            ring = &sctx->ring;
             xhci_dma_read_u32s(xhci, sctx->pctx, ctx2, sizeof(ctx2));
             ctx2[0] &= 0xe;
             ctx2[0] |= sctx->ring.dequeue | sctx->ring.ccs;
@@ -1204,8 +1206,12 @@ static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx,
             xhci_dma_write_u32s(xhci, sctx->pctx, ctx2, sizeof(ctx2));
         }
     } else {
-        ctx[2] = epctx->ring.dequeue | epctx->ring.ccs;
-        ctx[3] = (epctx->ring.dequeue >> 16) >> 16;
+        ring = &epctx->ring;
+    }
+    if (ring) {
+        ctx[2] = ring->dequeue | ring->ccs;
+        ctx[3] = (ring->dequeue >> 16) >> 16;
+
         DPRINTF("xhci: set epctx: " DMA_ADDR_FMT " state=%d dequeue=%08x%08x\n",
                 epctx->pctx, state, ctx[3], ctx[2]);
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH 08/13] usb: Add max_streams attribute to endpoint info

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/core.c    | 22 ++++++++++++++++++++++
 hw/usb/desc.c    |  2 ++
 include/hw/usb.h |  3 +++
 3 files changed, 27 insertions(+)

diff --git a/hw/usb/core.c b/hw/usb/core.c
index cf59a1a..67ba7d6 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -623,6 +623,7 @@ void usb_ep_reset(USBDevice *dev)
     dev->ep_ctl.type = USB_ENDPOINT_XFER_CONTROL;
     dev->ep_ctl.ifnum = 0;
     dev->ep_ctl.max_packet_size = 64;
+    dev->ep_ctl.max_streams = 0;
     dev->ep_ctl.dev = dev;
     dev->ep_ctl.pipeline = false;
     for (ep = 0; ep < USB_MAX_ENDPOINTS; ep++) {
@@ -636,6 +637,8 @@ void usb_ep_reset(USBDevice *dev)
         dev->ep_out[ep].ifnum = USB_INTERFACE_INVALID;
         dev->ep_in[ep].max_packet_size = 0;
         dev->ep_out[ep].max_packet_size = 0;
+        dev->ep_in[ep].max_streams = 0;
+        dev->ep_out[ep].max_streams = 0;
         dev->ep_in[ep].dev = dev;
         dev->ep_out[ep].dev = dev;
         dev->ep_in[ep].pipeline = false;
@@ -764,6 +767,25 @@ int usb_ep_get_max_packet_size(USBDevice *dev, int pid, int ep)
     return uep->max_packet_size;
 }
 
+void usb_ep_set_max_streams(USBDevice *dev, int pid, int ep, uint8_t raw)
+{
+    struct USBEndpoint *uep = usb_ep_get(dev, pid, ep);
+    int MaxStreams;
+
+    MaxStreams = raw & 0x1f;
+    if (MaxStreams) {
+        uep->max_streams = 1 << MaxStreams;
+    } else {
+        uep->max_streams = 0;
+    }
+}
+
+int usb_ep_get_max_streams(USBDevice *dev, int pid, int ep)
+{
+    struct USBEndpoint *uep = usb_ep_get(dev, pid, ep);
+    return uep->max_streams;
+}
+
 void usb_ep_set_pipeline(USBDevice *dev, int pid, int ep, bool enabled)
 {
     struct USBEndpoint *uep = usb_ep_get(dev, pid, ep);
diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index bf6c522..5dbe754 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -385,6 +385,8 @@ static void usb_desc_ep_init(USBDevice *dev)
             usb_ep_set_ifnum(dev, pid, ep, iface->bInterfaceNumber);
             usb_ep_set_max_packet_size(dev, pid, ep,
                                        iface->eps[e].wMaxPacketSize);
+            usb_ep_set_max_streams(dev, pid, ep,
+                                   iface->eps[e].bmAttributes_super);
         }
     }
 }
diff --git a/include/hw/usb.h b/include/hw/usb.h
index a7680d4..e9d96ba 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -189,6 +189,7 @@ struct USBEndpoint {
     uint8_t type;
     uint8_t ifnum;
     int max_packet_size;
+    int max_streams;
     bool pipeline;
     bool halted;
     USBDevice *dev;
@@ -421,6 +422,8 @@ void usb_ep_set_ifnum(USBDevice *dev, int pid, int ep, uint8_t ifnum);
 void usb_ep_set_max_packet_size(USBDevice *dev, int pid, int ep,
                                 uint16_t raw);
 int usb_ep_get_max_packet_size(USBDevice *dev, int pid, int ep);
+void usb_ep_set_max_streams(USBDevice *dev, int pid, int ep, uint8_t raw);
+int usb_ep_get_max_streams(USBDevice *dev, int pid, int ep);
 void usb_ep_set_pipeline(USBDevice *dev, int pid, int ep, bool enabled);
 void usb_ep_set_halted(USBDevice *dev, int pid, int ep, bool halted);
 USBPacket *usb_ep_find_packet_by_id(USBDevice *dev, int pid, int ep,
-- 
1.8.3.1

[Qemu-devel] [PATCH 09/13] usb: Add usb_device_alloc/free_streams

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/bus.c     | 18 ++++++++++++++++++
 include/hw/usb.h | 12 ++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 72d5b92..bba554c 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -203,6 +203,24 @@ void usb_device_ep_stopped(USBDevice *dev, USBEndpoint *ep)
     }
 }
 
+int usb_device_alloc_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps,
+                             int streams)
+{
+    USBDeviceClass *klass = USB_DEVICE_GET_CLASS(dev);
+    if (klass->alloc_streams) {
+        return klass->alloc_streams(dev, eps, nr_eps, streams);
+    }
+    return 0;
+}
+
+void usb_device_free_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps)
+{
+    USBDeviceClass *klass = USB_DEVICE_GET_CLASS(dev);
+    if (klass->free_streams) {
+        klass->free_streams(dev, eps, nr_eps);
+    }
+}
+
 static int usb_qdev_init(DeviceState *qdev)
 {
     USBDevice *dev = USB_DEVICE(qdev);
diff --git a/include/hw/usb.h b/include/hw/usb.h
index e9d96ba..0a6ef4a 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -315,6 +315,14 @@ typedef struct USBDeviceClass {
      */
     void (*ep_stopped)(USBDevice *dev, USBEndpoint *ep);
 
+    /*
+     * Called by the hcd to alloc / free streams on a bulk endpoint.
+     * Optional may be NULL.
+     */
+    int (*alloc_streams)(USBDevice *dev, USBEndpoint **eps, int nr_eps,
+                         int streams);
+    void (*free_streams)(USBDevice *dev, USBEndpoint **eps, int nr_eps);
+
     const char *product_desc;
     const USBDesc *usb_desc;
 } USBDeviceClass;
@@ -553,6 +561,10 @@ void usb_device_flush_ep_queue(USBDevice *dev, USBEndpoint *ep);
 
 void usb_device_ep_stopped(USBDevice *dev, USBEndpoint *ep);
 
+int usb_device_alloc_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps,
+                             int streams);
+void usb_device_free_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps);
+
 const char *usb_device_get_product_desc(USBDevice *dev);
 
 const USBDesc *usb_device_get_usb_desc(USBDevice *dev);
-- 
1.8.3.1

[Qemu-devel] [PATCH 10/13] xhci: Call usb_device_alloc/free_streams

[Hans de Goede]

Note this code is not as KISS as I would like, the reason for this is that
the Linux kernel interface wants streams on eps belonging to one interface
to be allocated in one call. Things will also work if we do this one ep at a
time (as long as all eps support the same amount of streams), but lets stick
to the kernel API.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/hcd-xhci.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index fa27299..c5889a9 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1151,6 +1151,111 @@ static void xhci_free_streams(XHCIEPContext *epctx)
     epctx->nr_pstreams = 0;
 }
 
+static int xhci_epmask_to_eps_with_streams(XHCIState *xhci,
+                                           unsigned int slotid,
+                                           uint32_t epmask,
+                                           XHCIEPContext **epctxs,
+                                           USBEndpoint **eps)
+{
+    XHCISlot *slot;
+    XHCIEPContext *epctx;
+    USBEndpoint *ep;
+    int i, j;
+
+    assert(slotid >= 1 && slotid <= xhci->numslots);
+
+    slot = &xhci->slots[slotid - 1];
+
+    for (i = 2, j = 0; i <= 31; i++) {
+        if (!(epmask & (1 << i))) {
+            continue;
+        }
+
+        epctx = slot->eps[i - 1];
+        ep = xhci_epid_to_usbep(xhci, slotid, i);
+        if (!epctx || !epctx->nr_pstreams || !ep) {
+            continue;
+        }
+
+        if (epctxs) {
+            epctxs[j] = epctx;
+        }
+        eps[j++] = ep;
+    }
+    return j;
+}
+
+static void xhci_free_device_streams(XHCIState *xhci, unsigned int slotid,
+                                     uint32_t epmask)
+{
+    USBEndpoint *eps[30];
+    int nr_eps;
+
+    nr_eps = xhci_epmask_to_eps_with_streams(xhci, slotid, epmask, NULL, eps);
+    if (nr_eps) {
+        usb_device_free_streams(eps[0]->dev, eps, nr_eps);
+    }
+}
+
+static TRBCCode xhci_alloc_device_streams(XHCIState *xhci, unsigned int slotid,
+                                          uint32_t epmask)
+{
+    XHCIEPContext *epctxs[30];
+    USBEndpoint *eps[30];
+    int i, r, nr_eps, req_nr_streams, dev_max_streams;
+
+    nr_eps = xhci_epmask_to_eps_with_streams(xhci, slotid, epmask, epctxs,
+                                             eps);
+    if (nr_eps == 0) {
+        return CC_SUCCESS;
+    }
+
+    req_nr_streams = epctxs[0]->nr_pstreams;
+    dev_max_streams = eps[0]->max_streams;
+
+    for (i = 1; i < nr_eps; i++) {
+        /*
+         * HdG: I don't expect these to ever trigger, but if they do we need
+         * to come up with another solution, ie group identical endpoints
+         * together and make an usb_device_alloc_streams call per group.
+         */
+        if (epctxs[i]->nr_pstreams != req_nr_streams) {
+            FIXME("guest streams config not identical for all eps");
+            return CC_RESOURCE_ERROR;
+        }
+        if (eps[i]->max_streams != dev_max_streams) {
+            FIXME("device streams config not identical for all eps");
+            return CC_RESOURCE_ERROR;
+        }
+    }
+
+    /*
+     * max-streams in both the device descriptor and in the controller is a
+     * power of 2. But stream id 0 is reserved, so if a device can do up to 4
+     * streams the guest will ask for 5 rounded up to the next power of 2 which
+     * becomes 8. For emulated devices usb_device_alloc_streams is a nop.
+     *
+     * For redirected devices however this is an issue, as there we must ask
+     * the real xhci controller to alloc streams, and the host driver for the
+     * real xhci controller will likely disallow allocating more streams then
+     * the device can handle.
+     *
+     * So we limit the requested nr_streams to the maximum number the device
+     * can handle.
+     */
+    if (req_nr_streams > dev_max_streams) {
+        req_nr_streams = dev_max_streams;
+    }
+
+    r = usb_device_alloc_streams(eps[0]->dev, eps, nr_eps, req_nr_streams);
+    if (r != 0) {
+        fprintf(stderr, "xhci: alloc streams failed\n");
+        return CC_RESOURCE_ERROR;
+    }
+
+    return CC_SUCCESS;
+}
+
 static XHCIStreamContext *xhci_find_stream(XHCIEPContext *epctx,
                                            unsigned int streamid,
                                            uint32_t *cc_error)
@@ -2314,6 +2419,8 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid,
         return CC_CONTEXT_STATE_ERROR;
     }
 
+    xhci_free_device_streams(xhci, slotid, ictl_ctx[0] | ictl_ctx[1]);
+
     for (i = 2; i <= 31; i++) {
         if (ictl_ctx[0] & (1<<i)) {
             xhci_disable_ep(xhci, slotid, i);
@@ -2335,6 +2442,16 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid,
         }
     }
 
+    res = xhci_alloc_device_streams(xhci, slotid, ictl_ctx[1]);
+    if (res != CC_SUCCESS) {
+        for (i = 2; i <= 31; i++) {
+            if (ictl_ctx[1] & (1 << i)) {
+                xhci_disable_ep(xhci, slotid, i);
+            }
+        }
+        return res;
+    }
+
     slot_ctx[3] &= ~(SLOT_STATE_MASK << SLOT_STATE_SHIFT);
     slot_ctx[3] |= SLOT_CONFIGURED << SLOT_STATE_SHIFT;
     slot_ctx[0] &= ~(SLOT_CONTEXT_ENTRIES_MASK << SLOT_CONTEXT_ENTRIES_SHIFT);
-- 
1.8.3.1

[Qemu-devel] [PATCH 11/13] usb-host-libusb: Fill in endpoint max_streams when available

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/host-libusb.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index fd320cd..0dd60b2 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -720,6 +720,9 @@ static void usb_host_ep_update(USBHostDevice *s)
     struct libusb_config_descriptor *conf;
     const struct libusb_interface_descriptor *intf;
     const struct libusb_endpoint_descriptor *endp;
+#if LIBUSBX_API_VERSION >= 0x01000102
+    struct libusb_ss_endpoint_companion_descriptor *endp_ss_comp;
+#endif
     uint8_t devep, type;
     int pid, ep;
     int rc, i, e;
@@ -765,6 +768,15 @@ static void usb_host_ep_update(USBHostDevice *s)
             usb_ep_set_type(udev, pid, ep, type);
             usb_ep_set_ifnum(udev, pid, ep, i);
             usb_ep_set_halted(udev, pid, ep, 0);
+#if LIBUSBX_API_VERSION >= 0x01000102
+            if (type == LIBUSB_TRANSFER_TYPE_BULK &&
+                    libusb_get_ss_endpoint_companion_descriptor(ctx, endp,
+                        &endp_ss_comp) == LIBUSB_SUCCESS) {
+                usb_ep_set_max_streams(udev, pid, ep,
+                                       endp_ss_comp->bmAttributes);
+                libusb_free_ss_endpoint_companion_descriptor(endp_ss_comp);
+            }
+#endif
         }
     }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 12/13] usb-host-libusb: Add alloc / free streams ops

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/host-libusb.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 0dd60b2..894875b 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -1280,6 +1280,54 @@ static void usb_host_handle_reset(USBDevice *udev)
     }
 }
 
+static int usb_host_alloc_streams(USBDevice *udev, USBEndpoint **eps,
+                                  int nr_eps, int streams)
+{
+#if LIBUSBX_API_VERSION >= 0x01000103
+    USBHostDevice *s = USB_HOST_DEVICE(udev);
+    unsigned char endpoints[30];
+    int i, rc;
+
+    for (i = 0; i < nr_eps; i++) {
+        endpoints[i] = eps[i]->nr;
+        if (eps[i]->pid == USB_TOKEN_IN) {
+            endpoints[i] |= 0x80;
+        }
+    }
+    rc = libusb_alloc_streams(s->dh, streams, endpoints, nr_eps);
+    if (rc < 0) {
+        usb_host_libusb_error("libusb_alloc_streams", rc);
+    } else if (rc != streams) {
+        fprintf(stderr,
+            "libusb_alloc_streams: got less streams then requested %d < %d\n",
+            rc, streams);
+    }
+
+    return (rc == streams) ? 0 : -1;
+#else
+    fprintf(stderr, "libusb_alloc_streams: error not implemented\n");
+    return -1;
+#endif
+}
+
+static void usb_host_free_streams(USBDevice *udev, USBEndpoint **eps,
+                                  int nr_eps)
+{
+#if LIBUSBX_API_VERSION >= 0x01000103
+    USBHostDevice *s = USB_HOST_DEVICE(udev);
+    unsigned char endpoints[30];
+    int i;
+
+    for (i = 0; i < nr_eps; i++) {
+        endpoints[i] = eps[i]->nr;
+        if (eps[i]->pid == USB_TOKEN_IN) {
+            endpoints[i] |= 0x80;
+        }
+    }
+    libusb_free_streams(s->dh, endpoints, nr_eps);
+#endif
+}
+
 /*
  * This is *NOT* about restoring state.  We have absolutely no idea
  * what state the host device is in at the moment and whenever it is
@@ -1361,6 +1409,8 @@ static void usb_host_class_initfn(ObjectClass *klass, void *data)
     uc->handle_reset   = usb_host_handle_reset;
     uc->handle_destroy = usb_host_handle_destroy;
     uc->flush_ep_queue = usb_host_flush_ep_queue;
+    uc->alloc_streams  = usb_host_alloc_streams;
+    uc->free_streams   = usb_host_free_streams;
     dc->vmsd = &vmstate_usb_host;
     dc->props = usb_host_dev_properties;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-- 
1.8.3.1

[Qemu-devel] [PATCH 13/13] usb-host-libusb: Set stream id when submitting bulk-stream transfers

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 hw/usb/host-libusb.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 894875b..3376b96 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -1214,10 +1214,22 @@ static void usb_host_handle_data(USBDevice *udev, USBPacket *p)
             usb_packet_copy(p, r->buffer, size);
         }
         ep = p->ep->nr | (r->in ? USB_DIR_IN : 0);
-        libusb_fill_bulk_transfer(r->xfer, s->dh, ep,
-                                  r->buffer, size,
-                                  usb_host_req_complete_data, r,
-                                  BULK_TIMEOUT);
+        if (p->stream) {
+#if LIBUSBX_API_VERSION >= 0x01000103
+            libusb_fill_bulk_stream_transfer(r->xfer, s->dh, ep, p->stream,
+                                             r->buffer, size,
+                                             usb_host_req_complete_data, r,
+                                             BULK_TIMEOUT);
+#else
+            usb_host_req_free(r);
+            return USB_RET_STALL;
+#endif
+        } else {
+            libusb_fill_bulk_transfer(r->xfer, s->dh, ep,
+                                      r->buffer, size,
+                                      usb_host_req_complete_data, r,
+                                      BULK_TIMEOUT);
+        }
         break;
     case USB_ENDPOINT_XFER_INT:
         r = usb_host_req_alloc(s, p, p->pid == USB_TOKEN_IN, p->iov.size);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 03/13] usb-host-libusb: Detach kernel drivers earlier

[Gerd Hoffmann]

On Di, 2013-10-08 at 21:58 +0200, Hans de Goede wrote:
> If we detach the kernel drivers on the first set_config, then they will
> be still attached when the device gets its initial reset. Causing the drivers
> to re-initialize the device after the reset, dirtying the device state.

> @@ -1051,7 +1055,6 @@ static void usb_host_set_config(USBHostDevice *s, int config, USBPacket *p)
>      trace_usb_host_set_config(s->bus_num, s->addr, config);
>  
>      usb_host_release_interfaces(s);
> -    usb_host_detach_kernel(s);
>      rc = libusb_set_configuration(s->dh, config);
>      if (rc != 0) {
>          usb_host_libusb_error("libusb_set_configuration", rc);

Sure we can safely remove the detach_kernel here?

Assuming we have a device with multiple configurations, each
configuration has a different set of interfaces, guest switches from one
config to another.  Do we correctly unbind kernel / claim interfaces
then?

Handling this correctly was the reason to do the kernel unbind in
response to the guests set_config instead of host_open btw, although I
can see the issues the late unbind introduces.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 03/13] usb-host-libusb: Detach kernel drivers earlier

[Hans de Goede]

Hi,

On 10/09/2013 10:55 AM, Gerd Hoffmann wrote:
> On Di, 2013-10-08 at 21:58 +0200, Hans de Goede wrote:
>> If we detach the kernel drivers on the first set_config, then they will
>> be still attached when the device gets its initial reset. Causing the drivers
>> to re-initialize the device after the reset, dirtying the device state.
>
>> @@ -1051,7 +1055,6 @@ static void usb_host_set_config(USBHostDevice *s, int config, USBPacket *p)
>>       trace_usb_host_set_config(s->bus_num, s->addr, config);
>>
>>       usb_host_release_interfaces(s);
>> -    usb_host_detach_kernel(s);
>>       rc = libusb_set_configuration(s->dh, config);
>>       if (rc != 0) {
>>           usb_host_libusb_error("libusb_set_configuration", rc);
>
> Sure we can safely remove the detach_kernel here?

Yes.

> Assuming we have a device with multiple configurations, each
> configuration has a different set of interfaces, guest switches from one
> config to another.  Do we correctly unbind kernel / claim interfaces
> then?

Yes we still have a usb_host_detach_kernel() call in the beginning
of usb_host_claim_interfaces(), which gets run on the new interfaces
immediately after the libusb_set_configuration call.

Doing a detach_kernel after a set_config has always been necessary,
since the kernel will automatically bind drivers to the new interfaces
if the set_config succeeds.

Doing detach_kernel before set_config is necessary if kernel drivers
are attached, because the kernel will not allow a set_config if any
drivers are bound. But we already do a set_config for the current
config on usb_host_open() now, and if a new config gets installed we
immediately detach the kernel drivers again from usb_host_claim_interfaces()
so when we enter usb_host_set_config no kernel drivers, other then
usbfs (the userspace access driver) will be attached, and usbfs is
detached by releasing the interfaces (*).

Regards,

Hans


*) In recent libusb versions libusb will even disallow using
libusb_detach_kernel_driver to detach usbfs, so as to stop a user space
program from breaking anothers userspaces program claim on the interface
by completelty detaching usbfs from the interface.

Re: [Qemu-devel] [PATCH 03/13] usb-host-libusb: Detach kernel drivers earlier

[Gerd Hoffmann]

  Hi,

> > Assuming we have a device with multiple configurations, each
> > configuration has a different set of interfaces, guest switches from one
> > config to another.  Do we correctly unbind kernel / claim interfaces
> > then?
> 
> Yes we still have a usb_host_detach_kernel() call in the beginning
> of usb_host_claim_interfaces(), which gets run on the new interfaces
> immediately after the libusb_set_configuration call.

Ok, good.

> Doing a detach_kernel after a set_config has always been necessary,
> since the kernel will automatically bind drivers to the new interfaces
> if the set_config succeeds.

Is there some way to avoid the kernel's autobind in the first place btw?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 03/13] usb-host-libusb: Detach kernel drivers earlier

[Hans de Goede]

Hi,

On 10/09/2013 03:35 PM, Gerd Hoffmann wrote:
>    Hi,
>
>>> Assuming we have a device with multiple configurations, each
>>> configuration has a different set of interfaces, guest switches from one
>>> config to another.  Do we correctly unbind kernel / claim interfaces
>>> then?
>>
>> Yes we still have a usb_host_detach_kernel() call in the beginning
>> of usb_host_claim_interfaces(), which gets run on the new interfaces
>> immediately after the libusb_set_configuration call.
>
> Ok, good.
>
>> Doing a detach_kernel after a set_config has always been necessary,
>> since the kernel will automatically bind drivers to the new interfaces
>> if the set_config succeeds.
>
> Is there some way to avoid the kernel's autobind in the first place btw?

Not atm, but so far the kernel guys have been open to adding (sane) API's
for things like this, and avoiding the whole re-bind after a set_config from
userspace would probably be nice to have.

Note that this is not triggering in 99% of all cases though, as the kernel has
this bit of code in its set_config handling for usbfs:

         /* SET_CONFIGURATION is often abused as a "cheap" driver reset,
          * so avoid usb_set_configuration()'s kick to sysfs
          */
         if (actconfig && actconfig->desc.bConfigurationValue == u)
                 status = usb_reset_configuration(ps->dev);
         else
                 status = usb_set_configuration(ps->dev, u);

So it only actually does a set_config (and binds the kernel drivers to
the interfaces) if the guest is asking for another config then the host os
has chosen for the device.

Since the guest assumes the device starts unconfigured, it does not issue
a set_config(0), only a set_config(1) (usually) which the above code
turns into a light weight reset.

Note that my usbredirhost code avoids even the unclaim / claim dance around
set_config and calling into the kernel at all in this case, it has:

     if (host->config &&
             host->config->bConfigurationValue == config) {
         goto exit;
     }

A downside of this is that not even the lightweight device reset is done,
but the guest always starts with a full device-reset, immediately following
that with a lightweight reset has little value.

I think we could and should to the same in host-libusb.c.

Regards,

Hans