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

From: Robin Murphy <robin.murphy@arm.com>
To: Will Deacon <will@kernel.org>, Jiaxing Hu <huhuvmb88@gmail.com>
Cc: Heiko Stuebner <heiko@sntech.de>, Joerg Roedel <joro@8bytes.org>,
	iommu@lists.linux.dev, linux-rockchip@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, jgg@ziepe.ca
Subject: Re: [PATCH] iommu/rockchip: implement .flush_iotlb_all
Date: Fri, 10 Jul 2026 13:11:41 +0100	[thread overview]
Message-ID: <15bbada2-3214-45dc-8934-a6dbe207c887@arm.com> (raw)
In-Reply-To: <alDQevX58XvdRcWc@willie-the-truck>

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