From nobody Sat Sep 26 10:02:06 2026 Received: from mail-4320.protonmail.ch (mail-4320.protonmail.ch [185.70.43.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B54BA423EB7; Wed, 2 Sep 2026 12:34:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.70.43.20 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788352462; cv=none; b=XXF/hasESupnGMdziuR91kjmO9vcuGzsInFlL0cquXGd/pAvN2IOg/X25Zgsxr+24sJtOLR6MYZZZij40v79BxM2mzcy3KEQGxtm/eKL7kX1FKih1ahgA0X9/rVV/3R02aXVXFlVx90aAw064GJGVNc/G5C+2mgzviQyJ/ja9jE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788352462; c=relaxed/simple; bh=EnS8XHoKStV+UB3f7yMKvWbW/AEdLoq6tsL64/OeUuQ=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=iePqQKIyXSRBYjBoljmph1c3OEG7e2gNd9zgnt4IjmD0dW+ncDbpBXVb51BiyP/FYLet1EUOxrTBvyCARHRGVFO6Zy4sl0JSGVBGnr5PrFbzSGX3OV8dmsQh3gArF+sjMCqC6gImwZUlRK6yecSzsnvQkciR+epd7KSpWgBI9dU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=empyreal.works; spf=pass smtp.mailfrom=empyreal.works; dkim=pass (2048-bit key) header.d=empyreal.works header.i=@empyreal.works header.b=JcDc6uhS; arc=none smtp.client-ip=185.70.43.20 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=empyreal.works Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=empyreal.works Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=empyreal.works header.i=@empyreal.works header.b="JcDc6uhS" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=empyreal.works; s=protonmail2; t=1788352458; x=1788611658; bh=mzPf5ChutrVKbDcFh8jVvjHPWsZSdk4VjFdxnOWMu+w=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=JcDc6uhSLmKQKA9PROvQprV7aWEHXEXXrmw1O19qtJGlIkLZJltbPdDueWYkgXdTt NmA51RdeofZTtAqxBhlDVzWPAB3ubnl0FouwfZBHCqKA/koziCnV/kR6zVT3sGnAxV QQNp1LmtpGFeCHNkZ56JNbm+eMQ3+6N49k3lsmYGreEP233QueITjbRFkEzhs2ZM4d 5FYTg7ynBYAToPDw06tC8zjjGeKKV1fVCckS+IAqGW7tKfcqBF7CWVtxYU0B9ihf9C +jipQtv08oH44snaMXWKK02cQDYUn4RxAsPia9C1AhXpZiF+lqD1wpx1Cnhc58yK7s COKGJIZuAF2Hg== Date: Wed, 02 Sep 2026 12:34:13 +0000 To: linux-usb@vger.kernel.org From: Andrei Rusu de Castro Cc: andreas.noever@gmail.com, westeri@kernel.org, YehezkelShB@gmail.com, Sanath.S@amd.com, Basavaraj.Natikar@amd.com, linux-kernel@vger.kernel.org Subject: [PATCH 1/2] thunderbolt: Do not warn when a reset clears ring interrupts Message-ID: <20260902-thunderbolt-1-c48cd6e7@empyreal.works> In-Reply-To: <20260902-thunderbolt-cover-2fdc1c1b@empyreal.works> References: <20260902-thunderbolt-cover-2fdc1c1b@empyreal.works> Feedback-ID: 182420409:user:proton X-Pm-Message-ID: da0ca1ae6d04d10dff90df9b6474a74d1812a5d8 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" 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 --- 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); } =20 +/** + * nhi_ring_interrupt_should_warn() - Is a no-op interrupt update unexpect= ed? + * @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 =3D=3D + atomic_read(&ring->nhi->reset_generation); +} + /* * ring_interrupt_active() - activate/deactivate interrupts for a single r= ing * @@ -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); =20 - if (new =3D=3D old) + if (nhi_ring_interrupt_should_warn(ring, active, new =3D=3D 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); } =20 - 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 =3D atomic_read(&ring->nhi->reset_generation); ring_interrupt_active(ring, true); + } ring->running =3D 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); } =20 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; =20 #if IS_REACHABLE(CONFIG_USB4) =20 +#include #include #include #include @@ -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; }; =20 /** @@ -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; From nobody Sat Sep 26 10:02:06 2026 Received: from mail-10625.protonmail.ch (mail-10625.protonmail.ch [79.135.106.25]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 036AD48A8C4 for ; Wed, 2 Sep 2026 12:34:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=79.135.106.25 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788352493; cv=none; b=DBZSS2xy1098ppX9Ja75LwZNLHmk/ATS0rnzuYaqQKyHubhA54CI9uZzOwl4cv3Ayv93cZHxogY+00xc9FYCjgIuSuBwoXGnmd8DgMZsUrYBpy06N68jKPfWUU8ZUwLL67yovYGRzaKH7rbDefG1ALwTT/EvO0us6/h0erSa4Yg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788352493; c=relaxed/simple; bh=bLm0ySQOMsgpTTTaqr5rs3M4eKG7B8qeNk91g7KG1Vc=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=BvuKDZ/C3q/mt69+iOQYcB9+z2QeQRKt2AFNkR06N7ghcbkcfAXhDFSzXhjuRyVUXljV4HFGm24atD1+jkvelpyqO997efNN0OrHjgbacsduikxnh3j5/o0aLmZhA4z/1HJ61E2p6P7H0wzItVQJLK53+mm++3Zn62tFuU+ELPQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=empyreal.works; spf=pass smtp.mailfrom=empyreal.works; dkim=pass (2048-bit key) header.d=empyreal.works header.i=@empyreal.works header.b=YxagJdIo; arc=none smtp.client-ip=79.135.106.25 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=empyreal.works Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=empyreal.works Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=empyreal.works header.i=@empyreal.works header.b="YxagJdIo" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=empyreal.works; s=protonmail2; t=1788352488; x=1788611688; bh=9Np8DWLpuQ1rRZUMmMGF6LiNN8JJ6X6W5GuPnYtHZqI=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=YxagJdIoRy72LJmBqMaPxu3Cs9M2sFB51VyRBp4aY1vW49EfxuKiCaRyHiPW2rqS7 V6v3dxMgisetHy06ygGQQV5uAxcnlT1CE4rfukHrFNMApwzUwe5UqWI1vwqdlXKi6T pZcNUCxWsdxHbUkeoFoBggSLR3DO65smvTU/45Tt1rpikGES8zfsd+lba9gCj/GGp0 CdUqAbl2nRV9GcuKPyTfLZFnbIMC8xfylLTLrAYB6Rf2RGoQO3KaLvv16i1fOJjRNy tElasYuHQ/72Xld+mf8u2V2MP9SVZcav+w/fMxsZWkcpLmLhj1nXdK1zzuknAUPfts 6hSPBrFJWfR6w== Date: Wed, 02 Sep 2026 12:34:43 +0000 To: linux-usb@vger.kernel.org From: Andrei Rusu de Castro Cc: andreas.noever@gmail.com, westeri@kernel.org, YehezkelShB@gmail.com, Sanath.S@amd.com, Basavaraj.Natikar@amd.com, linux-kernel@vger.kernel.org Subject: [PATCH 2/2] thunderbolt: Test ring interrupt warning after host reset Message-ID: <20260902-thunderbolt-2-2fdc1c1b@empyreal.works> In-Reply-To: <20260902-thunderbolt-cover-2fdc1c1b@empyreal.works> References: <20260902-thunderbolt-cover-2fdc1c1b@empyreal.works> Feedback-ID: 182420409:user:proton X-Pm-Message-ID: 1d297b7de060c95bc1fcd1bc78dba9c3d1aa0054 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" 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 --- 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 #include =20 +#include + #include "nhi.h" #include "nhi_regs.h" #include "tb.h" @@ -86,7 +88,7 @@ static void nhi_clear_interrupt(struct tb_nhi *nhi, int r= ing) * * 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 activ= e, + bool unchanged); +#endif =20 extern const struct dev_pm_ops nhi_pm_ops; =20 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 #include =20 +#include "nhi.h" #include "tb.h" #include "tunnel.h" =20 @@ -3095,6 +3096,83 @@ static void tb_test_property_merge(struct kunit *tes= t) tb_property_free_dir(dir1); } =20 +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 =3D kunit_kzalloc(test, sizeof(*nhi), GFP_KERNEL); + if (!nhi) + return NULL; + + ring =3D kunit_kzalloc(test, sizeof(*ring), GFP_KERNEL); + if (!ring) + return NULL; + + ring->nhi =3D nhi; + atomic_set(&nhi->reset_generation, nhi_generation); + ring->reset_generation =3D ring_generation; + + return ring; +} + +static void tb_test_ring_interrupt_warn_duplicate_enable(struct kunit *tes= t) +{ + struct tb_ring *ring; + + /* Enabling an already enabled interrupt is always a driver bug */ + ring =3D 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 =3D 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 *te= st) +{ + 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 =3D 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 =3D 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 =3D 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[] =3D { 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[] =3D { 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), { } }; =20