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

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

[Yiyu Zhou]

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.

Fixes: 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub disconnect")
Fixes: 7e0ef4332ed9 ("xhci: avoid xHC endpoint changes after disconnect or link error.")
Reported-by: Dennis Wissel <dennis@dsine.de>
Closes: https://lore.kernel.org/linux-usb/4MjB9K-2SoiA85WbI9iEig@dsine.de/
Tested-by: Dennis Wissel <dennis@dsine.de>
Signed-off-by: Yiyu Zhou <yiyu@yiyuzhou.io>
---
This is the RFC from the regression thread [1], unchanged apart from
Dennis's Tested-by.  It applies on top of 1f36cc9fa949 ("xhci: give
back all TDs when skipping endpoint reset after disconnect") in
for-usb-linus and depends on it: without that patch, ep0 requests are
not rejected while EP_DROP_PENDING is set, so they would wait for their
timeouts.  7.3 needs both, so they should probably go to stable
together.

I used an LLM to trace the code and draft the patch.

[1] https://lore.kernel.org/linux-usb/20261026203944.2235991-1-yiyu@yiyuzhou.io/

 drivers/usb/host/xhci.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index fa3012bf4c0f..15f4ee4aa2c3 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4091,7 +4091,13 @@ static int xhci_discover_or_reset_device(struct usb_hcd *hcd,
 	ret = 0;
 
 command_cleanup:
-	for (i = 0; i < EP_CTX_PER_DEV; i++)
+	/*
+	 * Reset Device leaves the ep0 ring and dequeue pointer as they were.
+	 * After a disconnect or link error, ep0 may still point at TRBs of TDs
+	 * that were given back, so keep it blocked until Address Device
+	 * reloads its context.
+	 */
+	for (i = 1; i < EP_CTX_PER_DEV; i++)
 		virt_dev->eps[i].ep_state &= ~EP_DROP_PENDING;
 
 	xhci_free_command(xhci, reset_device_cmd);
@@ -4463,6 +4469,16 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 	}
 	if (ret)
 		goto out;
+
+	/*
+	 * Address Device loaded the ep0 context from the input context, so
+	 * the xHC now starts at our enqueue pointer, past any TRBs of TDs
+	 * given back after a disconnect or link error.  Unblock ep0.
+	 */
+	spin_lock_irqsave(&xhci->lock, flags);
+	virt_dev->eps[0].ep_state &= ~EP_DROP_PENDING;
+	spin_unlock_irqrestore(&xhci->lock, flags);
+
 	temp_64 = xhci_read_64(xhci, &xhci->op_regs->dcbaa_ptr);
 	xhci_dbg_trace(xhci, trace_xhci_dbg_address,
 			"Op regs DCBAA ptr = %#016llx", temp_64);

base-commit: 1f36cc9fa9496189e95fe5762cde8a74e0cd9d68
-- 
2.55.0

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

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

[Michal Pecio]

On Tue, 27 Oct 2026 12:36:26 +0200, Mathias Nyman wrote:
> 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.

Addres Device is not suitable, unless preceded by Drop/Enable slot.

Reset Device:

  For the Default Control Endpoint the xHC shall terminate any USB
  activity, abort any pending events not already posted to an Event
  Ring, and transition the endpoint to the Running state.

I see no mention of "halted" in all of 4.6.11. Logically, the spec
seems to assume that SW can use this command when EP0 is running and
busy, so it can transition to halted while the command is pending,
so the xHC "should" be able to deal with that.

We know how dependable xHCI hardware is in practice, but actually this
might work because IIRC Windows also doesn't bother resetting endpoints
of disconnected devices. I haven't studied this very thoroughly, but
I recall noticing that it queries PORTSC before resetting endpoints
and it violates the spec by dropping halted endpoints, just like we do.

> 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.

Seems helpful and harmless, doesn't account for potential TRB caching.

> 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

Sensible, if Reset EP/Set TR Deq on EP0 of a disconnected USB3 device
don't trigger HW bugs. These commands only change some EP Ctx fields
and don't submit or abort USB transfers, so they may be safe. It's even
possible that they are safe on all endpoints, but who knows.

I don't think this will cause any issuess on Renesas, even generalized
to all endpoints, and I still have the HW and ability to test.

We could use some input from the author of b8c3b718087b ("usb: xhci:
Don't try to recover an endpoint if port is in error state.") about
those issues, whether they affected EP0 or not :)

> 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

Does this result in URBs failing to submit before the device is
addressed, causing USB core to drop and enumerate again? That part
does seem a little dodgy. We may agree or disagree with core's
policies, but they are what they are, and not an accidental bug.
Doesn't seem suitable for 7.3.

Regards,
Michal

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

[Yiyu Zhou]

On Tue, 27 Oct 2026 12:36:26 +0200, Mathias Nyman wrote:
> There are cases where EP_DROP_PENDING won't be cleared at all in this patch.

You are right.  xhci_setup_device() returns early with "Slot already in
default state" for the SETUP_CONTEXT_ONLY call from hub_enable_device(),
before my new clear, and Reset Device leaves the slot in that state.  I
missed that early return.  ep0 then stays blocked, and usbcore only gets
out of it by re-enumerating the device.

On Tue, 27 Oct 2026 11:58:05 +0100, Michal Pecio wrote:
> You seem to suggest that xhci-hcd should trick usbcore into abandoning
> "resuscitation", right?

Yes, that was the effect, and I agree it is the wrong place for it.  It
would also re-enumerate devices that recover from an in-place reset on
7.2, which is a bigger change than I weighed.

Please drop this patch.  I can't judge the controller side of the plan
in your mail, but I am happy to test it.  My HHKB covers the original
disconnect hang, and Dennis's Keychron covers the reset case.  If it
helps, I can also turn steps 1 and 2 into an RFC to test with, unless
you have already started on them.

-YZ