On 08/11/16 13:41, Marek Szyprowski wrote: > Hi Robin, > > > On 2016-11-08 12:37, Robin Murphy wrote: >> On 07/11/16 13:06, Marek Szyprowski wrote: >>> When one called iommu_dma_init_domain() with size smaller than device's >>> DMA mask, the alloc_iova() will not respect it and always assume that >>> all >>> IOVA addresses will be allocated from the the (base ... >>> dev->dma_mask) range. >> Is that actually a problem for anything? > > Yes, I found this issue while working on next version of ARM & ARM64 > DMA-mapping/IOMMU integration patchset and adapting Exynos drivers for the > new IOMMU/DMA-mapping glue. > > Some Exynos devices (codec and camera isp) operate only on the limited (and > really small: 256M for example) DMA window. They use non-standard way of > addressing memory: an offset from the firmware base. However they still > have > 32bit DMA mask, as the firmware can be located basically everywhere in the > real DMA address space, but then they can access only next 256M from that > firmware base.
OK, that's good to know, thanks. However, I think in this case it sounds like it's really the DMA mask that's the underlying problem - if those blocks themselves can only drive 28 address bits, then the struct devices representing them should have 28-bit DMA masks, and the "firmware base" of whoever's driving the upper bits modelled with a dma_pfn_offset. Even if they do have full 32-bit interfaces themselves, but are constrained to segment-offset addressing internally, I still think it would be tidier to represent things that way. At some point in dma-iommu development I did have support for DMA offsets upstream of the IOMMU, and am happy to reinstate it if there's a real use case (assuming you can't simply always set the firmware base to 0 when using the IOMMU). >>> This patch fixes this issue by taking the configured address space size >>> parameter into account (if it is smaller than the device's dma_mask). >> TBH I've been pondering ripping the size stuff out of dma-iommu, as it >> all stems from me originally failing to understand what dma_32bit_pfn is >> actually for. The truth is that iova_domains just don't have a size or >> upper limit; however if devices with both large and small DMA masks >> share a domain, then the top-down nature of the allocator means that >> allocating for the less-capable devices would involve walking through >> every out-of-range entry in the tree every time. Having cached32_node >> based on dma_32bit_pfn just provides an optimised starting point for >> searching within the smaller mask. > > Right, this dma_32bit_pfn was really misleading at the first glance, > but then I found that this was something like end_pfn in case of dma-iommu > code. Yes, that was my incorrect assumption - at the time I interpreted it as a de-facto upper limit which was still possible to allocate above in special circumstances, which turns out to be almost entirely backwards. I'd rather not bake that into the dma-iommu code any further if we can avoid it (I'll try throwing an RFC together to clear up what's there already). Robin. >> Would it hurt any of your use-cases to relax/rework the reinitialisation >> checks in iommu_dma_init_domain()? Alternatively if we really do have a >> case for wanting a hard upper limit, it might make more sense to add an >> end_pfn to the iova_domain and handle it in the allocator itself. > > The proper support for end_pfn would be a better approach probably, > especially if we consider readability of the code. > > Best regards _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu