[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] Move 16k limitation in request size for host de
From: |
Jan Kiszka |
Subject: |
[Qemu-devel] Re: [PATCH] Move 16k limitation in request size for host devices from ehci to usb-linux |
Date: |
Sun, 25 Apr 2010 18:40:10 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
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 <address@hidden>
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;
> }
>
signature.asc
Description: OpenPGP digital signature