From: Michal Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@intel.com>
Cc: linux-usb@vger.kernel.org
Subject: [PATCH 6/9] usb: xhci: Sanity check "spurious success" handling
Date: Tue, 27 Aug 2024 19:52:24 +0200 [thread overview]
Message-ID: <20240827195224.02c32551@foxbook> (raw)
In-Reply-To: <20240827194625.61be5733@foxbook>
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 --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;
}
--
2.43.0
next prev parent reply other threads:[~2024-08-27 17:52 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-27 17:46 [PATCH 0/9] Various xhci fixes and improvements Michal Pecio
2024-08-27 17:47 ` [PATCH 1/9] usb: xhci: Fix double newline in a debug message Michal Pecio
2024-08-27 17:48 ` [PATCH 2/9] usb: xhci: Fix handling errors mid TD followed by other errors Michal Pecio
2024-08-27 17:49 ` [PATCH 3/9] usb: xhci: Clean up the TD skipping loop Michal Pecio
2024-08-27 17:50 ` [PATCH 4/9] usb: xhci: Expedite processing missed isoch TDs on modern HCs Michal Pecio
2024-08-27 17:51 ` [PATCH 5/9] usb: xhci: Simplify error_mid_td marking Michal Pecio
2024-08-27 17:52 ` Michal Pecio [this message]
2024-08-27 17:53 ` [PATCH 7/9] usb: xhci: Don't lie about trb_comp_code in handle_tx_event() Michal Pecio
2024-08-27 17:54 ` [PATCH 8/9] usb: xhci: Print completion code in the empty ring warning Michal Pecio
2024-08-27 17:55 ` [PATCH 9/9] usb: xhci: Clean up and rename bad stream event handler Michal Pecio
2024-08-28 10:21 ` [PATCH 0/9] Various xhci fixes and improvements FPS
2024-08-28 13:24 ` Michał Pecio
2024-08-28 13:48 ` fps
2024-08-28 11:14 ` Mathias Nyman
2024-08-28 15:02 ` Michał Pecio
2024-08-30 11:53 ` Michał Pecio
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240827195224.02c32551@foxbook \
--to=michal.pecio@gmail.com \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).