linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* clearly mark DMA_OPS support as an architecture feasture
@ 2024-08-24  3:57 Christoph Hellwig
  2024-08-24  3:57 ` [PATCH] dma-mapping: clear mark DMA ops as an architecture feature Christoph Hellwig
  2024-08-26  6:16 ` clearly mark DMA_OPS support as an architecture feasture Jason Wang
  0 siblings, 2 replies; 9+ messages in thread
From: Christoph Hellwig @ 2024-08-24  3:57 UTC (permalink / raw)
  To: iommu
  Cc: Marek Szyprowski, Robin Murphy, Sakari Ailus, Bingbu Cao,
	Michael S . Tsirkin , Jason Wang, linux-kernel, linux-alpha,
	linux-arm-kernel, linux-mips, linux-parisc, linuxppc-dev,
	linux-s390, sparclinux, linux-media, virtualization, xen-devel

Hi all,

we've had a long standing problems where drivers try to hook into the
DMA_OPS mechanisms to override them for something that is not DMA, or
to introduce additional dispatching.

Now that we are not using DMA_OPS support for dma-iommu and can build
kernels without DMA_OPS support on many common setups this becomes even
more problematic.

This series renames the option to ARCH_DMA_OPS and adds very explicit
comment to not use it in drivers.  The ipu6 and vdpa_sim/user drivers
that abuse the mechanism are made to depend on the option instead of
selecting it with a big comment, but I expect this to be fixed rather
sooner than later (I know the ipu6 maintainers are on it based on a
previous discussion).


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] dma-mapping: clear mark DMA ops as an architecture feature
  2024-08-24  3:57 clearly mark DMA_OPS support as an architecture feasture Christoph Hellwig
@ 2024-08-24  3:57 ` Christoph Hellwig
  2024-08-24  7:58   ` Sakari Ailus
                     ` (2 more replies)
  2024-08-26  6:16 ` clearly mark DMA_OPS support as an architecture feasture Jason Wang
  1 sibling, 3 replies; 9+ messages in thread
From: Christoph Hellwig @ 2024-08-24  3:57 UTC (permalink / raw)
  To: iommu
  Cc: Marek Szyprowski, Robin Murphy, Sakari Ailus, Bingbu Cao,
	Michael S . Tsirkin , Jason Wang, linux-kernel, linux-alpha,
	linux-arm-kernel, linux-mips, linux-parisc, linuxppc-dev,
	linux-s390, sparclinux, linux-media, virtualization, xen-devel

DMA ops are a helper for architectures and not for drivers to override
the DMA implementation.  Unfortunately driver authors keep ignoring
this.  Make this more clear by renaming the symbol to ARCH_DMA_OPS,
have the three drivers overriding it depend on that.  They should
probably also be marked broken, but we can give them a bit of a grace
period for that.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/Kconfig                         |  9 +++++++++
 arch/alpha/Kconfig                   |  2 +-
 arch/arm/Kconfig                     |  2 +-
 arch/arm64/Kconfig                   |  1 +
 arch/mips/Kconfig                    |  2 +-
 arch/parisc/Kconfig                  |  2 +-
 arch/powerpc/Kconfig                 |  2 +-
 arch/s390/Kconfig                    |  2 +-
 arch/sparc/Kconfig                   |  2 +-
 arch/x86/Kconfig                     |  2 +-
 drivers/macintosh/macio_asic.c       |  4 ++--
 drivers/media/pci/intel/ipu6/Kconfig |  7 ++++++-
 drivers/vdpa/Kconfig                 | 14 ++++++++++++--
 drivers/xen/Kconfig                  |  4 ++--
 include/linux/device.h               |  2 +-
 include/linux/dma-map-ops.h          |  6 +++---
 kernel/dma/Kconfig                   |  9 ++-------
 kernel/dma/Makefile                  |  2 +-
 18 files changed, 47 insertions(+), 27 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 975dd22a2dbd22..6abd0f1c1d833e 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -17,6 +17,15 @@ config CPU_MITIGATIONS
 	def_bool y
 endif
 
+#
+# Selected by architectures that need custom DMA operations for e.g. legacy
+# IOMMUs not handled by dma-iommu.  Drivers must never select this symbol.
+#
+config ARCH_DMA_OPS
+	depends on HAS_DMA
+	select DMA_OPS_HELPERS
+	bool
+
 menu "General architecture-dependent options"
 
 config ARCH_HAS_SUBPAGE_FAULTS
diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
index 50ff06d5b799c9..c6d716d8bdd095 100644
--- a/arch/alpha/Kconfig
+++ b/arch/alpha/Kconfig
@@ -3,13 +3,13 @@ config ALPHA
 	bool
 	default y
 	select ARCH_32BIT_USTAT_F_TINODE
+	select ARCH_DMA_OPS if PCI
 	select ARCH_HAS_CURRENT_STACK_POINTER
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_MIGHT_HAVE_PC_SERIO
 	select ARCH_NO_PREEMPT
 	select ARCH_NO_SG_CHAIN
 	select ARCH_USE_CMPXCHG_LOCKREF
-	select DMA_OPS if PCI
 	select FORCE_PCI
 	select PCI_DOMAINS if PCI
 	select PCI_SYSCALL if PCI
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 54b2bb817a7fc0..a823fd14d65987 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -10,6 +10,7 @@ config ARM
 	select ARCH_HAS_CURRENT_STACK_POINTER
 	select ARCH_HAS_DEBUG_VIRTUAL if MMU
 	select ARCH_HAS_DMA_ALLOC if MMU
+	select ARCH_DMA_OPS
 	select ARCH_HAS_DMA_WRITE_COMBINE if !ARM_DMA_MEM_BUFFERABLE
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_FORTIFY_SOURCE
@@ -54,7 +55,6 @@ config ARM
 	select DCACHE_WORD_ACCESS if HAVE_EFFICIENT_UNALIGNED_ACCESS
 	select DMA_DECLARE_COHERENT
 	select DMA_GLOBAL_POOL if !MMU
-	select DMA_OPS
 	select DMA_NONCOHERENT_MMAP if MMU
 	select EDAC_SUPPORT
 	select EDAC_ATOMIC_SCRUB
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a2f8ff354ca670..ce1650ceb5b596 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -15,6 +15,7 @@ config ARM64
 	select ARCH_BINFMT_ELF_EXTRA_PHDRS
 	select ARCH_BINFMT_ELF_STATE
 	select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
+	select ARCH_DMA_OPS if XEN
 	select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
 	select ARCH_ENABLE_MEMORY_HOTPLUG
 	select ARCH_ENABLE_MEMORY_HOTREMOVE
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 60077e57693563..3b5a1aef1e9bc0 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -391,9 +391,9 @@ config MACH_JAZZ
 	bool "Jazz family of machines"
 	select ARC_MEMORY
 	select ARC_PROMLIB
+	select ARCH_DMA_OPS
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_MIGHT_HAVE_PC_SERIO
-	select DMA_OPS
 	select FW_ARC
 	select FW_ARC32
 	select ARCH_MAY_HAVE_PC_FDC
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index b0a2ac3ba91610..c77f9de3e8cc1b 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -3,6 +3,7 @@ config PARISC
 	def_bool y
 	select ALTERNATE_USER_ADDRESS_SPACE
 	select ARCH_32BIT_OFF_T if !64BIT
+	select ARCH_DMA_OPS
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select HAVE_FUNCTION_TRACER
 	select HAVE_FUNCTION_GRAPH_TRACER
@@ -23,7 +24,6 @@ config PARISC
 	select ARCH_HAS_CACHE_LINE_SIZE
 	select ARCH_HAS_DEBUG_VM_PGTABLE
 	select HAVE_RELIABLE_STACKTRACE
-	select DMA_OPS
 	select RTC_CLASS
 	select RTC_DRV_GENERIC
 	select INIT_ALL_POSSIBLE
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d7b09b064a8ac5..50c33aca1959a5 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -124,6 +124,7 @@ config PPC
 	select ARCH_32BIT_OFF_T if PPC32
 	select ARCH_DISABLE_KASAN_INLINE	if PPC_RADIX_MMU
 	select ARCH_DMA_DEFAULT_COHERENT	if !NOT_COHERENT_CACHE
+	select ARCH_DMA_OPS			if PPC64
 	select ARCH_ENABLE_MEMORY_HOTPLUG
 	select ARCH_ENABLE_MEMORY_HOTREMOVE
 	select ARCH_HAS_COPY_MC			if PPC64
@@ -185,7 +186,6 @@ config PPC
 	select CPUMASK_OFFSTACK			if NR_CPUS >= 8192
 	select DCACHE_WORD_ACCESS		if PPC64 && CPU_LITTLE_ENDIAN
 	select DMA_OPS_BYPASS			if PPC64
-	select DMA_OPS				if PPC64
 	select DYNAMIC_FTRACE			if FUNCTION_TRACER
 	select EDAC_ATOMIC_SCRUB
 	select EDAC_SUPPORT
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index a822f952f64a9c..1de639f1e50dc3 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -62,6 +62,7 @@ config S390
 	select ARCH_32BIT_USTAT_F_TINODE
 	select ARCH_BINFMT_ELF_STATE
 	select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
+	select ARCH_DMA_OPS if PCI
 	select ARCH_ENABLE_MEMORY_HOTPLUG if SPARSEMEM
 	select ARCH_ENABLE_MEMORY_HOTREMOVE
 	select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
@@ -137,7 +138,6 @@ config S390
 	select BUILDTIME_TABLE_SORT
 	select CLONE_BACKWARDS2
 	select DCACHE_WORD_ACCESS if !KMSAN
-	select DMA_OPS if PCI
 	select DYNAMIC_FTRACE if FUNCTION_TRACER
 	select FUNCTION_ALIGNMENT_8B if CC_IS_GCC
 	select FUNCTION_ALIGNMENT_16B if !CC_IS_GCC
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 11bf9d312318c6..ba9fd67c5c5327 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -13,10 +13,10 @@ config 64BIT
 config SPARC
 	bool
 	default y
+	select ARCH_DMA_OPS
 	select ARCH_HAS_CPU_CACHE_ALIASING
 	select ARCH_MIGHT_HAVE_PC_PARPORT if SPARC64 && PCI
 	select ARCH_MIGHT_HAVE_PC_SERIO
-	select DMA_OPS
 	select OF
 	select OF_PROMTREE
 	select HAVE_ASM_MODVERSIONS
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 007bab9f2a0e39..c105c822da0559 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -65,6 +65,7 @@ config X86
 	select ARCH_CLOCKSOURCE_INIT
 	select ARCH_CONFIGURES_CPU_MITIGATIONS
 	select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
+	select ARCH_DMA_OPS if GART_IOMMU || XEN
 	select ARCH_ENABLE_HUGEPAGE_MIGRATION if X86_64 && HUGETLB_PAGE && MIGRATION
 	select ARCH_ENABLE_MEMORY_HOTPLUG if X86_64
 	select ARCH_ENABLE_MEMORY_HOTREMOVE if MEMORY_HOTPLUG
@@ -943,7 +944,6 @@ config DMI
 
 config GART_IOMMU
 	bool "Old AMD GART IOMMU support"
-	select DMA_OPS
 	select IOMMU_HELPER
 	select SWIOTLB
 	depends on X86_64 && PCI && AMD_NB
diff --git a/drivers/macintosh/macio_asic.c b/drivers/macintosh/macio_asic.c
index 13626205530d36..e217e6e7b2a4d0 100644
--- a/drivers/macintosh/macio_asic.c
+++ b/drivers/macintosh/macio_asic.c
@@ -387,7 +387,7 @@ static struct macio_dev * macio_add_one_device(struct macio_chip *chip,
 	dma_set_max_seg_size(&dev->ofdev.dev, 65536);
 	dma_set_seg_boundary(&dev->ofdev.dev, 0xffffffff);
 
-#if defined(CONFIG_PCI) && defined(CONFIG_DMA_OPS)
+#if defined(CONFIG_PCI) && defined(CONFIG_ARCH_DMA_OPS)
 	/* Set the DMA ops to the ones from the PCI device, this could be
 	 * fishy if we didn't know that on PowerMac it's always direct ops
 	 * or iommu ops that will work fine
@@ -396,7 +396,7 @@ static struct macio_dev * macio_add_one_device(struct macio_chip *chip,
 	 */
 	dev->ofdev.dev.archdata = chip->lbus.pdev->dev.archdata;
 	dev->ofdev.dev.dma_ops = chip->lbus.pdev->dev.dma_ops;
-#endif /* CONFIG_PCI && CONFIG_DMA_OPS */
+#endif /* CONFIG_PCI && CONFIG_ARCH_DMA_OPS */
 
 #ifdef DEBUG
 	printk("preparing mdev @%p, ofdev @%p, dev @%p, kobj @%p\n",
diff --git a/drivers/media/pci/intel/ipu6/Kconfig b/drivers/media/pci/intel/ipu6/Kconfig
index 40e20f0aa5ae5d..0885d8f07a4d81 100644
--- a/drivers/media/pci/intel/ipu6/Kconfig
+++ b/drivers/media/pci/intel/ipu6/Kconfig
@@ -4,8 +4,13 @@ config VIDEO_INTEL_IPU6
 	depends on VIDEO_DEV
 	depends on X86 && X86_64 && HAS_DMA
 	depends on IPU_BRIDGE || !IPU_BRIDGE
+	#
+	# This driver incorrectly tries to override the dma_ops.  It should
+	# never have done that, but for now keep it working on architectures
+	# that use dma ops
+	#
+	depends on ARCH_DMA_OPS
 	select AUXILIARY_BUS
-	select DMA_OPS
 	select IOMMU_IOVA
 	select VIDEO_V4L2_SUBDEV_API
 	select MEDIA_CONTROLLER
diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
index 5265d09fc1c409..1ce9ca50c848e3 100644
--- a/drivers/vdpa/Kconfig
+++ b/drivers/vdpa/Kconfig
@@ -12,7 +12,12 @@ if VDPA
 config VDPA_SIM
 	tristate "vDPA device simulator core"
 	depends on RUNTIME_TESTING_MENU && HAS_DMA
-	select DMA_OPS
+	#
+	# This driver incorrectly tries to override the dma_ops.  It should
+	# never have done that, but for now keep it working on architectures
+	# that use dma ops
+	#
+	depends on ARCH_DMA_OPS
 	select VHOST_RING
 	select IOMMU_IOVA
 	help
@@ -36,7 +41,12 @@ config VDPA_SIM_BLOCK
 config VDPA_USER
 	tristate "VDUSE (vDPA Device in Userspace) support"
 	depends on EVENTFD && MMU && HAS_DMA
-	select DMA_OPS
+	#
+	# This driver incorrectly tries to override the dma_ops.  It should
+	# never have done that, but for now keep it working on architectures
+	# that use dma ops
+	#
+	depends on ARCH_DMA_OPS
 	select VHOST_IOTLB
 	select IOMMU_IOVA
 	help
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index d5989871dd5de3..a9c825974d3a31 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -178,7 +178,7 @@ config XEN_GRANT_DMA_ALLOC
 config SWIOTLB_XEN
 	def_bool y
 	depends on XEN_PV || ARM || ARM64
-	select DMA_OPS
+	depends on ARCH_DMA_OPS
 	select SWIOTLB
 
 config XEN_PCI_STUB
@@ -348,11 +348,11 @@ config XEN_GRANT_DMA_IOMMU
 
 config XEN_GRANT_DMA_OPS
 	bool
-	select DMA_OPS
 
 config XEN_VIRTIO
 	bool "Xen virtio support"
 	depends on VIRTIO
+	depends on ARCH_DMA_OPS
 	select XEN_GRANT_DMA_OPS
 	select XEN_GRANT_DMA_IOMMU if OF
 	help
diff --git a/include/linux/device.h b/include/linux/device.h
index 1c5280d28bc389..b25003d02c2b1f 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -750,7 +750,7 @@ struct device {
 	struct dev_pin_info	*pins;
 #endif
 	struct dev_msi_info	msi;
-#ifdef CONFIG_DMA_OPS
+#ifdef CONFIG_ARCH_DMA_OPS
 	const struct dma_map_ops *dma_ops;
 #endif
 	u64		*dma_mask;	/* dma mask (if dma'able device) */
diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index 077b15c93bb8ff..f4de2a4a355d44 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -75,7 +75,7 @@ struct dma_map_ops {
 	unsigned long (*get_merge_boundary)(struct device *dev);
 };
 
-#ifdef CONFIG_DMA_OPS
+#ifdef CONFIG_ARCH_DMA_OPS
 #include <asm/dma-mapping.h>
 
 static inline const struct dma_map_ops *get_dma_ops(struct device *dev)
@@ -90,7 +90,7 @@ static inline void set_dma_ops(struct device *dev,
 {
 	dev->dma_ops = dma_ops;
 }
-#else /* CONFIG_DMA_OPS */
+#else /* CONFIG_ARCH_DMA_OPS */
 static inline const struct dma_map_ops *get_dma_ops(struct device *dev)
 {
 	return NULL;
@@ -99,7 +99,7 @@ static inline void set_dma_ops(struct device *dev,
 			       const struct dma_map_ops *dma_ops)
 {
 }
-#endif /* CONFIG_DMA_OPS */
+#endif /* CONFIG_ARCH_DMA_OPS */
 
 #ifdef CONFIG_DMA_CMA
 extern struct cma *dma_contiguous_default_area;
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 21bae17008368a..6849675d8fcd08 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -11,11 +11,6 @@ config HAS_DMA
 config DMA_OPS_HELPERS
 	bool
 
-config DMA_OPS
-	depends on HAS_DMA
-	select DMA_OPS_HELPERS
-	bool
-
 #
 # IOMMU drivers that can bypass the IOMMU code and optionally use the direct
 # mapping fast path should select this option and set the dma_ops_bypass
@@ -113,8 +108,8 @@ config DMA_BOUNCE_UNALIGNED_KMALLOC
 
 config DMA_NEED_SYNC
 	def_bool ARCH_HAS_SYNC_DMA_FOR_DEVICE || ARCH_HAS_SYNC_DMA_FOR_CPU || \
-		 ARCH_HAS_SYNC_DMA_FOR_CPU_ALL || DMA_API_DEBUG || DMA_OPS || \
-		 SWIOTLB
+		 ARCH_HAS_SYNC_DMA_FOR_CPU_ALL || DMA_API_DEBUG || \
+		 ARCH_DMA_OPS || SWIOTLB
 
 config DMA_RESTRICTED_POOL
 	bool "DMA Restricted Pool"
diff --git a/kernel/dma/Makefile b/kernel/dma/Makefile
index 2e6e933cf7f3fb..f479b59002cb30 100644
--- a/kernel/dma/Makefile
+++ b/kernel/dma/Makefile
@@ -2,7 +2,7 @@
 
 obj-$(CONFIG_HAS_DMA)			+= mapping.o direct.o
 obj-$(CONFIG_DMA_OPS_HELPERS)		+= ops_helpers.o
-obj-$(CONFIG_DMA_OPS)			+= dummy.o
+obj-$(CONFIG_ARCH_DMA_OPS)		+= dummy.o
 obj-$(CONFIG_DMA_CMA)			+= contiguous.o
 obj-$(CONFIG_DMA_DECLARE_COHERENT)	+= coherent.o
 obj-$(CONFIG_DMA_API_DEBUG)		+= debug.o
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] dma-mapping: clear mark DMA ops as an architecture feature
  2024-08-24  3:57 ` [PATCH] dma-mapping: clear mark DMA ops as an architecture feature Christoph Hellwig
@ 2024-08-24  7:58   ` Sakari Ailus
  2024-08-25 11:33   ` Thomas Gleixner
  2024-08-27 12:31   ` Robin Murphy
  2 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2024-08-24  7:58 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: iommu, Marek Szyprowski, Robin Murphy, Bingbu Cao,
	Michael S . Tsirkin , Jason Wang, linux-kernel, linux-alpha,
	linux-arm-kernel, linux-mips, linux-parisc, linuxppc-dev,
	linux-s390, sparclinux, linux-media, virtualization, xen-devel

Hi Christoph,

On Sat, Aug 24, 2024 at 05:57:58AM +0200, Christoph Hellwig wrote:
> DMA ops are a helper for architectures and not for drivers to override
> the DMA implementation.  Unfortunately driver authors keep ignoring
> this.  Make this more clear by renaming the symbol to ARCH_DMA_OPS,
> have the three drivers overriding it depend on that.  They should
> probably also be marked broken, but we can give them a bit of a grace
> period for that.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> # for IPU6

We'll address this for IPU6 but I can't give a timeline for that right now.

-- 
Kind regards,

Sakari Ailus


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dma-mapping: clear mark DMA ops as an architecture feature
  2024-08-24  3:57 ` [PATCH] dma-mapping: clear mark DMA ops as an architecture feature Christoph Hellwig
  2024-08-24  7:58   ` Sakari Ailus
@ 2024-08-25 11:33   ` Thomas Gleixner
  2024-08-27 12:31   ` Robin Murphy
  2 siblings, 0 replies; 9+ messages in thread
From: Thomas Gleixner @ 2024-08-25 11:33 UTC (permalink / raw)
  To: Christoph Hellwig, iommu
  Cc: Marek Szyprowski, Robin Murphy, Sakari Ailus, Bingbu Cao,
	Michael S . Tsirkin, Jason Wang, linux-kernel, linux-alpha,
	linux-arm-kernel, linux-mips, linux-parisc, linuxppc-dev,
	linux-s390, sparclinux, linux-media, virtualization, xen-devel

On Sat, Aug 24 2024 at 05:57, Christoph Hellwig wrote:
> DMA ops are a helper for architectures and not for drivers to override
> the DMA implementation.  Unfortunately driver authors keep ignoring
> this.  Make this more clear by renaming the symbol to ARCH_DMA_OPS,
> have the three drivers overriding it depend on that.  They should
> probably also be marked broken, but we can give them a bit of a grace
> period for that.

One week :)

> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: clearly mark DMA_OPS support as an architecture feasture
  2024-08-24  3:57 clearly mark DMA_OPS support as an architecture feasture Christoph Hellwig
  2024-08-24  3:57 ` [PATCH] dma-mapping: clear mark DMA ops as an architecture feature Christoph Hellwig
@ 2024-08-26  6:16 ` Jason Wang
  2024-08-26  6:27   ` Jason Wang
  1 sibling, 1 reply; 9+ messages in thread
From: Jason Wang @ 2024-08-26  6:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: iommu, Marek Szyprowski, Robin Murphy, Sakari Ailus, Bingbu Cao,
	Michael S . Tsirkin, linux-kernel, linux-alpha, linux-arm-kernel,
	linux-mips, linux-parisc, linuxppc-dev, linux-s390, sparclinux,
	linux-media, virtualization, xen-devel

On Sat, Aug 24, 2024 at 11:58 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Hi all,
>
> we've had a long standing problems where drivers try to hook into the
> DMA_OPS mechanisms to override them for something that is not DMA, or
> to introduce additional dispatching.
>
> Now that we are not using DMA_OPS support for dma-iommu and can build
> kernels without DMA_OPS support on many common setups this becomes even
> more problematic.
>
> This series renames the option to ARCH_DMA_OPS and adds very explicit
> comment to not use it in drivers.  The ipu6 and vdpa_sim/user drivers
> that abuse the mechanism are made to depend on the option instead of
> selecting it with a big comment, but I expect this to be fixed rather
> sooner than later (I know the ipu6 maintainers are on it based on a
> previous discussion).
>

I will try to fix the simulator considering virtio has already had
mapping ops now.

Thanks



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: clearly mark DMA_OPS support as an architecture feasture
  2024-08-26  6:16 ` clearly mark DMA_OPS support as an architecture feasture Jason Wang
@ 2024-08-26  6:27   ` Jason Wang
  2024-08-26  6:30     ` Christoph Hellwig
  0 siblings, 1 reply; 9+ messages in thread
From: Jason Wang @ 2024-08-26  6:27 UTC (permalink / raw)
  To: Christoph Hellwig, Michael S . Tsirkin
  Cc: iommu, Marek Szyprowski, Robin Murphy, Sakari Ailus, Bingbu Cao,
	linux-kernel, linux-alpha, linux-arm-kernel, linux-mips,
	linux-parisc, linuxppc-dev, linux-s390, sparclinux, linux-media,
	virtualization, xen-devel

On Mon, Aug 26, 2024 at 2:16 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Sat, Aug 24, 2024 at 11:58 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > Hi all,
> >
> > we've had a long standing problems where drivers try to hook into the
> > DMA_OPS mechanisms to override them for something that is not DMA, or
> > to introduce additional dispatching.
> >
> > Now that we are not using DMA_OPS support for dma-iommu and can build
> > kernels without DMA_OPS support on many common setups this becomes even
> > more problematic.
> >
> > This series renames the option to ARCH_DMA_OPS and adds very explicit
> > comment to not use it in drivers.  The ipu6 and vdpa_sim/user drivers
> > that abuse the mechanism are made to depend on the option instead of
> > selecting it with a big comment, but I expect this to be fixed rather
> > sooner than later (I know the ipu6 maintainers are on it based on a
> > previous discussion).
> >
>
> I will try to fix the simulator considering virtio has already had
> mapping ops now.

Actually I meant, we can extend the virtio_config_ops to allow mapping
ops there, then simulator and VDUSE can hook the map ops there.

Not sure if Michael is fine with this.

Thanks

>
> Thanks



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: clearly mark DMA_OPS support as an architecture feasture
  2024-08-26  6:27   ` Jason Wang
@ 2024-08-26  6:30     ` Christoph Hellwig
  2024-08-26  6:32       ` Jason Wang
  0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2024-08-26  6:30 UTC (permalink / raw)
  To: Jason Wang
  Cc: Christoph Hellwig, Michael S . Tsirkin, iommu, Marek Szyprowski,
	Robin Murphy, Sakari Ailus, Bingbu Cao, linux-kernel, linux-alpha,
	linux-arm-kernel, linux-mips, linux-parisc, linuxppc-dev,
	linux-s390, sparclinux, linux-media, virtualization, xen-devel

On Mon, Aug 26, 2024 at 02:27:27PM +0800, Jason Wang wrote:
> Actually I meant, we can extend the virtio_config_ops to allow mapping
> ops there, then simulator and VDUSE can hook the map ops there.

From a quick glance that feels like the right layer of abstraction,
although the config part of the name feels wrong at that point.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: clearly mark DMA_OPS support as an architecture feasture
  2024-08-26  6:30     ` Christoph Hellwig
@ 2024-08-26  6:32       ` Jason Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Jason Wang @ 2024-08-26  6:32 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Michael S . Tsirkin, iommu, Marek Szyprowski, Robin Murphy,
	Sakari Ailus, Bingbu Cao, linux-kernel, linux-alpha,
	linux-arm-kernel, linux-mips, linux-parisc, linuxppc-dev,
	linux-s390, sparclinux, linux-media, virtualization, xen-devel

On Mon, Aug 26, 2024 at 2:30 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Mon, Aug 26, 2024 at 02:27:27PM +0800, Jason Wang wrote:
> > Actually I meant, we can extend the virtio_config_ops to allow mapping
> > ops there, then simulator and VDUSE can hook the map ops there.
>
> From a quick glance that feels like the right layer of abstraction,
> although the config part of the name feels wrong at that point.

Right, or we could have a dedicated map_ops instead of trying to use
virtio_config_ops.

Thanks

>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dma-mapping: clear mark DMA ops as an architecture feature
  2024-08-24  3:57 ` [PATCH] dma-mapping: clear mark DMA ops as an architecture feature Christoph Hellwig
  2024-08-24  7:58   ` Sakari Ailus
  2024-08-25 11:33   ` Thomas Gleixner
@ 2024-08-27 12:31   ` Robin Murphy
  2 siblings, 0 replies; 9+ messages in thread
From: Robin Murphy @ 2024-08-27 12:31 UTC (permalink / raw)
  To: Christoph Hellwig, iommu
  Cc: Marek Szyprowski, Sakari Ailus, Bingbu Cao, Michael S . Tsirkin,
	Jason Wang, linux-kernel, linux-alpha, linux-arm-kernel,
	linux-mips, linux-parisc, linuxppc-dev, linux-s390, sparclinux,
	linux-media, virtualization, xen-devel

On 24/08/2024 4:57 am, Christoph Hellwig wrote:
> DMA ops are a helper for architectures and not for drivers to override
> the DMA implementation.  Unfortunately driver authors keep ignoring
> this.  Make this more clear by renaming the symbol to ARCH_DMA_OPS,
> have the three drivers overriding it depend on that.  They should
> probably also be marked broken, but we can give them a bit of a grace
> period for that.

Nit: from a quick survey of "git grep 'select ARCH_'", maybe 
ARCH_HAS_DMA_OPS might be the clearest and most consistent name? 
Otherwise, now that any potential confusion from drivers/iommu is no 
more, I too thoroughly approve of the overall idea.

Acked-by: Robin Murphy <robin.murphy@arm.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-08-27 13:53 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-24  3:57 clearly mark DMA_OPS support as an architecture feasture Christoph Hellwig
2024-08-24  3:57 ` [PATCH] dma-mapping: clear mark DMA ops as an architecture feature Christoph Hellwig
2024-08-24  7:58   ` Sakari Ailus
2024-08-25 11:33   ` Thomas Gleixner
2024-08-27 12:31   ` Robin Murphy
2024-08-26  6:16 ` clearly mark DMA_OPS support as an architecture feasture Jason Wang
2024-08-26  6:27   ` Jason Wang
2024-08-26  6:30     ` Christoph Hellwig
2024-08-26  6:32       ` Jason Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).