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

Benjamin Gaignard
2026-03-24T16:28:54Z

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