Re: [PATCH v13 3/6] iommu: Add verisilicon IOMMU driver
From: Benjamin Gaignard
Date: Sun Mar 29 2026 - 13:02:04 EST
Le 25/03/2026 à 17:36, Will Deacon a écrit :
On Tue, Mar 24, 2026 at 05:28:44PM +0100, Benjamin Gaignard wrote:
Le 24/03/2026 à 16:46, Will Deacon a écrit :The hardware appears to have a register to invalidate the entire TLB.
On Mon, Feb 16, 2026 at 10:51:35AM +0100, Benjamin Gaignard wrote:I know you expect the hardware to work like that but that isn't not the
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,I still think you need TLB invalidation here.
+ 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 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.
case.
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 ?
I spend quite long to try to found hidden bit(s) or an other way to do likeThen we can invalidate the entire TLB.
you want but I can't find any solution.
As you mention the downstream code suggest that the iommu can't invalidateThe downstream code is a tangled mess; I don't think it suggests anything
TLB in unmap routine so I don't see how to progress.
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
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