[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;