linux-kernel - Re: [PATCH v11 3/7] 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: <aWeTQ50DOtntcniN@willie-the-truck>
Date: Wed, 14 Jan 2026 12:59:47 +0000
From: Will Deacon <will@...nel.org>
To: Benjamin Gaignard <benjamin.gaignard@...labora.com>
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, p.zabel@...gutronix.de,
	mchehab@...nel.org, iommu@...ts.linux.dev,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-rockchip@...ts.infradead.org, linux-media@...r.kernel.org,
	kernel@...labora.com
Subject: Re: [PATCH v11 3/7] iommu: Add verisilicon IOMMU driver

On Tue, Jan 13, 2026 at 05:25:38PM +0100, Benjamin Gaignard wrote:
> 
> Le 13/01/2026 à 17:10, Will Deacon a écrit :
> > Hi Benjamin,
> > 
> > Thanks for posting a v11.
> > 
> > On Wed, Jan 07, 2026 at 11:09:53AM +0100, Benjamin Gaignard wrote:
> > > The Verisilicon IOMMU hardware block can be found in combination
> > > with Verisilicon hardware video codecs (encoders or decoders) on
> > > different SoCs.
> > > Enable it will allow us to use non contiguous memory allocators
> > > for Verisilicon video codecs.
> > > If both decoder and this iommu driver are compiled has modules
> > > there is undefined symboles issues so this iommu driver could
> > > only be compiled has built-in.
> > > 
> > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...labora.com>
> > > ---
> > > changes in version 11:
> > > - Fix dependency issue when decoder driver is build as module.
> > > 
> > >   drivers/iommu/Kconfig     |  11 +
> > >   drivers/iommu/Makefile    |   1 +
> > >   drivers/iommu/vsi-iommu.c | 808 ++++++++++++++++++++++++++++++++++++++
> > >   include/linux/vsi-iommu.h |  21 +
> > >   4 files changed, 841 insertions(+)
> > >   create mode 100644 drivers/iommu/vsi-iommu.c
> > >   create mode 100644 include/linux/vsi-iommu.h
> > Based on your reply to v9:
> > 
> > https://lore.kernel.org/all/0eff8b1a-c45f-47b1-a871-59f4a0101f0f@collabora.com/
> > 
> > I took another look at this to see whether it had changed significantly
> > from v6 when compared to the rockchip driver. Sadly, they still look
> > very similar to me and I continue to suspect that the hardware is a
> > derivative. I really don't understand why having a shared implementation
> > of the default domain ops is difficult or controversial. Have you tried
> > to write it?
> > 
> > However, given that nobody from the Rockchip side has contributed to the
> > discussion and you claim that this is a distinct piece of IP, I don't
> > want to block the merging of the driver by leaving the conversation
> > hanging.
> > 
> > There is still one thing I don't understand (which, amusingly, the
> > rockchip driver doesn't seem to suffer from):
> > 
> > > +static void vsi_iommu_flush_tlb_all(struct iommu_domain *domain)
> > > +{
> > > +	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
> > > +	struct list_head *pos;
> > > +	unsigned long flags;
> > > +
> > > +	spin_lock_irqsave(&vsi_domain->lock, flags);
> > > +
> > > +	list_for_each(pos, &vsi_domain->iommus) {
> > > +		struct vsi_iommu *iommu;
> > > +		int ret;
> > > +
> > > +		iommu = list_entry(pos, struct vsi_iommu, node);
> > > +		ret = pm_runtime_resume_and_get(iommu->dev);
> > > +		if (ret < 0)
> > > +			continue;
> > > +
> > > +		spin_lock(&iommu->lock);
> > > +
> > > +		writel(VSI_MMU_BIT_FLUSH, iommu->regs + VSI_MMU_FLUSH_BASE);
> > > +		writel(0, iommu->regs + VSI_MMU_FLUSH_BASE);
> > > +
> > > +		spin_unlock(&iommu->lock);
> > > +		pm_runtime_put_autosuspend(iommu->dev);
> > > +	}
> > > +
> > > +	spin_unlock_irqrestore(&vsi_domain->lock, flags);
> > > +}
> > [...]
> > 
> > > +static const struct iommu_ops vsi_iommu_ops = {
> > > +	.identity_domain = &vsi_identity_domain,
> > > +	.release_domain = &vsi_identity_domain,
> > > +	.domain_alloc_paging = vsi_iommu_domain_alloc_paging,
> > > +	.of_xlate = vsi_iommu_of_xlate,
> > > +	.probe_device = vsi_iommu_probe_device,
> > > +	.release_device = vsi_iommu_release_device,
> > > +	.device_group = generic_single_device_group,
> > > +	.owner = THIS_MODULE,
> > > +	.default_domain_ops = &(const struct iommu_domain_ops) {
> > > +		.attach_dev		= vsi_iommu_attach_device,
> > > +		.map_pages		= vsi_iommu_map,
> > > +		.unmap_pages		= vsi_iommu_unmap,
> > > +		.flush_iotlb_all	= vsi_iommu_flush_tlb_all,
> > This has no callers and so your unmap routine appears to be broken.
> 
> It is a leftover of previous attempt to allow video decoder to clean/flush
> the iommu by using a function from the API.
> Now it is using vsi_iommu_restore_ctx().
> I while remove it in version 12.

Don't you still need some invalidation on the unmap path?

Will