Linux USB
 help / color / mirror / Atom feed
* [PATCH 0/2] PCI: pciehp / thunderbolt: Fix resume with a slow Thunderbolt host link
@ 2026-10-18 16:37 Cole Leavitt
  2026-10-18 16:39 ` [PATCH 1/2] PCI: pciehp: Defer device replacement handling until ->resume() Cole Leavitt
  0 siblings, 1 reply; 5+ messages in thread
From: Cole Leavitt @ 2026-10-18 16:37 UTC (permalink / raw)
  To: Bjorn Helgaas, Mika Westerberg
  Cc: Cole Leavitt, Lukas Wunner, Andreas Noever, Yehezkel Bernat,
	Ilpo Järvinen, linux-pci, linux-usb, linux-kernel

On a Lenovo ThinkPad P16 Gen 3 with a discrete Intel JHL9580 (Barlow
Ridge) Thunderbolt 5 host controller below Root Port 0000:80:1b.4,
resume from suspend-to-idle intermittently hangs with a black screen.
The link to the controller is not up in time when the Root Port leaves
D3cold, pciehp concludes that the device in the slot was replaced, and
the resulting removal races with the thunderbolt driver's
->resume_noirq() callback, which dereferences tb->root_switch after
tb_stop() cleared it.

Patch 1 defers acting on the synthesized Presence Detect Changed event
in pciehp until the ->resume() phase, where the PM core serializes the
removal against the drivers below the port.

Patch 2 stops the thunderbolt software connection manager from going
through its resume sequence when nhi_resume_noirq() already found that
the host controller is gone.  Without it every control channel request
fails with -ESHUTDOWN and triggers a WARN in tb_cfg_read() or
tb_cfg_write().

The patches are independent and can be applied through the PCI and
thunderbolt trees separately.

Cole Leavitt (2):
  PCI: pciehp: Defer device replacement handling until ->resume()
  thunderbolt: Don't resume the software CM if the host controller is
    gone

 drivers/pci/hotplug/pciehp_core.c | 26 ++++++++++++++++++++++++--
 drivers/thunderbolt/tb.c          | 13 +++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)


base-commit: 32c52ec87d6245ee2d12ed5dd509e40062939b50
-- 
2.54.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] PCI: pciehp: Defer device replacement handling until ->resume()
  2026-10-18 16:37 [PATCH 0/2] PCI: pciehp / thunderbolt: Fix resume with a slow Thunderbolt host link Cole Leavitt
@ 2026-10-18 16:39 ` Cole Leavitt
  2026-10-18 16:39   ` [PATCH 2/2] thunderbolt: Don't resume the software CM if the host controller is gone Cole Leavitt
  2026-10-18 16:47   ` [PATCH 1/2] PCI: pciehp: Defer device replacement handling until ->resume() sashiko-bot
  0 siblings, 2 replies; 5+ messages in thread
From: Cole Leavitt @ 2026-10-18 16:39 UTC (permalink / raw)
  To: Bjorn Helgaas, Mika Westerberg
  Cc: Cole Leavitt, Lukas Wunner, Andreas Noever, Yehezkel Bernat,
	Ilpo Järvinen, linux-pci, linux-usb, linux-kernel

On a Lenovo ThinkPad P16 Gen3 with a discrete Thunderbolt 5 host
controller (Intel JHL9580 "Barlow Ridge") below Root Port 0000:80:1b.4,
resume from suspend-to-idle intermittently hangs with a black screen:

  pcieport 0000:80:1b.4: Data Link Layer Link Active not set in 100 msec
  pcieport 0000:80:1b.4: pciehp: Slot(25): Card not present
  thunderbolt 0000:8a:00.0: RX ring 0 already stopped
  ...
  Oops: general protection fault, kernel NULL pointer dereference 0x32b
  Workqueue: async async_run_entry_fn
  RIP: 0010:tb_resume_noirq+0x3f/0x2e0 [thunderbolt]
  Call Trace:
   tb_domain_resume_noirq+0x42/0x60 [thunderbolt]
   dpm_run_callback+0x4b/0x140
   device_resume_noirq+0x16d/0x2c0
   async_resume_noirq+0x21/0x30
   async_run_entry_fn+0x36/0x160

The link to Barlow Ridge is not up in time when the Root Port returns
from D3cold, so the devices below it are marked disconnected and
pciehp_resume_noirq() considers the device in the slot replaced.  It
then synthesizes a Presence Detect Changed event with pciehp_request(),
which wakes the IRQ thread through irq_wake_thread().  Unlike hardirq
delivery, irq_wake_thread() is not inhibited by suspend_device_irqs(),
so the IRQ thread starts bringing down the slot immediately, in the
middle of the noirq phase.

The PM core resumes the devices below the hotplug port asynchronously
and holds no device_lock in the noirq and early phases, so the removal
is not serialized against their ->resume_noirq() callbacks:  The IRQ
thread unbinds the NHI (nhi_pci_remove() -> tb_domain_remove() ->
tb_stop(), which sets tb->root_switch to NULL) while nhi_resume_noirq()
-> tb_resume_noirq() runs concurrently on another CPU and dereferences
tb->root_switch.  Before commit e56249d8a68e ("thunderbolt: Set
tb->root_switch to NULL when domain is stopped") the pointer was left
dangling instead.  Any other driver below a hotplug port is exposed to
the same race.

Only record the synthesized event in pciehp_resume_noirq() and wake the
IRQ thread from pciehp_system_resume(), the ->resume() callback used for
system sleep transitions.  By then all noirq and early callbacks have
completed, and device_resume() holds the device_lock across each
driver's ->resume() callback, which the removal path acquires in
device_release_driver().  This is also where pciehp_check_presence()
already requests bringing down slots vacated during system sleep.
Runtime resume is left alone.

The old devices are still marked disconnected in the noirq phase, so
that their drivers and any nested hotplug ports (see commit e3260237aaad
("PCI: pciehp: Avoid unnecessary device replacement check")) keep
seeing them as gone.

A real hotplug interrupt cannot consume the deferred event earlier
either, as long as PME is native:  pciehp_suspend() then masks the
hotplug interrupt and only pciehp_resume() unmasks it again.  Without
native PME the interrupt stays enabled across system sleep, and an event
latched after resume_device_irqs() can still run the IRQ thread during
the early phase, but no longer during the noirq phase.

pciehp_resume_noirq() also serves as ->restore_noirq() and is likewise
followed by ->resume(), so resume from hibernation is fixed as well.

Fixes: 9d573d19547b ("PCI: pciehp: Detect device replacement during system sleep")
Cc: stable@vger.kernel.org # v6.11+
Signed-off-by: Cole Leavitt <cole@unwrap.rs>
---

Notes:
    Tested on a Lenovo ThinkPad P16 Gen 3 (Core Ultra 9 275HX, JHL9580
    Barlow Ridge host, I226 Ethernet), in daily use with s2idle since
    2026-09-17.

 drivers/pci/hotplug/pciehp_core.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index 2cafd3b26f34..850a6d3bb08a 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -302,12 +302,19 @@ static int pciehp_resume_noirq(struct pcie_device *dev)
 		 * during system sleep, mark the old device disconnected
 		 * (to prevent its driver from accessing the new device)
 		 * and synthesize a Presence Detect Changed event.
+		 *
+		 * Don't wake the IRQ thread yet:  The PM core resumes the
+		 * devices below asynchronously and without holding their
+		 * device_lock in the noirq and early phases, so bringing
+		 * the slot down now would race with their ->resume_noirq()
+		 * and ->resume_early() callbacks.  pciehp_system_resume()
+		 * wakes the IRQ thread once those have completed.
 		 */
 		if (pciehp_device_replaced(ctrl)) {
 			ctrl_dbg(ctrl, "device replaced during system sleep\n");
 			pci_walk_bus(ctrl->pcie->port->subordinate,
 				     pci_dev_set_disconnected, NULL);
-			pciehp_request(ctrl, PCI_EXP_SLTSTA_PDC);
+			atomic_or(PCI_EXP_SLTSTA_PDC, &ctrl->pending_events);
 		}
 	}
 
@@ -327,6 +334,21 @@ static int pciehp_resume(struct pcie_device *dev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int pciehp_system_resume(struct pcie_device *dev)
+{
+	struct controller *ctrl = get_service_data(dev);
+
+	pciehp_resume(dev);
+
+	/* Act on events deferred by pciehp_resume_noirq() */
+	if (atomic_read(&ctrl->pending_events))
+		pciehp_request(ctrl, 0);
+
+	return 0;
+}
+#endif
+
 static int pciehp_runtime_suspend(struct pcie_device *dev)
 {
 	pciehp_disable_interrupt(dev);
@@ -362,7 +384,7 @@ static struct pcie_port_service_driver hpdriver_portdrv = {
 #ifdef	CONFIG_PM_SLEEP
 	.suspend	= pciehp_suspend,
 	.resume_noirq	= pciehp_resume_noirq,
-	.resume		= pciehp_resume,
+	.resume		= pciehp_system_resume,
 #endif
 	.runtime_suspend = pciehp_runtime_suspend,
 	.runtime_resume	= pciehp_runtime_resume,
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] thunderbolt: Don't resume the software CM if the host controller is gone
  2026-10-18 16:39 ` [PATCH 1/2] PCI: pciehp: Defer device replacement handling until ->resume() Cole Leavitt
@ 2026-10-18 16:39   ` Cole Leavitt
  2026-10-18 16:45     ` sashiko-bot
  2026-10-18 16:47   ` [PATCH 1/2] PCI: pciehp: Defer device replacement handling until ->resume() sashiko-bot
  1 sibling, 1 reply; 5+ messages in thread
From: Cole Leavitt @ 2026-10-18 16:39 UTC (permalink / raw)
  To: Bjorn Helgaas, Mika Westerberg
  Cc: Cole Leavitt, Lukas Wunner, Andreas Noever, Yehezkel Bernat,
	Ilpo Järvinen, linux-pci, linux-usb, linux-kernel

nhi_resume_noirq() sets nhi->going_away when the host controller is not
present anymore on resume (commit bdccf295d7cd ("thunderbolt: Do not
touch the hardware if the NHI is gone on resume")). With going_away set
tb_ring_start() refuses to start the control channel rings, but the
software connection manager's tb_resume_noirq() still goes through the
whole resume sequence. Every control channel request then fails with
-ESHUTDOWN from __tb_ring_enqueue(), and tb_cfg_read()/tb_cfg_write()
WARN for each one:

  tb_cfg_write: -108
  WARNING: drivers/thunderbolt/ctl.c:1157 at tb_cfg_write+0x94/0xf0 [thunderbolt]
  Call Trace:
   tb_switch_configure+0x8f/0x1d0 [thunderbolt]
   tb_switch_resume+0xb9/0x3d0 [thunderbolt]
   tb_resume_noirq+0x5d/0x2e0 [thunderbolt]
   tb_domain_resume_noirq+0x42/0x60 [thunderbolt]
   ...
  tb_cfg_read: -108
  WARNING: drivers/thunderbolt/ctl.c:1131 at tb_cfg_read+0x94/0xf0 [thunderbolt]
  ...
  thunderbolt 0000:8a:00.0: 0: failed to restore TMU configuration

This happens on a discrete Barlow Ridge host when the root port link
does not come back in time on resume from s2idle: pciehp marks the
subtree disconnected in pciehp_resume_noirq(), so pci_device_is_present()
returns false for the NHI and the (about to be removed) domain is then
"resumed" through dead rings.

The firmware connection manager already honors the flag in
icm_complete(). Do the same in the software connection manager and skip
tb_resume_noirq() when the host controller is gone. The domain is torn
down when the controller is removed, and hotplug_active stays false so
no events are processed in the meantime.

Fixes: bdccf295d7cd ("thunderbolt: Do not touch the hardware if the NHI is gone on resume")
Cc: stable@vger.kernel.org
Signed-off-by: Cole Leavitt <cole@unwrap.rs>
---

Notes:
    Tested on a Lenovo ThinkPad P16 Gen 3 (Core Ultra 9 275HX, JHL9580
    Barlow Ridge host, I226 Ethernet), in daily use with s2idle since
    2026-09-17.

 drivers/thunderbolt/tb.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 4e5d0578fd4b..d236de1e5568 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -3177,6 +3177,19 @@ static int tb_resume_noirq(struct tb *tb)
 	unsigned int usb3_delay = 0;
 	LIST_HEAD(tunnels);
 
+	/*
+	 * The host controller is gone (for example the last device was
+	 * unplugged, or the PCIe hotplug driver found the controller
+	 * removed or replaced during system sleep). The control channel
+	 * rings are not started in that case so every config space access
+	 * below would just fail. Leave the domain as is; it is torn down
+	 * when the controller is removed.
+	 */
+	if (tb->nhi->going_away) {
+		tb_dbg(tb, "host controller not present, skipping resume\n");
+		return 0;
+	}
+
 	tb_dbg(tb, "resuming...\n");
 
 	/*
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] thunderbolt: Don't resume the software CM if the host controller is gone
  2026-10-18 16:39   ` [PATCH 2/2] thunderbolt: Don't resume the software CM if the host controller is gone Cole Leavitt
@ 2026-10-18 16:45     ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-18 16:45 UTC (permalink / raw)
  To: Cole Leavitt; +Cc: linux-pci, linux-usb

> nhi_resume_noirq() sets nhi->going_away when the host controller is not
> present anymore on resume (commit bdccf295d7cd ("thunderbolt: Do not
> touch the hardware if the NHI is gone on resume")). With going_away set
> tb_ring_start() refuses to start the control channel rings, but the
> software connection manager's tb_resume_noirq() still goes through the
> whole resume sequence. Every control channel request then fails with
> -ESHUTDOWN from __tb_ring_enqueue(), and tb_cfg_read()/tb_cfg_write()
> WARN for each one:
> 
>   tb_cfg_write: -108
>   WARNING: drivers/thunderbolt/ctl.c:1157 at tb_cfg_write+0x94/0xf0 [thunderbolt]
>   Call Trace:
>    tb_switch_configure+0x8f/0x1d0 [thunderbolt]
>    tb_switch_resume+0xb9/0x3d0 [thunderbolt]
>    tb_resume_noirq+0x5d/0x2e0 [thunderbolt]
> [ ... ]
> no events are processed in the meantime.
> 
> Fixes: bdccf295d7cd ("thunderbolt: Do not touch the hardware if the NHI is gone on resume")
> Cc: stable@vger.kernel.org
> Signed-off-by: Cole Leavitt <cole@unwrap.rs>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018163910.132231-1-cole@unwrap.rs?part=2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] PCI: pciehp: Defer device replacement handling until ->resume()
  2026-10-18 16:39 ` [PATCH 1/2] PCI: pciehp: Defer device replacement handling until ->resume() Cole Leavitt
  2026-10-18 16:39   ` [PATCH 2/2] thunderbolt: Don't resume the software CM if the host controller is gone Cole Leavitt
@ 2026-10-18 16:47   ` sashiko-bot
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-18 16:47 UTC (permalink / raw)
  To: Cole Leavitt; +Cc: linux-pci, linux-usb

> On a Lenovo ThinkPad P16 Gen3 with a discrete Thunderbolt 5 host
> controller (Intel JHL9580 "Barlow Ridge") below Root Port 0000:80:1b.4,
> resume from suspend-to-idle intermittently hangs with a black screen:
> 
>   pcieport 0000:80:1b.4: Data Link Layer Link Active not set in 100 msec
>   pcieport 0000:80:1b.4: pciehp: Slot(25): Card not present
>   thunderbolt 0000:8a:00.0: RX ring 0 already stopped
>   ...
>   Oops: general protection fault, kernel NULL pointer dereference 0x32b
>   Workqueue: async async_run_entry_fn
>   RIP: 0010:tb_resume_noirq+0x3f/0x2e0 [thunderbolt]
>   Call Trace:
>    tb_domain_resume_noirq+0x42/0x60 [thunderbolt]
>    dpm_run_callback+0x4b/0x140
>    device_resume_noirq+0x16d/0x2c0
> [ ... ]
> followed by ->resume(), so resume from hibernation is fixed as well.
> 
> Fixes: 9d573d19547b ("PCI: pciehp: Detect device replacement during system sleep")
> Cc: stable@vger.kernel.org # v6.11+
> Signed-off-by: Cole Leavitt <cole@unwrap.rs>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018163910.132231-1-cole@unwrap.rs?part=1


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-18 16:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-18 16:37 [PATCH 0/2] PCI: pciehp / thunderbolt: Fix resume with a slow Thunderbolt host link Cole Leavitt
2026-10-18 16:39 ` [PATCH 1/2] PCI: pciehp: Defer device replacement handling until ->resume() Cole Leavitt
2026-10-18 16:39   ` [PATCH 2/2] thunderbolt: Don't resume the software CM if the host controller is gone Cole Leavitt
2026-10-18 16:45     ` sashiko-bot
2026-10-18 16:47   ` [PATCH 1/2] PCI: pciehp: Defer device replacement handling until ->resume() sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox