[SeaBIOS] [PATCH 03/16] usb-uhci: fix race against host controller
Paolo Bonzini
pbonzini at redhat.com
Wed Nov 16 13:02:44 CET 2011
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
More information about the SeaBIOS
mailing list