Blame 0130-usb-ehci-Drop-cached-qhs-when-the-doorbell-gets-rung.patch

Hans de Goede 1b1995
From 35562fb521547e081e732453a6395fc00d9ee9e4 Mon Sep 17 00:00:00 2001
Hans de Goede 1b1995
From: Hans de Goede <hdegoede@redhat.com>
Hans de Goede 1b1995
Date: Thu, 1 Mar 2012 15:20:17 +0100
Hans de Goede 1b1995
Subject: [PATCH 130/140] usb-ehci: Drop cached qhs when the doorbell gets
Hans de Goede 1b1995
 rung
Hans de Goede 1b1995
Hans de Goede 1b1995
The purpose of the IAAD bit / the doorbell is to make the ehci controller
Hans de Goede 1b1995
forget about cached qhs, this is mainly used when cancelling transactions,
Hans de Goede 1b1995
the qh is unlinked from the async schedule and then the doorbell gets rung,
Hans de Goede 1b1995
once the doorbell is acked by the controller the hcd knows that the qh is
Hans de Goede 1b1995
no longer in use and that it can do something else with the memory, such
Hans de Goede 1b1995
as re-use it for a new qh! But we keep our struct representing this qh around
Hans de Goede 1b1995
for circa 250 ms. This allows for a (mightily large) race window where the
Hans de Goede 1b1995
following could happen:
Hans de Goede 1b1995
-hcd submits a qh at address 0xdeadbeef
Hans de Goede 1b1995
-our ehci code sees the qh, sends a request to a usb-device, gets a result
Hans de Goede 1b1995
 of USB_RET_ASYNC, sets the async_state of the qh to EHCI_ASYNC_INFLIGHT
Hans de Goede 1b1995
-hcd unlinks the qh at address 0xdeadbeef
Hans de Goede 1b1995
-hcd rings the doorbell, wait for us to ack it
Hans de Goede 1b1995
-hcd re-uses the qh at address 0xdeadbeef
Hans de Goede 1b1995
-our ehci code sees the qh, looks in the async_queue, sees there already is
Hans de Goede 1b1995
 a qh at address 0xdeadbeef there with async_state of EHCI_ASYNC_INFLIGHT,
Hans de Goede 1b1995
 does nothing
Hans de Goede 1b1995
-the *original* (which the hcd thinks it has cancelled) transaction finishes
Hans de Goede 1b1995
-our ehci code sees the qh on yet another pass through the async list,
Hans de Goede 1b1995
 looks in the async_queue, sees there already is a qh at address 0xdeadbeef
Hans de Goede 1b1995
 there with async_state of EHCI_ASYNC_COMPLETED, and finished the transaction
Hans de Goede 1b1995
 with the results of the *original* transaction.
Hans de Goede 1b1995
Hans de Goede 1b1995
Not good (tm), this patch fixes this race by removing all qhs which have not
Hans de Goede 1b1995
been seen during the last cycle through the async list immidiately when the
Hans de Goede 1b1995
doorbell is rung.
Hans de Goede 1b1995
Hans de Goede 1b1995
Note this patch does not fix any actually observed problem, but upon
Hans de Goede 1b1995
reading of the EHCI spec it became apparent to me that the above race could
Hans de Goede 1b1995
happen and the usb-ehci behavior from before this patch is not good.
Hans de Goede 1b1995
Hans de Goede 1b1995
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Hans de Goede 1b1995
---
Hans de Goede 1b1995
 hw/usb-ehci.c |   31 ++++++++++++++++---------------
Hans de Goede 1b1995
 1 file changed, 16 insertions(+), 15 deletions(-)
Hans de Goede 1b1995
Hans de Goede 1b1995
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
Hans de Goede 1b1995
index 422afc8..b8ba483 100644
Hans de Goede 1b1995
--- a/hw/usb-ehci.c
Hans de Goede 1b1995
+++ b/hw/usb-ehci.c
Hans de Goede 1b1995
@@ -697,7 +697,7 @@ static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr,
Hans de Goede 1b1995
     return NULL;
Hans de Goede 1b1995
 }
Hans de Goede 1b1995
 
Hans de Goede 1b1995
-static void ehci_queues_rip_unused(EHCIState *ehci, int async)
Hans de Goede 1b1995
+static void ehci_queues_rip_unused(EHCIState *ehci, int async, int flush)
Hans de Goede 1b1995
 {
Hans de Goede 1b1995
     EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
Hans de Goede 1b1995
     EHCIQueue *q, *tmp;
Hans de Goede 1b1995
@@ -708,7 +708,7 @@ static void ehci_queues_rip_unused(EHCIState *ehci, int async)
Hans de Goede 1b1995
             q->ts = ehci->last_run_ns;
Hans de Goede 1b1995
             continue;
Hans de Goede 1b1995
         }
Hans de Goede 1b1995
-        if (ehci->last_run_ns < q->ts + 250000000) {
Hans de Goede 1b1995
+        if (!flush && ehci->last_run_ns < q->ts + 250000000) {
Hans de Goede 1b1995
             /* allow 0.25 sec idle */
Hans de Goede 1b1995
             continue;
Hans de Goede 1b1995
         }
Hans de Goede 1b1995
@@ -1565,7 +1565,7 @@ static int ehci_state_waitlisthead(EHCIState *ehci,  int async)
Hans de Goede 1b1995
         ehci_set_usbsts(ehci, USBSTS_REC);
Hans de Goede 1b1995
     }
Hans de Goede 1b1995
 
Hans de Goede 1b1995
-    ehci_queues_rip_unused(ehci, async);
Hans de Goede 1b1995
+    ehci_queues_rip_unused(ehci, async, 0);
Hans de Goede 1b1995
 
Hans de Goede 1b1995
     /*  Find the head of the list (4.9.1.1) */
Hans de Goede 1b1995
     for(i = 0; i < MAX_QH; i++) {
Hans de Goede 1b1995
@@ -2121,18 +2121,7 @@ static void ehci_advance_async_state(EHCIState *ehci)
Hans de Goede 1b1995
             break;
Hans de Goede 1b1995
         }
Hans de Goede 1b1995
 
Hans de Goede 1b1995
-        /* If the doorbell is set, the guest wants to make a change to the
Hans de Goede 1b1995
-         * schedule. The host controller needs to release cached data.
Hans de Goede 1b1995
-         * (section 4.8.2)
Hans de Goede 1b1995
-         */
Hans de Goede 1b1995
-        if (ehci->usbcmd & USBCMD_IAAD) {
Hans de Goede 1b1995
-            DPRINTF("ASYNC: doorbell request acknowledged\n");
Hans de Goede 1b1995
-            ehci->usbcmd &= ~USBCMD_IAAD;
Hans de Goede 1b1995
-            ehci_set_interrupt(ehci, USBSTS_IAA);
Hans de Goede 1b1995
-            break;
Hans de Goede 1b1995
-        }
Hans de Goede 1b1995
-
Hans de Goede 1b1995
-        /* make sure guest has acknowledged */
Hans de Goede 1b1995
+        /* make sure guest has acknowledged the doorbell interrupt */
Hans de Goede 1b1995
         /* TO-DO: is this really needed? */
Hans de Goede 1b1995
         if (ehci->usbsts & USBSTS_IAA) {
Hans de Goede 1b1995
             DPRINTF("IAA status bit still set.\n");
Hans de Goede 1b1995
@@ -2146,6 +2135,18 @@ static void ehci_advance_async_state(EHCIState *ehci)
Hans de Goede 1b1995
 
Hans de Goede 1b1995
         ehci_set_state(ehci, async, EST_WAITLISTHEAD);
Hans de Goede 1b1995
         ehci_advance_state(ehci, async);
Hans de Goede 1b1995
+
Hans de Goede 1b1995
+        /* If the doorbell is set, the guest wants to make a change to the
Hans de Goede 1b1995
+         * schedule. The host controller needs to release cached data.
Hans de Goede 1b1995
+         * (section 4.8.2)
Hans de Goede 1b1995
+         */
Hans de Goede 1b1995
+        if (ehci->usbcmd & USBCMD_IAAD) {
Hans de Goede 1b1995
+            /* Remove all unseen qhs from the async qhs queue */
Hans de Goede 1b1995
+            ehci_queues_rip_unused(ehci, async, 1);
Hans de Goede 1b1995
+            DPRINTF("ASYNC: doorbell request acknowledged\n");
Hans de Goede 1b1995
+            ehci->usbcmd &= ~USBCMD_IAAD;
Hans de Goede 1b1995
+            ehci_set_interrupt(ehci, USBSTS_IAA);
Hans de Goede 1b1995
+        }
Hans de Goede 1b1995
         break;
Hans de Goede 1b1995
 
Hans de Goede 1b1995
     default:
Hans de Goede 1b1995
-- 
Hans de Goede 1b1995
1.7.9.3
Hans de Goede 1b1995