Le 25/03/2026 à 17:36, Will Deacon a écrit :

On Tue, Mar 24, 2026 at 05:28:44PM +0100, Benjamin Gaignard wrote:

quoted

Le 24/03/2026 à 16:46, Will Deacon a écrit :

quoted

On Mon, Feb 16, 2026 at 10:51:35AM +0100, Benjamin Gaignard wrote:

quoted

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@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

[...]

quoted

+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.

VSI_MMU_BIT_FLUSH ? it discards everything.
Is there an api to call it when all buffers have been unmapped ?

quoted

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.

quoted

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.

If you have an other source to tell the hardware capabilities, I will be
more than happy to read it and fix the driver.

Benjamin

quoted

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.

quoted

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