From d6fe9953a8277a54ae7f4cefa192b49d9bf99e3d 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 */

-- 

1.7.12.1