DMA IOMMU static calls

Changelog:
v1:
* Dropped extra layer and called directly to iommu_dma_* functions
* Added unmap_page and unmap_sg to dummy ops
* Converted all dma-mapping calls to use iommu directly
* Updated commit messages
v0: https://lore.kernel.org/all/98d1821780028434ff55b5d2f1feea287409fbc4.1720693745.git.leon@kernel.org/

Leon Romanovsky (2):
dma: call unconditionally to unmap_page and unmap_sg callbacks
dma: Add IOMMU static calls with clear default ops

 MAINTAINERS                 |   1 +
 drivers/iommu/dma-iommu.c   | 108 ++++++++++-------------
 include/linux/dma-map-ops.h |   3 +
 include/linux/iommu-dma.h   | 169 ++++++++++++++++++++++++++++++++++++
 kernel/dma/dummy.c          |  21 +++++
 kernel/dma/mapping.c        |  73 ++++++++++++++--
 6 files changed, 305 insertions(+), 70 deletions(-)
 create mode 100644 include/linux/iommu-dma.h

2.45.2

From: Leon Romanovsky <[email protected]>

Almost all users of ->map_page()/map_sg() callbacks implement ->unmap_page()/unmap_sg() callbacks too. One user which doesn't do it, is dummy DMA ops interface, and the use of this interface is to fail the operation and in such case, there won't be any call to ->unmap_page()/unmap_sg().

This patch removes the existence checks of ->unmap_page()/unmap_sg() and calls to it directly to create symmetrical interface to ->map_page()/map_sg().

---
 kernel/dma/dummy.c   | 21 +++++++++++++++++++++
 kernel/dma/mapping.c |  4 ++--
 2 files changed, 23 insertions(+), 2 deletions(-)

index b492d59ac77e..92de80e5b057 100644
@@ -17,6 +17,15 @@ static dma_addr_t dma_dummy_map_page(struct device *dev, struct page *page,
 {
 	return DMA_MAPPING_ERROR;
 }
+static void dma_dummy_unmap_page(struct device *dev, dma_addr_t dma_handle,
+		size_t size, enum dma_data_direction dir, unsigned long attrs)
+{
+	/*
+	 * Dummy ops doesn't support map_page, so unmap_page should never be
+	 * called.
+	 */
+	WARN_ON_ONCE(true);
+}
 
 static int dma_dummy_map_sg(struct device *dev, struct scatterlist *sgl,
 		int nelems, enum dma_data_direction dir,

@@ -25,6 +34,16 @@ static int dma_dummy_map_sg(struct device *dev, struct scatterlist *sgl,
 	return -EINVAL;
 }
 
+static void dma_dummy_unmap_sg(struct device *dev, struct scatterlist *sgl,
+		int nelems, enum dma_data_direction dir,
+		unsigned long attrs)
+{
+	/*
+	 * Dummy ops doesn't support map_sg, so unmap_sg should never be called.
+	 */
+	WARN_ON_ONCE(true);
+}
+
 static int dma_dummy_supported(struct device *hwdev, u64 mask)
 {
 	return 0;

@@ -33,6 +52,8 @@ static int dma_dummy_supported(struct device *hwdev, u64 mask)
 const struct dma_map_ops dma_dummy_ops = {
 	.mmap                   = dma_dummy_mmap,
 	.map_page               = dma_dummy_map_page,
+	.unmap_page             = dma_dummy_unmap_page,
 	.map_sg                 = dma_dummy_map_sg,
+	.unmap_sg               = dma_dummy_unmap_sg,
 	.dma_supported          = dma_dummy_supported,
 };

index 81de84318ccc..6832fd6f0796 100644
@@ -177,7 +177,7 @@ void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, size_t size,
 	if (dma_map_direct(dev, ops) ||
 	    arch_dma_unmap_page_direct(dev, addr + size))
 		dma_direct_unmap_page(dev, addr, size, dir, attrs);
-	else if (ops->unmap_page)
+	else
 		ops->unmap_page(dev, addr, size, dir, attrs);
 	debug_dma_unmap_page(dev, addr, size, dir);
 }

@@ -291,7 +291,7 @@ void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
 	if (dma_map_direct(dev, ops) ||
 	    arch_dma_unmap_sg_direct(dev, sg, nents))
 		dma_direct_unmap_sg(dev, sg, nents, dir, attrs);
-	else if (ops->unmap_sg)
+	else
 		ops->unmap_sg(dev, sg, nents, dir, attrs);
 }
 EXPORT_SYMBOL(dma_unmap_sg_attrs);

2.45.2

From: Leon Romanovsky <[email protected]>

Most of the arch DMA ops (which often, but not always, involve some sort of IOMMU) are using the same DMA operations. These DMA operations are default ones implemented in drivers/iomem/dma-iommu.c.

So let's make sure to call them directly without need to perform function pointers dereference.

During system initialization, the arch can set its own DMA and in such case, the default DMA operations will be overridden.

---
 MAINTAINERS                 |   1 +
 drivers/iommu/dma-iommu.c   | 108 ++++++++++-------------
 include/linux/dma-map-ops.h |   3 +
 include/linux/iommu-dma.h   | 169 ++++++++++++++++++++++++++++++++++++
 kernel/dma/mapping.c        |  69 +++++++++++++--
 5 files changed, 282 insertions(+), 68 deletions(-)
 create mode 100644 include/linux/iommu-dma.h

index da5352dbd4f3..1e64be463da7 100644
@@ -11544,6 +11544,7 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git
 F:	Documentation/devicetree/bindings/iommu/
 F:	Documentation/userspace-api/iommu.rst
 F:	drivers/iommu/
+F:	include/linux/iommu-dma.h
 F:	include/linux/iommu.h
 F:	include/linux/iova.h
 F:	include/linux/of_iommu.h

index 02a1c825896b..2192c0840a7d 100644
@@ -20,9 +20,12 @@ struct iommu_ops;
  * handle PCI P2PDMA pages in the map_sg/unmap_sg operation.
  * DMA_F_CAN_SKIP_SYNC: DMA sync operations can be skipped if the device is
  * coherent and it's not an SWIOTLB buffer.
+ * DMA_F_USE_DEFAULT_IOMMU: The dma_map_ops implementation can use the default
+ * IOMMU for the device and doesn't need to use function pointers.
  */
 #define DMA_F_PCI_P2PDMA_SUPPORTED     (1 << 0)
 #define DMA_F_CAN_SKIP_SYNC            (1 << 1)
+#define DMA_F_USE_DEFAULT_IOMMU        (1 << 2)
 
 struct dma_map_ops {
 	unsigned int flags;

2.45.2

dma_is_default_iommu still looks a bit odd to me - basically we have the ops structure just to say to bypass it. Why not add a single-bit dma_iommu field to struct device next to dma_ops_bypass to skip it? Then IOMMU_DMA also does not need to select DMA_OPS any more, which would be kinda silly with the direct calls.

It will cause to the situation where "struct device" memory footprint will increase, while we still need to keep dma_ops for archs that don't want to use default IOMMU.

Because dma_ops pointer exists anyway and has already specific flags. I decided to take evolutive approach and add a new flag to it, instead of revolutionary approach and add a new field to struct device.

I didn't know how far to go with that, as default IOMMU .flags are unique and can be removed if dma_iommu bit is set.

Thanks

It won't. Even with the new bit we still have 28 more spare bits next to it :)

The point is that with a little more work we can actually kill that for common configuration. For non-Xen setups that basically just means removing the dummy ops, so it's a pretty big quick win.

No problem, I'll do it.

Thanks