[PATCH 0/2] thunderbolt: suppress reset-induced ring interrupt warning

[PATCH 0/2] thunderbolt: suppress reset-induced ring interrupt warning

[Andrei Rusu de Castro]

The AMD DMA-teardown quirk resets the host interface before USB4NET
stops its service rings. The reset clears the ring interrupt bits while
the rings remain logically active. Their later interrupt-disable write
is therefore a no-op and emits a warning, even though hardware reset is
the cause.

Reordering teardown is unsafe because stopping the ring first clears its
descriptor base and unmaps frame buffers before pending path traffic has
drained.

Patch 1 tracks the host-interface reset generation and excuses only a
redundant disable by a ring that crossed an eligible reset. Genuine
software-state drift retains the warning. Patch 2 covers duplicate
enable, duplicate disable, post-reset suppression, and real register
changes.

The exact series passed all 49 Thunderbolt UML KUnit cases and three
production configuration builds with warnings fatal. The resulting
Thunderbolt module booted on two AMD USB4 systems. The affected peer-host
XDomain teardown could not be reproduced because the available attached
device is a hub rather than a peer host.

Andrei Rusu de Castro (2):
  thunderbolt: Do not warn when a reset clears ring interrupts
  thunderbolt: Test ring interrupt warning after host reset

 drivers/thunderbolt/nhi.c   | 56 ++++++++++++++++++++++++-
 drivers/thunderbolt/nhi.h   |  4 ++
 drivers/thunderbolt/test.c  | 82 +++++++++++++++++++++++++++++++++++++
 include/linux/thunderbolt.h |  9 ++++
 4 files changed, 149 insertions(+), 2 deletions(-)

[PATCH 1/2] thunderbolt: Do not warn when a reset clears ring interrupts

[Andrei Rusu de Castro]

The AMD DMA-teardown quirk resets the host interface before USB4NET
stops its service rings. The reset clears ring interrupt bits while the
rings remain logically running. When tb_ring_stop() later disables the
interrupt, the register update is a no-op and emits a dev_WARN() splat.

The path teardown order is required. Stopping a ring first clears its
descriptor base and unmaps its frame buffers, so pending path traffic
can no longer drain and some host routers never clear their pending bit.

Keep the warning for genuine software-state drift. Increment a host
interface generation after each eligible reset and sample it when an
interrupt-backed ring starts. Excuse a redundant disable only when that
ring crossed a reset. Duplicate enables, duplicate disables without a
reset, rings started after a reset, ineligible resets, and double
software stops retain their existing warnings.

The generation sample precedes interrupt enable while holding the NHI
lock. A reset racing with ring start is therefore observed as newer than
the sample and attributed to that ring.

Source and call-graph analysis identified the reset and
ring-teardown ordering. The change was compile-tested; KUnit coverage is
added separately. It has not run on affected peer-host XDomain hardware
because the attached USB4 device is a hub and does not form that path.

Fixes: f1de1fc5f632 ("thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers")

Signed-off-by: Andrei Rusu de Castro <arc@empyreal.works>
---
 drivers/thunderbolt/nhi.c   | 54 +++++++++++++++++++++++++++++++++++--
 include/linux/thunderbolt.h |  9 +++++++
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 5809809f64d4..f56590100aef 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -68,6 +68,43 @@ static void nhi_clear_interrupt(struct tb_nhi *nhi, int ring)
 		iowrite32(~0, nhi->iobase + REG_RING_INT_CLEAR + ring);
 }
 
+/**
+ * nhi_ring_interrupt_should_warn() - Is a no-op interrupt update unexpected?
+ * @ring: Ring whose interrupt state is being updated
+ * @active: %true if the interrupt is being enabled
+ * @unchanged: %true if the register value did not change
+ *
+ * Updating the interrupt mask normally toggles a bit, so an update that
+ * changes nothing means the driver lost track of the hardware state.
+ *
+ * There is one legitimate exception. Hosts with
+ * %QUIRK_RESET_DMA_ON_TEARDOWN reset the host interface as part of
+ * tearing down a DMA path, which clears the ring interrupt bits while
+ * the rings themselves are still running. A ring that was started
+ * before such a reset is therefore expected to find its interrupt
+ * already disabled when it is stopped afterwards.
+ *
+ * Return: %true if the caller should warn about the no-op update.
+ */
+static bool
+nhi_ring_interrupt_should_warn(const struct tb_ring *ring, bool active,
+			       bool unchanged)
+{
+	if (!unchanged)
+		return false;
+
+	/* Enabling an already enabled interrupt is always a driver bug */
+	if (active)
+		return true;
+
+	/*
+	 * Only excuse a redundant disable if the host interface was reset
+	 * while this ring was running.
+	 */
+	return ring->reset_generation ==
+		atomic_read(&ring->nhi->reset_generation);
+}
+
 /*
  * ring_interrupt_active() - activate/deactivate interrupts for a single ring
  *
@@ -138,7 +175,7 @@ static void ring_interrupt_active(struct tb_ring *ring, bool active)
 		"%s interrupt at register %#x bit %d (%#x -> %#x)\n",
 		active ? "enabling" : "disabling", reg, interrupt_bit, old, new);
 
-	if (new == old)
+	if (nhi_ring_interrupt_should_warn(ring, active, new == old))
 		dev_WARN(ring->nhi->dev, "interrupt for %s %d is already %s\n",
 			 RING_TYPE(ring), ring->hop,
 			 str_enabled_disabled(active));
@@ -714,8 +751,15 @@ void tb_ring_start(struct tb_ring *ring)
 		ring_iowrite32options(ring, flags, 0);
 	}
 
-	if (!(ring->flags & RING_FLAG_NO_INTERRUPT))
+	/*
+	 * Sample the reset generation before touching the interrupt so
+	 * that a reset racing with this start is seen as happening after
+	 * the ring started, and the eventual stop does not warn.
+	 */
+	if (!(ring->flags & RING_FLAG_NO_INTERRUPT)) {
+		ring->reset_generation = atomic_read(&ring->nhi->reset_generation);
 		ring_interrupt_active(ring, true);
+	}
 	ring->running = true;
 err:
 	spin_unlock(&ring->lock);
@@ -1199,6 +1243,12 @@ void nhi_reset_interface(struct tb_nhi *nhi)
 		  nhi->iobase + REG_HOST_INTERFACE_RESET);
 	/* Wait for tHIReset (10 ms) to complete */
 	usleep_range(10000, 20000);
+
+	/*
+	 * The reset cleared the ring interrupt state behind the back of
+	 * any ring that is still running, so record that it happened.
+	 */
+	atomic_inc(&nhi->reset_generation);
 }
 
 static struct tb *nhi_select_cm(struct tb_nhi *nhi)
diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h
index d48623fda79b..867ec3ed23c6 100644
--- a/include/linux/thunderbolt.h
+++ b/include/linux/thunderbolt.h
@@ -19,6 +19,7 @@ struct device;
 
 #if IS_REACHABLE(CONFIG_USB4)
 
+#include <linux/atomic.h>
 #include <linux/device.h>
 #include <linux/idr.h>
 #include <linux/list.h>
@@ -519,6 +520,10 @@ void tb_service_properties_changed(struct tb_service *svc);
  *		downstream ports to signal disconnect before tearing down the
  *		router tree. Only Thunderbolt 3 devices are reset; USB4
  *		routers are skipped.
+ * @reset_generation: Incremented every time the host interface is reset by
+ *		      nhi_reset_interface(). Rings sample this when they are
+ *		      started so that they can tell whether their interrupt
+ *		      state was cleared by a reset while they were running.
  */
 struct tb_nhi {
 	spinlock_t lock;
@@ -534,6 +539,7 @@ struct tb_nhi {
 	unsigned long quirks;
 	struct completion domain_released;
 	bool host_reset;
+	atomic_t reset_generation;
 };
 
 /**
@@ -552,6 +558,8 @@ struct tb_nhi {
  * @work: Interrupt work structure
  * @is_tx: Is the ring Tx or Rx
  * @running: Is the ring running
+ * @reset_generation: Host interface reset generation sampled when the ring
+ *		      was started. Protected by the NHI lock.
  * @irq: MSI-X irq number if the ring uses MSI-X. %0 otherwise.
  * @vector: MSI-X vector number the ring uses (only set if @irq is > 0)
  * @flags: Ring specific flags
@@ -580,6 +588,7 @@ struct tb_ring {
 	struct work_struct work;
 	bool is_tx:1;
 	bool running:1;
+	int reset_generation;
 	int irq;
 	u8 vector;
 	unsigned int flags;

Re: [PATCH 1/2] thunderbolt: Do not warn when a reset clears ring interrupts

[Mika Westerberg]

+Mario

Hi,

On Wed, Sep 02, 2026 at 12:34:13PM +0000, Andrei Rusu de Castro wrote:
> The AMD DMA-teardown quirk resets the host interface before USB4NET
> stops its service rings. The reset clears ring interrupt bits while the
> rings remain logically running. When tb_ring_stop() later disables the
> interrupt, the register update is a no-op and emits a dev_WARN() splat.

Yes it should not do that. It's too "big hammer" and we should avoid that
if possible. There is also the deadlock that resulted this series:

https://lore.kernel.org/linux-usb/20260825214237.4179813-1-juan.martinez@amd.com/

But that still kills the whole host interface if there are other users,
like USB4STREAM using the rings at the same time. I suggested that we do
the reset when the rings are idle and while they are not and we have spare
rings we hand off those instead:

https://lore.kernel.org/linux-usb/20260902054800.GI106095@black.igk.intel.com/

We still need confirmation from AMD if this even solves the problem or is
it hanging the whole host interface and not just a single ring.

> The path teardown order is required. Stopping a ring first clears its
> descriptor base and unmaps its frame buffers, so pending path traffic
> can no longer drain and some host routers never clear their pending bit.
> 
> Keep the warning for genuine software-state drift. Increment a host
> interface generation after each eligible reset and sample it when an
> interrupt-backed ring starts. Excuse a redundant disable only when that
> ring crossed a reset. Duplicate enables, duplicate disables without a
> reset, rings started after a reset, ineligible resets, and double
> software stops retain their existing warnings.
> 
> The generation sample precedes interrupt enable while holding the NHI
> lock. A reset racing with ring start is therefore observed as newer than
> the sample and attributed to that ring.
> 
> Source and call-graph analysis identified the reset and
> ring-teardown ordering. The change was compile-tested; KUnit coverage is
> added separately. It has not run on affected peer-host XDomain hardware
> because the attached USB4 device is a hub and does not form that path.

This looks pretty much like LLM generated so if that's the case you should
add proper assisted-by.

Anyways I don't think we want to do this just yet if we can avoid resetting
the host interace behind everyones back.

Re: [PATCH 1/2] thunderbolt: Do not warn when a reset clears ring interrupts

[Mario Limonciello]

On 9/2/26 07:50, Mika Westerberg wrote:
> +Mario
> 
> Hi,
> 
> On Wed, Sep 02, 2026 at 12:34:13PM +0000, Andrei Rusu de Castro wrote:
>> The AMD DMA-teardown quirk resets the host interface before USB4NET
>> stops its service rings. The reset clears ring interrupt bits while the
>> rings remain logically running. When tb_ring_stop() later disables the
>> interrupt, the register update is a no-op and emits a dev_WARN() splat.
> 
> Yes it should not do that. It's too "big hammer" and we should avoid that
> if possible. There is also the deadlock that resulted this series:
> 
> https://lore.kernel.org/linux-usb/20260825214237.4179813-1-juan.martinez@amd.com/
> 
> But that still kills the whole host interface if there are other users,
> like USB4STREAM using the rings at the same time. I suggested that we do
> the reset when the rings are idle and while they are not and we have spare
> rings we hand off those instead:
> 
> https://lore.kernel.org/linux-usb/20260902054800.GI106095@black.igk.intel.com/
> 
> We still need confirmation from AMD if this even solves the problem or is
> it hanging the whole host interface and not just a single ring.

I'll let Sanath and Basavaraj double check this on the affected failure 
case.

I believe think that the whole host interface hangs when this condition 
happens.  Another way to mitigate it can be to force a power state 
transition though.  If we can force the router into D3 and back out it 
should reset the condition that could lead to a host interface hang.

> 
>> The path teardown order is required. Stopping a ring first clears its
>> descriptor base and unmaps its frame buffers, so pending path traffic
>> can no longer drain and some host routers never clear their pending bit.
>>
>> Keep the warning for genuine software-state drift. Increment a host
>> interface generation after each eligible reset and sample it when an
>> interrupt-backed ring starts. Excuse a redundant disable only when that
>> ring crossed a reset. Duplicate enables, duplicate disables without a
>> reset, rings started after a reset, ineligible resets, and double
>> software stops retain their existing warnings.
>>
>> The generation sample precedes interrupt enable while holding the NHI
>> lock. A reset racing with ring start is therefore observed as newer than
>> the sample and attributed to that ring.
>>
>> Source and call-graph analysis identified the reset and
>> ring-teardown ordering. The change was compile-tested; KUnit coverage is
>> added separately. It has not run on affected peer-host XDomain hardware
>> because the attached USB4 device is a hub and does not form that path.
> 
> This looks pretty much like LLM generated so if that's the case you should
> add proper assisted-by.
> 
> Anyways I don't think we want to do this just yet if we can avoid resetting
> the host interace behind everyones back.

The resetting host interface /should/ only really happen when unplugging 
the cable.  If it's happening in more cases, that's not intended at least.

Re: [PATCH 1/2] thunderbolt: Do not warn when a reset clears ring interrupts

[Mika Westerberg]

Hi,

On Wed, Sep 02, 2026 at 03:53:42PM -0500, Mario Limonciello wrote:
> 
> 
> On 9/2/26 07:50, Mika Westerberg wrote:
> > +Mario
> > 
> > Hi,
> > 
> > On Wed, Sep 02, 2026 at 12:34:13PM +0000, Andrei Rusu de Castro wrote:
> > > The AMD DMA-teardown quirk resets the host interface before USB4NET
> > > stops its service rings. The reset clears ring interrupt bits while the
> > > rings remain logically running. When tb_ring_stop() later disables the
> > > interrupt, the register update is a no-op and emits a dev_WARN() splat.
> > 
> > Yes it should not do that. It's too "big hammer" and we should avoid that
> > if possible. There is also the deadlock that resulted this series:
> > 
> > https://lore.kernel.org/linux-usb/20260825214237.4179813-1-juan.martinez@amd.com/
> > 
> > But that still kills the whole host interface if there are other users,
> > like USB4STREAM using the rings at the same time. I suggested that we do
> > the reset when the rings are idle and while they are not and we have spare
> > rings we hand off those instead:
> > 
> > https://lore.kernel.org/linux-usb/20260902054800.GI106095@black.igk.intel.com/
> > 
> > We still need confirmation from AMD if this even solves the problem or is
> > it hanging the whole host interface and not just a single ring.
> 
> I'll let Sanath and Basavaraj double check this on the affected failure
> case.

Okay thanks.

> I believe think that the whole host interface hangs when this condition
> happens.  Another way to mitigate it can be to force a power state
> transition though.  If we can force the router into D3 and back out it
> should reset the condition that could lead to a host interface hang.

I don't think that's any better that the reset.

> > > The path teardown order is required. Stopping a ring first clears its
> > > descriptor base and unmaps its frame buffers, so pending path traffic
> > > can no longer drain and some host routers never clear their pending bit.
> > > 
> > > Keep the warning for genuine software-state drift. Increment a host
> > > interface generation after each eligible reset and sample it when an
> > > interrupt-backed ring starts. Excuse a redundant disable only when that
> > > ring crossed a reset. Duplicate enables, duplicate disables without a
> > > reset, rings started after a reset, ineligible resets, and double
> > > software stops retain their existing warnings.
> > > 
> > > The generation sample precedes interrupt enable while holding the NHI
> > > lock. A reset racing with ring start is therefore observed as newer than
> > > the sample and attributed to that ring.
> > > 
> > > Source and call-graph analysis identified the reset and
> > > ring-teardown ordering. The change was compile-tested; KUnit coverage is
> > > added separately. It has not run on affected peer-host XDomain hardware
> > > because the attached USB4 device is a hub and does not form that path.
> > 
> > This looks pretty much like LLM generated so if that's the case you should
> > add proper assisted-by.
> > 
> > Anyways I don't think we want to do this just yet if we can avoid resetting
> > the host interace behind everyones back.
> 
> The resetting host interface /should/ only really happen when unplugging the
> cable.  If it's happening in more cases, that's not intended at least.

The XDomain paths can be brought down also without unplug. Networking does
that when you "down" the interface and USB4STREAM does that when you close
the device node.

[PATCH 2/2] thunderbolt: Test ring interrupt warning after host reset

[Andrei Rusu de Castro]

Cover the warning decision independently of MMIO by constructing a ring
and NHI generation pair. Verify that duplicate enables always warn,
duplicate disables without an intervening reset warn, a disable after a
reset does not warn, and an update that changed the register never
warns.

Expose the predicate only in KUnit builds through VISIBLE_IF_KUNIT; it
remains private in production builds and is not exported outside the
Thunderbolt module.

The cases were verified under UML KUnit.

Signed-off-by: Andrei Rusu de Castro <arc@empyreal.works>
---
 drivers/thunderbolt/nhi.c  |  4 +-
 drivers/thunderbolt/nhi.h  |  4 ++
 drivers/thunderbolt/test.c | 82 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index f56590100aef..d768a84adaab 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -21,6 +21,8 @@
 #include <linux/string_choices.h>
 #include <linux/string_helpers.h>
 
+#include <kunit/visibility.h>
+
 #include "nhi.h"
 #include "nhi_regs.h"
 #include "tb.h"
@@ -86,7 +88,7 @@ static void nhi_clear_interrupt(struct tb_nhi *nhi, int ring)
  *
  * Return: %true if the caller should warn about the no-op update.
  */
-static bool
+VISIBLE_IF_KUNIT bool
 nhi_ring_interrupt_should_warn(const struct tb_ring *ring, bool active,
 			       bool unchanged)
 {
diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
index f72d6b274501..393bd831375f 100644
--- a/drivers/thunderbolt/nhi.h
+++ b/drivers/thunderbolt/nhi.h
@@ -37,6 +37,10 @@ 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);
+#if IS_ENABLED(CONFIG_KUNIT)
+bool nhi_ring_interrupt_should_warn(const struct tb_ring *ring, bool active,
+				    bool unchanged);
+#endif
 
 extern const struct dev_pm_ops nhi_pm_ops;
 
diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index 05652ee82fbf..3ccdd967396b 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -9,6 +9,7 @@
 #include <kunit/test.h>
 #include <linux/idr.h>
 
+#include "nhi.h"
 #include "tb.h"
 #include "tunnel.h"
 
@@ -3095,6 +3096,83 @@ static void tb_test_property_merge(struct kunit *test)
 	tb_property_free_dir(dir1);
 }
 
+static struct tb_ring *alloc_interrupt_test_ring(struct kunit *test,
+						 int nhi_generation,
+						 int ring_generation)
+{
+	struct tb_nhi *nhi;
+	struct tb_ring *ring;
+
+	nhi = kunit_kzalloc(test, sizeof(*nhi), GFP_KERNEL);
+	if (!nhi)
+		return NULL;
+
+	ring = kunit_kzalloc(test, sizeof(*ring), GFP_KERNEL);
+	if (!ring)
+		return NULL;
+
+	ring->nhi = nhi;
+	atomic_set(&nhi->reset_generation, nhi_generation);
+	ring->reset_generation = ring_generation;
+
+	return ring;
+}
+
+static void tb_test_ring_interrupt_warn_duplicate_enable(struct kunit *test)
+{
+	struct tb_ring *ring;
+
+	/* Enabling an already enabled interrupt is always a driver bug */
+	ring = alloc_interrupt_test_ring(test, 7, 7);
+	KUNIT_ASSERT_NOT_NULL(test, ring);
+	KUNIT_EXPECT_TRUE(test, nhi_ring_interrupt_should_warn(ring, true, true));
+
+	/* Including when the host interface was reset in between */
+	ring = alloc_interrupt_test_ring(test, 8, 7);
+	KUNIT_ASSERT_NOT_NULL(test, ring);
+	KUNIT_EXPECT_TRUE(test, nhi_ring_interrupt_should_warn(ring, true, true));
+}
+
+static void tb_test_ring_interrupt_warn_duplicate_disable(struct kunit *test)
+{
+	struct tb_ring *ring;
+
+	/*
+	 * No reset happened while this ring was running, so a redundant
+	 * disable means the driver lost track of the hardware state.
+	 */
+	ring = alloc_interrupt_test_ring(test, 7, 7);
+	KUNIT_ASSERT_NOT_NULL(test, ring);
+	KUNIT_EXPECT_TRUE(test, nhi_ring_interrupt_should_warn(ring, false, true));
+}
+
+static void tb_test_ring_interrupt_no_warn_after_reset(struct kunit *test)
+{
+	struct tb_ring *ring;
+
+	/*
+	 * The ring was started before the host interface was reset, which
+	 * cleared the ring interrupt bit underneath it.
+	 */
+	ring = alloc_interrupt_test_ring(test, 8, 7);
+	KUNIT_ASSERT_NOT_NULL(test, ring);
+	KUNIT_EXPECT_FALSE(test,
+			   nhi_ring_interrupt_should_warn(ring, false, true));
+}
+
+static void tb_test_ring_interrupt_no_warn_when_changed(struct kunit *test)
+{
+	struct tb_ring *ring;
+
+	/* An update that actually changed the register never warns */
+	ring = alloc_interrupt_test_ring(test, 8, 7);
+	KUNIT_ASSERT_NOT_NULL(test, ring);
+	KUNIT_EXPECT_FALSE(test,
+			   nhi_ring_interrupt_should_warn(ring, false, false));
+	KUNIT_EXPECT_FALSE(test,
+			   nhi_ring_interrupt_should_warn(ring, true, false));
+}
+
 static struct kunit_case tb_test_cases[] = {
 	KUNIT_CASE(tb_test_property_parse_u32_wrap),
 	KUNIT_CASE(tb_test_property_parse_recursion),
@@ -3141,6 +3219,10 @@ static struct kunit_case tb_test_cases[] = {
 	KUNIT_CASE(tb_test_property_parse_zero_length),
 	KUNIT_CASE(tb_test_property_parse_rootdir_overflow),
 	KUNIT_CASE(tb_test_property_merge),
+	KUNIT_CASE(tb_test_ring_interrupt_warn_duplicate_enable),
+	KUNIT_CASE(tb_test_ring_interrupt_warn_duplicate_disable),
+	KUNIT_CASE(tb_test_ring_interrupt_no_warn_after_reset),
+	KUNIT_CASE(tb_test_ring_interrupt_no_warn_when_changed),
 	{ }
 };