Linux USB
 help / color / mirror / Atom feed
* [PATCH] thunderbolt: Fix deadlock in tb_domain_reset_interface() during hot-unplug
@ 2026-08-25  3:03 Martinez, Juan
  2026-08-25 13:51 ` Mario Limonciello
  0 siblings, 1 reply; 2+ messages in thread
From: Martinez, Juan @ 2026-08-25  3:03 UTC (permalink / raw)
  To: westeri@kernel.org
  Cc: andreas.noever@gmail.com, YehezkelShB@gmail.com,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

From 6add8f1c36c672e59f5acfd4e1dd267fd4df727a Mon Sep 17 00:00:00 2001
From: Juan Martinez <juan.martinez@amd.com>
Date: Mon, 24 Aug 2026 14:00:13 -0500
Subject: [PATCH] thunderbolt: Fix deadlock in tb_domain_reset_interface()
 during hot-unplug

The AMD host-interface reset quirk introduced a deadlock during physical
cable disconnect. The tb_handle_hotplug() workqueue holds tb->lock for
the entire hotplug operation. When an XDomain is unplugged:

1. tb_handle_hotplug() acquires tb->lock
2. Sets xd->is_unplugged = true
3. Calls tb_xdomain_remove() which unbinds service drivers
4. Service removal triggers tbnet_remove() -> tbnet_tear_down()
5. tbnet_tear_down() calls tb_xdomain_disable_paths()
6. tb_domain_disconnect_xdomain_paths() calls the CM callback
7. tb_disconnect_xdomain_paths() sees is_unplugged=true, skips teardown
8. tb_domain_reset_interface() tries to acquire tb->lock -> DEADLOCK

The callback intentionally skips teardown during unplug because the
hotplug handler will perform the actual teardown later via
__tb_disconnect_xdomain_paths(). The reset must follow the same pattern.

Fix by:
- Adding __tb_domain_reset_interface_locked() for callers that already
  hold tb->lock
- Skipping reset in tb_domain_disconnect_xdomain_paths() when the
  callback was a no-op due to is_unplugged
- Calling the locked reset helper after __tb_disconnect_xdomain_paths()
  in tb_handle_hotplug()
- Keeping the locking wrapper for normal teardown paths (netdev stop,
  DMA test completion)

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 | 27 ++++++++++++++++++++++++---
 drivers/thunderbolt/tb.c     |  2 ++
 drivers/thunderbolt/tb.h     |  1 +
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index f46b8b2ee6e1..1f0588bae366 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -787,11 +787,21 @@ 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 with lock held
+ * @tb: Domain to reset
+ *
+ * Resets the host interface for AMD USB4 routers that require it after
+ * DMA path teardown. Caller must hold tb->lock.
+ */
+void __tb_domain_reset_interface_locked(struct tb *tb)
 {
 	struct tb_nhi *nhi = tb->nhi;
 
-	guard(mutex)(&tb->lock);
+	lockdep_assert_held(&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);
@@ -799,6 +809,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
@@ -831,7 +847,12 @@ 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)
+	/*
+	 * Only reset if teardown actually occurred. During physical unplug,
+	 * is_unplugged is set and the callback skips teardown - reset will
+	 * be done by tb_handle_hotplug() after actual path 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 d18bcca0b6e6..2370d8669156 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -2489,6 +2489,8 @@ 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);
+			/* Reset host interface after DMA path teardown (AMD quirk) */
+			__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 798840b24d2a..9243073cae06 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -792,6 +792,7 @@ 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 tb_domain_disconnect_all_paths(struct tb *tb);
+void __tb_domain_reset_interface_locked(struct tb *tb);
 
 static inline struct tb *tb_domain_get(struct tb *tb)
 {
-- 
2.34.1


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

* Re: [PATCH] thunderbolt: Fix deadlock in tb_domain_reset_interface() during hot-unplug
  2026-08-25  3:03 [PATCH] thunderbolt: Fix deadlock in tb_domain_reset_interface() during hot-unplug Martinez, Juan
@ 2026-08-25 13:51 ` Mario Limonciello
  0 siblings, 0 replies; 2+ messages in thread
From: Mario Limonciello @ 2026-08-25 13:51 UTC (permalink / raw)
  To: Martinez, Juan, westeri@kernel.org
  Cc: andreas.noever@gmail.com, YehezkelShB@gmail.com,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

On 8/24/26 10:03 PM, Martinez, Juan wrote:
>>From 6add8f1c36c672e59f5acfd4e1dd267fd4df727a Mon Sep 17 00:00:00 2001
> From: Juan Martinez <juan.martinez@amd.com>
> Date: Mon, 24 Aug 2026 14:00:13 -0500
> Subject: [PATCH] thunderbolt: Fix deadlock in tb_domain_reset_interface()
>   during hot-unplug

Something funny here with the whitespace.

> 
> The AMD host-interface reset quirk introduced a deadlock during physical
> cable disconnect. The tb_handle_hotplug() workqueue holds tb->lock for
> the entire hotplug operation. When an XDomain is unplugged:
> 
> 1. tb_handle_hotplug() acquires tb->lock
> 2. Sets xd->is_unplugged = true
> 3. Calls tb_xdomain_remove() which unbinds service drivers
> 4. Service removal triggers tbnet_remove() -> tbnet_tear_down()
> 5. tbnet_tear_down() calls tb_xdomain_disable_paths()
> 6. tb_domain_disconnect_xdomain_paths() calls the CM callback
> 7. tb_disconnect_xdomain_paths() sees is_unplugged=true, skips teardown
> 8. tb_domain_reset_interface() tries to acquire tb->lock -> DEADLOCK
> 
> The callback intentionally skips teardown during unplug because the
> hotplug handler will perform the actual teardown later via
> __tb_disconnect_xdomain_paths(). The reset must follow the same pattern.
> 
> Fix by:
> - Adding __tb_domain_reset_interface_locked() for callers that already
>    hold tb->lock
> - Skipping reset in tb_domain_disconnect_xdomain_paths() when the
>    callback was a no-op due to is_unplugged
> - Calling the locked reset helper after __tb_disconnect_xdomain_paths()
>    in tb_handle_hotplug()
> - Keeping the locking wrapper for normal teardown paths (netdev stop,
>    DMA test completion)
> 
> 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>

What tree did you base this off of?  I tried on thunderbolt.git/next 
(86feaba911f2f1a540a7695c8f4a98fd0fd60ac4 / tag 
thunderbolt-for-v7.3-rc1,) but it doesn't apply:

❮ b4 shazam -sl 
https://lore.kernel.org/linux-usb/DM4PR12MB6134B3493ED0534463F86496F1AF2@DM4PR12MB6134.namprd12.prod.outlook.com/
Looking up 
https://lore.kernel.org/all/DM4PR12MB6134B3493ED0534463F86496F1AF2@DM4PR12MB6134.namprd12.prod.outlook.com/
Grabbing thread from 
lore.kernel.org/all/DM4PR12MB6134B3493ED0534463F86496F1AF2@DM4PR12MB6134.namprd12.prod.outlook.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 1 messages in the thread
Looking for additional code-review trailers on lore.kernel.org
Analyzing 0 code-review messages
Checking attestation on all messages, may take a moment...
---
   [PATCH] thunderbolt: Fix deadlock in tb_domain_reset_interface() 
during hot-unplug
     + Link: 
https://patch.msgid.link/DM4PR12MB6134B3493ED0534463F86496F1AF2@DM4PR12MB6134.namprd12.prod.outlook.com
     + Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
   ---
   NOTE: install dkimpy for DKIM signature verification
---
Total patches: 1
---
Applying: thunderbolt: Fix deadlock in tb_domain_reset_interface() 
during hot-unplug
Patch failed at 0001 thunderbolt: Fix deadlock in 
tb_domain_reset_interface() during hot-unplug
error: patch failed: drivers/thunderbolt/domain.c:787
error: drivers/thunderbolt/domain.c: patch does not apply
error: patch failed: drivers/thunderbolt/tb.h:792
error: drivers/thunderbolt/tb.h: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am 
--abort".
hint: Disable this message with "git config set advice.mergeConflict false"

Nonetheless; directionally it makes sense.  Can you please rebase and 
send a v2 for review?
> ---
>   drivers/thunderbolt/domain.c | 27 ++++++++++++++++++++++++---
>   drivers/thunderbolt/tb.c     |  2 ++
>   drivers/thunderbolt/tb.h     |  1 +
>   3 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
> index f46b8b2ee6e1..1f0588bae366 100644
> --- a/drivers/thunderbolt/domain.c
> +++ b/drivers/thunderbolt/domain.c
> @@ -787,11 +787,21 @@ 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 with lock held
> + * @tb: Domain to reset
> + *
> + * Resets the host interface for AMD USB4 routers that require it after
> + * DMA path teardown. Caller must hold tb->lock.
> + */
> +void __tb_domain_reset_interface_locked(struct tb *tb)
>   {
>   	struct tb_nhi *nhi = tb->nhi;
>   
> -	guard(mutex)(&tb->lock);
> +	lockdep_assert_held(&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);
> @@ -799,6 +809,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
> @@ -831,7 +847,12 @@ 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)
> +	/*
> +	 * Only reset if teardown actually occurred. During physical unplug,
> +	 * is_unplugged is set and the callback skips teardown - reset will
> +	 * be done by tb_handle_hotplug() after actual path 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 d18bcca0b6e6..2370d8669156 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -2489,6 +2489,8 @@ 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);
> +			/* Reset host interface after DMA path teardown (AMD quirk) */
> +			__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 798840b24d2a..9243073cae06 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -792,6 +792,7 @@ 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 tb_domain_disconnect_all_paths(struct tb *tb);
> +void __tb_domain_reset_interface_locked(struct tb *tb);
>   
>   static inline struct tb *tb_domain_get(struct tb *tb)
>   {


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

end of thread, other threads:[~2026-08-25 13:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-25  3:03 [PATCH] thunderbolt: Fix deadlock in tb_domain_reset_interface() during hot-unplug Martinez, Juan
2026-08-25 13:51 ` Mario Limonciello

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