* [PATCH v2] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
@ 2026-08-25 21:42 juan.martinez
2026-08-26 3:02 ` Mario Limonciello
2026-08-27 21:57 ` [PATCH v3] " Juan Martinez
0 siblings, 2 replies; 13+ messages in thread
From: juan.martinez @ 2026-08-25 21:42 UTC (permalink / raw)
To: westeri
Cc: andreas.noever, YehezkelShB, Basavaraj.Natikar, Sanath.S,
linux-usb, linux-kernel, mario.limonciello
From: Juan Martinez <juan.martinez@amd.com>
Commit f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on
DMA path teardown for AMD USB4 routers") introduced a deadlock when
physically unplugging a Thunderbolt cable on AMD systems.
The problem occurs because tb_handle_hotplug() holds tb->lock while
processing the unplug event. When it removes the XDomain services,
tbnet_remove() calls tb_xdomain_disable_paths() which eventually calls
tb_domain_reset_interface(). That function tries to acquire tb->lock
via guard(mutex), but the hotplug worker already holds it, causing a
self-deadlock.
The deadlock manifests as a complete network hang because
tb_handle_hotplug() holds RTNL while waiting on its own mutex, blocking
all network operations system-wide.
The existing code already handles this scenario partially: when
xd->is_unplugged is true, tb_disconnect_xdomain_paths() intentionally
skips the DMA teardown because the hotplug handler will do it later
via __tb_disconnect_xdomain_paths(). However, the reset was still
being called unconditionally.
Fix this by:
1. Splitting tb_domain_reset_interface() into a locked inner function
__tb_domain_reset_interface_locked() and a locking wrapper
2. Skipping the reset in tb_domain_disconnect_xdomain_paths() when
xd->is_unplugged is true (matching the existing teardown skip logic)
3. Calling __tb_domain_reset_interface_locked() from tb_handle_hotplug()
after __tb_disconnect_xdomain_paths() where the actual DMA teardown
happens and tb->lock is already held
This preserves the reset behavior for normal shutdown paths while
avoiding the deadlock during physical cable unplug.
Fixes: f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers")
Signed-off-by: Juan Martinez <juan.martinez@amd.com>
---
drivers/thunderbolt/domain.c | 26 +++++++++++++++++++++-----
drivers/thunderbolt/tb.c | 1 +
drivers/thunderbolt/tb.h | 1 +
3 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index 12c88509a54f..253ea8c6b757 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -788,14 +788,19 @@ int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
transmit_ring, receive_path, receive_ring);
}
-static void tb_domain_reset_interface(struct tb *tb)
+/*
+ * __tb_domain_reset_interface_locked - Reset host interface (lock held)
+ *
+ * Caller must hold tb->lock. Used by hotplug path where lock is already held.
+ */
+void __tb_domain_reset_interface_locked(struct tb *tb)
{
struct tb_nhi *nhi = tb->nhi;
- if (!nhi->ops->reset_interface)
- return;
+ lockdep_assert_held(&tb->lock);
- guard(mutex)(&tb->lock);
+ if (!(nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN))
+ return;
/* The reset clears the ring state so stop the control channel */
tb_ctl_stop(tb->ctl);
@@ -803,6 +808,17 @@ static void tb_domain_reset_interface(struct tb *tb)
tb_ctl_start(tb->ctl);
}
+static void tb_domain_reset_interface(struct tb *tb)
+{
+ struct tb_nhi *nhi = tb->nhi;
+
+ if (!nhi->ops->reset_interface)
+ return;
+
+ guard(mutex)(&tb->lock);
+ __tb_domain_reset_interface_locked(tb);
+}
+
/**
* tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
* @tb: Domain disabling the DMA paths
@@ -835,7 +851,7 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
if (ret)
return ret;
- if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
+ if (!xd->is_unplugged)
tb_domain_reset_interface(tb);
return 0;
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index b7cc6894a598..89dfb3381345 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -2489,6 +2489,7 @@ static void tb_handle_hotplug(struct work_struct *work)
tb_xdomain_remove(xd);
port->xdomain = NULL;
__tb_disconnect_xdomain_paths(tb, xd, -1, -1, -1, -1);
+ __tb_domain_reset_interface_locked(tb);
tb_xdomain_put(xd);
tb_port_unconfigure_xdomain(port);
} else if (tb_port_is_dpout(port) || tb_port_is_dpin(port)) {
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 4373336d9425..2e6e0920cb1f 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -789,6 +789,7 @@ int tb_domain_disconnect_pcie_paths(struct tb *tb);
int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
int transmit_path, int transmit_ring,
int receive_path, int receive_ring);
+void __tb_domain_reset_interface_locked(struct tb *tb);
int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
int transmit_path, int transmit_ring,
int receive_path, int receive_ring);
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
2026-08-25 21:42 [PATCH v2] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers juan.martinez
@ 2026-08-26 3:02 ` Mario Limonciello
2026-08-27 21:57 ` [PATCH v3] " Juan Martinez
1 sibling, 0 replies; 13+ messages in thread
From: Mario Limonciello @ 2026-08-26 3:02 UTC (permalink / raw)
To: juan.martinez, westeri
Cc: andreas.noever, YehezkelShB, Basavaraj.Natikar, Sanath.S,
linux-usb, linux-kernel
On 8/25/26 16:42, juan.martinez@amd.com wrote:
> From: Juan Martinez <juan.martinez@amd.com>
>
> Commit f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on
> DMA path teardown for AMD USB4 routers") introduced a deadlock when
> physically unplugging a Thunderbolt cable on AMD systems.
>
> The problem occurs because tb_handle_hotplug() holds tb->lock while
> processing the unplug event. When it removes the XDomain services,
> tbnet_remove() calls tb_xdomain_disable_paths() which eventually calls
> tb_domain_reset_interface(). That function tries to acquire tb->lock
> via guard(mutex), but the hotplug worker already holds it, causing a
> self-deadlock.
>
> The deadlock manifests as a complete network hang because
> tb_handle_hotplug() holds RTNL while waiting on its own mutex, blocking
> all network operations system-wide.
>
> The existing code already handles this scenario partially: when
> xd->is_unplugged is true, tb_disconnect_xdomain_paths() intentionally
> skips the DMA teardown because the hotplug handler will do it later
> via __tb_disconnect_xdomain_paths(). However, the reset was still
> being called unconditionally.
>
> Fix this by:
> 1. Splitting tb_domain_reset_interface() into a locked inner function
> __tb_domain_reset_interface_locked() and a locking wrapper
> 2. Skipping the reset in tb_domain_disconnect_xdomain_paths() when
> xd->is_unplugged is true (matching the existing teardown skip logic)
> 3. Calling __tb_domain_reset_interface_locked() from tb_handle_hotplug()
> after __tb_disconnect_xdomain_paths() where the actual DMA teardown
> happens and tb->lock is already held
>
> This preserves the reset behavior for normal shutdown paths while
> avoiding the deadlock during physical cable unplug.
>
> Fixes: f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers")
> Signed-off-by: Juan Martinez <juan.martinez@amd.com>
> ---
> drivers/thunderbolt/domain.c | 26 +++++++++++++++++++++-----
> drivers/thunderbolt/tb.c | 1 +
> drivers/thunderbolt/tb.h | 1 +
> 3 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
> index 12c88509a54f..253ea8c6b757 100644
> --- a/drivers/thunderbolt/domain.c
> +++ b/drivers/thunderbolt/domain.c
> @@ -788,14 +788,19 @@ int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> transmit_ring, receive_path, receive_ring);
> }
>
> -static void tb_domain_reset_interface(struct tb *tb)
> +/*
> + * __tb_domain_reset_interface_locked - Reset host interface (lock held)
> + *
> + * Caller must hold tb->lock. Used by hotplug path where lock is already held.
> + */
> +void __tb_domain_reset_interface_locked(struct tb *tb)
> {
> struct tb_nhi *nhi = tb->nhi;
>
> - if (!nhi->ops->reset_interface)
> - return;
I'm not sure this is correct to move from here to
tb_domain_reset_interface() because you still call
__tb_domain_reset_interface_locked() from tb_handle_hotplug() which
doesn't do this check.
> + lockdep_assert_held(&tb->lock);
>
> - guard(mutex)(&tb->lock);
> + if (!(nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN))
> + return;
>
> /* The reset clears the ring state so stop the control channel */
> tb_ctl_stop(tb->ctl);
> @@ -803,6 +808,17 @@ static void tb_domain_reset_interface(struct tb *tb)
> tb_ctl_start(tb->ctl);
> }
>
> +static void tb_domain_reset_interface(struct tb *tb)
> +{
> + struct tb_nhi *nhi = tb->nhi;
> +
> + if (!nhi->ops->reset_interface)
> + return;
> +
> + guard(mutex)(&tb->lock);
> + __tb_domain_reset_interface_locked(tb);
> +}
> +
> /**
> * tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
> * @tb: Domain disabling the DMA paths
> @@ -835,7 +851,7 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> if (ret)
> return ret;
>
> - if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
> + if (!xd->is_unplugged)
> tb_domain_reset_interface(tb);
>
> return 0;
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index b7cc6894a598..89dfb3381345 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -2489,6 +2489,7 @@ static void tb_handle_hotplug(struct work_struct *work)
> tb_xdomain_remove(xd);
> port->xdomain = NULL;
> __tb_disconnect_xdomain_paths(tb, xd, -1, -1, -1, -1);
> + __tb_domain_reset_interface_locked(tb);
> tb_xdomain_put(xd);
> tb_port_unconfigure_xdomain(port);
> } else if (tb_port_is_dpout(port) || tb_port_is_dpin(port)) {
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 4373336d9425..2e6e0920cb1f 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -789,6 +789,7 @@ int tb_domain_disconnect_pcie_paths(struct tb *tb);
> int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> int transmit_path, int transmit_ring,
> int receive_path, int receive_ring);
> +void __tb_domain_reset_interface_locked(struct tb *tb);
> int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> int transmit_path, int transmit_ring,
> int receive_path, int receive_ring);
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
2026-08-25 21:42 [PATCH v2] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers juan.martinez
2026-08-26 3:02 ` Mario Limonciello
@ 2026-08-27 21:57 ` Juan Martinez
2026-08-28 4:43 ` Mario Limonciello
2026-08-28 5:19 ` [PATCH v4] " Juan Martinez
1 sibling, 2 replies; 13+ messages in thread
From: Juan Martinez @ 2026-08-27 21:57 UTC (permalink / raw)
To: westeri
Cc: mario.limonciello, andreas.noever, YehezkelShB, Basavaraj.Natikar,
Sanath.S, linux-usb, linux-kernel, Juan Martinez
Commit f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on
DMA path teardown for AMD USB4 routers") introduced a deadlock when
physically unplugging a Thunderbolt cable on AMD systems.
The problem occurs because tb_handle_hotplug() holds tb->lock while
processing the unplug event. When it removes the XDomain services,
tbnet_remove() calls tb_xdomain_disable_paths() which eventually calls
tb_domain_reset_interface(). That function tries to acquire tb->lock
via guard(mutex), but the hotplug worker already holds it, causing a
self-deadlock.
The deadlock manifests as a complete network hang because
tb_handle_hotplug() holds RTNL while waiting on its own mutex, blocking
all network operations system-wide.
The existing code already handles this scenario partially: when
xd->is_unplugged is true, tb_disconnect_xdomain_paths() intentionally
skips the DMA teardown because the hotplug handler will do it later
via __tb_disconnect_xdomain_paths(). However, the reset was still
being called unconditionally.
Fix this by:
1. Splitting tb_domain_reset_interface() into a locked inner function
__tb_domain_reset_interface_locked() and a locking wrapper
2. Skipping the reset in tb_domain_disconnect_xdomain_paths() when
xd->is_unplugged is true (matching the existing teardown skip logic)
3. Calling __tb_domain_reset_interface_locked() from tb_handle_hotplug()
after __tb_disconnect_xdomain_paths() where the actual DMA teardown
happens and tb->lock is already held
This preserves the reset behavior for normal shutdown paths while
avoiding the deadlock during physical cable unplug.
Fixes: f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers")
Signed-off-by: Juan Martinez <juan.martinez@amd.com>
---
drivers/thunderbolt/domain.c | 26 +++++++++++++++++++++-----
drivers/thunderbolt/tb.c | 1 +
drivers/thunderbolt/tb.h | 1 +
3 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index 12c88509a54f..8f33baafe9ae 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -788,14 +788,19 @@ int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
transmit_ring, receive_path, receive_ring);
}
-static void tb_domain_reset_interface(struct tb *tb)
+/*
+ * __tb_domain_reset_interface_locked - Reset host interface (lock held)
+ *
+ * Caller must hold tb->lock. Used by hotplug path where lock is already held.
+ */
+void __tb_domain_reset_interface_locked(struct tb *tb)
{
struct tb_nhi *nhi = tb->nhi;
- if (!nhi->ops->reset_interface)
- return;
+ lockdep_assert_held(&tb->lock);
- guard(mutex)(&tb->lock);
+ if (!nhi->ops->reset_interface || !(nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN))
+ return;
/* The reset clears the ring state so stop the control channel */
tb_ctl_stop(tb->ctl);
@@ -803,6 +808,17 @@ static void tb_domain_reset_interface(struct tb *tb)
tb_ctl_start(tb->ctl);
}
+static void tb_domain_reset_interface(struct tb *tb)
+{
+ struct tb_nhi *nhi = tb->nhi;
+
+ if (!nhi->ops->reset_interface)
+ return;
+
+ guard(mutex)(&tb->lock);
+ __tb_domain_reset_interface_locked(tb);
+}
+
/**
* tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
* @tb: Domain disabling the DMA paths
@@ -835,7 +851,7 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
if (ret)
return ret;
- if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
+ if (!xd->is_unplugged)
tb_domain_reset_interface(tb);
return 0;
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index b7cc6894a598..89dfb3381345 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -2489,6 +2489,7 @@ static void tb_handle_hotplug(struct work_struct *work)
tb_xdomain_remove(xd);
port->xdomain = NULL;
__tb_disconnect_xdomain_paths(tb, xd, -1, -1, -1, -1);
+ __tb_domain_reset_interface_locked(tb);
tb_xdomain_put(xd);
tb_port_unconfigure_xdomain(port);
} else if (tb_port_is_dpout(port) || tb_port_is_dpin(port)) {
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 4373336d9425..2e6e0920cb1f 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -789,6 +789,7 @@ int tb_domain_disconnect_pcie_paths(struct tb *tb);
int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
int transmit_path, int transmit_ring,
int receive_path, int receive_ring);
+void __tb_domain_reset_interface_locked(struct tb *tb);
int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
int transmit_path, int transmit_ring,
int receive_path, int receive_ring);
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
2026-08-27 21:57 ` [PATCH v3] " Juan Martinez
@ 2026-08-28 4:43 ` Mario Limonciello
2026-08-28 5:19 ` [PATCH v4] " Juan Martinez
1 sibling, 0 replies; 13+ messages in thread
From: Mario Limonciello @ 2026-08-28 4:43 UTC (permalink / raw)
To: Juan Martinez, westeri
Cc: andreas.noever, YehezkelShB, Basavaraj.Natikar, Sanath.S,
linux-usb, linux-kernel
On 8/27/26 16:57, Juan Martinez wrote:
> Commit f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on
> DMA path teardown for AMD USB4 routers") introduced a deadlock when
> physically unplugging a Thunderbolt cable on AMD systems.
>
> The problem occurs because tb_handle_hotplug() holds tb->lock while
> processing the unplug event. When it removes the XDomain services,
> tbnet_remove() calls tb_xdomain_disable_paths() which eventually calls
> tb_domain_reset_interface(). That function tries to acquire tb->lock
> via guard(mutex), but the hotplug worker already holds it, causing a
> self-deadlock.
>
> The deadlock manifests as a complete network hang because
> tb_handle_hotplug() holds RTNL while waiting on its own mutex, blocking
> all network operations system-wide.
>
> The existing code already handles this scenario partially: when
> xd->is_unplugged is true, tb_disconnect_xdomain_paths() intentionally
> skips the DMA teardown because the hotplug handler will do it later
> via __tb_disconnect_xdomain_paths(). However, the reset was still
> being called unconditionally.
>
> Fix this by:
> 1. Splitting tb_domain_reset_interface() into a locked inner function
> __tb_domain_reset_interface_locked() and a locking wrapper
> 2. Skipping the reset in tb_domain_disconnect_xdomain_paths() when
> xd->is_unplugged is true (matching the existing teardown skip logic)
> 3. Calling __tb_domain_reset_interface_locked() from tb_handle_hotplug()
> after __tb_disconnect_xdomain_paths() where the actual DMA teardown
> happens and tb->lock is already held
>
> This preserves the reset behavior for normal shutdown paths while
> avoiding the deadlock during physical cable unplug.
>
> Fixes: f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers")
> Signed-off-by: Juan Martinez <juan.martinez@amd.com>
> ---
> drivers/thunderbolt/domain.c | 26 +++++++++++++++++++++-----
> drivers/thunderbolt/tb.c | 1 +
> drivers/thunderbolt/tb.h | 1 +
> 3 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
> index 12c88509a54f..8f33baafe9ae 100644
> --- a/drivers/thunderbolt/domain.c
> +++ b/drivers/thunderbolt/domain.c
> @@ -788,14 +788,19 @@ int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> transmit_ring, receive_path, receive_ring);
> }
>
> -static void tb_domain_reset_interface(struct tb *tb)
> +/*
> + * __tb_domain_reset_interface_locked - Reset host interface (lock held)
> + *
> + * Caller must hold tb->lock. Used by hotplug path where lock is already held.
> + */
> +void __tb_domain_reset_interface_locked(struct tb *tb)
> {
> struct tb_nhi *nhi = tb->nhi;
>
> - if (!nhi->ops->reset_interface)
> - return;
> + lockdep_assert_held(&tb->lock);
>
> - guard(mutex)(&tb->lock);
> + if (!nhi->ops->reset_interface || !(nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN))
> + return;
>
> /* The reset clears the ring state so stop the control channel */
> tb_ctl_stop(tb->ctl);
> @@ -803,6 +808,17 @@ static void tb_domain_reset_interface(struct tb *tb)
> tb_ctl_start(tb->ctl);
> }
>
> +static void tb_domain_reset_interface(struct tb *tb)
> +{
> + struct tb_nhi *nhi = tb->nhi;
> +
> + if (!nhi->ops->reset_interface)
> + return;
Why is this check here? You already have the same check in
__tb_domain_reset_interface_locked().
> +
> + guard(mutex)(&tb->lock);
> + __tb_domain_reset_interface_locked(tb);
> +}
> +
> /**
> * tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
> * @tb: Domain disabling the DMA paths
> @@ -835,7 +851,7 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> if (ret)
> return ret;
>
> - if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
> + if (!xd->is_unplugged)
> tb_domain_reset_interface(tb);
>
> return 0;
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index b7cc6894a598..89dfb3381345 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -2489,6 +2489,7 @@ static void tb_handle_hotplug(struct work_struct *work)
> tb_xdomain_remove(xd);
> port->xdomain = NULL;
> __tb_disconnect_xdomain_paths(tb, xd, -1, -1, -1, -1);
> + __tb_domain_reset_interface_locked(tb);
> tb_xdomain_put(xd);
> tb_port_unconfigure_xdomain(port);
> } else if (tb_port_is_dpout(port) || tb_port_is_dpin(port)) {
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 4373336d9425..2e6e0920cb1f 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -789,6 +789,7 @@ int tb_domain_disconnect_pcie_paths(struct tb *tb);
> int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> int transmit_path, int transmit_ring,
> int receive_path, int receive_ring);
> +void __tb_domain_reset_interface_locked(struct tb *tb);
> int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> int transmit_path, int transmit_ring,
> int receive_path, int receive_ring);
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
2026-08-27 21:57 ` [PATCH v3] " Juan Martinez
2026-08-28 4:43 ` Mario Limonciello
@ 2026-08-28 5:19 ` Juan Martinez
2026-08-28 14:58 ` Mario Limonciello
2026-08-31 11:11 ` Mika Westerberg
1 sibling, 2 replies; 13+ messages in thread
From: Juan Martinez @ 2026-08-28 5:19 UTC (permalink / raw)
To: westeri
Cc: mario.limonciello, andreas.noever, YehezkelShB, Basavaraj.Natikar,
Sanath.S, linux-usb, linux-kernel, Juan Martinez
Commit f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on
DMA path teardown for AMD USB4 routers") introduced a deadlock when
physically unplugging a Thunderbolt cable on AMD systems.
The problem occurs because tb_handle_hotplug() holds tb->lock while
processing the unplug event. When it removes the XDomain services,
tbnet_remove() calls tb_xdomain_disable_paths() which eventually calls
tb_domain_reset_interface(). That function tries to acquire tb->lock
via guard(mutex), but the hotplug worker already holds it, causing a
self-deadlock.
The deadlock manifests as a complete network hang because
tb_handle_hotplug() holds RTNL while waiting on its own mutex, blocking
all network operations system-wide.
The existing code already handles this scenario partially: when
xd->is_unplugged is true, tb_disconnect_xdomain_paths() intentionally
skips the DMA teardown because the hotplug handler will do it later
via __tb_disconnect_xdomain_paths(). However, the reset was still
being called unconditionally.
Fix this by:
1. Splitting tb_domain_reset_interface() into a locked inner function
__tb_domain_reset_interface_locked() and a locking wrapper
2. Skipping the reset in tb_domain_disconnect_xdomain_paths() when
xd->is_unplugged is true (matching the existing teardown skip logic)
3. Calling __tb_domain_reset_interface_locked() from tb_handle_hotplug()
after __tb_disconnect_xdomain_paths() where the actual DMA teardown
happens and tb->lock is already held
This preserves the reset behavior for normal shutdown paths while
avoiding the deadlock during physical cable unplug.
Fixes: f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers")
Signed-off-by: Juan Martinez <juan.martinez@amd.com>
---
drivers/thunderbolt/domain.c | 20 +++++++++++++++++---
drivers/thunderbolt/tb.c | 1 +
drivers/thunderbolt/tb.h | 1 +
3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index 12c88509a54f..4cef9f4de523 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -788,14 +788,22 @@ int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
transmit_ring, receive_path, receive_ring);
}
-static void tb_domain_reset_interface(struct tb *tb)
+/*
+ * __tb_domain_reset_interface_locked - Reset host interface (lock held)
+ *
+ * Caller must hold tb->lock. Used by hotplug path where lock is already held.
+ */
+void __tb_domain_reset_interface_locked(struct tb *tb)
{
struct tb_nhi *nhi = tb->nhi;
+ lockdep_assert_held(&tb->lock);
+
if (!nhi->ops->reset_interface)
return;
- guard(mutex)(&tb->lock);
+ if (!(nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN))
+ return;
/* The reset clears the ring state so stop the control channel */
tb_ctl_stop(tb->ctl);
@@ -803,6 +811,12 @@ static void tb_domain_reset_interface(struct tb *tb)
tb_ctl_start(tb->ctl);
}
+static void tb_domain_reset_interface(struct tb *tb)
+{
+ guard(mutex)(&tb->lock);
+ __tb_domain_reset_interface_locked(tb);
+}
+
/**
* tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
* @tb: Domain disabling the DMA paths
@@ -835,7 +849,7 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
if (ret)
return ret;
- if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
+ if (!xd->is_unplugged)
tb_domain_reset_interface(tb);
return 0;
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index b7cc6894a598..89dfb3381345 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -2489,6 +2489,7 @@ static void tb_handle_hotplug(struct work_struct *work)
tb_xdomain_remove(xd);
port->xdomain = NULL;
__tb_disconnect_xdomain_paths(tb, xd, -1, -1, -1, -1);
+ __tb_domain_reset_interface_locked(tb);
tb_xdomain_put(xd);
tb_port_unconfigure_xdomain(port);
} else if (tb_port_is_dpout(port) || tb_port_is_dpin(port)) {
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 4373336d9425..2e6e0920cb1f 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -789,6 +789,7 @@ int tb_domain_disconnect_pcie_paths(struct tb *tb);
int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
int transmit_path, int transmit_ring,
int receive_path, int receive_ring);
+void __tb_domain_reset_interface_locked(struct tb *tb);
int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
int transmit_path, int transmit_ring,
int receive_path, int receive_ring);
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
2026-08-28 5:19 ` [PATCH v4] " Juan Martinez
@ 2026-08-28 14:58 ` Mario Limonciello
2026-08-31 11:11 ` Mika Westerberg
1 sibling, 0 replies; 13+ messages in thread
From: Mario Limonciello @ 2026-08-28 14:58 UTC (permalink / raw)
To: westeri
Cc: andreas.noever, YehezkelShB, Basavaraj.Natikar, Sanath.S,
linux-usb, linux-kernel, Juan Martinez
On 8/28/26 00:19, Juan Martinez wrote:
> Commit f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on
> DMA path teardown for AMD USB4 routers") introduced a deadlock when
> physically unplugging a Thunderbolt cable on AMD systems.
>
> The problem occurs because tb_handle_hotplug() holds tb->lock while
> processing the unplug event. When it removes the XDomain services,
> tbnet_remove() calls tb_xdomain_disable_paths() which eventually calls
> tb_domain_reset_interface(). That function tries to acquire tb->lock
> via guard(mutex), but the hotplug worker already holds it, causing a
> self-deadlock.
>
> The deadlock manifests as a complete network hang because
> tb_handle_hotplug() holds RTNL while waiting on its own mutex, blocking
> all network operations system-wide.
>
> The existing code already handles this scenario partially: when
> xd->is_unplugged is true, tb_disconnect_xdomain_paths() intentionally
> skips the DMA teardown because the hotplug handler will do it later
> via __tb_disconnect_xdomain_paths(). However, the reset was still
> being called unconditionally.
>
> Fix this by:
> 1. Splitting tb_domain_reset_interface() into a locked inner function
> __tb_domain_reset_interface_locked() and a locking wrapper
> 2. Skipping the reset in tb_domain_disconnect_xdomain_paths() when
> xd->is_unplugged is true (matching the existing teardown skip logic)
> 3. Calling __tb_domain_reset_interface_locked() from tb_handle_hotplug()
> after __tb_disconnect_xdomain_paths() where the actual DMA teardown
> happens and tb->lock is already held
>
> This preserves the reset behavior for normal shutdown paths while
> avoiding the deadlock during physical cable unplug.
>
> Fixes: f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers")
> Signed-off-by: Juan Martinez <juan.martinez@amd.com>
Looks good to me now, thanks for iterating.
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Mika,
The quirk this is fixing came in 7.3 cycle. So this patch should also
go to -fixes for 7.3 cycle.
> ---
> drivers/thunderbolt/domain.c | 20 +++++++++++++++++---
> drivers/thunderbolt/tb.c | 1 +
> drivers/thunderbolt/tb.h | 1 +
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
> index 12c88509a54f..4cef9f4de523 100644
> --- a/drivers/thunderbolt/domain.c
> +++ b/drivers/thunderbolt/domain.c
> @@ -788,14 +788,22 @@ int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> transmit_ring, receive_path, receive_ring);
> }
>
> -static void tb_domain_reset_interface(struct tb *tb)
> +/*
> + * __tb_domain_reset_interface_locked - Reset host interface (lock held)
> + *
> + * Caller must hold tb->lock. Used by hotplug path where lock is already held.
> + */
> +void __tb_domain_reset_interface_locked(struct tb *tb)
> {
> struct tb_nhi *nhi = tb->nhi;
>
> + lockdep_assert_held(&tb->lock);
> +
> if (!nhi->ops->reset_interface)
> return;
>
> - guard(mutex)(&tb->lock);
> + if (!(nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN))
> + return;
>
> /* The reset clears the ring state so stop the control channel */
> tb_ctl_stop(tb->ctl);
> @@ -803,6 +811,12 @@ static void tb_domain_reset_interface(struct tb *tb)
> tb_ctl_start(tb->ctl);
> }
>
> +static void tb_domain_reset_interface(struct tb *tb)
> +{
> + guard(mutex)(&tb->lock);
> + __tb_domain_reset_interface_locked(tb);
> +}
> +
> /**
> * tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
> * @tb: Domain disabling the DMA paths
> @@ -835,7 +849,7 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> if (ret)
> return ret;
>
> - if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
> + if (!xd->is_unplugged)
> tb_domain_reset_interface(tb);
>
> return 0;
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index b7cc6894a598..89dfb3381345 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -2489,6 +2489,7 @@ static void tb_handle_hotplug(struct work_struct *work)
> tb_xdomain_remove(xd);
> port->xdomain = NULL;
> __tb_disconnect_xdomain_paths(tb, xd, -1, -1, -1, -1);
> + __tb_domain_reset_interface_locked(tb);
> tb_xdomain_put(xd);
> tb_port_unconfigure_xdomain(port);
> } else if (tb_port_is_dpout(port) || tb_port_is_dpin(port)) {
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 4373336d9425..2e6e0920cb1f 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -789,6 +789,7 @@ int tb_domain_disconnect_pcie_paths(struct tb *tb);
> int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> int transmit_path, int transmit_ring,
> int receive_path, int receive_ring);
> +void __tb_domain_reset_interface_locked(struct tb *tb);
> int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> int transmit_path, int transmit_ring,
> int receive_path, int receive_ring);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
2026-08-28 5:19 ` [PATCH v4] " Juan Martinez
2026-08-28 14:58 ` Mario Limonciello
@ 2026-08-31 11:11 ` Mika Westerberg
2026-08-31 12:55 ` Mario Limonciello
1 sibling, 1 reply; 13+ messages in thread
From: Mika Westerberg @ 2026-08-31 11:11 UTC (permalink / raw)
To: Juan Martinez
Cc: westeri, mario.limonciello, andreas.noever, YehezkelShB,
Basavaraj.Natikar, Sanath.S, linux-usb, linux-kernel
Hi,
On Fri, Aug 28, 2026 at 12:19:03AM -0500, Juan Martinez wrote:
> Commit f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on
> DMA path teardown for AMD USB4 routers") introduced a deadlock when
> physically unplugging a Thunderbolt cable on AMD systems.
>
> The problem occurs because tb_handle_hotplug() holds tb->lock while
> processing the unplug event. When it removes the XDomain services,
> tbnet_remove() calls tb_xdomain_disable_paths() which eventually calls
> tb_domain_reset_interface(). That function tries to acquire tb->lock
> via guard(mutex), but the hotplug worker already holds it, causing a
> self-deadlock.
>
> The deadlock manifests as a complete network hang because
> tb_handle_hotplug() holds RTNL while waiting on its own mutex, blocking
> all network operations system-wide.
>
> The existing code already handles this scenario partially: when
> xd->is_unplugged is true, tb_disconnect_xdomain_paths() intentionally
> skips the DMA teardown because the hotplug handler will do it later
> via __tb_disconnect_xdomain_paths(). However, the reset was still
> being called unconditionally.
>
> Fix this by:
> 1. Splitting tb_domain_reset_interface() into a locked inner function
> __tb_domain_reset_interface_locked() and a locking wrapper
> 2. Skipping the reset in tb_domain_disconnect_xdomain_paths() when
> xd->is_unplugged is true (matching the existing teardown skip logic)
> 3. Calling __tb_domain_reset_interface_locked() from tb_handle_hotplug()
> after __tb_disconnect_xdomain_paths() where the actual DMA teardown
> happens and tb->lock is already held
>
> This preserves the reset behavior for normal shutdown paths while
> avoiding the deadlock during physical cable unplug.
>
> Fixes: f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers")
> Signed-off-by: Juan Martinez <juan.martinez@amd.com>
> ---
> drivers/thunderbolt/domain.c | 20 +++++++++++++++++---
> drivers/thunderbolt/tb.c | 1 +
> drivers/thunderbolt/tb.h | 1 +
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
> index 12c88509a54f..4cef9f4de523 100644
> --- a/drivers/thunderbolt/domain.c
> +++ b/drivers/thunderbolt/domain.c
> @@ -788,14 +788,22 @@ int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> transmit_ring, receive_path, receive_ring);
> }
>
> -static void tb_domain_reset_interface(struct tb *tb)
> +/*
> + * __tb_domain_reset_interface_locked - Reset host interface (lock held)
> + *
> + * Caller must hold tb->lock. Used by hotplug path where lock is already held.
> + */
> +void __tb_domain_reset_interface_locked(struct tb *tb)
> {
> struct tb_nhi *nhi = tb->nhi;
>
> + lockdep_assert_held(&tb->lock);
> +
> if (!nhi->ops->reset_interface)
> return;
>
> - guard(mutex)(&tb->lock);
> + if (!(nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN))
> + return;
>
> /* The reset clears the ring state so stop the control channel */
> tb_ctl_stop(tb->ctl);
> @@ -803,6 +811,12 @@ static void tb_domain_reset_interface(struct tb *tb)
> tb_ctl_start(tb->ctl);
> }
>
> +static void tb_domain_reset_interface(struct tb *tb)
> +{
> + guard(mutex)(&tb->lock);
> + __tb_domain_reset_interface_locked(tb);
> +}
> +
> /**
> * tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
> * @tb: Domain disabling the DMA paths
> @@ -835,7 +849,7 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> if (ret)
> return ret;
>
> - if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
> + if (!xd->is_unplugged)
If I read this right, if you unplug a tree (say a router and then after
that router there is inter-domain link) with this check the reset does not
happen and AMD system still hangs?
> tb_domain_reset_interface(tb);
>
> return 0;
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index b7cc6894a598..89dfb3381345 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -2489,6 +2489,7 @@ static void tb_handle_hotplug(struct work_struct *work)
> tb_xdomain_remove(xd);
> port->xdomain = NULL;
> __tb_disconnect_xdomain_paths(tb, xd, -1, -1, -1, -1);
> + __tb_domain_reset_interface_locked(tb);
> tb_xdomain_put(xd);
> tb_port_unconfigure_xdomain(port);
> } else if (tb_port_is_dpout(port) || tb_port_is_dpin(port)) {
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 4373336d9425..2e6e0920cb1f 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -789,6 +789,7 @@ int tb_domain_disconnect_pcie_paths(struct tb *tb);
> int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> int transmit_path, int transmit_ring,
> int receive_path, int receive_ring);
> +void __tb_domain_reset_interface_locked(struct tb *tb);
> int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> int transmit_path, int transmit_ring,
> int receive_path, int receive_ring);
> --
> 2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
2026-08-31 11:11 ` Mika Westerberg
@ 2026-08-31 12:55 ` Mario Limonciello
2026-08-31 13:06 ` Mika Westerberg
0 siblings, 1 reply; 13+ messages in thread
From: Mario Limonciello @ 2026-08-31 12:55 UTC (permalink / raw)
To: Mika Westerberg, Juan Martinez
Cc: westeri, andreas.noever, YehezkelShB, Basavaraj.Natikar, Sanath.S,
linux-usb, linux-kernel
On 8/31/26 06:11, Mika Westerberg wrote:
> Hi,
>
> On Fri, Aug 28, 2026 at 12:19:03AM -0500, Juan Martinez wrote:
>> Commit f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on
>> DMA path teardown for AMD USB4 routers") introduced a deadlock when
>> physically unplugging a Thunderbolt cable on AMD systems.
>>
>> The problem occurs because tb_handle_hotplug() holds tb->lock while
>> processing the unplug event. When it removes the XDomain services,
>> tbnet_remove() calls tb_xdomain_disable_paths() which eventually calls
>> tb_domain_reset_interface(). That function tries to acquire tb->lock
>> via guard(mutex), but the hotplug worker already holds it, causing a
>> self-deadlock.
>>
>> The deadlock manifests as a complete network hang because
>> tb_handle_hotplug() holds RTNL while waiting on its own mutex, blocking
>> all network operations system-wide.
>>
>> The existing code already handles this scenario partially: when
>> xd->is_unplugged is true, tb_disconnect_xdomain_paths() intentionally
>> skips the DMA teardown because the hotplug handler will do it later
>> via __tb_disconnect_xdomain_paths(). However, the reset was still
>> being called unconditionally.
>>
>> Fix this by:
>> 1. Splitting tb_domain_reset_interface() into a locked inner function
>> __tb_domain_reset_interface_locked() and a locking wrapper
>> 2. Skipping the reset in tb_domain_disconnect_xdomain_paths() when
>> xd->is_unplugged is true (matching the existing teardown skip logic)
>> 3. Calling __tb_domain_reset_interface_locked() from tb_handle_hotplug()
>> after __tb_disconnect_xdomain_paths() where the actual DMA teardown
>> happens and tb->lock is already held
>>
>> This preserves the reset behavior for normal shutdown paths while
>> avoiding the deadlock during physical cable unplug.
>>
>> Fixes: f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers")
>> Signed-off-by: Juan Martinez <juan.martinez@amd.com>
>> ---
>> drivers/thunderbolt/domain.c | 20 +++++++++++++++++---
>> drivers/thunderbolt/tb.c | 1 +
>> drivers/thunderbolt/tb.h | 1 +
>> 3 files changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
>> index 12c88509a54f..4cef9f4de523 100644
>> --- a/drivers/thunderbolt/domain.c
>> +++ b/drivers/thunderbolt/domain.c
>> @@ -788,14 +788,22 @@ int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
>> transmit_ring, receive_path, receive_ring);
>> }
>>
>> -static void tb_domain_reset_interface(struct tb *tb)
>> +/*
>> + * __tb_domain_reset_interface_locked - Reset host interface (lock held)
>> + *
>> + * Caller must hold tb->lock. Used by hotplug path where lock is already held.
>> + */
>> +void __tb_domain_reset_interface_locked(struct tb *tb)
>> {
>> struct tb_nhi *nhi = tb->nhi;
>>
>> + lockdep_assert_held(&tb->lock);
>> +
>> if (!nhi->ops->reset_interface)
>> return;
>>
>> - guard(mutex)(&tb->lock);
>> + if (!(nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN))
>> + return;
>>
>> /* The reset clears the ring state so stop the control channel */
>> tb_ctl_stop(tb->ctl);
>> @@ -803,6 +811,12 @@ static void tb_domain_reset_interface(struct tb *tb)
>> tb_ctl_start(tb->ctl);
>> }
>>
>> +static void tb_domain_reset_interface(struct tb *tb)
>> +{
>> + guard(mutex)(&tb->lock);
>> + __tb_domain_reset_interface_locked(tb);
>> +}
>> +
>> /**
>> * tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
>> * @tb: Domain disabling the DMA paths
>> @@ -835,7 +849,7 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
>> if (ret)
>> return ret;
>>
>> - if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
>> + if (!xd->is_unplugged)
>
> If I read this right, if you unplug a tree (say a router and then after
> that router there is inter-domain link) with this check the reset does not
> happen and AMD system still hangs?
>
It should be a different problem. The quirk fixed a problem in the USB4
router (router wasn't functional when problem occurred).
This fixes a deadlock in the kernel that system gets stuck while
shutting down.
>> tb_domain_reset_interface(tb);
>>
>> return 0;
>> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
>> index b7cc6894a598..89dfb3381345 100644
>> --- a/drivers/thunderbolt/tb.c
>> +++ b/drivers/thunderbolt/tb.c
>> @@ -2489,6 +2489,7 @@ static void tb_handle_hotplug(struct work_struct *work)
>> tb_xdomain_remove(xd);
>> port->xdomain = NULL;
>> __tb_disconnect_xdomain_paths(tb, xd, -1, -1, -1, -1);
>> + __tb_domain_reset_interface_locked(tb);
>> tb_xdomain_put(xd);
>> tb_port_unconfigure_xdomain(port);
>> } else if (tb_port_is_dpout(port) || tb_port_is_dpin(port)) {
>> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
>> index 4373336d9425..2e6e0920cb1f 100644
>> --- a/drivers/thunderbolt/tb.h
>> +++ b/drivers/thunderbolt/tb.h
>> @@ -789,6 +789,7 @@ int tb_domain_disconnect_pcie_paths(struct tb *tb);
>> int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
>> int transmit_path, int transmit_ring,
>> int receive_path, int receive_ring);
>> +void __tb_domain_reset_interface_locked(struct tb *tb);
>> int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
>> int transmit_path, int transmit_ring,
>> int receive_path, int receive_ring);
>> --
>> 2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
2026-08-31 12:55 ` Mario Limonciello
@ 2026-08-31 13:06 ` Mika Westerberg
2026-08-31 13:07 ` Mario Limonciello
2026-08-31 16:16 ` [PATCH v5] " juan.martinez
0 siblings, 2 replies; 13+ messages in thread
From: Mika Westerberg @ 2026-08-31 13:06 UTC (permalink / raw)
To: Mario Limonciello
Cc: Juan Martinez, westeri, andreas.noever, YehezkelShB,
Basavaraj.Natikar, Sanath.S, linux-usb, linux-kernel
Hi,
On Mon, Aug 31, 2026 at 07:55:16AM -0500, Mario Limonciello wrote:
> > > +static void tb_domain_reset_interface(struct tb *tb)
> > > +{
> > > + guard(mutex)(&tb->lock);
> > > + __tb_domain_reset_interface_locked(tb);
> > > +}
> > > +
> > > /**
> > > * tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
> > > * @tb: Domain disabling the DMA paths
> > > @@ -835,7 +849,7 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> > > if (ret)
> > > return ret;
> > > - if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
> > > + if (!xd->is_unplugged)
> >
> > If I read this right, if you unplug a tree (say a router and then after
> > that router there is inter-domain link) with this check the reset does not
> > happen and AMD system still hangs?
> >
>
> It should be a different problem. The quirk fixed a problem in the USB4
> router (router wasn't functional when problem occurred).
>
> This fixes a deadlock in the kernel that system gets stuck while shutting
> down.
Right, but now since this checks !xd->is_unplugged and only then does the
reset, if you unplug the whole chain including the XDomain connection (i.e.
the unplug happens say on host router downstream port) then we mark
everything below that with is_unplugged=1 so now this function will skip
the quirk and the DMA will get stuck, no?
Because..
>
> > > tb_domain_reset_interface(tb);
> > > return 0;
> > > diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> > > index b7cc6894a598..89dfb3381345 100644
> > > --- a/drivers/thunderbolt/tb.c
> > > +++ b/drivers/thunderbolt/tb.c
> > > @@ -2489,6 +2489,7 @@ static void tb_handle_hotplug(struct work_struct *work)
> > > tb_xdomain_remove(xd);
> > > port->xdomain = NULL;
> > > __tb_disconnect_xdomain_paths(tb, xd, -1, -1, -1, -1);
> > > + __tb_domain_reset_interface_locked(tb);
... this is not called when the unplug happens elsewhere (like before the
XDomain link).
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
2026-08-31 13:06 ` Mika Westerberg
@ 2026-08-31 13:07 ` Mario Limonciello
2026-08-31 16:16 ` [PATCH v5] " juan.martinez
1 sibling, 0 replies; 13+ messages in thread
From: Mario Limonciello @ 2026-08-31 13:07 UTC (permalink / raw)
To: Mika Westerberg
Cc: Juan Martinez, westeri, andreas.noever, YehezkelShB,
Basavaraj.Natikar, Sanath.S, linux-usb, linux-kernel
On 8/31/26 08:06, Mika Westerberg wrote:
> Hi,
>
> On Mon, Aug 31, 2026 at 07:55:16AM -0500, Mario Limonciello wrote:
>>>> +static void tb_domain_reset_interface(struct tb *tb)
>>>> +{
>>>> + guard(mutex)(&tb->lock);
>>>> + __tb_domain_reset_interface_locked(tb);
>>>> +}
>>>> +
>>>> /**
>>>> * tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
>>>> * @tb: Domain disabling the DMA paths
>>>> @@ -835,7 +849,7 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
>>>> if (ret)
>>>> return ret;
>>>> - if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
>>>> + if (!xd->is_unplugged)
>>>
>>> If I read this right, if you unplug a tree (say a router and then after
>>> that router there is inter-domain link) with this check the reset does not
>>> happen and AMD system still hangs?
>>>
>>
>> It should be a different problem. The quirk fixed a problem in the USB4
>> router (router wasn't functional when problem occurred).
>>
>> This fixes a deadlock in the kernel that system gets stuck while shutting
>> down.
>
> Right, but now since this checks !xd->is_unplugged and only then does the
> reset, if you unplug the whole chain including the XDomain connection (i.e.
> the unplug happens say on host router downstream port) then we mark
> everything below that with is_unplugged=1 so now this function will skip
> the quirk and the DMA will get stuck, no?
>
> Because..
>
>>
>>>> tb_domain_reset_interface(tb);
>>>> return 0;
>>>> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
>>>> index b7cc6894a598..89dfb3381345 100644
>>>> --- a/drivers/thunderbolt/tb.c
>>>> +++ b/drivers/thunderbolt/tb.c
>>>> @@ -2489,6 +2489,7 @@ static void tb_handle_hotplug(struct work_struct *work)
>>>> tb_xdomain_remove(xd);
>>>> port->xdomain = NULL;
>>>> __tb_disconnect_xdomain_paths(tb, xd, -1, -1, -1, -1);
>>>> + __tb_domain_reset_interface_locked(tb);
>
> ... this is not called when the unplug happens elsewhere (like before the
> XDomain link).
Ah yeah.
Juan - take a closer look at this, I think you're gonna have to spin it
again.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
2026-08-31 13:06 ` Mika Westerberg
2026-08-31 13:07 ` Mario Limonciello
@ 2026-08-31 16:16 ` juan.martinez
2026-09-01 22:16 ` Mario Limonciello
1 sibling, 1 reply; 13+ messages in thread
From: juan.martinez @ 2026-08-31 16:16 UTC (permalink / raw)
To: mika.westerberg
Cc: mario.limonciello, juan.martinez, westeri, andreas.noever,
YehezkelShB, Basavaraj.Natikar, Sanath.S, linux-usb, linux-kernel
From: Juan Martinez <juan.martinez@amd.com>
Commit f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on
DMA path teardown for AMD USB4 routers") introduced a deadlock when
physically unplugging a Thunderbolt cable on AMD systems.
The problem occurs because tb_handle_hotplug() holds tb->lock while
processing the unplug event. When it removes the XDomain services,
tbnet_remove() calls tb_xdomain_disable_paths() which eventually calls
tb_domain_reset_interface(). That function tries to acquire tb->lock
via guard(mutex), but the hotplug worker already holds it, causing a
self-deadlock.
The deadlock manifests as a complete network hang because
tb_handle_hotplug() holds RTNL while waiting on its own mutex, blocking
all network operations system-wide.
The existing code already handles this scenario partially: when
xd->is_unplugged is true, tb_disconnect_xdomain_paths() intentionally
skips the DMA teardown because the hotplug handler tears down the DMA
tunnels itself. However, the reset was still being called
unconditionally.
Fix this by splitting tb_domain_reset_interface() into a locked inner
function and a locking wrapper. Skip the reset from
tb_domain_disconnect_xdomain_paths() when xd->is_unplugged is true, and
instead reset the interface after the hotplug handler tears down the DMA
tunnel while already holding tb->lock.
Handle both unplug topologies: reset after the direct XDomain teardown,
and after invalid DMA tunnels are removed when an upstream router and its
downstream XDomain are unplugged together.
This preserves the reset behavior for normal shutdown paths while
avoiding the deadlock during physical cable unplug.
Fixes: f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers")
Signed-off-by: Juan Martinez <juan.martinez@amd.com>
---
Notes (v5):
Changes in v5:
- Rebase on the current thunderbolt/next branch.
- Reset the host interface after invalid DMA tunnels are removed, covering
XDomains below an unplugged router.
drivers/thunderbolt/domain.c | 20 +++++++++++++++++---
drivers/thunderbolt/tb.c | 10 +++++++++-
drivers/thunderbolt/tb.h | 1 +
3 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index 12c88509a54f..4cef9f4de523 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -788,14 +788,22 @@ int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
transmit_ring, receive_path, receive_ring);
}
-static void tb_domain_reset_interface(struct tb *tb)
+/*
+ * __tb_domain_reset_interface_locked - Reset host interface (lock held)
+ *
+ * Caller must hold tb->lock. Used by hotplug path where lock is already held.
+ */
+void __tb_domain_reset_interface_locked(struct tb *tb)
{
struct tb_nhi *nhi = tb->nhi;
+ lockdep_assert_held(&tb->lock);
+
if (!nhi->ops->reset_interface)
return;
- guard(mutex)(&tb->lock);
+ if (!(nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN))
+ return;
/* The reset clears the ring state so stop the control channel */
tb_ctl_stop(tb->ctl);
@@ -803,6 +811,12 @@ static void tb_domain_reset_interface(struct tb *tb)
tb_ctl_start(tb->ctl);
}
+static void tb_domain_reset_interface(struct tb *tb)
+{
+ guard(mutex)(&tb->lock);
+ __tb_domain_reset_interface_locked(tb);
+}
+
/**
* tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
* @tb: Domain disabling the DMA paths
@@ -835,7 +849,7 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
if (ret)
return ret;
- if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
+ if (!xd->is_unplugged)
tb_domain_reset_interface(tb);
return 0;
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 47753a5c0f2e..9300cdae10b1 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -1776,12 +1776,19 @@ static void tb_free_invalid_tunnels(struct tb *tb)
{
struct tb_cm *tcm = tb_priv(tb);
struct tb_tunnel *tunnel;
+ bool reset = false;
struct tb_tunnel *n;
list_for_each_entry_safe(tunnel, n, &tcm->tunnel_list, list) {
- if (tb_tunnel_is_invalid(tunnel))
+ if (tb_tunnel_is_invalid(tunnel)) {
+ if (tb_tunnel_is_dma(tunnel))
+ reset = true;
tb_deactivate_and_free_tunnel(tunnel);
+ }
}
+
+ if (reset)
+ __tb_domain_reset_interface_locked(tb);
}
/*
@@ -2489,6 +2496,7 @@ static void tb_handle_hotplug(struct work_struct *work)
tb_xdomain_remove(xd);
port->xdomain = NULL;
__tb_disconnect_xdomain_paths(tb, xd, -1, -1, -1, -1);
+ __tb_domain_reset_interface_locked(tb);
tb_xdomain_put(xd);
tb_port_unconfigure_xdomain(port);
} else if (tb_port_is_dpout(port) || tb_port_is_dpin(port)) {
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index c112954ce3fd..5bb448a71407 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -792,6 +792,7 @@ int tb_domain_disconnect_pcie_paths(struct tb *tb);
int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
int transmit_path, int transmit_ring,
int receive_path, int receive_ring);
+void __tb_domain_reset_interface_locked(struct tb *tb);
int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
int transmit_path, int transmit_ring,
int receive_path, int receive_ring);
base-commit: 48e989e33b715611438ce4b8d6ff712d4becd84f
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
2026-08-31 16:16 ` [PATCH v5] " juan.martinez
@ 2026-09-01 22:16 ` Mario Limonciello
2026-09-02 5:48 ` Mika Westerberg
0 siblings, 1 reply; 13+ messages in thread
From: Mario Limonciello @ 2026-09-01 22:16 UTC (permalink / raw)
To: juan.martinez, mika.westerberg
Cc: westeri, andreas.noever, YehezkelShB, Basavaraj.Natikar, Sanath.S,
linux-usb, linux-kernel
On 8/31/26 11:16, juan.martinez@amd.com wrote:
> From: Juan Martinez <juan.martinez@amd.com>
>
> Commit f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on
> DMA path teardown for AMD USB4 routers") introduced a deadlock when
> physically unplugging a Thunderbolt cable on AMD systems.
>
> The problem occurs because tb_handle_hotplug() holds tb->lock while
> processing the unplug event. When it removes the XDomain services,
> tbnet_remove() calls tb_xdomain_disable_paths() which eventually calls
> tb_domain_reset_interface(). That function tries to acquire tb->lock
> via guard(mutex), but the hotplug worker already holds it, causing a
> self-deadlock.
>
> The deadlock manifests as a complete network hang because
> tb_handle_hotplug() holds RTNL while waiting on its own mutex, blocking
> all network operations system-wide.
>
> The existing code already handles this scenario partially: when
> xd->is_unplugged is true, tb_disconnect_xdomain_paths() intentionally
> skips the DMA teardown because the hotplug handler tears down the DMA
> tunnels itself. However, the reset was still being called
> unconditionally.
>
> Fix this by splitting tb_domain_reset_interface() into a locked inner
> function and a locking wrapper. Skip the reset from
> tb_domain_disconnect_xdomain_paths() when xd->is_unplugged is true, and
> instead reset the interface after the hotplug handler tears down the DMA
> tunnel while already holding tb->lock.
>
> Handle both unplug topologies: reset after the direct XDomain teardown,
> and after invalid DMA tunnels are removed when an upstream router and its
> downstream XDomain are unplugged together.
>
> This preserves the reset behavior for normal shutdown paths while
> avoiding the deadlock during physical cable unplug.
>
> Fixes: f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers")
> Signed-off-by: Juan Martinez <juan.martinez@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
BTW -
I did take a look through the Sashiko feedback and the first point
doesn't matter because no pre-USB4 hosts take this quirk.
The second point is a side effect of this reset and accepted behavior.
> ---
>
> Notes (v5):
> Changes in v5:
> - Rebase on the current thunderbolt/next branch.
> - Reset the host interface after invalid DMA tunnels are removed, covering
> XDomains below an unplugged router.
>
> drivers/thunderbolt/domain.c | 20 +++++++++++++++++---
> drivers/thunderbolt/tb.c | 10 +++++++++-
> drivers/thunderbolt/tb.h | 1 +
> 3 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
> index 12c88509a54f..4cef9f4de523 100644
> --- a/drivers/thunderbolt/domain.c
> +++ b/drivers/thunderbolt/domain.c
> @@ -788,14 +788,22 @@ int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> transmit_ring, receive_path, receive_ring);
> }
>
> -static void tb_domain_reset_interface(struct tb *tb)
> +/*
> + * __tb_domain_reset_interface_locked - Reset host interface (lock held)
> + *
> + * Caller must hold tb->lock. Used by hotplug path where lock is already held.
> + */
> +void __tb_domain_reset_interface_locked(struct tb *tb)
> {
> struct tb_nhi *nhi = tb->nhi;
>
> + lockdep_assert_held(&tb->lock);
> +
> if (!nhi->ops->reset_interface)
> return;
>
> - guard(mutex)(&tb->lock);
> + if (!(nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN))
> + return;
>
> /* The reset clears the ring state so stop the control channel */
> tb_ctl_stop(tb->ctl);
> @@ -803,6 +811,12 @@ static void tb_domain_reset_interface(struct tb *tb)
> tb_ctl_start(tb->ctl);
> }
>
> +static void tb_domain_reset_interface(struct tb *tb)
> +{
> + guard(mutex)(&tb->lock);
> + __tb_domain_reset_interface_locked(tb);
> +}
> +
> /**
> * tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
> * @tb: Domain disabling the DMA paths
> @@ -835,7 +849,7 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> if (ret)
> return ret;
>
> - if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
> + if (!xd->is_unplugged)
> tb_domain_reset_interface(tb);
>
> return 0;
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index 47753a5c0f2e..9300cdae10b1 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -1776,12 +1776,19 @@ static void tb_free_invalid_tunnels(struct tb *tb)
> {
> struct tb_cm *tcm = tb_priv(tb);
> struct tb_tunnel *tunnel;
> + bool reset = false;
> struct tb_tunnel *n;
>
> list_for_each_entry_safe(tunnel, n, &tcm->tunnel_list, list) {
> - if (tb_tunnel_is_invalid(tunnel))
> + if (tb_tunnel_is_invalid(tunnel)) {
> + if (tb_tunnel_is_dma(tunnel))
> + reset = true;
> tb_deactivate_and_free_tunnel(tunnel);
> + }
> }
> +
> + if (reset)
> + __tb_domain_reset_interface_locked(tb);
> }
>
> /*
> @@ -2489,6 +2496,7 @@ static void tb_handle_hotplug(struct work_struct *work)
> tb_xdomain_remove(xd);
> port->xdomain = NULL;
> __tb_disconnect_xdomain_paths(tb, xd, -1, -1, -1, -1);
> + __tb_domain_reset_interface_locked(tb);
> tb_xdomain_put(xd);
> tb_port_unconfigure_xdomain(port);
> } else if (tb_port_is_dpout(port) || tb_port_is_dpin(port)) {
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index c112954ce3fd..5bb448a71407 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -792,6 +792,7 @@ int tb_domain_disconnect_pcie_paths(struct tb *tb);
> int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> int transmit_path, int transmit_ring,
> int receive_path, int receive_ring);
> +void __tb_domain_reset_interface_locked(struct tb *tb);
> int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> int transmit_path, int transmit_ring,
> int receive_path, int receive_ring);
>
> base-commit: 48e989e33b715611438ce4b8d6ff712d4becd84f
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers
2026-09-01 22:16 ` Mario Limonciello
@ 2026-09-02 5:48 ` Mika Westerberg
0 siblings, 0 replies; 13+ messages in thread
From: Mika Westerberg @ 2026-09-02 5:48 UTC (permalink / raw)
To: Mario Limonciello
Cc: juan.martinez, westeri, andreas.noever, YehezkelShB,
Basavaraj.Natikar, Sanath.S, linux-usb, linux-kernel
Hi,
On Tue, Sep 01, 2026 at 05:16:03PM -0500, Mario Limonciello wrote:
> On 8/31/26 11:16, juan.martinez@amd.com wrote:
> > From: Juan Martinez <juan.martinez@amd.com>
> >
> > Commit f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on
> > DMA path teardown for AMD USB4 routers") introduced a deadlock when
> > physically unplugging a Thunderbolt cable on AMD systems.
> >
> > The problem occurs because tb_handle_hotplug() holds tb->lock while
> > processing the unplug event. When it removes the XDomain services,
> > tbnet_remove() calls tb_xdomain_disable_paths() which eventually calls
> > tb_domain_reset_interface(). That function tries to acquire tb->lock
> > via guard(mutex), but the hotplug worker already holds it, causing a
> > self-deadlock.
> >
> > The deadlock manifests as a complete network hang because
> > tb_handle_hotplug() holds RTNL while waiting on its own mutex, blocking
> > all network operations system-wide.
> >
> > The existing code already handles this scenario partially: when
> > xd->is_unplugged is true, tb_disconnect_xdomain_paths() intentionally
> > skips the DMA teardown because the hotplug handler tears down the DMA
> > tunnels itself. However, the reset was still being called
> > unconditionally.
> >
> > Fix this by splitting tb_domain_reset_interface() into a locked inner
> > function and a locking wrapper. Skip the reset from
> > tb_domain_disconnect_xdomain_paths() when xd->is_unplugged is true, and
> > instead reset the interface after the hotplug handler tears down the DMA
> > tunnel while already holding tb->lock.
> >
> > Handle both unplug topologies: reset after the direct XDomain teardown,
> > and after invalid DMA tunnels are removed when an upstream router and its
> > downstream XDomain are unplugged together.
> >
> > This preserves the reset behavior for normal shutdown paths while
> > avoiding the deadlock during physical cable unplug.
> >
> > Fixes: f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers")
> > Signed-off-by: Juan Martinez <juan.martinez@amd.com>
>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
>
> BTW -
>
> I did take a look through the Sashiko feedback and the first point doesn't
> matter because no pre-USB4 hosts take this quirk.
>
> The second point is a side effect of this reset and accepted behavior.
I did not find Sashiko comments for this last version but started thinking
that the way we are doing it now is quite brutal. Say we have this setup:
- Thunderbolt networking for control traffic
- One USB4STREAM for data plane
The data plane comes and goes depending on the needs but what happens now
is that after USB4STREAM tears down the tunnels, the whole host interface
get reset so that makes the Thunderbolt networking to fail as well.
Is the original hardware hang per-ring? So for example in this case without
any fixes the rings for USB4STREAM would hang but the Thunderbolt
networking would keep working? I would expect so bot it would be good to
confirm.
Because then I think what we can do is to revert the original fix and then
handle this all in nhi.c so that we delay the reset until the rings are
idle and during that time we hand off "unused" rings (until running out of
them). After we find the rings to be idle we block the CM and control
channel and do the reset. I don't know how many rings AMD hardware has,
though.
I sketched something along those lines (only compile tested) with LLM see
below. I wonder if this could work?
diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index b6f5079cdf6f..1564ce6fca37 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -671,6 +671,32 @@ int tb_domain_runtime_resume(struct tb *tb)
return 0;
}
+/**
+ * tb_domain_pause() - Pause the domain
+ * @tb: Domain to pause
+ *
+ * Blocks the connection manager and stops the control channel so that
+ * the caller can touch the host interface hardware behind its back.
+ * Takes @tb->lock.
+ *
+ * Once done whatever operations needed call tb_domain_unpause().
+ */
+void tb_domain_pause(struct tb *tb)
+{
+ mutex_lock(&tb->lock);
+ tb_ctl_stop(tb->ctl);
+}
+
+/**
+ * tb_domain_unpause() - Resume paused domain
+ * @tb: Domain to unpause
+ */
+void tb_domain_unpause(struct tb *tb)
+{
+ tb_ctl_start(tb->ctl);
+ mutex_unlock(&tb->lock);
+}
+
/**
* tb_domain_disapprove_switch() - Disapprove switch
* @tb: Domain the switch belongs to
@@ -835,21 +861,6 @@ int tb_domain_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
transmit_ring, receive_path, receive_ring);
}
-static void tb_domain_reset_interface(struct tb *tb)
-{
- struct tb_nhi *nhi = tb->nhi;
-
- if (!nhi->ops->reset_interface)
- return;
-
- guard(mutex)(&tb->lock);
-
- /* The reset clears the ring state so stop the control channel */
- tb_ctl_stop(tb->ctl);
- nhi->ops->reset_interface(nhi);
- tb_ctl_start(tb->ctl);
-}
-
/**
* tb_domain_disconnect_xdomain_paths() - Disable DMA paths for XDomain
* @tb: Domain disabling the DMA paths
@@ -872,20 +883,11 @@ int tb_domain_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
int transmit_path, int transmit_ring,
int receive_path, int receive_ring)
{
- int ret;
-
if (!tb->cm_ops->disconnect_xdomain_paths)
return -ENOTSUPP;
- ret = tb->cm_ops->disconnect_xdomain_paths(tb, xd, transmit_path,
+ return tb->cm_ops->disconnect_xdomain_paths(tb, xd, transmit_path,
transmit_ring, receive_path, receive_ring);
- if (ret)
- return ret;
-
- if (tb->nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN)
- tb_domain_reset_interface(tb);
-
- return 0;
}
static int disconnect_xdomain(struct device *dev, void *data)
diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index a7e6184cdfe1..423e7dba473c 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -523,9 +523,45 @@ irqreturn_t ring_msix(int irq, void *data)
return IRQ_HANDLED;
}
+static bool ring_is_dma(const struct tb_ring *ring)
+{
+ return ring->hop >= RING_FIRST_USABLE_HOPID;
+}
+
+static bool nhi_dma_rings_running(const struct tb_nhi *nhi)
+{
+ int i;
+
+ lockdep_assert_held(&nhi->lock);
+
+ /*
+ * Holding nhi->lock is enough here because tb_ring_start() and
+ * tb_ring_stop() both hold it when they update ring->running.
+ */
+ for (i = RING_FIRST_USABLE_HOPID; i < nhi->hop_count; i++) {
+ if (nhi->tx_rings[i] && nhi->tx_rings[i]->running)
+ return true;
+ if (nhi->rx_rings[i] && nhi->rx_rings[i]->running)
+ return true;
+ }
+
+ return false;
+}
+
+static bool nhi_avoid_used_dma_rings(const struct tb_nhi *nhi)
+{
+ lockdep_assert_held(&nhi->lock);
+
+ if (!(nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN))
+ return false;
+
+ return nhi_dma_rings_running(nhi);
+}
+
static int nhi_alloc_hop(struct tb_nhi *nhi, struct tb_ring *ring)
{
unsigned int start_hop = RING_FIRST_USABLE_HOPID;
+ bool avoid_used;
int ret = 0;
if (nhi->quirks & QUIRK_E2E) {
@@ -539,6 +575,8 @@ static int nhi_alloc_hop(struct tb_nhi *nhi, struct tb_ring *ring)
spin_lock_irq(&nhi->lock);
+ avoid_used = nhi_avoid_used_dma_rings(nhi);
+
if (ring->hop < 0) {
unsigned int i;
@@ -547,6 +585,8 @@ static int nhi_alloc_hop(struct tb_nhi *nhi, struct tb_ring *ring)
* range 1 .. hop_count - 1.
*/
for (i = start_hop; i < nhi->hop_count; i++) {
+ if (avoid_used && test_bit(i, nhi->dma_hops_used))
+ continue;
if (ring->is_tx) {
if (!nhi->tx_rings[i]) {
ring->hop = i;
@@ -559,6 +599,13 @@ static int nhi_alloc_hop(struct tb_nhi *nhi, struct tb_ring *ring)
}
}
}
+
+ if (ring->hop < 0 && avoid_used) {
+ dev_warn(nhi->dev,
+ "out of HopIDs that do not need a host interface reset\n");
+ ret = -EBUSY;
+ goto err_unlock;
+ }
}
if (ring->hop > 0 && ring->hop < start_hop) {
@@ -583,6 +630,15 @@ static int nhi_alloc_hop(struct tb_nhi *nhi, struct tb_ring *ring)
ret = -EBUSY;
goto err_unlock;
}
+ /* Automatic allocation above already skips the used HopIDs */
+ if (avoid_used && ring_is_dma(ring) &&
+ test_bit(ring->hop, nhi->dma_hops_used)) {
+ dev_warn(nhi->dev,
+ "hop %d needs a host interface reset before reuse\n",
+ ring->hop);
+ ret = -EBUSY;
+ goto err_unlock;
+ }
if (ring->is_tx)
nhi->tx_rings[ring->hop] = ring;
@@ -710,6 +766,62 @@ struct tb_ring *tb_ring_alloc_rx(struct tb_nhi *nhi, int hop, int size,
}
EXPORT_SYMBOL_GPL(tb_ring_alloc_rx);
+/**
+ * nhi_reset_interface() - Reset the host interface
+ * @nhi: Host interface to reset
+ *
+ * Brings the registers in the memory BAR back to their default state and
+ * clears the End-to-End Flow Control state. The caller is responsible for
+ * stopping the control channel over the reset because it clears the ring
+ * state as well.
+ */
+static void nhi_reset_interface(struct tb_nhi *nhi)
+{
+ u32 val;
+
+ val = ioread32(nhi->iobase + REG_CAPS);
+ /* Only v1 host interfaces implement the reset */
+ if (FIELD_GET(REG_CAPS_VERSION_MASK, val) >= REG_CAPS_VERSION_2)
+ return;
+
+ dev_dbg(nhi->dev, "issuing host interface reset\n");
+
+ iowrite32(REG_HOST_INTERFACE_RESET_RST,
+ nhi->iobase + REG_HOST_INTERFACE_RESET);
+ /* Wait for tHIReset (10 ms) to complete */
+ usleep_range(10000, 20000);
+}
+
+static void nhi_reset_quirk(struct tb_ring *ring)
+{
+ struct tb_nhi *nhi = ring->nhi;
+ struct tb *tb = dev_get_drvdata(nhi->dev);
+
+ if (!(nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN))
+ return;
+ if (!ring_is_dma(ring))
+ return;
+
+ scoped_guard(spinlock_irq, &nhi->lock) {
+ if (nhi->going_away)
+ return;
+ if (bitmap_empty(nhi->dma_hops_used, nhi->hop_count))
+ return;
+ /*
+ * If any of the DMA rings are still running we cannot
+ * do the reset.
+ */
+ if (nhi_dma_rings_running(nhi))
+ return;
+
+ bitmap_zero(nhi->dma_hops_used, nhi->hop_count);
+ }
+
+ /* CM must be blocked before host interface reset can be done */
+ scoped_guard(tb_domain_paused, tb)
+ nhi_reset_interface(nhi);
+}
+
/**
* tb_ring_start() - enable a ring
* @ring: Ring to start
@@ -721,6 +833,8 @@ void tb_ring_start(struct tb_ring *ring)
u16 frame_size;
u32 flags;
+ nhi_reset_quirk(ring);
+
spin_lock_irq(&ring->nhi->lock);
spin_lock(&ring->lock);
if (ring->nhi->going_away)
@@ -781,6 +895,8 @@ void tb_ring_start(struct tb_ring *ring)
if (!(ring->flags & RING_FLAG_NO_INTERRUPT))
ring_interrupt_active(ring, true);
ring->running = true;
+ if (ring->nhi->dma_hops_used && ring_is_dma(ring))
+ __set_bit(ring->hop, ring->nhi->dma_hops_used);
err:
spin_unlock(&ring->lock);
spin_unlock_irq(&ring->nhi->lock);
@@ -1241,32 +1357,6 @@ static void nhi_reset(struct tb_nhi *nhi)
dev_warn(nhi->dev, "timeout resetting host router\n");
}
-/**
- * nhi_reset_interface() - Reset the host interface
- * @nhi: Host interface to reset
- *
- * Brings the registers in the memory BAR back to their default state and
- * clears the End-to-End Flow Control state. The caller is responsible for
- * stopping the control channel over the reset because it clears the ring
- * state as well.
- */
-void nhi_reset_interface(struct tb_nhi *nhi)
-{
- u32 val;
-
- val = ioread32(nhi->iobase + REG_CAPS);
- /* Only v1 host interfaces implement the reset */
- if (FIELD_GET(REG_CAPS_VERSION_MASK, val) >= REG_CAPS_VERSION_2)
- return;
-
- dev_dbg(nhi->dev, "issuing host interface reset\n");
-
- iowrite32(REG_HOST_INTERFACE_RESET_RST,
- nhi->iobase + REG_HOST_INTERFACE_RESET);
- /* Wait for tHIReset (10 ms) to complete */
- usleep_range(10000, 20000);
-}
-
static struct tb *nhi_select_cm(struct tb_nhi *nhi)
{
bool linked = false;
@@ -1331,6 +1421,13 @@ int nhi_probe(struct tb_nhi *nhi)
if (!nhi->tx_rings || !nhi->rx_rings || !nhi->interrupt_mask)
return -ENOMEM;
+ if (nhi->quirks & QUIRK_RESET_DMA_ON_TEARDOWN) {
+ nhi->dma_hops_used = devm_bitmap_zalloc(dev, nhi->hop_count,
+ GFP_KERNEL);
+ if (!nhi->dma_hops_used)
+ return -ENOMEM;
+ }
+
nhi_reset(nhi);
/* In case someone left them on. */
diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
index b2e2e2c413b2..53374c12b685 100644
--- a/drivers/thunderbolt/nhi.h
+++ b/drivers/thunderbolt/nhi.h
@@ -36,7 +36,6 @@ irqreturn_t nhi_msi(int irq, void *data);
irqreturn_t ring_msix(int irq, void *data);
int nhi_probe(struct tb_nhi *nhi);
void nhi_shutdown(struct tb_nhi *nhi);
-void nhi_reset_interface(struct tb_nhi *nhi);
extern const struct dev_pm_ops nhi_pm_ops;
@@ -55,7 +54,6 @@ extern const struct dev_pm_ops nhi_pm_ops;
* @release_ring_irq: NHI specific interrupt release hook
* @is_present: Whether the device is currently present on the parent bus
* @init_interrupts: NHI specific interrupt initialization hook
- * @reset_interface: Resets the host interface
*/
struct tb_nhi_ops {
int (*init)(struct tb_nhi *nhi);
@@ -71,7 +69,6 @@ struct tb_nhi_ops {
void (*release_ring_irq)(struct tb_ring *ring);
bool (*is_present)(struct tb_nhi *nhi);
int (*init_interrupts)(struct tb_nhi *nhi);
- void (*reset_interface)(struct tb_nhi *nhi);
};
/*
diff --git a/drivers/thunderbolt/pci.c b/drivers/thunderbolt/pci.c
index e40d4d6af071..0a586122db47 100644
--- a/drivers/thunderbolt/pci.c
+++ b/drivers/thunderbolt/pci.c
@@ -357,7 +357,6 @@ static const struct tb_nhi_ops pci_nhi_default_ops = {
.shutdown = nhi_pci_release_irq,
.is_present = nhi_pci_is_present,
.init_interrupts = nhi_pci_init_msi,
- .reset_interface = nhi_reset_interface,
};
/* Ice Lake specific NHI operations */
@@ -576,7 +575,6 @@ static const struct tb_nhi_ops icl_nhi_ops = {
.release_ring_irq = nhi_pci_ring_release_msix,
.is_present = nhi_pci_is_present,
.init_interrupts = nhi_pci_init_msi,
- .reset_interface = nhi_reset_interface,
};
static int nhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 1f78e2528c05..268bc7d90267 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -826,6 +826,10 @@ int tb_domain_thaw_noirq(struct tb *tb);
void tb_domain_complete(struct tb *tb);
int tb_domain_runtime_suspend(struct tb *tb);
int tb_domain_runtime_resume(struct tb *tb);
+void tb_domain_pause(struct tb *tb);
+void tb_domain_unpause(struct tb *tb);
+DEFINE_GUARD(tb_domain_paused, struct tb *, tb_domain_pause(_T),
+ tb_domain_unpause(_T))
int tb_domain_disapprove_switch(struct tb *tb, struct tb_switch *sw);
int tb_domain_approve_switch(struct tb *tb, struct tb_switch *sw);
int tb_domain_approve_switch_key(struct tb *tb, struct tb_switch *sw);
diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h
index 69839a514433..e1c270496cf6 100644
--- a/include/linux/thunderbolt.h
+++ b/include/linux/thunderbolt.h
@@ -537,6 +537,9 @@ void tb_service_properties_changed(struct tb_service *svc);
* MSI-X is used.
* @hop_count: Number of rings (end point hops) supported by NHI.
* @quirks: NHI specific quirks if any
+ * @dma_hops_used: Bitmap of HopIDs that have been programmed after the
+ * last host interface reset. Used only with
+ * %QUIRK_RESET_DMA_ON_TEARDOWN.
* @domain_released: Completed when domain has been fully released
* @host_reset: Host router was reset on driver load, or forced on system
* shutdown/reboot. When set, tb_stop() asserts DPR on connected
@@ -557,6 +560,7 @@ struct tb_nhi {
struct work_struct interrupt_work;
u32 hop_count;
unsigned long quirks;
+ unsigned long *dma_hops_used;
struct completion domain_released;
bool host_reset;
};
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-09-02 5:48 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-25 21:42 [PATCH v2] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers juan.martinez
2026-08-26 3:02 ` Mario Limonciello
2026-08-27 21:57 ` [PATCH v3] " Juan Martinez
2026-08-28 4:43 ` Mario Limonciello
2026-08-28 5:19 ` [PATCH v4] " Juan Martinez
2026-08-28 14:58 ` Mario Limonciello
2026-08-31 11:11 ` Mika Westerberg
2026-08-31 12:55 ` Mario Limonciello
2026-08-31 13:06 ` Mika Westerberg
2026-08-31 13:07 ` Mario Limonciello
2026-08-31 16:16 ` [PATCH v5] " juan.martinez
2026-09-01 22:16 ` Mario Limonciello
2026-09-02 5:48 ` Mika Westerberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox