| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <baa1fcde-f167-4a1b-afca-0a2957a688cc@collabora.com>
Date: Mon, 14 Jul 2025 16:56:53 +0200
From: Benjamin Gaignard <benjamin.gaignard@...labora.com>
To: Will Deacon <will@...nel.org>
Cc: joro@...tes.org, robin.murphy@....com, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, heiko@...ech.de,
nicolas.dufresne@...labora.com, jgg@...pe.ca, iommu@...ts.linux.dev,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
kernel@...labora.com
Subject: Re: [PATCH v6 3/5] iommu: Add verisilicon IOMMU driver
Le 14/07/2025 à 14:08, Will Deacon a écrit :
> Hi,
>
> On Thu, Jul 10, 2025 at 10:24:44AM +0200, Benjamin Gaignard wrote:
>> 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@...k-chips.com>
>> + * Simon Xue <xxm@...k-chips.com>
>> + * Benjamin Gaignard <benjamin.gaignard@...labora.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:
No they comes from different IP providers, this one is from Verisilicon.
I agree they looks very similar and my first attempt was to add it into
rockchip-iommu code but when doing it I realize that registers addresses
where all different so I had to code all the functions twice.
Regards,
Benjamin
>
>> +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
Powered by blists - more mailing lists