Re: [PATCH v5 04/15] iommu/arm-smmu-v3: Drain in-flight fault events on domain detach

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Sep 23, 2026 at 03:33:06PM -0700, Nicolin Chen wrote:
> > I'm not sure how this all can work, the queue is running on its own
> > with some other CPU handling interrupts.
> > 
> > You can't do this sort of Q_DIFF math unless you've somehow guaranteed
> > one side of the queue is stable for this logic. If both pointers are
> > moving forward then the points pointers can progress and wrap without
> > this noticing that happened. That will lock up.
> 
> One side of the queue (pointers) is actually stable. EVTQ/PRIQ uses
> the snapshot mode (until_empty=false):

It isn't stable, just because this reads it once doesn't mean the
actual values are not changing, which is the point.

If one of the pointers is held stable then the HW cannot advance its
value past it.

If both are advancing all bets are off and you have no idea how the
values are related to each other since everything is modulo the ring
size.

For instance you can read cons0=10, then you read prod=15, then you
next read prod=11. What does that mean? It means since cons was
actually advancing prod & cons went around the whole ring and
wrapped.

You could only do tricks like this if you had full 64 bit counters,
not truncated versions with modulo that can wrap quickly.

> > But I wonder if the point of this has been lost? Prior to calling the
> > driver attach functions the core code already changes the xarray:
> > 
> > 	curr = xa_cmpxchg(&group->pasid_array, pasid, NULL,
> > 			  XA_ZERO_ENTRY, GFP_KERNEL);
> > 
> > That immediately makes the threaded IRQ safe since it calls
> > iommu_attach_handle_get() which now fails.

Hmm, actually that's a sneaky cmpxchg that is only doing reserve..

> I am not sure about that. Looking at iommufd_hwpt_replace_device(),
> there can be a old_handle != NULL, in which case the cmpxchg() would
> not change the xarray?

I think this is wrong, there is no way it can work like this where the
attach continues to see the to-be-detached domain across the
flushes. No amount of flushing can fix it.

Somehow we broke it :\

> > So all that is needed is to synchronize_irq() to make sure the irq
> > thread sees the xa update
> > 
> > Then to flush the workqueue that iommu_report_device_fault() pushes
> > into.
> > 
> > We don't need to do anything with the HW queue.
> 
> FWIW, the idea of HW drain came from intel_iommu_drain_pasid_prq()..

Yeah, but I think they might have over done it too..

Jason