Hi,
I believe I may have found a bug in the xHCI driver, specifically in
xhci-ring's inc_enq(). If you insert a non-chained TRB immediately
prior to a link TRB, some logic kicks in to avoid having chained link
TRBs (which are not allowed in xHCI 0.95). However, I think this logic
may be faulty- it actually causes inc_enq() to ignore the link TRB
entirely.
The ring's enqueue pointer remains pointing to the link TRB, and the
next enqueued TRB on this ring overwrites it. Then, subsequent TRBs
write past the bounds of the ring. At best, this will cause "suspect
DMA" errors when handling completions. At worst, it's a kernel panic.
I'm not sure what the proper solution is to this, since I'm not
familiar with the way link TRBs and chain bits were defined in version
0.95 of the spec. But I've been using a very simple patch for my own
testing, and it seems to successfully fix this problem.
Thank you in advance for any comments on this problem or the patch :)
Signed-off-by: Micah Dowty <micah@xxxxxxxxxx>
---
drivers/usb/host/xhci-ring.c | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b03b0f1..88c3d5b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -194,14 +194,12 @@ static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring *ring, bool consumer
if (!consumer) {
if (ring != xhci->event_ring) {
if (chain) {
- next->link.control |= TRB_CHAIN;
-
- /* Give this link TRB to the hardware */
- wmb();
- next->link.control ^= TRB_CYCLE;
- } else {
- break;
+ next->link.control |= TRB_CHAIN;
}
+
+ /* Give this link TRB to the hardware */
+ wmb();
+ next->link.control ^= TRB_CYCLE;
}
/* Toggle the cycle bit after the last ring segment. */
if (last_trb_on_last_seg(xhci, ring, ring->enq_seg, next)) {
--
1.6.3.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
