[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260106142301.GS125261@ziepe.ca>
Date: Tue, 6 Jan 2026 10:23:01 -0400
From: Jason Gunthorpe <jgg@...pe.ca>
To: Dmytro Maluka <dmaluka@...omium.org>
Cc: David Woodhouse <dwmw2@...radead.org>,
Lu Baolu <baolu.lu@...ux.intel.com>, 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 v2 0/5] iommu/vt-d: Ensure memory ordering in context &
root entry updates
On Tue, Jan 06, 2026 at 02:51:38PM +0100, Dmytro Maluka wrote:
> > 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.
It is already integrated into <cmd to flush caches> I showed it here
for clarity.
> 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.
You don't know what state the HW fetcher is in. This kind of race is possible:
CPU FETCHER
read present = 1
present = 0
mangle qword 1
read qword 1
< fail - HW sees a corrupted entry >
The flush is not just a flush but a barrier to synchronize with the HW
that it is done all fetches that may have been dependent on seeing
present = 1.
So missing a flush after clearing present is possibly a bug today - I
don't remember what guarenteed the atomic size is for Intel IOMMU
though, if the atomic size is the whole entry it is OK since there is
only one fetcher read. Though AMD is 128 bits and ARM is 64 bits.
> 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?
The HW doesn't read the values the CPU is writing, so it doesn't
matter if the compiler does something strange. That is the whole
justification for why it is possible to code it like this at all.
The dma_mb() is also a compiler barrier and ensures all that
uncertainty is resolved. Once it completes a DMA from the HW will see
the program defined values only.
Jason
Powered by blists - more mailing lists