Skip to content

Various xhci fixes and improvements

Michal Pecio linux-usb 1 version In review

Get series with b4
b4 am 20240827194625.61be5733@foxbook

Versions newest first

  1. v1 Latest9 patches 6 repliesIn review

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/9usb: xhci: Fix double newline in a debug messagedrivers/usb/host/xhci-ring.c+1 -10 replies
2/9usb: xhci: Fix handling errors mid TD followed by other errorsdrivers/usb/host/xhci-ring.c+40 -380 replies
3/9usb: xhci: Clean up the TD skipping loopdrivers/usb/host/xhci-ring.c+52 -500 replies
4/9usb: xhci: Expedite processing missed isoch TDs on modern HCsdrivers/usb/host/xhci-ring.c+12 -20 replies
5/9usb: xhci: Simplify error_mid_td markingdrivers/usb/host/xhci-ring.c+3 -60 replies
6/9usb: xhci: Sanity check "spurious success" handlingdrivers/usb/host/xhci-ring.c+5 -20 replies
7/9usb: xhci: Don't lie about trb_comp_code in handle_tx_event()drivers/usb/host/xhci-ring.c+4 -50 replies
8/9usb: xhci: Print completion code in the empty ring warningdrivers/usb/host/xhci-ring.c+2 -20 replies
9/9usb: xhci: Clean up and rename bad stream event handlerdrivers/usb/host/xhci-ring.c+7 -50 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [PATCH 0/9] Various xhci fixes and improvements

Hi,

Here's a handful of bug fixes, cleanups and improvements for the xHCI driver.

The first is a trivial fix, the second also fixes a bug albeit a less trivial one. These two maybe deserve to go into usb-linus, the latter perhaps to stable as well, but the patch doesn't apply as-is.

The rest are new functionality or code cleanups with low anticipated user impact.

All patches were applied and tested on v6.11-rc5, all compiled cleanly and worked without regressions with some HID, storage, audio, video. Functional changes received additional functional testing described in their respective changelogs.

Notably missing is a solution to the "Underrun after Missed Service" problem. To recap, Underrun/Overrun typically has a NULL TRB pointer and ep_ring->td_list contains some missed TDs and possibly a few that have been added after the underrun occured but before we got the IRQ. No way to tell them apart, at least by the spec, as far as I see.

On USB 3.1+ hosts we can get out of it with "expedited skipping" - it ensures that the ring is never left with stale TDs in the first place.

On USB 3.0 hosts the underrun handler *will* sometimes face skipping and it needs to make a decision.

Currently, it skips everything. This may cause DMA-after-free - not great on IN endpoints - and "TRB DMA ptr not part of current TD" or "WARN Event TRB with no TDs queued" later.

The obvious alterantive is to never skip on empty ring events, but it can deadlock a driver which waits for its URBs to be given back when all of them were missed and we can't get a valid dequeue from the HC.

I wonder if it would make sense to always skip the first queued TD when we get an MSE with NULL pointer? I think it's legal, and likely sufficient to avoid the stupid deadlock.

Just a last minute idea. I will think about it, but now I'd like to flush this patch queue before it grows to infinity ;)

Thanks,
Michal