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

Thinh Nguyen
2026-04-03T02:00:18Z

On Wed, Apr 01, 2026, stern@rowland.harvard.edu wrote:
> On Thu, Apr 02, 2026 at 01:08:31AM +0300, Mathias Nyman wrote:
> > On 3/31/26 18:31, stern@rowland.harvard.edu wrote:
> > >
> > > How about this instead? We add a "halted" flag to the usb_host_endpoint
> > > structure, and the core will set this flag whenever a bulk or interrupt
> > > URB gets a status other than 0 (before putting the URB on the bh list).
> > > If an URB has one of these statuses, when its completion handler returns
> > > the core will unlink all the URBs queued to the same endpoint. Finally,
> > > the "halted" flag should be cleared after a completion handler returns
> > > if there are no more unlinked URBs still in the queue or URBs waiting on
> > > the bh list to be given back.
> > >
> > > The result of this is that any URB remaining in the queue when the flag
> > > is cleared must have been submitted by the class driver _after_ the
> > > failing URB's completion handler has run. We can assume the class
> > > driver knows what it's doing in this case.
> > >
> > > The endpoint queue shouldn't be restarted until the "halted" flag is
> > > cleared. Either right away, if there are any URBs in the queue, or not
> > > until the next URB is submitted. Doing this might require a new HCD
> > > callback. (It would also mean the kerneldoc for usb_unlink_urb() would
> > > need to be updated, because the endpoint might restart before all the
> > > completion handlers for the unlinked URBs have run.)
> > >
> > > What I'm trying to do here is come up with a single, consistent proposal
> > > for exactly when halted endpoint queues should restart. Maybe someone
> > > else has a better suggestion.
> >
> > Sounds like a possible solution to me.
> >
> > Just to clarify, core should unlink the remaining URBs queued to that endpoint
> > after setting the "halted" flag, but before URB completion is called.
>
> Above I wrote that the remaining URBs should be unlinked _after_ the
> completion handler is called. If we did the unlinks before then the
> class driver might submit a new URB after the unlinks were finished and
> before the completion handler learned about the transaction error, and
> this new URB then wouldn't get unlinked.
>
> It's a race between completion of one URB and submission of another.
>
> > "Halted" flag should be cleared after URB completion returns, and endpoint
> > should be restarted if there are any pending URBs.
>
> To be clear, the flag should be cleared after the completion handlers
> for _all_ the unlinked URBs (as well as the URB getting the original
> error) have returned.
>
> > This allows the class driver URB completion handler to re-queue the halted URB
> > without core unlinking it.
>
> The completion handler shouldn't do this, because it would mean
> resubmitting without doing a clear-halt first. (Completion handlers
> can't do clear-halts because they run in atomic context.) If it does
> try to do this anyway, I see nothing wrong with the core unlinking the
> resubmitted URB.
>
> (Are you going to ask about verification tests that set the endpoint's
> Halt feature, submit an URB, wait for it to fail with -EPIPE, and then
> submit another URB from within the completion handler? :-) )
>

How about this:

Introduce a halted flag the following conditions:

* Introduce the halted flag in usb_host_endpoint
* The halted flag must be implemented as a bit in a unsigned long so
we can use atomic bit operation
* Only the HCD may set the halted flag, and only upon checking the
first URB completing with a halted status
* Only the USB core may clear the halted flag, and only after
usb_reset_endpoint returns (this makes sure the HCD drained and reset
the endpoint before the flag is cleared and new URBs are accepted)
* The usb_reset_endpoint must be called after clear-halt, SetInterface,
and SetConfiguration.
* The USB core will not attempt to unlink pending URBs due to halted
condition
* The HCD is responsible for completing or canceling queued URBs
when the halted flag is set. Cancelled and newly submitted URBs will
be returned with -EPIPE as long as the halted flag is set
* The class driver is responsible to check the halted flag to
determine whether to initiate error recovery via usb_clear_halt

I'm trying to keep a clear separation of responsibilities between HCD
and the USB core. Also, I try to keep the halted flag more closely match
the state of the endpoint.

Let me know what you think?

BR,
Thinh