usb-uhci: fix race against host controller
authorPaolo Bonzini <pbonzini@redhat.com>
Thu, 17 Nov 2011 09:23:00 +0000 (10:23 +0100)
committerKevin O'Connor <kevin@koconnor.net>
Fri, 18 Nov 2011 02:01:32 +0000 (21:01 -0500)
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, at the end of the processing do not wait for each single
TD to become inactive, but for the host controller to invalidate the
element link (which implies it's done with all TDs).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
src/usb-uhci.c

index 47026952b6b57cc2fbd7d3d0d624533ada12078e..242f3ba5aa8c24060e5b16f8c74dc9ab9f418704 100644 (file)
@@ -483,16 +483,8 @@ uhci_send_bulk(struct usb_pipe *p, int dir, void *data, int datasize)
         data += transfer;
         datasize -= transfer;
     }
-    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, 5000);
 fail:
     dprintf(1, "uhci_send_bulk failed\n");
     SET_FLATPTR(pipe->qh.element, UHCI_PTR_TERM);