From 9a77a0f58923443913e1071ffb47b74c54566e70 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 1 Nov 2012 17:15:01 +0100 Subject: usb: split packet result into actual_length + status Since with the ehci and xhci controllers a single packet can be larger then maxpacketsize, it is possible for the result of a single packet to be both having transferred some data as well as the transfer to have an error. An example would be an input transfer from a bulk endpoint successfully receiving 1 or more maxpacketsize packets from the device, followed by a packet signalling halt. While already touching all the devices and controllers handle_packet / handle_data / handle_control code, also change the return type of these functions to void, solely storing the status in the packet. To make the code paths for regular versus async packet handling more uniform. This patch unfortunately is somewhat invasive, since makeing the qemu usb core deal with this requires changes everywhere. This patch only prepares the usb core for this, all the hcd / device changes are done in such a way that there are no functional changes. This patch has been tested with uhci and ehci hcds, together with usb-audio, usb-hid and usb-storage devices, as well as with usb-redir redirection with a wide variety of real devices. Note that there is usually no need to directly set packet->actual_length form devices handle_data callback, as that is done by usb_packet_copy() Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/combined-packet.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) (limited to 'hw/usb/combined-packet.c') diff --git a/hw/usb/combined-packet.c b/hw/usb/combined-packet.c index 3904e71..e722198 100644 --- a/hw/usb/combined-packet.c +++ b/hw/usb/combined-packet.c @@ -46,7 +46,7 @@ void usb_combined_input_packet_complete(USBDevice *dev, USBPacket *p) USBEndpoint *ep = p->ep; USBPacket *next; enum { completing, complete, leftover }; - int result, state = completing; + int status, actual_length, state = completing; bool short_not_ok; if (combined == NULL) { @@ -56,27 +56,34 @@ void usb_combined_input_packet_complete(USBDevice *dev, USBPacket *p) assert(combined->first == p && p == QTAILQ_FIRST(&combined->packets)); - result = combined->first->result; + status = combined->first->status; + actual_length = combined->first->actual_length; short_not_ok = QTAILQ_LAST(&combined->packets, packets_head)->short_not_ok; QTAILQ_FOREACH_SAFE(p, &combined->packets, combined_entry, next) { if (state == completing) { /* Distribute data over uncombined packets */ - if (result >= p->iov.size) { - p->result = p->iov.size; + if (actual_length >= p->iov.size) { + p->actual_length = p->iov.size; } else { /* Send short or error packet to complete the transfer */ - p->result = result; + p->actual_length = actual_length; state = complete; } + /* Report status on the last packet */ + if (state == complete || next == NULL) { + p->status = status; + } else { + p->status = USB_RET_SUCCESS; + } p->short_not_ok = short_not_ok; usb_combined_packet_remove(combined, p); usb_packet_complete_one(dev, p); - result -= p->result; + actual_length -= p->actual_length; } else { /* Remove any leftover packets from the queue */ state = leftover; - p->result = USB_RET_REMOVE_FROM_QUEUE; + p->status = USB_RET_REMOVE_FROM_QUEUE; dev->port->ops->complete(dev->port, p); } } @@ -117,7 +124,7 @@ void usb_ep_combine_input_packets(USBEndpoint *ep) { USBPacket *p, *u, *next, *prev = NULL, *first = NULL; USBPort *port = ep->dev->port; - int ret, totalsize; + int totalsize; assert(ep->pipeline); assert(ep->pid == USB_TOKEN_IN); @@ -125,7 +132,7 @@ void usb_ep_combine_input_packets(USBEndpoint *ep) QTAILQ_FOREACH_SAFE(p, &ep->queue, queue, next) { /* Empty the queue on a halt */ if (ep->halted) { - p->result = USB_RET_REMOVE_FROM_QUEUE; + p->status = USB_RET_REMOVE_FROM_QUEUE; port->ops->complete(port, p); continue; } @@ -166,8 +173,8 @@ void usb_ep_combine_input_packets(USBEndpoint *ep) next == NULL || /* Work around for Linux usbfs bulk splitting + migration */ (totalsize == 16348 && p->int_req)) { - ret = usb_device_handle_data(ep->dev, first); - assert(ret == USB_RET_ASYNC); + usb_device_handle_data(ep->dev, first); + assert(first->status == USB_RET_ASYNC); if (first->combined) { QTAILQ_FOREACH(u, &first->combined->packets, combined_entry) { usb_packet_set_state(u, USB_PACKET_ASYNC); -- cgit v1.1 From ffd8a97fb33d3b036d61c508bd73ee7fa4051c6b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 1 Nov 2012 17:15:06 +0100 Subject: usb/combined-packet: Move freeing of combined to usb_combined_packet_remove() Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb/combined-packet.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) (limited to 'hw/usb/combined-packet.c') diff --git a/hw/usb/combined-packet.c b/hw/usb/combined-packet.c index e722198..4a0c299 100644 --- a/hw/usb/combined-packet.c +++ b/hw/usb/combined-packet.c @@ -31,12 +31,16 @@ static void usb_combined_packet_add(USBCombinedPacket *combined, USBPacket *p) p->combined = combined; } +/* Note will free combined when the last packet gets removed */ static void usb_combined_packet_remove(USBCombinedPacket *combined, USBPacket *p) { assert(p->combined == combined); p->combined = NULL; QTAILQ_REMOVE(&combined->packets, p, combined_entry); + if (QTAILQ_EMPTY(&combined->packets)) { + g_free(combined); + } } /* Also handles completion of non combined packets for pipelined input eps */ @@ -45,9 +49,8 @@ void usb_combined_input_packet_complete(USBDevice *dev, USBPacket *p) USBCombinedPacket *combined = p->combined; USBEndpoint *ep = p->ep; USBPacket *next; - enum { completing, complete, leftover }; - int status, actual_length, state = completing; - bool short_not_ok; + int status, actual_length; + bool short_not_ok, done = false; if (combined == NULL) { usb_packet_complete_one(dev, p); @@ -61,39 +64,34 @@ void usb_combined_input_packet_complete(USBDevice *dev, USBPacket *p) short_not_ok = QTAILQ_LAST(&combined->packets, packets_head)->short_not_ok; QTAILQ_FOREACH_SAFE(p, &combined->packets, combined_entry, next) { - if (state == completing) { + if (!done) { /* Distribute data over uncombined packets */ if (actual_length >= p->iov.size) { p->actual_length = p->iov.size; } else { /* Send short or error packet to complete the transfer */ p->actual_length = actual_length; - state = complete; + done = true; } /* Report status on the last packet */ - if (state == complete || next == NULL) { + if (done || next == NULL) { p->status = status; } else { p->status = USB_RET_SUCCESS; } p->short_not_ok = short_not_ok; + /* Note will free combined when the last packet gets removed! */ usb_combined_packet_remove(combined, p); usb_packet_complete_one(dev, p); actual_length -= p->actual_length; } else { /* Remove any leftover packets from the queue */ - state = leftover; p->status = USB_RET_REMOVE_FROM_QUEUE; + /* Note will free combined on the last packet! */ dev->port->ops->complete(dev->port, p); } } - /* - * If we had leftover packets the hcd driver will have cancelled them - * and usb_combined_packet_cancel has already freed combined! - */ - if (state != leftover) { - g_free(combined); - } + /* Do not use combined here, it has been freed! */ leave: /* Check if there are packets in the queue waiting for our completion */ usb_ep_combine_input_packets(ep); @@ -104,14 +102,13 @@ void usb_combined_packet_cancel(USBDevice *dev, USBPacket *p) { USBCombinedPacket *combined = p->combined; assert(combined != NULL); + USBPacket *first = p->combined->first; + /* Note will free combined on the last packet! */ usb_combined_packet_remove(combined, p); - if (p == combined->first) { + if (p == first) { usb_device_cancel_packet(dev, p); } - if (QTAILQ_EMPTY(&combined->packets)) { - g_free(combined); - } } /* -- cgit v1.1