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

[Will Deacon <will@kernel.org>]

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.

Will