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

[Benjamin Gaignard]

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@xxxxxxxxxxxxx>
> > ---
> >   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.
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.
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.
Maybe we should just admit that is how the hardware work.
This v13 has fixed the documentation so I don't plan to spend more time on this driver.

Benjamin

> Will