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

Jie Deng posted 1 patch 1 week, 4 days ago
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(-)
[PATCH] usb: xhci: Restore MSI/MSI-X reinitialization on resume for RESET_ON_RESUME hosts
Posted by Jie Deng 1 week, 4 days ago
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

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);
+
 		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);
+
 		/*
 		 * USB core calls the PCI reinit and start functions twice:
 		 * first with the primary HCD, and then with the secondary HCD.
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2d3941b5e1e3..a72ebfbb634a 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1903,6 +1903,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume);
 
 irqreturn_t xhci_irq(struct usb_hcd *hcd);
 irqreturn_t xhci_msi_irq(int irq, void *hcd);
+void xhci_cleanup_msix(struct xhci_hcd *xhci);
+int xhci_try_enable_msi(struct usb_hcd *hcd);
 int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev);
 int xhci_alloc_tt_info(struct xhci_hcd *xhci,
 		struct xhci_virt_device *virt_dev,
-- 
2.25.1