From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O64sZ-0004Pz-QF for qemu-devel@nongnu.org; Sun, 25 Apr 2010 12:40:23 -0400 Received: from [140.186.70.92] (port=53624 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O64sX-0004NZ-JV for qemu-devel@nongnu.org; Sun, 25 Apr 2010 12:40:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O64sV-0002PO-DO for qemu-devel@nongnu.org; Sun, 25 Apr 2010 12:40:21 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:53128) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O64sU-0002Ou-S4 for qemu-devel@nongnu.org; Sun, 25 Apr 2010 12:40:19 -0400 Message-ID: <4BD4706A.60603@web.de> Date: Sun, 25 Apr 2010 18:40:10 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1272128818-24708-1-git-send-email-daahern@cisco.com> In-Reply-To: <1272128818-24708-1-git-send-email-daahern@cisco.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig099BBCA9002BFD7F5620753D" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [PATCH] Move 16k limitation in request size for host devices from ehci to usb-linux List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Ahern Cc: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig099BBCA9002BFD7F5620753D Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable 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. >=20 > USBFS cannot be increased to 20k due to constraints on memory use (want= s > 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. >=20 > Signed-off-by: David Ahern 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(-) >=20 > 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]; > =20 > - int more; > - > /* cached data from guest - needs to be flushed=20 > * when guest removes an entry (doorbell, handshake sequence) > */ > @@ -1005,43 +1003,6 @@ err: > // DPRINTF("Short packet condition\n"); > // TODO check 4.12 for splits > =20 > - /* 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 =3D &ehci->ports[i]; > - USBDevice *dev =3D port->dev; > - > - ehci->more =3D ret; > - > - ehci->usb_packet.pid =3D ehci->pid; > - ehci->usb_packet.devaddr =3D get_field(qh->epchar, QH_EPCHAR_D= EVADDR); > - ehci->usb_packet.devep =3D get_field(qh->epchar, QH_EPCHAR_EP)= ; > - ehci->usb_packet.data =3D ehci->buffer + ret; > - /* linux devio.c limits max to 16k */ > - ehci->usb_packet.len =3D ehci->tbytes - ret; > - ehci->usb_packet.complete_cb =3D ehci_async_complete_packet; > - ehci->usb_packet.complete_opaque =3D ehci; > - > - ret =3D 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 =3D=3D USB_RET_ASYNC) { > - ehci->async_port_in_progress =3D i; > - ehci->async_complete =3D 0; > - return ret; > - } > - if (ret < 0) { > - goto err; > - } > - } > - > - ret +=3D ehci->more; > - > if ((ret > ehci->tbytes) && (ehci->pid =3D=3D USB_TOKEN_IN)) {= > ret =3D USB_RET_BABBLE; > goto err; > @@ -1128,12 +1089,7 @@ static int ehci_execute(EHCIState *ehci, EHCIqh = *qh) > ehci->usb_packet.devaddr =3D devadr; > ehci->usb_packet.devep =3D endp; > ehci->usb_packet.data =3D ehci->buffer; > - /* linux devio.c limits max to 16k */ > - if (ehci->tbytes > 16384) { > - ehci->usb_packet.len =3D 16384; > - } else { > - ehci->usb_packet.len =3D ehci->tbytes; > - } > + ehci->usb_packet.len =3D ehci->tbytes; > ehci->usb_packet.complete_cb =3D ehci_async_complete_packet; > ehci->usb_packet.complete_opaque =3D ehci; > =20 > @@ -1611,7 +1567,7 @@ static int ehci_state_execute(EHCIState *ehci, in= t async, int *state) > =20 > if (async) > ehci->usbsts |=3D USBSTS_REC; > - ehci->more =3D 0; > + > ehci->exec_status =3D ehci_execute(ehci, qh); > if (ehci->exec_status =3D=3D USB_RET_PROCERR) { > again =3D -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 > =20 > USBPacket *packet; > USBHostDevice *hdev; > + > + int more; /* packet required multiple URBs */ > } AsyncURB; > =20 > 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 =3D aurb->urb.actual_length; > + p->len +=3D aurb->urb.actual_length; > if (aurb->urb.type =3D=3D USBDEVFS_URB_TYPE_CONTROL) {= > async_complete_ctrl(s, p); > } > @@ -263,7 +265,10 @@ static void async_complete(void *opaque) > break; > } > =20 > - usb_packet_complete(p); > + if (!aurb->more) { > + DPRINTF("invoking packet_complete. plen =3D %d\n", p->= len); > + usb_packet_complete(p); > + } > } > =20 > async_free(aurb); > @@ -408,69 +413,89 @@ static void usb_host_handle_destroy(USBDevice *de= v) > =20 > static int usb_linux_update_endp_table(USBHostDevice *s); > =20 > +/* 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 =3D p->len; > + uint8_t *pbuf =3D p->data; > =20 > - aurb =3D async_alloc(); > - aurb->hdev =3D s; > - aurb->packet =3D p; > + p->len =3D 0; > + while (rem) { > + aurb =3D async_alloc(); > + aurb->hdev =3D s; > + aurb->packet =3D p; > =20 > - urb =3D &aurb->urb; > + urb =3D &aurb->urb; > =20 > - if (p->pid =3D=3D USB_TOKEN_IN) { > - urb->endpoint =3D p->devep | 0x80; > - } else { > - urb->endpoint =3D p->devep; > - } > + if (p->pid =3D=3D USB_TOKEN_IN) { > + urb->endpoint =3D p->devep | 0x80; > + } else { > + urb->endpoint =3D p->devep; > + } > =20 > - if (is_halted(s, p->devep)) { > - ret =3D 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 =3D 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); > - } > =20 > - urb->buffer =3D p->data; > - urb->buffer_length =3D p->len; > + if (is_isoc(s, p->devep)) { > + /* Setup ISOC transfer */ > + urb->type =3D USBDEVFS_URB_TYPE_ISO; > + urb->flags =3D USBDEVFS_URB_ISO_ASAP; > + urb->number_of_packets =3D 1; > + urb->iso_frame_desc[0].length =3D p->len; > + } else { > + /* Setup bulk transfer */ > + urb->type =3D USBDEVFS_URB_TYPE_BULK; > + } > =20 > - if (is_isoc(s, p->devep)) { > - /* Setup ISOC transfer */ > - urb->type =3D USBDEVFS_URB_TYPE_ISO; > - urb->flags =3D USBDEVFS_URB_ISO_ASAP; > - urb->number_of_packets =3D 1; > - urb->iso_frame_desc[0].length =3D p->len; > - } else { > - /* Setup bulk transfer */ > - urb->type =3D USBDEVFS_URB_TYPE_BULK; > - } > + urb->usercontext =3D s; > =20 > - urb->usercontext =3D s; > + /* USBFS limits max request size to 16k */ > + if (rem > MAX_USBFS_BUFFER_SIZE) { > + len =3D MAX_USBFS_BUFFER_SIZE; > + aurb->more =3D 1; > + } else { > + len =3D rem; > + aurb->more =3D 0; > + } > + urb->buffer_length =3D len; > + urb->buffer =3D pbuf; > =20 > - ret =3D ioctl(s->fd, USBDEVFS_SUBMITURB, urb); > + ret =3D ioctl(s->fd, USBDEVFS_SUBMITURB, urb); > =20 > - 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); > =20 > - 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); > =20 > - 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 +=3D len; > + rem -=3D len; > } > =20 > - usb_defer_packet(p, async_cancel, aurb); > return USB_RET_ASYNC; > } > =20 --------------enig099BBCA9002BFD7F5620753D Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkvUcHAACgkQitSsb3rl5xQeJACfU4EcyL7caZbeIVJr5E/nE3vl AlAAoNE8n9sndwy75tVb8B0ltAmUQacW =Cl4/ -----END PGP SIGNATURE----- --------------enig099BBCA9002BFD7F5620753D--