On Wed, May 03, 2023 at 04:43:53PM +0200, Thierry Reding wrote: > > The only thing it does is cause dma-iommu.c in ARM64 to use the > > dma-ranges from OF instead of the domain aperture. sprd has no > > dma-ranges in arch/arm64/boot/dts/sprd. > > > > Further, sprd hard fails any map attempt outside the aperture, so it > > looks like a bug if the OF somehow chooses a wider aperture as > > dma-iommu.c will start failing maps. > > That all sounds odd. of_dma_configure_id() already sets up the DMA mask > based on dma-ranges and the DMA API uses that to restrict what IOVA any > buffers can get mapped to for a given device. Yes, and after it sets up the mask it also passes that range down like this: of_dma_configure_id(): end = dma_start + size - 1; mask = DMA_BIT_MASK(ilog2(end) + 1); dev->coherent_dma_mask &= mask; arch_setup_dma_ops(dev, dma_start, size, iommu, coherent); Which eventually goes to: iommu_setup_dma_ops() iommu_dma_init_domain() Which then does: if (domain->geometry.force_aperture) { And if not set uses the dma_start/size parameter as the actual aperture. (!?) Since sprd does this in the iommu driver: dom->domain.geometry.aperture_start = 0; dom->domain.geometry.aperture_end = SZ_256M - 1; And it is serious about enforcing it during map: unsigned long start = domain->geometry.aperture_start; unsigned long end = domain->geometry.aperture_end; if (iova < start || (iova + size) > (end + 1)) { return -EINVAL; We must see the dma_start/size parameter be a subset of the aperture or eventually dma-iommu.c will see map failures. I can't see how this is can happen, so it looks like omitting force_aperture is a mistake not a deliberate choice. I'll make a patch and see if the SPRD folks have any comment. If there is no reason I can go ahead and purge force_aperture and all the dma_start/size passing through arch_setup_dma_ops(). > > Thus, I propose we just remove the whole thing. All drivers must set > > an aperture and the aperture is the pure HW capability to map an > > IOPTE at that address. ie it reflects the design of the page table > > itself and nothing else. > > Yeah, that sounds reasonable. If the aperture represents what the IOMMU > supports. Together with each device's DMA mask we should have everything > we need. Arguably we should respect the dma-ranges as well, but I think that should come up from the iommu driver via the get_resv_regions() which is the usual place we return FW originated information. > For Tegra GART I think there's indeed no use-cases at the moment. Dmitry > had at one point tried to make use of it because it can be helpful on > some of the older devices that were very memory-constrained. That > support never made it upstream because it required significant changes > in various places, if I recall correctly. For anything with a decent > enough amount of RAM, CMA is usually a better option. So the actual use case of this HW is to linearize buffers? ie it is a general scatter/gather engine? > This has occasionally come up in the past and I seem to remember that it > had once been proposed to simply remove tegra-gart and there had been no > objections. Adding Dmitry, if he doesn't have objections to remaving it, > neither do I. Dmitry, please say yes and I will remove it instead of trying to carry it. The driver is almost 10 years old at this point, I'm skeptical anyone will need it on a 6.2 era kernel.. Thanks, Jason