[PATCH] thunderbolt: Optionally keep PCIe tunnels up through suspend-to-idle

[PATCH] thunderbolt: Optionally keep PCIe tunnels up through suspend-to-idle

[Andrei Rusu de Castro]

Every system sleep puts the USB4 routers to sleep, which takes all
tunnels down, and the connection manager re-creates them on resume. A
PCIe device behind a tunnel disappears from the bus in between. An
external GPU that drives a display does not survive that: on a ROG Flow
Z13 (GZ302EA) with an RTX 5090 in a Razer Core X V2 driving an external
monitor, the GPU driver found the device gone after suspend-to-idle, the
enclosure was removed and re-added under a compositor still using it,
and the desktop froze.

For suspend-to-idle the domain can keep running instead. Add a module
parameter, s2idle_keep_pcie, off by default. When it is set, the target
state is suspend-to-idle and the software connection manager has a PCIe
tunnel, leave the routers, links and control channel running and keep
the host interface in D0 by saving its PCI state in the noirq phase, so
the PCI core does not put it into D3. The devices behind the tunnel then
go through an ordinary PCI suspend and resume. Without a PCIe tunnel, in
other sleep states and with the firmware connection manager nothing
changes.

With the parameter set on the GZ302EA, suspend-to-idle with the eGPU
driving the monitor resumes with the GPU still on the bus, the same
compositor process and the dock's USB devices intact. With this
enclosure attached the system does not reach hardware sleep either
way, with the tunnels kept or torn down (amd-s2idle: 0% hardware
sleep); without it, the same system reaches 99% hardware sleep at
about 1.1 W. The cost of keeping a tunnel up on a system that
otherwise reaches hardware sleep was not measured.

An LLM assisted with finding the cause, writing this change and its
description, and running the tests above on my GZ302EA.

Signed-off-by: Andrei Rusu de Castro <arc@empyreal.works>
---
 drivers/thunderbolt/domain.c |  5 +++--
 drivers/thunderbolt/nhi.c    | 14 ++++++++++++
 drivers/thunderbolt/tb.c     | 43 ++++++++++++++++++++++++++++++++++++
 include/linux/thunderbolt.h  |  3 +++
 4 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index 24611f05b..168a286cd 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -535,7 +535,7 @@ int tb_domain_suspend_noirq(struct tb *tb)
 	mutex_lock(&tb->lock);
 	if (tb->cm_ops->suspend_noirq)
 		ret = tb->cm_ops->suspend_noirq(tb);
-	if (!ret)
+	if (!ret && !tb->kept_running)
 		tb_ctl_stop(tb->ctl);
 	mutex_unlock(&tb->lock);
 
@@ -556,7 +556,8 @@ int tb_domain_resume_noirq(struct tb *tb)
 	int ret = 0;
 
 	mutex_lock(&tb->lock);
-	tb_ctl_start(tb->ctl);
+	if (!tb->kept_running)
+		tb_ctl_start(tb->ctl);
 	if (tb->cm_ops->resume_noirq)
 		ret = tb->cm_ops->resume_noirq(tb);
 	mutex_unlock(&tb->lock);
diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index f54ab2189..c453153c2 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -1089,6 +1089,17 @@ static int __nhi_suspend_noirq(struct device *dev, bool wakeup)
 	if (ret)
 		return ret;
 
+	if (tb->kept_running) {
+		/*
+		 * Saving the state here keeps the PCI core from putting the
+		 * host router into D3, which would take the tunnels down;
+		 * for suspend-to-idle it then skips the bus PM of this
+		 * device on resume as well.
+		 */
+		pci_save_state(to_pci_dev(dev));
+		return 0;
+	}
+
 	if (nhi->ops->suspend_noirq) {
 		ret = nhi->ops->suspend_noirq(tb->nhi, wakeup);
 		if (ret)
@@ -1145,6 +1156,9 @@ static int nhi_resume_noirq(struct device *dev)
 	struct tb_nhi *nhi = tb->nhi;
 	int ret;
 
+	if (tb->kept_running)
+		return tb_domain_resume_noirq(tb);
+
 	/*
 	 * Check that the device is still there. It may be that the user
 	 * unplugged last device which causes the host controller to go
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 418ad0a61..641502ec2 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -9,8 +9,10 @@
 #include <linux/slab.h>
 #include <linux/errno.h>
 #include <linux/delay.h>
+#include <linux/moduleparam.h>
 #include <linux/pm_runtime.h>
 #include <linux/stringify.h>
+#include <linux/suspend.h>
 
 #include "tb.h"
 #include "tb_regs.h"
@@ -3106,10 +3108,45 @@ static int tb_start(struct tb *tb, bool reset)
 	return 0;
 }
 
+static bool s2idle_keep_pcie;
+module_param(s2idle_keep_pcie, bool, 0644);
+MODULE_PARM_DESC(s2idle_keep_pcie,
+		 "keep a domain with PCIe tunnels running through suspend-to-idle so the devices behind them stay connected (default: false)");
+
+/*
+ * Putting the routers to sleep takes every tunnel down, and the PCIe
+ * devices behind a tunnel then disappear from the bus until it is
+ * re-created on resume. A device that cannot lose its link while its
+ * driver holds it (an external GPU driving a display) does not survive
+ * that. For suspend-to-idle the domain can instead keep running: the
+ * routers stay awake, the links and tunnels stay up and the devices
+ * behind them go through an ordinary PCI suspend and resume.
+ */
+static bool tb_keep_running_s2idle(struct tb *tb)
+{
+	struct tb_cm *tcm = tb_priv(tb);
+	struct tb_tunnel *tunnel;
+
+	if (!s2idle_keep_pcie || pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
+		return false;
+
+	list_for_each_entry(tunnel, &tcm->tunnel_list, list) {
+		if (tb_tunnel_is_pci(tunnel))
+			return true;
+	}
+	return false;
+}
+
 static int tb_suspend_noirq(struct tb *tb)
 {
 	struct tb_cm *tcm = tb_priv(tb);
 
+	if (tb_keep_running_s2idle(tb)) {
+		tb_dbg(tb, "PCIe tunnels active, keeping the domain running\n");
+		tb->kept_running = true;
+		return 0;
+	}
+
 	tb_dbg(tb, "suspending...\n");
 	tb_disconnect_and_release_dp(tb);
 	tb_switch_exit_redrive(tb->root_switch);
@@ -3177,6 +3214,12 @@ static int tb_resume_noirq(struct tb *tb)
 	unsigned int usb3_delay = 0;
 	LIST_HEAD(tunnels);
 
+	if (tb->kept_running) {
+		tb->kept_running = false;
+		tb_dbg(tb, "domain kept running, tunnels intact\n");
+		return 0;
+	}
+
 	tb_dbg(tb, "resuming...\n");
 
 	/*
diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h
index 57502da29..8cd7c1b75 100644
--- a/include/linux/thunderbolt.h
+++ b/include/linux/thunderbolt.h
@@ -78,6 +78,8 @@ enum tb_security_level {
  * @index: Linux assigned domain number
  * @security_level: Current security level
  * @nboot_acl: Number of boot ACLs the domain supports
+ * @kept_running: The domain was left running through the current
+ *		  suspend-to-idle, its tunnels intact
  * @privdata: Private connection manager specific data
  */
 struct tb {
@@ -91,6 +93,7 @@ struct tb {
 	int index;
 	enum tb_security_level security_level;
 	size_t nboot_acl;
+	bool kept_running;
 	unsigned long privdata[];
 };
 
-- 
2.56.0

Re: [PATCH] thunderbolt: Optionally keep PCIe tunnels up through suspend-to-idle

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] thunderbolt: active ctl channel and hotplug work during s2idle
  in tb_suspend_noirq()
--

commit a20dd878deb9dda7dcf5deb8740b9a979bcccdaa
Author: Andrei Rusu de Castro <arc@empyreal.works>

thunderbolt: Optionally keep PCIe tunnels up through suspend-to-idle

This commit adds a thunderbolt.s2idle_keep_pcie module parameter to keep
the domain, control channel, and routers running during suspend-to-idle
when a PCIe tunnel is active.

> diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
> index 24611f05b3cda..168a286cd516b 100644
> --- a/drivers/thunderbolt/domain.c
> +++ b/drivers/thunderbolt/domain.c
> @@ -535,7 +535,7 @@ int tb_domain_suspend_noirq(struct tb *tb)
>  	mutex_lock(&tb->lock);
>  	if (tb->cm_ops->suspend_noirq)
>  		ret = tb->cm_ops->suspend_noirq(tb);
> -	if (!ret)
> +	if (!ret && !tb->kept_running)
>  		tb_ctl_stop(tb->ctl);
>  	mutex_unlock(&tb->lock);
>  

[ ... ]

> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> index f54ab2189cf42..c453153c2ef12 100644
> --- a/drivers/thunderbolt/nhi.c
> +++ b/drivers/thunderbolt/nhi.c
> @@ -1145,6 +1156,9 @@ static int nhi_resume_noirq(struct device *dev)
>  	struct tb_nhi *nhi = tb->nhi;
>  	int ret;
>  
> +	if (tb->kept_running)
> +		return tb_domain_resume_noirq(tb);
> +
>  	/*
>  	 * Check that the device is still there. It may be that the user
>  	 * unplugged last device which causes the host controller to go
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index 418ad0a61a5de..641502ec2efa4 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c

[ ... ]

> @@ -3106,10 +3108,45 @@ static int tb_start(struct tb *tb, bool reset)
> +static int tb_suspend_noirq(struct tb *tb)
> +{
> +	struct tb_cm *tcm = tb_priv(tb);
> +
> +	if (tb_keep_running_s2idle(tb)) {
> +		tb_dbg(tb, "PCIe tunnels active, keeping the domain running\n");
> +		tb->kept_running = true;
> +		return 0;
> +	}
> +
>  	tb_dbg(tb, "suspending...\n");
>  	tb_disconnect_and_release_dp(tb);
>  	tb_switch_exit_redrive(tb->root_switch);
> @@ -3177,6 +3214,12 @@ static int tb_resume_noirq(struct tb *tb)
>  	unsigned int usb3_delay = 0;
>  	LIST_HEAD(tunnels);
>  
> +	if (tb->kept_running) {
> +		tb->kept_running = false;
> +		tb_dbg(tb, "domain kept running, tunnels intact\n");
> +		return 0;
> +	}
> +
>  	tb_dbg(tb, "resuming...\n");

[Severity: High]
Can leaving tb->ctl running and tcm->hotplug_active set in
tb_suspend_noirq() cause hotplug and DisplayPort bandwidth work to run
while devices are suspended in the noirq phase?

tb_ctl_alloc() allocates ctl->tx and ctl->rx with Ring_Flag_No_Suspend,
so the control channel interrupt stays enabled across suspend_device_irqs()
without arming wakeup, and tb_domain_alloc() creates tb->wq without
Wq_Freezable.  If a connected monitor enters standby after the GPU
suspends and sends a DisplayPort notification, or a cable is plugged or
unplugged during suspend-to-idle, tb_handle_hotplug() or
tb_handle_dp_bandwidth_request() runs while runtime power management is
disabled on all devices.

In that state, pm_runtime_get_sync() fails with -Eacces, a plug event
calls tb_switch_add() -> device_add() -> device_pm_add() under a prepared
parent (warning and placing unsuspended devices on dpm_list while their
parents are on dpm_noirq_list), and an unplug event tears down tunnels and
unregisters switches while downstream PCI devices are suspended.

In addition, returning early in nhi_resume_noirq() and tb_resume_noirq()
skips the nhi->ops->is_present() check and tb_free_unplugged_children() if
the enclosure was unplugged during sleep.  Should this path set
tcm->hotplug_active = false and stop tb->ctl like tb_freeze_noirq() and
tb_domain_freeze_noirq() do when keeping tunnels up across freeze, and
still check presence and clean up unplugged devices on resume?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-tb-s2idle-keep-pcie-2a2a66a2@empyreal.works?part=1

[PATCH v2] thunderbolt: Optionally keep PCIe tunnels up through suspend-to-idle

[Andrei Rusu de Castro]

Every system sleep puts the USB4 routers to sleep, which takes all
tunnels down, and the connection manager re-creates them on resume. A
PCIe device behind a tunnel disappears from the bus in between. An
external GPU that drives a display does not survive that: on a ROG Flow
Z13 (GZ302EA) with an RTX 5090 in a Razer Core X V2 driving an external
monitor, the GPU driver found the device gone after suspend-to-idle, the
enclosure was removed and re-added under a compositor still using it,
and the desktop froze.

For suspend-to-idle the routers, links and tunnels can instead stay up,
as they already do across freeze. Add a module parameter,
s2idle_keep_pcie, off by default. When it is set, the target state is
suspend-to-idle and the software connection manager has a PCIe tunnel,
handle the domain as freeze does: stop hotplug handling and the control
channel, but leave the routers and tunnels up, and keep the host
interface in D0 by saving its PCI state in the noirq phase, so the PCI
core does not put it into D3. The devices behind the tunnel then go
through an ordinary PCI suspend and resume.

Hotplug events of the sleep are not handled, so on resume check every
router for presence and identity without reconfiguring it, free the
routers and tunnels that went away or were replaced and rescan for new
devices once the system has resumed. The presence check of the host
interface still runs. Without a PCIe tunnel, in other sleep states and
with the firmware connection manager nothing changes.

On a GZ302EA with an RTX 5090 in a Razer Core X V2 and the parameter
set, suspend-to-idle with the enclosure attached resumes with the GPU
still on the bus and working; the enclosure's domain is kept running
and the other domain suspends as before. With the enclosure unplugged
during the sleep, resume reports the router lost, disconnects it and
its retimers, and PCIe hotplug removes the GPU, without warnings or
blocked tasks. The previous version, which kept the control channel
running, was also tested with the eGPU driving the external monitor:
the compositor kept running and the dock's USB devices stayed intact.
With this enclosure attached the system does not reach hardware sleep
either way, with the tunnels kept or torn down (amd-s2idle: 0%
hardware sleep); without it, the same system reaches 99% hardware
sleep at about 1.1 W. The cost of keeping a tunnel up on a system
that otherwise reaches hardware sleep was not measured.

An LLM assisted with finding the cause, writing this change and its
description, and running the tests above on my GZ302EA.

Signed-off-by: Andrei Rusu de Castro <arc@empyreal.works>
---
Changes in v2:
- Handle the kept domain as freeze does: stop hotplug handling and
  the control channel during the sleep (Sashiko review of v1)
- On resume, check every router for presence and identity, free the
  routers and tunnels that went away and rescan in the complete phase
- Run the host interface presence check on resume again

v1: https://lore.kernel.org/all/20261111-tb-s2idle-keep-pcie-2a2a66a2@empyreal.works/

 drivers/thunderbolt/nhi.c   |  13 ++++-
 drivers/thunderbolt/tb.c    | 108 +++++++++++++++++++++++++++++++++++-
 include/linux/thunderbolt.h |   3 +
 3 files changed, 122 insertions(+), 2 deletions(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index f54ab2189..8264f7235 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -1089,6 +1089,17 @@ static int __nhi_suspend_noirq(struct device *dev, bool wakeup)
 	if (ret)
 		return ret;
 
+	if (tb->kept_running) {
+		/*
+		 * Saving the state here keeps the PCI core from putting the
+		 * host router into D3, which would take the tunnels down;
+		 * for suspend-to-idle it then skips the bus PM of this
+		 * device on resume as well.
+		 */
+		pci_save_state(to_pci_dev(dev));
+		return 0;
+	}
+
 	if (nhi->ops->suspend_noirq) {
 		ret = nhi->ops->suspend_noirq(tb->nhi, wakeup);
 		if (ret)
@@ -1152,7 +1163,7 @@ static int nhi_resume_noirq(struct device *dev)
 	 */
 	if ((nhi->ops->is_present && !nhi->ops->is_present(nhi))) {
 		nhi->going_away = true;
-	} else if (nhi->ops->resume_noirq) {
+	} else if (!tb->kept_running && nhi->ops->resume_noirq) {
 		ret = nhi->ops->resume_noirq(nhi);
 		if (ret)
 			return ret;
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 418ad0a61..a7528b93a 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -9,8 +9,10 @@
 #include <linux/slab.h>
 #include <linux/errno.h>
 #include <linux/delay.h>
+#include <linux/moduleparam.h>
 #include <linux/pm_runtime.h>
 #include <linux/stringify.h>
+#include <linux/suspend.h>
 
 #include "tb.h"
 #include "tb_regs.h"
@@ -59,6 +61,9 @@ MODULE_PARM_DESC(asym_threshold,
  *		    after cfg has been paused.
  * @remove_work: Work used to remove any unplugged routers after
  *		 runtime resume
+ * @rescan_on_complete: The domain was kept running through
+ *			suspend-to-idle, so look for routers plugged in
+ *			meanwhile once the system has resumed
  * @groups: Bandwidth groups used in this domain.
  */
 struct tb_cm {
@@ -66,6 +71,7 @@ struct tb_cm {
 	struct list_head dp_resources;
 	bool hotplug_active;
 	struct delayed_work remove_work;
+	bool rescan_on_complete;
 	struct tb_bandwidth_group groups[MAX_GROUPS];
 };
 
@@ -3106,10 +3112,94 @@ static int tb_start(struct tb *tb, bool reset)
 	return 0;
 }
 
+static bool s2idle_keep_pcie;
+module_param(s2idle_keep_pcie, bool, 0644);
+MODULE_PARM_DESC(s2idle_keep_pcie,
+		 "keep a domain with PCIe tunnels running through suspend-to-idle so the devices behind them stay connected (default: false)");
+
+/*
+ * Putting the routers to sleep takes every tunnel down, and the PCIe
+ * devices behind a tunnel then disappear from the bus until it is
+ * re-created on resume. A device that cannot lose its link while its
+ * driver holds it (an external GPU driving a display) does not survive
+ * that. For suspend-to-idle the routers, links and tunnels can instead
+ * stay up, as they do across freeze, and the devices behind them go
+ * through an ordinary PCI suspend and resume.
+ */
+static bool tb_keep_running_s2idle(struct tb *tb)
+{
+	struct tb_cm *tcm = tb_priv(tb);
+	struct tb_tunnel *tunnel;
+
+	if (!s2idle_keep_pcie || pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
+		return false;
+
+	list_for_each_entry(tunnel, &tcm->tunnel_list, list) {
+		if (tb_tunnel_is_pci(tunnel))
+			return true;
+	}
+	return false;
+}
+
+/*
+ * Hotplug events were not handled while the domain was kept running
+ * through suspend-to-idle. Mark the routers and XDomains that went away
+ * or were replaced meanwhile as unplugged, without reconfiguring those
+ * still present, whose tunnels are in use.
+ */
+static void tb_check_kept_running(struct tb_switch *sw)
+{
+	struct tb_port *port;
+
+	tb_switch_for_each_port(sw, port) {
+		struct tb_switch *child;
+		u64 uid;
+		int ret;
+
+		if (!tb_port_is_null(port))
+			continue;
+
+		if (port->xdomain) {
+			if (tb_wait_for_port(port, false) <= 0)
+				port->xdomain->is_unplugged = true;
+			continue;
+		}
+
+		if (!tb_port_has_remote(port))
+			continue;
+
+		child = port->remote->sw;
+		ret = tb_cfg_get_upstream_port(sw->tb->ctl, tb_route(child));
+		if (ret >= 0 && child->uid) {
+			if (tb_switch_is_usb4(child))
+				ret = usb4_switch_read_uid(child, &uid);
+			else
+				ret = tb_drom_read_uid_only(child, &uid);
+			if (!ret && uid != child->uid)
+				ret = -ENODEV;
+		}
+		if (ret < 0) {
+			tb_port_warn(port, "lost during suspend, disconnecting\n");
+			tb_sw_set_unplugged(child);
+			continue;
+		}
+
+		tb_check_kept_running(child);
+	}
+}
+
 static int tb_suspend_noirq(struct tb *tb)
 {
 	struct tb_cm *tcm = tb_priv(tb);
 
+	if (tb_keep_running_s2idle(tb)) {
+		tb_dbg(tb, "PCIe tunnels active, keeping the domain running\n");
+		/* As for freeze, no hotplug handling until resume */
+		tcm->hotplug_active = false;
+		tb->kept_running = true;
+		return 0;
+	}
+
 	tb_dbg(tb, "suspending...\n");
 	tb_disconnect_and_release_dp(tb);
 	tb_switch_exit_redrive(tb->root_switch);
@@ -3177,6 +3267,17 @@ static int tb_resume_noirq(struct tb *tb)
 	unsigned int usb3_delay = 0;
 	LIST_HEAD(tunnels);
 
+	if (tb->kept_running) {
+		tb->kept_running = false;
+		tb_dbg(tb, "domain kept running, checking for changes\n");
+		tb_check_kept_running(tb->root_switch);
+		tb_free_invalid_tunnels(tb);
+		tb_free_unplugged_children(tb->root_switch);
+		tcm->rescan_on_complete = true;
+		tcm->hotplug_active = true;
+		return 0;
+	}
+
 	tb_dbg(tb, "resuming...\n");
 
 	/*
@@ -3250,12 +3351,17 @@ static int tb_thaw_noirq(struct tb *tb)
 
 static void tb_complete(struct tb *tb)
 {
+	struct tb_cm *tcm = tb_priv(tb);
+	bool rescan = tcm->rescan_on_complete;
+
+	tcm->rescan_on_complete = false;
+
 	/*
 	 * Unregister unplugged XDomains and if there is a case where
 	 * another domain is swapped in place of unplugged XDomain we
 	 * need to run another rescan.
 	 */
-	if (tb_domain_unregister_unplugged_xdomains(tb)) {
+	if (tb_domain_unregister_unplugged_xdomains(tb) || rescan) {
 		scoped_guard(mutex, &tb->lock)
 			tb_scan_switch(tb->root_switch);
 	}
diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h
index 57502da29..8cd7c1b75 100644
--- a/include/linux/thunderbolt.h
+++ b/include/linux/thunderbolt.h
@@ -78,6 +78,8 @@ enum tb_security_level {
  * @index: Linux assigned domain number
  * @security_level: Current security level
  * @nboot_acl: Number of boot ACLs the domain supports
+ * @kept_running: The domain was left running through the current
+ *		  suspend-to-idle, its tunnels intact
  * @privdata: Private connection manager specific data
  */
 struct tb {
@@ -91,6 +93,7 @@ struct tb {
 	int index;
 	enum tb_security_level security_level;
 	size_t nboot_acl;
+	bool kept_running;
 	unsigned long privdata[];
 };
 
-- 
2.56.0

Re: [PATCH v2] thunderbolt: Optionally keep PCIe tunnels up through suspend-to-idle

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential
issues to consider:
- [High] thunderbolt: missing tb_free_unplugged_xdomains() in
  tb_resume_noirq()
- [Medium] thunderbolt: incomplete XDomain replacement and child rescan
  on s2idle resume
- [Medium] thunderbolt: incomplete topology rescan in tb_complete() via
  tb_scan_port()
- [Medium] thunderbolt: missing router enumeration and UID checks in
  tb_check_kept_running()
- [Medium] thunderbolt: unquiesced delayed works access stopped ctl in
  tb_suspend_noirq()
- [Medium] thunderbolt: stale DP tunnels after monitor unplug during
  kept-running s2idle
--

commit a1b4dee4a46d2ea714a73ee4360b3adbd9dd4c3f
Author: Andrei Rusu de Castro <arc@empyreal.works>

thunderbolt: Optionally keep PCIe tunnels up through suspend-to-idle

This commit adds an s2idle_keep_pcie module parameter to keep USB4 routers
and tunnels up across suspend-to-idle when a PCIe tunnel is active, while
stopping hotplug handling and the control channel during sleep and checking
for topology changes on resume.

> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index 418ad0a61a5de..a7528b93a7037 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
[ ... ]
> @@ -3106,10 +3112,94 @@ static int tb_start(struct tb *tb, bool reset)
[ ... ]
> +static void tb_check_kept_running(struct tb_switch *sw)
> +{
> +	struct tb_port *port;
> +
> +	tb_switch_for_each_port(sw, port) {
> +		struct tb_switch *child;
> +		u64 uid;
> +		int ret;
> +
> +		if (!tb_port_is_null(port))
> +			continue;
> +
> +		if (port->xdomain) {
> +			if (tb_wait_for_port(port, false) <= 0)
> +				port->xdomain->is_unplugged = true;
> +			continue;
> +		}

[Severity: Medium]
What happens if an XDomain is unplugged during suspend-to-idle and
replaced by a router on the same port?

Because the link is up, tb_wait_for_port() returns 1 and
port->xdomain->is_unplugged stays false.  Unlike tb_switch_resume(),
tb_check_kept_running() does not unlock the port and probe
tb_cfg_get_upstream_port(sw->tb->ctl, port->xdomain->route) to detect that
a router is now at that route.

When tb_complete() then runs, tb_domain_unregister_unplugged_xdomains()
does not unregister port->xdomain, and tb_scan_port() calls
tb_xdomain_remove() (which leaves a registered xd->dev on tb_bus_type) and
clears port->xdomain, leaking xd->dev on the bus and causing
tb_switch_add() for the new router to fail with -EEXIST due to the
colliding device name.

Also, in tb_complete(), tb_scan_port() returns early whenever port->remote
is already set without recursing into port->remote->sw, so replacements or
newly plugged devices on surviving child switches are skipped.

> +		if (!tb_port_has_remote(port))
> +			continue;
> +
> +		child = port->remote->sw;
> +		ret = tb_cfg_get_upstream_port(sw->tb->ctl, tb_route(child));
> +		if (ret >= 0 && child->uid) {
> +			if (tb_switch_is_usb4(child))
> +				ret = usb4_switch_read_uid(child, &uid);
> +			else
> +				ret = tb_drom_read_uid_only(child, &uid);
> +			if (!ret && uid != child->uid)
> +				ret = -ENODEV;
> +		}

[Severity: Medium]
Can a child router that power-cycled or reset in place during
suspend-to-idle pass this check while unenumerated?

ROUTER_CS_0 (read by tb_cfg_get_upstream_port()) and the UID registers or
DROM remain readable after a router reset even though ROUTER_CS_3_V
(checked by tb_switch_enumerated()) was cleared and all hardware HopID
tunnel entries were wiped.  Because tb_resume_noirq() skips
tb_switch_configure() and tb_tunnel_activate() when tb->kept_running is
set, keeping a router whose ROUTER_CS_3_V bit is cleared leaves its
tunnels dead in hardware.

In addition, if child->uid is 0 (for example in safe mode), the
child->uid check skips UID verification altogether (whereas
tb_switch_resume() rejects !sw->uid with -ENODEV), and the upstream port
returned by tb_cfg_get_upstream_port() is not compared against
child->config.upstream_port_number.

> +		if (ret < 0) {
> +			tb_port_warn(port, "lost during suspend, disconnecting\n");
> +			tb_sw_set_unplugged(child);
> +			continue;
> +		}
> +
> +		tb_check_kept_running(child);
> +	}
> +}
> +
>  static int tb_suspend_noirq(struct tb *tb)
>  {
>  	struct tb_cm *tcm = tb_priv(tb);
>  
> +	if (tb_keep_running_s2idle(tb)) {
> +		tb_dbg(tb, "PCIe tunnels active, keeping the domain running\n");
> +		/* As for freeze, no hotplug handling until resume */
> +		tcm->hotplug_active = false;
> +		tb->kept_running = true;
> +		return 0;
> +	}

[Severity: Medium]
Can delayed work items access tb->ctl while it is stopped during
suspend-to-idle?

Skipping tb_disconnect_and_release_dp() leaves DP tunnels and bandwidth
groups active while tb_domain_suspend_noirq() stops tb->ctl.  Because
group->release_work (on system_percpu_wq) and tunnel->dprx_work or
tcm->remove_work (on tb->wq, which is allocated without WQ_FREEZABLE) are
neither canceled nor guarded by tcm->hotplug_active, they can run during
suspend-to-idle and issue config reads/writes on the stopped control
channel, triggering WARN() in tb_cfg_read() and tb_cfg_write() on
-ENOTCONN and leaving software state out of sync with hardware.

[Severity: Medium]
What happens if a DisplayPort monitor on a surviving router is unplugged
during suspend-to-idle while tb->kept_running is set?

Because tb_disconnect_and_release_dp() is skipped and hotplug events are
ignored while asleep, the DP tunnel stays active.  On resume,
tb_check_kept_running() skips non-null ports, and tb_scan_port() only
checks DP OUT ports where !tb_dp_port_is_enabled(port), so lost HPD on an
enabled DP OUT port is never handled and the stale DP tunnel and DP IN
resource remain allocated.

[ ... ]
> @@ -3177,6 +3267,17 @@ static int tb_resume_noirq(struct tb *tb)
>  	unsigned int usb3_delay = 0;
>  	LIST_HEAD(tunnels);
>  
> +	if (tb->kept_running) {
> +		tb->kept_running = false;
> +		tb_dbg(tb, "domain kept running, checking for changes\n");
> +		tb_check_kept_running(tb->root_switch);
> +		tb_free_invalid_tunnels(tb);
> +		tb_free_unplugged_children(tb->root_switch);
> +		tcm->rescan_on_complete = true;
> +		tcm->hotplug_active = true;
> +		return 0;
> +	}

[Severity: High]
Does this miss a call to tb_free_unplugged_xdomains(tb->root_switch)?

If an XDomain attached to tb->root_switch or a surviving child switch is
unplugged during suspend-to-idle, tb_check_kept_running() sets
port->xdomain->is_unplugged = true, but tb_free_unplugged_children() only
processes ports with tb_port_has_remote(port).  Without
tb_free_unplugged_xdomains(), tb_xdomain_remove() is never called and
port->xdomain is not cleared.

During dpm_resume, tb_xdomain_resume() then restarts xd->state_work, and
tb_complete() -> tb_domain_unregister_unplugged_xdomains() unregisters and
frees the struct tb_xdomain while xd->state_work is still queued and
port->xdomain still points to the freed object, causing a use-after-free
(or leaking xd if xd->dev was not yet registered on tb_bus_type).

[ ... ]
> @@ -3250,12 +3351,17 @@ static int tb_thaw_noirq(struct tb *tb)
>  
>  static void tb_complete(struct tb *tb)
>  {
> +	struct tb_cm *tcm = tb_priv(tb);
> +	bool rescan = tcm->rescan_on_complete;
> +
> +	tcm->rescan_on_complete = false;
> +
>  	/*
>  	 * Unregister unplugged XDomains and if there is a case where
>  	 * another domain is swapped in place of unplugged XDomain we
>  	 * need to run another rescan.
>  	 */
> -	if (tb_domain_unregister_unplugged_xdomains(tb)) {
> +	if (tb_domain_unregister_unplugged_xdomains(tb) || rescan) {
>  		scoped_guard(mutex, &tb->lock)
>  			tb_scan_switch(tb->root_switch);
>  	}

[Severity: Medium]
Does tb_scan_switch(tb->root_switch) rescan downstream ports of child
routers that stayed up during suspend-to-idle?

In tb_scan_port(), any port that already has port->remote set returns
early without recursing into port->remote->sw:

drivers/thunderbolt/tb.c:tb_scan_port() {
    ...
	if (port->remote) {
		tb_port_dbg(port, "port already has a remote\n");
		goto out_rpm_put;
	}
    ...
}

Because s2idle_keep_pcie only triggers when a downstream router with a
PCIe tunnel is present on tb->root_switch, port->remote is non-NULL for
every surviving child router, so devices or monitors plugged into or
replaced behind a kept-running dock or enclosure during sleep are never
scanned on resume.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-tb-s2idle-keep-pcie-v2-b3c197ac@empyreal.works?part=1

Re: [PATCH] thunderbolt: Optionally keep PCIe tunnels up through suspend-to-idle

[Mika Westerberg]

Hi,

On Wed, Nov 11, 2026 at 10:57:08PM +0000, Andrei Rusu de Castro wrote:
> Every system sleep puts the USB4 routers to sleep, which takes all
> tunnels down, and the connection manager re-creates them on resume. A
> PCIe device behind a tunnel disappears from the bus in between. An
> external GPU that drives a display does not survive that: on a ROG Flow
> Z13 (GZ302EA) with an RTX 5090 in a Razer Core X V2 driving an external
> monitor, the GPU driver found the device gone after suspend-to-idle, the
> enclosure was removed and re-added under a compositor still using it,
> and the desktop froze.

It is supposed to work so that the tunnels get re-established before the
native stacks are resumed, for that we have the device links. In you case
it may be that they are missing.

> For suspend-to-idle the domain can keep running instead. Add a module
> parameter, s2idle_keep_pcie, off by default. When it is set, the target
> state is suspend-to-idle and the software connection manager has a PCIe
> tunnel, leave the routers, links and control channel running and keep
> the host interface in D0 by saving its PCI state in the noirq phase, so
> the PCI core does not put it into D3. The devices behind the tunnel then
> go through an ordinary PCI suspend and resume. Without a PCIe tunnel, in
> other sleep states and with the firmware connection manager nothing
> changes.

I don't think this is a good idea to be honest. We should really understand
why the PCIe link is not coming back as it should be and for that it would
be good if you can share full dmesg with thunderbolt.dyndbg=+p in the
kernel command line.