Re: [PATCH v4] thunderbolt: Fix tb->lock deadlock during hot-unplug on AMD USB4 routers

[Mario Limonciello <mario.limonciello@amd.com>]

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