[PATCH] usb: xhci: return an error if the host is not halted

[PATCH] usb: xhci: return an error if the host is not halted

[Henry Tseng]

xhci_reset() returns 0 when the host is not halted, without ever writing
CMD_RESET. Every other path that fails to reset the host returns an
error, so callers that check the return value are told the reset
succeeded on the one path where it did not happen.

Return -EBUSY when the reset is aborted because the host is not halted.

Signed-off-by: Henry Tseng <henrytseng@qnap.com>
---
 drivers/usb/host/xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a9e47e178c28..2af6a7b91e55 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -195,7 +195,7 @@ int xhci_reset(struct xhci_hcd *xhci, u64 timeout_us)
 
 	if ((state & STS_HALT) == 0) {
 		xhci_warn(xhci, "Host controller not halted, aborting reset.\n");
-		return 0;
+		return -EBUSY;
 	}
 
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "// Reset the HC");
-- 
2.43.0

Re: [PATCH] usb: xhci: return an error if the host is not halted

[Michal Pecio]

On Wed,  2 Sep 2026 18:15:10 +0800, Henry Tseng wrote:
> xhci_reset() returns 0 when the host is not halted, without ever writing
> CMD_RESET. Every other path that fails to reset the host returns an
> error, so callers that check the return value are told the reset
> succeeded on the one path where it did not happen.
> 
> Return -EBUSY when the reset is aborted because the host is not halted.
> 
> Signed-off-by: Henry Tseng <henrytseng@qnap.com>
> ---
>  drivers/usb/host/xhci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index a9e47e178c28..2af6a7b91e55 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -195,7 +195,7 @@ int xhci_reset(struct xhci_hcd *xhci, u64 timeout_us)
>  
>  	if ((state & STS_HALT) == 0) {
>  		xhci_warn(xhci, "Host controller not halted, aborting reset.\n");
> -		return 0;
> +		return -EBUSY;

That's probably how it should be, but I wonder how did you find this
bug and are you aware of any cases where it makes a difference?

There is apparently one caller (xhci_resume()) which cares about
xhci_reset() return value but not about the preceding xhci_halt().
 
>  	}
>  
>  	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "// Reset the HC");
> -- 
> 2.43.0
> 

Re: [PATCH] usb: xhci: return an error if the host is not halted

[Henry Tseng]

On Wed, 2 Sep 2026 13:50:16 +0200, Michal Pecio <michal.pecio@gmail.com> wrote:
> That's probably how it should be, but I wonder how did you find this
> bug and are you aware of any cases where it makes a difference?

I noticed it while debugging a downstream kernel (based on v6.6) for
an embedded NAS platform, which carries an xHC error recovery routine
doing the same sequence as the reset_registers path in xhci_resume().

Since the NAS platform doesn't run a mainline kernel easily, I did the
testing on a separate machine. On this machine (Intel Meteor Lake-P
xHC, 8086:7e7d) STS_HALT is set by the time xhci_resume() runs, so I
added a fault injection flag to xhci_reset() on mainline to force the
abort path, making it return 0 without doing the reset. Resume then
restarts the HCD on a host that was never reset. Every command
afterwards times out and gets aborted, and devices loop on failed
re-enumeration for minutes:

  [  100.109411] xhci_hcd 0000:00:14.0: Host controller not halted, aborting reset.
  [  100.110025] xhci_hcd 0000:00:14.0: Start the primary HCD
  [  100.110132] xhci_hcd 0000:00:14.0: Start the secondary HCD
  [  100.194666] xhci_hcd 0000:00:14.0: The device to be reset with slot ID 0 does not exist. Re-allocate the device
  [  107.573055] xhci_hcd 0000:00:14.0: Error while assigning device slot ID: Command Aborted
  [  107.573059] xhci_hcd 0000:00:14.0: Max number of devices this xHCI host supports is 64.
  ...
  [  259.125506] xhci_hcd 0000:00:14.0: Error while assigning device slot ID: Command Aborted
  [  259.125509] xhci_hcd 0000:00:14.0: Max number of devices this xHCI host supports is 64.
  [  259.548200] xhci_hcd 0000:00:14.0: xHCI xhci_drop_endpoint called with unaddressed device
  [  259.549751] xhci_hcd 0000:00:14.0: xHCI xhci_free_dev called with unaddressed device

On the test machine I couldn't reproduce the not-halted case through a
normal xhci_suspend()/xhci_resume() cycle. But the reset_registers path
is taken exactly when the host state is not trusted (power lost, broken
suspend, HCE, RESET_ON_RESUME), so making sure xhci_reset() actually
completed seems right regardless.

> There is apparently one caller (xhci_resume()) which cares about
> xhci_reset() return value but not about the preceding xhci_halt().

Yes, xhci_resume() is exactly the caller I had in mind. Since it doesn't
check xhci_halt(), the reset return value is the only thing standing
between a host that failed to halt and the reinit.

Thanks,
Henry