On Tue, Jan 13, 2026 at 05:25:38PM +0100, Benjamin Gaignard wrote:

Le 13/01/2026 à 17:10, Will Deacon a écrit :

quoted

Hi Benjamin,

Thanks for posting a v11.

On Wed, Jan 07, 2026 at 11:09:53AM +0100, Benjamin Gaignard wrote:

quoted

The Verisilicon IOMMU hardware block can be found in combination
with Verisilicon hardware video codecs (encoders or decoders) on
different SoCs.
Enable it will allow us to use non contiguous memory allocators
for Verisilicon video codecs.
If both decoder and this iommu driver are compiled has modules
there is undefined symboles issues so this iommu driver could
only be compiled has built-in.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
changes in version 11:
- Fix dependency issue when decoder driver is build as module.

  drivers/iommu/Kconfig     |  11 +
  drivers/iommu/Makefile    |   1 +
  drivers/iommu/vsi-iommu.c | 808 ++++++++++++++++++++++++++++++++++++++
  include/linux/vsi-iommu.h |  21 +
  4 files changed, 841 insertions(+)
  create mode 100644 drivers/iommu/vsi-iommu.c
  create mode 100644 include/linux/vsi-iommu.h

Based on your reply to v9:

https://lore.kernel.org/all/0eff8b1a-c45f-47b1-a871-59f4a0101f0f@collabora.com/ (local)

I took another look at this to see whether it had changed significantly
from v6 when compared to the rockchip driver. Sadly, they still look
very similar to me and I continue to suspect that the hardware is a
derivative. I really don't understand why having a shared implementation
of the default domain ops is difficult or controversial. Have you tried
to write it?

However, given that nobody from the Rockchip side has contributed to the
discussion and you claim that this is a distinct piece of IP, I don't
want to block the merging of the driver by leaving the conversation
hanging.

There is still one thing I don't understand (which, amusingly, the
rockchip driver doesn't seem to suffer from):

quoted

+static void vsi_iommu_flush_tlb_all(struct iommu_domain *domain)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	struct list_head *pos;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+
+	list_for_each(pos, &vsi_domain->iommus) {
+		struct vsi_iommu *iommu;
+		int ret;
+
+		iommu = list_entry(pos, struct vsi_iommu, node);
+		ret = pm_runtime_resume_and_get(iommu->dev);
+		if (ret < 0)
+			continue;
+
+		spin_lock(&iommu->lock);
+
+		writel(VSI_MMU_BIT_FLUSH, iommu->regs + VSI_MMU_FLUSH_BASE);
+		writel(0, iommu->regs + VSI_MMU_FLUSH_BASE);
+
+		spin_unlock(&iommu->lock);
+		pm_runtime_put_autosuspend(iommu->dev);
+	}
+
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+}

[...]

quoted

+static const struct iommu_ops vsi_iommu_ops = {
+	.identity_domain = &vsi_identity_domain,
+	.release_domain = &vsi_identity_domain,
+	.domain_alloc_paging = vsi_iommu_domain_alloc_paging,
+	.of_xlate = vsi_iommu_of_xlate,
+	.probe_device = vsi_iommu_probe_device,
+	.release_device = vsi_iommu_release_device,
+	.device_group = generic_single_device_group,
+	.owner = THIS_MODULE,
+	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.attach_dev		= vsi_iommu_attach_device,
+		.map_pages		= vsi_iommu_map,
+		.unmap_pages		= vsi_iommu_unmap,
+		.flush_iotlb_all	= vsi_iommu_flush_tlb_all,

This has no callers and so your unmap routine appears to be broken.

It is a leftover of previous attempt to allow video decoder to clean/flush
the iommu by using a function from the API.
Now it is using vsi_iommu_restore_ctx().
I while remove it in version 12.

Don't you still need some invalidation on the unmap path?

Will