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