diff mbox series

[6/9] usb: xhci: Sanity check "spurious success" handling

Message ID 20240827195224.02c32551@foxbook (mailing list archive)
State New
Headers show
Series Various xhci fixes and improvements | expand

Commit Message

MichaƂ Pecio Aug. 27, 2024, 5:52 p.m. UTC
It's not spurious, it's expected, it's required by the spec since its
final 1.0 revision 14 years ago, and it's handled incorrectly here.
But until somebody puts some effort to get it all right, try at least
not to do obviously wrong things here.

This code claims to handle "spurious" Success events, but in reality
it is ready and willing to silently swallow any kind of event, on most
host controllers these days, after any short transfer.

It got in my way while debugging genuinely incorrect events from the
xHC, which this code thought were meant for it, because it has no way
of knowing better, because it's utterly broken.

Limit it at least to only accept Success and Short Packet completions
so that rightful warnings will be printed in other cases.

So far I found no instances of this change exposing previously hidden
errors, besides the aforementioned case of a buggy xHC. The buggy xHC
completely fails to acknowledge some TDs in any way. It proceeeds to
the next TD, whose event then doesn't match the current TD, so if the
prior TD got a short packet, the "spurious" code swallows the event.

Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---
 drivers/usb/host/xhci-ring.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c777cb897579..e19c8a17b59c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2785,9 +2785,10 @@  static int handle_tx_event(struct xhci_hcd *xhci,
 			 */
 
 			if (!(trb_comp_code == COMP_STOPPED ||
 			      trb_comp_code == COMP_STOPPED_LENGTH_INVALID ||
-			      ep_ring->last_td_was_short)) {
+			      (trb_comp_code == COMP_SUCCESS && ep_ring->last_td_was_short) ||
+			      (trb_comp_code == COMP_SHORT_PACKET && ep_ring->last_td_was_short))) {
 				xhci_warn(xhci, "WARN Event TRB for slot %u ep %d with no TDs queued?\n",
 					  slot_id, ep_index);
 			}
 			if (ep->skip) {
@@ -2878,9 +2879,11 @@  static int handle_tx_event(struct xhci_hcd *xhci,
 		 * transfer. Ignore it.
 		 * FIXME xHCI 4.10.1.1: this should be freed now, not mid-TD
 		 */
 		if ((xhci->quirks & XHCI_SPURIOUS_SUCCESS) &&
-		    ep_ring->last_td_was_short) {
+		    ep_ring->last_td_was_short &&
+		    (trb_comp_code == COMP_SUCCESS ||
+		    trb_comp_code == COMP_SHORT_PACKET)) {
 			ep_ring->last_td_was_short = false;
 			return 0;
 		}