[Qemu-devel] [PATCH 0/7] usb: packet pipelining

[Qemu-devel] [PATCH 0/7] usb: packet pipelining

[Gerd Hoffmann]

  Hi,

After a bunch of usb core restructions since release 1.0, this patch
series now puts the last bits into place, then flips the big switch.
usb can do pipelining now, i.e. have multiple usb transfers in flight
the same time.

Pipelining is supported by usb-host, uhci and xhci.  ehci can't handle
it yet.  Great opportunity to check out xhci ;)

The patch series features some xhci bugfixes too, with them applied xhci
and usb-host play alot better together.

please test & review,
  Gerd

Gerd Hoffmann (7):
  uhci_fill_queue: zap debug printf
  usb: queue can have async packets
  usb: add pipelining option to usb endpoints
  usb-host: enable pipelineing for bulk endpoints.
  usb: add shortcut for control transfers
  xhci: fix control xfers
  xhci: fix port status

 hw/usb-uhci.c |    1 -
 hw/usb-xhci.c |   15 ++++-------
 hw/usb.c      |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/usb.h      |    3 ++
 usb-linux.c   |    3 ++
 5 files changed, 84 insertions(+), 11 deletions(-)

[Qemu-devel] [PATCH 1/7] uhci_fill_queue: zap debug printf

[Gerd Hoffmann]

---
 hw/usb-uhci.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 70e3881..29cb2d8 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -950,7 +950,6 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td)
     UHCI_TD ptd;
     int ret;
 
-    fprintf(stderr, "%s: -- %x\n", __func__, token);
     while (is_valid(plink)) {
         pci_dma_read(&s->dev, plink & ~0xf, &ptd, sizeof(ptd));
         le32_to_cpus(&ptd.link);
-- 
1.7.1

[Qemu-devel] [PATCH 2/7] usb: queue can have async packets

[Gerd Hoffmann]

This can happen today in case the ->complete() callback queues up the
next packet.  Also we'll support pipelining soon, which allows to have
multiple packets per queue in flight (aka ASYNC) state.

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

diff --git a/hw/usb.c b/hw/usb.c
index 57fc5e3..fc41d62 100644
--- a/hw/usb.c
+++ b/hw/usb.c
@@ -356,6 +356,9 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p)
 
     while (!QTAILQ_EMPTY(&ep->queue)) {
         p = QTAILQ_FIRST(&ep->queue);
+        if (p->state == USB_PACKET_ASYNC) {
+            break;
+        }
         assert(p->state == USB_PACKET_QUEUED);
         ret = usb_process_one(p);
         if (ret == USB_RET_ASYNC) {
-- 
1.7.1

[Qemu-devel] [PATCH 3/7] usb: add pipelining option to usb endpoints

[Gerd Hoffmann]

With this patch applied USB drivers can enable pipelining per endpoint.
With pipelining enabled the usb core will continue submitting packets
even when there are still async transfers in flight instead of passing
them on one by one.

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

diff --git a/hw/usb.c b/hw/usb.c
index fc41d62..800d912 100644
--- a/hw/usb.c
+++ b/hw/usb.c
@@ -323,7 +323,7 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
     assert(p->state == USB_PACKET_SETUP);
     assert(p->ep != NULL);
 
-    if (QTAILQ_EMPTY(&p->ep->queue)) {
+    if (QTAILQ_EMPTY(&p->ep->queue) || p->ep->pipeline) {
         ret = usb_process_one(p);
         if (ret == USB_RET_ASYNC) {
             usb_packet_set_state(p, USB_PACKET_ASYNC);
@@ -468,6 +468,7 @@ void usb_ep_init(USBDevice *dev)
     dev->ep_ctl.type = USB_ENDPOINT_XFER_CONTROL;
     dev->ep_ctl.ifnum = 0;
     dev->ep_ctl.dev = dev;
+    dev->ep_ctl.pipeline = false;
     QTAILQ_INIT(&dev->ep_ctl.queue);
     for (ep = 0; ep < USB_MAX_ENDPOINTS; ep++) {
         dev->ep_in[ep].nr = ep + 1;
@@ -480,6 +481,8 @@ void usb_ep_init(USBDevice *dev)
         dev->ep_out[ep].ifnum = 0;
         dev->ep_in[ep].dev = dev;
         dev->ep_out[ep].dev = dev;
+        dev->ep_in[ep].pipeline = false;
+        dev->ep_out[ep].pipeline = false;
         QTAILQ_INIT(&dev->ep_in[ep].queue);
         QTAILQ_INIT(&dev->ep_out[ep].queue);
     }
@@ -593,3 +596,9 @@ int usb_ep_get_max_packet_size(USBDevice *dev, int pid, int ep)
     struct USBEndpoint *uep = usb_ep_get(dev, pid, ep);
     return uep->max_packet_size;
 }
+
+void usb_ep_set_pipeline(USBDevice *dev, int pid, int ep, bool enabled)
+{
+    struct USBEndpoint *uep = usb_ep_get(dev, pid, ep);
+    uep->pipeline = enabled;
+}
diff --git a/hw/usb.h b/hw/usb.h
index 8e83697..5bcc9b5 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -176,6 +176,7 @@ struct USBEndpoint {
     uint8_t type;
     uint8_t ifnum;
     int max_packet_size;
+    bool pipeline;
     USBDevice *dev;
     QTAILQ_HEAD(, USBPacket) queue;
 };
@@ -363,6 +364,7 @@ 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_pipeline(USBDevice *dev, int pid, int ep, bool enabled);
 
 void usb_attach(USBPort *port);
 void usb_detach(USBPort *port);
-- 
1.7.1

[Qemu-devel] [PATCH 4/7] usb-host: enable pipelineing for bulk endpoints.

[Gerd Hoffmann]

We really don't want to wait for packets finish before submitting the
next, we want keep the data flow running.

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

diff --git a/usb-linux.c b/usb-linux.c
index 47994f3..4ecb323 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1186,6 +1186,9 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
                    USB_ENDPOINT_XFER_INVALID);
             usb_ep_set_type(&s->dev, pid, ep, type);
             usb_ep_set_ifnum(&s->dev, pid, ep, interface);
+            if (type == USB_ENDPOINT_XFER_BULK) {
+                usb_ep_set_pipeline(&s->dev, pid, ep, true);
+            }
 
             epd = get_endp(s, pid, ep);
             epd->halted = 0;
-- 
1.7.1

[Qemu-devel] [PATCH 5/7] usb: add shortcut for control transfers

[Gerd Hoffmann]

Add a more direct code path to submit control transfers.  Instead of
feeding three usb packets (setup, data, ack) to usb_handle_packet and
have the do_token_* functions in usb.c poke the control transfer
parameters out of it just submit a single packet carrying the actual
data with the control xfer parameters filled into USBPacket->parameters.

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

diff --git a/hw/usb.c b/hw/usb.c
index 800d912..1ec2e90 100644
--- a/hw/usb.c
+++ b/hw/usb.c
@@ -95,6 +95,7 @@ void usb_wakeup(USBEndpoint *ep)
 #define SETUP_STATE_SETUP 1
 #define SETUP_STATE_DATA  2
 #define SETUP_STATE_ACK   3
+#define SETUP_STATE_PARAM 4
 
 static int do_token_setup(USBDevice *s, USBPacket *p)
 {
@@ -226,6 +227,50 @@ static int do_token_out(USBDevice *s, USBPacket *p)
     }
 }
 
+static int do_parameter(USBDevice *s, USBPacket *p)
+{
+    int request, value, index;
+    int i, ret = 0;
+
+    for (i = 0; i < 8; i++) {
+        s->setup_buf[i] = p->parameter >> (i*8);
+    }
+
+    s->setup_state = SETUP_STATE_PARAM;
+    s->setup_len   = (s->setup_buf[7] << 8) | s->setup_buf[6];
+    s->setup_index = 0;
+
+    request = (s->setup_buf[0] << 8) | s->setup_buf[1];
+    value   = (s->setup_buf[3] << 8) | s->setup_buf[2];
+    index   = (s->setup_buf[5] << 8) | s->setup_buf[4];
+
+    if (s->setup_len > sizeof(s->data_buf)) {
+        fprintf(stderr,
+                "usb_generic_handle_packet: ctrl buffer too small (%d > %zu)\n",
+                s->setup_len, sizeof(s->data_buf));
+        return USB_RET_STALL;
+    }
+
+    if (p->pid == USB_TOKEN_OUT) {
+        usb_packet_copy(p, s->data_buf, s->setup_len);
+    }
+
+    ret = usb_device_handle_control(s, p, request, value, index,
+                                    s->setup_len, s->data_buf);
+    if (ret < 0) {
+        return ret;
+    }
+
+    if (ret < s->setup_len) {
+        s->setup_len = ret;
+    }
+    if (p->pid == USB_TOKEN_IN) {
+        usb_packet_copy(p, s->data_buf, s->setup_len);
+    }
+
+    return ret;
+}
+
 /* ctrl complete function for devices which use usb_generic_handle_packet and
    may return USB_RET_ASYNC from their handle_control callback. Device code
    which does this *must* call this function instead of the normal
@@ -250,6 +295,16 @@ void usb_generic_async_ctrl_complete(USBDevice *s, USBPacket *p)
         p->result = 0;
         break;
 
+    case SETUP_STATE_PARAM:
+        if (p->result < s->setup_len) {
+            s->setup_len = p->result;
+        }
+        if (p->pid == USB_TOKEN_IN) {
+            p->result = 0;
+            usb_packet_copy(p, s->data_buf, s->setup_len);
+        }
+        break;
+
     default:
         break;
     }
@@ -292,6 +347,9 @@ static int usb_process_one(USBPacket *p)
 
     if (p->ep->nr == 0) {
         /* control pipe */
+        if (p->parameter) {
+            return do_parameter(dev, p);
+        }
         switch (p->pid) {
         case USB_TOKEN_SETUP:
             return do_token_setup(dev, p);
@@ -416,6 +474,7 @@ void usb_packet_setup(USBPacket *p, int pid, USBEndpoint *ep)
     p->pid = pid;
     p->ep = ep;
     p->result = 0;
+    p->parameter = 0;
     qemu_iovec_reset(&p->iov);
     usb_packet_set_state(p, USB_PACKET_SETUP);
 }
diff --git a/hw/usb.h b/hw/usb.h
index 5bcc9b5..24147e9 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -326,6 +326,7 @@ struct USBPacket {
     int pid;
     USBEndpoint *ep;
     QEMUIOVector iov;
+    uint64_t parameter; /* control transfers */
     int result; /* transfer length or USB_RET_* status code */
     /* Internal use by the USB layer.  */
     USBPacketState state;
-- 
1.7.1

[Qemu-devel] [PATCH 6/7] xhci: fix control xfers

[Gerd Hoffmann]

Use the new, direct control transfer submission method instead of
bypassing the usb core by calling usb_device_handle_control directly.
The later fails for async control transfers.

This patch gets xhci + usb-host combo going.
---
 hw/usb-xhci.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/hw/usb-xhci.c b/hw/usb-xhci.c
index fc5b542..8305489 100644
--- a/hw/usb-xhci.c
+++ b/hw/usb-xhci.c
@@ -1470,8 +1470,8 @@ static USBDevice *xhci_find_device(XHCIPort *port, uint8_t addr)
 static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer)
 {
     XHCITRB *trb_setup, *trb_status;
-    uint8_t bmRequestType, bRequest;
-    uint16_t wValue, wLength, wIndex;
+    uint8_t bmRequestType;
+    uint16_t wLength;
     XHCIPort *port;
     USBDevice *dev;
     int ret;
@@ -1508,9 +1508,6 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer)
     }
 
     bmRequestType = trb_setup->parameter;
-    bRequest = trb_setup->parameter >> 8;
-    wValue = trb_setup->parameter >> 16;
-    wIndex = trb_setup->parameter >> 32;
     wLength = trb_setup->parameter >> 48;
 
     if (xfer->data && xfer->data_alloced < wLength) {
@@ -1537,12 +1534,12 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer)
     xfer->iso_xfer = false;
 
     xhci_setup_packet(xfer, dev);
+    xfer->packet.parameter = trb_setup->parameter;
     if (!xfer->in_xfer) {
         xhci_xfer_data(xfer, xfer->data, wLength, 0, 1, 0);
     }
-    ret = usb_device_handle_control(dev, &xfer->packet,
-                                    (bmRequestType << 8) | bRequest,
-                                    wValue, wIndex, wLength, xfer->data);
+
+    ret = usb_handle_packet(dev, &xfer->packet);
 
     xhci_complete_packet(xfer, ret);
     if (!xfer->running_async && !xfer->running_retry) {
-- 
1.7.1

[Qemu-devel] [PATCH 7/7] xhci: fix port status

[Gerd Hoffmann]

Don't signal port status change if the usb device isn't in attached
state.  Happens with usb-host devices with the pass-through device
being plugged out at the host.

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

diff --git a/hw/usb-xhci.c b/hw/usb-xhci.c
index 8305489..e8f1b6e 100644
--- a/hw/usb-xhci.c
+++ b/hw/usb-xhci.c
@@ -2279,7 +2279,7 @@ static void xhci_update_port(XHCIState *xhci, XHCIPort *port, int is_detach)
     int nr = port->port.index + 1;
 
     port->portsc = PORTSC_PP;
-    if (port->port.dev && !is_detach) {
+    if (port->port.dev && port->port.dev->attached && !is_detach) {
         port->portsc |= PORTSC_CCS;
         switch (port->port.dev->speed) {
         case USB_SPEED_LOW:
-- 
1.7.1