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
