Re: [PATCH] usb: xhci: Restore MSI/MSI-X reinitialization on resume for RESET_ON_RESUME hosts

[Mathias Nyman <mathias.nyman@linux.intel.com>]

Hi

On 7/14/26 12:02, Jie Deng wrote:
> Commit 944e7deb4238 ("xhci: Avoid PCI MSI/MSIX interrupt
> reinitialization at resume") moved MSI/MSI-X setup out of xhci_run()
> into a PCI-specific xhci_pci_run() that is only invoked at probe time,
> and removed the xhci_cleanup_msix() call from xhci_resume().
> 
> The rationale (avoid redundant MSI reconfiguration on every resume) is
> sound for controllers that preserve state across suspend.  It is however
> *harmful* for controllers carrying XHCI_RESET_ON_RESUME: those hosts are
> fully reset on every S3 resume, and on some of them (e.g. ASMedia 3042)
> the MSI/MSI-X delivery path is no longer functional after the controller
> reset + D3->D0 transition.  Because the MSI vectors are now kept around
> unchanged across resume, the first post-resume command-completion event
> (ENABLE_SLOT) is delivered to a stale MSI channel and never reaches the
> CPU, which surfaces as:
>    xhci_hcd 0000:05:00.0: Error while assigning device slot ID: Command Aborted
>    xhci_hcd 0000:05:00.0: Max number of devices this xHCI host supports is 127.
>    usb 1-2: device not accepting address 3, error -22
>    xhci_hcd 0000:05:00.0: Error while assigning device slot ID: Command Aborted
>    xhci_hcd 0000:05:00.0: Max number of devices this xHCI host supports is 127.
>    usb 1-2: device not accepting address 3, error -22
>    xhci_hcd 0000:05:00.0: Error while assigning device slot ID: Command Aborted
>    xhci_hcd 0000:05:00.0: Max number of devices this xHCI host supports is 127.
>    usb 2-2: device not accepting address 3, error -22
> 

Thanks for reporting and debugging this.

Is this also an issue in runtime resume where system stays in S0 while
xHC transitions from D3 to D0, and is then reset?

Or is it only in system suspend, or in hibernate?
Does it matter if xHC was in D3hot or D3cold before resume?

Not sure if gating with XHCI_RESET_ON_RESUME is the right way as some of those
xHCI PCI hosts may just need to be reset in resume, but retain their MSI settings.
(xhci specs states that resetting xHC should not touch PCI config space)

This may just be a ASMedia PCI xHCI issue.


> Up to v6.3 the resume reset path did xhci_cleanup_msix() and, through
> xhci_run() -> xhci_try_enable_msi(), re-allocated the vectors, which
> incidentally re-armed the MSI delivery path and masked the hardware
> issue.  Work it around by restoring that behaviour, but only for hosts
> marked XHCI_RESET_ON_RESUME and only for PCI devices (platform/RCAR
> hosts using the same quirk must not be touched).
> 
> Fix: 944e7deb4238 ("xhci: Avoid PCI MSI/MSIX interrupt reinitialization at resume")
> Signed-off-by: Jie Deng <dengjie03@kylinos.cn>
> ---
>   drivers/usb/host/xhci-pci.c |  6 ++++--
>   drivers/usb/host/xhci.c     | 19 +++++++++++++++++++
>   drivers/usb/host/xhci.h     |  2 ++
>   3 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 6b3fcba44b08..77b3215c95f3 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -126,7 +126,7 @@ static void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
>   }
>   
>   /* Legacy IRQ is freed by usb_remove_hcd() or usb_hcd_pci_shutdown() */
> -static void xhci_cleanup_msix(struct xhci_hcd *xhci)
> +void xhci_cleanup_msix(struct xhci_hcd *xhci)
>   {
>   	struct usb_hcd *hcd = xhci_to_hcd(xhci);
>   	struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
> @@ -138,9 +138,10 @@ static void xhci_cleanup_msix(struct xhci_hcd *xhci)
>   	pci_free_irq_vectors(pdev);
>   	hcd->msix_enabled = 0;
>   }
> +EXPORT_SYMBOL_GPL(xhci_cleanup_msix);
>   
>   /* Try enabling MSI-X with MSI and legacy IRQ as fallback */
> -static int xhci_try_enable_msi(struct usb_hcd *hcd)
> +int xhci_try_enable_msi(struct usb_hcd *hcd)
>   {
>   	struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
>   	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> @@ -207,6 +208,7 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
>   	hcd->irq = pdev->irq;
>   	return 0;
>   }
> +EXPORT_SYMBOL_GPL(xhci_try_enable_msi);
>   
>   static int xhci_pci_run(struct usb_hcd *hcd)
>   {
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 091c82ca8ee2..7dd3afcf6b2e 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1190,10 +1190,29 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
>   		xhci_for_each_ring_seg(xhci->cmd_ring->first_seg, seg)
>   			memset(seg->trbs, 0, sizeof(union xhci_trb) * TRBS_PER_SEGMENT);
>   
> +		/* XHCI_RESET_ON_RESUME hosts are fully reset on every resume.
> +		 * On some of them (e.g. ASMedia 3042) the MSI/MSI-X delivery
> +		 * path is no longer valid after the reset + D3->D0 cycle, so
> +		 * tear the vectors down here and let xhci_try_enable_msi()
> +		 * re-allocate them below, matching pre-6.4 behaviour.
> +		 */
> +		if (xhci->quirks & XHCI_RESET_ON_RESUME)
> +			xhci_cleanup_msix(xhci);

Ideally all PCI related xHCI code should be done in xhci-pci.c, like xhci_pci_resume()

In this case it's a bit tricky as msi cleanup and enabling is done mid xhci_resume(),
and would require a lot of refactoring to get it done.

Might need to do it this way first to get the ASMedia host working, and then refactor later.

> +
>   		xhci_debugfs_exit(xhci);
>   
>   		xhci_init(hcd);
>   
> +		/* Re-arm the MSI/MSI-X delivery path for RESET_ON_RESUME PCI
> +		 * hosts. xhci_run() no longer does this (it was moved to the
> +		 * probe-only xhci_pci_run()), and the stale vectors left over
> +		 * from before suspend cannot carry the first post-resume
> +		 * command-completion event on affected hardware.
> +		 */
> +		if ((xhci->quirks & XHCI_RESET_ON_RESUME) &&
> +		    dev_is_pci(hcd->self.controller))
> +			xhci_try_enable_msi(hcd);

Same comment as above

Thanks
Mathias