linux-kernel - Re: [PATCH 0/3] iommu/vt-d: Add support to hitless replace IOMMU domain

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAywjhTEyOUmxWeCX6GwCBSnMf-p18Ksu2TUYeQ57K8H4RW-9w@mail.gmail.com>
Date: Fri, 9 Jan 2026 12:06:13 -0800
From: Samiullah Khawaja <skhawaja@...gle.com>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: David Woodhouse <dwmw2@...radead.org>, Lu Baolu <baolu.lu@...ux.intel.com>, 
	Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>, 
	Pasha Tatashin <pasha.tatashin@...een.com>, David Matlack <dmatlack@...gle.com>, 
	Robin Murphy <robin.murphy@....com>, Pratyush Yadav <pratyush@...nel.org>, 
	Kevin Tian <kevin.tian@...el.com>, Alex Williamson <alex@...zbot.org>, Shuah Khan <shuah@...nel.org>, 
	iommu@...ts.linux.dev, linux-kernel@...r.kernel.org, kvm@...r.kernel.org, 
	Saeed Mahameed <saeedm@...dia.com>, Adithya Jayachandran <ajayachandra@...dia.com>, 
	Parav Pandit <parav@...dia.com>, Leon Romanovsky <leonro@...dia.com>, William Tu <witu@...dia.com>
Subject: Re: [PATCH 0/3] iommu/vt-d: Add support to hitless replace IOMMU domain

On Wed, Jan 7, 2026 at 12:46 PM Jason Gunthorpe <jgg@...pe.ca> wrote:
>
> On Wed, Jan 07, 2026 at 04:28:12PM -0400, Jason Gunthorpe wrote:
> > On Wed, Jan 07, 2026 at 08:17:57PM +0000, Samiullah Khawaja wrote:
> > > Intel IOMMU Driver already supports replacing IOMMU domain hitlessly in
> > > scalable mode.
> >
> > It does? We were just talking about how it doesn't work because it
> > makes the PASID entry non-present while loading the new domain.
>
> If you tried your tests in scalable mode they are probably only
> working because the HW is holding the entry in cache while the CPU is
> completely mangling it:
>
> int intel_pasid_replace_first_level(struct intel_iommu *iommu,
>                                     struct device *dev, phys_addr_t fsptptr,
>                                     u32 pasid, u16 did, u16 old_did,
>                                     int flags)
> {
> [..]
>         *pte = new_pte;
>
> That just doesn't work for "replace", it isn't hitless unless the
> entry stays in the cache. Since your test effectively will hold the
> context entry in the cache while testing for "hitless" it doesn't
> really test if it is really working without races..

Ah.. you are absolutely correct. This will not work if the entries are
not cached.
>
> All of this needs to be reworked to always use the stack to build the
> entry, like the replace path does, and have a ARM-like algorithm to
> update the live memory in just the right order to guarentee the HW
> does not see a corrupted entry.

Agreed. I will go through the VTD specs and also the ARM driver to
determine the right order to set this up.
>
> It is a little bit tricky, but it should start with reworking
> everything to consistently use the stack to create the new entry and
> calling a centralized function to set the new entry to the live
> memory. This replace/not replace split should be purged completely.
>
> Some discussion is here
>
> https://lore.kernel.org/all/20260106142301.GS125261@ziepe.ca/
>
> It also needs to be very careful that the invalidation is doing both
> the old and new context entry concurrently while it is being replaced.

Yes, let me follow the discussion over there closely.
>
> For instance the placement of cache_tag_assign_domain() looks wrong to
> me, it can't be *after* the HW has been programmed to use the new tags
> :\
>
> I also didn't note where the currently active cache_tag is removed
> from the linked list during attach, is that another bug?

You are correct, the placement of cache_tag_assign_domain is wrong.
The removal of the currently active cache tag is done in
dmar_domain_attach_device, but I will re-evaluate the placement of
both in the v2.
>
> In short, this needs alot of work to actually properly implement
> hitless replace the way ARM can. Fortunately I think it is mostly
> mechanical and should be fairly straightfoward. Refer to the ARM
> driver and try to structure vtd to have the same essential flow..

Thank you for the feedback. I will prepare a v2 series addressing these points.
>
> Jason