[PATCH] usb: xhci: bail out of setup if the controller is inaccessible

[PATCH] usb: xhci: bail out of setup if the controller is inaccessible

[Breno Leitao]

xhci_gen_setup() locates the operational registers using the capability
length read from the very first register:

	xhci->op_regs = hcd->regs +
		HC_LENGTH(readl(&xhci->cap_regs->hc_capbase));

If the controller is dead or has dropped off the bus, that read returns
~0, as I saw in practice.

The first access through it, xhci_halt() -> xhci_handshake() reading
op_regs->status,  unaligned readl() on device memory. arm64
faults on unaligned device accesses, so instead of xhci_handshake()
catching the all-ones value and returning -ENODEV, setup oopses:

  xhci-pci-renesas 0005:08:00.0: Unable to change power state from D3cold to D0, device inaccessible
  xhci-pci-renesas 0005:08:00.0: xHCI Host Controller
  xhci-pci-renesas 0005:08:00.0: new USB bus registered, assigned bus number 1
  Unable to handle kernel paging request at virtual address ffff80030a770103
    ESR = 0x0000000096000021
    FSC = 0x21: alignment fault
  Internal error: Oops: 0000000096000021 [#1]  SMP
  pc : xhci_halt [xhci_hcd]
  Call trace:
   xhci_halt
   xhci_gen_setup
   xhci_pci_setup
   usb_add_hcd
   usb_hcd_pci_probe
   xhci_pci_common_prob
   xhci_pci_renesas_probe

This was hit with a Renesas uPD720201 that failed to power up ("Unable
to change power state from D3cold to D0, device inaccessible") yet still
reached the HCD probe path.

Detect the removed controller the way xhci_handshake() and xhci_reset()
already do, by testing the register for the all-ones value, and abort
setup with -ENODEV before op_regs is derived from it.

Fixes: 66d4eadd8d06 ("USB: xhci: BIOS handoff and HW initialization.")
Cc: stable@vger.kernel.org
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/usb/host/xhci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 091c82ca8ee29..4e8a87df91d9d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -5453,6 +5453,10 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 	mutex_init(&xhci->mutex);
 	xhci->main_hcd = hcd;
 	xhci->cap_regs = hcd->regs;
+	if (readl(&xhci->cap_regs->hc_capbase) == U32_MAX) {
+		xhci_warn(xhci, "Host controller not accessible, removed?\n");
+		return -ENODEV;
+	}
 	xhci->op_regs = hcd->regs +
 		HC_LENGTH(readl(&xhci->cap_regs->hc_capbase));
 	xhci->run_regs = hcd->regs +

---
base-commit: 290aaf24a551d5a0dce037e3fab30820f9113a10
change-id: 20260722-xhci_dead_hc-35fa846923b2

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH] usb: xhci: bail out of setup if the controller is inaccessible

[Michal Pecio]

On Wed, 22 Jul 2026 04:27:36 -0700, Breno Leitao wrote:
> xhci_gen_setup() locates the operational registers using the capability
> length read from the very first register:
> 
> 	xhci->op_regs = hcd->regs +
> 		HC_LENGTH(readl(&xhci->cap_regs->hc_capbase));
> 
> If the controller is dead or has dropped off the bus, that read returns
> ~0, as I saw in practice.
> 
> The first access through it, xhci_halt() -> xhci_handshake() reading
> op_regs->status,  unaligned readl() on device memory. arm64
> faults on unaligned device accesses, so instead of xhci_handshake()
> catching the all-ones value and returning -ENODEV, setup oopses:

And if it didn't oops then it would read some other register, and later
write it, and maybe do stupid things. At least HC_LENGTH macro
truncates ~0 to 255, so the other register would most likely still
belong to this xHCI.

>   xhci-pci-renesas 0005:08:00.0: Unable to change power state from D3cold to D0, device inaccessible
>   xhci-pci-renesas 0005:08:00.0: xHCI Host Controller
>   xhci-pci-renesas 0005:08:00.0: new USB bus registered, assigned bus number 1
>   Unable to handle kernel paging request at virtual address ffff80030a770103
>     ESR = 0x0000000096000021
>     FSC = 0x21: alignment fault
>   Internal error: Oops: 0000000096000021 [#1]  SMP
>   pc : xhci_halt [xhci_hcd]
>   Call trace:
>    xhci_halt
>    xhci_gen_setup
>    xhci_pci_setup
>    usb_add_hcd
>    usb_hcd_pci_probe
>    xhci_pci_common_prob
>    xhci_pci_renesas_probe
> 
> This was hit with a Renesas uPD720201 that failed to power up ("Unable
> to change power state from D3cold to D0, device inaccessible") yet still
> reached the HCD probe path.

Seems unproductive, I wonder if it's somehow intentional or a PCI bug.
You would need to ask linux-pci about it.

> Detect the removed controller the way xhci_handshake() and xhci_reset()
> already do, by testing the register for the all-ones value, and abort
> setup with -ENODEV before op_regs is derived from it.
> 
> Fixes: 66d4eadd8d06 ("USB: xhci: BIOS handoff and HW initialization.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  drivers/usb/host/xhci.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 091c82ca8ee29..4e8a87df91d9d 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -5453,6 +5453,10 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
>  	mutex_init(&xhci->mutex);
>  	xhci->main_hcd = hcd;
>  	xhci->cap_regs = hcd->regs;
> +	if (readl(&xhci->cap_regs->hc_capbase) == U32_MAX) {
> +		xhci_warn(xhci, "Host controller not accessible, removed?\n");
> +		return -ENODEV;
> +	}

Good idea, but there is one more case to potentially worry about: hot
removal. In theory, you could read a valid value here and then U32_MAX
below and crash the same as before.

It would be more robust (and efficient) to read the register once to
some tmp variable, validate it and then use that to set xhci->op_regs.

>  	xhci->op_regs = hcd->regs +
>  		HC_LENGTH(readl(&xhci->cap_regs->hc_capbase));
>  	xhci->run_regs = hcd->regs +
>  		(readl(&xhci->cap_regs->run_regs_off) & RTSOFF_MASK);

One may wonder if run_regs shouldn't have similar check. However, if
the chip is hot removed here after successfully reading hc_capbase,
xhci_halt() will not crash and return -ENODEV before anyone uses the
bogus run_regs pointer. So there is no urgent problem here, I think.

Regards,
Michal