Various xhci fixes and improvements
Get series with b4
b4 am 20240827194625.61be5733@foxbookVersions newest first
- v1 Latest9 patches 6 repliesIn review
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/9 | usb: xhci: Fix double newline in a debug messagedrivers/ | +1 | 0 replies | |
| 2/9 | usb: xhci: Fix handling errors mid TD followed by other errorsdrivers/ | +40 | 0 replies | |
| 3/9 | usb: xhci: Clean up the TD skipping loopdrivers/ | +52 | 0 replies | |
| 4/9 | usb: xhci: Expedite processing missed isoch TDs on modern HCsdrivers/ | +12 | 0 replies | |
| 5/9 | usb: xhci: Simplify error_mid_td markingdrivers/ | +3 | 0 replies | |
| 6/9 | usb: xhci: Sanity check "spurious success" handlingdrivers/ | +5 | 0 replies | |
| 7/9 | usb: xhci: Don't lie about trb_comp_code in handle_tx_event()drivers/ | +4 | 0 replies | |
| 8/9 | usb: xhci: Print completion code in the empty ring warningdrivers/ | +2 | 0 replies | |
| 9/9 | usb: xhci: Clean up and rename bad stream event handlerdrivers/ | +7 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
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