Re: diagnosing resume failures after disconnected USB4 drives (Was: Re: PCI/ASPM: Fix L1SS saving (linus/master commit 7507eb3e7bfac))

[Lukas Wunner]

On Wed, Feb 26, 2025 at 11:19:58AM +0200, Mika Westerberg wrote:
> On Wed, Feb 26, 2025 at 10:10:43AM +0100, Lukas Wunner wrote:
> > On Wed, Feb 26, 2025 at 10:44:04AM +0200, Mika Westerberg wrote:
> > >   [Meteor Lake host] <--> [TB 4 dock] <--> [TB 3 NVMe]
> > [...]
> > > I added "no_console_suspend" to the command line and the did sysrq-w to
> > > get list of blocked tasks. I've attached it just in case it is needed.
> > 
> > This looks like the deadlock we've had for years when hot-removing
> > nested hotplug ports.
> > 
> > If you attach only a single device to the host, I guess the issue
> > does not occur, right?
> 
> Yes.
> 
> > Previous attempts to fix this:
> > 
> > https://lore.kernel.org/all/4c882e25194ba8282b78fe963fec8faae7cf23eb.1529173804.git.lukas@xxxxxxxxx/
> > 
> > https://lore.kernel.org/all/20240612181625.3604512-1-kbusch@xxxxxxxx/
> 
> Well, it does not happen if I revert the commit so isn't that a
> regresssion?

Does the below fix the issue?

-- >8 --

diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index ff458e6..b0b4d46 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -287,24 +287,26 @@ static int pciehp_suspend(struct pcie_device *dev)
 static bool pciehp_device_replaced(struct controller *ctrl)
 {
 	struct pci_dev *pdev __free(pci_dev_put);
+	u64 dsn;
 	u32 reg;
 
 	pdev = pci_get_slot(ctrl->pcie->port->subordinate, PCI_DEVFN(0, 0));
 	if (!pdev)
-		return true;
+		return false;
 
-	if (pci_read_config_dword(pdev, PCI_VENDOR_ID, &reg) ||
-	    reg != (pdev->vendor | (pdev->device << 16)) ||
-	    pci_read_config_dword(pdev, PCI_CLASS_REVISION, &reg) ||
-	    reg != (pdev->revision | (pdev->class << 8)))
+	if ((pci_read_config_dword(pdev, PCI_VENDOR_ID, &reg) == 0 &&
+	     reg != (pdev->vendor | (pdev->device << 16))) ||
+	    (pci_read_config_dword(pdev, PCI_CLASS_REVISION, &reg) == 0 &&
+	     reg != (pdev->revision | (pdev->class << 8))))
 		return true;
 
 	if (pdev->hdr_type == PCI_HEADER_TYPE_NORMAL &&
-	    (pci_read_config_dword(pdev, PCI_SUBSYSTEM_VENDOR_ID, &reg) ||
-	     reg != (pdev->subsystem_vendor | (pdev->subsystem_device << 16))))
+	    pci_read_config_dword(pdev, PCI_SUBSYSTEM_VENDOR_ID, &reg) == 0 &&
+	    reg != (pdev->subsystem_vendor | (pdev->subsystem_device << 16)))
 		return true;
 
-	if (pci_get_dsn(pdev) != ctrl->dsn)
+	dsn = pci_get_dsn(pdev);
+	if ((dsn || ctrl->dsn) && dsn != ctrl->dsn)
 		return true;
 
 	return false;