linux-kernel - Re: [PATCH 1/2] iommu/vt-d: Ensure memory ordering in context entry updates

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aVAgC81PMN0gA32r@mitya-t14-2025>
Date: Sat, 27 Dec 2025 19:06:03 +0100
From: Dmytro Maluka <dmaluka@...omium.org>
To: Baolu Lu <baolu.lu@...ux.intel.com>
Cc: David Woodhouse <dwmw2@...radead.org>, iommu@...ts.linux.dev,
	Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
	Robin Murphy <robin.murphy@....com>, linux-kernel@...r.kernel.org,
	"Vineeth Pillai (Google)" <vineeth@...byteword.org>,
	Aashish Sharma <aashish@...hishsharma.net>,
	Grzegorz Jaszczyk <jaszczyk@...omium.org>,
	Chuanxiao Dong <chuanxiao.dong@...el.com>,
	Kevin Tian <kevin.tian@...el.com>
Subject: Re: [PATCH 1/2] iommu/vt-d: Ensure memory ordering in context entry
 updates

On Tue, Dec 23, 2025 at 02:10:45PM +0800, Baolu Lu wrote:
> On 12/21/25 21:11, Dmytro Maluka wrote:
> > On Sun, Dec 21, 2025 at 05:04:27PM +0800, Baolu Lu wrote:
> > > On 12/21/25 09:43, Dmytro Maluka wrote:
> > > >    static inline void context_set_present(struct context_entry *context)
> > > >    {
> > > > -	context->lo |= 1;
> > > > +	context_set_bits(&context->lo, 1 << 0, 1);
> > > >    }
> > > 
> > > How about adding a smp_wmb() before setting the present bit? Maybe it's
> > > unnecessary for x86 architecture, but at least it's harmless and more
> > > readable. Or not?
> > > 
> > > static inline void context_set_present(struct context_entry *context)
> > > {
> > > 	smp_wmb();
> > > 	context_set_bits(&context->lo, 1ULL << 0, 1ULL);
> > > }
> > 
> > Maybe... And if so, then in pasid_set_present() as well?
> > 
> > Actually this would make a slight behavioral difference even on x86:
> > right now this patch only provides ordering of context entry updates
> > between each other, while smp_wmb() would add a full compiler barrier
> > here.
> > 
> > So this barrier may be redundant as long as we always use these
> > context_*() helpers (and thus always use WRITE_ONCE) for any updates of
> > context entries. On the other hand, it might make it more robust if we
> > still occasionally do that without WRITE_ONCE, for example in
> > context_entry_set_pasid_table() which I also changed to use WRITE_ONCE
> > in this patch.
> 
> Fair point. It is somewhat redundant. However, I would suggest adding a
> comment there. For example:
> 
> static inline void context_set_present(struct context_entry *context)
> {
>  	/*
> 	 * smp_wmb() is unnecessary here because x86 hardware naturally
> 	 * keeps writes in order and all context entry modifications
> 	 * use WRITE_ONCE().
> 	 */
>  	context_set_bits(&context->lo, 1ULL << 0, 1ULL);
> }
> 
> So that people are still aware of this when porting this driver to
> platforms with a weak memory model.

What I actually had in mind was to add smp_wmb() here anyway. It might
be useful as an extra safety measure in case we still forget to use
WRITE_ONCE in some cases, and its performance cost would be virtually
non-existent, I guess. Done in v2.