linux-kernel - Re: [PATCH v6 3/5] iommu: Add verisilicon IOMMU driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aHpQXy-bnwW56rCn@willie-the-truck>
Date: Fri, 18 Jul 2025 14:47:11 +0100
From: Will Deacon <will@...nel.org>
To: Robin Murphy <robin.murphy@....com>
Cc: Benjamin Gaignard <benjamin.gaignard@...labora.com>, joro@...tes.org,
	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

On Fri, Jul 18, 2025 at 01:56:36PM +0100, Robin Murphy wrote:
> On 2025-07-18 12:45 pm, Will Deacon wrote:
> > On Mon, Jul 14, 2025 at 04:56:53PM +0200, Benjamin Gaignard wrote:
> > > Le 14/07/2025 à 14:08, Will Deacon a écrit :
> > > > On Thu, Jul 10, 2025 at 10:24:44AM +0200, Benjamin Gaignard wrote:
> > > > > +/* 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.
> > 
> > Understood, and I'm not suggesting to merge the drivers or try to
> > consolidate the register layouts. What I _am_ saying is that the
> > in-memory page-table format should be factored out in a way that can
> > be reused by the two drivers and also that some of the logic (as highlighted
> > by vsi_iommu_domain_free()) is practically identical between the drivers
> > and should also be shared.
> 
> All they really have in common is that they're 2-level formats with 32-bit
> PTEs and 10 bits per level (as is tegra-smmu too). The permission encodings
> have some overlap but are not fully equivalent. Crucially, the schemes for
> packing >32-bit PAs into PTEs are incompatibly different, so if you're
> really keen to genericise things to *that* extent, that's what Jason's
> already working on.
> 
> As for domain_free, you could argue that it also looks basically the same as
> exynos_iommu_domain_free(), because at the end of the day, there's only so
> many ways to free a 2-level pagetable, and it's at least better than what,
> say, sun50i, omap or tegra are doing (or rather, not doing...)

Just because the existing drivers are a mess doesn't mean we should
proliferate it!

Will