Re: [PATCH RFC v2 3/4] iommu: Introduce iommu_dev_reset_prepare() and iommu_dev_reset_done()

[Jason Gunthorpe]

On Tue, Jul 22, 2025 at 02:58:21PM -0700, Nicolin Chen wrote:
> > /*
> >  * This is called on the dma mapping fast path so avoid locking. This
> >  * is racy, but we have an expectation that the driver will setup its
> >  * DMAs inside probe while still single threaded to avoid racing.
> >  */
> > if (dev->iommu && !READ_ONCE(dev->iommu->attach_deferred))
> 
> This triggers a build error as attach_deferred is a bit-field. So I
> am changing it from "u32 attach_deferred:1" to "bool" for this.

Bleck, that seems undesirable.

> And, to keep the original logic, I think it should be:
> 	if (!dev->iommu || !READ_ONCE(dev->iommu->attach_deferred))

That doesn't seem right, if there is no iommu by the time a driver is
probed there never will be an iommu and this device should be running
in direct mode only.

> > And of course it is already quite crazy to be doing FLR during a
> > device probe so this is not a realistic scenario.
> 
> Hmm, I am not sure about that, as I see iommu_deferred_attach() get
> mostly invoked by a dma_alloc() or even a dma_map(). So, this might
> not be confined to a device probe?

Once you do deferred_attach the first time it is done and won't have
any further impact. So long as the dev->iommu->attach_deferred guards
any changes to domains it is unlikely to be racing with FLR.

> > Either ignore this condition with the rational that we are about to
> > reset it so it doesn't matter, or we need to establish a new paging
> > domain for isolation purposes that has the RMR setup.
> 
> Ah, you are right. ARM MSI in a VM uses RMR and sets this.
> 
> But does it also raise a question that a VM having RMR can't use
> the blocked_domain, as __iommu_device_set_domain() has the exact
> same check rejecting blocked_domain? Not sure if there would be
> some unintended consequnce though...

Sounds like it needs some sorting out.. For the purposes of FLR I
think the blocked domain is OK, so maybe just move some of those
checks around?

Jason