dma mask related fixups (including full bus_dma_mask support)

dma mask related fixups (including full bus_dma_mask support)

[Christoph Hellwig]

Hi all,

the dma_get_required_mask dma API implementation has always been a little
odd, in that we by default don't wire it up struct dma_map_ops, but
instead hard code a default implementation.  powerpc and ia64 override
this default and either call a method or otherwise duplicate the default.

This series always enabled the method and just falls back to the previous
default implementation when it is not available, as well as fixing up
a few bits in the default implementations.  This already allows removing
the ia64 override of the implementation, and will also allow to remove
the powerpc one together with a few additional cleanups in the powerpc
code, but those will be sent separately with other powerpc DMA API
patches.  Last but not least the method will allow us to return a more
sensible value for typical iommu dma_ops eventually, but that is left
to another series as well.

Additionally the dma-direct code has been a bit sloppy in when it was
using phys_to_dma in a few places, so this gets fixed up as well as
actually respecting the bus_dma_mask everywhere instead of just
rejecting dma mask that don't fit into it.

Alltogether this should be all core dma-direct changes required to
move powerpc over to the generic code.

[PATCH 1/5] dma-mapping: make the get_required_mask method available unconditionally

[Christoph Hellwig]

This save some duplication for ia64, and makes the interface more
general.  In the long run we want each dma_map_ops instance to fill this
out, but this will take a little more prep work.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/ia64/include/asm/dma-mapping.h  |  2 --
 arch/ia64/include/asm/machvec.h      |  7 -------
 arch/ia64/include/asm/machvec_init.h |  1 -
 arch/ia64/include/asm/machvec_sn2.h  |  2 --
 arch/ia64/pci/pci.c                  | 26 --------------------------
 arch/ia64/sn/pci/pci_dma.c           |  4 ++--
 drivers/base/platform.c              | 13 +++++++++++--
 drivers/pci/controller/vmd.c         |  4 ----
 include/linux/dma-mapping.h          |  2 --
 9 files changed, 13 insertions(+), 48 deletions(-)

diff --git a/arch/ia64/include/asm/dma-mapping.h b/arch/ia64/include/asm/dma-mapping.h
index 76e4d6632d68..522745ae67bb 100644
--- a/arch/ia64/include/asm/dma-mapping.h
+++ b/arch/ia64/include/asm/dma-mapping.h
@@ -10,8 +10,6 @@
 #include <linux/scatterlist.h>
 #include <linux/dma-debug.h>
 
-#define ARCH_HAS_DMA_GET_REQUIRED_MASK
-
 extern const struct dma_map_ops *dma_ops;
 extern struct ia64_machine_vector ia64_mv;
 extern void set_iommu_machvec(void);
diff --git a/arch/ia64/include/asm/machvec.h b/arch/ia64/include/asm/machvec.h
index 267f4f170191..5133739966bc 100644
--- a/arch/ia64/include/asm/machvec.h
+++ b/arch/ia64/include/asm/machvec.h
@@ -44,7 +44,6 @@ typedef void ia64_mv_kernel_launch_event_t(void);
 
 /* DMA-mapping interface: */
 typedef void ia64_mv_dma_init (void);
-typedef u64 ia64_mv_dma_get_required_mask (struct device *);
 typedef const struct dma_map_ops *ia64_mv_dma_get_ops(struct device *);
 
 /*
@@ -127,7 +126,6 @@ extern void machvec_tlb_migrate_finish (struct mm_struct *);
 #  define platform_global_tlb_purge	ia64_mv.global_tlb_purge
 #  define platform_tlb_migrate_finish	ia64_mv.tlb_migrate_finish
 #  define platform_dma_init		ia64_mv.dma_init
-#  define platform_dma_get_required_mask ia64_mv.dma_get_required_mask
 #  define platform_dma_get_ops		ia64_mv.dma_get_ops
 #  define platform_irq_to_vector	ia64_mv.irq_to_vector
 #  define platform_local_vector_to_irq	ia64_mv.local_vector_to_irq
@@ -171,7 +169,6 @@ struct ia64_machine_vector {
 	ia64_mv_global_tlb_purge_t *global_tlb_purge;
 	ia64_mv_tlb_migrate_finish_t *tlb_migrate_finish;
 	ia64_mv_dma_init *dma_init;
-	ia64_mv_dma_get_required_mask *dma_get_required_mask;
 	ia64_mv_dma_get_ops *dma_get_ops;
 	ia64_mv_irq_to_vector *irq_to_vector;
 	ia64_mv_local_vector_to_irq *local_vector_to_irq;
@@ -211,7 +208,6 @@ struct ia64_machine_vector {
 	platform_global_tlb_purge,		\
 	platform_tlb_migrate_finish,		\
 	platform_dma_init,			\
-	platform_dma_get_required_mask,		\
 	platform_dma_get_ops,			\
 	platform_irq_to_vector,			\
 	platform_local_vector_to_irq,		\
@@ -286,9 +282,6 @@ extern const struct dma_map_ops *dma_get_ops(struct device *);
 #ifndef platform_dma_get_ops
 # define platform_dma_get_ops		dma_get_ops
 #endif
-#ifndef platform_dma_get_required_mask
-# define  platform_dma_get_required_mask	ia64_dma_get_required_mask
-#endif
 #ifndef platform_irq_to_vector
 # define platform_irq_to_vector		__ia64_irq_to_vector
 #endif
diff --git a/arch/ia64/include/asm/machvec_init.h b/arch/ia64/include/asm/machvec_init.h
index 2b32fd06b7c6..2aafb69a3787 100644
--- a/arch/ia64/include/asm/machvec_init.h
+++ b/arch/ia64/include/asm/machvec_init.h
@@ -4,7 +4,6 @@
 
 extern ia64_mv_send_ipi_t ia64_send_ipi;
 extern ia64_mv_global_tlb_purge_t ia64_global_tlb_purge;
-extern ia64_mv_dma_get_required_mask ia64_dma_get_required_mask;
 extern ia64_mv_irq_to_vector __ia64_irq_to_vector;
 extern ia64_mv_local_vector_to_irq __ia64_local_vector_to_irq;
 extern ia64_mv_pci_get_legacy_mem_t ia64_pci_get_legacy_mem;
diff --git a/arch/ia64/include/asm/machvec_sn2.h b/arch/ia64/include/asm/machvec_sn2.h
index ece9fa85be88..b5153d300289 100644
--- a/arch/ia64/include/asm/machvec_sn2.h
+++ b/arch/ia64/include/asm/machvec_sn2.h
@@ -55,7 +55,6 @@ extern ia64_mv_readb_t __sn_readb_relaxed;
 extern ia64_mv_readw_t __sn_readw_relaxed;
 extern ia64_mv_readl_t __sn_readl_relaxed;
 extern ia64_mv_readq_t __sn_readq_relaxed;
-extern ia64_mv_dma_get_required_mask	sn_dma_get_required_mask;
 extern ia64_mv_dma_init			sn_dma_init;
 extern ia64_mv_migrate_t		sn_migrate;
 extern ia64_mv_kernel_launch_event_t	sn_kernel_launch_event;
@@ -100,7 +99,6 @@ extern ia64_mv_pci_fixup_bus_t		sn_pci_fixup_bus;
 #define platform_pci_get_legacy_mem	sn_pci_get_legacy_mem
 #define platform_pci_legacy_read	sn_pci_legacy_read
 #define platform_pci_legacy_write	sn_pci_legacy_write
-#define platform_dma_get_required_mask	sn_dma_get_required_mask
 #define platform_dma_init		sn_dma_init
 #define platform_migrate		sn_migrate
 #define platform_kernel_launch_event    sn_kernel_launch_event
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 7ccc64d5fe3e..5d71800df431 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -568,32 +568,6 @@ static void __init set_pci_dfl_cacheline_size(void)
 	pci_dfl_cache_line_size = (1 << cci.pcci_line_size) / 4;
 }
 
-u64 ia64_dma_get_required_mask(struct device *dev)
-{
-	u32 low_totalram = ((max_pfn - 1) << PAGE_SHIFT);
-	u32 high_totalram = ((max_pfn - 1) >> (32 - PAGE_SHIFT));
-	u64 mask;
-
-	if (!high_totalram) {
-		/* convert to mask just covering totalram */
-		low_totalram = (1 << (fls(low_totalram) - 1));
-		low_totalram += low_totalram - 1;
-		mask = low_totalram;
-	} else {
-		high_totalram = (1 << (fls(high_totalram) - 1));
-		high_totalram += high_totalram - 1;
-		mask = (((u64)high_totalram) << 32) + 0xffffffff;
-	}
-	return mask;
-}
-EXPORT_SYMBOL_GPL(ia64_dma_get_required_mask);
-
-u64 dma_get_required_mask(struct device *dev)
-{
-	return platform_dma_get_required_mask(dev);
-}
-EXPORT_SYMBOL_GPL(dma_get_required_mask);
-
 static int __init pcibios_init(void)
 {
 	set_pci_dfl_cacheline_size();
diff --git a/arch/ia64/sn/pci/pci_dma.c b/arch/ia64/sn/pci/pci_dma.c
index 74c934a997bb..96eb2567718a 100644
--- a/arch/ia64/sn/pci/pci_dma.c
+++ b/arch/ia64/sn/pci/pci_dma.c
@@ -344,11 +344,10 @@ static int sn_dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
 	return 0;
 }
 
-u64 sn_dma_get_required_mask(struct device *dev)
+static u64 sn_dma_get_required_mask(struct device *dev)
 {
 	return DMA_BIT_MASK(64);
 }
-EXPORT_SYMBOL_GPL(sn_dma_get_required_mask);
 
 char *sn_pci_get_legacy_mem(struct pci_bus *bus)
 {
@@ -473,6 +472,7 @@ static struct dma_map_ops sn_dma_ops = {
 	.sync_sg_for_device	= sn_dma_sync_sg_for_device,
 	.mapping_error		= sn_dma_mapping_error,
 	.dma_supported		= sn_dma_supported,
+	.get_required_mask	= sn_dma_get_required_mask,
 };
 
 void sn_dma_init(void)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index dff82a3c2caa..709ed36026a1 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -1179,8 +1179,7 @@ int __init platform_bus_init(void)
 	return error;
 }
 
-#ifndef ARCH_HAS_DMA_GET_REQUIRED_MASK
-u64 dma_get_required_mask(struct device *dev)
+static u64 __dma_get_required_mask(struct device *dev)
 {
 	u32 low_totalram = ((max_pfn - 1) << PAGE_SHIFT);
 	u32 high_totalram = ((max_pfn - 1) >> (32 - PAGE_SHIFT));
@@ -1198,6 +1197,16 @@ u64 dma_get_required_mask(struct device *dev)
 	}
 	return mask;
 }
+
+#ifndef ARCH_HAS_DMA_GET_REQUIRED_MASK
+u64 dma_get_required_mask(struct device *dev)
+{
+	const struct dma_map_ops *ops = get_dma_ops(dev);
+
+	if (ops->get_required_mask)
+		return ops->get_required_mask(dev);
+	return __dma_get_required_mask(dev);
+}
 EXPORT_SYMBOL_GPL(dma_get_required_mask);
 #endif
 
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index fd2dbd7eed7b..f31ed62d518c 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -404,12 +404,10 @@ static int vmd_dma_supported(struct device *dev, u64 mask)
 	return vmd_dma_ops(dev)->dma_supported(to_vmd_dev(dev), mask);
 }
 
-#ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
 static u64 vmd_get_required_mask(struct device *dev)
 {
 	return vmd_dma_ops(dev)->get_required_mask(to_vmd_dev(dev));
 }
-#endif
 
 static void vmd_teardown_dma_ops(struct vmd_dev *vmd)
 {
@@ -450,9 +448,7 @@ static void vmd_setup_dma_ops(struct vmd_dev *vmd)
 	ASSIGN_VMD_DMA_OPS(source, dest, sync_sg_for_device);
 	ASSIGN_VMD_DMA_OPS(source, dest, mapping_error);
 	ASSIGN_VMD_DMA_OPS(source, dest, dma_supported);
-#ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
 	ASSIGN_VMD_DMA_OPS(source, dest, get_required_mask);
-#endif
 	add_dma_domain(domain);
 }
 #undef ASSIGN_VMD_DMA_OPS
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index bd81e74cca7b..bddafc2513d8 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -130,9 +130,7 @@ struct dma_map_ops {
 			enum dma_data_direction direction);
 	int (*mapping_error)(struct device *dev, dma_addr_t dma_addr);
 	int (*dma_supported)(struct device *dev, u64 mask);
-#ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
 	u64 (*get_required_mask)(struct device *dev);
-#endif
 };
 
 extern const struct dma_map_ops dma_direct_ops;
-- 
2.18.0

[PATCH 2/5] dma-direct: add an explicit dma_direct_get_required_mask

[Christoph Hellwig]

This is somewhat modelled after the powerpc version, and differs from
the legacy fallback in use fls64 instead of pointlessly splitting up the
address into low and high dwords and in that it takes (__)phys_to_dma
into account.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/dma-direct.h |  1 +
 kernel/dma/direct.c        | 21 ++++++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 86a59ba5a7f3..b79496d8c75b 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -55,6 +55,7 @@ static inline void dma_mark_clean(void *addr, size_t size)
 }
 #endif /* CONFIG_ARCH_HAS_DMA_MARK_CLEAN */
 
+u64 dma_direct_get_required_mask(struct device *dev);
 void *dma_direct_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
 		gfp_t gfp, unsigned long attrs);
 void dma_direct_free(struct device *dev, size_t size, void *cpu_addr,
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index c954f0a6dc62..81b73a5bba54 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -53,11 +53,25 @@ check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
 	return true;
 }
 
+static inline dma_addr_t phys_to_dma_direct(struct device *dev,
+		phys_addr_t phys)
+{
+	if (force_dma_unencrypted())
+		return __phys_to_dma(dev, phys);
+	return phys_to_dma(dev, phys);
+}
+
+u64 dma_direct_get_required_mask(struct device *dev)
+{
+	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
+
+	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
+}
+
 static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
 {
-	dma_addr_t addr = force_dma_unencrypted() ?
-		__phys_to_dma(dev, phys) : phys_to_dma(dev, phys);
-	return addr + size - 1 <= dev->coherent_dma_mask;
+	return phys_to_dma_direct(dev, phys) + size - 1 <=
+			dev->coherent_dma_mask;
 }
 
 void *dma_direct_alloc_pages(struct device *dev, size_t size,
@@ -296,6 +310,7 @@ const struct dma_map_ops dma_direct_ops = {
 	.unmap_page		= dma_direct_unmap_page,
 	.unmap_sg		= dma_direct_unmap_sg,
 #endif
+	.get_required_mask	= dma_direct_get_required_mask,
 	.dma_supported		= dma_direct_supported,
 	.mapping_error		= dma_direct_mapping_error,
 	.cache_sync		= arch_dma_cache_sync,
-- 
2.18.0

[PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Christoph Hellwig]

We need to take the DMA offset and encryption bit into account when
selecting a zone.  User the opportunity to factor out the zone
selection into a helper for reuse.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 kernel/dma/direct.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 81b73a5bba54..3c404e33d946 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -68,6 +68,22 @@ u64 dma_direct_get_required_mask(struct device *dev)
 	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
 }
 
+static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
+		u64 *phys_mask)
+{
+	if (force_dma_unencrypted())
+		*phys_mask = __dma_to_phys(dev, dma_mask);
+	else
+		*phys_mask = dma_to_phys(dev, dma_mask);
+
+	/* GFP_DMA32 and GFP_DMA are no ops without the corresponding zones: */
+	if (*phys_mask <= DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
+		return GFP_DMA;
+	if (*phys_mask <= DMA_BIT_MASK(32))
+		return GFP_DMA32;
+	return 0;
+}
+
 static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
 {
 	return phys_to_dma_direct(dev, phys) + size - 1 <=
@@ -80,17 +96,13 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
 	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
 	int page_order = get_order(size);
 	struct page *page = NULL;
+	u64 phys_mask;
 	void *ret;
 
 	/* we always manually zero the memory once we are done: */
 	gfp &= ~__GFP_ZERO;
-
-	/* GFP_DMA32 and GFP_DMA are no ops without the corresponding zones: */
-	if (dev->coherent_dma_mask <= DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
-		gfp |= GFP_DMA;
-	if (dev->coherent_dma_mask <= DMA_BIT_MASK(32) && !(gfp & GFP_DMA))
-		gfp |= GFP_DMA32;
-
+	gfp |= __dma_direct_optimal_gfp_mask(dev, dev->coherent_dma_mask,
+			&phys_mask);
 again:
 	/* CMA can be used only in the context which permits sleeping */
 	if (gfpflags_allow_blocking(gfp)) {
@@ -109,15 +121,14 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
 		page = NULL;
 
 		if (IS_ENABLED(CONFIG_ZONE_DMA32) &&
-		    dev->coherent_dma_mask < DMA_BIT_MASK(64) &&
+		    phys_mask < DMA_BIT_MASK(64) &&
 		    !(gfp & (GFP_DMA32 | GFP_DMA))) {
 			gfp |= GFP_DMA32;
 			goto again;
 		}
 
 		if (IS_ENABLED(CONFIG_ZONE_DMA) &&
-		    dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
-		    !(gfp & GFP_DMA)) {
+		    phys_mask < DMA_BIT_MASK(32) && !(gfp & GFP_DMA)) {
 			gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
 			goto again;
 		}
-- 
2.18.0

[PATCH 4/5] dma-direct: implement complete bus_dma_mask handling

[Christoph Hellwig]

Instead of rejecting devices with a too small bus_dma_mask we can handle
by taking the bus dma_mask into account for allocations and bounce
buffering decisions.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/dma-direct.h |  3 ++-
 kernel/dma/direct.c        | 21 +++++++++++----------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index b79496d8c75b..fbca184ff5a0 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -27,7 +27,8 @@ static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
 	if (!dev->dma_mask)
 		return false;
 
-	return addr + size - 1 <= *dev->dma_mask;
+	return addr + size - 1 <=
+		min_not_zero(*dev->dma_mask, dev->bus_dma_mask);
 }
 #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
 
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 3c404e33d946..64466b7ef67b 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -43,10 +43,11 @@ check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
 			return false;
 		}
 
-		if (*dev->dma_mask >= DMA_BIT_MASK(32)) {
+		if (*dev->dma_mask >= DMA_BIT_MASK(32) || dev->bus_dma_mask) {
 			dev_err(dev,
-				"%s: overflow %pad+%zu of device mask %llx\n",
-				caller, &dma_addr, size, *dev->dma_mask);
+				"%s: overflow %pad+%zu of device mask %llx bus mask %llx\n",
+				caller, &dma_addr, size,
+				*dev->dma_mask, dev->bus_dma_mask);
 		}
 		return false;
 	}
@@ -65,12 +66,18 @@ u64 dma_direct_get_required_mask(struct device *dev)
 {
 	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
 
+	if (dev->bus_dma_mask && dev->bus_dma_mask < max_dma)
+		max_dma = dev->bus_dma_mask;
+
 	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
 }
 
 static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
 		u64 *phys_mask)
 {
+	if (dev->bus_dma_mask && dev->bus_dma_mask < dma_mask)
+		dma_mask = dev->bus_dma_mask;
+
 	if (force_dma_unencrypted())
 		*phys_mask = __dma_to_phys(dev, dma_mask);
 	else
@@ -87,7 +94,7 @@ static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
 static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
 {
 	return phys_to_dma_direct(dev, phys) + size - 1 <=
-			dev->coherent_dma_mask;
+			min_not_zero(dev->coherent_dma_mask, dev->bus_dma_mask);
 }
 
 void *dma_direct_alloc_pages(struct device *dev, size_t size,
@@ -291,12 +298,6 @@ int dma_direct_supported(struct device *dev, u64 mask)
 	if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
 		return 0;
 #endif
-	/*
-	 * Upstream PCI/PCIe bridges or SoC interconnects may not carry
-	 * as many DMA address bits as the device itself supports.
-	 */
-	if (dev->bus_dma_mask && mask > dev->bus_dma_mask)
-		return 0;
 	return 1;
 }
 
-- 
2.18.0

[PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Christoph Hellwig]

This way an architecture with less than 4G of RAM can support dma_mask
smaller than 32-bit without a ZONE_DMA.  Apparently that is a common
case on powerpc.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 kernel/dma/direct.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 64466b7ef67b..d1e103c6b107 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -4,6 +4,7 @@
  *
  * DMA operations that map physical memory directly without using an IOMMU.
  */
+#include <linux/bootmem.h> /* for max_pfn */
 #include <linux/export.h>
 #include <linux/mm.h>
 #include <linux/dma-direct.h>
@@ -283,21 +284,24 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
 	return nents;
 }
 
+/*
+ * Because 32-bit DMA masks are so common we expect every architecture to be
+ * able to satisfy them - either by not supporting more physical memory, or by
+ * providing a ZONE_DMA32.  If neither is the case, the architecture needs to
+ * use an IOMMU instead of the direct mapping.
+ */
 int dma_direct_supported(struct device *dev, u64 mask)
 {
-#ifdef CONFIG_ZONE_DMA
-	if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
-		return 0;
-#else
-	/*
-	 * Because 32-bit DMA masks are so common we expect every architecture
-	 * to be able to satisfy them - either by not supporting more physical
-	 * memory, or by providing a ZONE_DMA32.  If neither is the case, the
-	 * architecture needs to use an IOMMU instead of the direct mapping.
-	 */
-	if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
+	u64 min_mask;
+
+	if (IS_ENABLED(CONFIG_ZONE_DMA))
+		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
+	else
+		min_mask = min_t(u64, DMA_BIT_MASK(32),
+				 (max_pfn - 1) << PAGE_SHIFT);
+
+	if (mask >= phys_to_dma(dev, min_mask))
 		return 0;
-#endif
 	return 1;
 }
 
-- 
2.18.0

Re: [PATCH 1/5] dma-mapping: make the get_required_mask method available unconditionally

[Benjamin Herrenschmidt]

On Thu, 2018-09-20 at 20:52 +0200, Christoph Hellwig wrote:
> This save some duplication for ia64, and makes the interface more
> general.  In the long run we want each dma_map_ops instance to fill this
> out, but this will take a little more prep work.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

(For powerpc)

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
>  arch/ia64/include/asm/dma-mapping.h  |  2 --
>  arch/ia64/include/asm/machvec.h      |  7 -------
>  arch/ia64/include/asm/machvec_init.h |  1 -
>  arch/ia64/include/asm/machvec_sn2.h  |  2 --
>  arch/ia64/pci/pci.c                  | 26 --------------------------
>  arch/ia64/sn/pci/pci_dma.c           |  4 ++--
>  drivers/base/platform.c              | 13 +++++++++++--
>  drivers/pci/controller/vmd.c         |  4 ----
>  include/linux/dma-mapping.h          |  2 --
>  9 files changed, 13 insertions(+), 48 deletions(-)
> 
> diff --git a/arch/ia64/include/asm/dma-mapping.h b/arch/ia64/include/asm/dma-mapping.h
> index 76e4d6632d68..522745ae67bb 100644
> --- a/arch/ia64/include/asm/dma-mapping.h
> +++ b/arch/ia64/include/asm/dma-mapping.h
> @@ -10,8 +10,6 @@
>  #include <linux/scatterlist.h>
>  #include <linux/dma-debug.h>
>  
> -#define ARCH_HAS_DMA_GET_REQUIRED_MASK
> -
>  extern const struct dma_map_ops *dma_ops;
>  extern struct ia64_machine_vector ia64_mv;
>  extern void set_iommu_machvec(void);
> diff --git a/arch/ia64/include/asm/machvec.h b/arch/ia64/include/asm/machvec.h
> index 267f4f170191..5133739966bc 100644
> --- a/arch/ia64/include/asm/machvec.h
> +++ b/arch/ia64/include/asm/machvec.h
> @@ -44,7 +44,6 @@ typedef void ia64_mv_kernel_launch_event_t(void);
>  
>  /* DMA-mapping interface: */
>  typedef void ia64_mv_dma_init (void);
> -typedef u64 ia64_mv_dma_get_required_mask (struct device *);
>  typedef const struct dma_map_ops *ia64_mv_dma_get_ops(struct device *);
>  
>  /*
> @@ -127,7 +126,6 @@ extern void machvec_tlb_migrate_finish (struct mm_struct *);
>  #  define platform_global_tlb_purge	ia64_mv.global_tlb_purge
>  #  define platform_tlb_migrate_finish	ia64_mv.tlb_migrate_finish
>  #  define platform_dma_init		ia64_mv.dma_init
> -#  define platform_dma_get_required_mask ia64_mv.dma_get_required_mask
>  #  define platform_dma_get_ops		ia64_mv.dma_get_ops
>  #  define platform_irq_to_vector	ia64_mv.irq_to_vector
>  #  define platform_local_vector_to_irq	ia64_mv.local_vector_to_irq
> @@ -171,7 +169,6 @@ struct ia64_machine_vector {
>  	ia64_mv_global_tlb_purge_t *global_tlb_purge;
>  	ia64_mv_tlb_migrate_finish_t *tlb_migrate_finish;
>  	ia64_mv_dma_init *dma_init;
> -	ia64_mv_dma_get_required_mask *dma_get_required_mask;
>  	ia64_mv_dma_get_ops *dma_get_ops;
>  	ia64_mv_irq_to_vector *irq_to_vector;
>  	ia64_mv_local_vector_to_irq *local_vector_to_irq;
> @@ -211,7 +208,6 @@ struct ia64_machine_vector {
>  	platform_global_tlb_purge,		\
>  	platform_tlb_migrate_finish,		\
>  	platform_dma_init,			\
> -	platform_dma_get_required_mask,		\
>  	platform_dma_get_ops,			\
>  	platform_irq_to_vector,			\
>  	platform_local_vector_to_irq,		\
> @@ -286,9 +282,6 @@ extern const struct dma_map_ops *dma_get_ops(struct device *);
>  #ifndef platform_dma_get_ops
>  # define platform_dma_get_ops		dma_get_ops
>  #endif
> -#ifndef platform_dma_get_required_mask
> -# define  platform_dma_get_required_mask	ia64_dma_get_required_mask
> -#endif
>  #ifndef platform_irq_to_vector
>  # define platform_irq_to_vector		__ia64_irq_to_vector
>  #endif
> diff --git a/arch/ia64/include/asm/machvec_init.h b/arch/ia64/include/asm/machvec_init.h
> index 2b32fd06b7c6..2aafb69a3787 100644
> --- a/arch/ia64/include/asm/machvec_init.h
> +++ b/arch/ia64/include/asm/machvec_init.h
> @@ -4,7 +4,6 @@
>  
>  extern ia64_mv_send_ipi_t ia64_send_ipi;
>  extern ia64_mv_global_tlb_purge_t ia64_global_tlb_purge;
> -extern ia64_mv_dma_get_required_mask ia64_dma_get_required_mask;
>  extern ia64_mv_irq_to_vector __ia64_irq_to_vector;
>  extern ia64_mv_local_vector_to_irq __ia64_local_vector_to_irq;
>  extern ia64_mv_pci_get_legacy_mem_t ia64_pci_get_legacy_mem;
> diff --git a/arch/ia64/include/asm/machvec_sn2.h b/arch/ia64/include/asm/machvec_sn2.h
> index ece9fa85be88..b5153d300289 100644
> --- a/arch/ia64/include/asm/machvec_sn2.h
> +++ b/arch/ia64/include/asm/machvec_sn2.h
> @@ -55,7 +55,6 @@ extern ia64_mv_readb_t __sn_readb_relaxed;
>  extern ia64_mv_readw_t __sn_readw_relaxed;
>  extern ia64_mv_readl_t __sn_readl_relaxed;
>  extern ia64_mv_readq_t __sn_readq_relaxed;
> -extern ia64_mv_dma_get_required_mask	sn_dma_get_required_mask;
>  extern ia64_mv_dma_init			sn_dma_init;
>  extern ia64_mv_migrate_t		sn_migrate;
>  extern ia64_mv_kernel_launch_event_t	sn_kernel_launch_event;
> @@ -100,7 +99,6 @@ extern ia64_mv_pci_fixup_bus_t		sn_pci_fixup_bus;
>  #define platform_pci_get_legacy_mem	sn_pci_get_legacy_mem
>  #define platform_pci_legacy_read	sn_pci_legacy_read
>  #define platform_pci_legacy_write	sn_pci_legacy_write
> -#define platform_dma_get_required_mask	sn_dma_get_required_mask
>  #define platform_dma_init		sn_dma_init
>  #define platform_migrate		sn_migrate
>  #define platform_kernel_launch_event    sn_kernel_launch_event
> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> index 7ccc64d5fe3e..5d71800df431 100644
> --- a/arch/ia64/pci/pci.c
> +++ b/arch/ia64/pci/pci.c
> @@ -568,32 +568,6 @@ static void __init set_pci_dfl_cacheline_size(void)
>  	pci_dfl_cache_line_size = (1 << cci.pcci_line_size) / 4;
>  }
>  
> -u64 ia64_dma_get_required_mask(struct device *dev)
> -{
> -	u32 low_totalram = ((max_pfn - 1) << PAGE_SHIFT);
> -	u32 high_totalram = ((max_pfn - 1) >> (32 - PAGE_SHIFT));
> -	u64 mask;
> -
> -	if (!high_totalram) {
> -		/* convert to mask just covering totalram */
> -		low_totalram = (1 << (fls(low_totalram) - 1));
> -		low_totalram += low_totalram - 1;
> -		mask = low_totalram;
> -	} else {
> -		high_totalram = (1 << (fls(high_totalram) - 1));
> -		high_totalram += high_totalram - 1;
> -		mask = (((u64)high_totalram) << 32) + 0xffffffff;
> -	}
> -	return mask;
> -}
> -EXPORT_SYMBOL_GPL(ia64_dma_get_required_mask);
> -
> -u64 dma_get_required_mask(struct device *dev)
> -{
> -	return platform_dma_get_required_mask(dev);
> -}
> -EXPORT_SYMBOL_GPL(dma_get_required_mask);
> -
>  static int __init pcibios_init(void)
>  {
>  	set_pci_dfl_cacheline_size();
> diff --git a/arch/ia64/sn/pci/pci_dma.c b/arch/ia64/sn/pci/pci_dma.c
> index 74c934a997bb..96eb2567718a 100644
> --- a/arch/ia64/sn/pci/pci_dma.c
> +++ b/arch/ia64/sn/pci/pci_dma.c
> @@ -344,11 +344,10 @@ static int sn_dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
>  	return 0;
>  }
>  
> -u64 sn_dma_get_required_mask(struct device *dev)
> +static u64 sn_dma_get_required_mask(struct device *dev)
>  {
>  	return DMA_BIT_MASK(64);
>  }
> -EXPORT_SYMBOL_GPL(sn_dma_get_required_mask);
>  
>  char *sn_pci_get_legacy_mem(struct pci_bus *bus)
>  {
> @@ -473,6 +472,7 @@ static struct dma_map_ops sn_dma_ops = {
>  	.sync_sg_for_device	= sn_dma_sync_sg_for_device,
>  	.mapping_error		= sn_dma_mapping_error,
>  	.dma_supported		= sn_dma_supported,
> +	.get_required_mask	= sn_dma_get_required_mask,
>  };
>  
>  void sn_dma_init(void)
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index dff82a3c2caa..709ed36026a1 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -1179,8 +1179,7 @@ int __init platform_bus_init(void)
>  	return error;
>  }
>  
> -#ifndef ARCH_HAS_DMA_GET_REQUIRED_MASK
> -u64 dma_get_required_mask(struct device *dev)
> +static u64 __dma_get_required_mask(struct device *dev)
>  {
>  	u32 low_totalram = ((max_pfn - 1) << PAGE_SHIFT);
>  	u32 high_totalram = ((max_pfn - 1) >> (32 - PAGE_SHIFT));
> @@ -1198,6 +1197,16 @@ u64 dma_get_required_mask(struct device *dev)
>  	}
>  	return mask;
>  }
> +
> +#ifndef ARCH_HAS_DMA_GET_REQUIRED_MASK
> +u64 dma_get_required_mask(struct device *dev)
> +{
> +	const struct dma_map_ops *ops = get_dma_ops(dev);
> +
> +	if (ops->get_required_mask)
> +		return ops->get_required_mask(dev);
> +	return __dma_get_required_mask(dev);
> +}
>  EXPORT_SYMBOL_GPL(dma_get_required_mask);
>  #endif
>  
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index fd2dbd7eed7b..f31ed62d518c 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -404,12 +404,10 @@ static int vmd_dma_supported(struct device *dev, u64 mask)
>  	return vmd_dma_ops(dev)->dma_supported(to_vmd_dev(dev), mask);
>  }
>  
> -#ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
>  static u64 vmd_get_required_mask(struct device *dev)
>  {
>  	return vmd_dma_ops(dev)->get_required_mask(to_vmd_dev(dev));
>  }
> -#endif
>  
>  static void vmd_teardown_dma_ops(struct vmd_dev *vmd)
>  {
> @@ -450,9 +448,7 @@ static void vmd_setup_dma_ops(struct vmd_dev *vmd)
>  	ASSIGN_VMD_DMA_OPS(source, dest, sync_sg_for_device);
>  	ASSIGN_VMD_DMA_OPS(source, dest, mapping_error);
>  	ASSIGN_VMD_DMA_OPS(source, dest, dma_supported);
> -#ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
>  	ASSIGN_VMD_DMA_OPS(source, dest, get_required_mask);
> -#endif
>  	add_dma_domain(domain);
>  }
>  #undef ASSIGN_VMD_DMA_OPS
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index bd81e74cca7b..bddafc2513d8 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -130,9 +130,7 @@ struct dma_map_ops {
>  			enum dma_data_direction direction);
>  	int (*mapping_error)(struct device *dev, dma_addr_t dma_addr);
>  	int (*dma_supported)(struct device *dev, u64 mask);
> -#ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
>  	u64 (*get_required_mask)(struct device *dev);
> -#endif
>  };
>  
>  extern const struct dma_map_ops dma_direct_ops;

Re: [PATCH 1/5] dma-mapping: make the get_required_mask method available unconditionally

[Benjamin Herrenschmidt]

On Thu, 2018-09-20 at 20:52 +0200, Christoph Hellwig wrote:
> This save some duplication for ia64, and makes the interface more
> general.  In the long run we want each dma_map_ops instance to fill this
> out, but this will take a little more prep work.
> 
> Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>

(For powerpc)

Acked-by: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>

> ---
>  arch/ia64/include/asm/dma-mapping.h  |  2 --
>  arch/ia64/include/asm/machvec.h      |  7 -------
>  arch/ia64/include/asm/machvec_init.h |  1 -
>  arch/ia64/include/asm/machvec_sn2.h  |  2 --
>  arch/ia64/pci/pci.c                  | 26 --------------------------
>  arch/ia64/sn/pci/pci_dma.c           |  4 ++--
>  drivers/base/platform.c              | 13 +++++++++++--
>  drivers/pci/controller/vmd.c         |  4 ----
>  include/linux/dma-mapping.h          |  2 --
>  9 files changed, 13 insertions(+), 48 deletions(-)
> 
> diff --git a/arch/ia64/include/asm/dma-mapping.h b/arch/ia64/include/asm/dma-mapping.h
> index 76e4d6632d68..522745ae67bb 100644
> --- a/arch/ia64/include/asm/dma-mapping.h
> +++ b/arch/ia64/include/asm/dma-mapping.h
> @@ -10,8 +10,6 @@
>  #include <linux/scatterlist.h>
>  #include <linux/dma-debug.h>
>  
> -#define ARCH_HAS_DMA_GET_REQUIRED_MASK
> -
>  extern const struct dma_map_ops *dma_ops;
>  extern struct ia64_machine_vector ia64_mv;
>  extern void set_iommu_machvec(void);
> diff --git a/arch/ia64/include/asm/machvec.h b/arch/ia64/include/asm/machvec.h
> index 267f4f170191..5133739966bc 100644
> --- a/arch/ia64/include/asm/machvec.h
> +++ b/arch/ia64/include/asm/machvec.h
> @@ -44,7 +44,6 @@ typedef void ia64_mv_kernel_launch_event_t(void);
>  
>  /* DMA-mapping interface: */
>  typedef void ia64_mv_dma_init (void);
> -typedef u64 ia64_mv_dma_get_required_mask (struct device *);
>  typedef const struct dma_map_ops *ia64_mv_dma_get_ops(struct device *);
>  
>  /*
> @@ -127,7 +126,6 @@ extern void machvec_tlb_migrate_finish (struct mm_struct *);
>  #  define platform_global_tlb_purge	ia64_mv.global_tlb_purge
>  #  define platform_tlb_migrate_finish	ia64_mv.tlb_migrate_finish
>  #  define platform_dma_init		ia64_mv.dma_init
> -#  define platform_dma_get_required_mask ia64_mv.dma_get_required_mask
>  #  define platform_dma_get_ops		ia64_mv.dma_get_ops
>  #  define platform_irq_to_vector	ia64_mv.irq_to_vector
>  #  define platform_local_vector_to_irq	ia64_mv.local_vector_to_irq
> @@ -171,7 +169,6 @@ struct ia64_machine_vector {
>  	ia64_mv_global_tlb_purge_t *global_tlb_purge;
>  	ia64_mv_tlb_migrate_finish_t *tlb_migrate_finish;
>  	ia64_mv_dma_init *dma_init;
> -	ia64_mv_dma_get_required_mask *dma_get_required_mask;
>  	ia64_mv_dma_get_ops *dma_get_ops;
>  	ia64_mv_irq_to_vector *irq_to_vector;
>  	ia64_mv_local_vector_to_irq *local_vector_to_irq;
> @@ -211,7 +208,6 @@ struct ia64_machine_vector {
>  	platform_global_tlb_purge,		\
>  	platform_tlb_migrate_finish,		\
>  	platform_dma_init,			\
> -	platform_dma_get_required_mask,		\
>  	platform_dma_get_ops,			\
>  	platform_irq_to_vector,			\
>  	platform_local_vector_to_irq,		\
> @@ -286,9 +282,6 @@ extern const struct dma_map_ops *dma_get_ops(struct device *);
>  #ifndef platform_dma_get_ops
>  # define platform_dma_get_ops		dma_get_ops
>  #endif
> -#ifndef platform_dma_get_required_mask
> -# define  platform_dma_get_required_mask	ia64_dma_get_required_mask
> -#endif
>  #ifndef platform_irq_to_vector
>  # define platform_irq_to_vector		__ia64_irq_to_vector
>  #endif
> diff --git a/arch/ia64/include/asm/machvec_init.h b/arch/ia64/include/asm/machvec_init.h
> index 2b32fd06b7c6..2aafb69a3787 100644
> --- a/arch/ia64/include/asm/machvec_init.h
> +++ b/arch/ia64/include/asm/machvec_init.h
> @@ -4,7 +4,6 @@
>  
>  extern ia64_mv_send_ipi_t ia64_send_ipi;
>  extern ia64_mv_global_tlb_purge_t ia64_global_tlb_purge;
> -extern ia64_mv_dma_get_required_mask ia64_dma_get_required_mask;
>  extern ia64_mv_irq_to_vector __ia64_irq_to_vector;
>  extern ia64_mv_local_vector_to_irq __ia64_local_vector_to_irq;
>  extern ia64_mv_pci_get_legacy_mem_t ia64_pci_get_legacy_mem;
> diff --git a/arch/ia64/include/asm/machvec_sn2.h b/arch/ia64/include/asm/machvec_sn2.h
> index ece9fa85be88..b5153d300289 100644
> --- a/arch/ia64/include/asm/machvec_sn2.h
> +++ b/arch/ia64/include/asm/machvec_sn2.h
> @@ -55,7 +55,6 @@ extern ia64_mv_readb_t __sn_readb_relaxed;
>  extern ia64_mv_readw_t __sn_readw_relaxed;
>  extern ia64_mv_readl_t __sn_readl_relaxed;
>  extern ia64_mv_readq_t __sn_readq_relaxed;
> -extern ia64_mv_dma_get_required_mask	sn_dma_get_required_mask;
>  extern ia64_mv_dma_init			sn_dma_init;
>  extern ia64_mv_migrate_t		sn_migrate;
>  extern ia64_mv_kernel_launch_event_t	sn_kernel_launch_event;
> @@ -100,7 +99,6 @@ extern ia64_mv_pci_fixup_bus_t		sn_pci_fixup_bus;
>  #define platform_pci_get_legacy_mem	sn_pci_get_legacy_mem
>  #define platform_pci_legacy_read	sn_pci_legacy_read
>  #define platform_pci_legacy_write	sn_pci_legacy_write
> -#define platform_dma_get_required_mask	sn_dma_get_required_mask
>  #define platform_dma_init		sn_dma_init
>  #define platform_migrate		sn_migrate
>  #define platform_kernel_launch_event    sn_kernel_launch_event
> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> index 7ccc64d5fe3e..5d71800df431 100644
> --- a/arch/ia64/pci/pci.c
> +++ b/arch/ia64/pci/pci.c
> @@ -568,32 +568,6 @@ static void __init set_pci_dfl_cacheline_size(void)
>  	pci_dfl_cache_line_size = (1 << cci.pcci_line_size) / 4;
>  }
>  
> -u64 ia64_dma_get_required_mask(struct device *dev)
> -{
> -	u32 low_totalram = ((max_pfn - 1) << PAGE_SHIFT);
> -	u32 high_totalram = ((max_pfn - 1) >> (32 - PAGE_SHIFT));
> -	u64 mask;
> -
> -	if (!high_totalram) {
> -		/* convert to mask just covering totalram */
> -		low_totalram = (1 << (fls(low_totalram) - 1));
> -		low_totalram += low_totalram - 1;
> -		mask = low_totalram;
> -	} else {
> -		high_totalram = (1 << (fls(high_totalram) - 1));
> -		high_totalram += high_totalram - 1;
> -		mask = (((u64)high_totalram) << 32) + 0xffffffff;
> -	}
> -	return mask;
> -}
> -EXPORT_SYMBOL_GPL(ia64_dma_get_required_mask);
> -
> -u64 dma_get_required_mask(struct device *dev)
> -{
> -	return platform_dma_get_required_mask(dev);
> -}
> -EXPORT_SYMBOL_GPL(dma_get_required_mask);
> -
>  static int __init pcibios_init(void)
>  {
>  	set_pci_dfl_cacheline_size();
> diff --git a/arch/ia64/sn/pci/pci_dma.c b/arch/ia64/sn/pci/pci_dma.c
> index 74c934a997bb..96eb2567718a 100644
> --- a/arch/ia64/sn/pci/pci_dma.c
> +++ b/arch/ia64/sn/pci/pci_dma.c
> @@ -344,11 +344,10 @@ static int sn_dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
>  	return 0;
>  }
>  
> -u64 sn_dma_get_required_mask(struct device *dev)
> +static u64 sn_dma_get_required_mask(struct device *dev)
>  {
>  	return DMA_BIT_MASK(64);
>  }
> -EXPORT_SYMBOL_GPL(sn_dma_get_required_mask);
>  
>  char *sn_pci_get_legacy_mem(struct pci_bus *bus)
>  {
> @@ -473,6 +472,7 @@ static struct dma_map_ops sn_dma_ops = {
>  	.sync_sg_for_device	= sn_dma_sync_sg_for_device,
>  	.mapping_error		= sn_dma_mapping_error,
>  	.dma_supported		= sn_dma_supported,
> +	.get_required_mask	= sn_dma_get_required_mask,
>  };
>  
>  void sn_dma_init(void)
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index dff82a3c2caa..709ed36026a1 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -1179,8 +1179,7 @@ int __init platform_bus_init(void)
>  	return error;
>  }
>  
> -#ifndef ARCH_HAS_DMA_GET_REQUIRED_MASK
> -u64 dma_get_required_mask(struct device *dev)
> +static u64 __dma_get_required_mask(struct device *dev)
>  {
>  	u32 low_totalram = ((max_pfn - 1) << PAGE_SHIFT);
>  	u32 high_totalram = ((max_pfn - 1) >> (32 - PAGE_SHIFT));
> @@ -1198,6 +1197,16 @@ u64 dma_get_required_mask(struct device *dev)
>  	}
>  	return mask;
>  }
> +
> +#ifndef ARCH_HAS_DMA_GET_REQUIRED_MASK
> +u64 dma_get_required_mask(struct device *dev)
> +{
> +	const struct dma_map_ops *ops = get_dma_ops(dev);
> +
> +	if (ops->get_required_mask)
> +		return ops->get_required_mask(dev);
> +	return __dma_get_required_mask(dev);
> +}
>  EXPORT_SYMBOL_GPL(dma_get_required_mask);
>  #endif
>  
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index fd2dbd7eed7b..f31ed62d518c 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -404,12 +404,10 @@ static int vmd_dma_supported(struct device *dev, u64 mask)
>  	return vmd_dma_ops(dev)->dma_supported(to_vmd_dev(dev), mask);
>  }
>  
> -#ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
>  static u64 vmd_get_required_mask(struct device *dev)
>  {
>  	return vmd_dma_ops(dev)->get_required_mask(to_vmd_dev(dev));
>  }
> -#endif
>  
>  static void vmd_teardown_dma_ops(struct vmd_dev *vmd)
>  {
> @@ -450,9 +448,7 @@ static void vmd_setup_dma_ops(struct vmd_dev *vmd)
>  	ASSIGN_VMD_DMA_OPS(source, dest, sync_sg_for_device);
>  	ASSIGN_VMD_DMA_OPS(source, dest, mapping_error);
>  	ASSIGN_VMD_DMA_OPS(source, dest, dma_supported);
> -#ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
>  	ASSIGN_VMD_DMA_OPS(source, dest, get_required_mask);
> -#endif
>  	add_dma_domain(domain);
>  }
>  #undef ASSIGN_VMD_DMA_OPS
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index bd81e74cca7b..bddafc2513d8 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -130,9 +130,7 @@ struct dma_map_ops {
>  			enum dma_data_direction direction);
>  	int (*mapping_error)(struct device *dev, dma_addr_t dma_addr);
>  	int (*dma_supported)(struct device *dev, u64 mask);
> -#ifdef ARCH_HAS_DMA_GET_REQUIRED_MASK
>  	u64 (*get_required_mask)(struct device *dev);
> -#endif
>  };
>  
>  extern const struct dma_map_ops dma_direct_ops;

Re: [PATCH 2/5] dma-direct: add an explicit dma_direct_get_required_mask

[Benjamin Herrenschmidt]

On Thu, 2018-09-20 at 20:52 +0200, Christoph Hellwig wrote:
> This is somewhat modelled after the powerpc version, and differs from
> the legacy fallback in use fls64 instead of pointlessly splitting up the
> address into low and high dwords and in that it takes (__)phys_to_dma
> into account.

This looks like it will be usable if/when we switch powerpc to
dma/direct.c

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  include/linux/dma-direct.h |  1 +
>  kernel/dma/direct.c        | 21 ++++++++++++++++++---
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index 86a59ba5a7f3..b79496d8c75b 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -55,6 +55,7 @@ static inline void dma_mark_clean(void *addr, size_t size)
>  }
>  #endif /* CONFIG_ARCH_HAS_DMA_MARK_CLEAN */
>  
> +u64 dma_direct_get_required_mask(struct device *dev);
>  void *dma_direct_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
>  		gfp_t gfp, unsigned long attrs);
>  void dma_direct_free(struct device *dev, size_t size, void *cpu_addr,
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index c954f0a6dc62..81b73a5bba54 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -53,11 +53,25 @@ check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
>  	return true;
>  }
>  
> +static inline dma_addr_t phys_to_dma_direct(struct device *dev,
> +		phys_addr_t phys)
> +{
> +	if (force_dma_unencrypted())
> +		return __phys_to_dma(dev, phys);
> +	return phys_to_dma(dev, phys);
> +}
> +
> +u64 dma_direct_get_required_mask(struct device *dev)
> +{
> +	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
> +
> +	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
> +}
> +
>  static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
>  {
> -	dma_addr_t addr = force_dma_unencrypted() ?
> -		__phys_to_dma(dev, phys) : phys_to_dma(dev, phys);
> -	return addr + size - 1 <= dev->coherent_dma_mask;
> +	return phys_to_dma_direct(dev, phys) + size - 1 <=
> +			dev->coherent_dma_mask;
>  }
>  
>  void *dma_direct_alloc_pages(struct device *dev, size_t size,
> @@ -296,6 +310,7 @@ const struct dma_map_ops dma_direct_ops = {
>  	.unmap_page		= dma_direct_unmap_page,
>  	.unmap_sg		= dma_direct_unmap_sg,
>  #endif
> +	.get_required_mask	= dma_direct_get_required_mask,
>  	.dma_supported		= dma_direct_supported,
>  	.mapping_error		= dma_direct_mapping_error,
>  	.cache_sync		= arch_dma_cache_sync,

Re: [PATCH 2/5] dma-direct: add an explicit dma_direct_get_required_mask

[Benjamin Herrenschmidt]

On Thu, 2018-09-20 at 20:52 +0200, Christoph Hellwig wrote:
> This is somewhat modelled after the powerpc version, and differs from
> the legacy fallback in use fls64 instead of pointlessly splitting up the
> address into low and high dwords and in that it takes (__)phys_to_dma
> into account.

This looks like it will be usable if/when we switch powerpc to
dma/direct.c

Acked-by: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
---
> Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
> ---
>  include/linux/dma-direct.h |  1 +
>  kernel/dma/direct.c        | 21 ++++++++++++++++++---
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index 86a59ba5a7f3..b79496d8c75b 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -55,6 +55,7 @@ static inline void dma_mark_clean(void *addr, size_t size)
>  }
>  #endif /* CONFIG_ARCH_HAS_DMA_MARK_CLEAN */
>  
> +u64 dma_direct_get_required_mask(struct device *dev);
>  void *dma_direct_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
>  		gfp_t gfp, unsigned long attrs);
>  void dma_direct_free(struct device *dev, size_t size, void *cpu_addr,
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index c954f0a6dc62..81b73a5bba54 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -53,11 +53,25 @@ check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
>  	return true;
>  }
>  
> +static inline dma_addr_t phys_to_dma_direct(struct device *dev,
> +		phys_addr_t phys)
> +{
> +	if (force_dma_unencrypted())
> +		return __phys_to_dma(dev, phys);
> +	return phys_to_dma(dev, phys);
> +}
> +
> +u64 dma_direct_get_required_mask(struct device *dev)
> +{
> +	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
> +
> +	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
> +}
> +
>  static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
>  {
> -	dma_addr_t addr = force_dma_unencrypted() ?
> -		__phys_to_dma(dev, phys) : phys_to_dma(dev, phys);
> -	return addr + size - 1 <= dev->coherent_dma_mask;
> +	return phys_to_dma_direct(dev, phys) + size - 1 <=
> +			dev->coherent_dma_mask;
>  }
>  
>  void *dma_direct_alloc_pages(struct device *dev, size_t size,
> @@ -296,6 +310,7 @@ const struct dma_map_ops dma_direct_ops = {
>  	.unmap_page		= dma_direct_unmap_page,
>  	.unmap_sg		= dma_direct_unmap_sg,
>  #endif
> +	.get_required_mask	= dma_direct_get_required_mask,
>  	.dma_supported		= dma_direct_supported,
>  	.mapping_error		= dma_direct_mapping_error,
>  	.cache_sync		= arch_dma_cache_sync,

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Benjamin Herrenschmidt]

On Thu, 2018-09-20 at 20:52 +0200, Christoph Hellwig wrote:
> +static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
> +               u64 *phys_mask)
> +{
> +       if (force_dma_unencrypted())
> +               *phys_mask = __dma_to_phys(dev, dma_mask);
> +       else
> +               *phys_mask = dma_to_phys(dev, dma_mask);
> +
> +       /* GFP_DMA32 and GFP_DMA are no ops without the corresponding zones: */
> +       if (*phys_mask <= DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
> +               return GFP_DMA;
> +       if (*phys_mask <= DMA_BIT_MASK(32))
> +               return GFP_DMA32;
> +       return 0;
> +}

I'm not sure this is entirely right.

Let's say the mask is 30 bits. You will return GFP_DMA32, which will
fail if you allocate something above 1G (which is legit for
ZONE_DMA32).

I think the logic should be:

	if (mask < ZONE_DMA)
		fail;
	else if (mask < ZONE_DMA32)
		use ZONE_DMA or fail if doesn't exist
	else if (mask < top_of_ram)
		use ZONE_DMA32 or fail if doesn't exist
	else
		use ZONE_NORMAL

Additionally, we want to fold-in the top-of-ram test such that we don't
fail the second case if the mask is 31-bits (smaller than ZONE_DMA32)
but top of ram is also small enough.

So the top of ram test should take precendence.

Cheers,
Ben.

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Benjamin Herrenschmidt]

On Thu, 2018-09-20 at 20:52 +0200, Christoph Hellwig wrote:
> +static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
> +               u64 *phys_mask)
> +{
> +       if (force_dma_unencrypted())
> +               *phys_mask = __dma_to_phys(dev, dma_mask);
> +       else
> +               *phys_mask = dma_to_phys(dev, dma_mask);
> +
> +       /* GFP_DMA32 and GFP_DMA are no ops without the corresponding zones: */
> +       if (*phys_mask <= DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
> +               return GFP_DMA;
> +       if (*phys_mask <= DMA_BIT_MASK(32))
> +               return GFP_DMA32;
> +       return 0;
> +}

I'm not sure this is entirely right.

Let's say the mask is 30 bits. You will return GFP_DMA32, which will
fail if you allocate something above 1G (which is legit for
ZONE_DMA32).

I think the logic should be:

	if (mask < ZONE_DMA)
		fail;
	else if (mask < ZONE_DMA32)
		use ZONE_DMA or fail if doesn't exist
	else if (mask < top_of_ram)
		use ZONE_DMA32 or fail if doesn't exist
	else
		use ZONE_NORMAL

Additionally, we want to fold-in the top-of-ram test such that we don't
fail the second case if the mask is 31-bits (smaller than ZONE_DMA32)
but top of ram is also small enough.

So the top of ram test should take precendence.

Cheers,
Ben.

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Benjamin Herrenschmidt]

On Thu, 2018-09-20 at 20:52 +0200, Christoph Hellwig wrote:
> This way an architecture with less than 4G of RAM can support dma_mask
> smaller than 32-bit without a ZONE_DMA.  Apparently that is a common
> case on powerpc.

Anything that uses a b43 wifi adapter which has a 31-bit limitation
actually :-)
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  kernel/dma/direct.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 64466b7ef67b..d1e103c6b107 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -4,6 +4,7 @@
>   *
>   * DMA operations that map physical memory directly without using an IOMMU.
>   */
> +#include <linux/bootmem.h> /* for max_pfn */
>  #include <linux/export.h>
>  #include <linux/mm.h>
>  #include <linux/dma-direct.h>
> @@ -283,21 +284,24 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
>  	return nents;
>  }
>  
> +/*
> + * Because 32-bit DMA masks are so common we expect every architecture to be
> + * able to satisfy them - either by not supporting more physical memory, or by
> + * providing a ZONE_DMA32.  If neither is the case, the architecture needs to
> + * use an IOMMU instead of the direct mapping.
> + */
>  int dma_direct_supported(struct device *dev, u64 mask)
>  {
> -#ifdef CONFIG_ZONE_DMA
> -	if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
> -		return 0;
> -#else
> -	/*
> -	 * Because 32-bit DMA masks are so common we expect every architecture
> -	 * to be able to satisfy them - either by not supporting more physical
> -	 * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> -	 * architecture needs to use an IOMMU instead of the direct mapping.
> -	 */
> -	if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> +	u64 min_mask;
> +
> +	if (IS_ENABLED(CONFIG_ZONE_DMA))
> +		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> +	else
> +		min_mask = min_t(u64, DMA_BIT_MASK(32),
> +				 (max_pfn - 1) << PAGE_SHIFT);
> +
> +	if (mask >= phys_to_dma(dev, min_mask))
>  		return 0;

nitpick ... to be completely "correct", I would have written

	if (IS_ENABLED(CONFIG_ZONE_DMA))
		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
	else
		min_mask = DMA_BIT_MASK(32);

	min_mask = min_t(u64, min_mask,	(max_pfn - 1) << PAGE_SHIFT);

In "theory" it's also ok to have a mask < ZONE_DMA_BITS as long as it's
big enough to fit all memory :-)
	
> -#endif
>  	return 1;
>  }
>  

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Benjamin Herrenschmidt]

On Thu, 2018-09-20 at 20:52 +0200, Christoph Hellwig wrote:
> This way an architecture with less than 4G of RAM can support dma_mask
> smaller than 32-bit without a ZONE_DMA.  Apparently that is a common
> case on powerpc.

Anything that uses a b43 wifi adapter which has a 31-bit limitation
actually :-)
> 
> Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
> ---
>  kernel/dma/direct.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 64466b7ef67b..d1e103c6b107 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -4,6 +4,7 @@
>   *
>   * DMA operations that map physical memory directly without using an IOMMU.
>   */
> +#include <linux/bootmem.h> /* for max_pfn */
>  #include <linux/export.h>
>  #include <linux/mm.h>
>  #include <linux/dma-direct.h>
> @@ -283,21 +284,24 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
>  	return nents;
>  }
>  
> +/*
> + * Because 32-bit DMA masks are so common we expect every architecture to be
> + * able to satisfy them - either by not supporting more physical memory, or by
> + * providing a ZONE_DMA32.  If neither is the case, the architecture needs to
> + * use an IOMMU instead of the direct mapping.
> + */
>  int dma_direct_supported(struct device *dev, u64 mask)
>  {
> -#ifdef CONFIG_ZONE_DMA
> -	if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
> -		return 0;
> -#else
> -	/*
> -	 * Because 32-bit DMA masks are so common we expect every architecture
> -	 * to be able to satisfy them - either by not supporting more physical
> -	 * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> -	 * architecture needs to use an IOMMU instead of the direct mapping.
> -	 */
> -	if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> +	u64 min_mask;
> +
> +	if (IS_ENABLED(CONFIG_ZONE_DMA))
> +		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> +	else
> +		min_mask = min_t(u64, DMA_BIT_MASK(32),
> +				 (max_pfn - 1) << PAGE_SHIFT);
> +
> +	if (mask >= phys_to_dma(dev, min_mask))
>  		return 0;

nitpick ... to be completely "correct", I would have written

	if (IS_ENABLED(CONFIG_ZONE_DMA))
		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
	else
		min_mask = DMA_BIT_MASK(32);

	min_mask = min_t(u64, min_mask,	(max_pfn - 1) << PAGE_SHIFT);

In "theory" it's also ok to have a mask < ZONE_DMA_BITS as long as it's
big enough to fit all memory :-)
	
> -#endif
>  	return 1;
>  }
>  

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Christoph Hellwig]

On Thu, Sep 27, 2018 at 11:45:15AM +1000, Benjamin Herrenschmidt wrote:
> I'm not sure this is entirely right.
> 
> Let's say the mask is 30 bits. You will return GFP_DMA32, which will
> fail if you allocate something above 1G (which is legit for
> ZONE_DMA32).

And then we will try GFP_DMA further down in the function:

		if (IS_ENABLED(CONFIG_ZONE_DMA) &&
		    dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
		    !(gfp & GFP_DMA)) {
			gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
			goto again;
		}

This is and old optimization from x86, because chances are high that
GFP_DMA32 will give you suitable memory for the infamous 31-bit
dma mask devices (at least at boot time) and thus we don't have
to deplete the tiny ZONE_DMA pool.

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Christoph Hellwig]

On Thu, Sep 27, 2018 at 11:45:15AM +1000, Benjamin Herrenschmidt wrote:
> I'm not sure this is entirely right.
> 
> Let's say the mask is 30 bits. You will return GFP_DMA32, which will
> fail if you allocate something above 1G (which is legit for
> ZONE_DMA32).

And then we will try GFP_DMA further down in the function:

		if (IS_ENABLED(CONFIG_ZONE_DMA) &&
		    dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
		    !(gfp & GFP_DMA)) {
			gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
			goto again;
		}

This is and old optimization from x86, because chances are high that
GFP_DMA32 will give you suitable memory for the infamous 31-bit
dma mask devices (at least at boot time) and thus we don't have
to deplete the tiny ZONE_DMA pool.

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Christoph Hellwig]

On Thu, Sep 27, 2018 at 11:50:14AM +1000, Benjamin Herrenschmidt wrote:
> > -	 * to be able to satisfy them - either by not supporting more physical
> > -	 * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> > -	 * architecture needs to use an IOMMU instead of the direct mapping.
> > -	 */
> > -	if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> > +	u64 min_mask;
> > +
> > +	if (IS_ENABLED(CONFIG_ZONE_DMA))
> > +		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> > +	else
> > +		min_mask = min_t(u64, DMA_BIT_MASK(32),
> > +				 (max_pfn - 1) << PAGE_SHIFT);
> > +
> > +	if (mask >= phys_to_dma(dev, min_mask))
> >  		return 0;
> 
> nitpick ... to be completely "correct", I would have written
> 
> 	if (IS_ENABLED(CONFIG_ZONE_DMA))
> 		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> 	else
> 		min_mask = DMA_BIT_MASK(32);
> 
> 	min_mask = min_t(u64, min_mask,	(max_pfn - 1) << PAGE_SHIFT);
> 
> In "theory" it's also ok to have a mask < ZONE_DMA_BITS as long as it's
> big enough to fit all memory :-)

Yeah, we could do that.

Re: [PATCH 2/5] dma-direct: add an explicit dma_direct_get_required_mask

[Robin Murphy]

On 20/09/18 19:52, Christoph Hellwig wrote:
> This is somewhat modelled after the powerpc version, and differs from
> the legacy fallback in use fls64 instead of pointlessly splitting up the
> address into low and high dwords and in that it takes (__)phys_to_dma
> into account.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   include/linux/dma-direct.h |  1 +
>   kernel/dma/direct.c        | 21 ++++++++++++++++++---
>   2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index 86a59ba5a7f3..b79496d8c75b 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -55,6 +55,7 @@ static inline void dma_mark_clean(void *addr, size_t size)
>   }
>   #endif /* CONFIG_ARCH_HAS_DMA_MARK_CLEAN */
>   
> +u64 dma_direct_get_required_mask(struct device *dev);
>   void *dma_direct_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
>   		gfp_t gfp, unsigned long attrs);
>   void dma_direct_free(struct device *dev, size_t size, void *cpu_addr,
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index c954f0a6dc62..81b73a5bba54 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -53,11 +53,25 @@ check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
>   	return true;
>   }
>   
> +static inline dma_addr_t phys_to_dma_direct(struct device *dev,
> +		phys_addr_t phys)
> +{
> +	if (force_dma_unencrypted())
> +		return __phys_to_dma(dev, phys);
> +	return phys_to_dma(dev, phys);
> +}
> +
> +u64 dma_direct_get_required_mask(struct device *dev)
> +{
> +	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
> +
> +	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;

I think that may as well just use __fls64() - it seems reasonable to 
assume max_dma > 0. Otherwise,

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

> +}
> +
>   static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
>   {
> -	dma_addr_t addr = force_dma_unencrypted() ?
> -		__phys_to_dma(dev, phys) : phys_to_dma(dev, phys);
> -	return addr + size - 1 <= dev->coherent_dma_mask;
> +	return phys_to_dma_direct(dev, phys) + size - 1 <=
> +			dev->coherent_dma_mask;
>   }
>   
>   void *dma_direct_alloc_pages(struct device *dev, size_t size,
> @@ -296,6 +310,7 @@ const struct dma_map_ops dma_direct_ops = {
>   	.unmap_page		= dma_direct_unmap_page,
>   	.unmap_sg		= dma_direct_unmap_sg,
>   #endif
> +	.get_required_mask	= dma_direct_get_required_mask,
>   	.dma_supported		= dma_direct_supported,
>   	.mapping_error		= dma_direct_mapping_error,
>   	.cache_sync		= arch_dma_cache_sync,
> 

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Robin Murphy]

On 20/09/18 19:52, Christoph Hellwig wrote:
> We need to take the DMA offset and encryption bit into account when
> selecting a zone.  User the opportunity to factor out the zone
> selection into a helper for reuse.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   kernel/dma/direct.c | 31 +++++++++++++++++++++----------
>   1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 81b73a5bba54..3c404e33d946 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -68,6 +68,22 @@ u64 dma_direct_get_required_mask(struct device *dev)
>   	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
>   }
>   
> +static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
> +		u64 *phys_mask)
> +{
> +	if (force_dma_unencrypted())
> +		*phys_mask = __dma_to_phys(dev, dma_mask);
> +	else
> +		*phys_mask = dma_to_phys(dev, dma_mask);

Maybe make phys_to_dma_direct() take u64 instead of phys_addr_t so we 
can reuse it here?

> +	/* GFP_DMA32 and GFP_DMA are no ops without the corresponding zones: */
> +	if (*phys_mask <= DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
> +		return GFP_DMA;

If we don't have ZONE_DMA it would in theory be a tiny bit better to 
fall back to GFP_DMA32 instead of returning 0 here, but I'm not sure 
it's worth the bother.

> +	if (*phys_mask <= DMA_BIT_MASK(32))
> +		return GFP_DMA32;
> +	return 0;
> +}
> +
>   static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
>   {
>   	return phys_to_dma_direct(dev, phys) + size - 1 <=
> @@ -80,17 +96,13 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
>   	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
>   	int page_order = get_order(size);
>   	struct page *page = NULL;
> +	u64 phys_mask;
>   	void *ret;
>   
>   	/* we always manually zero the memory once we are done: */
>   	gfp &= ~__GFP_ZERO;
> -
> -	/* GFP_DMA32 and GFP_DMA are no ops without the corresponding zones: */
> -	if (dev->coherent_dma_mask <= DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
> -		gfp |= GFP_DMA;
> -	if (dev->coherent_dma_mask <= DMA_BIT_MASK(32) && !(gfp & GFP_DMA))
> -		gfp |= GFP_DMA32;
> -
> +	gfp |= __dma_direct_optimal_gfp_mask(dev, dev->coherent_dma_mask,
> +			&phys_mask);
>   again:
>   	/* CMA can be used only in the context which permits sleeping */
>   	if (gfpflags_allow_blocking(gfp)) {
> @@ -109,15 +121,14 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
>   		page = NULL;
>   
>   		if (IS_ENABLED(CONFIG_ZONE_DMA32) &&
> -		    dev->coherent_dma_mask < DMA_BIT_MASK(64) &&
> +		    phys_mask < DMA_BIT_MASK(64) &&
>   		    !(gfp & (GFP_DMA32 | GFP_DMA))) {
>   			gfp |= GFP_DMA32;
>   			goto again;

Ah right, in that situation we should probably end up here anyway, so 
that's good enough - definitely not worth any more #ifdeffery above. 
Nits aside,

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

>   		}
>   
>   		if (IS_ENABLED(CONFIG_ZONE_DMA) &&
> -		    dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
> -		    !(gfp & GFP_DMA)) {
> +		    phys_mask < DMA_BIT_MASK(32) && !(gfp & GFP_DMA)) {
>   			gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
>   			goto again;
>   		}
> 

Re: [PATCH 4/5] dma-direct: implement complete bus_dma_mask handling

[Robin Murphy]

On 20/09/18 19:52, Christoph Hellwig wrote:
> Instead of rejecting devices with a too small bus_dma_mask we can handle
> by taking the bus dma_mask into account for allocations and bounce
> buffering decisions.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   include/linux/dma-direct.h |  3 ++-
>   kernel/dma/direct.c        | 21 +++++++++++----------
>   2 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index b79496d8c75b..fbca184ff5a0 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -27,7 +27,8 @@ static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
>   	if (!dev->dma_mask)
>   		return false;
>   
> -	return addr + size - 1 <= *dev->dma_mask;
> +	return addr + size - 1 <=
> +		min_not_zero(*dev->dma_mask, dev->bus_dma_mask);
>   }
>   #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>   
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 3c404e33d946..64466b7ef67b 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -43,10 +43,11 @@ check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
>   			return false;
>   		}
>   
> -		if (*dev->dma_mask >= DMA_BIT_MASK(32)) {
> +		if (*dev->dma_mask >= DMA_BIT_MASK(32) || dev->bus_dma_mask) {

Hmm... say *dev->dma_mask is 31 bits and dev->bus_dma_mask is 40 bits 
due to a global DT property, we'll now scream where we didn't before 
even though the bus mask is almost certainly irrelevant - is that desirable?

>   			dev_err(dev,
> -				"%s: overflow %pad+%zu of device mask %llx\n",
> -				caller, &dma_addr, size, *dev->dma_mask);
> +				"%s: overflow %pad+%zu of device mask %llx bus mask %llx\n",
> +				caller, &dma_addr, size,
> +				*dev->dma_mask, dev->bus_dma_mask);
>   		}
>   		return false;
>   	}
> @@ -65,12 +66,18 @@ u64 dma_direct_get_required_mask(struct device *dev)
>   {
>   	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
>   
> +	if (dev->bus_dma_mask && dev->bus_dma_mask < max_dma)
> +		max_dma = dev->bus_dma_mask;

Again, I think we could just do another min_not_zero() here. A device 
wired to address only one single page of RAM isn't a realistic prospect 
(and we could just flip the -1 and the shift in the max_dma calculation 
if we *really* wanted to support such things).

> +
>   	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
>   }
>   
>   static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
>   		u64 *phys_mask)
>   {
> +	if (dev->bus_dma_mask && dev->bus_dma_mask < dma_mask)
> +		dma_mask = dev->bus_dma_mask;
> +

Similarly, can't we assume dma_mask to be nonzero here too? It feels 
like we really shouldn't have managed to get this far without one.

Robin.

>   	if (force_dma_unencrypted())
>   		*phys_mask = __dma_to_phys(dev, dma_mask);
>   	else
> @@ -87,7 +94,7 @@ static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
>   static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
>   {
>   	return phys_to_dma_direct(dev, phys) + size - 1 <=
> -			dev->coherent_dma_mask;
> +			min_not_zero(dev->coherent_dma_mask, dev->bus_dma_mask);
>   }
>   
>   void *dma_direct_alloc_pages(struct device *dev, size_t size,
> @@ -291,12 +298,6 @@ int dma_direct_supported(struct device *dev, u64 mask)
>   	if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
>   		return 0;
>   #endif
> -	/*
> -	 * Upstream PCI/PCIe bridges or SoC interconnects may not carry
> -	 * as many DMA address bits as the device itself supports.
> -	 */
> -	if (dev->bus_dma_mask && mask > dev->bus_dma_mask)
> -		return 0;
>   	return 1;
>   }
>   
> 

Re: [PATCH 4/5] dma-direct: implement complete bus_dma_mask handling

[Robin Murphy]

On 20/09/18 19:52, Christoph Hellwig wrote:
> Instead of rejecting devices with a too small bus_dma_mask we can handle
> by taking the bus dma_mask into account for allocations and bounce
> buffering decisions.
> 
> Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
> ---
>   include/linux/dma-direct.h |  3 ++-
>   kernel/dma/direct.c        | 21 +++++++++++----------
>   2 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index b79496d8c75b..fbca184ff5a0 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -27,7 +27,8 @@ static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
>   	if (!dev->dma_mask)
>   		return false;
>   
> -	return addr + size - 1 <= *dev->dma_mask;
> +	return addr + size - 1 <=
> +		min_not_zero(*dev->dma_mask, dev->bus_dma_mask);
>   }
>   #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>   
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 3c404e33d946..64466b7ef67b 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -43,10 +43,11 @@ check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
>   			return false;
>   		}
>   
> -		if (*dev->dma_mask >= DMA_BIT_MASK(32)) {
> +		if (*dev->dma_mask >= DMA_BIT_MASK(32) || dev->bus_dma_mask) {

Hmm... say *dev->dma_mask is 31 bits and dev->bus_dma_mask is 40 bits 
due to a global DT property, we'll now scream where we didn't before 
even though the bus mask is almost certainly irrelevant - is that desirable?

>   			dev_err(dev,
> -				"%s: overflow %pad+%zu of device mask %llx\n",
> -				caller, &dma_addr, size, *dev->dma_mask);
> +				"%s: overflow %pad+%zu of device mask %llx bus mask %llx\n",
> +				caller, &dma_addr, size,
> +				*dev->dma_mask, dev->bus_dma_mask);
>   		}
>   		return false;
>   	}
> @@ -65,12 +66,18 @@ u64 dma_direct_get_required_mask(struct device *dev)
>   {
>   	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
>   
> +	if (dev->bus_dma_mask && dev->bus_dma_mask < max_dma)
> +		max_dma = dev->bus_dma_mask;

Again, I think we could just do another min_not_zero() here. A device 
wired to address only one single page of RAM isn't a realistic prospect 
(and we could just flip the -1 and the shift in the max_dma calculation 
if we *really* wanted to support such things).

> +
>   	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
>   }
>   
>   static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
>   		u64 *phys_mask)
>   {
> +	if (dev->bus_dma_mask && dev->bus_dma_mask < dma_mask)
> +		dma_mask = dev->bus_dma_mask;
> +

Similarly, can't we assume dma_mask to be nonzero here too? It feels 
like we really shouldn't have managed to get this far without one.

Robin.

>   	if (force_dma_unencrypted())
>   		*phys_mask = __dma_to_phys(dev, dma_mask);
>   	else
> @@ -87,7 +94,7 @@ static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
>   static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
>   {
>   	return phys_to_dma_direct(dev, phys) + size - 1 <=
> -			dev->coherent_dma_mask;
> +			min_not_zero(dev->coherent_dma_mask, dev->bus_dma_mask);
>   }
>   
>   void *dma_direct_alloc_pages(struct device *dev, size_t size,
> @@ -291,12 +298,6 @@ int dma_direct_supported(struct device *dev, u64 mask)
>   	if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
>   		return 0;
>   #endif
> -	/*
> -	 * Upstream PCI/PCIe bridges or SoC interconnects may not carry
> -	 * as many DMA address bits as the device itself supports.
> -	 */
> -	if (dev->bus_dma_mask && mask > dev->bus_dma_mask)
> -		return 0;
>   	return 1;
>   }
>   
> 

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Robin Murphy]

[ oops, I should have looked at the replies first, now I see Ben already 
had the same thing to say about #3... ]

On 27/09/18 14:49, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 11:50:14AM +1000, Benjamin Herrenschmidt wrote:
>>> -	 * to be able to satisfy them - either by not supporting more physical
>>> -	 * memory, or by providing a ZONE_DMA32.  If neither is the case, the
>>> -	 * architecture needs to use an IOMMU instead of the direct mapping.
>>> -	 */
>>> -	if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
>>> +	u64 min_mask;
>>> +
>>> +	if (IS_ENABLED(CONFIG_ZONE_DMA))
>>> +		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
>>> +	else
>>> +		min_mask = min_t(u64, DMA_BIT_MASK(32),
>>> +				 (max_pfn - 1) << PAGE_SHIFT);
>>> +
>>> +	if (mask >= phys_to_dma(dev, min_mask))
>>>   		return 0;
>>
>> nitpick ... to be completely "correct", I would have written
>>
>> 	if (IS_ENABLED(CONFIG_ZONE_DMA))
>> 		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
>> 	else
>> 		min_mask = DMA_BIT_MASK(32);
>>
>> 	min_mask = min_t(u64, min_mask,	(max_pfn - 1) << PAGE_SHIFT);
>>
>> In "theory" it's also ok to have a mask < ZONE_DMA_BITS as long as it's
>> big enough to fit all memory :-)
> 
> Yeah, we could do that.

FWIW I like it even if just for looking slightly more readable. With 
that fixup,

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

Re: [PATCH 2/5] dma-direct: add an explicit dma_direct_get_required_mask

[Christoph Hellwig]

On Thu, Sep 27, 2018 at 03:12:25PM +0100, Robin Murphy wrote:
>> +u64 dma_direct_get_required_mask(struct device *dev)
>> +{
>> +	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
>> +
>> +	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
>
> I think that may as well just use __fls64() - it seems reasonable to assume 
> max_dma > 0. Otherwise,

Is there any good reason to micro-optimize given that this isn't
a fast path?

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Christoph Hellwig]

On Thu, Sep 27, 2018 at 03:30:20PM +0100, Robin Murphy wrote:
>>   +static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 
>> dma_mask,
>> +		u64 *phys_mask)
>> +{
>> +	if (force_dma_unencrypted())
>> +		*phys_mask = __dma_to_phys(dev, dma_mask);
>> +	else
>> +		*phys_mask = dma_to_phys(dev, dma_mask);
>
> Maybe make phys_to_dma_direct() take u64 instead of phys_addr_t so we can 
> reuse it here?

This is a dma_to_phys and not a phys_to_dma.

Re: [PATCH 4/5] dma-direct: implement complete bus_dma_mask handling

[Christoph Hellwig]

On Thu, Sep 27, 2018 at 03:58:04PM +0100, Robin Murphy wrote:
>>   }
>>   #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>>   diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index 3c404e33d946..64466b7ef67b 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -43,10 +43,11 @@ check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
>>   			return false;
>>   		}
>>   -		if (*dev->dma_mask >= DMA_BIT_MASK(32)) {
>> +		if (*dev->dma_mask >= DMA_BIT_MASK(32) || dev->bus_dma_mask) {
>
> Hmm... say *dev->dma_mask is 31 bits and dev->bus_dma_mask is 40 bits due 
> to a global DT property, we'll now scream where we didn't before even 
> though the bus mask is almost certainly irrelevant - is that desirable?

This is just the reporting in the error case - we'll only hit this
IFF dma_capable already returned false.  But if you don't want a message
here we can probably drop this hunk.

>> @@ -65,12 +66,18 @@ u64 dma_direct_get_required_mask(struct device *dev)
>>   {
>>   	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
>>   +	if (dev->bus_dma_mask && dev->bus_dma_mask < max_dma)
>> +		max_dma = dev->bus_dma_mask;
>
> Again, I think we could just do another min_not_zero() here. A device wired 
> to address only one single page of RAM isn't a realistic prospect (and we 
> could just flip the -1 and the shift in the max_dma calculation if we 
> *really* wanted to support such things).

This just seemed more readable to me than min_not_zero, but if others
prefer min_not_zero I can switch.

Re: [PATCH 2/5] dma-direct: add an explicit dma_direct_get_required_mask

[Robin Murphy]

On 27/09/18 16:28, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 03:12:25PM +0100, Robin Murphy wrote:
>>> +u64 dma_direct_get_required_mask(struct device *dev)
>>> +{
>>> +	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
>>> +
>>> +	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
>>
>> I think that may as well just use __fls64() - it seems reasonable to assume
>> max_dma > 0. Otherwise,
> 
> Is there any good reason to micro-optimize given that this isn't
> a fast path?

Not at all, I wasn't even thinking in terms of optimisation other than 
in terms of number of source characters and levels of parentheses.

But more importantly I was also being a big idiot because no matter how 
much I have the fls()/__fls() thing in mind, __fls64() doesn't actually 
exist. Nitpick rescinded!

Robin.

Re: [PATCH 2/5] dma-direct: add an explicit dma_direct_get_required_mask

[Robin Murphy]

On 27/09/18 16:28, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 03:12:25PM +0100, Robin Murphy wrote:
>>> +u64 dma_direct_get_required_mask(struct device *dev)
>>> +{
>>> +	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
>>> +
>>> +	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
>>
>> I think that may as well just use __fls64() - it seems reasonable to assume
>> max_dma > 0. Otherwise,
> 
> Is there any good reason to micro-optimize given that this isn't
> a fast path?

Not at all, I wasn't even thinking in terms of optimisation other than 
in terms of number of source characters and levels of parentheses.

But more importantly I was also being a big idiot because no matter how 
much I have the fls()/__fls() thing in mind, __fls64() doesn't actually 
exist. Nitpick rescinded!

Robin.

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Robin Murphy]

On 27/09/18 16:30, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 03:30:20PM +0100, Robin Murphy wrote:
>>>    +static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64
>>> dma_mask,
>>> +		u64 *phys_mask)
>>> +{
>>> +	if (force_dma_unencrypted())
>>> +		*phys_mask = __dma_to_phys(dev, dma_mask);
>>> +	else
>>> +		*phys_mask = dma_to_phys(dev, dma_mask);
>>
>> Maybe make phys_to_dma_direct() take u64 instead of phys_addr_t so we can
>> reuse it here?
> 
> This is a dma_to_phys and not a phys_to_dma.

Ugh, clearly it's time to stop reviewing patches for today... sorry :(

Robin.

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Robin Murphy]

On 27/09/18 16:30, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 03:30:20PM +0100, Robin Murphy wrote:
>>>    +static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64
>>> dma_mask,
>>> +		u64 *phys_mask)
>>> +{
>>> +	if (force_dma_unencrypted())
>>> +		*phys_mask = __dma_to_phys(dev, dma_mask);
>>> +	else
>>> +		*phys_mask = dma_to_phys(dev, dma_mask);
>>
>> Maybe make phys_to_dma_direct() take u64 instead of phys_addr_t so we can
>> reuse it here?
> 
> This is a dma_to_phys and not a phys_to_dma.

Ugh, clearly it's time to stop reviewing patches for today... sorry :(

Robin.

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Christoph Hellwig]

On Thu, Sep 27, 2018 at 04:38:31PM +0100, Robin Murphy wrote:
> On 27/09/18 16:30, Christoph Hellwig wrote:
>> On Thu, Sep 27, 2018 at 03:30:20PM +0100, Robin Murphy wrote:
>>>>    +static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64
>>>> dma_mask,
>>>> +		u64 *phys_mask)
>>>> +{
>>>> +	if (force_dma_unencrypted())
>>>> +		*phys_mask = __dma_to_phys(dev, dma_mask);
>>>> +	else
>>>> +		*phys_mask = dma_to_phys(dev, dma_mask);
>>>
>>> Maybe make phys_to_dma_direct() take u64 instead of phys_addr_t so we can
>>> reuse it here?
>>
>> This is a dma_to_phys and not a phys_to_dma.
>
> Ugh, clearly it's time to stop reviewing patches for today... sorry :(

I actually made the same mistake when writing it..

ALthough I'd really like to see some feedback from you on the arm64
swiotlb series once you had more cofee ;-)

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Christoph Hellwig]

On Thu, Sep 27, 2018 at 04:38:31PM +0100, Robin Murphy wrote:
> On 27/09/18 16:30, Christoph Hellwig wrote:
>> On Thu, Sep 27, 2018 at 03:30:20PM +0100, Robin Murphy wrote:
>>>>    +static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64
>>>> dma_mask,
>>>> +		u64 *phys_mask)
>>>> +{
>>>> +	if (force_dma_unencrypted())
>>>> +		*phys_mask = __dma_to_phys(dev, dma_mask);
>>>> +	else
>>>> +		*phys_mask = dma_to_phys(dev, dma_mask);
>>>
>>> Maybe make phys_to_dma_direct() take u64 instead of phys_addr_t so we can
>>> reuse it here?
>>
>> This is a dma_to_phys and not a phys_to_dma.
>
> Ugh, clearly it's time to stop reviewing patches for today... sorry :(

I actually made the same mistake when writing it..

ALthough I'd really like to see some feedback from you on the arm64
swiotlb series once you had more cofee ;-)

Re: [PATCH 4/5] dma-direct: implement complete bus_dma_mask handling

[Robin Murphy]

On 27/09/18 16:32, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 03:58:04PM +0100, Robin Murphy wrote:
>>>    }
>>>    #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>>>    diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>>> index 3c404e33d946..64466b7ef67b 100644
>>> --- a/kernel/dma/direct.c
>>> +++ b/kernel/dma/direct.c
>>> @@ -43,10 +43,11 @@ check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
>>>    			return false;
>>>    		}
>>>    -		if (*dev->dma_mask >= DMA_BIT_MASK(32)) {
>>> +		if (*dev->dma_mask >= DMA_BIT_MASK(32) || dev->bus_dma_mask) {
>>
>> Hmm... say *dev->dma_mask is 31 bits and dev->bus_dma_mask is 40 bits due
>> to a global DT property, we'll now scream where we didn't before even
>> though the bus mask is almost certainly irrelevant - is that desirable?
> 
> This is just the reporting in the error case - we'll only hit this
> IFF dma_capable already returned false.  But if you don't want a message
> here we can probably drop this hunk.

It was only a question of consistency - whatever the reason was to avoid 
warning for small device masks before, it's not obvious why it wouldn't 
still apply in the presence of nonzero but larger bus mask. The fact 
that the current check is there at all implied to me that we're 
expecting less-capable device to hit this often and thus wanted to avoid 
being noisy. If that's not the case then it's fine as-is AFAIC.

>>> @@ -65,12 +66,18 @@ u64 dma_direct_get_required_mask(struct device *dev)
>>>    {
>>>    	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
>>>    +	if (dev->bus_dma_mask && dev->bus_dma_mask < max_dma)
>>> +		max_dma = dev->bus_dma_mask;
>>
>> Again, I think we could just do another min_not_zero() here. A device wired
>> to address only one single page of RAM isn't a realistic prospect (and we
>> could just flip the -1 and the shift in the max_dma calculation if we
>> *really* wanted to support such things).
> 
> This just seemed more readable to me than min_not_zero, but if others
> prefer min_not_zero I can switch.

Nah, just checking whether there were any intentionally different 
assumptions compared to the couple of other places in the patch where 
min_not_zero() *is* used. If it's purely a style thing then no worries 
(personally I'd have written it yet another way anyway).

Robin.

Re: [PATCH 4/5] dma-direct: implement complete bus_dma_mask handling

[Robin Murphy]

On 27/09/18 16:32, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 03:58:04PM +0100, Robin Murphy wrote:
>>>    }
>>>    #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
>>>    diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>>> index 3c404e33d946..64466b7ef67b 100644
>>> --- a/kernel/dma/direct.c
>>> +++ b/kernel/dma/direct.c
>>> @@ -43,10 +43,11 @@ check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
>>>    			return false;
>>>    		}
>>>    -		if (*dev->dma_mask >= DMA_BIT_MASK(32)) {
>>> +		if (*dev->dma_mask >= DMA_BIT_MASK(32) || dev->bus_dma_mask) {
>>
>> Hmm... say *dev->dma_mask is 31 bits and dev->bus_dma_mask is 40 bits due
>> to a global DT property, we'll now scream where we didn't before even
>> though the bus mask is almost certainly irrelevant - is that desirable?
> 
> This is just the reporting in the error case - we'll only hit this
> IFF dma_capable already returned false.  But if you don't want a message
> here we can probably drop this hunk.

It was only a question of consistency - whatever the reason was to avoid 
warning for small device masks before, it's not obvious why it wouldn't 
still apply in the presence of nonzero but larger bus mask. The fact 
that the current check is there at all implied to me that we're 
expecting less-capable device to hit this often and thus wanted to avoid 
being noisy. If that's not the case then it's fine as-is AFAIC.

>>> @@ -65,12 +66,18 @@ u64 dma_direct_get_required_mask(struct device *dev)
>>>    {
>>>    	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
>>>    +	if (dev->bus_dma_mask && dev->bus_dma_mask < max_dma)
>>> +		max_dma = dev->bus_dma_mask;
>>
>> Again, I think we could just do another min_not_zero() here. A device wired
>> to address only one single page of RAM isn't a realistic prospect (and we
>> could just flip the -1 and the shift in the max_dma calculation if we
>> *really* wanted to support such things).
> 
> This just seemed more readable to me than min_not_zero, but if others
> prefer min_not_zero I can switch.

Nah, just checking whether there were any intentionally different 
assumptions compared to the couple of other places in the patch where 
min_not_zero() *is* used. If it's purely a style thing then no worries 
(personally I'd have written it yet another way anyway).

Robin.

Re: [PATCH 4/5] dma-direct: implement complete bus_dma_mask handling

[Christoph Hellwig]

On Thu, Sep 27, 2018 at 05:14:56PM +0100, Robin Murphy wrote:
>> This just seemed more readable to me than min_not_zero, but if others
>> prefer min_not_zero I can switch.
>
> Nah, just checking whether there were any intentionally different 
> assumptions compared to the couple of other places in the patch where 
> min_not_zero() *is* used. If it's purely a style thing then no worries 
> (personally I'd have written it yet another way anyway).

I'm curious: how would you have written it?

Re: [PATCH 4/5] dma-direct: implement complete bus_dma_mask handling

[Christoph Hellwig]

On Thu, Sep 27, 2018 at 05:14:56PM +0100, Robin Murphy wrote:
>> This just seemed more readable to me than min_not_zero, but if others
>> prefer min_not_zero I can switch.
>
> Nah, just checking whether there were any intentionally different 
> assumptions compared to the couple of other places in the patch where 
> min_not_zero() *is* used. If it's purely a style thing then no worries 
> (personally I'd have written it yet another way anyway).

I'm curious: how would you have written it?

Re: [PATCH 4/5] dma-direct: implement complete bus_dma_mask handling

[Christoph Hellwig]

On Thu, Sep 27, 2018 at 05:14:56PM +0100, Robin Murphy wrote:
>> This just seemed more readable to me than min_not_zero, but if others
>> prefer min_not_zero I can switch.
>
> Nah, just checking whether there were any intentionally different 
> assumptions compared to the couple of other places in the patch where 
> min_not_zero() *is* used. If it's purely a style thing then no worries 
> (personally I'd have written it yet another way anyway).

I'm curious: how would you have written it?

Re: [PATCH 4/5] dma-direct: implement complete bus_dma_mask handling

[Robin Murphy]

On 27/09/18 17:27, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 05:14:56PM +0100, Robin Murphy wrote:
>>> This just seemed more readable to me than min_not_zero, but if others
>>> prefer min_not_zero I can switch.
>>
>> Nah, just checking whether there were any intentionally different
>> assumptions compared to the couple of other places in the patch where
>> min_not_zero() *is* used. If it's purely a style thing then no worries
>> (personally I'd have written it yet another way anyway).
> 
> I'm curious: how would you have written it?

Come to think of it, I actually already have, in iommu-dma:

	if (dev->bus_dma_mask)
		mask &= dev->bus_dma_mask;

but of course it's not so pretty for those cases where you don't already 
have the local variable ready to go.

Robin.

Re: [PATCH 4/5] dma-direct: implement complete bus_dma_mask handling

[Robin Murphy]

On 27/09/18 17:27, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 05:14:56PM +0100, Robin Murphy wrote:
>>> This just seemed more readable to me than min_not_zero, but if others
>>> prefer min_not_zero I can switch.
>>
>> Nah, just checking whether there were any intentionally different
>> assumptions compared to the couple of other places in the patch where
>> min_not_zero() *is* used. If it's purely a style thing then no worries
>> (personally I'd have written it yet another way anyway).
> 
> I'm curious: how would you have written it?

Come to think of it, I actually already have, in iommu-dma:

	if (dev->bus_dma_mask)
		mask &= dev->bus_dma_mask;

but of course it's not so pretty for those cases where you don't already 
have the local variable ready to go.

Robin.

Re: [PATCH 4/5] dma-direct: implement complete bus_dma_mask handling

[Robin Murphy]

On 27/09/18 17:27, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 05:14:56PM +0100, Robin Murphy wrote:
>>> This just seemed more readable to me than min_not_zero, but if others
>>> prefer min_not_zero I can switch.
>>
>> Nah, just checking whether there were any intentionally different
>> assumptions compared to the couple of other places in the patch where
>> min_not_zero() *is* used. If it's purely a style thing then no worries
>> (personally I'd have written it yet another way anyway).
> 
> I'm curious: how would you have written it?

Come to think of it, I actually already have, in iommu-dma:

	if (dev->bus_dma_mask)
		mask &= dev->bus_dma_mask;

but of course it's not so pretty for those cases where you don't already 
have the local variable ready to go.

Robin.

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Benjamin Herrenschmidt]

On Thu, 2018-09-27 at 15:49 +0200, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 11:45:15AM +1000, Benjamin Herrenschmidt wrote:
> > I'm not sure this is entirely right.
> > 
> > Let's say the mask is 30 bits. You will return GFP_DMA32, which will
> > fail if you allocate something above 1G (which is legit for
> > ZONE_DMA32).
> 
> And then we will try GFP_DMA further down in the function:
> 
> 		if (IS_ENABLED(CONFIG_ZONE_DMA) &&
> 		    dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
> 		    !(gfp & GFP_DMA)) {
> 			gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
> 			goto again;
> 		}
> 
> This is and old optimization from x86, because chances are high that
> GFP_DMA32 will give you suitable memory for the infamous 31-bit
> dma mask devices (at least at boot time) and thus we don't have
> to deplete the tiny ZONE_DMA pool.

I see, it's rather confusing :-) Wouldn't it be better to check against
top of 32-bit memory instead here too ?


Cheers,
Ben.

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Benjamin Herrenschmidt]

On Thu, 2018-09-27 at 15:49 +0200, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 11:45:15AM +1000, Benjamin Herrenschmidt wrote:
> > I'm not sure this is entirely right.
> > 
> > Let's say the mask is 30 bits. You will return GFP_DMA32, which will
> > fail if you allocate something above 1G (which is legit for
> > ZONE_DMA32).
> 
> And then we will try GFP_DMA further down in the function:
> 
> 		if (IS_ENABLED(CONFIG_ZONE_DMA) &&
> 		    dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
> 		    !(gfp & GFP_DMA)) {
> 			gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
> 			goto again;
> 		}
> 
> This is and old optimization from x86, because chances are high that
> GFP_DMA32 will give you suitable memory for the infamous 31-bit
> dma mask devices (at least at boot time) and thus we don't have
> to deplete the tiny ZONE_DMA pool.

I see, it's rather confusing :-) Wouldn't it be better to check against
top of 32-bit memory instead here too ?


Cheers,
Ben.

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Benjamin Herrenschmidt]

On Thu, 2018-09-27 at 15:49 +0200, Christoph Hellwig wrote:
> On Thu, Sep 27, 2018 at 11:45:15AM +1000, Benjamin Herrenschmidt wrote:
> > I'm not sure this is entirely right.
> > 
> > Let's say the mask is 30 bits. You will return GFP_DMA32, which will
> > fail if you allocate something above 1G (which is legit for
> > ZONE_DMA32).
> 
> And then we will try GFP_DMA further down in the function:
> 
> 		if (IS_ENABLED(CONFIG_ZONE_DMA) &&
> 		    dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
> 		    !(gfp & GFP_DMA)) {
> 			gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
> 			goto again;
> 		}
> 
> This is and old optimization from x86, because chances are high that
> GFP_DMA32 will give you suitable memory for the infamous 31-bit
> dma mask devices (at least at boot time) and thus we don't have
> to deplete the tiny ZONE_DMA pool.

I see, it's rather confusing :-) Wouldn't it be better to check against
top of 32-bit memory instead here too ?


Cheers,
Ben.

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Christoph Hellwig]

On Fri, Sep 28, 2018 at 10:06:48AM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2018-09-27 at 15:49 +0200, Christoph Hellwig wrote:
> > On Thu, Sep 27, 2018 at 11:45:15AM +1000, Benjamin Herrenschmidt wrote:
> > > I'm not sure this is entirely right.
> > > 
> > > Let's say the mask is 30 bits. You will return GFP_DMA32, which will
> > > fail if you allocate something above 1G (which is legit for
> > > ZONE_DMA32).
> > 
> > And then we will try GFP_DMA further down in the function:
> > 
> > 		if (IS_ENABLED(CONFIG_ZONE_DMA) &&
> > 		    dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
> > 		    !(gfp & GFP_DMA)) {
> > 			gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
> > 			goto again;
> > 		}
> > 
> > This is and old optimization from x86, because chances are high that
> > GFP_DMA32 will give you suitable memory for the infamous 31-bit
> > dma mask devices (at least at boot time) and thus we don't have
> > to deplete the tiny ZONE_DMA pool.
> 
> I see, it's rather confusing :-) Wouldn't it be better to check against
> top of 32-bit memory instead here too ?

Where is here?  In __dma_direct_optimal_gfp_mask we already handled
it due to the optimistic zone selection we are discussing.

In the fallback quoted above there is no point for it, as with a
physical memory size smaller than ZONE_DMA32 (or ZONE_DMA for that matter)
we will have succeeded with the optimistic zone selection and not hit
the fallback path.

Either way this code probably needs much better comments.  I'll send
a patch on top of the recent series.

Re: [PATCH 3/5] dma-direct: refine dma_direct_alloc zone selection

[Christoph Hellwig]

On Fri, Sep 28, 2018 at 10:06:48AM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2018-09-27 at 15:49 +0200, Christoph Hellwig wrote:
> > On Thu, Sep 27, 2018 at 11:45:15AM +1000, Benjamin Herrenschmidt wrote:
> > > I'm not sure this is entirely right.
> > > 
> > > Let's say the mask is 30 bits. You will return GFP_DMA32, which will
> > > fail if you allocate something above 1G (which is legit for
> > > ZONE_DMA32).
> > 
> > And then we will try GFP_DMA further down in the function:
> > 
> > 		if (IS_ENABLED(CONFIG_ZONE_DMA) &&
> > 		    dev->coherent_dma_mask < DMA_BIT_MASK(32) &&
> > 		    !(gfp & GFP_DMA)) {
> > 			gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
> > 			goto again;
> > 		}
> > 
> > This is and old optimization from x86, because chances are high that
> > GFP_DMA32 will give you suitable memory for the infamous 31-bit
> > dma mask devices (at least at boot time) and thus we don't have
> > to deplete the tiny ZONE_DMA pool.
> 
> I see, it's rather confusing :-) Wouldn't it be better to check against
> top of 32-bit memory instead here too ?

Where is here?  In __dma_direct_optimal_gfp_mask we already handled
it due to the optimistic zone selection we are discussing.

In the fallback quoted above there is no point for it, as with a
physical memory size smaller than ZONE_DMA32 (or ZONE_DMA for that matter)
we will have succeeded with the optimistic zone selection and not hit
the fallback path.

Either way this code probably needs much better comments.  I'll send
a patch on top of the recent series.

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Ramon Fried]

On Mon, Nov 19, 2018 at 5:50 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 19/11/2018 14:18, Ramon Fried wrote:
> > On Tue, Oct 9, 2018 at 8:02 AM Benjamin Herrenschmidt
> > <benh@kernel.crashing.org> wrote:
> >>
> >> On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote:
> >>>> -        * Because 32-bit DMA masks are so common we expect every architecture
> >>>> -        * to be able to satisfy them - either by not supporting more physical
> >>>> -        * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> >>>> -        * architecture needs to use an IOMMU instead of the direct mapping.
> >>>> -        */
> >>>> -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> >>>> +       u64 min_mask;
> >>>> +
> >>>> +       if (IS_ENABLED(CONFIG_ZONE_DMA))
> >>>> +               min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> >>>> +       else
> >>>> +               min_mask = DMA_BIT_MASK(32);
> >>>> +
> >>>> +       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> >>>> +
> >>>> +       if (mask >= phys_to_dma(dev, min_mask))
> >>>>                   return 0;
> >>>> -#endif
> >>>>           return 1;
> >>>>    }
> >>>
> >>> So I believe I have run into the same issue that Guenter reported. On
> >>> an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
> >>> all probe attempts for various devices were failing with -EIO errors.
> >>>
> >>> I believe the last mask check should be "if (mask < phys_to_dma(dev,
> >>> min_mask))" not a ">=" check.
> >>
> >> Right, that test is backwards. I needed to change it here too (powermac
> >> with the rest of the powerpc series).
> >>
> >> Cheers,
> >> Ben.
> >>
> >>
> >
> > Hi, I'm working on a MIPS64 soc with PCIe root complex on it, and it
> > appears that this series of patches are causing all PCI drivers that
> > request 64bit mask to fail with -5.
> > It's broken in 4.19. However, I just checked, it working on master.
> > We may need to backport a couple of patches to 4.19. I'm not sure
> > though which patches should be backported as there were at least 10
> > patches resolving this dma_direct area recently.
> > Christoph, Robin.
> > Can we ask Greg to backport all these changes ? What do you think ?
>
> As far as I'm aware, the only real issue in 4.19 was my subtle breakage
> around setting bus_dma_mask - that's fixed by 6778be4e5209, which
> according to my inbox got picked up by autosel for 4.19 stable last week.
>
> Robin.
Yep, 6778be4e5209 fixes the issue.
Thanks a lot !
Ramon.

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Ramon Fried]

On Mon, Nov 19, 2018 at 5:50 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 19/11/2018 14:18, Ramon Fried wrote:
> > On Tue, Oct 9, 2018 at 8:02 AM Benjamin Herrenschmidt
> > <benh@kernel.crashing.org> wrote:
> >>
> >> On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote:
> >>>> -        * Because 32-bit DMA masks are so common we expect every architecture
> >>>> -        * to be able to satisfy them - either by not supporting more physical
> >>>> -        * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> >>>> -        * architecture needs to use an IOMMU instead of the direct mapping.
> >>>> -        */
> >>>> -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> >>>> +       u64 min_mask;
> >>>> +
> >>>> +       if (IS_ENABLED(CONFIG_ZONE_DMA))
> >>>> +               min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> >>>> +       else
> >>>> +               min_mask = DMA_BIT_MASK(32);
> >>>> +
> >>>> +       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> >>>> +
> >>>> +       if (mask >= phys_to_dma(dev, min_mask))
> >>>>                   return 0;
> >>>> -#endif
> >>>>           return 1;
> >>>>    }
> >>>
> >>> So I believe I have run into the same issue that Guenter reported. On
> >>> an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
> >>> all probe attempts for various devices were failing with -EIO errors.
> >>>
> >>> I believe the last mask check should be "if (mask < phys_to_dma(dev,
> >>> min_mask))" not a ">=" check.
> >>
> >> Right, that test is backwards. I needed to change it here too (powermac
> >> with the rest of the powerpc series).
> >>
> >> Cheers,
> >> Ben.
> >>
> >>
> >
> > Hi, I'm working on a MIPS64 soc with PCIe root complex on it, and it
> > appears that this series of patches are causing all PCI drivers that
> > request 64bit mask to fail with -5.
> > It's broken in 4.19. However, I just checked, it working on master.
> > We may need to backport a couple of patches to 4.19. I'm not sure
> > though which patches should be backported as there were at least 10
> > patches resolving this dma_direct area recently.
> > Christoph, Robin.
> > Can we ask Greg to backport all these changes ? What do you think ?
>
> As far as I'm aware, the only real issue in 4.19 was my subtle breakage
> around setting bus_dma_mask - that's fixed by 6778be4e5209, which
> according to my inbox got picked up by autosel for 4.19 stable last week.
>
> Robin.
Yep, 6778be4e5209 fixes the issue.
Thanks a lot !
Ramon.

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Robin Murphy]

On 19/11/2018 14:18, Ramon Fried wrote:
> On Tue, Oct 9, 2018 at 8:02 AM Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>>
>> On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote:
>>>> -        * Because 32-bit DMA masks are so common we expect every architecture
>>>> -        * to be able to satisfy them - either by not supporting more physical
>>>> -        * memory, or by providing a ZONE_DMA32.  If neither is the case, the
>>>> -        * architecture needs to use an IOMMU instead of the direct mapping.
>>>> -        */
>>>> -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
>>>> +       u64 min_mask;
>>>> +
>>>> +       if (IS_ENABLED(CONFIG_ZONE_DMA))
>>>> +               min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
>>>> +       else
>>>> +               min_mask = DMA_BIT_MASK(32);
>>>> +
>>>> +       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
>>>> +
>>>> +       if (mask >= phys_to_dma(dev, min_mask))
>>>>                   return 0;
>>>> -#endif
>>>>           return 1;
>>>>    }
>>>
>>> So I believe I have run into the same issue that Guenter reported. On
>>> an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
>>> all probe attempts for various devices were failing with -EIO errors.
>>>
>>> I believe the last mask check should be "if (mask < phys_to_dma(dev,
>>> min_mask))" not a ">=" check.
>>
>> Right, that test is backwards. I needed to change it here too (powermac
>> with the rest of the powerpc series).
>>
>> Cheers,
>> Ben.
>>
>>
> 
> Hi, I'm working on a MIPS64 soc with PCIe root complex on it, and it
> appears that this series of patches are causing all PCI drivers that
> request 64bit mask to fail with -5.
> It's broken in 4.19. However, I just checked, it working on master.
> We may need to backport a couple of patches to 4.19. I'm not sure
> though which patches should be backported as there were at least 10
> patches resolving this dma_direct area recently.
> Christoph, Robin.
> Can we ask Greg to backport all these changes ? What do you think ?

As far as I'm aware, the only real issue in 4.19 was my subtle breakage 
around setting bus_dma_mask - that's fixed by 6778be4e5209, which 
according to my inbox got picked up by autosel for 4.19 stable last week.

Robin.

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Robin Murphy]

On 19/11/2018 14:18, Ramon Fried wrote:
> On Tue, Oct 9, 2018 at 8:02 AM Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>>
>> On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote:
>>>> -        * Because 32-bit DMA masks are so common we expect every architecture
>>>> -        * to be able to satisfy them - either by not supporting more physical
>>>> -        * memory, or by providing a ZONE_DMA32.  If neither is the case, the
>>>> -        * architecture needs to use an IOMMU instead of the direct mapping.
>>>> -        */
>>>> -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
>>>> +       u64 min_mask;
>>>> +
>>>> +       if (IS_ENABLED(CONFIG_ZONE_DMA))
>>>> +               min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
>>>> +       else
>>>> +               min_mask = DMA_BIT_MASK(32);
>>>> +
>>>> +       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
>>>> +
>>>> +       if (mask >= phys_to_dma(dev, min_mask))
>>>>                   return 0;
>>>> -#endif
>>>>           return 1;
>>>>    }
>>>
>>> So I believe I have run into the same issue that Guenter reported. On
>>> an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
>>> all probe attempts for various devices were failing with -EIO errors.
>>>
>>> I believe the last mask check should be "if (mask < phys_to_dma(dev,
>>> min_mask))" not a ">=" check.
>>
>> Right, that test is backwards. I needed to change it here too (powermac
>> with the rest of the powerpc series).
>>
>> Cheers,
>> Ben.
>>
>>
> 
> Hi, I'm working on a MIPS64 soc with PCIe root complex on it, and it
> appears that this series of patches are causing all PCI drivers that
> request 64bit mask to fail with -5.
> It's broken in 4.19. However, I just checked, it working on master.
> We may need to backport a couple of patches to 4.19. I'm not sure
> though which patches should be backported as there were at least 10
> patches resolving this dma_direct area recently.
> Christoph, Robin.
> Can we ask Greg to backport all these changes ? What do you think ?

As far as I'm aware, the only real issue in 4.19 was my subtle breakage 
around setting bus_dma_mask - that's fixed by 6778be4e5209, which 
according to my inbox got picked up by autosel for 4.19 stable last week.

Robin.

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Ramon Fried]

On Tue, Oct 9, 2018 at 8:02 AM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote:
> > > -        * Because 32-bit DMA masks are so common we expect every architecture
> > > -        * to be able to satisfy them - either by not supporting more physical
> > > -        * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> > > -        * architecture needs to use an IOMMU instead of the direct mapping.
> > > -        */
> > > -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> > > +       u64 min_mask;
> > > +
> > > +       if (IS_ENABLED(CONFIG_ZONE_DMA))
> > > +               min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> > > +       else
> > > +               min_mask = DMA_BIT_MASK(32);
> > > +
> > > +       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> > > +
> > > +       if (mask >= phys_to_dma(dev, min_mask))
> > >                  return 0;
> > > -#endif
> > >          return 1;
> > >   }
> >
> > So I believe I have run into the same issue that Guenter reported. On
> > an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
> > all probe attempts for various devices were failing with -EIO errors.
> >
> > I believe the last mask check should be "if (mask < phys_to_dma(dev,
> > min_mask))" not a ">=" check.
>
> Right, that test is backwards. I needed to change it here too (powermac
> with the rest of the powerpc series).
>
> Cheers,
> Ben.
>
>

Hi, I'm working on a MIPS64 soc with PCIe root complex on it, and it
appears that this series of patches are causing all PCI drivers that
request 64bit mask to fail with -5.
It's broken in 4.19. However, I just checked, it working on master.
We may need to backport a couple of patches to 4.19. I'm not sure
though which patches should be backported as there were at least 10
patches resolving this dma_direct area recently.
Christoph, Robin.
Can we ask Greg to backport all these changes ? What do you think ?

Thanks,
Ramon.

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Ramon Fried]

On Tue, Oct 9, 2018 at 8:02 AM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote:
> > > -        * Because 32-bit DMA masks are so common we expect every architecture
> > > -        * to be able to satisfy them - either by not supporting more physical
> > > -        * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> > > -        * architecture needs to use an IOMMU instead of the direct mapping.
> > > -        */
> > > -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> > > +       u64 min_mask;
> > > +
> > > +       if (IS_ENABLED(CONFIG_ZONE_DMA))
> > > +               min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> > > +       else
> > > +               min_mask = DMA_BIT_MASK(32);
> > > +
> > > +       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> > > +
> > > +       if (mask >= phys_to_dma(dev, min_mask))
> > >                  return 0;
> > > -#endif
> > >          return 1;
> > >   }
> >
> > So I believe I have run into the same issue that Guenter reported. On
> > an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
> > all probe attempts for various devices were failing with -EIO errors.
> >
> > I believe the last mask check should be "if (mask < phys_to_dma(dev,
> > min_mask))" not a ">=" check.
>
> Right, that test is backwards. I needed to change it here too (powermac
> with the rest of the powerpc series).
>
> Cheers,
> Ben.
>
>

Hi, I'm working on a MIPS64 soc with PCIe root complex on it, and it
appears that this series of patches are causing all PCI drivers that
request 64bit mask to fail with -5.
It's broken in 4.19. However, I just checked, it working on master.
We may need to backport a couple of patches to 4.19. I'm not sure
though which patches should be backported as there were at least 10
patches resolving this dma_direct area recently.
Christoph, Robin.
Can we ask Greg to backport all these changes ? What do you think ?

Thanks,
Ramon.

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Benjamin Herrenschmidt]

On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote:
> > -        * Because 32-bit DMA masks are so common we expect every architecture
> > -        * to be able to satisfy them - either by not supporting more physical
> > -        * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> > -        * architecture needs to use an IOMMU instead of the direct mapping.
> > -        */
> > -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> > +       u64 min_mask;
> > +
> > +       if (IS_ENABLED(CONFIG_ZONE_DMA))
> > +               min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> > +       else
> > +               min_mask = DMA_BIT_MASK(32);
> > +
> > +       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> > +
> > +       if (mask >= phys_to_dma(dev, min_mask))
> >                  return 0;
> > -#endif
> >          return 1;
> >   }
> 
> So I believe I have run into the same issue that Guenter reported. On
> an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
> all probe attempts for various devices were failing with -EIO errors.
> 
> I believe the last mask check should be "if (mask < phys_to_dma(dev,
> min_mask))" not a ">=" check.

Right, that test is backwards. I needed to change it here too (powermac
with the rest of the powerpc series).

Cheers,
Ben.

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Benjamin Herrenschmidt]

On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote:
> > -        * Because 32-bit DMA masks are so common we expect every architecture
> > -        * to be able to satisfy them - either by not supporting more physical
> > -        * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> > -        * architecture needs to use an IOMMU instead of the direct mapping.
> > -        */
> > -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> > +       u64 min_mask;
> > +
> > +       if (IS_ENABLED(CONFIG_ZONE_DMA))
> > +               min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> > +       else
> > +               min_mask = DMA_BIT_MASK(32);
> > +
> > +       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> > +
> > +       if (mask >= phys_to_dma(dev, min_mask))
> >                  return 0;
> > -#endif
> >          return 1;
> >   }
> 
> So I believe I have run into the same issue that Guenter reported. On
> an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
> all probe attempts for various devices were failing with -EIO errors.
> 
> I believe the last mask check should be "if (mask < phys_to_dma(dev,
> min_mask))" not a ">=" check.

Right, that test is backwards. I needed to change it here too (powermac
with the rest of the powerpc series).

Cheers,
Ben.

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Benjamin Herrenschmidt]

On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote:
> > -        * Because 32-bit DMA masks are so common we expect every architecture
> > -        * to be able to satisfy them - either by not supporting more physical
> > -        * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> > -        * architecture needs to use an IOMMU instead of the direct mapping.
> > -        */
> > -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> > +       u64 min_mask;
> > +
> > +       if (IS_ENABLED(CONFIG_ZONE_DMA))
> > +               min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> > +       else
> > +               min_mask = DMA_BIT_MASK(32);
> > +
> > +       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> > +
> > +       if (mask >= phys_to_dma(dev, min_mask))
> >                  return 0;
> > -#endif
> >          return 1;
> >   }
> 
> So I believe I have run into the same issue that Guenter reported. On
> an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
> all probe attempts for various devices were failing with -EIO errors.
> 
> I believe the last mask check should be "if (mask < phys_to_dma(dev,
> min_mask))" not a ">=" check.

Right, that test is backwards. I needed to change it here too (powermac
with the rest of the powerpc series).

Cheers,
Ben.

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Alexander Duyck]

On Thu, Sep 27, 2018 at 3:38 PM Christoph Hellwig <hch@lst.de> wrote:
>
> This way an architecture with less than 4G of RAM can support dma_mask
> smaller than 32-bit without a ZONE_DMA.  Apparently that is a common
> case on powerpc.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  kernel/dma/direct.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 60c433b880e0..170bd322a94a 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -284,21 +284,25 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
>         return nents;
>  }
>
> +/*
> + * Because 32-bit DMA masks are so common we expect every architecture to be
> + * able to satisfy them - either by not supporting more physical memory, or by
> + * providing a ZONE_DMA32.  If neither is the case, the architecture needs to
> + * use an IOMMU instead of the direct mapping.
> + */
>  int dma_direct_supported(struct device *dev, u64 mask)
>  {
> -#ifdef CONFIG_ZONE_DMA
> -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
> -               return 0;
> -#else
> -       /*
> -        * Because 32-bit DMA masks are so common we expect every architecture
> -        * to be able to satisfy them - either by not supporting more physical
> -        * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> -        * architecture needs to use an IOMMU instead of the direct mapping.
> -        */
> -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> +       u64 min_mask;
> +
> +       if (IS_ENABLED(CONFIG_ZONE_DMA))
> +               min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> +       else
> +               min_mask = DMA_BIT_MASK(32);
> +
> +       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> +
> +       if (mask >= phys_to_dma(dev, min_mask))
>                 return 0;
> -#endif
>         return 1;
>  }

So I believe I have run into the same issue that Guenter reported. On
an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
all probe attempts for various devices were failing with -EIO errors.

I believe the last mask check should be "if (mask < phys_to_dma(dev,
min_mask))" not a ">=" check.

- Alex

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Alexander Duyck]

On Thu, Sep 27, 2018 at 3:38 PM Christoph Hellwig <hch@lst.de> wrote:
>
> This way an architecture with less than 4G of RAM can support dma_mask
> smaller than 32-bit without a ZONE_DMA.  Apparently that is a common
> case on powerpc.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  kernel/dma/direct.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 60c433b880e0..170bd322a94a 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -284,21 +284,25 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
>         return nents;
>  }
>
> +/*
> + * Because 32-bit DMA masks are so common we expect every architecture to be
> + * able to satisfy them - either by not supporting more physical memory, or by
> + * providing a ZONE_DMA32.  If neither is the case, the architecture needs to
> + * use an IOMMU instead of the direct mapping.
> + */
>  int dma_direct_supported(struct device *dev, u64 mask)
>  {
> -#ifdef CONFIG_ZONE_DMA
> -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
> -               return 0;
> -#else
> -       /*
> -        * Because 32-bit DMA masks are so common we expect every architecture
> -        * to be able to satisfy them - either by not supporting more physical
> -        * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> -        * architecture needs to use an IOMMU instead of the direct mapping.
> -        */
> -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> +       u64 min_mask;
> +
> +       if (IS_ENABLED(CONFIG_ZONE_DMA))
> +               min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> +       else
> +               min_mask = DMA_BIT_MASK(32);
> +
> +       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> +
> +       if (mask >= phys_to_dma(dev, min_mask))
>                 return 0;
> -#endif
>         return 1;
>  }

So I believe I have run into the same issue that Guenter reported. On
an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
all probe attempts for various devices were failing with -EIO errors.

I believe the last mask check should be "if (mask < phys_to_dma(dev,
min_mask))" not a ">=" check.

- Alex

Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Alexander Duyck]

On Thu, Sep 27, 2018 at 3:38 PM Christoph Hellwig <hch@lst.de> wrote:
>
> This way an architecture with less than 4G of RAM can support dma_mask
> smaller than 32-bit without a ZONE_DMA.  Apparently that is a common
> case on powerpc.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  kernel/dma/direct.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 60c433b880e0..170bd322a94a 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -284,21 +284,25 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
>         return nents;
>  }
>
> +/*
> + * Because 32-bit DMA masks are so common we expect every architecture to be
> + * able to satisfy them - either by not supporting more physical memory, or by
> + * providing a ZONE_DMA32.  If neither is the case, the architecture needs to
> + * use an IOMMU instead of the direct mapping.
> + */
>  int dma_direct_supported(struct device *dev, u64 mask)
>  {
> -#ifdef CONFIG_ZONE_DMA
> -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
> -               return 0;
> -#else
> -       /*
> -        * Because 32-bit DMA masks are so common we expect every architecture
> -        * to be able to satisfy them - either by not supporting more physical
> -        * memory, or by providing a ZONE_DMA32.  If neither is the case, the
> -        * architecture needs to use an IOMMU instead of the direct mapping.
> -        */
> -       if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> +       u64 min_mask;
> +
> +       if (IS_ENABLED(CONFIG_ZONE_DMA))
> +               min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> +       else
> +               min_mask = DMA_BIT_MASK(32);
> +
> +       min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> +
> +       if (mask >= phys_to_dma(dev, min_mask))
>                 return 0;
> -#endif
>         return 1;
>  }

So I believe I have run into the same issue that Guenter reported. On
an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
all probe attempts for various devices were failing with -EIO errors.

I believe the last mask check should be "if (mask < phys_to_dma(dev,
min_mask))" not a ">=" check.

- Alex

[PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Christoph Hellwig]

This way an architecture with less than 4G of RAM can support dma_mask
smaller than 32-bit without a ZONE_DMA.  Apparently that is a common
case on powerpc.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
---
 kernel/dma/direct.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 60c433b880e0..170bd322a94a 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -284,21 +284,25 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
 	return nents;
 }
 
+/*
+ * Because 32-bit DMA masks are so common we expect every architecture to be
+ * able to satisfy them - either by not supporting more physical memory, or by
+ * providing a ZONE_DMA32.  If neither is the case, the architecture needs to
+ * use an IOMMU instead of the direct mapping.
+ */
 int dma_direct_supported(struct device *dev, u64 mask)
 {
-#ifdef CONFIG_ZONE_DMA
-	if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
-		return 0;
-#else
-	/*
-	 * Because 32-bit DMA masks are so common we expect every architecture
-	 * to be able to satisfy them - either by not supporting more physical
-	 * memory, or by providing a ZONE_DMA32.  If neither is the case, the
-	 * architecture needs to use an IOMMU instead of the direct mapping.
-	 */
-	if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
+	u64 min_mask;
+
+	if (IS_ENABLED(CONFIG_ZONE_DMA))
+		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
+	else
+		min_mask = DMA_BIT_MASK(32);
+
+	min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
+
+	if (mask >= phys_to_dma(dev, min_mask))
 		return 0;
-#endif
 	return 1;
 }
 
-- 
2.19.0

[PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Christoph Hellwig]

This way an architecture with less than 4G of RAM can support dma_mask
smaller than 32-bit without a ZONE_DMA.  Apparently that is a common
case on powerpc.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
---
 kernel/dma/direct.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 60c433b880e0..170bd322a94a 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -284,21 +284,25 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
 	return nents;
 }
 
+/*
+ * Because 32-bit DMA masks are so common we expect every architecture to be
+ * able to satisfy them - either by not supporting more physical memory, or by
+ * providing a ZONE_DMA32.  If neither is the case, the architecture needs to
+ * use an IOMMU instead of the direct mapping.
+ */
 int dma_direct_supported(struct device *dev, u64 mask)
 {
-#ifdef CONFIG_ZONE_DMA
-	if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
-		return 0;
-#else
-	/*
-	 * Because 32-bit DMA masks are so common we expect every architecture
-	 * to be able to satisfy them - either by not supporting more physical
-	 * memory, or by providing a ZONE_DMA32.  If neither is the case, the
-	 * architecture needs to use an IOMMU instead of the direct mapping.
-	 */
-	if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
+	u64 min_mask;
+
+	if (IS_ENABLED(CONFIG_ZONE_DMA))
+		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
+	else
+		min_mask = DMA_BIT_MASK(32);
+
+	min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
+
+	if (mask >= phys_to_dma(dev, min_mask))
 		return 0;
-#endif
 	return 1;
 }
 
-- 
2.19.0

[PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size

[Christoph Hellwig]

This way an architecture with less than 4G of RAM can support dma_mask
smaller than 32-bit without a ZONE_DMA.  Apparently that is a common
case on powerpc.

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Reviewed-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
---
 kernel/dma/direct.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 60c433b880e0..170bd322a94a 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -284,21 +284,25 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
 	return nents;
 }
 
+/*
+ * Because 32-bit DMA masks are so common we expect every architecture to be
+ * able to satisfy them - either by not supporting more physical memory, or by
+ * providing a ZONE_DMA32.  If neither is the case, the architecture needs to
+ * use an IOMMU instead of the direct mapping.
+ */
 int dma_direct_supported(struct device *dev, u64 mask)
 {
-#ifdef CONFIG_ZONE_DMA
-	if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
-		return 0;
-#else
-	/*
-	 * Because 32-bit DMA masks are so common we expect every architecture
-	 * to be able to satisfy them - either by not supporting more physical
-	 * memory, or by providing a ZONE_DMA32.  If neither is the case, the
-	 * architecture needs to use an IOMMU instead of the direct mapping.
-	 */
-	if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
+	u64 min_mask;
+
+	if (IS_ENABLED(CONFIG_ZONE_DMA))
+		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
+	else
+		min_mask = DMA_BIT_MASK(32);
+
+	min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
+
+	if (mask >= phys_to_dma(dev, min_mask))
 		return 0;
-#endif
 	return 1;
 }
 
-- 
2.19.0