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] [day] [month] [year] [list]

Message-ID: <aH5yR9CkYSJ4PaZV@willie-the-truck>
Date: Mon, 21 Jul 2025 18:00:55 +0100
From: Will Deacon <will@...nel.org>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: Robin Murphy <robin.murphy@....com>,
	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, 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 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