Blame 0369-ehci-Fix-interrupt-packet-MULT-handling.patch

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