Re: [PATCH 5/5] usb: xhci: Skip only one TD on Ring Underrun/Overrun

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 10.2.2025 9.42, Michal Pecio wrote:
> If skipping is deferred to events other than Missed Service Error itsef,
> it means we are running on an xHCI 1.0 host and don't know how many TDs
> were missed until we reach some ordinary transfer completion event.
> 
> And in case of ring xrun, we can't know where the xrun happened either.
> 
> If we skip all pending TDs, we may prematurely give back TDs added after
> the xrun had occurred, risking data loss or buffer UAF by the xHC.
> 
> If we skip none, a driver may become confused and stop working when all
> its URBs are missed and appear to be "in flight" forever.
> 
> Skip exactly one TD on each xrun event - the first one that was missed,
> as we can now be sure that the HC has finished processing it. Provided
> that one more TD is queued before any subsequent doorbell ring, it will
> become safe to skip another TD by the time we get an xrun again.
> 
> Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
> ---
>   drivers/usb/host/xhci-ring.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 878abf5b745d..049206a1db76 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -2875,6 +2875,18 @@ static int handle_tx_event(struct xhci_hcd *xhci,
>   
>   			if (!ep_seg && usb_endpoint_xfer_isoc(&td->urb->ep->desc)) {
>   				skip_isoc_td(xhci, td, ep, status);
> +
> +				if (ring_xrun_event) {
> +					/*
> +					 * If we are here, we are on xHCI 1.0 host with no idea how
> +					 * many TDs were missed and where the xrun occurred. Don't
> +					 * skip more TDs, they may have been queued after the xrun.
> +					 */
> +					xhci_dbg(xhci, "Skipped one TD for slot %u ep %u",
> +							slot_id, ep_index);
> +					break;

This would be the same as return 0; right?

Whole series looks good, I'll add it

-Mathias