Re: [PATCH v6 3/5] iommu: Add verisilicon IOMMU driver
From: Will Deacon <will@kernel.org>
Date: 2025-07-14 12:08:37
Also in:
linux-devicetree, linux-iommu, linux-rockchip, lkml
Hi, On Thu, Jul 10, 2025 at 10:24:44AM +0200, Benjamin Gaignard wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/iommu/vsi-iommu.c b/drivers/iommu/vsi-iommu.c new file mode 100644 index 000000000000..15322b9929af --- /dev/null +++ b/drivers/iommu/vsi-iommu.c@@ -0,0 +1,781 @@ +// 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 "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; + 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; +}; + +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)
How does this page-table format relate to the one supported already by rockchip-iommu.c? From a quick glance, I suspect this is a derivative and so ideally we'd be able to have a common implementation of the page-table code which can be used by both of the drivers. Similarly:
+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);
+}is almost a carbon copy of rk_iommu_domain_free(), so it seems that there's room for code re-use even beyond the page-table support. I think that also means we'll want Heiko's Ack before we merge anything. Will