[PATCH v11 0/7] Add support for Verisilicon IOMMU used by media

[PATCH v11 0/7] Add support for Verisilicon IOMMU used by media

[Benjamin Gaignard]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 2946 bytes --]

Hi all,

This patch series adds support for the Verisilicon IOMMU, which is found
in front
of hardware encoder and decoder blocks in several SoCs using Verisilicon
IP. 
A first implementation of this IOMMU is available on the Rockchip RK3588
SoC.

Rockchip provides a driver for this hardware in their 6.1 kernel branch:
https://github.com/rockchip-linux/kernel/blob/develop-6.1/drivers/iommu/rockchip-iommu-av1d.c

This series includes:
- a new binding for the Verisilicon IOMMU
- a driver implementation
- DT updates for RK3588

The driver was forward-ported from Rockchip’s 6.1 implementation, 
the prefix was renamed to vsi for generality, and several fixes were
applied.

AV1 decoding was tested using the stateless VPU driver and Fluster.
The test results show a score of 205/239, which confirms that no
regressions
were introduced by this series.

Feedback and testing welcome.

changes in version 11:
- Fix dependency issue when decoder driver is build as module.

changes in version 10:
- Update vsi_iommu_identity_attach() and vsi_iommu_attach_device()
  prototypes.
- Fix build as module issue when Verisilicon video decoder is built-in.
- Rebase on master branch.

changes in version 9:
- removing blanks lines.

changes in version 8:
- Add myself in MAINTAINERS file.
- Add API to restore VSI iommu context from decoder driver
- Fix reported checkpatch issues: add comment in pinlock_t declaration
  and remove blank line.
- Include board name in defconfig patch commit message

changes in version 7:
- fix locking issues.
- add a patch in AV1 video decoder to manage per context iommu domain.
- fix compilation issues when build as module.
- remove useless "rockchip,rk3588-av1-iommu" compatible in driver code.

Benjamin Gaignard (7):
  dt-bindings: vendor-prefixes: Add Verisilicon
  dt-bindings: iommu: verisilicon: Add binding for VSI IOMMU
  iommu: Add verisilicon IOMMU driver
  MAINTAINERS: Add entry for Verisilicon IOMMU driver
  media: verisilicon: AV1: Restore IOMMU context before decoding a frame
  arm64: dts: rockchip: Add verisilicon IOMMU node on RK3588
  arm64: defconfig: enable Verisilicon IOMMU for Rockchip RK3588

 .../bindings/iommu/verisilicon,iommu.yaml     |  71 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   8 +
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  11 +
 arch/arm64/configs/defconfig                  |   1 +
 drivers/iommu/Kconfig                         |  11 +
 drivers/iommu/Makefile                        |   1 +
 drivers/iommu/vsi-iommu.c                     | 808 ++++++++++++++++++
 .../verisilicon/rockchip_vpu981_hw_av1_dec.c  |  15 +
 include/linux/vsi-iommu.h                     |  21 +
 10 files changed, 949 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/verisilicon,iommu.yaml
 create mode 100644 drivers/iommu/vsi-iommu.c
 create mode 100644 include/linux/vsi-iommu.h

-- 
2.43.0

[PATCH v11 1/7] dt-bindings: vendor-prefixes: Add Verisilicon

[Benjamin Gaignard]

Verisilicon Microelectronics is a company based in Shanghai, China,
developping hardware blocks for SoC.

https://verisilicon.com/

Add their name to the list of vendors.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index c7591b2aec2a..ce5c413948b1 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1745,6 +1745,8 @@ patternProperties:
     description: Variscite Ltd.
   "^vdl,.*":
     description: Van der Laan b.v.
+  "^verisilicon,.*":
+    description: VeriSilicon Microelectronics
   "^vertexcom,.*":
     description: Vertexcom Technologies, Inc.
   "^via,.*":
-- 
2.43.0

[PATCH v11 2/7] dt-bindings: iommu: verisilicon: Add binding for VSI IOMMU

[Benjamin Gaignard]

Add a device tree binding for the Verisilicon (VSI) IOMMU.
This IOMMU sits in front of hardware encoder and decoder
blocks on SoCs using Verisilicon IP, such as the Rockchip RK3588.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/iommu/verisilicon,iommu.yaml     | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/verisilicon,iommu.yaml

diff --git a/Documentation/devicetree/bindings/iommu/verisilicon,iommu.yaml b/Documentation/devicetree/bindings/iommu/verisilicon,iommu.yaml
new file mode 100644
index 000000000000..d3ce9e603b61
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/verisilicon,iommu.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/verisilicon,iommu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Verisilicon IOMMU
+
+maintainers:
+  - Benjamin Gaignard <benjamin.gaignard@collabora.com>
+
+description: |+
+  A Versilicon iommu translates io virtual addresses to physical addresses for
+  its associated video decoder.
+
+properties:
+  compatible:
+    items:
+      - const: rockchip,rk3588-av1-iommu
+      - const: verisilicon,iommu-1.2
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Core clock
+      - description: Interface clock
+
+  clock-names:
+    items:
+      - const: core
+      - const: iface
+
+  "#iommu-cells":
+    const: 0
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - "#iommu-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    bus {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      iommu@fdca0000 {
+        compatible = "rockchip,rk3588-av1-iommu","verisilicon,iommu-1.2";
+        reg = <0x0 0xfdca0000 0x0 0x600>;
+        interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH 0>;
+        clocks = <&cru ACLK_AV1>, <&cru PCLK_AV1>;
+        clock-names = "core", "iface";
+        #iommu-cells = <0>;
+      };
+    };
-- 
2.43.0

[PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Benjamin Gaignard]

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

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 99095645134f..cfa712d2d035 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -384,6 +384,17 @@ config SPRD_IOMMU
 
 	  Say Y here if you want to use the multimedia devices listed above.
 
+config VSI_IOMMU
+	bool "Verisilicon IOMMU Support"
+	depends on (ARCH_ROCKCHIP && ARM64) || COMPILE_TEST
+	select IOMMU_API
+	help
+	  Support for IOMMUs used by Verisilicon sub-systems like video
+	  decoders or encoder hardware blocks.
+
+	  Say Y here if you want to use this IOMMU in front of these
+	  hardware blocks.
+
 endif # IOMMU_SUPPORT
 
 source "drivers/iommu/generic_pt/Kconfig"
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 8e8843316c4b..f4043350d023 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -36,3 +36,4 @@ obj-$(CONFIG_IOMMU_SVA) += iommu-sva.o
 obj-$(CONFIG_IOMMU_IOPF) += io-pgfault.o
 obj-$(CONFIG_SPRD_IOMMU) += sprd-iommu.o
 obj-$(CONFIG_APPLE_DART) += apple-dart.o
+obj-$(CONFIG_VSI_IOMMU) += vsi-iommu.o
diff --git a/drivers/iommu/vsi-iommu.c b/drivers/iommu/vsi-iommu.c
new file mode 100644
index 000000000000..ea961adf5eb9
--- /dev/null
+++ b/drivers/iommu/vsi-iommu.c
@@ -0,0 +1,808 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (C) 2025 Collabora Ltd.
+ *
+ * IOMMU API for Verisilicon
+ *
+ * Module Authors:	Yandong Lin <yandong.lin@rock-chips.com>
+ *			Simon Xue <xxm@rock-chips.com>
+ *			Benjamin Gaignard <benjamin.gaignard@collabora.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/compiler.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iommu.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_iommu.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/vsi-iommu.h>
+
+#include "iommu-pages.h"
+
+struct vsi_iommu {
+	struct device *dev;
+	void __iomem *regs;
+	struct clk_bulk_data *clocks;
+	int num_clocks;
+	struct iommu_device iommu;
+	struct list_head node; /* entry in vsi_iommu_domain.iommus */
+	struct iommu_domain *domain; /* domain to which iommu is attached */
+	spinlock_t lock; /* lock to protect vsi_iommu fields */
+	int irq;
+};
+
+struct vsi_iommu_domain {
+	struct list_head iommus;
+	struct device *dev;
+	u32 *dt;
+	dma_addr_t dt_dma;
+	struct iommu_domain domain;
+	u64 *pta;
+	dma_addr_t pta_dma;
+	spinlock_t lock; /* lock to protect vsi_iommu_domain fields */
+};
+
+static struct iommu_domain vsi_identity_domain;
+
+#define NUM_DT_ENTRIES	1024
+#define NUM_PT_ENTRIES	1024
+#define PT_SIZE		(NUM_PT_ENTRIES * sizeof(u32))
+
+#define SPAGE_SIZE	BIT(12)
+
+/* vsi iommu regs address */
+#define VSI_MMU_CONFIG1_BASE			0x1ac
+#define VSI_MMU_AHB_EXCEPTION_BASE		0x380
+#define VSI_MMU_AHB_CONTROL_BASE		0x388
+#define VSI_MMU_AHB_TLB_ARRAY_BASE_L_BASE	0x38C
+
+/* MMU register offsets */
+#define VSI_MMU_FLUSH_BASE		0x184
+#define VSI_MMU_BIT_FLUSH		BIT(4)
+
+#define VSI_MMU_PAGE_FAULT_ADDR		0x380
+#define VSI_MMU_STATUS_BASE		0x384	/* IRQ status */
+
+#define VSI_MMU_BIT_ENABLE		BIT(0)
+
+#define VSI_MMU_OUT_OF_BOUND		BIT(28)
+/* Irq mask */
+#define VSI_MMU_IRQ_MASK		0x7
+
+#define VSI_DTE_PT_ADDRESS_MASK		0xffffffc0
+#define VSI_DTE_PT_VALID		BIT(0)
+
+#define VSI_PAGE_DESC_LO_MASK		0xfffff000
+#define VSI_PAGE_DESC_HI_MASK		GENMASK_ULL(39, 32)
+#define VSI_PAGE_DESC_HI_SHIFT		(32 - 4)
+
+static inline phys_addr_t vsi_dte_pt_address(u32 dte)
+{
+	return (phys_addr_t)dte & VSI_DTE_PT_ADDRESS_MASK;
+}
+
+static inline u32 vsi_mk_dte(u32 dte)
+{
+	return (phys_addr_t)dte | VSI_DTE_PT_VALID;
+}
+
+#define VSI_PTE_PAGE_WRITABLE		BIT(2)
+#define VSI_PTE_PAGE_VALID		BIT(0)
+
+static inline phys_addr_t vsi_pte_page_address(u64 pte)
+{
+	return ((pte << VSI_PAGE_DESC_HI_SHIFT) & VSI_PAGE_DESC_HI_MASK) |
+	       (pte & VSI_PAGE_DESC_LO_MASK);
+}
+
+static u32 vsi_mk_pte(phys_addr_t page, int prot)
+{
+	u32 flags = 0;
+
+	flags |= (prot & IOMMU_WRITE) ? VSI_PTE_PAGE_WRITABLE : 0;
+
+	page = (page & VSI_PAGE_DESC_LO_MASK) |
+	       ((page & VSI_PAGE_DESC_HI_MASK) >> VSI_PAGE_DESC_HI_SHIFT);
+
+	return page | flags | VSI_PTE_PAGE_VALID;
+}
+
+#define VSI_DTE_PT_VALID	BIT(0)
+
+static inline bool vsi_dte_is_pt_valid(u32 dte)
+{
+	return dte & VSI_DTE_PT_VALID;
+}
+
+static inline bool vsi_pte_is_page_valid(u32 pte)
+{
+	return pte & VSI_PTE_PAGE_VALID;
+}
+
+static u32 vsi_mk_pte_invalid(u32 pte)
+{
+	return pte & ~VSI_PTE_PAGE_VALID;
+}
+
+#define VSI_MASTER_TLB_MASK	GENMASK_ULL(31, 10)
+/* mode 0 : 4k */
+#define VSI_PTA_4K_MODE	0
+
+static u64 vsi_mk_pta(dma_addr_t dt_dma)
+{
+	u64 val = (dt_dma & VSI_MASTER_TLB_MASK) | VSI_PTA_4K_MODE;
+
+	return val;
+}
+
+static struct vsi_iommu_domain *to_vsi_domain(struct iommu_domain *dom)
+{
+	return container_of(dom, struct vsi_iommu_domain, domain);
+}
+
+static inline void vsi_table_flush(struct vsi_iommu_domain *vsi_domain, dma_addr_t dma,
+				   unsigned int count)
+{
+	size_t size = count * sizeof(u32); /* count of u32 entry */
+
+	dma_sync_single_for_device(vsi_domain->dev, dma, size, DMA_TO_DEVICE);
+}
+
+#define VSI_IOVA_DTE_MASK	0xffc00000
+#define VSI_IOVA_DTE_SHIFT	22
+#define VSI_IOVA_PTE_MASK	0x003ff000
+#define VSI_IOVA_PTE_SHIFT	12
+#define VSI_IOVA_PAGE_MASK	0x00000fff
+#define VSI_IOVA_PAGE_SHIFT	0
+
+static u32 vsi_iova_dte_index(u32 iova)
+{
+	return (iova & VSI_IOVA_DTE_MASK) >> VSI_IOVA_DTE_SHIFT;
+}
+
+static u32 vsi_iova_pte_index(u32 iova)
+{
+	return (iova & VSI_IOVA_PTE_MASK) >> VSI_IOVA_PTE_SHIFT;
+}
+
+static u32 vsi_iova_page_offset(u32 iova)
+{
+	return (iova & VSI_IOVA_PAGE_MASK) >> VSI_IOVA_PAGE_SHIFT;
+}
+
+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);
+}
+
+static irqreturn_t vsi_iommu_irq(int irq, void *dev_id)
+{
+	struct vsi_iommu *iommu = dev_id;
+	unsigned long flags;
+	dma_addr_t iova;
+	u32 status;
+
+	if (pm_runtime_resume_and_get(iommu->dev) < 0)
+		return IRQ_NONE;
+
+	spin_lock_irqsave(&iommu->lock, flags);
+
+	status = readl(iommu->regs + VSI_MMU_STATUS_BASE);
+	if (status & VSI_MMU_IRQ_MASK) {
+		dev_err(iommu->dev, "unexpected int_status=%08x\n", status);
+		iova = readl(iommu->regs + VSI_MMU_PAGE_FAULT_ADDR);
+		report_iommu_fault(iommu->domain, iommu->dev, iova, status);
+	}
+	writel(0, iommu->regs + VSI_MMU_STATUS_BASE);
+
+	spin_unlock_irqrestore(&iommu->lock, flags);
+	pm_runtime_put_autosuspend(iommu->dev);
+
+	return IRQ_HANDLED;
+}
+
+static struct vsi_iommu *vsi_iommu_get_from_dev(struct device *dev)
+{
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+	struct device *iommu_dev = bus_find_device_by_fwnode(&platform_bus_type,
+							     fwspec->iommu_fwnode);
+
+	put_device(iommu_dev);
+
+	return iommu_dev ? dev_get_drvdata(iommu_dev) : NULL;
+}
+
+static struct iommu_domain *vsi_iommu_domain_alloc_paging(struct device *dev)
+{
+	struct vsi_iommu *iommu = dev_iommu_priv_get(dev);
+	struct vsi_iommu_domain *vsi_domain;
+
+	vsi_domain = kzalloc(sizeof(*vsi_domain), GFP_KERNEL);
+	if (!vsi_domain)
+		return NULL;
+
+	vsi_domain->dev = iommu->dev;
+	spin_lock_init(&vsi_domain->lock);
+
+	/*
+	 * iommu use a 2 level pagetable.
+	 * Each level1 (dt) and level2 (pt) table has 1024 4-byte entries.
+	 * Allocate one 4 KiB page for each table.
+	 */
+	vsi_domain->dt = iommu_alloc_pages_sz(GFP_KERNEL | GFP_DMA32,
+					      SPAGE_SIZE);
+	if (!vsi_domain->dt)
+		goto err_free_domain;
+
+	vsi_domain->dt_dma = dma_map_single(vsi_domain->dev, vsi_domain->dt,
+					    SPAGE_SIZE, DMA_TO_DEVICE);
+	if (dma_mapping_error(vsi_domain->dev, vsi_domain->dt_dma)) {
+		dev_err(dev, "DMA map error for DT\n");
+		goto err_free_dt;
+	}
+
+	vsi_domain->pta = iommu_alloc_pages_sz(GFP_KERNEL | GFP_DMA32,
+					       SPAGE_SIZE);
+	if (!vsi_domain->pta)
+		goto err_unmap_dt;
+
+	vsi_domain->pta[0] = vsi_mk_pta(vsi_domain->dt_dma);
+	vsi_domain->pta_dma = dma_map_single(vsi_domain->dev, vsi_domain->pta,
+					     SPAGE_SIZE, DMA_TO_DEVICE);
+	if (dma_mapping_error(vsi_domain->dev, vsi_domain->pta_dma)) {
+		dev_err(dev, "DMA map error for PTA\n");
+		goto err_free_pta;
+	}
+
+	INIT_LIST_HEAD(&vsi_domain->iommus);
+
+	vsi_domain->domain.geometry.aperture_start = 0;
+	vsi_domain->domain.geometry.aperture_end   = DMA_BIT_MASK(32);
+	vsi_domain->domain.geometry.force_aperture = true;
+	vsi_domain->domain.pgsize_bitmap	   = SZ_4K;
+
+	return &vsi_domain->domain;
+
+err_free_pta:
+	iommu_free_pages(vsi_domain->pta);
+err_unmap_dt:
+	dma_unmap_single(vsi_domain->dev, vsi_domain->dt_dma,
+			 SPAGE_SIZE, DMA_TO_DEVICE);
+err_free_dt:
+	iommu_free_pages(vsi_domain->dt);
+err_free_domain:
+	kfree(vsi_domain);
+
+	return NULL;
+}
+
+static phys_addr_t vsi_iommu_iova_to_phys(struct iommu_domain *domain,
+					  dma_addr_t iova)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	phys_addr_t pt_phys, phys = 0;
+	unsigned long flags;
+	u32 dte, pte;
+	u32 *page_table;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+	dte = vsi_domain->dt[vsi_iova_dte_index(iova)];
+	if (!vsi_dte_is_pt_valid(dte))
+		goto unlock;
+
+	pt_phys = vsi_dte_pt_address(dte);
+	page_table = (u32 *)phys_to_virt(pt_phys);
+	pte = page_table[vsi_iova_pte_index(iova)];
+	if (!vsi_pte_is_page_valid(pte))
+		goto unlock;
+
+	phys = vsi_pte_page_address(pte) + vsi_iova_page_offset(iova);
+
+unlock:
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+	return phys;
+}
+
+static size_t vsi_iommu_unmap_iova(struct vsi_iommu_domain *vsi_domain,
+				   u32 *pte_addr, dma_addr_t pte_dma,
+				   size_t size)
+{
+	unsigned int pte_count;
+	unsigned int pte_total = size / SPAGE_SIZE;
+
+	for (pte_count = 0;
+	     pte_count < pte_total && pte_count < NUM_PT_ENTRIES; pte_count++) {
+		u32 pte = pte_addr[pte_count];
+
+		if (!vsi_pte_is_page_valid(pte))
+			break;
+
+		pte_addr[pte_count] = vsi_mk_pte_invalid(pte);
+	}
+
+	vsi_table_flush(vsi_domain, pte_dma, pte_total);
+
+	return pte_count * SPAGE_SIZE;
+}
+
+static int vsi_iommu_map_iova(struct vsi_iommu_domain *vsi_domain, u32 *pte_addr,
+			      dma_addr_t pte_dma, dma_addr_t iova,
+			      phys_addr_t paddr, size_t size, int prot)
+{
+	unsigned int pte_count;
+	unsigned int pte_total = size / SPAGE_SIZE;
+
+	for (pte_count = 0;
+	     pte_count < pte_total && pte_count < NUM_PT_ENTRIES; pte_count++) {
+		u32 pte = pte_addr[pte_count];
+
+		if (vsi_pte_is_page_valid(pte))
+			return (pte_count - 1) * SPAGE_SIZE;
+
+		pte_addr[pte_count] = vsi_mk_pte(paddr, prot);
+
+		paddr += SPAGE_SIZE;
+	}
+
+	vsi_table_flush(vsi_domain, pte_dma, pte_total);
+
+	return 0;
+}
+
+static size_t vsi_iommu_unmap(struct iommu_domain *domain, unsigned long _iova,
+			      size_t size, size_t count, struct iommu_iotlb_gather *gather)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	dma_addr_t pte_dma, iova = (dma_addr_t)_iova;
+	unsigned long flags;
+	phys_addr_t pt_phys;
+	u32 dte;
+	u32 *pte_addr;
+	size_t unmap_size = 0;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+
+	dte = vsi_domain->dt[vsi_iova_dte_index(iova)];
+	/* Just return 0 if iova is unmapped */
+	if (!vsi_dte_is_pt_valid(dte))
+		goto unlock;
+
+	pt_phys = vsi_dte_pt_address(dte);
+	pte_addr = (u32 *)phys_to_virt(pt_phys) + vsi_iova_pte_index(iova);
+	pte_dma = pt_phys + vsi_iova_pte_index(iova) * sizeof(u32);
+	unmap_size = vsi_iommu_unmap_iova(vsi_domain, pte_addr, pte_dma, size);
+
+unlock:
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+
+	return unmap_size;
+}
+
+static u32 *vsi_dte_get_page_table(struct vsi_iommu_domain *vsi_domain,
+				   dma_addr_t iova, gfp_t gfp)
+{
+	u32 *page_table, *dte_addr;
+	u32 dte_index, dte;
+	phys_addr_t pt_phys;
+	dma_addr_t pt_dma;
+	gfp_t flags;
+
+	dte_index = vsi_iova_dte_index(iova);
+	dte_addr = &vsi_domain->dt[dte_index];
+	dte = *dte_addr;
+	if (vsi_dte_is_pt_valid(dte))
+		goto done;
+
+	/* Do not allow to sleep while allocating the buffer */
+	flags = (gfp & ~GFP_KERNEL) | GFP_ATOMIC | GFP_DMA32;
+	page_table = iommu_alloc_pages_sz(flags, PAGE_SIZE);
+	if (!page_table)
+		return ERR_PTR(-ENOMEM);
+
+	pt_dma = dma_map_single(vsi_domain->dev, page_table, PAGE_SIZE, DMA_TO_DEVICE);
+	if (dma_mapping_error(vsi_domain->dev, pt_dma)) {
+		dev_err(vsi_domain->dev, "DMA mapping error while allocating page table\n");
+		iommu_free_pages(page_table);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	dte = vsi_mk_dte(pt_dma);
+	*dte_addr = dte;
+
+	vsi_table_flush(vsi_domain,
+			vsi_domain->dt_dma + dte_index * sizeof(u32), 1);
+done:
+	pt_phys = vsi_dte_pt_address(dte);
+	return (u32 *)phys_to_virt(pt_phys);
+}
+
+static int vsi_iommu_map(struct iommu_domain *domain, unsigned long _iova,
+			 phys_addr_t paddr, size_t size, size_t count,
+			 int prot, gfp_t gfp, size_t *mapped)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	dma_addr_t pte_dma, iova = (dma_addr_t)_iova;
+	u32 *page_table, *pte_addr;
+	u32 dte, pte_index;
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+
+	page_table = vsi_dte_get_page_table(vsi_domain, iova, gfp);
+	if (IS_ERR(page_table)) {
+		spin_unlock_irqrestore(&vsi_domain->lock, flags);
+		return PTR_ERR(page_table);
+	}
+
+	dte = vsi_domain->dt[vsi_iova_dte_index(iova)];
+	pte_index = vsi_iova_pte_index(iova);
+	pte_addr = &page_table[pte_index];
+	pte_dma = vsi_dte_pt_address(dte) + pte_index * sizeof(u32);
+	ret = vsi_iommu_map_iova(vsi_domain, pte_addr, pte_dma, iova,
+				 paddr, size, prot);
+	if (!ret)
+		*mapped = size;
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+
+	return ret;
+}
+
+static void vsi_iommu_disable(struct vsi_iommu *iommu)
+{
+	writel(0, iommu->regs + VSI_MMU_AHB_CONTROL_BASE);
+}
+
+static int vsi_iommu_identity_attach(struct iommu_domain *domain,
+				     struct device *dev, struct iommu_domain *old)
+{
+	struct vsi_iommu *iommu = dev_iommu_priv_get(dev);
+	unsigned long flags;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(iommu->dev);
+	if (ret < 0)
+		return ret;
+
+	spin_lock_irqsave(&iommu->lock, flags);
+	if (iommu->domain == domain)
+		goto unlock;
+
+	vsi_iommu_disable(iommu);
+	list_del_init(&iommu->node);
+
+	iommu->domain = domain;
+
+unlock:
+	spin_unlock_irqrestore(&iommu->lock, flags);
+	pm_runtime_put_autosuspend(iommu->dev);
+	return 0;
+}
+
+static const struct iommu_domain_ops vsi_identity_ops = {
+	.attach_dev = vsi_iommu_identity_attach,
+};
+
+static struct iommu_domain vsi_identity_domain = {
+	.type = IOMMU_DOMAIN_IDENTITY,
+	.ops = &vsi_identity_ops,
+};
+
+static void vsi_iommu_enable(struct vsi_iommu *iommu, struct iommu_domain *domain)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+
+	if (domain == &vsi_identity_domain)
+		return;
+
+	writel(vsi_domain->pta_dma, iommu->regs + VSI_MMU_AHB_TLB_ARRAY_BASE_L_BASE);
+	writel(VSI_MMU_OUT_OF_BOUND, iommu->regs + VSI_MMU_CONFIG1_BASE);
+	writel(VSI_MMU_BIT_ENABLE, iommu->regs + VSI_MMU_AHB_EXCEPTION_BASE);
+	writel(VSI_MMU_BIT_ENABLE, iommu->regs + VSI_MMU_AHB_CONTROL_BASE);
+}
+
+void vsi_iommu_restore_ctx(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;
+
+		iommu = list_entry(pos, struct vsi_iommu, node);
+		if (!iommu)
+			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);
+	}
+
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+}
+EXPORT_SYMBOL_GPL(vsi_iommu_restore_ctx);
+
+static int vsi_iommu_attach_device(struct iommu_domain *domain,
+				   struct device *dev, struct iommu_domain *old)
+{
+	struct vsi_iommu *iommu = dev_iommu_priv_get(dev);
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	unsigned long flags, flags2;
+	int ret = 0;
+
+	ret = pm_runtime_resume_and_get(iommu->dev);
+	if (ret < 0)
+		return ret;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+	spin_lock_irqsave(&iommu->lock, flags2);
+
+	vsi_iommu_enable(iommu, domain);
+	writel(VSI_MMU_BIT_FLUSH, iommu->regs + VSI_MMU_FLUSH_BASE);
+	writel(0, iommu->regs + VSI_MMU_FLUSH_BASE);
+
+	list_del_init(&iommu->node);
+	list_add_tail(&iommu->node, &vsi_domain->iommus);
+
+	iommu->domain = domain;
+
+	spin_unlock_irqrestore(&iommu->lock, flags2);
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+	pm_runtime_put_autosuspend(iommu->dev);
+	return ret;
+}
+
+static void vsi_iommu_domain_free(struct iommu_domain *domain)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	unsigned long flags;
+	int i;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+
+	WARN_ON(!list_empty(&vsi_domain->iommus));
+
+	for (i = 0; i < NUM_DT_ENTRIES; i++) {
+		u32 dte = vsi_domain->dt[i];
+
+		if (vsi_dte_is_pt_valid(dte)) {
+			phys_addr_t pt_phys = vsi_dte_pt_address(dte);
+			u32 *page_table = phys_to_virt(pt_phys);
+
+			dma_unmap_single(vsi_domain->dev, pt_phys,
+					 SPAGE_SIZE, DMA_TO_DEVICE);
+			iommu_free_pages(page_table);
+		}
+	}
+
+	dma_unmap_single(vsi_domain->dev, vsi_domain->dt_dma,
+			 SPAGE_SIZE, DMA_TO_DEVICE);
+	iommu_free_pages(vsi_domain->dt);
+
+	dma_unmap_single(vsi_domain->dev, vsi_domain->pta_dma,
+			 SPAGE_SIZE, DMA_TO_DEVICE);
+	iommu_free_pages(vsi_domain->pta);
+
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+
+	kfree(vsi_domain);
+}
+
+static struct iommu_device *vsi_iommu_probe_device(struct device *dev)
+{
+	struct vsi_iommu *iommu = vsi_iommu_get_from_dev(dev);
+	struct device_link *link;
+
+	link = device_link_add(dev, iommu->dev,
+			       DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
+	if (!link)
+		dev_err(dev, "Unable to link %s\n", dev_name(iommu->dev));
+
+	dev_iommu_priv_set(dev, iommu);
+	return &iommu->iommu;
+}
+
+static void vsi_iommu_release_device(struct device *dev)
+{
+	struct vsi_iommu *iommu = dev_iommu_priv_get(dev);
+
+	device_link_remove(dev, iommu->dev);
+}
+
+static int vsi_iommu_of_xlate(struct device *dev, const struct of_phandle_args *args)
+{
+	return iommu_fwspec_add_ids(dev, args->args, 1);
+}
+
+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,
+		.iova_to_phys		= vsi_iommu_iova_to_phys,
+		.free			= vsi_iommu_domain_free,
+	}
+};
+
+static const struct of_device_id vsi_iommu_dt_ids[] = {
+	{
+		.compatible = "verisilicon,iommu-1.2",
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, vsi_iommu_dt_ids);
+
+static int vsi_iommu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct vsi_iommu *iommu;
+	int err;
+
+	iommu = devm_kzalloc(dev, sizeof(*iommu), GFP_KERNEL);
+	if (!iommu)
+		return -ENOMEM;
+
+	iommu->dev = dev;
+	spin_lock_init(&iommu->lock);
+	INIT_LIST_HEAD(&iommu->node);
+
+	iommu->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(iommu->regs))
+		return -ENOMEM;
+
+	iommu->num_clocks = devm_clk_bulk_get_all(dev, &iommu->clocks);
+	if  (iommu->num_clocks < 0)
+		return iommu->num_clocks;
+
+	err = clk_bulk_prepare(iommu->num_clocks, iommu->clocks);
+	if (err)
+		return err;
+
+	iommu->irq = platform_get_irq(pdev, 0);
+	if (iommu->irq < 0)
+		return iommu->irq;
+
+	err = devm_request_irq(iommu->dev, iommu->irq, vsi_iommu_irq,
+			       IRQF_SHARED, dev_name(dev), iommu);
+	if (err)
+		goto err_unprepare_clocks;
+
+	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+	platform_set_drvdata(pdev, iommu);
+
+	pm_runtime_set_autosuspend_delay(dev, 100);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_enable(dev);
+
+	err = iommu_device_sysfs_add(&iommu->iommu, dev, NULL, dev_name(dev));
+	if (err)
+		goto err_runtime_disable;
+
+	err = iommu_device_register(&iommu->iommu, &vsi_iommu_ops, dev);
+	if (err)
+		goto err_remove_sysfs;
+
+	return 0;
+
+err_remove_sysfs:
+	iommu_device_sysfs_remove(&iommu->iommu);
+err_runtime_disable:
+	pm_runtime_disable(dev);
+err_unprepare_clocks:
+	clk_bulk_unprepare(iommu->num_clocks, iommu->clocks);
+	return err;
+}
+
+static void vsi_iommu_shutdown(struct platform_device *pdev)
+{
+	struct vsi_iommu *iommu = platform_get_drvdata(pdev);
+
+	disable_irq(iommu->irq);
+	pm_runtime_force_suspend(&pdev->dev);
+}
+
+static int __maybe_unused vsi_iommu_suspend(struct device *dev)
+{
+	struct vsi_iommu *iommu = dev_get_drvdata(dev);
+
+	vsi_iommu_disable(iommu);
+
+	clk_bulk_disable(iommu->num_clocks, iommu->clocks);
+
+	return 0;
+}
+
+static int __maybe_unused vsi_iommu_resume(struct device *dev)
+{
+	struct vsi_iommu *iommu = dev_get_drvdata(dev);
+	unsigned long flags, flags2;
+	int ret;
+
+	ret = clk_bulk_enable(iommu->num_clocks, iommu->clocks);
+	if (ret)
+		return ret;
+
+	if (iommu->domain) {
+		struct vsi_iommu_domain *vsi_domain = to_vsi_domain(iommu->domain);
+
+		spin_lock_irqsave(&vsi_domain->lock, flags);
+		spin_lock_irqsave(&iommu->lock, flags2);
+		vsi_iommu_enable(iommu, iommu->domain);
+		spin_unlock_irqrestore(&iommu->lock, flags2);
+		spin_unlock_irqrestore(&vsi_domain->lock, flags);
+	}
+
+	return 0;
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(vsi_iommu_pm_ops,
+				 vsi_iommu_suspend, vsi_iommu_resume,
+				 NULL);
+
+static struct platform_driver rockchip_vsi_iommu_driver = {
+	.probe = vsi_iommu_probe,
+	.shutdown = vsi_iommu_shutdown,
+	.driver = {
+		   .name = "vsi_iommu",
+		   .of_match_table = vsi_iommu_dt_ids,
+		   .pm = pm_sleep_ptr(&vsi_iommu_pm_ops),
+		   .suppress_bind_attrs = true,
+	},
+};
+module_platform_driver(rockchip_vsi_iommu_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Benjamin Gaignard <benjamin.gaignard@collabora.com>");
+MODULE_DESCRIPTION("Verisilicon IOMMU driver");
diff --git a/include/linux/vsi-iommu.h b/include/linux/vsi-iommu.h
new file mode 100644
index 000000000000..81a80f9219f4
--- /dev/null
+++ b/include/linux/vsi-iommu.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * verisilicon iommu: simple virtual address space management
+ *
+ * Copyright (c) 2025, Collabora
+ *
+ * Written by Benjamin Gaignard <benjamin.gaignard@collabora.com>
+ */
+
+#ifndef _VSI_IOMMU_H_
+#define _VSI_IOMMU_H_
+
+struct iommu_domain;
+
+#if IS_ENABLED(CONFIG_VSI_IOMMU)
+void vsi_iommu_restore_ctx(struct iommu_domain *domain);
+#else
+static inline void vsi_iommu_restore_ctx(struct iommu_domain *domain) {}
+#endif
+
+#endif
-- 
2.43.0

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Will Deacon]

Hi Benjamin,

Thanks for posting a v11.

On Wed, Jan 07, 2026 at 11:09:53AM +0100, Benjamin Gaignard wrote:
> 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/

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):

> +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);
> +}

[...]

> +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.

Will

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Benjamin Gaignard]

Le 13/01/2026 à 17:10, Will Deacon a écrit :
> Hi Benjamin,
>
> Thanks for posting a v11.
>
> On Wed, Jan 07, 2026 at 11:09:53AM +0100, Benjamin Gaignard wrote:
>> 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/
>
> 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):
>
>> +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);
>> +}
> [...]
>
>> +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.

Thanks for your review.
Benjamin

>
> Will

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Will Deacon]

On Tue, Jan 13, 2026 at 05:25:38PM +0100, Benjamin Gaignard wrote:
> 
> Le 13/01/2026 à 17:10, Will Deacon a écrit :
> > Hi Benjamin,
> > 
> > Thanks for posting a v11.
> > 
> > On Wed, Jan 07, 2026 at 11:09:53AM +0100, Benjamin Gaignard wrote:
> > > 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/
> > 
> > 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):
> > 
> > > +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);
> > > +}
> > [...]
> > 
> > > +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

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Benjamin Gaignard]

Le 14/01/2026 à 13:59, Will Deacon a écrit :
> On Tue, Jan 13, 2026 at 05:25:38PM +0100, Benjamin Gaignard wrote:
>> Le 13/01/2026 à 17:10, Will Deacon a écrit :
>>> Hi Benjamin,
>>>
>>> Thanks for posting a v11.
>>>
>>> On Wed, Jan 07, 2026 at 11:09:53AM +0100, Benjamin Gaignard wrote:
>>>> 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/
>>>
>>> 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):
>>>
>>>> +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);
>>>> +}
>>> [...]
>>>
>>>> +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?

In vsi_iommu_unmap_iova() page is invalided by calling vsi_mk_pte_invalid().
That clear BIT(0) so the hardware knows the page is invalid.
Do I have miss something here ?

Benjamin

>
> Will
>

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Will Deacon]

On Wed, Jan 14, 2026 at 02:10:48PM +0100, Benjamin Gaignard wrote:
> 
> Le 14/01/2026 à 13:59, Will Deacon a écrit :
> > On Tue, Jan 13, 2026 at 05:25:38PM +0100, Benjamin Gaignard wrote:
> > > Le 13/01/2026 à 17:10, Will Deacon a écrit :
> > > > Hi Benjamin,
> > > > 
> > > > Thanks for posting a v11.
> > > > 
> > > > On Wed, Jan 07, 2026 at 11:09:53AM +0100, Benjamin Gaignard wrote:
> > > > > 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/
> > > > 
> > > > 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):
> > > > 
> > > > > +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);
> > > > > +}
> > > > [...]
> > > > 
> > > > > +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?
> 
> In vsi_iommu_unmap_iova() page is invalided by calling vsi_mk_pte_invalid().

But that just writes an invalid descriptor and doesn't appear to invalidate
the TLB at all.

> That clear BIT(0) so the hardware knows the page is invalid.
> Do I have miss something here ?

Yes, the TLB structure needs to be invalidated so that the page-table
walker sees the new value that you have written in memory.

The rockchip driver gets this correct...

Will

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Benjamin Gaignard]

Le 19/01/2026 à 13:32, Will Deacon a écrit :
> On Wed, Jan 14, 2026 at 02:10:48PM +0100, Benjamin Gaignard wrote:
>> Le 14/01/2026 à 13:59, Will Deacon a écrit :
>>> On Tue, Jan 13, 2026 at 05:25:38PM +0100, Benjamin Gaignard wrote:
>>>> Le 13/01/2026 à 17:10, Will Deacon a écrit :
>>>>> Hi Benjamin,
>>>>>
>>>>> Thanks for posting a v11.
>>>>>
>>>>> On Wed, Jan 07, 2026 at 11:09:53AM +0100, Benjamin Gaignard wrote:
>>>>>> 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/
>>>>>
>>>>> 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):
>>>>>
>>>>>> +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);
>>>>>> +}
>>>>> [...]
>>>>>
>>>>>> +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?
>> In vsi_iommu_unmap_iova() page is invalided by calling vsi_mk_pte_invalid().
> But that just writes an invalid descriptor and doesn't appear to invalidate
> the TLB at all.
>
>> That clear BIT(0) so the hardware knows the page is invalid.
>> Do I have miss something here ?
> Yes, the TLB structure needs to be invalidated so that the page-table
> walker sees the new value that you have written in memory.
>
> The rockchip driver gets this correct...

Rockchip hardware have a ZAP_ONE_LINE register which didn't exist on Verisilicon
hardware.
I have tried to use VSI_MMU_BIT_FLUSH on VSI driver after unmapping iova
but it doesn't work.
So far calling dma_sync_single_for_device() seems to be enough to make iommu
and video decoder work together.

Regards,
Benjamin

> Will
>

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Will Deacon]

On Mon, Jan 19, 2026 at 03:03:44PM +0100, Benjamin Gaignard wrote:
> > > > > > > +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?
> > > In vsi_iommu_unmap_iova() page is invalided by calling vsi_mk_pte_invalid().
> > But that just writes an invalid descriptor and doesn't appear to invalidate
> > the TLB at all.
> > 
> > > That clear BIT(0) so the hardware knows the page is invalid.
> > > Do I have miss something here ?
> > Yes, the TLB structure needs to be invalidated so that the page-table
> > walker sees the new value that you have written in memory.
> > 
> > The rockchip driver gets this correct...
> 
> Rockchip hardware have a ZAP_ONE_LINE register which didn't exist on Verisilicon
> hardware.

Presumably you have some sort of Verisilicon datasheet or downstream driver
from which you can infer the TLB invalidation runes?

> I have tried to use VSI_MMU_BIT_FLUSH on VSI driver after unmapping iova
> but it doesn't work.

What do you mean by "doesn't work"? If it works without doing any
invalidation at all, then it's very peculiar that adding the invalidation
would introduce issues.

> So far calling dma_sync_single_for_device() seems to be enough to make iommu
> and video decoder work together.

I don't think we should settle for "seems to enough"! If we can reason
about the operation of the hardware then the driver will be undebuggable
when it eventually goes wrong.

Will

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Benjamin Gaignard]

Le 21/01/2026 à 13:51, Will Deacon a écrit :
> On Mon, Jan 19, 2026 at 03:03:44PM +0100, Benjamin Gaignard wrote:
>>>>>>>> +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?
>>>> In vsi_iommu_unmap_iova() page is invalided by calling vsi_mk_pte_invalid().
>>> But that just writes an invalid descriptor and doesn't appear to invalidate
>>> the TLB at all.
>>>
>>>> That clear BIT(0) so the hardware knows the page is invalid.
>>>> Do I have miss something here ?
>>> Yes, the TLB structure needs to be invalidated so that the page-table
>>> walker sees the new value that you have written in memory.
>>>
>>> The rockchip driver gets this correct...
>> Rockchip hardware have a ZAP_ONE_LINE register which didn't exist on Verisilicon
>> hardware.
> Presumably you have some sort of Verisilicon datasheet or downstream driver
> from which you can infer the TLB invalidation runes?

I have only this downstream driver:
https://github.com/rockchip-linux/kernel/blob/develop-6.1/drivers/iommu/rockchip-iommu-av1d.c
No datasheet...

>
>> I have tried to use VSI_MMU_BIT_FLUSH on VSI driver after unmapping iova
>> but it doesn't work.
> What do you mean by "doesn't work"? If it works without doing any
> invalidation at all, then it's very peculiar that adding the invalidation
> would introduce issues.

I mean VSI_MMU_BIT_FLUSH register can't be used to invalid the TLB.
I think the hardware iterates over the pages tables in memory and
check the valid/invalid bit.

Benjamin

>
>> So far calling dma_sync_single_for_device() seems to be enough to make iommu
>> and video decoder work together.
> I don't think we should settle for "seems to enough"! If we can reason
> about the operation of the hardware then the driver will be undebuggable
> when it eventually goes wrong.
>
> Will
>

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Will Deacon]

On Wed, Jan 21, 2026 at 02:50:18PM +0100, Benjamin Gaignard wrote:
> 
> Le 21/01/2026 à 13:51, Will Deacon a écrit :
> > On Mon, Jan 19, 2026 at 03:03:44PM +0100, Benjamin Gaignard wrote:
> > > > > > > > > +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?
> > > > > In vsi_iommu_unmap_iova() page is invalided by calling vsi_mk_pte_invalid().
> > > > But that just writes an invalid descriptor and doesn't appear to invalidate
> > > > the TLB at all.
> > > > 
> > > > > That clear BIT(0) so the hardware knows the page is invalid.
> > > > > Do I have miss something here ?
> > > > Yes, the TLB structure needs to be invalidated so that the page-table
> > > > walker sees the new value that you have written in memory.
> > > > 
> > > > The rockchip driver gets this correct...
> > > Rockchip hardware have a ZAP_ONE_LINE register which didn't exist on Verisilicon
> > > hardware.
> > Presumably you have some sort of Verisilicon datasheet or downstream driver
> > from which you can infer the TLB invalidation runes?
> 
> I have only this downstream driver:
> https://github.com/rockchip-linux/kernel/blob/develop-6.1/drivers/iommu/rockchip-iommu-av1d.c
> No datasheet...
> 
> > 
> > > I have tried to use VSI_MMU_BIT_FLUSH on VSI driver after unmapping iova
> > > but it doesn't work.
> > What do you mean by "doesn't work"? If it works without doing any
> > invalidation at all, then it's very peculiar that adding the invalidation
> > would introduce issues.
> 
> I mean VSI_MMU_BIT_FLUSH register can't be used to invalid the TLB.
> I think the hardware iterates over the pages tables in memory and
> check the valid/invalid bit.

I bet it doesn't: that would be horrible for performance.

The hardware clearly has TLB invalidation support, as the downstream driver
that you linked above implements av1_iommu_flush_tlb_all() to poke it.
If the hardware has a TLB, then unmapping a page-table means you need to:

1. Clear the valid bit from the descriptor in memory
2. Have some sort of memory barrier
3. Invalidate the TLB
4. Wait for the invalidation to complete

All IOMMUs tend to work like that and I don't think this one is any
different.

Will

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Benjamin Gaignard]

Le 23/01/2026 à 18:14, Will Deacon a écrit :
> On Wed, Jan 21, 2026 at 02:50:18PM +0100, Benjamin Gaignard wrote:
>> Le 21/01/2026 à 13:51, Will Deacon a écrit :
>>> On Mon, Jan 19, 2026 at 03:03:44PM +0100, Benjamin Gaignard wrote:
>>>>>>>>>> +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?
>>>>>> In vsi_iommu_unmap_iova() page is invalided by calling vsi_mk_pte_invalid().
>>>>> But that just writes an invalid descriptor and doesn't appear to invalidate
>>>>> the TLB at all.
>>>>>
>>>>>> That clear BIT(0) so the hardware knows the page is invalid.
>>>>>> Do I have miss something here ?
>>>>> Yes, the TLB structure needs to be invalidated so that the page-table
>>>>> walker sees the new value that you have written in memory.
>>>>>
>>>>> The rockchip driver gets this correct...
>>>> Rockchip hardware have a ZAP_ONE_LINE register which didn't exist on Verisilicon
>>>> hardware.
>>> Presumably you have some sort of Verisilicon datasheet or downstream driver
>>> from which you can infer the TLB invalidation runes?
>> I have only this downstream driver:
>> https://github.com/rockchip-linux/kernel/blob/develop-6.1/drivers/iommu/rockchip-iommu-av1d.c
>> No datasheet...
>>
>>>> I have tried to use VSI_MMU_BIT_FLUSH on VSI driver after unmapping iova
>>>> but it doesn't work.
>>> What do you mean by "doesn't work"? If it works without doing any
>>> invalidation at all, then it's very peculiar that adding the invalidation
>>> would introduce issues.
>> I mean VSI_MMU_BIT_FLUSH register can't be used to invalid the TLB.
>> I think the hardware iterates over the pages tables in memory and
>> check the valid/invalid bit.
> I bet it doesn't: that would be horrible for performance.
>
> The hardware clearly has TLB invalidation support, as the downstream driver
> that you linked above implements av1_iommu_flush_tlb_all() to poke it.
> If the hardware has a TLB, then unmapping a page-table means you need to:
>
> 1. Clear the valid bit from the descriptor in memory
> 2. Have some sort of memory barrier
> 3. Invalidate the TLB
> 4. Wait for the invalidation to complete

That exactly what I had tried to do by calling vsi_iommu_flush_tlb_all() (minux the lock)
after calling vsi_iommu_unmap_iova() in vsi_iommu_unmap() but that doesn't work
and even make the system crash sometimes.

Benjamin

>
> All IOMMUs tend to work like that and I don't think this one is any
> different.
>
> Will
>

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Will Deacon]

On Mon, Jan 26, 2026 at 10:03:19AM +0100, Benjamin Gaignard wrote:
> 
> Le 23/01/2026 à 18:14, Will Deacon a écrit :
> > On Wed, Jan 21, 2026 at 02:50:18PM +0100, Benjamin Gaignard wrote:
> > > Le 21/01/2026 à 13:51, Will Deacon a écrit :
> > > > On Mon, Jan 19, 2026 at 03:03:44PM +0100, Benjamin Gaignard wrote:
> > > > > > > > > > > +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?
> > > > > > > In vsi_iommu_unmap_iova() page is invalided by calling vsi_mk_pte_invalid().
> > > > > > But that just writes an invalid descriptor and doesn't appear to invalidate
> > > > > > the TLB at all.
> > > > > > 
> > > > > > > That clear BIT(0) so the hardware knows the page is invalid.
> > > > > > > Do I have miss something here ?
> > > > > > Yes, the TLB structure needs to be invalidated so that the page-table
> > > > > > walker sees the new value that you have written in memory.
> > > > > > 
> > > > > > The rockchip driver gets this correct...
> > > > > Rockchip hardware have a ZAP_ONE_LINE register which didn't exist on Verisilicon
> > > > > hardware.
> > > > Presumably you have some sort of Verisilicon datasheet or downstream driver
> > > > from which you can infer the TLB invalidation runes?
> > > I have only this downstream driver:
> > > https://github.com/rockchip-linux/kernel/blob/develop-6.1/drivers/iommu/rockchip-iommu-av1d.c
> > > No datasheet...
> > > 
> > > > > I have tried to use VSI_MMU_BIT_FLUSH on VSI driver after unmapping iova
> > > > > but it doesn't work.
> > > > What do you mean by "doesn't work"? If it works without doing any
> > > > invalidation at all, then it's very peculiar that adding the invalidation
> > > > would introduce issues.
> > > I mean VSI_MMU_BIT_FLUSH register can't be used to invalid the TLB.
> > > I think the hardware iterates over the pages tables in memory and
> > > check the valid/invalid bit.
> > I bet it doesn't: that would be horrible for performance.
> > 
> > The hardware clearly has TLB invalidation support, as the downstream driver
> > that you linked above implements av1_iommu_flush_tlb_all() to poke it.
> > If the hardware has a TLB, then unmapping a page-table means you need to:
> > 
> > 1. Clear the valid bit from the descriptor in memory
> > 2. Have some sort of memory barrier
> > 3. Invalidate the TLB
> > 4. Wait for the invalidation to complete
> 
> That exactly what I had tried to do by calling vsi_iommu_flush_tlb_all() (minux the lock)
> after calling vsi_iommu_unmap_iova() in vsi_iommu_unmap() but that doesn't work
> and even make the system crash sometimes.

Then it sounds like you have some debugging to do...

I don't think we should elide the TLB invalidation just because you
couldn't get it to work.

Will

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Benjamin Gaignard]

Le 26/01/2026 à 15:19, Will Deacon a écrit :
> On Mon, Jan 26, 2026 at 10:03:19AM +0100, Benjamin Gaignard wrote:
>> Le 23/01/2026 à 18:14, Will Deacon a écrit :
>>> On Wed, Jan 21, 2026 at 02:50:18PM +0100, Benjamin Gaignard wrote:
>>>> Le 21/01/2026 à 13:51, Will Deacon a écrit :
>>>>> On Mon, Jan 19, 2026 at 03:03:44PM +0100, Benjamin Gaignard wrote:
>>>>>>>>>>>> +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?
>>>>>>>> In vsi_iommu_unmap_iova() page is invalided by calling vsi_mk_pte_invalid().
>>>>>>> But that just writes an invalid descriptor and doesn't appear to invalidate
>>>>>>> the TLB at all.
>>>>>>>
>>>>>>>> That clear BIT(0) so the hardware knows the page is invalid.
>>>>>>>> Do I have miss something here ?
>>>>>>> Yes, the TLB structure needs to be invalidated so that the page-table
>>>>>>> walker sees the new value that you have written in memory.
>>>>>>>
>>>>>>> The rockchip driver gets this correct...
>>>>>> Rockchip hardware have a ZAP_ONE_LINE register which didn't exist on Verisilicon
>>>>>> hardware.
>>>>> Presumably you have some sort of Verisilicon datasheet or downstream driver
>>>>> from which you can infer the TLB invalidation runes?
>>>> I have only this downstream driver:
>>>> https://github.com/rockchip-linux/kernel/blob/develop-6.1/drivers/iommu/rockchip-iommu-av1d.c
>>>> No datasheet...
>>>>
>>>>>> I have tried to use VSI_MMU_BIT_FLUSH on VSI driver after unmapping iova
>>>>>> but it doesn't work.
>>>>> What do you mean by "doesn't work"? If it works without doing any
>>>>> invalidation at all, then it's very peculiar that adding the invalidation
>>>>> would introduce issues.
>>>> I mean VSI_MMU_BIT_FLUSH register can't be used to invalid the TLB.
>>>> I think the hardware iterates over the pages tables in memory and
>>>> check the valid/invalid bit.
>>> I bet it doesn't: that would be horrible for performance.
>>>
>>> The hardware clearly has TLB invalidation support, as the downstream driver
>>> that you linked above implements av1_iommu_flush_tlb_all() to poke it.
>>> If the hardware has a TLB, then unmapping a page-table means you need to:
>>>
>>> 1. Clear the valid bit from the descriptor in memory
>>> 2. Have some sort of memory barrier
>>> 3. Invalidate the TLB
>>> 4. Wait for the invalidation to complete
>> That exactly what I had tried to do by calling vsi_iommu_flush_tlb_all() (minux the lock)
>> after calling vsi_iommu_unmap_iova() in vsi_iommu_unmap() but that doesn't work
>> and even make the system crash sometimes.
> Then it sounds like you have some debugging to do...
>
> I don't think we should elide the TLB invalidation just because you
> couldn't get it to work.

It is working but not in the order you expect.
TLB invalidation occurs before each decoding frames by calling vsi_iommu_restore_ctx().
After that decoder map all the needed buffer and perform decoding.

Benjamin

>
> Will
>

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Benjamin Gaignard]

Le 26/01/2026 à 15:19, Will Deacon a écrit :
> On Mon, Jan 26, 2026 at 10:03:19AM +0100, Benjamin Gaignard wrote:
>> Le 23/01/2026 à 18:14, Will Deacon a écrit :
>>> On Wed, Jan 21, 2026 at 02:50:18PM +0100, Benjamin Gaignard wrote:
>>>> Le 21/01/2026 à 13:51, Will Deacon a écrit :
>>>>> On Mon, Jan 19, 2026 at 03:03:44PM +0100, Benjamin Gaignard wrote:
>>>>>>>>>>>> +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?
>>>>>>>> In vsi_iommu_unmap_iova() page is invalided by calling vsi_mk_pte_invalid().
>>>>>>> But that just writes an invalid descriptor and doesn't appear to invalidate
>>>>>>> the TLB at all.
>>>>>>>
>>>>>>>> That clear BIT(0) so the hardware knows the page is invalid.
>>>>>>>> Do I have miss something here ?
>>>>>>> Yes, the TLB structure needs to be invalidated so that the page-table
>>>>>>> walker sees the new value that you have written in memory.
>>>>>>>
>>>>>>> The rockchip driver gets this correct...
>>>>>> Rockchip hardware have a ZAP_ONE_LINE register which didn't exist on Verisilicon
>>>>>> hardware.
>>>>> Presumably you have some sort of Verisilicon datasheet or downstream driver
>>>>> from which you can infer the TLB invalidation runes?
>>>> I have only this downstream driver:
>>>> https://github.com/rockchip-linux/kernel/blob/develop-6.1/drivers/iommu/rockchip-iommu-av1d.c
>>>> No datasheet...
>>>>
>>>>>> I have tried to use VSI_MMU_BIT_FLUSH on VSI driver after unmapping iova
>>>>>> but it doesn't work.
>>>>> What do you mean by "doesn't work"? If it works without doing any
>>>>> invalidation at all, then it's very peculiar that adding the invalidation
>>>>> would introduce issues.
>>>> I mean VSI_MMU_BIT_FLUSH register can't be used to invalid the TLB.
>>>> I think the hardware iterates over the pages tables in memory and
>>>> check the valid/invalid bit.
>>> I bet it doesn't: that would be horrible for performance.
>>>
>>> The hardware clearly has TLB invalidation support, as the downstream driver
>>> that you linked above implements av1_iommu_flush_tlb_all() to poke it.
>>> If the hardware has a TLB, then unmapping a page-table means you need to:
>>>
>>> 1. Clear the valid bit from the descriptor in memory
>>> 2. Have some sort of memory barrier
>>> 3. Invalidate the TLB
>>> 4. Wait for the invalidation to complete
>> That exactly what I had tried to do by calling vsi_iommu_flush_tlb_all() (minux the lock)
>> after calling vsi_iommu_unmap_iova() in vsi_iommu_unmap() but that doesn't work
>> and even make the system crash sometimes.
> Then it sounds like you have some debugging to do...
>
> I don't think we should elide the TLB invalidation just because you
> couldn't get it to work.

Hi Will,

I have send a v13 of this patchset where I had included Joerg's comments.
V13 doesn't change how the TLB invalidation is done.
I have look everywhere and there is no additional bit(s) or register(s) to
do what you ask.
V13 is update to date and rebased on v6.19. Unless I receive new documents
or specifications about the hardware I don't plan to work on this anymore.

Regards,
Benjamin

>
> Will
>

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Jörg Rödel]

Benjamin,

On Tue, Jan 13, 2026 at 04:10:51PM +0000, Will Deacon wrote:
> 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?

When updating for v12, can you please put an explanatory comment at the top of
the file explaining the relationship of the IP this driver is for to the
RockChip IOMMU and the rationale for having it as a separate driver? I want
this part of the discussion documented in the code in case it comes up again.


-Joerg

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Benjamin Gaignard]

Le 18/01/2026 à 10:41, Jörg Rödel a écrit :
> Benjamin,
>
> On Tue, Jan 13, 2026 at 04:10:51PM +0000, Will Deacon wrote:
>> 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?
> When updating for v12, can you please put an explanatory comment at the top of
> the file explaining the relationship of the IP this driver is for to the
> RockChip IOMMU and the rationale for having it as a separate driver? I want
> this part of the discussion documented in the code in case it comes up again.

I have already send v12 where I have removed the useless function.

Regards,
Benjamin

>
>
> -Joerg
>

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Jörg Rödel]

On Mon, Jan 19, 2026 at 08:51:46AM +0100, Benjamin Gaignard wrote:
> I have already send v12 where I have removed the useless function.

Then either send a patch on-top or a v13.

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Benjamin Gaignard]

Le 18/01/2026 à 10:41, Jörg Rödel a écrit :
> Benjamin,
>
> On Tue, Jan 13, 2026 at 04:10:51PM +0000, Will Deacon wrote:
>> 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?
> When updating for v12, can you please put an explanatory comment at the top of
> the file explaining the relationship of the IP this driver is for to the
> RockChip IOMMU and the rationale for having it as a separate driver? I want
> this part of the discussion documented in the code in case it comes up again.

Does the follow wording summarize the situation correctly for you if I put it
in header comment ?

  * This hardware block is using a 2 pages tables allocation structure.
  * That make very similar to Rockhip iommu hardware blocks but it has
  * it own driver because the registers offset and configuration bits
  * are completely different. An additional raison is that this hardware
  * has been developped by Verisilicon to be used by their hardware video
  * decoders and for a general purpose like Rockchip iommus.

Regards,
Benjamin

>
>
> -Joerg
>

Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

[Jörg Rödel]

On Mon, Jan 19, 2026 at 11:28:11AM +0100, Benjamin Gaignard wrote:
>  * This hardware block is using a 2 pages tables allocation structure.
>  * That make very similar to Rockhip iommu hardware blocks but it has
>  * it own driver because the registers offset and configuration bits
>  * are completely different. An additional raison is that this hardware
>  * has been developped by Verisilicon to be used by their hardware video
>  * decoders and for a general purpose like Rockchip iommus.

Please run a spell checker on this (e.g. raion -> reason, developped ->
developed) and I think the last sentence lacks a "not". Looks good otherwise.

-Joerg

[PATCH v11 4/7] MAINTAINERS: Add entry for Verisilicon IOMMU driver

[Benjamin Gaignard]

Add maintainer for Verisilicon iommu driver.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a0dd762f5648..baf09a3d60b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27315,6 +27315,14 @@ F:	drivers/media/v4l2-core/v4l2-isp.c
 F:	include/media/v4l2-isp.h
 F:	include/uapi/linux/media/v4l2-isp.h
 
+VERISILICON IOMMU DRIVER
+M:	Benjamin Gaignard <benjamin.gaignard@collabora.com>
+L:	iommu@lists.linux.dev
+S:	Maintained
+F:	Documentation/devicetree/bindings/iommu/verisilicon,iommu.yaml
+F:	drivers/iommu/vsi-iommu.c
+F:	include/linux/vsi-iommu.h
+
 VF610 NAND DRIVER
 M:	Stefan Agner <stefan@agner.ch>
 L:	linux-mtd@lists.infradead.org
-- 
2.43.0

[PATCH v11 5/7] media: verisilicon: AV1: Restore IOMMU context before decoding a frame

[Benjamin Gaignard]

AV1 is a stateless decoder which means multiple AV1 bitstreams could be
decoded at the same time using the same hardware block. Before decoding
a frame it is needed to restore the iommu tables to avoid mixing decode
contexts.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 .../verisilicon/rockchip_vpu981_hw_av1_dec.c      | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
index e4703bb6be7c..d9e68e0ded68 100644
--- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
+++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
@@ -5,6 +5,9 @@
  * Author: Benjamin Gaignard <benjamin.gaignard@collabora.com>
  */
 
+#include <linux/iommu.h>
+#include <linux/vsi-iommu.h>
+
 #include <media/v4l2-mem2mem.h>
 #include "hantro.h"
 #include "hantro_v4l2.h"
@@ -2095,12 +2098,24 @@ rockchip_vpu981_av1_dec_set_output_buffer(struct hantro_ctx *ctx)
 	hantro_write_addr(vpu, AV1_TILE_OUT_MV, mv_addr);
 }
 
+static void rockchip_vpu981_av1_restore_iommu(struct hantro_ctx *ctx)
+{
+	struct iommu_domain *domain;
+
+	/* Before decoding any frame iommu context need to be restored */
+	domain = iommu_get_domain_for_dev(ctx->dev->v4l2_dev.dev);
+	if (domain)
+		vsi_iommu_restore_ctx(domain);
+}
+
 int rockchip_vpu981_av1_dec_run(struct hantro_ctx *ctx)
 {
 	struct hantro_dev *vpu = ctx->dev;
 	struct vb2_v4l2_buffer *vb2_src;
 	int ret;
 
+	rockchip_vpu981_av1_restore_iommu(ctx);
+
 	hantro_start_prepare_run(ctx);
 
 	ret = rockchip_vpu981_av1_dec_prepare_run(ctx);
-- 
2.43.0

[PATCH v11 6/7] arm64: dts: rockchip: Add verisilicon IOMMU node on RK3588

[Benjamin Gaignard]

Add the device tree node for the Verisilicon IOMMU present
in the RK3588 SoC.
This IOMMU handles address translation for the VPU hardware blocks.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 2a7921793020..acff8bb3a612 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -1364,6 +1364,17 @@ av1d: video-codec@fdc70000 {
 		clock-names = "aclk", "hclk";
 		power-domains = <&power RK3588_PD_AV1>;
 		resets = <&cru SRST_A_AV1>, <&cru SRST_P_AV1>, <&cru SRST_A_AV1_BIU>, <&cru SRST_P_AV1_BIU>;
+		iommus = <&av1d_mmu>;
+	};
+
+	av1d_mmu: iommu@fdca0000 {
+		compatible = "rockchip,rk3588-av1-iommu", "verisilicon,iommu-1.2";
+		reg = <0x0 0xfdca0000 0x0 0x600>;
+		interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru ACLK_AV1>, <&cru PCLK_AV1>;
+		clock-names = "core", "iface";
+		#iommu-cells = <0>;
+		power-domains = <&power RK3588_PD_AV1>;
 	};
 
 	vop: vop@fdd90000 {
-- 
2.43.0

[PATCH v11 7/7] arm64: defconfig: enable Verisilicon IOMMU for Rockchip RK3588

[Benjamin Gaignard]

Enable Verisilicon IOMMU used by Rockchip RK3588 AV1 hardware codec.
This hardware block could be found in Radxa Rock 5B board.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 45288ec9eaf7..4751f7fa8bb0 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1525,6 +1525,7 @@ CONFIG_ARM_SMMU=y
 CONFIG_ARM_SMMU_V3=y
 CONFIG_MTK_IOMMU=y
 CONFIG_QCOM_IOMMU=y
+CONFIG_VSI_IOMMU=y
 CONFIG_REMOTEPROC=y
 CONFIG_IMX_REMOTEPROC=y
 CONFIG_MTK_SCP=m
-- 
2.43.0