[PATCH rc] iommu/arm-smmu-v3: Drain in-flight fault handlers

[PATCH rc] iommu/arm-smmu-v3: Drain in-flight fault handlers

[Nicolin Chen]

From: Malak Marrid <mmarrid@nvidia.com>

When a device is switching away from a domain, either through a detach or a
replace operation, it must drain its IOPF queue that only contains the page
requests for the old domain.

Currently, the IOPF infrastructure is used by master->stall_enabled. So the
stalled transaction for the old domain should be resumed/terminated. Fix it
properly.

Fixes: cfea71aea921 ("iommu/arm-smmu-v3: Put iopf enablement in the domain attach path")
Cc: stable@vger.kernel.org
Co-developed-by: Barak Biber <bbiber@nvidia.com>
Signed-off-by: Barak Biber <bbiber@nvidia.com>
Co-developed-by: Stefan Kaestle <skaestle@nvidia.com>
Signed-off-by: Stefan Kaestle <skaestle@nvidia.com>
Signed-off-by: Malak Marrid <mmarrid@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 4d00d796f0783..2176ee8bec767 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2843,6 +2843,12 @@ static int arm_smmu_enable_iopf(struct arm_smmu_master *master,
 	if (master->iopf_refcount) {
 		master->iopf_refcount++;
 		master_domain->using_iopf = true;
+		/*
+		 * If the device is already on the IOPF queue (domain replace),
+		 * drain in-flight fault handlers so nothing will hold the old
+		 * domain when the core switches the attach handle.
+		 */
+		iopf_queue_flush_dev(master->dev);
 		return 0;
 	}
 
@@ -2866,8 +2872,11 @@ static void arm_smmu_disable_iopf(struct arm_smmu_master *master,
 		return;
 
 	master->iopf_refcount--;
-	if (master->iopf_refcount == 0)
+	if (master->iopf_refcount == 0) {
+		/* Drain in-flight fault handlers before removing device */
+		iopf_queue_flush_dev(master->dev);
 		iopf_queue_remove_device(master->smmu->evtq.iopf, master->dev);
+	}
 }
 
 static void arm_smmu_remove_master_domain(struct arm_smmu_master *master,
-- 
2.43.0

Re: [PATCH rc] iommu/arm-smmu-v3: Drain in-flight fault handlers

[Will Deacon]

On Fri, Mar 06, 2026 at 04:17:23PM -0800, Nicolin Chen wrote:
> From: Malak Marrid <mmarrid@nvidia.com>
> 
> When a device is switching away from a domain, either through a detach or a
> replace operation, it must drain its IOPF queue that only contains the page
> requests for the old domain.
> 
> Currently, the IOPF infrastructure is used by master->stall_enabled. So the
> stalled transaction for the old domain should be resumed/terminated. Fix it
> properly.
> 
> Fixes: cfea71aea921 ("iommu/arm-smmu-v3: Put iopf enablement in the domain attach path")
> Cc: stable@vger.kernel.org
> Co-developed-by: Barak Biber <bbiber@nvidia.com>
> Signed-off-by: Barak Biber <bbiber@nvidia.com>
> Co-developed-by: Stefan Kaestle <skaestle@nvidia.com>
> Signed-off-by: Stefan Kaestle <skaestle@nvidia.com>
> Signed-off-by: Malak Marrid <mmarrid@nvidia.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 4d00d796f0783..2176ee8bec767 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2843,6 +2843,12 @@ static int arm_smmu_enable_iopf(struct arm_smmu_master *master,
>  	if (master->iopf_refcount) {
>  		master->iopf_refcount++;
>  		master_domain->using_iopf = true;
> +		/*
> +		 * If the device is already on the IOPF queue (domain replace),
> +		 * drain in-flight fault handlers so nothing will hold the old
> +		 * domain when the core switches the attach handle.
> +		 */
> +		iopf_queue_flush_dev(master->dev);

So this drains the iopf workqueue, but don't you still have a race with
the hardware generating a fault on the old domain and then that only
showing up once you've switched to the new one? What is the actual
problem you're trying to solve with this patch?

>  		return 0;
>  	}
>  
> @@ -2866,8 +2872,11 @@ static void arm_smmu_disable_iopf(struct arm_smmu_master *master,
>  		return;
>  
>  	master->iopf_refcount--;
> -	if (master->iopf_refcount == 0)
> +	if (master->iopf_refcount == 0) {
> +		/* Drain in-flight fault handlers before removing device */
> +		iopf_queue_flush_dev(master->dev);
>  		iopf_queue_remove_device(master->smmu->evtq.iopf, master->dev);

Why doesn't iopf_queue_remove_device() handle the draining? Is there a
case where you _don't_ want to drain the faults on the disable path?

Will

Re: [PATCH rc] iommu/arm-smmu-v3: Drain in-flight fault handlers

[Jason Gunthorpe]

On Thu, Mar 12, 2026 at 01:51:26PM +0000, Will Deacon wrote:
> On Fri, Mar 06, 2026 at 04:17:23PM -0800, Nicolin Chen wrote:
> > From: Malak Marrid <mmarrid@nvidia.com>
> > 
> > When a device is switching away from a domain, either through a detach or a
> > replace operation, it must drain its IOPF queue that only contains the page
> > requests for the old domain.
> > 
> > Currently, the IOPF infrastructure is used by master->stall_enabled. So the
> > stalled transaction for the old domain should be resumed/terminated. Fix it
> > properly.
> > 
> > Fixes: cfea71aea921 ("iommu/arm-smmu-v3: Put iopf enablement in the domain attach path")
> > Cc: stable@vger.kernel.org
> > Co-developed-by: Barak Biber <bbiber@nvidia.com>
> > Signed-off-by: Barak Biber <bbiber@nvidia.com>
> > Co-developed-by: Stefan Kaestle <skaestle@nvidia.com>
> > Signed-off-by: Stefan Kaestle <skaestle@nvidia.com>
> > Signed-off-by: Malak Marrid <mmarrid@nvidia.com>
> > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> > ---
> >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index 4d00d796f0783..2176ee8bec767 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -2843,6 +2843,12 @@ static int arm_smmu_enable_iopf(struct arm_smmu_master *master,
> >  	if (master->iopf_refcount) {
> >  		master->iopf_refcount++;
> >  		master_domain->using_iopf = true;
> > +		/*
> > +		 * If the device is already on the IOPF queue (domain replace),
> > +		 * drain in-flight fault handlers so nothing will hold the old
> > +		 * domain when the core switches the attach handle.
> > +		 */
> > +		iopf_queue_flush_dev(master->dev);
> 
> So this drains the iopf workqueue, but don't you still have a race with
> the hardware generating a fault on the old domain and then that only
> showing up once you've switched to the new one? What is the actual
> problem you're trying to solve with this patch?

HW doesn't generate faults on domains, it calls
iommu_report_device_fault() which calls find_fault_handler() that
uses iommu_attach_handle_get() to find the domain. It then shoves the
domain pointer onto a WQ.

The ordering is supposed to be
 1) IOMMU HW starts using the new domain
 2) iommu_attach_handle_get() returns the new domain
 3) IOMMU driver flushes its own IRQs/queues that may be concurrently
    calling iommu_attach_handle_get()
 4) iopf_queue_flush_dev() to clear the iopf work queue
 5) domain is freed, no pointers in WQs or other threads

So the naked iopf_queue_flush_dev() doesn't seem right, I'd expect a
synchronize_irq() (is that right for threaded IRQs?) too as the
threaded IRQ is concurrently calling iommu_attach_handle_get().

Next, something has gone wrong with the ordering of the xarray stores
in iommu_replace_device_pasid(), it doesn't follow the above since the
store for replace is after attach and flushing. Not sure how that
happened, I remember pointing this ordering out at various times..

Maybe we need to add a dedicated driver callback that does 
#3 and have the core do #4 internally. The driver shouldn't disable
its iopf during attach, it should happen in the flush handler.

> > @@ -2866,8 +2872,11 @@ static void arm_smmu_disable_iopf(struct arm_smmu_master *master,
> >  		return;
> >  
> >  	master->iopf_refcount--;
> > -	if (master->iopf_refcount == 0)
> > +	if (master->iopf_refcount == 0) {
> > +		/* Drain in-flight fault handlers before removing device */
> > +		iopf_queue_flush_dev(master->dev);
> >  		iopf_queue_remove_device(master->smmu->evtq.iopf, master->dev);
> 
> Why doesn't iopf_queue_remove_device() handle the draining? Is there a
> case where you _don't_ want to drain the faults on the disable path?

Because it isn't needed, this hunk is redundant.

We never disable iopf on a master that currently has an attached iopf
capable domain. When the domain was changed all the required flushing
should have been done - there should be exactly one
iopf_queue_flush_dev() inside a driver and it must be inside the
attach flow.

Jason

Re: [PATCH rc] iommu/arm-smmu-v3: Drain in-flight fault handlers

[Will Deacon]

On Thu, Mar 12, 2026 at 11:25:09AM -0300, Jason Gunthorpe wrote:
> On Thu, Mar 12, 2026 at 01:51:26PM +0000, Will Deacon wrote:
> > On Fri, Mar 06, 2026 at 04:17:23PM -0800, Nicolin Chen wrote:
> > > From: Malak Marrid <mmarrid@nvidia.com>
> > > 
> > > When a device is switching away from a domain, either through a detach or a
> > > replace operation, it must drain its IOPF queue that only contains the page
> > > requests for the old domain.
> > > 
> > > Currently, the IOPF infrastructure is used by master->stall_enabled. So the
> > > stalled transaction for the old domain should be resumed/terminated. Fix it
> > > properly.
> > > 
> > > Fixes: cfea71aea921 ("iommu/arm-smmu-v3: Put iopf enablement in the domain attach path")
> > > Cc: stable@vger.kernel.org
> > > Co-developed-by: Barak Biber <bbiber@nvidia.com>
> > > Signed-off-by: Barak Biber <bbiber@nvidia.com>
> > > Co-developed-by: Stefan Kaestle <skaestle@nvidia.com>
> > > Signed-off-by: Stefan Kaestle <skaestle@nvidia.com>
> > > Signed-off-by: Malak Marrid <mmarrid@nvidia.com>
> > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> > > ---
> > >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 ++++++++++-
> > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > index 4d00d796f0783..2176ee8bec767 100644
> > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > @@ -2843,6 +2843,12 @@ static int arm_smmu_enable_iopf(struct arm_smmu_master *master,
> > >  	if (master->iopf_refcount) {
> > >  		master->iopf_refcount++;
> > >  		master_domain->using_iopf = true;
> > > +		/*
> > > +		 * If the device is already on the IOPF queue (domain replace),
> > > +		 * drain in-flight fault handlers so nothing will hold the old
> > > +		 * domain when the core switches the attach handle.
> > > +		 */
> > > +		iopf_queue_flush_dev(master->dev);
> > 
> > So this drains the iopf workqueue, but don't you still have a race with
> > the hardware generating a fault on the old domain and then that only
> > showing up once you've switched to the new one? What is the actual
> > problem you're trying to solve with this patch?
> 
> HW doesn't generate faults on domains, it calls
> iommu_report_device_fault() which calls find_fault_handler() that
> uses iommu_attach_handle_get() to find the domain. It then shoves the
> domain pointer onto a WQ.

Sorry, that was sloppy terminology on my part. I'm trying to reason about
faults that are generated by accesses that were translated with the
page-tables of the old domain being reported once we think we are using
the new domain.

> The ordering is supposed to be
>  1) IOMMU HW starts using the new domain
>  2) iommu_attach_handle_get() returns the new domain
>  3) IOMMU driver flushes its own IRQs/queues that may be concurrently
>     calling iommu_attach_handle_get()

Does that mean we should kick the evtq thread? I'm not sure what this
means for the priq.

>  4) iopf_queue_flush_dev() to clear the iopf work queue
>  5) domain is freed, no pointers in WQs or other threads
> 
> So the naked iopf_queue_flush_dev() doesn't seem right, I'd expect a
> synchronize_irq() (is that right for threaded IRQs?) too as the
> threaded IRQ is concurrently calling iommu_attach_handle_get().

I don't think we can rely on the IRQ being taken, though, so presumably
we have to kick the irq thread manually and see what's actually sitting
in the event queue after the CMD_SYNC?

Will

Re: [PATCH rc] iommu/arm-smmu-v3: Drain in-flight fault handlers

[Jason Gunthorpe]

On Tue, Mar 24, 2026 at 02:04:03PM +0000, Will Deacon wrote:
> Sorry, that was sloppy terminology on my part. I'm trying to reason about
> faults that are generated by accesses that were translated with the
> page-tables of the old domain being reported once we think we are using
> the new domain.

It doesn't matter.

If a concurrent fault is resolving on the old domain and it completes
after the STE is in the new domain the device will restart and if the
IOVA is still non-present it will refault. This is normal and fine.

If it is resolving on the new domain and the new domain has a present
PTE so the PRI is spurious then the fault handler should NOP it and
restart the device.

> > The ordering is supposed to be
> >  1) IOMMU HW starts using the new domain
> >  2) iommu_attach_handle_get() returns the new domain
> >  3) IOMMU driver flushes its own IRQs/queues that may be concurrently
> >     calling iommu_attach_handle_get()
> 
> Does that mean we should kick the evtq thread? I'm not sure what this
> means for the priq.

The locking issue is around iommu_attach_handle_get(), so any
thread/irq concurrently calling that has to be fenced. That's it. We
don't have to expedite or synchronize with concurrent PRI at all.

> I don't think we can rely on the IRQ being taken, though, so presumably
> we have to kick the irq thread manually and see what's actually sitting
> in the event queue after the CMD_SYNC?

Er, I thought the iommu_attach_handle_get() was in a threaded irq? If
it is in a WQ then yeah more is needed.

Jason

Re: [PATCH rc] iommu/arm-smmu-v3: Drain in-flight fault handlers

[Will Deacon]

On Tue, Mar 24, 2026 at 11:17:16AM -0300, Jason Gunthorpe wrote:
> On Tue, Mar 24, 2026 at 02:04:03PM +0000, Will Deacon wrote:
> > Sorry, that was sloppy terminology on my part. I'm trying to reason about
> > faults that are generated by accesses that were translated with the
> > page-tables of the old domain being reported once we think we are using
> > the new domain.
> 
> It doesn't matter.
> 
> If a concurrent fault is resolving on the old domain and it completes
> after the STE is in the new domain the device will restart and if the
> IOVA is still non-present it will refault. This is normal and fine.
> 
> If it is resolving on the new domain and the new domain has a present
> PTE so the PRI is spurious then the fault handler should NOP it and
> restart the device.

Hmm, I can see that working out if both domains expect faults, but if
I'm switching to a domain without a handler wouldn't I be better off
draining the outstanding faults generated on the old domain first?
Otherwise, won't we see a bunch of noise from the eventq thread as
it dumps unexpected events to the console?

Will

Re: [PATCH rc] iommu/arm-smmu-v3: Drain in-flight fault handlers

[Jason Gunthorpe]

On Tue, Mar 24, 2026 at 02:35:14PM +0000, Will Deacon wrote:
> On Tue, Mar 24, 2026 at 11:17:16AM -0300, Jason Gunthorpe wrote:
> > On Tue, Mar 24, 2026 at 02:04:03PM +0000, Will Deacon wrote:
> > > Sorry, that was sloppy terminology on my part. I'm trying to reason about
> > > faults that are generated by accesses that were translated with the
> > > page-tables of the old domain being reported once we think we are using
> > > the new domain.
> > 
> > It doesn't matter.
> > 
> > If a concurrent fault is resolving on the old domain and it completes
> > after the STE is in the new domain the device will restart and if the
> > IOVA is still non-present it will refault. This is normal and fine.
> > 
> > If it is resolving on the new domain and the new domain has a present
> > PTE so the PRI is spurious then the fault handler should NOP it and
> > restart the device.
> 
> Hmm, I can see that working out if both domains expect faults, but if
> I'm switching to a domain without a handler 

iommu_report_device_fault() still handles the event and generates an
error ack.

> wouldn't I be better off draining the outstanding faults generated
> on the old domain first?  Otherwise, won't we see a bunch of noise
> from the eventq thread as it dumps unexpected events to the console?

Yes, it does look like since iommu_report_device_fault() handles it
but returns an error code we will get a print.

You'd need to double flush the the event queue. We always have to
flush after changing the group->domain since that is preventing a UAF

Then you'd have to flush before changing the group->domain to avoid
the prints if faulting is being disabled.

IDK, may not be worth worrying about or maybe we should just remove
the prints..

Jason