From mboxrd@z Thu Jan 1 00:00:00 1970 From: robin.murphy@arm.com (Robin Murphy) Date: Tue, 27 Jan 2015 17:30:45 +0000 Subject: [RFC PATCH 4/5] arm64: add IOMMU dma_ops In-Reply-To: <54C5B3B9.1040300@nvidia.com> References: <54C5B3B9.1040300@nvidia.com> Message-ID: <54C7CB45.7030907@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Joseph, Thanks for giving it a spin, On 26/01/15 03:25, Joseph Lo wrote: > On 01/13/2015 04:48 AM, Robin Murphy wrote: >> Taking some inspiration from the arch/arm code, implement the >> arch-specific side of the DMA mapping ops using the new IOMMU-DMA layer. >> >> Signed-off-by: Robin Murphy >> --- >> arch/arm64/include/asm/device.h | 3 + >> arch/arm64/include/asm/dma-mapping.h | 12 ++ >> arch/arm64/mm/dma-mapping.c | 297 +++++++++++++++++++++++++++++++++++ >> 3 files changed, 312 insertions(+) >> > [snip] >> +static struct dma_map_ops iommu_dma_ops = { >> + .alloc = __iommu_alloc_attrs, >> + .free = __iommu_free_attrs, >> + .mmap = __swiotlb_mmap, > > Hi Robin, > > Thanks for posting this series. I spent some time to re-write Tegra-smmu > driver to work with the new DT-based iommu initialization procedure and > do some modification in Tegra/DRM driver to make it work with > IOMMU-backed DMA mapping API. > > Found two issues, one is the mmap function here. > The mmap function is not reliable, the userspace application and kernel > easily leads to crash after calling this. Can we mmap the CPU VA to the > IOVA? > This issue was gone after replacing it with the same function > (arm_iommu_mmap_attrs) in the ARM32 kernel. And everything just works fine. > Hmm, it looks like __dma_common_mmap might rely on pages being contiguous, which would be a big problem I hadn't noticed. I'll have a dig into it and see if it's feasible to extend the existing implementation, otherwise we'll have to just pull in the arm_iommu version and have two. >> + .map_page = __iommu_map_page, >> + .unmap_page = __iommu_unmap_page, >> + .map_sg = __iommu_map_sg_attrs, >> + .unmap_sg = __iommu_unmap_sg_attrs, >> + .sync_single_for_cpu = __iommu_sync_single_for_cpu, >> + .sync_single_for_device = __iommu_sync_single_for_device, >> + .sync_sg_for_cpu = __iommu_sync_sg_for_cpu, >> + .sync_sg_for_device = __iommu_sync_sg_for_device, >> + .dma_supported = iommu_dma_supported, >> + .mapping_error = iommu_dma_mapping_error, >> +}; >> + >> +static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, >> + struct iommu_ops *ops) >> +{ >> + struct iommu_dma_mapping *mapping; >> + >> + if (!ops) >> + return; >> + >> + mapping = iommu_dma_create_mapping(ops, dma_base, size); > > The other issue is here. We create a DMA range here, but the > __iova_alloc not really working with that, if the end of the range was > under 4G limitation. The allocation address always starts from > 0xfffxxxxx, because we set the .dma_mask to DMA_BIT_MASK(32) by default. > If we don't expect that, we need one more function call like > dma_coerce_mask_and_coherent(dev, 0x80000000) for the range of 0 to 2G > for example. > > Does this the expectation when we use this framework? > > (Except the issue of __alloc_iova, I am ok with this. We could set up a > default range from 0 to 4G for every device by default, and then isolate > the virtual range by reset the .dma_mask of the device later.) > Yes, I failed to consider dma-ranges in this initial version, so it allocates based on the DMA mask alone. I hacked up an unsatisfactory workaround in preparation for v2, but it turns out the fix as per your suggestion is already in progress elsewhere[1], so I'll rework based on that idea. Robin. [1]:https://lkml.org/lkml/2015/1/23/694 > Thanks, > -Joseph > >> + if (!mapping) { >> + pr_warn("Failed to create %llu-byte IOMMU mapping for device %s\n", >> + size, dev_name(dev)); >> + return; >> + } >> + >> + if (iommu_dma_attach_device(dev, mapping)) >> + pr_warn("Failed to attach device %s to IOMMU mapping\n", >> + dev_name(dev)); >> + else >> + dev->archdata.dma_ops = &iommu_dma_ops; >> + >> + /* drop the initial mapping refcount */ >> + iommu_dma_release_mapping(mapping); >> +} >> + >> +#else >> + >> +static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, >> + struct iommu_ops *iommu) >> +{ } >> + >> +#endif /* CONFIG_IOMMU_DMA */ >> > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >