Patchwork Move 16k limitation in request size for host devices from ehci to usb-linux

login
register
mail settings
Submitter David S. Ahern
Date April 24, 2010, 5:06 p.m.
Message ID <1272128818-24708-1-git-send-email-daahern@cisco.com>
Download mbox | patch
Permalink /patch/50914/
State New
Headers show

Comments

David S. Ahern - April 24, 2010, 5:06 p.m.
Update usb-linux to handle maximum of 16k transfers. The 16k limit
is imposed by USBFS. EHCI allows up to 20k per request.

USBFS cannot be increased to 20k due to constraints on memory use (wants
contiguous memory in allocations that are powers of 2). This change
breaks large requests from a host controller into 2 URB submissions
to the host devices.

Signed-off-by: David Ahern <daahern@cisco.com>
---
 hw/usb-ehci.c |   48 +----------------------
 usb-linux.c   |  121 ++++++++++++++++++++++++++++++++++----------------------
 2 files changed, 75 insertions(+), 94 deletions(-)
Jan Kiszka - April 25, 2010, 4:40 p.m.
David Ahern wrote:
> Update usb-linux to handle maximum of 16k transfers. The 16k limit
> is imposed by USBFS. EHCI allows up to 20k per request.
> 
> USBFS cannot be increased to 20k due to constraints on memory use (wants
> contiguous memory in allocations that are powers of 2). This change
> breaks large requests from a host controller into 2 URB submissions
> to the host devices.
> 
> Signed-off-by: David Ahern <daahern@cisco.com>

Thanks, applied.

Jan

PS: This is what I get when attaching an external USB disk:

husb: open device 1.12
husb: config #1 need -1
husb: 1 interfaces claimed for configuration 1
husb: grabbed usb device 1.12
husb: config #1 need 1
husb: 1 interfaces claimed for configuration 1
husb: config #1 need 1
husb: 1 interfaces claimed for configuration 1
husb: config #1 need 1
husb: 1 interfaces claimed for configuration 1
USB stall
USB stall

That "stall" is irritating - but it seems to work fine otherwise.

> ---
>  hw/usb-ehci.c |   48 +----------------------
>  usb-linux.c   |  121 ++++++++++++++++++++++++++++++++++----------------------
>  2 files changed, 75 insertions(+), 94 deletions(-)
> 
> diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
> index c91a6b5..29baf74 100644
> --- a/hw/usb-ehci.c
> +++ b/hw/usb-ehci.c
> @@ -380,8 +380,6 @@ typedef struct {
>      USBPort ports[NB_PORTS];
>      uint8_t buffer[BUFF_SIZE];
>  
> -    int more;
> -
>      /* cached data from guest - needs to be flushed 
>       * when guest removes an entry (doorbell, handshake sequence)
>       */
> @@ -1005,43 +1003,6 @@ err:
>          // DPRINTF("Short packet condition\n");
>          // TODO check 4.12 for splits
>  
> -        /* see if a follow-up rquest is needed - request for
> -         * 20k yet OS only allows 16k requests at a time
> -         */
> -        if ((ret > 0) && (ehci->tbytes > 16384) && !ehci->more) {
> -        /* TO-DO: put this in a function for use here and by execute */
> -        USBPort *port = &ehci->ports[i];
> -        USBDevice *dev = port->dev;
> -
> -        ehci->more = ret;
> -
> -        ehci->usb_packet.pid = ehci->pid;
> -        ehci->usb_packet.devaddr = get_field(qh->epchar, QH_EPCHAR_DEVADDR);
> -        ehci->usb_packet.devep = get_field(qh->epchar, QH_EPCHAR_EP);
> -        ehci->usb_packet.data = ehci->buffer + ret;
> -        /* linux devio.c limits max to 16k */
> -        ehci->usb_packet.len = ehci->tbytes - ret;
> -        ehci->usb_packet.complete_cb = ehci_async_complete_packet;
> -        ehci->usb_packet.complete_opaque = ehci;
> -
> -        ret = dev->info->handle_packet(dev, &ehci->usb_packet);
> -
> -        DPRINTF("submit followup: qh %x qtd %x pid %x len %d ret %d\n",
> -                ehci->qhaddr, ehci->qtdaddr, ehci->pid,
> -                ehci->usb_packet.len, ret);
> -
> -        if (ret == USB_RET_ASYNC) {
> -            ehci->async_port_in_progress = i;
> -            ehci->async_complete = 0;
> -            return ret;
> -        }
> -        if (ret < 0) {
> -            goto err;
> -        }
> -        }
> -
> -        ret += ehci->more;
> -
>          if ((ret > ehci->tbytes) && (ehci->pid == USB_TOKEN_IN)) {
>              ret = USB_RET_BABBLE;
>              goto err;
> @@ -1128,12 +1089,7 @@ static int ehci_execute(EHCIState *ehci, EHCIqh *qh)
>          ehci->usb_packet.devaddr = devadr;
>          ehci->usb_packet.devep = endp;
>          ehci->usb_packet.data = ehci->buffer;
> -        /* linux devio.c limits max to 16k */
> -        if (ehci->tbytes > 16384) {
> -            ehci->usb_packet.len = 16384;
> -        } else {
> -            ehci->usb_packet.len = ehci->tbytes;
> -        }
> +        ehci->usb_packet.len = ehci->tbytes;
>          ehci->usb_packet.complete_cb = ehci_async_complete_packet;
>          ehci->usb_packet.complete_opaque = ehci;
>  
> @@ -1611,7 +1567,7 @@ static int ehci_state_execute(EHCIState *ehci, int async, int *state)
>  
>      if (async)
>          ehci->usbsts |= USBSTS_REC;
> -    ehci->more = 0;
> +
>      ehci->exec_status = ehci_execute(ehci, qh);
>      if (ehci->exec_status == USB_RET_PROCERR) {
>          again = -1;
> diff --git a/usb-linux.c b/usb-linux.c
> index b3d6b28..7181e2f 100644
> --- a/usb-linux.c
> +++ b/usb-linux.c
> @@ -182,6 +182,8 @@ typedef struct AsyncURB
>  
>      USBPacket     *packet;
>      USBHostDevice *hdev;
> +
> +    int more;     /* packet required multiple URBs */
>  } AsyncURB;
>  
>  static AsyncURB *async_alloc(void)
> @@ -247,7 +249,7 @@ static void async_complete(void *opaque)
>          if (p) {
>              switch (aurb->urb.status) {
>              case 0:
> -                p->len = aurb->urb.actual_length;
> +                p->len += aurb->urb.actual_length;
>                  if (aurb->urb.type == USBDEVFS_URB_TYPE_CONTROL) {
>                      async_complete_ctrl(s, p);
>                  }
> @@ -263,7 +265,10 @@ static void async_complete(void *opaque)
>                  break;
>              }
>  
> -            usb_packet_complete(p);
> +            if (!aurb->more) {
> +                DPRINTF("invoking packet_complete. plen = %d\n", p->len);
> +                usb_packet_complete(p);
> +            }
>          }
>  
>          async_free(aurb);
> @@ -408,69 +413,89 @@ static void usb_host_handle_destroy(USBDevice *dev)
>  
>  static int usb_linux_update_endp_table(USBHostDevice *s);
>  
> +/* devio.c limits single requests to 16k */
> +#define MAX_USBFS_BUFFER_SIZE 16384
> +
>  static int usb_host_handle_data(USBHostDevice *s, USBPacket *p)
>  {
>      struct usbdevfs_urb *urb;
>      AsyncURB *aurb;
> -    int ret;
> +    int ret, len;
> +    int rem = p->len;
> +    uint8_t *pbuf = p->data;
>  
> -    aurb = async_alloc();
> -    aurb->hdev   = s;
> -    aurb->packet = p;
> +    p->len = 0;
> +    while (rem) {
> +        aurb = async_alloc();
> +        aurb->hdev   = s;
> +        aurb->packet = p;
>  
> -    urb = &aurb->urb;
> +        urb = &aurb->urb;
>  
> -    if (p->pid == USB_TOKEN_IN) {
> -        urb->endpoint = p->devep | 0x80;
> -    } else {
> -        urb->endpoint = p->devep;
> -    }
> +        if (p->pid == USB_TOKEN_IN) {
> +            urb->endpoint = p->devep | 0x80;
> +        } else {
> +            urb->endpoint = p->devep;
> +        }
>  
> -    if (is_halted(s, p->devep)) {
> -        ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &urb->endpoint);
> -        if (ret < 0) {
> -            DPRINTF("husb: failed to clear halt. ep 0x%x errno %d\n",
> -                   urb->endpoint, errno);
> -            return USB_RET_NAK;
> +        if (is_halted(s, p->devep)) {
> +            ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &urb->endpoint);
> +            if (ret < 0) {
> +                DPRINTF("husb: failed to clear halt. ep 0x%x errno %d\n",
> +                        urb->endpoint, errno);
> +                return USB_RET_NAK;
> +            }
> +            clear_halt(s, p->devep);
>          }
> -        clear_halt(s, p->devep);
> -    }
>  
> -    urb->buffer        = p->data;
> -    urb->buffer_length = p->len;
> +        if (is_isoc(s, p->devep)) {
> +            /* Setup ISOC transfer */
> +            urb->type     = USBDEVFS_URB_TYPE_ISO;
> +            urb->flags    = USBDEVFS_URB_ISO_ASAP;
> +            urb->number_of_packets = 1;
> +            urb->iso_frame_desc[0].length = p->len;
> +        } else {
> +            /* Setup bulk transfer */
> +            urb->type     = USBDEVFS_URB_TYPE_BULK;
> +        }
>  
> -    if (is_isoc(s, p->devep)) {
> -        /* Setup ISOC transfer */
> -        urb->type     = USBDEVFS_URB_TYPE_ISO;
> -        urb->flags    = USBDEVFS_URB_ISO_ASAP;
> -        urb->number_of_packets = 1;
> -        urb->iso_frame_desc[0].length = p->len;
> -    } else {
> -        /* Setup bulk transfer */
> -        urb->type     = USBDEVFS_URB_TYPE_BULK;
> -    }
> +        urb->usercontext = s;
>  
> -    urb->usercontext = s;
> +        /* USBFS limits max request size to 16k */
> +        if (rem > MAX_USBFS_BUFFER_SIZE) {
> +            len = MAX_USBFS_BUFFER_SIZE;
> +            aurb->more = 1;
> +        } else {
> +            len = rem;
> +            aurb->more = 0;
> +        }
> +        urb->buffer_length = len;
> +        urb->buffer = pbuf;
>  
> -    ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
> +        ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
>  
> -    DPRINTF("husb: data submit. ep 0x%x len %u aurb %p\n",
> -            urb->endpoint, p->len, aurb);
> +        DPRINTF("husb: data submit. ep 0x%x len %u aurb %p\n",
> +                urb->endpoint, len, aurb);
>  
> -    if (ret < 0) {
> -        DPRINTF("husb: submit failed. errno %d\n", errno);
> -        async_free(aurb);
> +        if (ret < 0) {
> +            DPRINTF("husb: submit failed. errno %d\n", errno);
> +                async_free(aurb);
>  
> -        switch(errno) {
> -        case ETIMEDOUT:
> -            return USB_RET_NAK;
> -        case EPIPE:
> -        default:
> -            return USB_RET_STALL;
> +            switch(errno) {
> +            case ETIMEDOUT:
> +                return USB_RET_NAK;
> +            case EPIPE:
> +            default:
> +                return USB_RET_STALL;
> +            }
>          }
> +
> +        usb_defer_packet(p, async_cancel, aurb);
> +
> +        pbuf += len;
> +        rem -= len;
>      }
>  
> -    usb_defer_packet(p, async_cancel, aurb);
>      return USB_RET_ASYNC;
>  }
>

Patch

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index c91a6b5..29baf74 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -380,8 +380,6 @@  typedef struct {
     USBPort ports[NB_PORTS];
     uint8_t buffer[BUFF_SIZE];
 
-    int more;
-
     /* cached data from guest - needs to be flushed 
      * when guest removes an entry (doorbell, handshake sequence)
      */
@@ -1005,43 +1003,6 @@  err:
         // DPRINTF("Short packet condition\n");
         // TODO check 4.12 for splits
 
-        /* see if a follow-up rquest is needed - request for
-         * 20k yet OS only allows 16k requests at a time
-         */
-        if ((ret > 0) && (ehci->tbytes > 16384) && !ehci->more) {
-        /* TO-DO: put this in a function for use here and by execute */
-        USBPort *port = &ehci->ports[i];
-        USBDevice *dev = port->dev;
-
-        ehci->more = ret;
-
-        ehci->usb_packet.pid = ehci->pid;
-        ehci->usb_packet.devaddr = get_field(qh->epchar, QH_EPCHAR_DEVADDR);
-        ehci->usb_packet.devep = get_field(qh->epchar, QH_EPCHAR_EP);
-        ehci->usb_packet.data = ehci->buffer + ret;
-        /* linux devio.c limits max to 16k */
-        ehci->usb_packet.len = ehci->tbytes - ret;
-        ehci->usb_packet.complete_cb = ehci_async_complete_packet;
-        ehci->usb_packet.complete_opaque = ehci;
-
-        ret = dev->info->handle_packet(dev, &ehci->usb_packet);
-
-        DPRINTF("submit followup: qh %x qtd %x pid %x len %d ret %d\n",
-                ehci->qhaddr, ehci->qtdaddr, ehci->pid,
-                ehci->usb_packet.len, ret);
-
-        if (ret == USB_RET_ASYNC) {
-            ehci->async_port_in_progress = i;
-            ehci->async_complete = 0;
-            return ret;
-        }
-        if (ret < 0) {
-            goto err;
-        }
-        }
-
-        ret += ehci->more;
-
         if ((ret > ehci->tbytes) && (ehci->pid == USB_TOKEN_IN)) {
             ret = USB_RET_BABBLE;
             goto err;
@@ -1128,12 +1089,7 @@  static int ehci_execute(EHCIState *ehci, EHCIqh *qh)
         ehci->usb_packet.devaddr = devadr;
         ehci->usb_packet.devep = endp;
         ehci->usb_packet.data = ehci->buffer;
-        /* linux devio.c limits max to 16k */
-        if (ehci->tbytes > 16384) {
-            ehci->usb_packet.len = 16384;
-        } else {
-            ehci->usb_packet.len = ehci->tbytes;
-        }
+        ehci->usb_packet.len = ehci->tbytes;
         ehci->usb_packet.complete_cb = ehci_async_complete_packet;
         ehci->usb_packet.complete_opaque = ehci;
 
@@ -1611,7 +1567,7 @@  static int ehci_state_execute(EHCIState *ehci, int async, int *state)
 
     if (async)
         ehci->usbsts |= USBSTS_REC;
-    ehci->more = 0;
+
     ehci->exec_status = ehci_execute(ehci, qh);
     if (ehci->exec_status == USB_RET_PROCERR) {
         again = -1;
diff --git a/usb-linux.c b/usb-linux.c
index b3d6b28..7181e2f 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -182,6 +182,8 @@  typedef struct AsyncURB
 
     USBPacket     *packet;
     USBHostDevice *hdev;
+
+    int more;     /* packet required multiple URBs */
 } AsyncURB;
 
 static AsyncURB *async_alloc(void)
@@ -247,7 +249,7 @@  static void async_complete(void *opaque)
         if (p) {
             switch (aurb->urb.status) {
             case 0:
-                p->len = aurb->urb.actual_length;
+                p->len += aurb->urb.actual_length;
                 if (aurb->urb.type == USBDEVFS_URB_TYPE_CONTROL) {
                     async_complete_ctrl(s, p);
                 }
@@ -263,7 +265,10 @@  static void async_complete(void *opaque)
                 break;
             }
 
-            usb_packet_complete(p);
+            if (!aurb->more) {
+                DPRINTF("invoking packet_complete. plen = %d\n", p->len);
+                usb_packet_complete(p);
+            }
         }
 
         async_free(aurb);
@@ -408,69 +413,89 @@  static void usb_host_handle_destroy(USBDevice *dev)
 
 static int usb_linux_update_endp_table(USBHostDevice *s);
 
+/* devio.c limits single requests to 16k */
+#define MAX_USBFS_BUFFER_SIZE 16384
+
 static int usb_host_handle_data(USBHostDevice *s, USBPacket *p)
 {
     struct usbdevfs_urb *urb;
     AsyncURB *aurb;
-    int ret;
+    int ret, len;
+    int rem = p->len;
+    uint8_t *pbuf = p->data;
 
-    aurb = async_alloc();
-    aurb->hdev   = s;
-    aurb->packet = p;
+    p->len = 0;
+    while (rem) {
+        aurb = async_alloc();
+        aurb->hdev   = s;
+        aurb->packet = p;
 
-    urb = &aurb->urb;
+        urb = &aurb->urb;
 
-    if (p->pid == USB_TOKEN_IN) {
-        urb->endpoint = p->devep | 0x80;
-    } else {
-        urb->endpoint = p->devep;
-    }
+        if (p->pid == USB_TOKEN_IN) {
+            urb->endpoint = p->devep | 0x80;
+        } else {
+            urb->endpoint = p->devep;
+        }
 
-    if (is_halted(s, p->devep)) {
-        ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &urb->endpoint);
-        if (ret < 0) {
-            DPRINTF("husb: failed to clear halt. ep 0x%x errno %d\n",
-                   urb->endpoint, errno);
-            return USB_RET_NAK;
+        if (is_halted(s, p->devep)) {
+            ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &urb->endpoint);
+            if (ret < 0) {
+                DPRINTF("husb: failed to clear halt. ep 0x%x errno %d\n",
+                        urb->endpoint, errno);
+                return USB_RET_NAK;
+            }
+            clear_halt(s, p->devep);
         }
-        clear_halt(s, p->devep);
-    }
 
-    urb->buffer        = p->data;
-    urb->buffer_length = p->len;
+        if (is_isoc(s, p->devep)) {
+            /* Setup ISOC transfer */
+            urb->type     = USBDEVFS_URB_TYPE_ISO;
+            urb->flags    = USBDEVFS_URB_ISO_ASAP;
+            urb->number_of_packets = 1;
+            urb->iso_frame_desc[0].length = p->len;
+        } else {
+            /* Setup bulk transfer */
+            urb->type     = USBDEVFS_URB_TYPE_BULK;
+        }
 
-    if (is_isoc(s, p->devep)) {
-        /* Setup ISOC transfer */
-        urb->type     = USBDEVFS_URB_TYPE_ISO;
-        urb->flags    = USBDEVFS_URB_ISO_ASAP;
-        urb->number_of_packets = 1;
-        urb->iso_frame_desc[0].length = p->len;
-    } else {
-        /* Setup bulk transfer */
-        urb->type     = USBDEVFS_URB_TYPE_BULK;
-    }
+        urb->usercontext = s;
 
-    urb->usercontext = s;
+        /* USBFS limits max request size to 16k */
+        if (rem > MAX_USBFS_BUFFER_SIZE) {
+            len = MAX_USBFS_BUFFER_SIZE;
+            aurb->more = 1;
+        } else {
+            len = rem;
+            aurb->more = 0;
+        }
+        urb->buffer_length = len;
+        urb->buffer = pbuf;
 
-    ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
+        ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
 
-    DPRINTF("husb: data submit. ep 0x%x len %u aurb %p\n",
-            urb->endpoint, p->len, aurb);
+        DPRINTF("husb: data submit. ep 0x%x len %u aurb %p\n",
+                urb->endpoint, len, aurb);
 
-    if (ret < 0) {
-        DPRINTF("husb: submit failed. errno %d\n", errno);
-        async_free(aurb);
+        if (ret < 0) {
+            DPRINTF("husb: submit failed. errno %d\n", errno);
+                async_free(aurb);
 
-        switch(errno) {
-        case ETIMEDOUT:
-            return USB_RET_NAK;
-        case EPIPE:
-        default:
-            return USB_RET_STALL;
+            switch(errno) {
+            case ETIMEDOUT:
+                return USB_RET_NAK;
+            case EPIPE:
+            default:
+                return USB_RET_STALL;
+            }
         }
+
+        usb_defer_packet(p, async_cancel, aurb);
+
+        pbuf += len;
+        rem -= len;
     }
 
-    usb_defer_packet(p, async_cancel, aurb);
     return USB_RET_ASYNC;
 }