From: Hans de Goede <address@hidden>
For controllers which queue up more then 1 packet at a time, we must halt the
ep queue, and inside the controller code cancel all pending packets on an
error.
There are multiple reasons for this:
1) Guests expect the controllers to halt ep queues on error, so that they
get the opportunity to cancel transfers which the scheduled after the failing
one, before processing continues
2) Not cancelling queued up packets after a failed transfer also messes up
the controller state machine, in the case of EHCI causing the following
assert to trigger: "assert(p->qtdaddr == q->qtdaddr)" at hcd-ehci.c:2075
3) For bulk endpoints with pipelining enabled (redirection to a real USB
device), we must cancel all the transfers after this a failed one so that:
a) If they've completed already, they are not processed further causing more
stalls to be reported, originating from the same failed transfer
b) If still in flight, they are cancelled before the guest does
a clear stall, otherwise the guest and device can loose sync!
Note this patch only touches the ehci and uhci controller changes, since AFAIK
no other controllers actually queue up multiple transfer. If I'm wrong on this
other controllers need to be updated too!
Also note that this patch was heavily tested with the ehci code, where I had
a reproducer for a device causing a transfer to fail. The uhci code is not
tested with actually failing transfers and could do with a thorough review!
Signed-off-by: Hans de Goede <address@hidden>
Signed-off-by: Gerd Hoffmann <address@hidden>
---
hw/usb.h | 1 +
hw/usb/core.c | 35 ++++++++++++++++++++++++++++-------
hw/usb/hcd-ehci.c | 13 +++++++++++++
hw/usb/hcd-uhci.c | 16 ++++++++++++++++
4 files changed, 58 insertions(+), 7 deletions(-)
diff --git a/hw/usb.h b/hw/usb.h
index 432ccae..e574477 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 c7e5bc0..28b840e 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)