public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Will Deacon <will@kernel.org>
Cc: Nicolin Chen <nicolinc@nvidia.com>,
	robin.murphy@arm.com, joro@8bytes.org,
	linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
	linux-kernel@vger.kernel.org, skolothumtho@nvidia.com,
	praan@google.com, xueshuai@linux.alibaba.com,
	smostafa@google.com
Subject: Re: [PATCH rc v5 1/4] iommu/arm-smmu-v3: Add update_safe bits to fix STE update sequence
Date: Mon, 12 Jan 2026 12:10:10 -0400	[thread overview]
Message-ID: <20260112161010.GC812923@nvidia.com> (raw)
In-Reply-To: <aWUY-f3kvM94z4qh@willie-the-truck>

On Mon, Jan 12, 2026 at 03:53:29PM +0000, Will Deacon wrote:
> On Wed, Jan 07, 2026 at 08:36:46PM -0400, Jason Gunthorpe wrote:
> > On Wed, Jan 07, 2026 at 09:20:06PM +0000, Will Deacon wrote:
> > > >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  2 ++
> > > >  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c  | 18 ++++++++++---
> > > >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 27 ++++++++++++++-----
> > > >  3 files changed, 37 insertions(+), 10 deletions(-)
> > > 
> > > Hmm. So this appears to ignore the safe bits entirely, whereas the
> > > rationale for the change is that going from {MEV,EATS} disabled to
> > > enabled is safe (which I agree with). 
> > 
> > The argument was it doesn't matter for either direction be it disabled
> > to enabled or vice versa, see my reply to Mustfa in the v4 posting:
> > 
> > https://lore.kernel.org/all/20251218180129.GA254720@nvidia.com/
> 
> It would be good to include some of that rationale in the comment and
> commit message for patch 3, as at the moment it only talks about the
> change in one direction.

Sure, I can help Nicolin with that.

> I'm also still not convinced that this is generally safe, even if it
> works within what Linux currently does. For example, if somebody tries
> to disable S2S and enable ATS at the same time, couldn't you transiently
> get an illegal STE?

I would argue that the driver will never concurrently support S2S and
ATS together for the same device, it doesn't make sense as far as I
can understand.

You are correct that there is an illegal STE issue here in this case.

However, keep in mind, if there is concurrent DMA while the driver is
trying to do such a thing there must be a STE error, and we should try
to make it be a non-valid STE error.

Still, it seems easy enough to improve, do not add EATS to the safe
bits if either the current or new STE has S2S set. That will force a
V=0 and avoid the illegal STE. Nicolin?

Thanks,
Jason

  reply	other threads:[~2026-01-12 16:10 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-18 21:41 [PATCH rc v5 0/4] iommu/arm-smmu-v3: Fix hitless STE update in nesting cases Nicolin Chen
2025-12-18 21:41 ` [PATCH rc v5 1/4] iommu/arm-smmu-v3: Add update_safe bits to fix STE update sequence Nicolin Chen
2026-01-02 18:26   ` Mostafa Saleh
2026-01-07 21:20   ` Will Deacon
2026-01-08  0:36     ` Jason Gunthorpe
2026-01-12 15:53       ` Will Deacon
2026-01-12 16:10         ` Jason Gunthorpe [this message]
2026-01-12 18:58           ` Nicolin Chen
2026-01-13 15:05             ` Will Deacon
2026-01-13 16:12               ` Jason Gunthorpe
2026-01-13 20:29                 ` Nicolin Chen
2026-01-13 20:51                   ` Jason Gunthorpe
2026-01-15 13:11                     ` Jason Gunthorpe
2026-01-15 16:25                       ` Nicolin Chen
2026-01-15 16:29                         ` Jason Gunthorpe
2026-01-15 16:34                           ` Nicolin Chen
2026-01-15 17:39                             ` Will Deacon
2025-12-18 21:41 ` [PATCH rc v5 2/4] iommu/arm-smmu-v3: Mark STE MEV safe when computing the " Nicolin Chen
2026-01-02 18:27   ` Mostafa Saleh
2025-12-18 21:41 ` [PATCH rc v5 3/4] iommu/arm-smmu-v3: Mark STE EATS " Nicolin Chen
2025-12-18 21:41 ` [PATCH rc v5 4/4] iommu/arm-smmu-v3-test: Add nested s1bypass/s1dssbypass coverage Nicolin Chen
2026-01-02 18:27   ` Mostafa Saleh

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=20260112161010.GC812923@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=iommu@lists.linux.dev \
    --cc=joro@8bytes.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nicolinc@nvidia.com \
    --cc=praan@google.com \
    --cc=robin.murphy@arm.com \
    --cc=skolothumtho@nvidia.com \
    --cc=smostafa@google.com \
    --cc=will@kernel.org \
    --cc=xueshuai@linux.alibaba.com \
    /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