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@xxxxxxxxx>
---
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
