DMA IOMMU static calls

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().

Signed-off-by Leon Romanovsky <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

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

kernel/dma/dummy.c +21 -0

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,
  };

kernel/dma/mapping.c +2 -2

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);

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, but for all modern platforms dma-iommu implementation is really matters.

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.

Signed-off-by Leon Romanovsky <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
  MAINTAINERS                 |   1 +
  drivers/iommu/Kconfig       |   2 +-
  drivers/iommu/dma-iommu.c   | 121 ++++++++++----------------
  drivers/iommu/intel/Kconfig |   1 -
  include/linux/device.h      |   5 ++
  include/linux/dma-map-ops.h |  39 +++++----
  include/linux/iommu-dma.h   | 169 ++++++++++++++++++++++++++++++++++++
  kernel/dma/Kconfig          |   6 ++
  kernel/dma/Makefile         |   2 +-
  kernel/dma/mapping.c        |  90 ++++++++++++++++---
  10 files changed, 327 insertions(+), 109 deletions(-)

create mode 100644 include/linux/iommu-dma.h

MAINTAINERS +0 -0

index da5352dbd4f3..1e64be463da7 100644

MAINTAINERS +0 -0

  F:	include/linux/iommu.h
  F:	include/linux/iova.h
  F:	include/linux/of_iommu.h

drivers/iommu/Kconfig +1 -1

index c04584be3089..e24cb857b66c 100644
@@ -151,7 +151,7 @@ config OF_IOMMU
  # IOMMU-agnostic DMA-mapping layer
  config IOMMU_DMA
  	def_bool ARM64 || X86 || S390
-	select DMA_OPS
+	select DMA_OPS_HELPERS
  	select IOMMU_API
  	select IOMMU_IOVA
  	select IRQ_MSI_IOMMU

drivers/iommu/dma-iommu.c +3 -3

index 43520e7275cc..ab2d3092ac23 100644
@@ -17,6 +17,7 @@
  #include <linux/gfp.h>
  #include <linux/huge_mm.h>
  #include <linux/iommu.h>
+#include <linux/iommu-dma.h>
  #include <linux/iova.h>
  #include <linux/irq.h>
  #include <linux/list_sort.h>

@@ -1039,9 +1040,8 @@ static void *iommu_dma_alloc_remap(struct device *dev, size_t size,
  	return NULL;
  }
 
-static struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev,
-		size_t size, enum dma_data_direction dir, gfp_t gfp,
-		unsigned long attrs)
+struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev, size_t size,
+	       enum dma_data_direction dir, gfp_t gfp, unsigned long attrs)
  {
  	struct dma_sgt_handle *sh;
 

drivers/iommu/dma-iommu.c +33 -63

  {
  	struct dma_sgt_handle *sh = sgt_handle(sgt);
 

@@ -1069,8 +1069,8 @@ static void iommu_dma_free_noncontiguous(struct device *dev, size_t size,
  	kfree(sh);
  }
 
-static void iommu_dma_sync_single_for_cpu(struct device *dev,
-		dma_addr_t dma_handle, size_t size, enum dma_data_direction dir)
+void iommu_dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle,
+				   size_t size, enum dma_data_direction dir)
  {
  	phys_addr_t phys;
 

@@ -1085,8 +1085,8 @@ static void iommu_dma_sync_single_for_cpu(struct device *dev,
  		swiotlb_sync_single_for_cpu(dev, phys, size, dir);
  }
 
-static void iommu_dma_sync_single_for_device(struct device *dev,
-		dma_addr_t dma_handle, size_t size, enum dma_data_direction dir)
+void iommu_dma_sync_single_for_device(struct device *dev, dma_addr_t dma_handle,
+				      size_t size, enum dma_data_direction dir)
  {
  	phys_addr_t phys;
 

@@ -1101,9 +1101,8 @@ static void iommu_dma_sync_single_for_device(struct device *dev,
  		arch_sync_dma_for_device(phys, size, dir);
  }
 
-static void iommu_dma_sync_sg_for_cpu(struct device *dev,
-		struct scatterlist *sgl, int nelems,
-		enum dma_data_direction dir)
+void iommu_dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sgl,
+			       int nelems, enum dma_data_direction dir)
  {
  	struct scatterlist *sg;
  	int i;

@@ -1117,9 +1116,8 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
  			arch_sync_dma_for_cpu(sg_phys(sg), sg->length, dir);
  }
 
-static void iommu_dma_sync_sg_for_device(struct device *dev,
-		struct scatterlist *sgl, int nelems,
-		enum dma_data_direction dir)
+void iommu_dma_sync_sg_for_device(struct device *dev, struct scatterlist *sgl,
+				  int nelems, enum dma_data_direction dir)
  {
  	struct scatterlist *sg;
  	int i;

@@ -1134,9 +1132,9 @@ static void iommu_dma_sync_sg_for_device(struct device *dev,
  			arch_sync_dma_for_device(sg_phys(sg), sg->length, dir);
  }
 
-static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
-		unsigned long offset, size_t size, enum dma_data_direction dir,
-		unsigned long attrs)
+dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
+	      unsigned long offset, size_t size, enum dma_data_direction dir,
+	      unsigned long attrs)
  {
  	phys_addr_t phys = page_to_phys(page) + offset;
  	bool coherent = dev_is_dma_coherent(dev);

@@ -1194,8 +1192,8 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
  	return iova;
  }
 
-static void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
-		size_t size, enum dma_data_direction dir, unsigned long attrs)
+void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
+	  size_t size, enum dma_data_direction dir, unsigned long attrs)
  {
  	struct iommu_domain *domain = iommu_get_dma_domain(dev);
  	phys_addr_t phys;

@@ -1348,8 +1346,8 @@ static int iommu_dma_map_sg_swiotlb(struct device *dev, struct scatterlist *sg,
   * impedance-matching, to be able to hand off a suitably-aligned list,
   * but still preserve the original offsets and sizes for the caller.
   */
-static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
-		int nents, enum dma_data_direction dir, unsigned long attrs)
+int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+		     enum dma_data_direction dir, unsigned long attrs)
  {
  	struct iommu_domain *domain = iommu_get_dma_domain(dev);
  	struct iommu_dma_cookie *cookie = domain->iova_cookie;

@@ -1468,8 +1466,8 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
  	return ret;
  }
 
-static void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
-		int nents, enum dma_data_direction dir, unsigned long attrs)
+void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
+			enum dma_data_direction dir, unsigned long attrs)
  {
  	dma_addr_t end = 0, start;
  	struct scatterlist *tmp;

@@ -1518,16 +1516,17 @@ static void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
  		__iommu_dma_unmap(dev, start, end - start);
  }
 
-static dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys,
-		size_t size, enum dma_data_direction dir, unsigned long attrs)
+dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys,
+				  size_t size, enum dma_data_direction dir,
+				  unsigned long attrs)
  {
  	return __iommu_dma_map(dev, phys, size,
  			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO,
  			dma_get_mask(dev));
  }
 
-static void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
-		size_t size, enum dma_data_direction dir, unsigned long attrs)
+void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
+	      size_t size, enum dma_data_direction dir, unsigned long attrs)
  {
  	__iommu_dma_unmap(dev, handle, size);
  }

@@ -1563,8 +1562,8 @@ static void __iommu_dma_free(struct device *dev, size_t size, void *cpu_addr)
  		dma_free_contiguous(dev, page, alloc_size);
  }
 
-static void iommu_dma_free(struct device *dev, size_t size, void *cpu_addr,
-		dma_addr_t handle, unsigned long attrs)
+void iommu_dma_free(struct device *dev, size_t size, void *cpu_addr,
+		    dma_addr_t handle, unsigned long attrs)
  {
  	__iommu_dma_unmap(dev, handle, size);
  	__iommu_dma_free(dev, size, cpu_addr);

@@ -1607,8 +1606,8 @@ static void *iommu_dma_alloc_pages(struct device *dev, size_t size,
  	return NULL;
  }
 
-static void *iommu_dma_alloc(struct device *dev, size_t size,
-		dma_addr_t *handle, gfp_t gfp, unsigned long attrs)
+void *iommu_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
+		      gfp_t gfp, unsigned long attrs)
  {
  	bool coherent = dev_is_dma_coherent(dev);
  	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);

@@ -1642,9 +1641,8 @@ static void *iommu_dma_alloc(struct device *dev, size_t size,
  	return cpu_addr;
  }
 
-static int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
-		void *cpu_addr, dma_addr_t dma_addr, size_t size,
-		unsigned long attrs)
+int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+	void *cpu_addr, dma_addr_t dma_addr, size_t size, unsigned long attrs)
  {
  	unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
  	unsigned long pfn, off = vma->vm_pgoff;

@@ -1673,9 +1671,8 @@ static int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
  			       vma->vm_page_prot);
  }
 
-static int iommu_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
-		void *cpu_addr, dma_addr_t dma_addr, size_t size,
-		unsigned long attrs)
+int iommu_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
+	void *cpu_addr, dma_addr_t dma_addr, size_t size, unsigned long attrs)
  {
  	struct page *page;
  	int ret;

@@ -1700,19 +1697,19 @@ static int iommu_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
  	return ret;
  }
 
-static unsigned long iommu_dma_get_merge_boundary(struct device *dev)
+unsigned long iommu_dma_get_merge_boundary(struct device *dev)
  {
  	struct iommu_domain *domain = iommu_get_dma_domain(dev);
 
  	return (1UL << __ffs(domain->pgsize_bitmap)) - 1;
  }
 
-static size_t iommu_dma_opt_mapping_size(void)
+size_t iommu_dma_opt_mapping_size(void)
  {
  	return iova_rcache_range();
  }
 
-static size_t iommu_dma_max_mapping_size(struct device *dev)
+size_t iommu_dma_max_mapping_size(struct device *dev)
  {
  	if (dev_is_untrusted(dev))
  		return swiotlb_max_mapping_size(dev);

@@ -1720,32 +1717,6 @@ static size_t iommu_dma_max_mapping_size(struct device *dev)
  	return SIZE_MAX;
  }
 
-static const struct dma_map_ops iommu_dma_ops = {
-	.flags			= DMA_F_PCI_P2PDMA_SUPPORTED |
-				  DMA_F_CAN_SKIP_SYNC,
-	.alloc			= iommu_dma_alloc,
-	.free			= iommu_dma_free,
-	.alloc_pages_op		= dma_common_alloc_pages,
-	.free_pages		= dma_common_free_pages,
-	.alloc_noncontiguous	= iommu_dma_alloc_noncontiguous,
-	.free_noncontiguous	= iommu_dma_free_noncontiguous,
-	.mmap			= iommu_dma_mmap,
-	.get_sgtable		= iommu_dma_get_sgtable,
-	.map_page		= iommu_dma_map_page,
-	.unmap_page		= iommu_dma_unmap_page,
-	.map_sg			= iommu_dma_map_sg,
-	.unmap_sg		= iommu_dma_unmap_sg,
-	.sync_single_for_cpu	= iommu_dma_sync_single_for_cpu,
-	.sync_single_for_device	= iommu_dma_sync_single_for_device,
-	.sync_sg_for_cpu	= iommu_dma_sync_sg_for_cpu,
-	.sync_sg_for_device	= iommu_dma_sync_sg_for_device,
-	.map_resource		= iommu_dma_map_resource,
-	.unmap_resource		= iommu_dma_unmap_resource,
-	.get_merge_boundary	= iommu_dma_get_merge_boundary,
-	.opt_mapping_size	= iommu_dma_opt_mapping_size,
-	.max_mapping_size       = iommu_dma_max_mapping_size,
-};
-
  void iommu_setup_dma_ops(struct device *dev)
  {
  	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);

@@ -1753,19 +1724,15 @@ void iommu_setup_dma_ops(struct device *dev)
  	if (dev_is_pci(dev))
  		dev->iommu->pci_32bit_workaround = !iommu_dma_forcedac;
 
-	if (iommu_is_dma_domain(domain)) {

drivers/iommu/dma-iommu.c +3 -2

  	return;
  out_err:
-	 pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
-		 dev_name(dev));
+	pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
+		dev_name(dev));
+	dev->dma_iommu = false;
  }
 
  static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,

drivers/iommu/intel/Kconfig +0 -1

index f52fb39c968e..88fd32a9323c 100644
@@ -12,7 +12,6 @@ config DMAR_DEBUG
  config INTEL_IOMMU
  	bool "Support for Intel IOMMU using DMA Remapping Devices"
  	depends on PCI_MSI && ACPI && X86
-	select DMA_OPS
  	select IOMMU_API
  	select IOMMU_IOVA
  	select IOMMUFD_DRIVER if IOMMUFD

include/linux/device.h +2 -0

index ace039151cb8..e66ec47ceb09 100644
@@ -707,6 +707,8 @@ struct device_physical_location {
   *		for dma allocations.  This flag is managed by the dma ops
   *		instance from ->dma_supported.
   * @dma_skip_sync: DMA sync operations can be skipped for coherent buffers.
+ * @dma_iommu: Device is using default IOMMU implementation for DMA and
+ *		doesn't rely on dma_ops structure.
   *
   * At the lowest level, every device in a Linux system is represented by an
   * instance of struct device. The device structure contains the information

include/linux/device.h +1 -0

+#endif
  };
 
  /**

include/linux/dma-map-ops.h +0 -13

index 02a1c825896b..103d9c66c445 100644
@@ -13,20 +13,7 @@
  struct cma;
  struct iommu_ops;
 
-/*
- * Values for struct dma_map_ops.flags:
- *
- * DMA_F_PCI_P2PDMA_SUPPORTED: Indicates the dma_map_ops implementation can
- * 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.
- */
-#define DMA_F_PCI_P2PDMA_SUPPORTED     (1 << 0)
-#define DMA_F_CAN_SKIP_SYNC            (1 << 1)
-
  struct dma_map_ops {
-	unsigned int flags;
-
  	void *(*alloc)(struct device *dev, size_t size,
  			dma_addr_t *dma_handle, gfp_t gfp,
  			unsigned long attrs);

include/linux/dma-map-ops.h +15 -4

+#else /* CONFIG_DMA_OPS_HELPERS */
+static inline struct page *
+dma_common_alloc_pages(struct device *dev, size_t size, dma_addr_t *dma_handle,
+		       enum dma_data_direction dir, gfp_t gfp)
+{
+	return NULL;
+}
+static inline void dma_common_free_pages(struct device *dev, size_t size,
+					 struct page *vaddr,
+					 dma_addr_t dma_handle,
+					 enum dma_data_direction dir)
+{
+}
+#endif /* CONFIG_DMA_OPS_HELPERS */
+
  #ifdef CONFIG_DMA_CMA
  extern struct cma *dma_contiguous_default_area;
 

@@ -239,10 +248,6 @@ int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
  int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
  		void *cpu_addr, dma_addr_t dma_addr, size_t size,
  		unsigned long attrs);
-struct page *dma_common_alloc_pages(struct device *dev, size_t size,
-		dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp);
-void dma_common_free_pages(struct device *dev, size_t size, struct page *vaddr,
-		dma_addr_t dma_handle, enum dma_data_direction dir);
 
  struct page **dma_common_find_pages(void *cpu_addr);
  void *dma_common_contiguous_remap(struct page *page, size_t size, pgprot_t prot,

include/linux/iommu-dma.h new file +169 -0

new file mode 100644
index 000000000000..622232fc9510
@@ -0,0 +1,169 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved
+ *
+ * DMA operations that map physical memory through IOMMU.
+ */
+#ifndef _LINUX_IOMMU_DMA_H
+#define _LINUX_IOMMU_DMA_H
+
+#include <linux/dma-direction.h>
+
+#ifdef CONFIG_IOMMU_DMA
+dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
+			      unsigned long offset, size_t size,
+			      enum dma_data_direction dir, unsigned long attrs);
+void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
+			  size_t size, enum dma_data_direction dir,
+			  unsigned long attrs);
+int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+		     enum dma_data_direction dir, unsigned long attrs);
+void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
+			enum dma_data_direction dir, unsigned long attrs);
+void *iommu_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
+		      gfp_t gfp, unsigned long attrs);
+int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+		   void *cpu_addr, dma_addr_t dma_addr,
+		   size_t size, unsigned long attrs);
+int iommu_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
+			  void *cpu_addr, dma_addr_t dma_addr, size_t size,
+			  unsigned long attrs);
+unsigned long iommu_dma_get_merge_boundary(struct device *dev);
+size_t iommu_dma_opt_mapping_size(void);
+size_t iommu_dma_max_mapping_size(struct device *dev);
+void iommu_dma_free(struct device *dev, size_t size, void *cpu_addr,
+		    dma_addr_t handle, unsigned long attrs);
+dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys,
+				  size_t size, enum dma_data_direction dir,
+				  unsigned long attrs);
+void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
+			      size_t size, enum dma_data_direction dir,
+			      unsigned long attrs);
+struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev, size_t size,
+					       enum dma_data_direction dir,
+					       gfp_t gfp, unsigned long attrs);
+void iommu_dma_free_noncontiguous(struct device *dev, size_t size,
+				  struct sg_table *sgt,
+				  enum dma_data_direction dir);
+void iommu_dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle,
+				   size_t size, enum dma_data_direction dir);
+void iommu_dma_sync_single_for_device(struct device *dev, dma_addr_t dma_handle,
+				      size_t size, enum dma_data_direction dir);
+void iommu_dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sgl,
+			       int nelems, enum dma_data_direction dir);
+void iommu_dma_sync_sg_for_device(struct device *dev, struct scatterlist *sgl,
+				  int nelems, enum dma_data_direction dir);
+#else
+static inline dma_addr_t iommu_dma_map_page(struct device *dev,
+					    struct page *page,
+					    unsigned long offset, size_t size,
+					    enum dma_data_direction dir,
+					    unsigned long attrs)
+{
+	return DMA_MAPPING_ERROR;
+}
+static inline void iommu_dma_unmap_page(struct device *dev,
+					dma_addr_t dma_handle, size_t size,
+					enum dma_data_direction dir,
+					unsigned long attrs)
+{
+}
+static inline int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
+				   int nents, enum dma_data_direction dir,
+				   unsigned long attrs)
+{
+	return -EINVAL;
+}
+static inline void iommu_dma_unmap_sg(struct device *dev,
+				      struct scatterlist *sg, int nents,
+				      enum dma_data_direction dir,
+				      unsigned long attrs)
+{
+}
+static inline void *iommu_dma_alloc(struct device *dev, size_t size,
+				    dma_addr_t *handle, gfp_t gfp,
+				    unsigned long attrs)
+{
+	return NULL;
+}
+static inline int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+				 void *cpu_addr, dma_addr_t dma_addr,
+				 size_t size, unsigned long attrs)
+{
+	return -EINVAL;
+}
+static inline int iommu_dma_get_sgtable(struct device *dev,
+					struct sg_table *sgt, void *cpu_addr,
+					dma_addr_t dma_addr, size_t size,
+					unsigned long attrs)
+{
+	return -EINVAL;
+}
+static inline unsigned long iommu_dma_get_merge_boundary(struct device *dev)
+{
+	return 0;
+}
+static inline size_t iommu_dma_opt_mapping_size(void)
+{
+	return 0;
+}
+static inline size_t iommu_dma_max_mapping_size(struct device *dev)
+{
+	return 0;
+}
+static inline void iommu_dma_free(struct device *dev, size_t size,
+				  void *cpu_addr, dma_addr_t handle,
+				  unsigned long attrs)
+{
+}
+static inline dma_addr_t iommu_dma_map_resource(struct device *dev,
+						phys_addr_t phys, size_t size,
+						enum dma_data_direction dir,
+						unsigned long attrs)
+{
+	return DMA_MAPPING_ERROR;
+}
+static inline void iommu_dma_unmap_resource(struct device *dev,
+					    dma_addr_t handle, size_t size,
+					    enum dma_data_direction dir,
+					    unsigned long attrs)
+{
+}
+static inline struct sg_table *
+iommu_dma_alloc_noncontiguous(struct device *dev, size_t size,
+			      enum dma_data_direction dir, gfp_t gfp,
+			      unsigned long attrs)
+{
+	return NULL;
+}
+static inline void iommu_dma_free_noncontiguous(struct device *dev, size_t size,
+						struct sg_table *sgt,
+						enum dma_data_direction dir)
+{
+}
+static inline void iommu_dma_sync_single_for_cpu(struct device *dev,
+						 dma_addr_t dma_handle,
+						 size_t size,
+						 enum dma_data_direction dir)
+{
+}
+static inline void iommu_dma_sync_single_for_device(struct device *dev,
+						    dma_addr_t dma_handle,
+						    size_t size,
+						    enum dma_data_direction dir)
+{
+}
+static inline void iommu_dma_sync_sg_for_cpu(struct device *dev,
+					     struct scatterlist *sgl,
+					     int nelems,
+					     enum dma_data_direction dir)
+{
+}
+static inline void iommu_dma_sync_sg_for_device(struct device *dev,
+						struct scatterlist *sgl,
+						int nelems,
+						enum dma_data_direction dir)
+{
+}
+#endif /* CONFIG_IOMMU_DMA */
+#endif /* _LINUX_IOMMU_DMA_H */

kernel/dma/Kconfig +0 -0

index c06e56be0ca1..03bb925014a7 100644

kernel/dma/Kconfig +4 -0

+config DMA_OPS_HELPERS
+	bool
+
  config DMA_OPS
  	depends on HAS_DMA
+	select DMA_OPS_HELPERS
  	bool
 
  #

kernel/dma/Makefile +1 -1

index 21926e46ef4f..2e6e933cf7f3 100644
@@ -1,7 +1,7 @@
  # SPDX-License-Identifier: GPL-2.0
 
  obj-$(CONFIG_HAS_DMA)			+= mapping.o direct.o
-obj-$(CONFIG_DMA_OPS)			+= ops_helpers.o
+obj-$(CONFIG_DMA_OPS_HELPERS)		+= ops_helpers.o
  obj-$(CONFIG_DMA_OPS)			+= dummy.o
  obj-$(CONFIG_DMA_CMA)			+= contiguous.o
  obj-$(CONFIG_DMA_DECLARE_COHERENT)	+= coherent.o

kernel/dma/mapping.c +0 -0

index 6832fd6f0796..02451e27e0b1 100644

kernel/dma/mapping.c +27 -1

  #include <linux/export.h>
  #include <linux/gfp.h>
  #include <linux/kmsan.h>

@@ -113,11 +114,27 @@ void *dmam_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
  }
  EXPORT_SYMBOL(dmam_alloc_attrs);
 
+#ifdef CONFIG_IOMMU_DMA
+static bool use_dma_iommu(struct device *dev)
+{
+	return dev->dma_iommu;
+}
+#else
+static bool use_dma_iommu(struct device *dev)
+{
+	return false;
+}
+#endif
+
  static bool dma_go_direct(struct device *dev, dma_addr_t mask,
  		const struct dma_map_ops *ops)
  {
+	if (use_dma_iommu(dev))
+		return false;
+
  	if (likely(!ops))
  		return true;
+
  #ifdef CONFIG_DMA_OPS_BYPASS
  	if (dev->dma_ops_bypass)
  		return min_not_zero(mask, dev->bus_dma_limit) >=

@@ -159,6 +176,8 @@ dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page,
  	if (dma_map_direct(dev, ops) ||
  	    arch_dma_map_page_direct(dev, page_to_phys(page) + offset + size))
  		addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
+	else if (use_dma_iommu(dev))
+		addr = iommu_dma_map_page(dev, page, offset, size, dir, attrs);
  	else
  		addr = ops->map_page(dev, page, offset, size, dir, attrs);
  	kmsan_handle_dma(page, offset, size, dir);

@@ -177,6 +196,8 @@ 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 (use_dma_iommu(dev))
+		iommu_dma_unmap_page(dev, addr, size, dir, attrs);
  	else
  		ops->unmap_page(dev, addr, size, dir, attrs);
  	debug_dma_unmap_page(dev, addr, size, dir);

@@ -197,6 +218,8 @@ static int __dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
  	if (dma_map_direct(dev, ops) ||
  	    arch_dma_map_sg_direct(dev, sg, nents))
  		ents = dma_direct_map_sg(dev, sg, nents, dir, attrs);
+	else if (use_dma_iommu(dev))
+		ents = iommu_dma_map_sg(dev, sg, nents, dir, attrs);
  	else
  		ents = ops->map_sg(dev, sg, nents, dir, attrs);
 

@@ -291,7 +314,9 @@ 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
+	else if (use_dma_iommu(dev))
+		iommu_dma_unmap_sg(dev, sg, nents, dir, attrs);
+	else if (ops->unmap_sg)
  		ops->unmap_sg(dev, sg, nents, dir, attrs);
  }
  EXPORT_SYMBOL(dma_unmap_sg_attrs);

@@ -309,6 +334,8 @@ dma_addr_t dma_map_resource(struct device *dev, phys_addr_t phys_addr,
 
  	if (dma_map_direct(dev, ops))
  		addr = dma_direct_map_resource(dev, phys_addr, size, dir, attrs);
+	else if (use_dma_iommu(dev))
+		addr = iommu_dma_map_resource(dev, phys_addr, size, dir, attrs);
  	else if (ops->map_resource)
  		addr = ops->map_resource(dev, phys_addr, size, dir, attrs);
 

@@ -323,8 +350,12 @@ void dma_unmap_resource(struct device *dev, dma_addr_t addr, size_t size,
  	const struct dma_map_ops *ops = get_dma_ops(dev);

kernel/dma/mapping.c +38 -8

  	debug_dma_unmap_resource(dev, addr, size, dir);
  }
  EXPORT_SYMBOL(dma_unmap_resource);

@@ -338,6 +369,8 @@ void __dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr, size_t size,
  	BUG_ON(!valid_dma_direction(dir));
  	if (dma_map_direct(dev, ops))
  		dma_direct_sync_single_for_cpu(dev, addr, size, dir);
+	else if (use_dma_iommu(dev))
+		iommu_dma_sync_single_for_cpu(dev, addr, size, dir);
  	else if (ops->sync_single_for_cpu)
  		ops->sync_single_for_cpu(dev, addr, size, dir);
  	debug_dma_sync_single_for_cpu(dev, addr, size, dir);

@@ -352,6 +385,8 @@ void __dma_sync_single_for_device(struct device *dev, dma_addr_t addr,
  	BUG_ON(!valid_dma_direction(dir));
  	if (dma_map_direct(dev, ops))
  		dma_direct_sync_single_for_device(dev, addr, size, dir);
+	else if (use_dma_iommu(dev))
+		iommu_dma_sync_single_for_device(dev, addr, size, dir);
  	else if (ops->sync_single_for_device)
  		ops->sync_single_for_device(dev, addr, size, dir);
  	debug_dma_sync_single_for_device(dev, addr, size, dir);

@@ -366,6 +401,8 @@ void __dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
  	BUG_ON(!valid_dma_direction(dir));
  	if (dma_map_direct(dev, ops))
  		dma_direct_sync_sg_for_cpu(dev, sg, nelems, dir);
+	else if (use_dma_iommu(dev))
+		iommu_dma_sync_sg_for_cpu(dev, sg, nelems, dir);
  	else if (ops->sync_sg_for_cpu)
  		ops->sync_sg_for_cpu(dev, sg, nelems, dir);
  	debug_dma_sync_sg_for_cpu(dev, sg, nelems, dir);

@@ -380,6 +417,8 @@ void __dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
  	BUG_ON(!valid_dma_direction(dir));
  	if (dma_map_direct(dev, ops))
  		dma_direct_sync_sg_for_device(dev, sg, nelems, dir);
+	else if (use_dma_iommu(dev))
+		iommu_dma_sync_sg_for_device(dev, sg, nelems, dir);
  	else if (ops->sync_sg_for_device)
  		ops->sync_sg_for_device(dev, sg, nelems, dir);
  	debug_dma_sync_sg_for_device(dev, sg, nelems, dir);

@@ -405,7 +444,7 @@ static void dma_setup_need_sync(struct device *dev)
  {
  	const struct dma_map_ops *ops = get_dma_ops(dev);
 
-	if (dma_map_direct(dev, ops) || (ops->flags & DMA_F_CAN_SKIP_SYNC))
+	if (dma_map_direct(dev, ops) || use_dma_iommu(dev))
  		/*
  		 * dma_skip_sync will be reset to %false on first SWIOTLB buffer
  		 * mapping, if any. During the device initialization, it's

@@ -446,6 +485,9 @@ int dma_get_sgtable_attrs(struct device *dev, struct sg_table *sgt,
  	if (dma_alloc_direct(dev, ops))
  		return dma_direct_get_sgtable(dev, sgt, cpu_addr, dma_addr,
  				size, attrs);
+	if (use_dma_iommu(dev))
+		return iommu_dma_get_sgtable(dev, sgt, cpu_addr, dma_addr,
+				size, attrs);
  	if (!ops->get_sgtable)
  		return -ENXIO;
  	return ops->get_sgtable(dev, sgt, cpu_addr, dma_addr, size, attrs);

@@ -482,6 +524,8 @@ bool dma_can_mmap(struct device *dev)
 
  	if (dma_alloc_direct(dev, ops))
  		return dma_direct_can_mmap(dev);
+	if (use_dma_iommu(dev))
+		return true;
  	return ops->mmap != NULL;
  }
  EXPORT_SYMBOL_GPL(dma_can_mmap);

@@ -508,6 +552,9 @@ int dma_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
  	if (dma_alloc_direct(dev, ops))
  		return dma_direct_mmap(dev, vma, cpu_addr, dma_addr, size,
  				attrs);
+	if (use_dma_iommu(dev))
+		return iommu_dma_mmap(dev, vma, cpu_addr, dma_addr, size,
+				      attrs);
  	if (!ops->mmap)
  		return -ENXIO;
  	return ops->mmap(dev, vma, cpu_addr, dma_addr, size, attrs);

@@ -559,6 +606,8 @@ void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
 
  	if (dma_alloc_direct(dev, ops))
  		cpu_addr = dma_direct_alloc(dev, size, dma_handle, flag, attrs);
+	else if (use_dma_iommu(dev))
+		cpu_addr = iommu_dma_alloc(dev, size, dma_handle, flag, attrs);
  	else if (ops->alloc)
  		cpu_addr = ops->alloc(dev, size, dma_handle, flag, attrs);
  	else

@@ -591,6 +640,8 @@ void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
  	debug_dma_free_coherent(dev, size, cpu_addr, dma_handle);
  	if (dma_alloc_direct(dev, ops))
  		dma_direct_free(dev, size, cpu_addr, dma_handle, attrs);
+	else if (use_dma_iommu(dev))
+		iommu_dma_free(dev, size, cpu_addr, dma_handle, attrs);
  	else if (ops->free)
  		ops->free(dev, size, cpu_addr, dma_handle, attrs);
  }

@@ -611,6 +662,8 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size,
  	size = PAGE_ALIGN(size);
  	if (dma_alloc_direct(dev, ops))
  		return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp);
+	if (use_dma_iommu(dev))
+		return dma_common_alloc_pages(dev, size, dma_handle, dir, gfp);
  	if (!ops->alloc_pages_op)
  		return NULL;
  	return ops->alloc_pages_op(dev, size, dma_handle, dir, gfp);

@@ -635,6 +688,8 @@ static void __dma_free_pages(struct device *dev, size_t size, struct page *page,
  	size = PAGE_ALIGN(size);
  	if (dma_alloc_direct(dev, ops))
  		dma_direct_free_pages(dev, size, page, dma_handle, dir);
+	else if (use_dma_iommu(dev))
+		dma_common_free_pages(dev, size, page, dma_handle, dir);
  	else if (ops->free_pages)
  		ops->free_pages(dev, size, page, dma_handle, dir);
  }

@@ -697,6 +752,8 @@ struct sg_table *dma_alloc_noncontiguous(struct device *dev, size_t size,
 
  	if (ops && ops->alloc_noncontiguous)
  		sgt = ops->alloc_noncontiguous(dev, size, dir, gfp, attrs);
+	else if (use_dma_iommu(dev))
+		sgt = iommu_dma_alloc_noncontiguous(dev, size, dir, gfp, attrs);
  	else
  		sgt = alloc_single_sgt(dev, size, dir, gfp);
 

@@ -725,6 +782,8 @@ void dma_free_noncontiguous(struct device *dev, size_t size,
  	debug_dma_unmap_sg(dev, sgt->sgl, sgt->orig_nents, dir);
  	if (ops && ops->free_noncontiguous)
  		ops->free_noncontiguous(dev, size, sgt, dir);
+	else if (use_dma_iommu(dev))
+		iommu_dma_free_noncontiguous(dev, size, sgt, dir);
  	else
  		free_single_sgt(dev, size, sgt, dir);
  }

@@ -772,6 +831,8 @@ static int dma_supported(struct device *dev, u64 mask)
  {
  	const struct dma_map_ops *ops = get_dma_ops(dev);
 
+	if (WARN_ON(ops && use_dma_iommu(dev)))
+		return false;
  	/*
  	 * ->dma_supported sets the bypass flag, so we must always call
  	 * into the method here unless the device is truly direct mapped.

@@ -787,17 +848,14 @@ bool dma_pci_p2pdma_supported(struct device *dev)
  {
  	const struct dma_map_ops *ops = get_dma_ops(dev);
 
-	/* if ops is not set, dma direct will be used which supports P2PDMA */
-	if (!ops)
-		return true;
-
  	/*
  	 * Note: dma_ops_bypass is not checked here because P2PDMA should
  	 * not be used with dma mapping ops that do not have support even
  	 * if the specific device is bypassing them.
  	 */
 
-	return ops->flags & DMA_F_PCI_P2PDMA_SUPPORTED;
+	/* if ops is not set, dma direct and default IOMMU support P2PDMA */
+	return !ops;
  }
  EXPORT_SYMBOL_GPL(dma_pci_p2pdma_supported);
 

@@ -865,6 +923,8 @@ size_t dma_max_mapping_size(struct device *dev)
 
  	if (dma_map_direct(dev, ops))
  		size = dma_direct_max_mapping_size(dev);
+	else if (use_dma_iommu(dev))
+		size = iommu_dma_max_mapping_size(dev);
  	else if (ops && ops->max_mapping_size)
  		size = ops->max_mapping_size(dev);
 

@@ -877,9 +937,10 @@ size_t dma_opt_mapping_size(struct device *dev)
  	const struct dma_map_ops *ops = get_dma_ops(dev);
  	size_t size = SIZE_MAX;
 
-	if (ops && ops->opt_mapping_size)
+	if (use_dma_iommu(dev))
+		size = iommu_dma_opt_mapping_size();
+	else if (ops && ops->opt_mapping_size)
  		size = ops->opt_mapping_size();
-
  	return min(dma_max_mapping_size(dev), size);
  }
  EXPORT_SYMBOL_GPL(dma_opt_mapping_size);

@@ -888,7 +949,12 @@ unsigned long dma_get_merge_boundary(struct device *dev)
  {

Quoted 28 lines · Leon Romanovsky

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, but for all modern platforms dma-iommu implementation is really matters.

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.

Signed-off-by Leon Romanovsky <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---
  MAINTAINERS                 |   1 +
  drivers/iommu/Kconfig       |   2 +-
  drivers/iommu/dma-iommu.c   | 121 ++++++++++----------------
  drivers/iommu/intel/Kconfig |   1 -
  include/linux/device.h      |   5 ++
  include/linux/dma-map-ops.h |  39 +++++----
  include/linux/iommu-dma.h   | 169 ++++++++++++++++++++++++++++++++++++
  kernel/dma/Kconfig          |   6 ++
  kernel/dma/Makefile         |   2 +-
  kernel/dma/mapping.c        |  90 ++++++++++++++++---
  10 files changed, 327 insertions(+), 109 deletions(-)

create mode 100644 include/linux/iommu-dma.h

MAINTAINERS T: git git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git · line 11547

Quoted 9 lines

MAINTAINERS +1 -0

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

This would belong to the "IOMMU DMA-API LAYER" section just above.

Quoted 45 lines

MAINTAINERS +0 -0

  F:	include/linux/iommu.h
  F:	include/linux/iova.h
  F:	include/linux/of_iommu.h

drivers/iommu/Kconfig +1 -1

index c04584be3089..e24cb857b66c 100644
@@ -151,7 +151,7 @@ config OF_IOMMU
  # IOMMU-agnostic DMA-mapping layer
  config IOMMU_DMA
  	def_bool ARM64 || X86 || S390
-	select DMA_OPS
+	select DMA_OPS_HELPERS
  	select IOMMU_API
  	select IOMMU_IOVA
  	select IRQ_MSI_IOMMU

drivers/iommu/dma-iommu.c +3 -3

index 43520e7275cc..ab2d3092ac23 100644
@@ -17,6 +17,7 @@
  #include <linux/gfp.h>
  #include <linux/huge_mm.h>
  #include <linux/iommu.h>
+#include <linux/iommu-dma.h>
  #include <linux/iova.h>
  #include <linux/irq.h>
  #include <linux/list_sort.h>

@@ -1039,9 +1040,8 @@ static void *iommu_dma_alloc_remap(struct device *dev, size_t size,
  	return NULL;
  }
 
-static struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev,
-		size_t size, enum dma_data_direction dir, gfp_t gfp,
-		unsigned long attrs)
+struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev, size_t size,
+	       enum dma_data_direction dir, gfp_t gfp, unsigned long attrs)
  {
  	struct dma_sgt_handle *sh;
 

@@ -1058,8 +1058,8 @@ static struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev,
  	return &sh->sgt;
  }
 

drivers/iommu/dma-iommu.c static struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev, · line 1062

Quoted 4 lines

drivers/iommu/dma-iommu.c +2 -2

-static void iommu_dma_free_noncontiguous(struct device *dev, size_t size,
-		struct sg_table *sgt, enum dma_data_direction dir)
+void iommu_dma_free_noncontiguous(struct device *dev, size_t size,
+		  struct sg_table *sgt, enum dma_data_direction dir)

Nit: Weird indentation (and in a few other places as well). The original style here was two tabs, and TBH I don't see any particularly compelling reason to churn that excepct in places where rewrapping will actually reduce the line count.

Quoted 225 lines

drivers/iommu/dma-iommu.c +33 -63

  {
  	struct dma_sgt_handle *sh = sgt_handle(sgt);
 

@@ -1069,8 +1069,8 @@ static void iommu_dma_free_noncontiguous(struct device *dev, size_t size,
  	kfree(sh);
  }
 
-static void iommu_dma_sync_single_for_cpu(struct device *dev,
-		dma_addr_t dma_handle, size_t size, enum dma_data_direction dir)
+void iommu_dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle,
+				   size_t size, enum dma_data_direction dir)
  {
  	phys_addr_t phys;
 

@@ -1085,8 +1085,8 @@ static void iommu_dma_sync_single_for_cpu(struct device *dev,
  		swiotlb_sync_single_for_cpu(dev, phys, size, dir);
  }
 
-static void iommu_dma_sync_single_for_device(struct device *dev,
-		dma_addr_t dma_handle, size_t size, enum dma_data_direction dir)
+void iommu_dma_sync_single_for_device(struct device *dev, dma_addr_t dma_handle,
+				      size_t size, enum dma_data_direction dir)
  {
  	phys_addr_t phys;
 

@@ -1101,9 +1101,8 @@ static void iommu_dma_sync_single_for_device(struct device *dev,
  		arch_sync_dma_for_device(phys, size, dir);
  }
 
-static void iommu_dma_sync_sg_for_cpu(struct device *dev,
-		struct scatterlist *sgl, int nelems,
-		enum dma_data_direction dir)
+void iommu_dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sgl,
+			       int nelems, enum dma_data_direction dir)
  {
  	struct scatterlist *sg;
  	int i;

@@ -1117,9 +1116,8 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
  			arch_sync_dma_for_cpu(sg_phys(sg), sg->length, dir);
  }
 
-static void iommu_dma_sync_sg_for_device(struct device *dev,
-		struct scatterlist *sgl, int nelems,
-		enum dma_data_direction dir)
+void iommu_dma_sync_sg_for_device(struct device *dev, struct scatterlist *sgl,
+				  int nelems, enum dma_data_direction dir)
  {
  	struct scatterlist *sg;
  	int i;

@@ -1134,9 +1132,9 @@ static void iommu_dma_sync_sg_for_device(struct device *dev,
  			arch_sync_dma_for_device(sg_phys(sg), sg->length, dir);
  }
 
-static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
-		unsigned long offset, size_t size, enum dma_data_direction dir,
-		unsigned long attrs)
+dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
+	      unsigned long offset, size_t size, enum dma_data_direction dir,
+	      unsigned long attrs)
  {
  	phys_addr_t phys = page_to_phys(page) + offset;
  	bool coherent = dev_is_dma_coherent(dev);

@@ -1194,8 +1192,8 @@ static dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
  	return iova;
  }
 
-static void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
-		size_t size, enum dma_data_direction dir, unsigned long attrs)
+void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
+	  size_t size, enum dma_data_direction dir, unsigned long attrs)
  {
  	struct iommu_domain *domain = iommu_get_dma_domain(dev);
  	phys_addr_t phys;

@@ -1348,8 +1346,8 @@ static int iommu_dma_map_sg_swiotlb(struct device *dev, struct scatterlist *sg,
   * impedance-matching, to be able to hand off a suitably-aligned list,
   * but still preserve the original offsets and sizes for the caller.
   */
-static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
-		int nents, enum dma_data_direction dir, unsigned long attrs)
+int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+		     enum dma_data_direction dir, unsigned long attrs)
  {
  	struct iommu_domain *domain = iommu_get_dma_domain(dev);
  	struct iommu_dma_cookie *cookie = domain->iova_cookie;

@@ -1468,8 +1466,8 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
  	return ret;
  }
 
-static void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
-		int nents, enum dma_data_direction dir, unsigned long attrs)
+void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
+			enum dma_data_direction dir, unsigned long attrs)
  {
  	dma_addr_t end = 0, start;
  	struct scatterlist *tmp;

@@ -1518,16 +1516,17 @@ static void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
  		__iommu_dma_unmap(dev, start, end - start);
  }
 
-static dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys,
-		size_t size, enum dma_data_direction dir, unsigned long attrs)
+dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys,
+				  size_t size, enum dma_data_direction dir,
+				  unsigned long attrs)
  {
  	return __iommu_dma_map(dev, phys, size,
  			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO,
  			dma_get_mask(dev));
  }
 
-static void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
-		size_t size, enum dma_data_direction dir, unsigned long attrs)
+void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
+	      size_t size, enum dma_data_direction dir, unsigned long attrs)
  {
  	__iommu_dma_unmap(dev, handle, size);
  }

@@ -1563,8 +1562,8 @@ static void __iommu_dma_free(struct device *dev, size_t size, void *cpu_addr)
  		dma_free_contiguous(dev, page, alloc_size);
  }
 
-static void iommu_dma_free(struct device *dev, size_t size, void *cpu_addr,
-		dma_addr_t handle, unsigned long attrs)
+void iommu_dma_free(struct device *dev, size_t size, void *cpu_addr,
+		    dma_addr_t handle, unsigned long attrs)
  {
  	__iommu_dma_unmap(dev, handle, size);
  	__iommu_dma_free(dev, size, cpu_addr);

@@ -1607,8 +1606,8 @@ static void *iommu_dma_alloc_pages(struct device *dev, size_t size,
  	return NULL;
  }
 
-static void *iommu_dma_alloc(struct device *dev, size_t size,
-		dma_addr_t *handle, gfp_t gfp, unsigned long attrs)
+void *iommu_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
+		      gfp_t gfp, unsigned long attrs)
  {
  	bool coherent = dev_is_dma_coherent(dev);
  	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);

@@ -1642,9 +1641,8 @@ static void *iommu_dma_alloc(struct device *dev, size_t size,
  	return cpu_addr;
  }
 
-static int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
-		void *cpu_addr, dma_addr_t dma_addr, size_t size,
-		unsigned long attrs)
+int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+	void *cpu_addr, dma_addr_t dma_addr, size_t size, unsigned long attrs)
  {
  	unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
  	unsigned long pfn, off = vma->vm_pgoff;

@@ -1673,9 +1671,8 @@ static int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
  			       vma->vm_page_prot);
  }
 
-static int iommu_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
-		void *cpu_addr, dma_addr_t dma_addr, size_t size,
-		unsigned long attrs)
+int iommu_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
+	void *cpu_addr, dma_addr_t dma_addr, size_t size, unsigned long attrs)
  {
  	struct page *page;
  	int ret;

@@ -1700,19 +1697,19 @@ static int iommu_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
  	return ret;
  }
 
-static unsigned long iommu_dma_get_merge_boundary(struct device *dev)
+unsigned long iommu_dma_get_merge_boundary(struct device *dev)
  {
  	struct iommu_domain *domain = iommu_get_dma_domain(dev);
 
  	return (1UL << __ffs(domain->pgsize_bitmap)) - 1;
  }
 
-static size_t iommu_dma_opt_mapping_size(void)
+size_t iommu_dma_opt_mapping_size(void)
  {
  	return iova_rcache_range();
  }
 
-static size_t iommu_dma_max_mapping_size(struct device *dev)
+size_t iommu_dma_max_mapping_size(struct device *dev)
  {
  	if (dev_is_untrusted(dev))
  		return swiotlb_max_mapping_size(dev);

@@ -1720,32 +1717,6 @@ static size_t iommu_dma_max_mapping_size(struct device *dev)
  	return SIZE_MAX;
  }
 
-static const struct dma_map_ops iommu_dma_ops = {
-	.flags			= DMA_F_PCI_P2PDMA_SUPPORTED |
-				  DMA_F_CAN_SKIP_SYNC,
-	.alloc			= iommu_dma_alloc,
-	.free			= iommu_dma_free,
-	.alloc_pages_op		= dma_common_alloc_pages,
-	.free_pages		= dma_common_free_pages,
-	.alloc_noncontiguous	= iommu_dma_alloc_noncontiguous,
-	.free_noncontiguous	= iommu_dma_free_noncontiguous,
-	.mmap			= iommu_dma_mmap,
-	.get_sgtable		= iommu_dma_get_sgtable,
-	.map_page		= iommu_dma_map_page,
-	.unmap_page		= iommu_dma_unmap_page,
-	.map_sg			= iommu_dma_map_sg,
-	.unmap_sg		= iommu_dma_unmap_sg,
-	.sync_single_for_cpu	= iommu_dma_sync_single_for_cpu,
-	.sync_single_for_device	= iommu_dma_sync_single_for_device,
-	.sync_sg_for_cpu	= iommu_dma_sync_sg_for_cpu,
-	.sync_sg_for_device	= iommu_dma_sync_sg_for_device,
-	.map_resource		= iommu_dma_map_resource,
-	.unmap_resource		= iommu_dma_unmap_resource,
-	.get_merge_boundary	= iommu_dma_get_merge_boundary,
-	.opt_mapping_size	= iommu_dma_opt_mapping_size,
-	.max_mapping_size       = iommu_dma_max_mapping_size,
-};
-
  void iommu_setup_dma_ops(struct device *dev)
  {
  	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);

@@ -1753,19 +1724,15 @@ void iommu_setup_dma_ops(struct device *dev)
  	if (dev_is_pci(dev))
  		dev->iommu->pci_32bit_workaround = !iommu_dma_forcedac;
 
-	if (iommu_is_dma_domain(domain)) {

drivers/iommu/dma-iommu.c void iommu_setup_dma_ops(struct device *dev) · line 1729

Quoted 10 lines

drivers/iommu/dma-iommu.c +3 -7

-		if (iommu_dma_init_domain(domain, dev))
-			goto out_err;
-		dev->dma_ops = &iommu_dma_ops;
-	} else if (dev->dma_ops == &iommu_dma_ops) {
-		/* Clean up if we've switched *from* a DMA domain */
-		dev->dma_ops = NULL;
-	}
+	dev->dma_iommu = iommu_is_dma_domain(domain);
+	if (dev->dma_iommu && iommu_dma_init_domain(domain, dev))
+		goto out_err;

FWIW I find this a little less clear than the original flow, but then I'm hoping to rip this all apart soon enough anyway, so I guess it doesn't really matter.

Quoted 36 lines

drivers/iommu/dma-iommu.c +3 -2

  	return;
  out_err:
-	 pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
-		 dev_name(dev));
+	pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
+		dev_name(dev));
+	dev->dma_iommu = false;
  }
 
  static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,

drivers/iommu/intel/Kconfig +0 -1

index f52fb39c968e..88fd32a9323c 100644
@@ -12,7 +12,6 @@ config DMAR_DEBUG
  config INTEL_IOMMU
  	bool "Support for Intel IOMMU using DMA Remapping Devices"
  	depends on PCI_MSI && ACPI && X86
-	select DMA_OPS
  	select IOMMU_API
  	select IOMMU_IOVA
  	select IOMMUFD_DRIVER if IOMMUFD

include/linux/device.h +2 -0

index ace039151cb8..e66ec47ceb09 100644
@@ -707,6 +707,8 @@ struct device_physical_location {
   *		for dma allocations.  This flag is managed by the dma ops
   *		instance from ->dma_supported.
   * @dma_skip_sync: DMA sync operations can be skipped for coherent buffers.
+ * @dma_iommu: Device is using default IOMMU implementation for DMA and
+ *		doesn't rely on dma_ops structure.
   *
   * At the lowest level, every device in a Linux system is represented by an
   * instance of struct device. The device structure contains the information

include/linux/device.h struct device { · line 828

Quoted 5 lines

include/linux/device.h +2 -0

@@ -822,6 +824,9 @@ struct device {
  #ifdef CONFIG_DMA_NEED_SYNC
  	bool			dma_skip_sync:1;
  #endif
+#ifdef CONFIG_IOMMU_DMA
+	bool			dma_iommu : 1;

Nit: I guess dma_ops_bypass already set a precedent for inconsistent formatting here, but still...

Quoted 30 lines

include/linux/device.h +1 -0

+#endif
  };
 
  /**

include/linux/dma-map-ops.h +0 -13

index 02a1c825896b..103d9c66c445 100644
@@ -13,20 +13,7 @@
  struct cma;
  struct iommu_ops;
 
-/*
- * Values for struct dma_map_ops.flags:
- *
- * DMA_F_PCI_P2PDMA_SUPPORTED: Indicates the dma_map_ops implementation can
- * 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.
- */
-#define DMA_F_PCI_P2PDMA_SUPPORTED     (1 << 0)
-#define DMA_F_CAN_SKIP_SYNC            (1 << 1)
-
  struct dma_map_ops {
-	unsigned int flags;
-
  	void *(*alloc)(struct device *dev, size_t size,
  			dma_addr_t *dma_handle, gfp_t gfp,
  			unsigned long attrs);

include/linux/dma-map-ops.h static inline void set_dma_ops(struct device *dev, · line 105

Quoted 5 lines

include/linux/dma-map-ops.h +2 -0

@@ -114,6 +101,28 @@ static inline void set_dma_ops(struct device *dev,
  }
  #endif /* CONFIG_DMA_OPS */
 
+#ifdef CONFIG_DMA_OPS_HELPERS
+#include <asm/dma-mapping.h>

What needs a definition of get_arch_dma_ops() in this scope?

include/linux/dma-map-ops.h static inline void set_dma_ops(struct device *dev, · line 110

Quoted 4 lines

include/linux/dma-map-ops.h +4 -0

+struct page *dma_common_alloc_pages(struct device *dev, size_t size,
+		dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp);
+void dma_common_free_pages(struct device *dev, size_t size, struct page *vaddr,
+		dma_addr_t dma_handle, enum dma_data_direction dir);

Why not bring the rest of the dma_common_* declarations in here as well? (Or more literally, add the #ifdef around where they all are already)

Quoted 209 lines

include/linux/dma-map-ops.h +15 -4

+#else /* CONFIG_DMA_OPS_HELPERS */
+static inline struct page *
+dma_common_alloc_pages(struct device *dev, size_t size, dma_addr_t *dma_handle,
+		       enum dma_data_direction dir, gfp_t gfp)
+{
+	return NULL;
+}
+static inline void dma_common_free_pages(struct device *dev, size_t size,
+					 struct page *vaddr,
+					 dma_addr_t dma_handle,
+					 enum dma_data_direction dir)
+{
+}
+#endif /* CONFIG_DMA_OPS_HELPERS */
+
  #ifdef CONFIG_DMA_CMA
  extern struct cma *dma_contiguous_default_area;
 

@@ -239,10 +248,6 @@ int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,
  int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
  		void *cpu_addr, dma_addr_t dma_addr, size_t size,
  		unsigned long attrs);
-struct page *dma_common_alloc_pages(struct device *dev, size_t size,
-		dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp);
-void dma_common_free_pages(struct device *dev, size_t size, struct page *vaddr,
-		dma_addr_t dma_handle, enum dma_data_direction dir);
 
  struct page **dma_common_find_pages(void *cpu_addr);
  void *dma_common_contiguous_remap(struct page *page, size_t size, pgprot_t prot,

include/linux/iommu-dma.h new file +169 -0

new file mode 100644
index 000000000000..622232fc9510
@@ -0,0 +1,169 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved
+ *
+ * DMA operations that map physical memory through IOMMU.
+ */
+#ifndef _LINUX_IOMMU_DMA_H
+#define _LINUX_IOMMU_DMA_H
+
+#include <linux/dma-direction.h>
+
+#ifdef CONFIG_IOMMU_DMA
+dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
+			      unsigned long offset, size_t size,
+			      enum dma_data_direction dir, unsigned long attrs);
+void iommu_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
+			  size_t size, enum dma_data_direction dir,
+			  unsigned long attrs);
+int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+		     enum dma_data_direction dir, unsigned long attrs);
+void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
+			enum dma_data_direction dir, unsigned long attrs);
+void *iommu_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
+		      gfp_t gfp, unsigned long attrs);
+int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+		   void *cpu_addr, dma_addr_t dma_addr,
+		   size_t size, unsigned long attrs);
+int iommu_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
+			  void *cpu_addr, dma_addr_t dma_addr, size_t size,
+			  unsigned long attrs);
+unsigned long iommu_dma_get_merge_boundary(struct device *dev);
+size_t iommu_dma_opt_mapping_size(void);
+size_t iommu_dma_max_mapping_size(struct device *dev);
+void iommu_dma_free(struct device *dev, size_t size, void *cpu_addr,
+		    dma_addr_t handle, unsigned long attrs);
+dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys,
+				  size_t size, enum dma_data_direction dir,
+				  unsigned long attrs);
+void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
+			      size_t size, enum dma_data_direction dir,
+			      unsigned long attrs);
+struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev, size_t size,
+					       enum dma_data_direction dir,
+					       gfp_t gfp, unsigned long attrs);
+void iommu_dma_free_noncontiguous(struct device *dev, size_t size,
+				  struct sg_table *sgt,
+				  enum dma_data_direction dir);
+void iommu_dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle,
+				   size_t size, enum dma_data_direction dir);
+void iommu_dma_sync_single_for_device(struct device *dev, dma_addr_t dma_handle,
+				      size_t size, enum dma_data_direction dir);
+void iommu_dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sgl,
+			       int nelems, enum dma_data_direction dir);
+void iommu_dma_sync_sg_for_device(struct device *dev, struct scatterlist *sgl,
+				  int nelems, enum dma_data_direction dir);
+#else
+static inline dma_addr_t iommu_dma_map_page(struct device *dev,
+					    struct page *page,
+					    unsigned long offset, size_t size,
+					    enum dma_data_direction dir,
+					    unsigned long attrs)
+{
+	return DMA_MAPPING_ERROR;
+}
+static inline void iommu_dma_unmap_page(struct device *dev,
+					dma_addr_t dma_handle, size_t size,
+					enum dma_data_direction dir,
+					unsigned long attrs)
+{
+}
+static inline int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
+				   int nents, enum dma_data_direction dir,
+				   unsigned long attrs)
+{
+	return -EINVAL;
+}
+static inline void iommu_dma_unmap_sg(struct device *dev,
+				      struct scatterlist *sg, int nents,
+				      enum dma_data_direction dir,
+				      unsigned long attrs)
+{
+}
+static inline void *iommu_dma_alloc(struct device *dev, size_t size,
+				    dma_addr_t *handle, gfp_t gfp,
+				    unsigned long attrs)
+{
+	return NULL;
+}
+static inline int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+				 void *cpu_addr, dma_addr_t dma_addr,
+				 size_t size, unsigned long attrs)
+{
+	return -EINVAL;
+}
+static inline int iommu_dma_get_sgtable(struct device *dev,
+					struct sg_table *sgt, void *cpu_addr,
+					dma_addr_t dma_addr, size_t size,
+					unsigned long attrs)
+{
+	return -EINVAL;
+}
+static inline unsigned long iommu_dma_get_merge_boundary(struct device *dev)
+{
+	return 0;
+}
+static inline size_t iommu_dma_opt_mapping_size(void)
+{
+	return 0;
+}
+static inline size_t iommu_dma_max_mapping_size(struct device *dev)
+{
+	return 0;
+}
+static inline void iommu_dma_free(struct device *dev, size_t size,
+				  void *cpu_addr, dma_addr_t handle,
+				  unsigned long attrs)
+{
+}
+static inline dma_addr_t iommu_dma_map_resource(struct device *dev,
+						phys_addr_t phys, size_t size,
+						enum dma_data_direction dir,
+						unsigned long attrs)
+{
+	return DMA_MAPPING_ERROR;
+}
+static inline void iommu_dma_unmap_resource(struct device *dev,
+					    dma_addr_t handle, size_t size,
+					    enum dma_data_direction dir,
+					    unsigned long attrs)
+{
+}
+static inline struct sg_table *
+iommu_dma_alloc_noncontiguous(struct device *dev, size_t size,
+			      enum dma_data_direction dir, gfp_t gfp,
+			      unsigned long attrs)
+{
+	return NULL;
+}
+static inline void iommu_dma_free_noncontiguous(struct device *dev, size_t size,
+						struct sg_table *sgt,
+						enum dma_data_direction dir)
+{
+}
+static inline void iommu_dma_sync_single_for_cpu(struct device *dev,
+						 dma_addr_t dma_handle,
+						 size_t size,
+						 enum dma_data_direction dir)
+{
+}
+static inline void iommu_dma_sync_single_for_device(struct device *dev,
+						    dma_addr_t dma_handle,
+						    size_t size,
+						    enum dma_data_direction dir)
+{
+}
+static inline void iommu_dma_sync_sg_for_cpu(struct device *dev,
+					     struct scatterlist *sgl,
+					     int nelems,
+					     enum dma_data_direction dir)
+{
+}
+static inline void iommu_dma_sync_sg_for_device(struct device *dev,
+						struct scatterlist *sgl,
+						int nelems,
+						enum dma_data_direction dir)
+{
+}
+#endif /* CONFIG_IOMMU_DMA */
+#endif /* _LINUX_IOMMU_DMA_H */

kernel/dma/Kconfig +0 -0

index c06e56be0ca1..03bb925014a7 100644

kernel/dma/Kconfig config HAS_DMA · line 12

Quoted 5 lines

kernel/dma/Kconfig +2 -0

@@ -8,8 +8,14 @@ config HAS_DMA
  	depends on !NO_DMA
  	default y
 
+# DMA IOMMU uses common ops helpers for certain operations, so let's allow to build
+# ops_helpers.c even if DMA_OPS is not enabled

Hmm, but actually dma-direct also uses dma_common_contiguous_remap(), so something seems a little inconsistent here...

Quoted 27 lines

kernel/dma/Kconfig +4 -0

+config DMA_OPS_HELPERS
+	bool
+
  config DMA_OPS
  	depends on HAS_DMA
+	select DMA_OPS_HELPERS
  	bool
 
  #

kernel/dma/Makefile +1 -1

index 21926e46ef4f..2e6e933cf7f3 100644
@@ -1,7 +1,7 @@
  # SPDX-License-Identifier: GPL-2.0
 
  obj-$(CONFIG_HAS_DMA)			+= mapping.o direct.o
-obj-$(CONFIG_DMA_OPS)			+= ops_helpers.o
+obj-$(CONFIG_DMA_OPS_HELPERS)		+= ops_helpers.o
  obj-$(CONFIG_DMA_OPS)			+= dummy.o
  obj-$(CONFIG_DMA_CMA)			+= contiguous.o
  obj-$(CONFIG_DMA_DECLARE_COHERENT)	+= coherent.o

kernel/dma/mapping.c +0 -0

index 6832fd6f0796..02451e27e0b1 100644

kernel/dma/mapping.c line 11

Quoted 4 lines

kernel/dma/mapping.c +1 -0

@@ -8,6 +8,7 @@
  #include <linux/memblock.h> /* for max_pfn */
  #include <linux/acpi.h>
  #include <linux/dma-map-ops.h>
+#include <linux/iommu-dma.h>

Nit: it would be nice to ignore the one existing outlier and maintain the otherwise alphabetical order.

Quoted 80 lines

kernel/dma/mapping.c +27 -1

  #include <linux/export.h>
  #include <linux/gfp.h>
  #include <linux/kmsan.h>

@@ -113,11 +114,27 @@ void *dmam_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
  }
  EXPORT_SYMBOL(dmam_alloc_attrs);
 
+#ifdef CONFIG_IOMMU_DMA
+static bool use_dma_iommu(struct device *dev)
+{
+	return dev->dma_iommu;
+}
+#else
+static bool use_dma_iommu(struct device *dev)
+{
+	return false;
+}
+#endif
+
  static bool dma_go_direct(struct device *dev, dma_addr_t mask,
  		const struct dma_map_ops *ops)
  {
+	if (use_dma_iommu(dev))
+		return false;
+
  	if (likely(!ops))
  		return true;
+
  #ifdef CONFIG_DMA_OPS_BYPASS
  	if (dev->dma_ops_bypass)
  		return min_not_zero(mask, dev->bus_dma_limit) >=

@@ -159,6 +176,8 @@ dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page,
  	if (dma_map_direct(dev, ops) ||
  	    arch_dma_map_page_direct(dev, page_to_phys(page) + offset + size))
  		addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
+	else if (use_dma_iommu(dev))
+		addr = iommu_dma_map_page(dev, page, offset, size, dir, attrs);
  	else
  		addr = ops->map_page(dev, page, offset, size, dir, attrs);
  	kmsan_handle_dma(page, offset, size, dir);

@@ -177,6 +196,8 @@ 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 (use_dma_iommu(dev))
+		iommu_dma_unmap_page(dev, addr, size, dir, attrs);
  	else
  		ops->unmap_page(dev, addr, size, dir, attrs);
  	debug_dma_unmap_page(dev, addr, size, dir);

@@ -197,6 +218,8 @@ static int __dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
  	if (dma_map_direct(dev, ops) ||
  	    arch_dma_map_sg_direct(dev, sg, nents))
  		ents = dma_direct_map_sg(dev, sg, nents, dir, attrs);
+	else if (use_dma_iommu(dev))
+		ents = iommu_dma_map_sg(dev, sg, nents, dir, attrs);
  	else
  		ents = ops->map_sg(dev, sg, nents, dir, attrs);
 

@@ -291,7 +314,9 @@ 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
+	else if (use_dma_iommu(dev))
+		iommu_dma_unmap_sg(dev, sg, nents, dir, attrs);
+	else if (ops->unmap_sg)
  		ops->unmap_sg(dev, sg, nents, dir, attrs);
  }
  EXPORT_SYMBOL(dma_unmap_sg_attrs);

@@ -309,6 +334,8 @@ dma_addr_t dma_map_resource(struct device *dev, phys_addr_t phys_addr,
 
  	if (dma_map_direct(dev, ops))
  		addr = dma_direct_map_resource(dev, phys_addr, size, dir, attrs);
+	else if (use_dma_iommu(dev))
+		addr = iommu_dma_map_resource(dev, phys_addr, size, dir, attrs);
  	else if (ops->map_resource)
  		addr = ops->map_resource(dev, phys_addr, size, dir, attrs);
 

@@ -323,8 +350,12 @@ void dma_unmap_resource(struct device *dev, dma_addr_t addr, size_t size,
  	const struct dma_map_ops *ops = get_dma_ops(dev);

kernel/dma/mapping.c void dma_unmap_resource(struct device *dev, dma_addr_t addr, size_t size, · line 358

Quoted 10 lines

kernel/dma/mapping.c +6 -2

 
  	BUG_ON(!valid_dma_direction(dir));
-	if (!dma_map_direct(dev, ops) && ops->unmap_resource)
-		ops->unmap_resource(dev, addr, size, dir, attrs);
+	if (dma_map_direct(dev, ops))
+		; /* nothing to do: uncached and no swiotlb */
+	else if (use_dma_iommu(dev))
+			iommu_dma_unmap_resource(dev, addr, size, dir, attrs);
+	else if (ops->unmap_resource)
+			ops->unmap_resource(dev, addr, size, dir, attrs);

Nit: extra tabs here and above.

Quoted 184 lines

kernel/dma/mapping.c +38 -8

  	debug_dma_unmap_resource(dev, addr, size, dir);
  }
  EXPORT_SYMBOL(dma_unmap_resource);

@@ -338,6 +369,8 @@ void __dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr, size_t size,
  	BUG_ON(!valid_dma_direction(dir));
  	if (dma_map_direct(dev, ops))
  		dma_direct_sync_single_for_cpu(dev, addr, size, dir);
+	else if (use_dma_iommu(dev))
+		iommu_dma_sync_single_for_cpu(dev, addr, size, dir);
  	else if (ops->sync_single_for_cpu)
  		ops->sync_single_for_cpu(dev, addr, size, dir);
  	debug_dma_sync_single_for_cpu(dev, addr, size, dir);

@@ -352,6 +385,8 @@ void __dma_sync_single_for_device(struct device *dev, dma_addr_t addr,
  	BUG_ON(!valid_dma_direction(dir));
  	if (dma_map_direct(dev, ops))
  		dma_direct_sync_single_for_device(dev, addr, size, dir);
+	else if (use_dma_iommu(dev))
+		iommu_dma_sync_single_for_device(dev, addr, size, dir);
  	else if (ops->sync_single_for_device)
  		ops->sync_single_for_device(dev, addr, size, dir);
  	debug_dma_sync_single_for_device(dev, addr, size, dir);

@@ -366,6 +401,8 @@ void __dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
  	BUG_ON(!valid_dma_direction(dir));
  	if (dma_map_direct(dev, ops))
  		dma_direct_sync_sg_for_cpu(dev, sg, nelems, dir);
+	else if (use_dma_iommu(dev))
+		iommu_dma_sync_sg_for_cpu(dev, sg, nelems, dir);
  	else if (ops->sync_sg_for_cpu)
  		ops->sync_sg_for_cpu(dev, sg, nelems, dir);
  	debug_dma_sync_sg_for_cpu(dev, sg, nelems, dir);

@@ -380,6 +417,8 @@ void __dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
  	BUG_ON(!valid_dma_direction(dir));
  	if (dma_map_direct(dev, ops))
  		dma_direct_sync_sg_for_device(dev, sg, nelems, dir);
+	else if (use_dma_iommu(dev))
+		iommu_dma_sync_sg_for_device(dev, sg, nelems, dir);
  	else if (ops->sync_sg_for_device)
  		ops->sync_sg_for_device(dev, sg, nelems, dir);
  	debug_dma_sync_sg_for_device(dev, sg, nelems, dir);

@@ -405,7 +444,7 @@ static void dma_setup_need_sync(struct device *dev)
  {
  	const struct dma_map_ops *ops = get_dma_ops(dev);
 
-	if (dma_map_direct(dev, ops) || (ops->flags & DMA_F_CAN_SKIP_SYNC))
+	if (dma_map_direct(dev, ops) || use_dma_iommu(dev))
  		/*
  		 * dma_skip_sync will be reset to %false on first SWIOTLB buffer
  		 * mapping, if any. During the device initialization, it's

@@ -446,6 +485,9 @@ int dma_get_sgtable_attrs(struct device *dev, struct sg_table *sgt,
  	if (dma_alloc_direct(dev, ops))
  		return dma_direct_get_sgtable(dev, sgt, cpu_addr, dma_addr,
  				size, attrs);
+	if (use_dma_iommu(dev))
+		return iommu_dma_get_sgtable(dev, sgt, cpu_addr, dma_addr,
+				size, attrs);
  	if (!ops->get_sgtable)
  		return -ENXIO;
  	return ops->get_sgtable(dev, sgt, cpu_addr, dma_addr, size, attrs);

@@ -482,6 +524,8 @@ bool dma_can_mmap(struct device *dev)
 
  	if (dma_alloc_direct(dev, ops))
  		return dma_direct_can_mmap(dev);
+	if (use_dma_iommu(dev))
+		return true;
  	return ops->mmap != NULL;
  }
  EXPORT_SYMBOL_GPL(dma_can_mmap);

@@ -508,6 +552,9 @@ int dma_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
  	if (dma_alloc_direct(dev, ops))
  		return dma_direct_mmap(dev, vma, cpu_addr, dma_addr, size,
  				attrs);
+	if (use_dma_iommu(dev))
+		return iommu_dma_mmap(dev, vma, cpu_addr, dma_addr, size,
+				      attrs);
  	if (!ops->mmap)
  		return -ENXIO;
  	return ops->mmap(dev, vma, cpu_addr, dma_addr, size, attrs);

@@ -559,6 +606,8 @@ void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
 
  	if (dma_alloc_direct(dev, ops))
  		cpu_addr = dma_direct_alloc(dev, size, dma_handle, flag, attrs);
+	else if (use_dma_iommu(dev))
+		cpu_addr = iommu_dma_alloc(dev, size, dma_handle, flag, attrs);
  	else if (ops->alloc)
  		cpu_addr = ops->alloc(dev, size, dma_handle, flag, attrs);
  	else

@@ -591,6 +640,8 @@ void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
  	debug_dma_free_coherent(dev, size, cpu_addr, dma_handle);
  	if (dma_alloc_direct(dev, ops))
  		dma_direct_free(dev, size, cpu_addr, dma_handle, attrs);
+	else if (use_dma_iommu(dev))
+		iommu_dma_free(dev, size, cpu_addr, dma_handle, attrs);
  	else if (ops->free)
  		ops->free(dev, size, cpu_addr, dma_handle, attrs);
  }

@@ -611,6 +662,8 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size,
  	size = PAGE_ALIGN(size);
  	if (dma_alloc_direct(dev, ops))
  		return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp);
+	if (use_dma_iommu(dev))
+		return dma_common_alloc_pages(dev, size, dma_handle, dir, gfp);
  	if (!ops->alloc_pages_op)
  		return NULL;
  	return ops->alloc_pages_op(dev, size, dma_handle, dir, gfp);

@@ -635,6 +688,8 @@ static void __dma_free_pages(struct device *dev, size_t size, struct page *page,
  	size = PAGE_ALIGN(size);
  	if (dma_alloc_direct(dev, ops))
  		dma_direct_free_pages(dev, size, page, dma_handle, dir);
+	else if (use_dma_iommu(dev))
+		dma_common_free_pages(dev, size, page, dma_handle, dir);
  	else if (ops->free_pages)
  		ops->free_pages(dev, size, page, dma_handle, dir);
  }

@@ -697,6 +752,8 @@ struct sg_table *dma_alloc_noncontiguous(struct device *dev, size_t size,
 
  	if (ops && ops->alloc_noncontiguous)
  		sgt = ops->alloc_noncontiguous(dev, size, dir, gfp, attrs);
+	else if (use_dma_iommu(dev))
+		sgt = iommu_dma_alloc_noncontiguous(dev, size, dir, gfp, attrs);
  	else
  		sgt = alloc_single_sgt(dev, size, dir, gfp);
 

@@ -725,6 +782,8 @@ void dma_free_noncontiguous(struct device *dev, size_t size,
  	debug_dma_unmap_sg(dev, sgt->sgl, sgt->orig_nents, dir);
  	if (ops && ops->free_noncontiguous)
  		ops->free_noncontiguous(dev, size, sgt, dir);
+	else if (use_dma_iommu(dev))
+		iommu_dma_free_noncontiguous(dev, size, sgt, dir);
  	else
  		free_single_sgt(dev, size, sgt, dir);
  }

@@ -772,6 +831,8 @@ static int dma_supported(struct device *dev, u64 mask)
  {
  	const struct dma_map_ops *ops = get_dma_ops(dev);
 
+	if (WARN_ON(ops && use_dma_iommu(dev)))
+		return false;
  	/*
  	 * ->dma_supported sets the bypass flag, so we must always call
  	 * into the method here unless the device is truly direct mapped.

@@ -787,17 +848,14 @@ bool dma_pci_p2pdma_supported(struct device *dev)
  {
  	const struct dma_map_ops *ops = get_dma_ops(dev);
 
-	/* if ops is not set, dma direct will be used which supports P2PDMA */
-	if (!ops)
-		return true;
-
  	/*
  	 * Note: dma_ops_bypass is not checked here because P2PDMA should
  	 * not be used with dma mapping ops that do not have support even
  	 * if the specific device is bypassing them.
  	 */
 
-	return ops->flags & DMA_F_PCI_P2PDMA_SUPPORTED;
+	/* if ops is not set, dma direct and default IOMMU support P2PDMA */
+	return !ops;
  }
  EXPORT_SYMBOL_GPL(dma_pci_p2pdma_supported);
 

@@ -865,6 +923,8 @@ size_t dma_max_mapping_size(struct device *dev)
 
  	if (dma_map_direct(dev, ops))
  		size = dma_direct_max_mapping_size(dev);
+	else if (use_dma_iommu(dev))
+		size = iommu_dma_max_mapping_size(dev);
  	else if (ops && ops->max_mapping_size)
  		size = ops->max_mapping_size(dev);
 

@@ -877,9 +937,10 @@ size_t dma_opt_mapping_size(struct device *dev)
  	const struct dma_map_ops *ops = get_dma_ops(dev);
  	size_t size = SIZE_MAX;
 
-	if (ops && ops->opt_mapping_size)
+	if (use_dma_iommu(dev))
+		size = iommu_dma_opt_mapping_size();
+	else if (ops && ops->opt_mapping_size)
  		size = ops->opt_mapping_size();
-
  	return min(dma_max_mapping_size(dev), size);
  }
  EXPORT_SYMBOL_GPL(dma_opt_mapping_size);

@@ -888,7 +949,12 @@ unsigned long dma_get_merge_boundary(struct device *dev)
  {

kernel/dma/mapping.c unsigned long dma_get_merge_boundary(struct device *dev) · line 958

Quoted 10 lines

kernel/dma/mapping.c +6 -1

  	const struct dma_map_ops *ops = get_dma_ops(dev);
 
-	if (!ops || !ops->get_merge_boundary)
+	if (use_dma_iommu(dev))
+		return iommu_dma_get_merge_boundary(dev);
+
+	if (!ops)
+		return 0;	/* can't merge */
+	if (!ops->get_merge_boundary)
  		return 0;	/* can't merge */

Nit: I'd keep the single || condition - those two cases are never going to be different.

Thanks,
Robin.

Quoted 2 lines

+#ifdef CONFIG_DMA_OPS_HELPERS
+#include <asm/dma-mapping.h>

What needs a definition of get_arch_dma_ops() in this scope?

include/linux/dma-map-ops.h static inline void set_dma_ops(struct device *dev, · line 110

Quoted 4 lines

include/linux/dma-map-ops.h +4 -0

+struct page *dma_common_alloc_pages(struct device *dev, size_t size,
+		dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp);
+void dma_common_free_pages(struct device *dev, size_t size, struct page *vaddr,
+		dma_addr_t dma_handle, enum dma_data_direction dir);

Why not bring the rest of the dma_common_* declarations in here as well? (Or more literally, add the #ifdef around where they all are already)

Quoted 5 lines

depends on !NO_DMA
default y
+# DMA IOMMU uses common ops helpers for certain operations, so let's
allow to build
+# ops_helpers.c even if DMA_OPS is not enabled

Hmm, but actually dma-direct also uses dma_common_contiguous_remap(), so something seems a little inconsistent here...

Quoted 4 lines

Quoted 3 lines

+# DMA IOMMU uses common ops helpers for certain operations, so let's
allow to build
+# ops_helpers.c even if DMA_OPS is not enabled

Quoted 2 lines

Hmm, but actually dma-direct also uses dma_common_contiguous_remap(), so something seems a little inconsistent here...

Yes, but that's not really new. I'll look into a patch to select the helpers based on the conditions that make dma-direct use it.

I'll fix up all style issues and will apply the patch with that over the weekend so that we can get it into this merge window.

Quoted 5 lines · Robin Murphy

Quoted 2 lines

+#ifdef CONFIG_DMA_OPS_HELPERS
+#include <asm/dma-mapping.h>

What needs a definition of get_arch_dma_ops() in this scope?

Quoted 7 lines

include/linux/dma-map-ops.h static inline void set_dma_ops(struct device *dev, · line 110

Quoted 4 lines

include/linux/dma-map-ops.h +4 -0

+struct page *dma_common_alloc_pages(struct device *dev, size_t size,
+		dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp);
+void dma_common_free_pages(struct device *dev, size_t size, struct page *vaddr,
+		dma_addr_t dma_handle, enum dma_data_direction dir);

Why not bring the rest of the dma_common_* declarations in here as well? (Or more literally, add the #ifdef around where they all are already)

We actually don't need the ifdef at all, the calls in mapping.c are all keyed off compile time constants, so just leaving the stray prorotypes for this code that won't ever be called around should be just fine.

Quoted 6 lines

Quoted 5 lines

depends on !NO_DMA
default y
+# DMA IOMMU uses common ops helpers for certain operations, so let's
allow to build
+# ops_helpers.c even if DMA_OPS is not enabled

Quoted 2 lines

Hmm, but actually dma-direct also uses dma_common_contiguous_remap(), so something seems a little inconsistent here...

Yes, but that's not really new. I'll look into a patch to select the helpers based on the conditions that make dma-direct use it.

I'll fix up all style issues and will apply the patch with that over the weekend so that we can get it into this merge window.

Thanks, I've just had a quick look over what you queued on dma-iommu-direct-calls, and you're welcome to stick my ack on that if you like.

Quoted 3 lines · Robin Murphy

Thanks, I've just had a quick look over what you queued on dma-iommu-direct-calls, and you're welcome to stick my ack on that if you like.

Yes, thank you a lot for your review!

While I have your attention - with these two patches we stop building dummy_dma_ops for most common configs. Do you think we need additional safeguards for this case? My idea would be to remove them and force the bus_dma_mask to zero where we currently set the dummy ops, but I could use a little reality check for that idea.

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, but for all modern platforms dma-iommu implementation is really matters.

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.

Signed-off-by Leon Romanovsky <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---

kernel/dma/mapping.c +0 -0

index 6832fd6f0796..02451e27e0b1 100644

Quoted 9 lines · Leon Romanovsky

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, but for all modern platforms dma-iommu implementation is really matters.

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

Quoted 6 lines

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

Signed-off-by Leon Romanovsky <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---

Hi,

KernelCI has identified a boot regression originating from this patch. I've verified that reverting the patch fixes the issue.

Affected platforms:
* sc7180-trogdor-kingoftown
* sc7180-trogdor-lazor-limozeen

Relevant kernel log:

[    5.790809] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000040
[    5.799844] Mem abort info:
[    5.799846]   ESR = 0x0000000096000006
[    5.808708]   EC = 0x25: DABT (current EL), IL = 32 bits
[    5.808712]   SET = 0, FnV = 0
[    5.808714]   EA = 0, S1PTW = 0
[    5.818465]   FSC = 0x06: level 2 translation fault
[    5.818468] Data abort info:
[    5.818469]   ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
[    5.827063]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[    5.827065]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[    5.838768] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000d20bb000
[    5.838771] [0000000000000040] pgd=08000000d20c1003
[    5.863071] , p4d=08000000d20c1003
[    5.898011] , pud=08000000d20c2003, pmd=0000000000000000
[    5.898014] Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
[    5.898016] Modules linked in: ipv6 hci_uart venus_core btqca v4l2_mem2mem btrtl qcom_spmi_adc5 sbs_battery btbcm qcom_vadc_common cros_ec_typec videobuf2_v4l2 leds_cros_ec cros_kbd_led_backlight cros_ec_chardev videodev elan_i2c videobuf2_common qcom_stats mc bluetooth coresight_stm stm_core ecdh_generic ecc pwrseq_core panel_edp icc_bwmon ath10k_snoc ath10k_core ath mac80211 phy_qcom_qmp_combo aux_bridge libarc4 coresight_replicator coresight_etm4x coresight_tmc coresight_funnel cfg80211 rfkill coresight qcom_wdt cbmem ramoops reed_solomon pwm_bl coreboot_table backlight crct10dif_ce
[    5.898057] CPU: 7 UID: 0 PID: 70 Comm: kworker/u32:4 Not tainted 6.11.0-rc6-next-20240903-00003-gdfc6015d0711 #660
[    5.898061] Hardware name: Google Lazor Limozeen without Touchscreen (rev5 - rev8) (DT)
[    5.898062] Workqueue: events_unbound deferred_probe_work_func
[    5.904227] hub 2-1:1.0: 4 ports detected
[    5.906827]
[    5.906828] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    5.906831] pc : dma_common_alloc_pages+0x54/0x1b4
[    5.906837] lr : dma_common_alloc_pages+0x4c/0x1b4
[    5.906839] sp : ffff8000807d3730
[    5.906840] x29: ffff8000807d3730 x28: ffff02a7d312f880 x27: 0000000000000001
[    5.906843] x26: 000000000000c000 x25: 0000000000000000 x24: 0000000000000001
[    5.906845] x23: ffff02a7d23b6898 x22: 0000000000006cc0 x21: 000000000000c000
[    5.906847] x20: ffff02a7858bf410 x19: fffffe0a60006000 x18: 0000000000000001
[    5.906850] x17: 00000000000000d5 x16: 1fffe054f0bcc261 x15: 0000000000000001
[    5.906852] x14: ffff02a7844dc680 x13: 0000000000100180 x12: dead000000000100
[    5.906855] x11: dead000000000122 x10: 00000000001001ff x9 : ffff02a87f7b7b00
[    5.906857] x8 : ffff02a87f7b7b00 x7 : ffff405977d6b000 x6 : ffff8000807d3310
[    5.906860] x5 : ffff02a87f6b6398 x4 : 0000000000000001 x3 : ffff405977d6b000
[    6.092491] x2 : ffff02a7844dc600 x1 : 0000000100000000 x0 : fffffe0a60006000
[    6.099809] Call trace:
[    6.102327]  dma_common_alloc_pages+0x54/0x1b4
[    6.106895]  __dma_alloc_pages+0x68/0x90
[    6.110921]  dma_alloc_pages+0x10/0x1c
[    6.114772]  snd_dma_noncoherent_alloc+0x28/0x8c
[    6.119514]  __snd_dma_alloc_pages+0x30/0x50
[    6.123897]  snd_dma_alloc_dir_pages+0x40/0x80
[    6.128465]  do_alloc_pages+0xb8/0x13c
[    6.132315]  preallocate_pcm_pages+0x6c/0xf8
[    6.132317]  preallocate_pages+0x160/0x1a4
[    6.132319]  snd_pcm_set_managed_buffer_all+0x64/0xb0
[    6.152964]  lpass_platform_pcm_new+0xc0/0xe8
[    6.157443]  snd_soc_pcm_component_new+0x3c/0xc8
[    6.162184]  soc_new_pcm+0x4fc/0x668
[    6.165853]  snd_soc_bind_card+0xabc/0xbac
[    6.170063]  snd_soc_register_card+0xf0/0x108
[    6.174533]  devm_snd_soc_register_card+0x4c/0xa4
[    6.179361]  sc7180_snd_platform_probe+0x180/0x224
[    6.184285]  platform_probe+0x68/0xc0
[    6.188050]  really_probe+0xbc/0x298
[    6.191717]  __driver_probe_device+0x78/0x12c
[    6.196186]  driver_probe_device+0x3c/0x15c
[    6.200481]  __device_attach_driver+0xb8/0x134
[    6.205047]  bus_for_each_drv+0x84/0xe0
[    6.208985]  __device_attach+0x9c/0x188
[    6.212924]  device_initial_probe+0x14/0x20
[    6.217219]  bus_probe_device+0xac/0xb0
[    6.221157]  deferred_probe_work_func+0x88/0xc0
[    6.225810]  process_one_work+0x14c/0x28c
[    6.229923]  worker_thread+0x2cc/0x3d4
[    6.233773]  kthread+0x114/0x118
[    6.237093]  ret_from_fork+0x10/0x20
[    6.240763] Code: f9411c19 940000c9 aa0003f3 b4000460 (f9402326)
[    6.247012] ---[ end trace 0000000000000000 ]---

See below for the suspicious hunk.

[..]

Quoted 4 lines

kernel/dma/mapping.c +0 -0

index 6832fd6f0796..02451e27e0b1 100644

[..]

kernel/dma/mapping.c static struct page *__dma_alloc_pages(struct device *dev, size_t size, · line 666

Quoted 6 lines

kernel/dma/mapping.c +2 -0

@@ -611,6 +662,8 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size,
 	size = PAGE_ALIGN(size);
 	if (dma_alloc_direct(dev, ops))
 		return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp);
+	if (use_dma_iommu(dev))
+		return dma_common_alloc_pages(dev, size, dma_handle, dir, gfp);

Is this check correct? dma_common_alloc_pages uses the dma_ops, but the comment in dma_iommu said it meant that dma_ops wouldn't be used.

And similarly for dma_common_free_pages below.

Quoted 12 lines

kernel/dma/mapping.c +2 -0

 	if (!ops->alloc_pages_op)
 		return NULL;
 	return ops->alloc_pages_op(dev, size, dma_handle, dir, gfp);

@@ -635,6 +688,8 @@ static void __dma_free_pages(struct device *dev, size_t size, struct page *page,
 	size = PAGE_ALIGN(size);
 	if (dma_alloc_direct(dev, ops))
 		dma_direct_free_pages(dev, size, page, dma_handle, dir);
+	else if (use_dma_iommu(dev))
+		dma_common_free_pages(dev, size, page, dma_handle, dir);
 	else if (ops->free_pages)
 		ops->free_pages(dev, size, page, dma_handle, dir);
 }

[..]

Please add

Reported-by Nícolas F. R. A. Prado <[email protected]> #KernelCI

when fixing this.

Happy to provide any other details necessary.

#regzbot introduced: next-20240822..20240823
#regzbot title: Null pointer dereference in dma_common_alloc_pages() for sc7180 platforms

Thanks,
Nícolas

Quoted 133 lines · Nícolas F. R. A. Prado

Quoted 9 lines · Leon Romanovsky

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, but for all modern platforms dma-iommu implementation is really matters.

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

Quoted 6 lines

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

Signed-off-by Leon Romanovsky <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---

Hi,

KernelCI has identified a boot regression originating from this patch. I've verified that reverting the patch fixes the issue.

Affected platforms:
* sc7180-trogdor-kingoftown
* sc7180-trogdor-lazor-limozeen

Relevant kernel log:

[    5.790809] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000040
[    5.799844] Mem abort info:
[    5.799846]   ESR = 0x0000000096000006
[    5.808708]   EC = 0x25: DABT (current EL), IL = 32 bits
[    5.808712]   SET = 0, FnV = 0
[    5.808714]   EA = 0, S1PTW = 0
[    5.818465]   FSC = 0x06: level 2 translation fault
[    5.818468] Data abort info:
[    5.818469]   ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
[    5.827063]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[    5.827065]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[    5.838768] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000d20bb000
[    5.838771] [0000000000000040] pgd=08000000d20c1003
[    5.863071] , p4d=08000000d20c1003
[    5.898011] , pud=08000000d20c2003, pmd=0000000000000000
[    5.898014] Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
[    5.898016] Modules linked in: ipv6 hci_uart venus_core btqca v4l2_mem2mem btrtl qcom_spmi_adc5 sbs_battery btbcm qcom_vadc_common cros_ec_typec videobuf2_v4l2 leds_cros_ec cros_kbd_led_backlight cros_ec_chardev videodev elan_i2c videobuf2_common qcom_stats mc bluetooth coresight_stm stm_core ecdh_generic ecc pwrseq_core panel_edp icc_bwmon ath10k_snoc ath10k_core ath mac80211 phy_qcom_qmp_combo aux_bridge libarc4 coresight_replicator coresight_etm4x coresight_tmc coresight_funnel cfg80211 rfkill coresight qcom_wdt cbmem ramoops reed_solomon pwm_bl coreboot_table backlight crct10dif_ce
[    5.898057] CPU: 7 UID: 0 PID: 70 Comm: kworker/u32:4 Not tainted 6.11.0-rc6-next-20240903-00003-gdfc6015d0711 #660
[    5.898061] Hardware name: Google Lazor Limozeen without Touchscreen (rev5 - rev8) (DT)
[    5.898062] Workqueue: events_unbound deferred_probe_work_func
[    5.904227] hub 2-1:1.0: 4 ports detected
[    5.906827]
[    5.906828] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    5.906831] pc : dma_common_alloc_pages+0x54/0x1b4
[    5.906837] lr : dma_common_alloc_pages+0x4c/0x1b4
[    5.906839] sp : ffff8000807d3730
[    5.906840] x29: ffff8000807d3730 x28: ffff02a7d312f880 x27: 0000000000000001
[    5.906843] x26: 000000000000c000 x25: 0000000000000000 x24: 0000000000000001
[    5.906845] x23: ffff02a7d23b6898 x22: 0000000000006cc0 x21: 000000000000c000
[    5.906847] x20: ffff02a7858bf410 x19: fffffe0a60006000 x18: 0000000000000001
[    5.906850] x17: 00000000000000d5 x16: 1fffe054f0bcc261 x15: 0000000000000001
[    5.906852] x14: ffff02a7844dc680 x13: 0000000000100180 x12: dead000000000100
[    5.906855] x11: dead000000000122 x10: 00000000001001ff x9 : ffff02a87f7b7b00
[    5.906857] x8 : ffff02a87f7b7b00 x7 : ffff405977d6b000 x6 : ffff8000807d3310
[    5.906860] x5 : ffff02a87f6b6398 x4 : 0000000000000001 x3 : ffff405977d6b000
[    6.092491] x2 : ffff02a7844dc600 x1 : 0000000100000000 x0 : fffffe0a60006000
[    6.099809] Call trace:
[    6.102327]  dma_common_alloc_pages+0x54/0x1b4
[    6.106895]  __dma_alloc_pages+0x68/0x90
[    6.110921]  dma_alloc_pages+0x10/0x1c
[    6.114772]  snd_dma_noncoherent_alloc+0x28/0x8c
[    6.119514]  __snd_dma_alloc_pages+0x30/0x50
[    6.123897]  snd_dma_alloc_dir_pages+0x40/0x80
[    6.128465]  do_alloc_pages+0xb8/0x13c
[    6.132315]  preallocate_pcm_pages+0x6c/0xf8
[    6.132317]  preallocate_pages+0x160/0x1a4
[    6.132319]  snd_pcm_set_managed_buffer_all+0x64/0xb0
[    6.152964]  lpass_platform_pcm_new+0xc0/0xe8
[    6.157443]  snd_soc_pcm_component_new+0x3c/0xc8
[    6.162184]  soc_new_pcm+0x4fc/0x668
[    6.165853]  snd_soc_bind_card+0xabc/0xbac
[    6.170063]  snd_soc_register_card+0xf0/0x108
[    6.174533]  devm_snd_soc_register_card+0x4c/0xa4
[    6.179361]  sc7180_snd_platform_probe+0x180/0x224
[    6.184285]  platform_probe+0x68/0xc0
[    6.188050]  really_probe+0xbc/0x298
[    6.191717]  __driver_probe_device+0x78/0x12c
[    6.196186]  driver_probe_device+0x3c/0x15c
[    6.200481]  __device_attach_driver+0xb8/0x134
[    6.205047]  bus_for_each_drv+0x84/0xe0
[    6.208985]  __device_attach+0x9c/0x188
[    6.212924]  device_initial_probe+0x14/0x20
[    6.217219]  bus_probe_device+0xac/0xb0
[    6.221157]  deferred_probe_work_func+0x88/0xc0
[    6.225810]  process_one_work+0x14c/0x28c
[    6.229923]  worker_thread+0x2cc/0x3d4
[    6.233773]  kthread+0x114/0x118
[    6.237093]  ret_from_fork+0x10/0x20
[    6.240763] Code: f9411c19 940000c9 aa0003f3 b4000460 (f9402326)
[    6.247012] ---[ end trace 0000000000000000 ]---

See below for the suspicious hunk.

[..]

Quoted 4 lines

kernel/dma/mapping.c +0 -0

index 6832fd6f0796..02451e27e0b1 100644

[..]

kernel/dma/mapping.c static struct page *__dma_alloc_pages(struct device *dev, size_t size, · line 666

Quoted 6 lines

kernel/dma/mapping.c +2 -0

@@ -611,6 +662,8 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size,
 	size = PAGE_ALIGN(size);
 	if (dma_alloc_direct(dev, ops))
 		return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp);
+	if (use_dma_iommu(dev))
+		return dma_common_alloc_pages(dev, size, dma_handle, dir, gfp);

Is this check correct? dma_common_alloc_pages uses the dma_ops, but the comment in dma_iommu said it meant that dma_ops wouldn't be used.

And similarly for dma_common_free_pages below.

Quoted 12 lines

kernel/dma/mapping.c +2 -0

 	if (!ops->alloc_pages_op)
 		return NULL;
 	return ops->alloc_pages_op(dev, size, dma_handle, dir, gfp);

@@ -635,6 +688,8 @@ static void __dma_free_pages(struct device *dev, size_t size, struct page *page,
 	size = PAGE_ALIGN(size);
 	if (dma_alloc_direct(dev, ops))
 		dma_direct_free_pages(dev, size, page, dma_handle, dir);
+	else if (use_dma_iommu(dev))
+		dma_common_free_pages(dev, size, page, dma_handle, dir);
 	else if (ops->free_pages)
 		ops->free_pages(dev, size, page, dma_handle, dir);
 }

[..]

Quoted 5 lines

Please add

Reported-by Nícolas F. R. A. Prado <[email protected]> #KernelCI

when fixing this.

Happy to provide any other details necessary.

Thanks for the report, can you try the following patch? I'll prepare patch later today.

kernel/dma/ops_helpers.c +7 -2

index af4a6ef48ce0..7e2b36cba61e 100644
@@ -4,6 +4,7 @@
  * the allocated memory contains normal pages in the direct kernel mapping.
  */
 #include <linux/dma-map-ops.h>
+#include <linux/iommu-dma.h>
 
 static struct page *dma_common_vaddr_to_page(void *cpu_addr)
 {

@@ -70,8 +71,12 @@ struct page *dma_common_alloc_pages(struct device *dev, size_t size,
 	if (!page)
 		return NULL;
 
-	*dma_handle = ops->map_page(dev, page, 0, size, dir,
-				    DMA_ATTR_SKIP_CPU_SYNC);
+	if (dev->dma_iommu)
+		*dma_handle = iommu_dma_map_page(dev, page, 0, size, dir,
+						 DMA_ATTR_SKIP_CPU_SYNC);
+	else
+		*dma_handle = ops->map_page(dev, page, 0, size, dir,
+					    DMA_ATTR_SKIP_CPU_SYNC);
 	if (*dma_handle == DMA_MAPPING_ERROR) {
 		dma_free_contiguous(dev, page, size);
 		return NULL;

@@ -86,7 +91,10 @@ void dma_common_free_pages(struct device *dev, size_t size, struct page *page,
 {

Quoted 172 lines · Leon Romanovsky

Quoted 133 lines · Nícolas F. R. A. Prado

Quoted 9 lines · Leon Romanovsky

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, but for all modern platforms dma-iommu implementation is really matters.

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

Quoted 6 lines

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

Signed-off-by Leon Romanovsky <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

---

Hi,

KernelCI has identified a boot regression originating from this patch. I've verified that reverting the patch fixes the issue.

Affected platforms:
* sc7180-trogdor-kingoftown
* sc7180-trogdor-lazor-limozeen

Relevant kernel log:

[    5.790809] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000040
[    5.799844] Mem abort info:
[    5.799846]   ESR = 0x0000000096000006
[    5.808708]   EC = 0x25: DABT (current EL), IL = 32 bits
[    5.808712]   SET = 0, FnV = 0
[    5.808714]   EA = 0, S1PTW = 0
[    5.818465]   FSC = 0x06: level 2 translation fault
[    5.818468] Data abort info:
[    5.818469]   ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
[    5.827063]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[    5.827065]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[    5.838768] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000d20bb000
[    5.838771] [0000000000000040] pgd=08000000d20c1003
[    5.863071] , p4d=08000000d20c1003
[    5.898011] , pud=08000000d20c2003, pmd=0000000000000000
[    5.898014] Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
[    5.898016] Modules linked in: ipv6 hci_uart venus_core btqca v4l2_mem2mem btrtl qcom_spmi_adc5 sbs_battery btbcm qcom_vadc_common cros_ec_typec videobuf2_v4l2 leds_cros_ec cros_kbd_led_backlight cros_ec_chardev videodev elan_i2c videobuf2_common qcom_stats mc bluetooth coresight_stm stm_core ecdh_generic ecc pwrseq_core panel_edp icc_bwmon ath10k_snoc ath10k_core ath mac80211 phy_qcom_qmp_combo aux_bridge libarc4 coresight_replicator coresight_etm4x coresight_tmc coresight_funnel cfg80211 rfkill coresight qcom_wdt cbmem ramoops reed_solomon pwm_bl coreboot_table backlight crct10dif_ce
[    5.898057] CPU: 7 UID: 0 PID: 70 Comm: kworker/u32:4 Not tainted 6.11.0-rc6-next-20240903-00003-gdfc6015d0711 #660
[    5.898061] Hardware name: Google Lazor Limozeen without Touchscreen (rev5 - rev8) (DT)
[    5.898062] Workqueue: events_unbound deferred_probe_work_func
[    5.904227] hub 2-1:1.0: 4 ports detected
[    5.906827]
[    5.906828] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    5.906831] pc : dma_common_alloc_pages+0x54/0x1b4
[    5.906837] lr : dma_common_alloc_pages+0x4c/0x1b4
[    5.906839] sp : ffff8000807d3730
[    5.906840] x29: ffff8000807d3730 x28: ffff02a7d312f880 x27: 0000000000000001
[    5.906843] x26: 000000000000c000 x25: 0000000000000000 x24: 0000000000000001
[    5.906845] x23: ffff02a7d23b6898 x22: 0000000000006cc0 x21: 000000000000c000
[    5.906847] x20: ffff02a7858bf410 x19: fffffe0a60006000 x18: 0000000000000001
[    5.906850] x17: 00000000000000d5 x16: 1fffe054f0bcc261 x15: 0000000000000001
[    5.906852] x14: ffff02a7844dc680 x13: 0000000000100180 x12: dead000000000100
[    5.906855] x11: dead000000000122 x10: 00000000001001ff x9 : ffff02a87f7b7b00
[    5.906857] x8 : ffff02a87f7b7b00 x7 : ffff405977d6b000 x6 : ffff8000807d3310
[    5.906860] x5 : ffff02a87f6b6398 x4 : 0000000000000001 x3 : ffff405977d6b000
[    6.092491] x2 : ffff02a7844dc600 x1 : 0000000100000000 x0 : fffffe0a60006000
[    6.099809] Call trace:
[    6.102327]  dma_common_alloc_pages+0x54/0x1b4
[    6.106895]  __dma_alloc_pages+0x68/0x90
[    6.110921]  dma_alloc_pages+0x10/0x1c
[    6.114772]  snd_dma_noncoherent_alloc+0x28/0x8c
[    6.119514]  __snd_dma_alloc_pages+0x30/0x50
[    6.123897]  snd_dma_alloc_dir_pages+0x40/0x80
[    6.128465]  do_alloc_pages+0xb8/0x13c
[    6.132315]  preallocate_pcm_pages+0x6c/0xf8
[    6.132317]  preallocate_pages+0x160/0x1a4
[    6.132319]  snd_pcm_set_managed_buffer_all+0x64/0xb0
[    6.152964]  lpass_platform_pcm_new+0xc0/0xe8
[    6.157443]  snd_soc_pcm_component_new+0x3c/0xc8
[    6.162184]  soc_new_pcm+0x4fc/0x668
[    6.165853]  snd_soc_bind_card+0xabc/0xbac
[    6.170063]  snd_soc_register_card+0xf0/0x108
[    6.174533]  devm_snd_soc_register_card+0x4c/0xa4
[    6.179361]  sc7180_snd_platform_probe+0x180/0x224
[    6.184285]  platform_probe+0x68/0xc0
[    6.188050]  really_probe+0xbc/0x298
[    6.191717]  __driver_probe_device+0x78/0x12c
[    6.196186]  driver_probe_device+0x3c/0x15c
[    6.200481]  __device_attach_driver+0xb8/0x134
[    6.205047]  bus_for_each_drv+0x84/0xe0
[    6.208985]  __device_attach+0x9c/0x188
[    6.212924]  device_initial_probe+0x14/0x20
[    6.217219]  bus_probe_device+0xac/0xb0
[    6.221157]  deferred_probe_work_func+0x88/0xc0
[    6.225810]  process_one_work+0x14c/0x28c
[    6.229923]  worker_thread+0x2cc/0x3d4
[    6.233773]  kthread+0x114/0x118
[    6.237093]  ret_from_fork+0x10/0x20
[    6.240763] Code: f9411c19 940000c9 aa0003f3 b4000460 (f9402326)
[    6.247012] ---[ end trace 0000000000000000 ]---

See below for the suspicious hunk.

[..]

Quoted 4 lines

kernel/dma/mapping.c +0 -0

index 6832fd6f0796..02451e27e0b1 100644

[..]

kernel/dma/mapping.c static struct page *__dma_alloc_pages(struct device *dev, size_t size, · line 666

Quoted 6 lines

kernel/dma/mapping.c +2 -0

@@ -611,6 +662,8 @@ static struct page *__dma_alloc_pages(struct device *dev, size_t size,
 	size = PAGE_ALIGN(size);
 	if (dma_alloc_direct(dev, ops))
 		return dma_direct_alloc_pages(dev, size, dma_handle, dir, gfp);
+	if (use_dma_iommu(dev))
+		return dma_common_alloc_pages(dev, size, dma_handle, dir, gfp);

Is this check correct? dma_common_alloc_pages uses the dma_ops, but the comment in dma_iommu said it meant that dma_ops wouldn't be used.

And similarly for dma_common_free_pages below.

Quoted 12 lines

kernel/dma/mapping.c +2 -0

 	if (!ops->alloc_pages_op)
 		return NULL;
 	return ops->alloc_pages_op(dev, size, dma_handle, dir, gfp);

@@ -635,6 +688,8 @@ static void __dma_free_pages(struct device *dev, size_t size, struct page *page,
 	size = PAGE_ALIGN(size);
 	if (dma_alloc_direct(dev, ops))
 		dma_direct_free_pages(dev, size, page, dma_handle, dir);
+	else if (use_dma_iommu(dev))
+		dma_common_free_pages(dev, size, page, dma_handle, dir);
 	else if (ops->free_pages)
 		ops->free_pages(dev, size, page, dma_handle, dir);
 }

[..]

Quoted 5 lines

Please add

Reported-by Nícolas F. R. A. Prado <[email protected]> #KernelCI

when fixing this.

Happy to provide any other details necessary.

Thanks for the report, can you try the following patch? I'll prepare patch later today.

kernel/dma/ops_helpers.c +7 -2

index af4a6ef48ce0..7e2b36cba61e 100644
@@ -4,6 +4,7 @@
  * the allocated memory contains normal pages in the direct kernel mapping.
  */
 #include <linux/dma-map-ops.h>
+#include <linux/iommu-dma.h>
 
 static struct page *dma_common_vaddr_to_page(void *cpu_addr)
 {

@@ -70,8 +71,12 @@ struct page *dma_common_alloc_pages(struct device *dev, size_t size,
 	if (!page)
 		return NULL;
 
-	*dma_handle = ops->map_page(dev, page, 0, size, dir,
-				    DMA_ATTR_SKIP_CPU_SYNC);
+	if (dev->dma_iommu)
+		*dma_handle = iommu_dma_map_page(dev, page, 0, size, dir,
+						 DMA_ATTR_SKIP_CPU_SYNC);
+	else
+		*dma_handle = ops->map_page(dev, page, 0, size, dir,
+					    DMA_ATTR_SKIP_CPU_SYNC);
 	if (*dma_handle == DMA_MAPPING_ERROR) {
 		dma_free_contiguous(dev, page, size);
 		return NULL;

@@ -86,7 +91,10 @@ void dma_common_free_pages(struct device *dev, size_t size, struct page *page,
 {

kernel/dma/ops_helpers.c void dma_common_free_pages(struct device *dev, size_t size, struct page *page, · line 100

Quoted 10 lines

kernel/dma/ops_helpers.c +4 -1

 	const struct dma_map_ops *ops = get_dma_ops(dev);
 
-	if (ops->unmap_page)
+	if (dev->dma_iommu)
+		iommu_dma_unmap_page(dev, dma_handle, size, dir,
+				     DMA_ATTR_SKIP_CPU_SYNC);
+	else if (ops->unmap_page)
 		ops->unmap_page(dev, dma_handle, size, dir,
 				DMA_ATTR_SKIP_CPU_SYNC);
 	dma_free_contiguous(dev, page, size);

Hi,

Indeed this patch fixes the issue.

Tested-by Nícolas F. R. A. Prado <[email protected]>

Thanks,
Nícolas

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, but for all modern platforms dma-iommu implementation is really matters.

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.

Signed-off-by Leon Romanovsky <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

Quoted 6 lines · Leon Romanovsky

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, but for all modern platforms dma-iommu implementation is really matters.

Quoted 8 lines

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.

Signed-off-by Leon Romanovsky <[email protected]>

Signed-off-by Leon Romanovsky <[email protected]>

Hi,

KernelCI has identified another regression originating from this patch. It affects the same platforms:
* sc7180-trogdor-kingoftown
* sc7180-trogdor-lazor-limozeen

But this time the issue is that the venus video codecs are failing to probe as indicated by the DT kselftest:

not ok 184 /soc@0/video-codec@aa00000
ok 185 /soc@0/video-codec@aa00000/opp-table # SKIP not ok 186 /soc@0/video-codec@aa00000/video-decoder not ok 187 /soc@0/video-codec@aa00000/video-encoder

The kernel logs show the error:

qcom-venus aa00000.video-codec: probe with driver qcom-venus failed with error -5

A quick ftrace run showed that the error comes from dma_set_mask_and_coherent() in venus_probe():

7) | venus_probe() {
...

  7)               |    dma_set_mask() {
  7)               |      dma_supported() {
  7)   0.989 us    |        dma_direct_supported(); /* = 0x0 */
  7)   2.864 us    |      } /* dma_supported = 0x0 */
  7)   4.636 us    |    } /* dma_set_mask = -5 */

For comparison, here is the ftrace run with the commit reverted:

7) | venus_probe() {
...

  7)   1.093 us    |    dma_set_mask(); /* = 0x0 */
  7)   1.041 us    |    dma_set_coherent_mask(); /* = 0x0 */

The issue is still present as of next-20240909 and reverting this commit fixes it.

Happy to provide any other details necessary.

Please add

Reported-by Nícolas F. R. A. Prado <[email protected]> #KernelCI

when fixing this.

#regzbot introduced: next-20240822..20240823
#regzbot title: Venus codec probe regression for sc7180 platforms in dma_set_mask_and_coherent()

Thanks,
Nícolas

Thanks for the report, I'm looking into it. However, it is unclear to me why my patch is causing this issue. The change in dma_supported() should produce WARN_ON [1] if new path is taken, otherwise, we return to previous behavior.

Quoted 4 lines · Leon Romanovsky

Thanks for the report, I'm looking into it. However, it is unclear to me why my patch is causing this issue. The change in dma_supported() should produce WARN_ON [1] if new path is taken, otherwise, we return to previous behavior.

dma-iommu never implemented .dma_supported and thus claims to support all dma masks. To restore that behavior we'd need something like the patch below:

kernel/dma/mapping.c +0 -0

index 7550b5dc5e55df..d23a4d5a6b31a1 100644

kernel/dma/mapping.c +0 -3

 	/*
 	 * ->dma_supported sets the bypass flag, so we must always call
 	 * into the method here unless the device is truly direct mapped.
 	 */
-	if (!ops)
-		return dma_direct_supported(dev, mask);
-	if (!ops->dma_supported)

kernel/dma/mapping.c +1 -0

+	return dma_direct_supported(dev, mask);
 }
 
 bool dma_pci_p2pdma_supported(struct device *dev)

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/Kconfig       |   2 +-
 drivers/iommu/dma-iommu.c   | 121 ++++++++++----------------
 drivers/iommu/intel/Kconfig |   1 -
 include/linux/device.h      |   5 ++

 include/linux/dma-map-ops.h |  39 +++++----
 include/linux/iommu-dma.h   | 169 ++++++++++++++++++++++++++++++++++++
 kernel/dma/Kconfig          |   6 ++
 kernel/dma/Makefile         |   2 +-
 kernel/dma/dummy.c          |  21 +++++
 kernel/dma/mapping.c        |  90 ++++++++++++++++---
 11 files changed, 348 insertions(+), 109 deletions(-)
 create mode 100644 include/linux/iommu-dma.h

Signature · 1 line

2.45.2

On Wed, Jul 24, 2024 at 09:04:47PM +0300, Leon Romanovsky wrote:

<...>

Quoted 9 lines

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/Kconfig       |   2 +-
 drivers/iommu/dma-iommu.c   | 121 ++++++++++----------------
 drivers/iommu/intel/Kconfig |   1 -
 include/linux/device.h      |   5 ++

Greg,

Can you please ack the change in the include/linux/device.h file?

I thought we were trying to get away from adding iommu stuff to struct device, but oh well :(