[PATCH v13 3/6] iommu: Add verisilicon IOMMU driver

[Will Deacon]

On Tue, Mar 24, 2026 at 05:28:44PM +0100, Benjamin Gaignard wrote:
> 
> Le 24/03/2026 à 16:46, Will Deacon a écrit :
> > On Mon, Feb 16, 2026 at 10:51:35AM +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 at collabora.com>
> > > ---
> > >   MAINTAINERS               |   8 +
> > >   drivers/iommu/Kconfig     |  11 +
> > >   drivers/iommu/Makefile    |   1 +
> > >   drivers/iommu/vsi-iommu.c | 794 ++++++++++++++++++++++++++++++++++++++
> > >   include/linux/vsi-iommu.h |  21 +
> > >   5 files changed, 835 insertions(+)
> > >   create mode 100644 drivers/iommu/vsi-iommu.c
> > >   create mode 100644 include/linux/vsi-iommu.h
> > [...]
> > 
> > > +static size_t vsi_iommu_unmap(struct iommu_domain *domain, unsigned long _iova,
> > > +			      size_t size, size_t count, struct iommu_iotlb_gather *gather)
> > > +{
> > > +	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
> > > +	dma_addr_t pte_dma, iova = (dma_addr_t)_iova;
> > > +	unsigned long flags;
> > > +	phys_addr_t pt_phys;
> > > +	u32 dte;
> > > +	u32 *pte_addr;
> > > +	size_t unmap_size = 0;
> > > +
> > > +	spin_lock_irqsave(&vsi_domain->lock, flags);
> > > +
> > > +	dte = vsi_domain->dt[vsi_iova_dte_index(iova)];
> > > +	/* Just return 0 if iova is unmapped */
> > > +	if (!vsi_dte_is_pt_valid(dte))
> > > +		goto unlock;
> > > +
> > > +	pt_phys = vsi_dte_pt_address(dte);
> > > +	pte_addr = (u32 *)phys_to_virt(pt_phys) + vsi_iova_pte_index(iova);
> > > +	pte_dma = pt_phys + vsi_iova_pte_index(iova) * sizeof(u32);
> > > +	unmap_size = vsi_iommu_unmap_iova(vsi_domain, pte_addr, pte_dma, size);
> > > +
> > > +unlock:
> > > +	spin_unlock_irqrestore(&vsi_domain->lock, flags);
> > > +
> > > +	return unmap_size;
> > > +}
> > I still think you need TLB invalidation here.
> > 
> > I looked at the downstream code that you linked to and it litters the
> > invalidation in the callers via mpp_iommu_flush_tlb(), which tend to
> > invalidate _before_ starting an operation. That's very likely buggy and
> > certainly not something we want upstream.
> > 
> > The unmap routine should do the invalidation so that, when it returns,
> > the pages really are unmapped from the device (assuming strict mode).
> > 
> > I know you said that you tried to add invalidation here and it "didn't
> > work", but that's not something I can really help you with.
> 
> I know you expect the hardware to work like that but that isn't not the
> case.

The hardware appears to have a register to invalidate the entire TLB.
We can use that if there's nothing else.

> I spend quite long to try to found hidden bit(s) or an other way to do like
> you want but I can't find any solution.

Then we can invalidate the entire TLB.

> As you mention the downstream code suggest that the iommu can't invalidate
> TLB in unmap routine so I don't see how to progress.

The downstream code is a tangled mess; I don't think it suggests anything
about what the IOMMU hardware is capable of.

> Maybe we should just admit that is how the hardware work.

No.

The upstream kernel isn't a dumping ground for vendor crap. The hardware
has TLB invalidation functionality and so we should use it. If we don't,
then we're not giving the IOMMU API what it expects and any callers
outside of the video codecs will be landed with problems when unmap
doesn't work as expected.

> This v13 has fixed the documentation so I don't plan to spend more time on this driver.

That's a shame, I'm really not asking for much.

Will