129 lines
5.0 KiB
Diff
129 lines
5.0 KiB
Diff
From dc9b6b076e03412e67597c207f2202493e37db06 Mon Sep 17 00:00:00 2001
|
|
From: Hans de Goede <hdegoede@redhat.com>
|
|
Date: Thu, 20 Sep 2012 16:55:02 +0200
|
|
Subject: [PATCH] ehci: Fix interrupt packet MULT handling
|
|
|
|
There are several issues with our handling of the MULT epcap field
|
|
of interrupt qhs, which this patch fixes.
|
|
|
|
1) When we don't execute a transaction because of the transaction counter
|
|
being 0, p->async stays EHCI_ASYNC_NONE, and the next time we process the
|
|
same qtd we hit an assert in ehci_state_fetchqtd because of this. Even though
|
|
I believe that this is caused by 3 below, this patch still removes the assert,
|
|
as that can still happen without 3, when multiple packets are queued for the
|
|
same interrupt ep.
|
|
|
|
2) We only *check* the transaction counter from ehci_state_execute, any
|
|
packets queued up by fill_queue bypass this check. This is fixed by not calling
|
|
fill_queue for interrupt packets.
|
|
|
|
3) Some versions of Windows set the MULT field of the qh to 0, which is a
|
|
clear violation of the EHCI spec, but still they do it. This means that we
|
|
will never execute a qtd for these, making interrupt ep-s on USB-2 devices
|
|
not work, and after recent changes, triggering 1).
|
|
|
|
So far we've stored the transaction counter in our copy of the mult field,
|
|
but with this beginnig at 0 already when dealing with these version of windows
|
|
this won't work. So this patch adds a transact_ctr field to our qh struct,
|
|
and sets this to the MULT field value on fetchqh. When the MULT field value
|
|
is 0, we set it to 4. Assuming that windows gets way with setting it to 0,
|
|
by the actual hardware going horizontal on a 1 -> 0 transition, which will
|
|
give it 4 transactions (MULT goes from 0 - 3).
|
|
|
|
Note that we cannot stop on detecting the 1 -> 0 transition, as our decrement
|
|
of the transaction counter, and checking for it are done in 2 different places.
|
|
|
|
Reported-by: Shawn Starr <shawn.starr@rogers.com>
|
|
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
|
|
---
|
|
hw/usb/hcd-ehci.c | 39 +++++++++++++++++++--------------------
|
|
1 file changed, 19 insertions(+), 20 deletions(-)
|
|
|
|
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
|
|
index 6a5da84..46f6d99 100644
|
|
--- a/hw/usb/hcd-ehci.c
|
|
+++ b/hw/usb/hcd-ehci.c
|
|
@@ -373,6 +373,7 @@ struct EHCIQueue {
|
|
uint32_t seen;
|
|
uint64_t ts;
|
|
int async;
|
|
+ int transact_ctr;
|
|
|
|
/* cached data from guest - needs to be flushed
|
|
* when guest removes an entry (doorbell, handshake sequence)
|
|
@@ -1837,6 +1838,10 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
|
|
}
|
|
q->qh = qh;
|
|
|
|
+ q->transact_ctr = get_field(q->qh.epcap, QH_EPCAP_MULT);
|
|
+ if (q->transact_ctr == 0) /* Guest bug in some versions of windows */
|
|
+ q->transact_ctr = 4;
|
|
+
|
|
if (q->dev == NULL) {
|
|
q->dev = ehci_find_device(q->ehci, devaddr);
|
|
}
|
|
@@ -2014,11 +2019,8 @@ static int ehci_state_fetchqtd(EHCIQueue *q)
|
|
} else if (p != NULL) {
|
|
switch (p->async) {
|
|
case EHCI_ASYNC_NONE:
|
|
- /* Should never happen packet should at least be initialized */
|
|
- assert(0);
|
|
- break;
|
|
case EHCI_ASYNC_INITIALIZED:
|
|
- /* Previously nacked packet (likely interrupt ep) */
|
|
+ /* Not yet executed (MULT), or previously nacked (int) packet */
|
|
ehci_set_state(q->ehci, q->async, EST_EXECUTE);
|
|
break;
|
|
case EHCI_ASYNC_INFLIGHT:
|
|
@@ -2107,15 +2109,12 @@ static int ehci_state_execute(EHCIQueue *q)
|
|
|
|
// TODO verify enough time remains in the uframe as in 4.4.1.1
|
|
// TODO write back ptr to async list when done or out of time
|
|
- // TODO Windows does not seem to ever set the MULT field
|
|
|
|
- if (!q->async) {
|
|
- int transactCtr = get_field(q->qh.epcap, QH_EPCAP_MULT);
|
|
- if (!transactCtr) {
|
|
- ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
|
|
- again = 1;
|
|
- goto out;
|
|
- }
|
|
+ /* 4.10.3, bottom of page 82, go horizontal on transaction counter == 0 */
|
|
+ if (!q->async && q->transact_ctr == 0) {
|
|
+ ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
|
|
+ again = 1;
|
|
+ goto out;
|
|
}
|
|
|
|
if (q->async) {
|
|
@@ -2132,7 +2131,11 @@ static int ehci_state_execute(EHCIQueue *q)
|
|
trace_usb_ehci_packet_action(p->queue, p, "async");
|
|
p->async = EHCI_ASYNC_INFLIGHT;
|
|
ehci_set_state(q->ehci, q->async, EST_HORIZONTALQH);
|
|
- again = (ehci_fill_queue(p) == USB_RET_PROCERR) ? -1 : 1;
|
|
+ if (q->async) {
|
|
+ again = (ehci_fill_queue(p) == USB_RET_PROCERR) ? -1 : 1;
|
|
+ } else {
|
|
+ again = 1;
|
|
+ }
|
|
goto out;
|
|
}
|
|
|
|
@@ -2152,13 +2155,9 @@ static int ehci_state_executing(EHCIQueue *q)
|
|
|
|
ehci_execute_complete(q);
|
|
|
|
- // 4.10.3
|
|
- if (!q->async) {
|
|
- int transactCtr = get_field(q->qh.epcap, QH_EPCAP_MULT);
|
|
- transactCtr--;
|
|
- set_field(&q->qh.epcap, transactCtr, QH_EPCAP_MULT);
|
|
- // 4.10.3, bottom of page 82, should exit this state when transaction
|
|
- // counter decrements to 0
|
|
+ /* 4.10.3 */
|
|
+ if (!q->async && q->transact_ctr > 0) {
|
|
+ q->transact_ctr--;
|
|
}
|
|
|
|
/* 4.10.5 */
|