[SeaBIOS] [PATCH 03/16] usb-uhci: fix race against host controller

[Paolo Bonzini]

While processing a frame, the host controller will write to the queue
head's element link field.  The following sequence could then happen
when two consecutive sends occur to the same pipe.

    controller                      SeaBIOS
    ---------------------------------------------------------------------
                                    td->link = UHCI_PTR_TERM;
                                    td->ctrl |= TD_CTRL_ACTIVE;
    read TD from memory
                                    wait_td(td);
    td->ctrl &= ~TD_CTRL_ACTIVE;
    write back td->ctrl
                                    exit usb_send_bulk
                                    restart usb_send_bulk
                                    pipe->qh.element = &tds;
    pipe->qh.element = td->link;    ... go on and set up the first td ...
    write back pipe->qh.element
                                    td->ctrl |= TD_CTRL_ACTIVE;

Once the host controller has written UHCI_PTR_TERM to the element link,
subsequent tds would never be processed.  This is surprisingly frequent
when the two consecutive sends are in the OUT direction (and just as
surprisingly, it seems like it never happens in the IN direction).

To fix this, re-enable TDs whenever the host controller completes the
pending ones.  Also, at the end of the processing do not wait for each
single TD to become inactive, but simply wait for the host controller
to invalidate the element link.

Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
---
 src/usb-uhci.c |   25 ++++++++++++-------------
 1 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/src/usb-uhci.c b/src/usb-uhci.c
index 5957567..84153a7 100644
--- a/src/usb-uhci.c
+++ b/src/usb-uhci.c
@@ -455,11 +455,8 @@ uhci_send_bulk(struct usb_pipe *p, int dir, void *data, int datasize)
     struct uhci_td *tds = (void*)ALIGN((u32)tdsbuf, TDALIGN);
     memset(tds, 0, sizeof(*tds) * STACKTDS);
 
-    // Enable tds
-    barrier();
-    SET_FLATPTR(pipe->qh.element, (u32)MAKE_FLATPTR(GET_SEG(SS), tds));
-
     int tdpos = 0;
+    struct uhci_td *start_td = tds;
     while (datasize) {
         struct uhci_td *td = &tds[tdpos++ % STACKTDS];
         int ret = wait_td(td);
@@ -479,21 +476,23 @@ uhci_send_bulk(struct usb_pipe *p, int dir, void *data, int datasize)
         barrier();
         td->status = (uhci_maxerr(3) | (lowspeed ? TD_CTRL_LS : 0)
                       | TD_CTRL_ACTIVE);
+        if (GET_FLATPTR(pipe->qh.element) & UHCI_PTR_TERM) {
+            // Enable tds.  The previous td might be the first one, since
+            // there is still a race window between reading and writing
+            // pipe->qh.element (we do not use CMPXCHG).  start_td ensures
+            // that what we write will always be linked to the first active td.
+            barrier();
+            SET_FLATPTR(pipe->qh.element, (u32)MAKE_FLATPTR(GET_SEG(SS)
+                                                            , start_td));
+        }
         toggle ^= TD_TOKEN_TOGGLE;
 
         data += transfer;
         datasize -= transfer;
+        start_td = td;
     }
-    int i;
-    for (i=0; i<STACKTDS; i++) {
-        struct uhci_td *td = &tds[tdpos++ % STACKTDS];
-        int ret = wait_td(td);
-        if (ret)
-            goto fail;
-    }
-
     SET_FLATPTR(pipe->toggle, !!toggle);
-    return 0;
+    return wait_pipe(pipe);
 fail:
     dprintf(1, "uhci_send_bulk failed\n");
     SET_FLATPTR(pipe->qh.element, UHCI_PTR_TERM);
-- 
1.7.7.1