From: Michal Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@intel.com>,
Niklas Neronin <niklas.neronin@linux.intel.com>
Cc: linux-usb@vger.kernel.org
Subject: [PATCH 5/5] usb: xhci: Fix Ring Underrun/Overrun handling
Date: Tue, 10 Sep 2024 13:16:51 +0200 [thread overview]
Message-ID: <20240910131651.6c4c0195@foxbook> (raw)
In-Reply-To: <20240910131233.409c6481@foxbook>
Depending on implemented spec revision, the TRB pointer of these events
may either be NULL or point at enqueue at the time of error occurrence.
By the time we handle the event, a new TD may be queued at this address.
Ensure that the new TD is not completed prematurely if the event handler
is entered due to the skip flag being set on the endpoint. Or, when the
TRB pointer is NULL, prevent the empty ring warning being printed.
Now that it is safe to enter the event handler, also enter it when the
skip flag is not set. This enables completion of any TD stuck in 'error
mid TD' state on buggy hosts. Such problem could, for example, happen
when a USBFS application knows in advance how many frames it needs and
submits the exact number of URBs, then an error occurs on the last TD.
One bug remains: when enqueue points at a Link TRB and a new TD appears
after that, skipping will remove the new TD prematurely. This should be
255 times less common that the 'matching TD' bug being fixed here, and
it will take a major improvement to the skipping loop to fix.
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
---
drivers/usb/host/xhci-ring.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index db9bc7db5aac..475b4d69551b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2726,14 +2726,10 @@ static int handle_tx_event(struct xhci_hcd *xhci,
* Underrun Event for OUT Isoch endpoint.
*/
xhci_dbg(xhci, "Underrun event on slot %u ep %u\n", slot_id, ep_index);
- if (ep->skip)
- break;
- return 0;
+ break;
case COMP_RING_OVERRUN:
xhci_dbg(xhci, "Overrun event on slot %u ep %u\n", slot_id, ep_index);
- if (ep->skip)
- break;
- return 0;
+ break;
case COMP_MISSED_SERVICE_ERROR:
/*
* When encounter missed service error, one or more isoc tds
@@ -2824,6 +2820,15 @@ static int handle_tx_event(struct xhci_hcd *xhci,
skipped, td ? "":"not ", slot_id, ep_index);
}
+ /*
+ * In these events ep_trb_dma is NULL or points at enqueue from the time
+ * of error occurrence. If it matches a new TD queued since then, don't
+ * complete the TD now. And otherwise, don't print senseless warnings.
+ */
+ if (trb_comp_code == COMP_RING_UNDERRUN ||
+ trb_comp_code == COMP_RING_OVERRUN)
+ return 0;
+
if (!ep_seg) {
/*
--
2.43.0
prev parent reply other threads:[~2024-09-10 11:16 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-10 11:12 [PATCH 0/5] Quick and effective handle_tx_event() cleanup Michal Pecio
2024-09-10 11:13 ` [PATCH 1/5] usb: xhci: Fix handling errors mid TD followed by other errors Michal Pecio
2024-09-10 11:14 ` [PATCH 2/5] usb: xhci: Clean up the TD skipping loop Michal Pecio
2024-09-10 11:15 ` [PATCH 3/5] usb: xhci: Unify event handler's 'empty list' and 'no match' cases Michal Pecio
2024-09-10 11:16 ` [PATCH 4/5] usb: xhci: Simplify the TD skipping loop further Michal Pecio
2024-09-10 11:16 ` Michal Pecio [this message]
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=20240910131651.6c4c0195@foxbook \
--to=michal.pecio@gmail.com \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
--cc=niklas.neronin@linux.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