diff --git a/hw/usb.h b/hw/usb.h index 432ccae57f..e5744773bd 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -179,6 +179,7 @@ struct USBEndpoint { uint8_t ifnum; int max_packet_size; bool pipeline; + bool halted; USBDevice *dev; QTAILQ_HEAD(, USBPacket) queue; }; diff --git a/hw/usb/core.c b/hw/usb/core.c index c7e5bc047f..28b840e52d 100644 --- a/hw/usb/core.c +++ b/hw/usb/core.c @@ -382,12 +382,23 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p) usb_packet_check_state(p, USB_PACKET_SETUP); assert(p->ep != NULL); + /* Submitting a new packet clears halt */ + if (p->ep->halted) { + assert(QTAILQ_EMPTY(&p->ep->queue)); + p->ep->halted = false; + } + 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); QTAILQ_INSERT_TAIL(&p->ep->queue, p, queue); } else { + /* + * When pipelining is enabled usb-devices must always return async, + * otherwise packets can complete out of order! + */ + assert(!p->ep->pipeline); p->result = ret; usb_packet_set_state(p, USB_PACKET_COMPLETE); } @@ -399,6 +410,20 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p) return ret; } +static void __usb_packet_complete(USBDevice *dev, USBPacket *p) +{ + USBEndpoint *ep = p->ep; + + assert(p->result != USB_RET_ASYNC && p->result != USB_RET_NAK); + + if (p->result < 0) { + ep->halted = true; + } + usb_packet_set_state(p, USB_PACKET_COMPLETE); + QTAILQ_REMOVE(&ep->queue, p, queue); + dev->port->ops->complete(dev->port, p); +} + /* Notify the controller that an async packet is complete. This should only be called for packets previously deferred by returning USB_RET_ASYNC from handle_packet. */ @@ -409,11 +434,9 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p) usb_packet_check_state(p, USB_PACKET_ASYNC); assert(QTAILQ_FIRST(&ep->queue) == p); - usb_packet_set_state(p, USB_PACKET_COMPLETE); - QTAILQ_REMOVE(&ep->queue, p, queue); - dev->port->ops->complete(dev->port, p); + __usb_packet_complete(dev, p); - while (!QTAILQ_EMPTY(&ep->queue)) { + while (!ep->halted && !QTAILQ_EMPTY(&ep->queue)) { p = QTAILQ_FIRST(&ep->queue); if (p->state == USB_PACKET_ASYNC) { break; @@ -425,9 +448,7 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p) break; } p->result = ret; - usb_packet_set_state(p, USB_PACKET_COMPLETE); - QTAILQ_REMOVE(&ep->queue, p, queue); - dev->port->ops->complete(dev->port, p); + __usb_packet_complete(ep->dev, p); } } diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 8b94b17723..8504a6a6b0 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -2138,6 +2138,19 @@ static int ehci_state_writeback(EHCIQueue *q) * bit is clear. */ if (q->qh.token & QTD_TOKEN_HALT) { + /* + * We should not do any further processing on a halted queue! + * This is esp. important for bulk endpoints with pipelining enabled + * (redirection to a real USB device), where we must cancel all the + * transfers after this one so that: + * 1) If they've completed already, they are not processed further + * causing more stalls, originating from the same failed transfer + * 2) If still in flight, they are cancelled before the guest does + * a clear stall, otherwise the guest and device can loose sync! + */ + while ((p = QTAILQ_FIRST(&q->packets)) != NULL) { + ehci_free_packet(p); + } ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH); again = 1; } else { diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 1ace2a41da..898773414e 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -748,6 +748,22 @@ static int uhci_complete_td(UHCIState *s, UHCI_TD *td, UHCIAsync *async, uint32_ return TD_RESULT_COMPLETE; out: + /* + * We should not do any further processing on a queue with errors! + * This is esp. important for bulk endpoints with pipelining enabled + * (redirection to a real USB device), where we must cancel all the + * transfers after this one so that: + * 1) If they've completed already, they are not processed further + * causing more stalls, originating from the same failed transfer + * 2) If still in flight, they are cancelled before the guest does + * a clear stall, otherwise the guest and device can loose sync! + */ + while (!QTAILQ_EMPTY(&async->queue->asyncs)) { + UHCIAsync *as = QTAILQ_FIRST(&async->queue->asyncs); + uhci_async_unlink(as); + uhci_async_cancel(as); + } + switch(ret) { case USB_RET_STALL: td->ctrl |= TD_CTRL_STALL;