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 <robin.mur...@arm.com>
---
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-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
