| Message ID | 1478523973-8828-1-git-send-email-m.szyprowski@samsung.com |
|---|---|
| State | New |
| Headers | show |
Hi Marek, 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? > 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. 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. Robin. > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/iommu/dma-iommu.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index c5ab8667e6f2..8b4b72654359 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -212,11 +212,13 @@ static struct iova *__alloc_iova(struct iommu_domain *domain, size_t size, > > if (domain->geometry.force_aperture) > dma_limit = min(dma_limit, domain->geometry.aperture_end); > + > + dma_limit = min(dma_limit >> shift, (dma_addr_t)iovad->dma_32bit_pfn); > /* > * Enforce size-alignment to be safe - there could perhaps be an > * attribute to control this per-device, or at least per-domain... > */ > - return alloc_iova(iovad, length, dma_limit >> shift, true); > + return alloc_iova(iovad, length, dma_limit, true); > } > > /* The IOVA allocator knows what we mapped, so just unmap whatever that was */ > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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. > >> 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. > 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 -- Marek Szyprowski, PhD Samsung R&D Institute Poland _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index c5ab8667e6f2..8b4b72654359 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -212,11 +212,13 @@ static struct iova *__alloc_iova(struct iommu_domain *domain, size_t size, if (domain->geometry.force_aperture) dma_limit = min(dma_limit, domain->geometry.aperture_end); + + dma_limit = min(dma_limit >> shift, (dma_addr_t)iovad->dma_32bit_pfn); /* * Enforce size-alignment to be safe - there could perhaps be an * attribute to control this per-device, or at least per-domain... */ - return alloc_iova(iovad, length, dma_limit >> shift, true); + return alloc_iova(iovad, length, dma_limit, true); } /* The IOVA allocator knows what we mapped, so just unmap whatever that was */
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. 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). Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/iommu/dma-iommu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- 1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel