Re: [PATCH 02/20] iommu/terga-gart: Replace set_platform_dma_ops() with IOMMU_DOMAIN_PLATFORM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023-05-03 12:01, Jason Gunthorpe wrote:
On Wed, May 03, 2023 at 10:17:29AM +0100, Robin Murphy wrote:
On 2023-05-01 19:02, Jason Gunthorpe wrote:
tegra-gart seems to be kind of wonky since from the start its 'detach_dev'
op doesn't actually touch hardware. It is supposed to empty the GART of
all translations loaded into it.

No, detach should never tear down translations - what if other devices are
still using the domain?

?? All other drivers do this.

The only driver I'm aware of which effectively tore down mappings by freeing its pagetable on detach was sprd-iommu, and that was recently fixed on account of it being clearly wrong.

Remember that the GART registers here are literally just its pagetable, nothing more.

The core contract is that this sequence:

    dom = iommu_domain_alloc()
    iommu_attach_device(dom, dev)
    iommu_map(dom,...)
    iommu_detach_device(dom, dev)

Will not continue to have the IOVA mapped to the device. We rely on
this for various error paths.

Yes, I'm not disputing that we expect detach to remove that device's *access* to the IOVA (which is what GART can't do...), but it should absolutely not destroy the IOVA mapping itself. Follow that sequence with iommu_attach_device(dom, dev) again and the caller can expect to be able to continue using the same translation.

If the HW is multi-device then it is supposed to have groups.

Groups are in fact the most practical example: set up a VFIO domain, attach two groups to it, map some IOVAs, detach one of the groups, keep using the other. If the detach carried an implicit iommu_unmap() there would be fireworks.

Call this weirdness PLATFORM which keeps the basic original
ops->detach_dev() semantic alive without needing much special core code
support. I'm guessing it really ends up in a BLOCKING configuration, but
without any forced cleanup it is unsafe.

The GART translation aperture is in physical address space, so the truth is
that all devices have access to it at the same time as having access to the
rest of physical address space. Attach/detach here really are only
bookkeeping for which domain currently owns the aperture.

Oh yuk, that is not an UNMANAGED domain either as we now assume empty
UNMANAGED domains are blocking in the core...

They are, in the sense that accesses within the aperture won't go anywhere. It might help if domain->geometry.force_aperture was meaningful, because it's never been clear whether it was supposed to reflect a hardware capability (in which case it should be false for GART) or be an instruction to the user of the domain (wherein it's a bit pointless that everyone always sets it).

Thanks,
Robin.



[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux for Synopsys ARC Processors]    
  • [Linux on Unisoc (RDA Micro) SoCs]     [Linux Actions SoC]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  •   Powered by Linux