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

Lukas Wunner
2025-03-03T20:00:36Z

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@wunner.de/
> >
> > https://lore.kernel.org/all/20240612181625.3604512-1-kbusch@meta.com/
>
> 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;