[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BN9PR11MB5276EFACF8AA21215568550F8C8FA@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Wed, 14 Jan 2026 07:54:10 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Lu Baolu <baolu.lu@...ux.intel.com>, Joerg Roedel <joro@...tes.org>, "Will
Deacon" <will@...nel.org>, Robin Murphy <robin.murphy@....com>, "Jason
Gunthorpe" <jgg@...dia.com>
CC: Dmytro Maluka <dmaluka@...omium.org>, Samiullah Khawaja
<skhawaja@...gle.com>, "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/3] iommu/vt-d: Use 128-bit atomic updates for context
entries
> From: Lu Baolu <baolu.lu@...ux.intel.com>
> Sent: Tuesday, January 13, 2026 11:01 AM
>
> On Intel IOMMU, device context entries are accessed by hardware in
> 128-bit chunks. Currently, the driver updates these entries by
> programming the 'lo' and 'hi' 64-bit fields individually.
>
> This creates a potential race condition where the IOMMU hardware may
> fetch
> a context entry while the CPU has only completed one of the two 64-bit
> writes. This "torn" entry — consisting of half-old and half-new data —
> could lead to unpredictable hardware behavior, especially when
> transitioning the 'Present' bit or changing translation types.
this is not accurate. context entry is 128bits only. Scalable context
entry is 256bits but only the lower 128bits are defined. so hardware always
fetches the context entry atomically. Then if software ensures the right
order of updates (clear present first then other bits), the hardware won't
look at the partial entry after seeing present=0.
But now as Dmytro reported there is no barrier in place so two 64bits
updates to the context entry might be reordered so hw could fetch
an entry with old lower half (present=1) and new higher half.
then 128bit atomic operation avoids this ordering concern.
> @@ -1170,19 +1170,19 @@ static int domain_context_mapping_one(struct
> dmar_domain *domain,
> goto out_unlock;
>
> copied_context_tear_down(iommu, context, bus, devfn);
> - context_clear_entry(context);
> - context_set_domain_id(context, did);
> + context_set_domain_id(&new, did);
I wonder whether it's necessary to use atomic in the attach path, from
fix p.o.v.
The assumption is that the context should have been cleared already
before calling this function (and following ones). Does it make more
sense to check the present bit, warning if set, then fail the operation?
We could refactor them to do atomic update, but then it's for cleanup
instead of being part of a fix.
Then this may be split into three patches:
- change context_clear_entry() to be atomic, to fix the teardown path
- add present bit check in other functions in this patch, to scrutinize the
attach path
- change those functions to be atomic, as a clean up
Does it make sense?
Powered by blists - more mailing lists