Linux USB
 help / color / mirror / Atom feed
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Mario Limonciello <mario.limonciello@amd.com>
Cc: "S, Sanath" <sanath.s@amd.com>,
	"Natikar, Basavaraj" <basavaraj.natikar@amd.com>,
	linux-usb@vger.kernel.org, andreas.noever@gmail.com,
	westeri@kernel.org, YehezkelShB@gmail.com,
	linux-kernel@vger.kernel.org,
	Andrei Rusu de Castro <arc@empyreal.works>
Subject: Re: [PATCH 1/2] thunderbolt: Do not warn when a reset clears ring interrupts
Date: Thu, 3 Sep 2026 05:44:15 +0200	[thread overview]
Message-ID: <20260903034415.GL106095@black.igk.intel.com> (raw)
In-Reply-To: <318bc76c-2fc9-484f-b009-1f21fbdd4d49@amd.com>

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.

  reply	other threads:[~2026-09-03  3:44 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-02 12:33 [PATCH 0/2] thunderbolt: suppress reset-induced ring interrupt warning Andrei Rusu de Castro
2026-09-02 12:34 ` [PATCH 1/2] thunderbolt: Do not warn when a reset clears ring interrupts Andrei Rusu de Castro
2026-09-02 12:50   ` Mika Westerberg
2026-09-02 20:53     ` Mario Limonciello
2026-09-03  3:44       ` Mika Westerberg [this message]
2026-09-02 12:34 ` [PATCH 2/2] thunderbolt: Test ring interrupt warning after host reset Andrei Rusu de Castro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260903034415.GL106095@black.igk.intel.com \
    --to=mika.westerberg@linux.intel.com \
    --cc=YehezkelShB@gmail.com \
    --cc=andreas.noever@gmail.com \
    --cc=arc@empyreal.works \
    --cc=basavaraj.natikar@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=sanath.s@amd.com \
    --cc=westeri@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox