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

[Will Deacon <will@kernel.org>]

On Fri, Jul 18, 2025 at 11:14:01AM -0300, Jason Gunthorpe wrote:
> On Fri, Jul 18, 2025 at 02:47:11PM +0100, Will Deacon wrote:
> 
> > Just because the existing drivers are a mess doesn't mean we should
> > proliferate it!
> 
> If you want to insist on something here it should be for this driver
> to use the new generic page table code I've written.
> 
> Otherwise I don't see the point in trying to improve this in some
> lesser way.
> 
> If this had come in a years time I would probably insist on that, but
> right now it isn't merged yet and it will still be a little bit before
> people have time to review it.
> 
> Perhaps a compromise where Benjamin comes with an iommupt format
> header that works for this and we can progress this series and be
> ready to swap it out down the road?

I went back and applied the verisilicon patches locally so that I could
look at them side-by-side with the rockchip driver. Even then, setting
aside the generic page-table code (which I agree is premature to start
insisting on for new drivers), the callbacks for .default_domain_ops()
are very clearly doing the same thing:

.attach_dev:
	The two big differences are that (1) the VSI driver has two
	locks instead of one (and it makes me wonder about the RK
	locking in the IRQ handler and suspend/resume) and (2) the VSI
	hardware has a TLB flush register whereas the RK driver does
	a disable/enable cycle.

.map_pages:
	Basically the same but note that the RK driver _already_ has a
	hook in 'rk_ops' for decoding the DTE

.unmap_pages:
	The big difference here is that the RK driver has TLB
	invalidation whereas I don't think the VSI one does. Yes, it
	implements .flush_iotlb_all, but that's not used any more (and
	we should probably try to remove it again).

.iova_to_phys:
	Same comments as .map_pages.

.free:
	The only difference is that the VSI driver has to free its
	single-entry top-level (the "PTA").

and so moving these somewhere where they can be shared just seems like
the obvious, straightforward thing to do.

Will