mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dmytro Maluka <dmaluka@chromium.org>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: David Woodhouse <dwmw2@infradead.org>,
	Lu Baolu <baolu.lu@linux.intel.com>,
	iommu@lists.linux.dev, Joerg Roedel <joro@8bytes.org>,
	Will Deacon <will@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	linux-kernel@vger.kernel.org,
	"Vineeth Pillai (Google)" <vineeth@bitbyteword.org>,
	Aashish Sharma <aashish@aashishsharma.net>,
	Grzegorz Jaszczyk <jaszczyk@chromium.org>,
	Chuanxiao Dong <chuanxiao.dong@intel.com>,
	Kevin Tian <kevin.tian@intel.com>
Subject: Re: [PATCH v2 0/5] iommu/vt-d: Ensure memory ordering in context & root entry updates
Date: Tue, 6 Jan 2026 14:51:38 +0100	[thread overview]
Message-ID: <aV0TalseaDGW-1Jp@mitya-t14-2025> (raw)
In-Reply-To: <20260106001418.GK125261@ziepe.ca>

On Mon, Jan 05, 2026 at 08:14:18PM -0400, Jason Gunthorpe wrote:
> On Mon, Jan 05, 2026 at 09:05:36PM +0100, Dmytro Maluka wrote:
> > > and WRITE_ONCE is pointless if the HW is promising not to
> > > read it due to non-present.
> > 
> > As long as we use a barrier. And IIUC vice versa, if we use WRITE_ONCE
> > for any updates, a barrier is not necessary (on x86). And WRITE_ONCE for
> > any updates (for PASID entries) is what was already done before this
> > series.
> 
> That is an x86 ism and it shouldn't be needlessly leaked into drivers.
> As this is not performance it should have the portable flow:

Agree. And as a matter of fact, patch 4 in this series already does that
- it adds a barrier before setting present bit, despite it being
"redundant on x86".

It adds smp_wmb() as suggested by Baolu but I can change it to dma_wmb()
which indeed seems more suitable. (FWIW on x86 there is no difference
between smp_wmb() and dma_wmb(), both are just barrier().)

>    WRITE_ONCE(non-present)
>    dma_wmb()
>    <cmd to flush caches>

Regarding a barrier after clearing present bit - good point, I should
probably add that to my patch 4 as well.

Regarding flushing caches right after that - what for? (BTW the Intel
driver doesn't do that either.) If we don't do that and as a result the
HW is using an old entry cached before we cleared the present bit, it
is not affected by our later modifications anyway.

Of course we flush caches at the end, after setting the present bit, but
that's a different story.

> > Although, perhaps even with a barrier, WRITE_ONCE is still desirable to
> > prevent the compiler from doing strange but legal things that involve
> > transiently setting and then clearing the present bit behind our back?
> > (Not that I'm aware of any compilers doing that, but I'm no compiler
> > expert.)
> 
> You do have to write once the present bit, but not the others. The
> dma_wmb() will make sure the HW cannot observe other states when it
> observes present.

I was talking about compiler guarantees, not HW guarantees. I mean: when
setting some other bit in the entry before the barrier, if we do that
without WRITE_ONCE, with a mere "foo |= bar", are we certain the
compiler will not implement that as, for example, setting the value to
0xffffffffffffffff and then clearing other bits (for whatever crazy
reason)? That would be still a legal thing for the compiler to do, in
terms of its single-thread guarantees?

So it still seems a good idea to use WRITE_ONCE (or anything not weaker
than WRITE_ONCE) for any updates of memory structures used by hardware,
to prevent such things? (And using WRITE_ONCE for that doesn't really
cost us anything anyway.)

  parent reply	other threads:[~2026-01-06 13:51 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-27 17:57 Dmytro Maluka
2025-12-27 17:57 ` [PATCH v2 1/5] iommu/vt-d: Sanitize set bits in pasid_set_bits() Dmytro Maluka
2025-12-27 17:57 ` [PATCH v2 2/5] iommu/vt-d: Generalize pasid_set_bits() Dmytro Maluka
2025-12-27 17:57 ` [PATCH v2 3/5] iommu/vt-d: Ensure memory ordering in context entry updates Dmytro Maluka
2025-12-27 17:57 ` [PATCH v2 4/5] iommu/vt-d: Use smp_wmb() before setting context/pasid present bit Dmytro Maluka
2025-12-27 17:57 ` [PATCH v2 5/5] iommu/vt-d: Use WRITE_ONCE for setting root table entries Dmytro Maluka
2026-01-05 18:12 ` [PATCH v2 0/5] iommu/vt-d: Ensure memory ordering in context & root entry updates Jason Gunthorpe
2026-01-05 18:54   ` Dmytro Maluka
2026-01-05 19:14     ` Jason Gunthorpe
2026-01-05 20:05       ` Dmytro Maluka
2026-01-06  0:14         ` Jason Gunthorpe
2026-01-06  7:48           ` Tian, Kevin
2026-01-06 14:40             ` Dmytro Maluka
2026-01-08  2:22               ` Tian, Kevin
2026-01-06 13:51           ` Dmytro Maluka [this message]
2026-01-06 14:23             ` Jason Gunthorpe
2026-01-06 15:50               ` Dmytro Maluka
2026-01-06 16:45                 ` Jason Gunthorpe
2026-01-06 17:14                   ` Dmytro Maluka
2026-01-08  2:09                 ` Tian, Kevin
2026-01-09  6:32                   ` Baolu Lu
     [not found]                 ` <BN9PR11MB5276FB0F465DBFB4EE4D742B8C85A@BN9PR11MB5276.namprd11.prod.outlook.com>
2026-01-08  7:00                   ` Tian, Kevin
2026-01-06  3:37   ` Baolu Lu

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=aV0TalseaDGW-1Jp@mitya-t14-2025 \
    --to=dmaluka@chromium.org \
    --cc=aashish@aashishsharma.net \
    --cc=baolu.lu@linux.intel.com \
    --cc=chuanxiao.dong@intel.com \
    --cc=dwmw2@infradead.org \
    --cc=iommu@lists.linux.dev \
    --cc=jaszczyk@chromium.org \
    --cc=jgg@ziepe.ca \
    --cc=joro@8bytes.org \
    --cc=kevin.tian@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=vineeth@bitbyteword.org \
    --cc=will@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

all inboxes | Powered by JetHome®