Re: [PATCH] xhci: keep ep0 blocked after disconnect until Address Device reloads it

[Mathias Nyman]

On 10/27/26 10:48, Yiyu Zhou wrote:
> After a root port disconnect or link error, xhci_handle_halted_endpoint()
> sets EP_DROP_PENDING instead of resetting the endpoint, and gives back
> its TDs.  Their TRBs stay on the ring, and the endpoint's dequeue
> pointer still points at the halted TD.
>
> If the device comes back before hub_wq handles the disconnect, a driver
> may reset it on the same slot.  usbhid does this after 1 s of
> interrupt-in errors.  xhci_discover_or_reset_device() then clears
> EP_DROP_PENDING on every endpoint.  The other endpoints are disabled by
> Reset Device and get new rings, but ep0 keeps its ring and dequeue
> pointer.  For USB 2 devices, usbcore by default sends a 64 byte
> GET_DESCRIPTOR before SET_ADDRESS, and so before Address Device reloads
> the ep0 context.  That request is queued behind the stale TRBs.  With a
> Keychron M5 the reset stalled for 11 s, and the xHC then ran the old
> TDs ("Event dma ... for ep 0 ... not part of TD"), writing into buffers
> of URBs that had already been given back.
>
> Keep EP_DROP_PENDING set on ep0 through Reset Device, and clear it when
> an Address Device command succeeds.  That command loads the ep0 context
> from the input context, whose dequeue pointer is our enqueue pointer, so
> the xHC skips the stale TRBs.  Until then, ep0 requests fail with
> -ENODEV.  If Address Device comes first, as for SuperSpeed devices, the
> device recovers in place.  Otherwise the reset fails quickly, and
> usbcore re-enumerates the device on a new slot, as after a real
> disconnect.  The same applies when Reset Device fails because the slot
> was never addressed: the next enumeration attempt uses a new slot
> instead of the stale ring.

Thanks for the detailed description, this looks like a very targeted fix
for this specific hid case.

It still leaves the stale TRBs on ep0 ring, and relies on a successful
address device command to clear the EP_DROP_PENDING flag, blocking ep0
in all other scenarios. It also leaves xHC internal ep0 in the
'halted' state. Not sure if xHC internal halted ep0 state is cleared during
'reset device' or 'address device' command.

There are cases where EP_DROP_PENDING won't be cleared at all in this patch.
For example if slot is already in the default state when hci_setup_device()
is called. (slot is in default state after reset device command)

I'm thinking we should take a step backwards first when fixing this regression,
and then look if we can tune fine tune ep0 after that.

1. Make sure All TDs are turned to no-op before returning the URBs in the disconnect
   case.

   Add all TDs in ring to cancelled list, turn them no-op, and then give them back.

2. Treat ep0 differently, cancel all TDs on ring as above, but still reset it when halted.
   Allow a Set TR deq pointer command as well, and only give back ep0 URBs after this.
   This partially reverts the two patches that caused this issue, and clears the ep0
   halted state

3. Possibly keep EP_DROP_PENDING flag for ep0 until xhci_setup_device() as this patch
   does, but always clear it in xhci_setup_device() before it returns

Let me know if you find any flaws with this approach.

Thanks
Mathias