Re: [RFC PATCH 1/2] xhci: prevent automatic endpoint restart after stall or error

Alan Stern
2026-04-18T02:35:03Z

On Fri, Apr 17, 2026 at 11:48:46PM +0200, Michal Pecio wrote:
> On Fri, 17 Apr 2026 13:38:46 -0400, Alan Stern wrote:
> > The behavior for control endpoints isn't clear. When a transaction
> > error occurs in a control transfer, does xHCI stop the endpoint
> > queue? Presumably usb_clear_halt() isn't needed, but what about
> > usb_reset_endpoint()?
> >
> > I will assume that all stalls for control endpoints are protocol
> > stalls and therefore do not need to be cleared (the device will
> > automatically clear the halt condition when it receives the next
> > SETUP packet for the endpoint). But does xhci-hcd still require a
> > usb_reset_endpoint() after a control stall?
>
> Nope.
>
> The host endpoint halts on every STALL handshake or non-isoc protocol
> error, but the driver immediately resets every halted endpoint because
> it's a precondition to even remove the failed URB from the HW queue
> before giving it back, which looks like a prudent thing to do.

Okay, good, we'll require all HCDs to reset control endpoints
automatically after every error and stall.

> Currently, by the time the URB is given back, its endpoint is already
> in a "stopped but runnable" state and its sequence state is zeroed.
> And it may have already been restarted if there are more pending URBs.

Ah, I was going to ask about that. This will be different from the way
bulk and interrupt endpoints will behave, but I think it is acceptable.
Control endpoints aren't used for anything that requires high
throughput; if a driver wants an error to prevent later transfers from
starting right away then it can simply avoid submitting those later
transfers until the earlier ones have completed.

> This seems to need no change for control endpoints?

No change needed.

> For bulk/interrupt it also means that if we delay the restart until
> endpoint_reset() then endpoint_reset() has nothing to do besides
> restarting the endpoint, which is a good thing because this method
> under normal conditions requires no URBs to be present.

That could be an issue, because there may well be some URBs that were on
the queue when the error occurred and were not unlinked, or were added
afterward but before usb_endpoint_reset() could be called.

> > Recovery from a transaction error on a bulk or interrupt endpoint
> > involves sending a Clear-Halt request to the device. But if the
> > error was caused by some sort of transient interference that hasn't
> > ended yet, the Clear-Halt might itself fail with the same error. To
> > handle this, we should retry the Clear-Halt at increasing time
> > intervals. At what point should the core give up?
>
> Good question, I don't know. One thing I noticed is that Windows does
> tend to lose patience with completely unresponsive devices and kicks
> them out, but I don't know the exact criteria.

Two reasonable possibilities are 250 ms (because that's about how long
an intermediate hub might take to notify the core about a disconnect) or
5 seconds (the normal timeout for control transfers). Of course, 5
seconds is an awfully long time to wait for a mouse or keyboard to
recover, so maybe something in between would be best.

> And if core is to clear halt autonomously, things may get interesting
> if a driver tries to do the same, or something else like removing the
> endpoint with usb_set_interface().

I will set things up so that an extraneous clear-halt (such as one
submitted by the driver) will prevent the core from doing its own. This
leaves the possibility of the core clearing the halt and restarting the
endpoint and then the driver doing it again, while the endpoint is
running and the queue is nonempty. Hopefully drivers avoid doing this.
But if it helps, I could print a warning if usb_clear_halt() is called
for an endpoint that isn't stopped and has a nonempty queue.

Also I will be careful to make the core's actions mutually exclusive
with: suspend/resume, reset, disconnect, set-config, and set-interface.
Should anything else be added to this list?

> A related issue is clearing TT buffers. AFAIK this has no retries, it
> fails silently and leaves the endpoint potentially broken, and it is
> waited for to complete in case of usb_set_interface().

Is there anything we can do besides calling usb_clear_halt() and
usb_reset_endpoint()? If not, and data loss is unavoidable, then so be
it.

Alan Stern