[PATCH] iommu/rockchip: implement .flush_iotlb_all

[PATCH] iommu/rockchip: implement .flush_iotlb_all

[Jiaxing Hu]

rk_iommu invalidates the IOTLB synchronously in .unmap (via
rk_iommu_zap_iova), so it stays consistent without a deferred .iotlb_sync
and does not advertise IOMMU_CAP_DEFERRED_FLUSH.

It never implemented the optional .flush_iotlb_all op, though, and
iommu_flush_iotlb_all() is a no-op when that op is absent -- so on Rockchip
a driver that owns an rk_iommu domain and calls it to flush the whole
domain's TLB (e.g. before reusing a mapping, without unmapping it) gets
nothing, even though the hardware can do it: rk_iommu already issues
ZAP_CACHE for range shootdowns.

Implement it with that same primitive: ZAP_CACHE every bank of every IOMMU
on the domain, under the runtime-PM + clk guard already used by
rk_iommu_zap_iova().

Found while bringing up an out-of-tree RK3576 NPU (accel/rocket) driver
that flushes before each submit; on Rockchip that flush did nothing.

Signed-off-by: Jiaxing Hu <huhuvmb88@gmail.com>
---
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1202,6 +1202,30 @@
 	return 0;
 }
 
+/* shootdown the entire tlb on all iommus using this domain */
+static void rk_iommu_flush_iotlb_all(struct iommu_domain *domain)
+{
+	struct rk_iommu_domain *rk_domain = to_rk_domain(domain);
+	struct list_head *pos;
+	unsigned long flags;
+
+	spin_lock_irqsave(&rk_domain->iommus_lock, flags);
+	list_for_each(pos, &rk_domain->iommus) {
+		struct rk_iommu *iommu = list_entry(pos, struct rk_iommu, node);
+		int ret = pm_runtime_get_if_in_use(iommu->dev);
+
+		if (WARN_ON_ONCE(ret < 0))
+			continue;
+		if (ret) {
+			WARN_ON(clk_bulk_enable(iommu->num_clocks, iommu->clocks));
+			rk_iommu_command(iommu, RK_MMU_CMD_ZAP_CACHE);
+			clk_bulk_disable(iommu->num_clocks, iommu->clocks);
+			pm_runtime_put(iommu->dev);
+		}
+	}
+	spin_unlock_irqrestore(&rk_domain->iommus_lock, flags);
+}
+
 static const struct iommu_ops rk_iommu_ops = {
 	.identity_domain = &rk_identity_domain,
 	.domain_alloc_paging = rk_iommu_domain_alloc_paging,
@@ -1214,6 +1245,7 @@
 		.map_pages	= rk_iommu_map,
 		.unmap_pages	= rk_iommu_unmap,
 		.iova_to_phys	= rk_iommu_iova_to_phys,
+		.flush_iotlb_all = rk_iommu_flush_iotlb_all,
 		.free		= rk_iommu_domain_free,
 	}
 };

Re: [PATCH] iommu/rockchip: implement .flush_iotlb_all

[Will Deacon]

On Fri, Jul 10, 2026 at 06:05:13PM +1200, Jiaxing Hu wrote:
> rk_iommu invalidates the IOTLB synchronously in .unmap (via
> rk_iommu_zap_iova), so it stays consistent without a deferred .iotlb_sync
> and does not advertise IOMMU_CAP_DEFERRED_FLUSH.
> 
> It never implemented the optional .flush_iotlb_all op, though, and
> iommu_flush_iotlb_all() is a no-op when that op is absent -- so on Rockchip
> a driver that owns an rk_iommu domain and calls it to flush the whole
> domain's TLB (e.g. before reusing a mapping, without unmapping it) gets
> nothing, even though the hardware can do it: rk_iommu already issues
> ZAP_CACHE for range shootdowns.
> 
> Implement it with that same primitive: ZAP_CACHE every bank of every IOMMU
> on the domain, under the runtime-PM + clk guard already used by
> rk_iommu_zap_iova().
> 
> Found while bringing up an out-of-tree RK3576 NPU (accel/rocket) driver
> that flushes before each submit; on Rockchip that flush did nothing.
> 
> Signed-off-by: Jiaxing Hu <huhuvmb88@gmail.com>
> ---

We tried to get rid of iommu_flush_iotlb_all() in 69e5a17511f6
("iommu: Remove useless flush from iommu_create_device_direct_mappings()")
but we had to revert that in a33bf8d8ce7e ("iommu: Restore
iommu_flush_iotlb_all()") because of a user in the MSM driver.

So I think this is the wrong direction. We should try harder to remove
the function rather than add new implementations of it. I'm assuming
you're only adding it for some out-of-tree code that calls it?

Will

Re: [PATCH] iommu/rockchip: implement .flush_iotlb_all

[Robin Murphy]

On 10/07/2026 11:59 am, Will Deacon wrote:
> On Fri, Jul 10, 2026 at 06:05:13PM +1200, Jiaxing Hu wrote:
>> rk_iommu invalidates the IOTLB synchronously in .unmap (via
>> rk_iommu_zap_iova), so it stays consistent without a deferred .iotlb_sync
>> and does not advertise IOMMU_CAP_DEFERRED_FLUSH.
>>
>> It never implemented the optional .flush_iotlb_all op, though, and
>> iommu_flush_iotlb_all() is a no-op when that op is absent -- so on Rockchip
>> a driver that owns an rk_iommu domain and calls it to flush the whole
>> domain's TLB (e.g. before reusing a mapping, without unmapping it) gets
>> nothing, even though the hardware can do it: rk_iommu already issues
>> ZAP_CACHE for range shootdowns.
>>
>> Implement it with that same primitive: ZAP_CACHE every bank of every IOMMU
>> on the domain, under the runtime-PM + clk guard already used by
>> rk_iommu_zap_iova().
>>
>> Found while bringing up an out-of-tree RK3576 NPU (accel/rocket) driver
>> that flushes before each submit; on Rockchip that flush did nothing.
>>
>> Signed-off-by: Jiaxing Hu <huhuvmb88@gmail.com>
>> ---
> 
> We tried to get rid of iommu_flush_iotlb_all() in 69e5a17511f6
> ("iommu: Remove useless flush from iommu_create_device_direct_mappings()")
> but we had to revert that in a33bf8d8ce7e ("iommu: Restore
> iommu_flush_iotlb_all()") because of a user in the MSM driver.

We wanted rid of the public API (indeed partly due to the prospect of 
client drivers abusing it), but the actual ->flush_tlb_all driver op 
remains crucial to the flush queue mechanism, so there's no harm in 
drivers implementing it *if* they also implement the relevant 
optimisations to avoid synchronous flushing in unmap_pages to make 
CAP_DEFERRED_FLUSH worthwhile.

Thanks,
Robin.

> So I think this is the wrong direction. We should try harder to remove
> the function rather than add new implementations of it. I'm assuming
> you're only adding it for some out-of-tree code that calls it?
> 
> Will

Re: [PATCH] iommu/rockchip: implement .flush_iotlb_all

[Jiaxing Hu]

On Fri, 10 Jul 2026, Will Deacon wrote:
> So I think this is the wrong direction. We should try harder to remove
> the function rather than add new implementations of it. I'm assuming
> you're only adding it for some out-of-tree code that calls it?

Yes, that's right. The only caller is the out-of-tree RK3576 NPU
(accel/rocket) bring-up I mentioned in the commit message. There's no
in-tree rk_iommu user, since the in-tree consumers all invalidate through
.unmap and none of them call iommu_flush_iotlb_all().

So if the plan is to remove the function rather than add new
implementations of it, then I've got this backwards. Please drop the
patch. If the NPU driver ever goes upstream I'll make it stop relying on
iommu_flush_iotlb_all() rather than bring this one back.

Thanks for taking a look, and for the pointers to those two commits.

Jiaxing

Re: [PATCH] iommu/rockchip: implement .flush_iotlb_all

[Heiko Stübner]

Hi,

Am Freitag, 10. Juli 2026, 13:15:08 Mitteleuropäische Sommerzeit schrieb Jiaxing Hu:
> On Fri, 10 Jul 2026, Will Deacon wrote:
> > So I think this is the wrong direction. We should try harder to remove
> > the function rather than add new implementations of it. I'm assuming
> > you're only adding it for some out-of-tree code that calls it?
> 
> Yes, that's right. The only caller is the out-of-tree RK3576 NPU
> (accel/rocket) bring-up I mentioned in the commit message. There's no
> in-tree rk_iommu user, since the in-tree consumers all invalidate through
> .unmap and none of them call iommu_flush_iotlb_all().

accel/rocket is in mainline, so I guess the interesting question is, why
are you expecting your changes to be (stay?) out of tree?

When you show your changes - even as a RFC - on the lists you might get
helpful comments - like the ones here for the iommu change.
That might also prevent you from going too far in a suboptimal direction
and thus could save time.

Additionally, you would show you're working on that, so that could
prevent someone else starting a duplicate of your work -  or at least
pool resources.

There are a bunch of people working on the RK3576 for different projects
so there is a pretty high chance of getting valuable feedback.
(like what happened with this patch ;-) )


Heiko

Re: [PATCH] iommu/rockchip: implement .flush_iotlb_all

[Jiaxing Hu]

On Fri, 10 Jul 2026, Heiko Stübner wrote:
> accel/rocket is in mainline, so I guess the interesting question is, why
> are you expecting your changes to be (stay?) out of tree?

Thanks. I had been treating it as exploratory: single-layer inference runs
on the open stack (rocket + Teflon) on RK3576, but chained inference does
not, and I narrowed the failure to below the software surface
(fixed-function hardware I cannot reach from an open board), so it felt too
unfinished to send.

But you are right that an RFC is the better way to handle it. I will clean
up the RK3576 bring-up (power domains, clocks, IOMMU, DPU/RDMA, DTS) into
an RFC against current mainline and post it, with the chained-task
limitation described honestly. If you can point me at who else is working
on the RK3576 NPU, I would be glad to compare notes.

Thanks,
Jiaxing

Re: [PATCH] iommu/rockchip: implement .flush_iotlb_all

[Jiaxing Hu]

On Sat, 11 Jul 2026, Robin Murphy wrote:
> there's no harm in drivers implementing it *if* they also implement the
> relevant optimisations to avoid synchronous flushing in unmap_pages to
> make CAP_DEFERRED_FLUSH worthwhile.

Understood, thanks. I will drop the standalone patch. If I come back to
rk_iommu it will be the full version: move invalidation out of unmap_pages
into an iotlb_sync/gather path and advertise CAP_DEFERRED_FLUSH with
numbers, rather than flush_iotlb_all on its own.

Thanks,
Jiaxing