linux-parisc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* remove the ->mapping_error method from dma_map_ops V2
@ 2018-11-22 14:02 Christoph Hellwig
  2018-11-22 14:02 ` [PATCH 01/24] dma-direct: Make DIRECT_MAPPING_ERROR viable for SWIOTLB Christoph Hellwig
                   ` (24 more replies)
  0 siblings, 25 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:02 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Error reporting for the dma_map_single and dma_map_page operations is
currently a mess.  Both APIs directly return the dma_addr_t to be used for
the DMA, with a magic error escape that is specific to the instance and
checked by another method provided.  This has a few downsides:

 - the error check is easily forgotten and a __must_check marker doesn't
   help as the value always is consumed anyway
 - the error checking requires another indirect call, which have gotten
   incredibly expensive
 - a lot of code is wasted on implementing these methods

The historical reason for this is that people thought DMA mappings would
not fail when the API was created, which sounds like a really bad
assumption in retrospective, and then we tried to cram error handling
onto it later on.

There basically are two variants:  the error code is 0 because the
implementation will never return 0 as a valid DMA address, or the error
code is all-F as the implementation won't ever return an address that
high.  The old AMD GART is the only one not falling into these two camps
as it picks sort of a relative zero relative to where it is mapped.

The 0 return doesn't work for direct mappings that have ram at address
zero and a lot of IOMMUs that start allocating bus space from address
zero, so we can't consolidate on that, but I think we can move everyone
to all-Fs, which the patch here does.  The reason for that is that
there is only one way to ever get this address: by doing a 1-byte long,
1-byte aligned mapping, but all our mappings are not only longer but
generally aligned, and the mappings have to keep at least the basic
alignment.

Note that the first two patches are queued up for 4.20 and included
for reference.

A git tree is also available here:

    git://git.infradead.org/users/hch/misc.git dma-mapping-error.2

Gitweb:

    http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-mapping-error.2

Changes since v1:
 - dropped the signature change
 - split into multiple patches
 - fixed the iova allocator return check in amd-iommu
 - remove EMERGENCY_PAGES in amd_gart and calgary

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

* [PATCH 01/24] dma-direct: Make DIRECT_MAPPING_ERROR viable for SWIOTLB
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
@ 2018-11-22 14:02 ` Christoph Hellwig
  2018-11-22 14:02 ` [PATCH 02/24] swiotlb: Skip cache maintenance on map error Christoph Hellwig
                   ` (23 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:02 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

From: Robin Murphy <robin.murphy@arm.com>

With the overflow buffer removed, we no longer have a unique address
which is guaranteed not to be a valid DMA target to use as an error
token. The DIRECT_MAPPING_ERROR value of 0 tries to at least represent
an unlikely DMA target, but unfortunately there are already SWIOTLB
users with DMA-able memory at physical address 0 which now gets falsely
treated as a mapping failure and leads to all manner of misbehaviour.

The best we can do to mitigate that is flip DIRECT_MAPPING_ERROR to the
other commonly-used error value of all-bits-set, since the last single
byte of memory is by far the least-likely-valid DMA target.

Fixes: dff8d6c1ed58 ("swiotlb: remove the overflow buffer")
Reported-by: John Stultz <john.stultz@linaro.org>
Tested-by: John Stultz <john.stultz@linaro.org>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/dma-direct.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index bd73e7a91410..9e66bfe369aa 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -5,7 +5,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/mem_encrypt.h>
 
-#define DIRECT_MAPPING_ERROR		0
+#define DIRECT_MAPPING_ERROR		(~(dma_addr_t)0)
 
 #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
 #include <asm/dma-direct.h>
-- 
2.19.1


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

* [PATCH 02/24] swiotlb: Skip cache maintenance on map error
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
  2018-11-22 14:02 ` [PATCH 01/24] dma-direct: Make DIRECT_MAPPING_ERROR viable for SWIOTLB Christoph Hellwig
@ 2018-11-22 14:02 ` Christoph Hellwig
  2018-11-22 14:02 ` [PATCH 03/24] dma-mapping: provide a generic DMA_MAPPING_ERROR Christoph Hellwig
                   ` (22 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:02 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

From: Robin Murphy <robin.murphy@arm.com>

If swiotlb_bounce_page() failed, calling arch_sync_dma_for_device() may
lead to such delights as performing cache maintenance on whatever
address phys_to_virt(SWIOTLB_MAP_ERROR) looks like, which is typically
outside the kernel memory map and goes about as well as expected.

Don't do that.

Fixes: a4a4330db46a ("swiotlb: add support for non-coherent DMA")
Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 kernel/dma/swiotlb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 5731daa09a32..045930e32c0e 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -679,7 +679,8 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 	}
 
 	if (!dev_is_dma_coherent(dev) &&
-	    (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
+	    (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0 &&
+	    dev_addr != DIRECT_MAPPING_ERROR)
 		arch_sync_dma_for_device(dev, phys, size, dir);
 
 	return dev_addr;
-- 
2.19.1


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

* [PATCH 03/24] dma-mapping: provide a generic DMA_MAPPING_ERROR
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
  2018-11-22 14:02 ` [PATCH 01/24] dma-direct: Make DIRECT_MAPPING_ERROR viable for SWIOTLB Christoph Hellwig
  2018-11-22 14:02 ` [PATCH 02/24] swiotlb: Skip cache maintenance on map error Christoph Hellwig
@ 2018-11-22 14:02 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 04/24] dma-direct: remove the mapping_error dma_map_ops method Christoph Hellwig
                   ` (21 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:02 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Error handling of the dma_map_single and dma_map_page APIs is a little
problematic at the moment, in that we use different encodings in the
returned dma_addr_t to indicate an error.  That means we require an
additional indirect call to figure out if a dma mapping call returned
an error, and a lot of boilerplate code to implement these semantics.

Instead return the maximum addressable value as the error.  As long
as we don't allow mapping single-byte ranges with single-byte alignment
this value can never be a valid return.  Additionaly if drivers do
not check the return value from the dma_map* routines this values means
they will generally not be pointed to actual memory.

Once the default value is added here we can start removing the
various mapping_error methods and just rely on this generic check.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/dma-mapping.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 15bd41447025..c323c539b7cb 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -133,6 +133,8 @@ struct dma_map_ops {
 	u64 (*get_required_mask)(struct device *dev);
 };
 
+#define DMA_MAPPING_ERROR		(~(dma_addr_t)0)
+
 extern const struct dma_map_ops dma_direct_ops;
 extern const struct dma_map_ops dma_virt_ops;
 
@@ -576,6 +578,10 @@ static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
 	const struct dma_map_ops *ops = get_dma_ops(dev);
 
 	debug_dma_mapping_error(dev, dma_addr);
+
+	if (dma_addr == DMA_MAPPING_ERROR)
+		return 1;
+
 	if (ops->mapping_error)
 		return ops->mapping_error(dev, dma_addr);
 	return 0;
-- 
2.19.1


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

* [PATCH 04/24] dma-direct: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2018-11-22 14:02 ` [PATCH 03/24] dma-mapping: provide a generic DMA_MAPPING_ERROR Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 05/24] arm: " Christoph Hellwig
                   ` (20 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

The dma-direct code already returns (~(dma_addr_t)0x0) on mapping
failures, so we can switch over to returning DMA_MAPPING_ERROR and let
the core dma-mapping code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/kernel/dma-swiotlb.c |  1 -
 include/linux/dma-direct.h        |  3 ---
 kernel/dma/direct.c               |  8 +-------
 kernel/dma/swiotlb.c              | 11 +++++------
 4 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-swiotlb.c
index 5fc335f4d9cd..3d8df2cf8be9 100644
--- a/arch/powerpc/kernel/dma-swiotlb.c
+++ b/arch/powerpc/kernel/dma-swiotlb.c
@@ -59,7 +59,6 @@ const struct dma_map_ops powerpc_swiotlb_dma_ops = {
 	.sync_single_for_device = swiotlb_sync_single_for_device,
 	.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
 	.sync_sg_for_device = swiotlb_sync_sg_for_device,
-	.mapping_error = dma_direct_mapping_error,
 	.get_required_mask = swiotlb_powerpc_get_required,
 };
 
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 9e66bfe369aa..e7600f92d876 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -5,8 +5,6 @@
 #include <linux/dma-mapping.h>
 #include <linux/mem_encrypt.h>
 
-#define DIRECT_MAPPING_ERROR		(~(dma_addr_t)0)
-
 #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
 #include <asm/dma-direct.h>
 #else
@@ -73,5 +71,4 @@ dma_addr_t dma_direct_map_page(struct device *dev, struct page *page,
 int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
 		enum dma_data_direction dir, unsigned long attrs);
 int dma_direct_supported(struct device *dev, u64 mask);
-int dma_direct_mapping_error(struct device *dev, dma_addr_t dma_addr);
 #endif /* _LINUX_DMA_DIRECT_H */
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 22a12ab5a5e9..d4335a03193a 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -265,7 +265,7 @@ dma_addr_t dma_direct_map_page(struct device *dev, struct page *page,
 	dma_addr_t dma_addr = phys_to_dma(dev, phys);
 
 	if (!check_addr(dev, dma_addr, size, __func__))
-		return DIRECT_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 
 	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
 		dma_direct_sync_single_for_device(dev, dma_addr, size, dir);
@@ -312,11 +312,6 @@ int dma_direct_supported(struct device *dev, u64 mask)
 	return mask >= phys_to_dma(dev, min_mask);
 }
 
-int dma_direct_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == DIRECT_MAPPING_ERROR;
-}
-
 const struct dma_map_ops dma_direct_ops = {
 	.alloc			= dma_direct_alloc,
 	.free			= dma_direct_free,
@@ -335,7 +330,6 @@ const struct dma_map_ops dma_direct_ops = {
 #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,
 };
 EXPORT_SYMBOL(dma_direct_ops);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 045930e32c0e..ff1ce81bb623 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -631,21 +631,21 @@ static dma_addr_t swiotlb_bounce_page(struct device *dev, phys_addr_t *phys,
 	if (unlikely(swiotlb_force == SWIOTLB_NO_FORCE)) {
 		dev_warn_ratelimited(dev,
 			"Cannot do DMA to address %pa\n", phys);
-		return DIRECT_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 	}
 
 	/* Oh well, have to allocate and map a bounce buffer. */
 	*phys = swiotlb_tbl_map_single(dev, __phys_to_dma(dev, io_tlb_start),
 			*phys, size, dir, attrs);
 	if (*phys == SWIOTLB_MAP_ERROR)
-		return DIRECT_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 
 	/* Ensure that the address returned is DMA'ble */
 	dma_addr = __phys_to_dma(dev, *phys);
 	if (unlikely(!dma_capable(dev, dma_addr, size))) {
 		swiotlb_tbl_unmap_single(dev, *phys, size, dir,
 			attrs | DMA_ATTR_SKIP_CPU_SYNC);
-		return DIRECT_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 	}
 
 	return dma_addr;
@@ -680,7 +680,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
 
 	if (!dev_is_dma_coherent(dev) &&
 	    (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0 &&
-	    dev_addr != DIRECT_MAPPING_ERROR)
+	    dev_addr != DMA_MAPPING_ERROR)
 		arch_sync_dma_for_device(dev, phys, size, dir);
 
 	return dev_addr;
@@ -789,7 +789,7 @@ swiotlb_map_sg_attrs(struct device *dev, struct scatterlist *sgl, int nelems,
 	for_each_sg(sgl, sg, nelems, i) {
 		sg->dma_address = swiotlb_map_page(dev, sg_page(sg), sg->offset,
 				sg->length, dir, attrs);
-		if (sg->dma_address == DIRECT_MAPPING_ERROR)
+		if (sg->dma_address == DMA_MAPPING_ERROR)
 			goto out_error;
 		sg_dma_len(sg) = sg->length;
 	}
@@ -869,7 +869,6 @@ swiotlb_dma_supported(struct device *hwdev, u64 mask)
 }
 
 const struct dma_map_ops swiotlb_dma_ops = {
-	.mapping_error		= dma_direct_mapping_error,
 	.alloc			= dma_direct_alloc,
 	.free			= dma_direct_free,
 	.sync_single_for_cpu	= swiotlb_sync_single_for_cpu,
-- 
2.19.1


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

* [PATCH 05/24] arm: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 04/24] dma-direct: remove the mapping_error dma_map_ops method Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 06/24] powerpc/iommu: " Christoph Hellwig
                   ` (19 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Arm already returns (~(dma_addr_t)0x0) on mapping failures, so we can
switch over to returning DMA_MAPPING_ERROR and let the core dma-mapping
code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/arm/common/dmabounce.c      | 12 +++-------
 arch/arm/include/asm/dma-iommu.h |  2 --
 arch/arm/mm/dma-mapping.c        | 39 ++++++++++++--------------------
 3 files changed, 18 insertions(+), 35 deletions(-)

diff --git a/arch/arm/common/dmabounce.c b/arch/arm/common/dmabounce.c
index 9a92de63426f..5ba4622030ca 100644
--- a/arch/arm/common/dmabounce.c
+++ b/arch/arm/common/dmabounce.c
@@ -257,7 +257,7 @@ static inline dma_addr_t map_single(struct device *dev, void *ptr, size_t size,
 	if (buf == NULL) {
 		dev_err(dev, "%s: unable to map unsafe buffer %p!\n",
 		       __func__, ptr);
-		return ARM_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 	}
 
 	dev_dbg(dev, "%s: unsafe buffer %p (dma=%#x) mapped to %p (dma=%#x)\n",
@@ -327,7 +327,7 @@ static dma_addr_t dmabounce_map_page(struct device *dev, struct page *page,
 
 	ret = needs_bounce(dev, dma_addr, size);
 	if (ret < 0)
-		return ARM_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 
 	if (ret == 0) {
 		arm_dma_ops.sync_single_for_device(dev, dma_addr, size, dir);
@@ -336,7 +336,7 @@ static dma_addr_t dmabounce_map_page(struct device *dev, struct page *page,
 
 	if (PageHighMem(page)) {
 		dev_err(dev, "DMA buffer bouncing of HIGHMEM pages is not supported\n");
-		return ARM_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 	}
 
 	return map_single(dev, page_address(page) + offset, size, dir, attrs);
@@ -453,11 +453,6 @@ static int dmabounce_dma_supported(struct device *dev, u64 dma_mask)
 	return arm_dma_ops.dma_supported(dev, dma_mask);
 }
 
-static int dmabounce_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return arm_dma_ops.mapping_error(dev, dma_addr);
-}
-
 static const struct dma_map_ops dmabounce_ops = {
 	.alloc			= arm_dma_alloc,
 	.free			= arm_dma_free,
@@ -472,7 +467,6 @@ static const struct dma_map_ops dmabounce_ops = {
 	.sync_sg_for_cpu	= arm_dma_sync_sg_for_cpu,
 	.sync_sg_for_device	= arm_dma_sync_sg_for_device,
 	.dma_supported		= dmabounce_dma_supported,
-	.mapping_error		= dmabounce_mapping_error,
 };
 
 static int dmabounce_init_pool(struct dmabounce_pool *pool, struct device *dev,
diff --git a/arch/arm/include/asm/dma-iommu.h b/arch/arm/include/asm/dma-iommu.h
index 6821f1249300..772f48ef84b7 100644
--- a/arch/arm/include/asm/dma-iommu.h
+++ b/arch/arm/include/asm/dma-iommu.h
@@ -9,8 +9,6 @@
 #include <linux/dma-debug.h>
 #include <linux/kref.h>
 
-#define ARM_MAPPING_ERROR		(~(dma_addr_t)0x0)
-
 struct dma_iommu_mapping {
 	/* iommu specific data */
 	struct iommu_domain	*domain;
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 661fe48ab78d..2cfb17bad1e6 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -179,11 +179,6 @@ static void arm_dma_sync_single_for_device(struct device *dev,
 	__dma_page_cpu_to_dev(page, offset, size, dir);
 }
 
-static int arm_dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == ARM_MAPPING_ERROR;
-}
-
 const struct dma_map_ops arm_dma_ops = {
 	.alloc			= arm_dma_alloc,
 	.free			= arm_dma_free,
@@ -197,7 +192,6 @@ const struct dma_map_ops arm_dma_ops = {
 	.sync_single_for_device	= arm_dma_sync_single_for_device,
 	.sync_sg_for_cpu	= arm_dma_sync_sg_for_cpu,
 	.sync_sg_for_device	= arm_dma_sync_sg_for_device,
-	.mapping_error		= arm_dma_mapping_error,
 	.dma_supported		= arm_dma_supported,
 };
 EXPORT_SYMBOL(arm_dma_ops);
@@ -217,7 +211,6 @@ const struct dma_map_ops arm_coherent_dma_ops = {
 	.get_sgtable		= arm_dma_get_sgtable,
 	.map_page		= arm_coherent_dma_map_page,
 	.map_sg			= arm_dma_map_sg,
-	.mapping_error		= arm_dma_mapping_error,
 	.dma_supported		= arm_dma_supported,
 };
 EXPORT_SYMBOL(arm_coherent_dma_ops);
@@ -774,7 +767,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
 	gfp &= ~(__GFP_COMP);
 	args.gfp = gfp;
 
-	*handle = ARM_MAPPING_ERROR;
+	*handle = DMA_MAPPING_ERROR;
 	allowblock = gfpflags_allow_blocking(gfp);
 	cma = allowblock ? dev_get_cma_area(dev) : false;
 
@@ -1217,7 +1210,7 @@ static inline dma_addr_t __alloc_iova(struct dma_iommu_mapping *mapping,
 	if (i == mapping->nr_bitmaps) {
 		if (extend_iommu_mapping(mapping)) {
 			spin_unlock_irqrestore(&mapping->lock, flags);
-			return ARM_MAPPING_ERROR;
+			return DMA_MAPPING_ERROR;
 		}
 
 		start = bitmap_find_next_zero_area(mapping->bitmaps[i],
@@ -1225,7 +1218,7 @@ static inline dma_addr_t __alloc_iova(struct dma_iommu_mapping *mapping,
 
 		if (start > mapping->bits) {
 			spin_unlock_irqrestore(&mapping->lock, flags);
-			return ARM_MAPPING_ERROR;
+			return DMA_MAPPING_ERROR;
 		}
 
 		bitmap_set(mapping->bitmaps[i], start, count);
@@ -1409,7 +1402,7 @@ __iommu_create_mapping(struct device *dev, struct page **pages, size_t size,
 	int i;
 
 	dma_addr = __alloc_iova(mapping, size);
-	if (dma_addr == ARM_MAPPING_ERROR)
+	if (dma_addr == DMA_MAPPING_ERROR)
 		return dma_addr;
 
 	iova = dma_addr;
@@ -1436,7 +1429,7 @@ __iommu_create_mapping(struct device *dev, struct page **pages, size_t size,
 fail:
 	iommu_unmap(mapping->domain, dma_addr, iova-dma_addr);
 	__free_iova(mapping, dma_addr, size);
-	return ARM_MAPPING_ERROR;
+	return DMA_MAPPING_ERROR;
 }
 
 static int __iommu_remove_mapping(struct device *dev, dma_addr_t iova, size_t size)
@@ -1497,7 +1490,7 @@ static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
 		return NULL;
 
 	*handle = __iommu_create_mapping(dev, &page, size, attrs);
-	if (*handle == ARM_MAPPING_ERROR)
+	if (*handle == DMA_MAPPING_ERROR)
 		goto err_mapping;
 
 	return addr;
@@ -1525,7 +1518,7 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
 	struct page **pages;
 	void *addr = NULL;
 
-	*handle = ARM_MAPPING_ERROR;
+	*handle = DMA_MAPPING_ERROR;
 	size = PAGE_ALIGN(size);
 
 	if (coherent_flag  == COHERENT || !gfpflags_allow_blocking(gfp))
@@ -1546,7 +1539,7 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
 		return NULL;
 
 	*handle = __iommu_create_mapping(dev, pages, size, attrs);
-	if (*handle == ARM_MAPPING_ERROR)
+	if (*handle == DMA_MAPPING_ERROR)
 		goto err_buffer;
 
 	if (attrs & DMA_ATTR_NO_KERNEL_MAPPING)
@@ -1696,10 +1689,10 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
 	int prot;
 
 	size = PAGE_ALIGN(size);
-	*handle = ARM_MAPPING_ERROR;
+	*handle = DMA_MAPPING_ERROR;
 
 	iova_base = iova = __alloc_iova(mapping, size);
-	if (iova == ARM_MAPPING_ERROR)
+	if (iova == DMA_MAPPING_ERROR)
 		return -ENOMEM;
 
 	for (count = 0, s = sg; count < (size >> PAGE_SHIFT); s = sg_next(s)) {
@@ -1739,7 +1732,7 @@ static int __iommu_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 	for (i = 1; i < nents; i++) {
 		s = sg_next(s);
 
-		s->dma_address = ARM_MAPPING_ERROR;
+		s->dma_address = DMA_MAPPING_ERROR;
 		s->dma_length = 0;
 
 		if (s->offset || (size & ~PAGE_MASK) || size + s->length > max) {
@@ -1914,7 +1907,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p
 	int ret, prot, len = PAGE_ALIGN(size + offset);
 
 	dma_addr = __alloc_iova(mapping, len);
-	if (dma_addr == ARM_MAPPING_ERROR)
+	if (dma_addr == DMA_MAPPING_ERROR)
 		return dma_addr;
 
 	prot = __dma_info_to_prot(dir, attrs);
@@ -1926,7 +1919,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p
 	return dma_addr + offset;
 fail:
 	__free_iova(mapping, dma_addr, len);
-	return ARM_MAPPING_ERROR;
+	return DMA_MAPPING_ERROR;
 }
 
 /**
@@ -2020,7 +2013,7 @@ static dma_addr_t arm_iommu_map_resource(struct device *dev,
 	size_t len = PAGE_ALIGN(size + offset);
 
 	dma_addr = __alloc_iova(mapping, len);
-	if (dma_addr == ARM_MAPPING_ERROR)
+	if (dma_addr == DMA_MAPPING_ERROR)
 		return dma_addr;
 
 	prot = __dma_info_to_prot(dir, attrs) | IOMMU_MMIO;
@@ -2032,7 +2025,7 @@ static dma_addr_t arm_iommu_map_resource(struct device *dev,
 	return dma_addr + offset;
 fail:
 	__free_iova(mapping, dma_addr, len);
-	return ARM_MAPPING_ERROR;
+	return DMA_MAPPING_ERROR;
 }
 
 /**
@@ -2105,7 +2098,6 @@ const struct dma_map_ops iommu_ops = {
 	.map_resource		= arm_iommu_map_resource,
 	.unmap_resource		= arm_iommu_unmap_resource,
 
-	.mapping_error		= arm_dma_mapping_error,
 	.dma_supported		= arm_dma_supported,
 };
 
@@ -2124,7 +2116,6 @@ const struct dma_map_ops iommu_coherent_ops = {
 	.map_resource	= arm_iommu_map_resource,
 	.unmap_resource	= arm_iommu_unmap_resource,
 
-	.mapping_error		= arm_dma_mapping_error,
 	.dma_supported		= arm_dma_supported,
 };
 
-- 
2.19.1


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

* [PATCH 06/24] powerpc/iommu: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (4 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 05/24] arm: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 07/24] mips/jazz: " Christoph Hellwig
                   ` (18 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

The powerpc iommu code already returns (~(dma_addr_t)0x0) on mapping
failures, so we can switch over to returning DMA_MAPPING_ERROR and let
the core dma-mapping code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/include/asm/iommu.h     |  4 ----
 arch/powerpc/kernel/dma-iommu.c      |  6 ------
 arch/powerpc/kernel/iommu.c          | 28 ++++++++++++++--------------
 arch/powerpc/platforms/cell/iommu.c  |  1 -
 arch/powerpc/platforms/pseries/vio.c |  3 +--
 5 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index 35db0cbc9222..55312990d1d2 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -143,8 +143,6 @@ struct scatterlist;
 
 #ifdef CONFIG_PPC64
 
-#define IOMMU_MAPPING_ERROR		(~(dma_addr_t)0x0)
-
 static inline void set_iommu_table_base(struct device *dev,
 					struct iommu_table *base)
 {
@@ -242,8 +240,6 @@ static inline int __init tce_iommu_bus_notifier_init(void)
 }
 #endif /* !CONFIG_IOMMU_API */
 
-int dma_iommu_mapping_error(struct device *dev, dma_addr_t dma_addr);
-
 #else
 
 static inline void *get_iommu_table_base(struct device *dev)
diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
index f9fe2080ceb9..5ebacf0fe41a 100644
--- a/arch/powerpc/kernel/dma-iommu.c
+++ b/arch/powerpc/kernel/dma-iommu.c
@@ -106,11 +106,6 @@ static u64 dma_iommu_get_required_mask(struct device *dev)
 	return mask;
 }
 
-int dma_iommu_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == IOMMU_MAPPING_ERROR;
-}
-
 struct dma_map_ops dma_iommu_ops = {
 	.alloc			= dma_iommu_alloc_coherent,
 	.free			= dma_iommu_free_coherent,
@@ -121,6 +116,5 @@ struct dma_map_ops dma_iommu_ops = {
 	.map_page		= dma_iommu_map_page,
 	.unmap_page		= dma_iommu_unmap_page,
 	.get_required_mask	= dma_iommu_get_required_mask,
-	.mapping_error		= dma_iommu_mapping_error,
 };
 EXPORT_SYMBOL(dma_iommu_ops);
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index f0dc680e659a..ca7f73488c62 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -197,11 +197,11 @@ static unsigned long iommu_range_alloc(struct device *dev,
 	if (unlikely(npages == 0)) {
 		if (printk_ratelimit())
 			WARN_ON(1);
-		return IOMMU_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 	}
 
 	if (should_fail_iommu(dev))
-		return IOMMU_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 
 	/*
 	 * We don't need to disable preemption here because any CPU can
@@ -277,7 +277,7 @@ static unsigned long iommu_range_alloc(struct device *dev,
 		} else {
 			/* Give up */
 			spin_unlock_irqrestore(&(pool->lock), flags);
-			return IOMMU_MAPPING_ERROR;
+			return DMA_MAPPING_ERROR;
 		}
 	}
 
@@ -309,13 +309,13 @@ static dma_addr_t iommu_alloc(struct device *dev, struct iommu_table *tbl,
 			      unsigned long attrs)
 {
 	unsigned long entry;
-	dma_addr_t ret = IOMMU_MAPPING_ERROR;
+	dma_addr_t ret = DMA_MAPPING_ERROR;
 	int build_fail;
 
 	entry = iommu_range_alloc(dev, tbl, npages, NULL, mask, align_order);
 
-	if (unlikely(entry == IOMMU_MAPPING_ERROR))
-		return IOMMU_MAPPING_ERROR;
+	if (unlikely(entry == DMA_MAPPING_ERROR))
+		return DMA_MAPPING_ERROR;
 
 	entry += tbl->it_offset;	/* Offset into real TCE table */
 	ret = entry << tbl->it_page_shift;	/* Set the return dma address */
@@ -327,12 +327,12 @@ static dma_addr_t iommu_alloc(struct device *dev, struct iommu_table *tbl,
 
 	/* tbl->it_ops->set() only returns non-zero for transient errors.
 	 * Clean up the table bitmap in this case and return
-	 * IOMMU_MAPPING_ERROR. For all other errors the functionality is
+	 * DMA_MAPPING_ERROR. For all other errors the functionality is
 	 * not altered.
 	 */
 	if (unlikely(build_fail)) {
 		__iommu_free(tbl, ret, npages);
-		return IOMMU_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 	}
 
 	/* Flush/invalidate TLB caches if necessary */
@@ -477,7 +477,7 @@ int ppc_iommu_map_sg(struct device *dev, struct iommu_table *tbl,
 		DBG("  - vaddr: %lx, size: %lx\n", vaddr, slen);
 
 		/* Handle failure */
-		if (unlikely(entry == IOMMU_MAPPING_ERROR)) {
+		if (unlikely(entry == DMA_MAPPING_ERROR)) {
 			if (!(attrs & DMA_ATTR_NO_WARN) &&
 			    printk_ratelimit())
 				dev_info(dev, "iommu_alloc failed, tbl %p "
@@ -544,7 +544,7 @@ int ppc_iommu_map_sg(struct device *dev, struct iommu_table *tbl,
 	 */
 	if (outcount < incount) {
 		outs = sg_next(outs);
-		outs->dma_address = IOMMU_MAPPING_ERROR;
+		outs->dma_address = DMA_MAPPING_ERROR;
 		outs->dma_length = 0;
 	}
 
@@ -562,7 +562,7 @@ int ppc_iommu_map_sg(struct device *dev, struct iommu_table *tbl,
 			npages = iommu_num_pages(s->dma_address, s->dma_length,
 						 IOMMU_PAGE_SIZE(tbl));
 			__iommu_free(tbl, vaddr, npages);
-			s->dma_address = IOMMU_MAPPING_ERROR;
+			s->dma_address = DMA_MAPPING_ERROR;
 			s->dma_length = 0;
 		}
 		if (s == outs)
@@ -776,7 +776,7 @@ dma_addr_t iommu_map_page(struct device *dev, struct iommu_table *tbl,
 			  unsigned long mask, enum dma_data_direction direction,
 			  unsigned long attrs)
 {
-	dma_addr_t dma_handle = IOMMU_MAPPING_ERROR;
+	dma_addr_t dma_handle = DMA_MAPPING_ERROR;
 	void *vaddr;
 	unsigned long uaddr;
 	unsigned int npages, align;
@@ -796,7 +796,7 @@ dma_addr_t iommu_map_page(struct device *dev, struct iommu_table *tbl,
 		dma_handle = iommu_alloc(dev, tbl, vaddr, npages, direction,
 					 mask >> tbl->it_page_shift, align,
 					 attrs);
-		if (dma_handle == IOMMU_MAPPING_ERROR) {
+		if (dma_handle == DMA_MAPPING_ERROR) {
 			if (!(attrs & DMA_ATTR_NO_WARN) &&
 			    printk_ratelimit())  {
 				dev_info(dev, "iommu_alloc failed, tbl %p "
@@ -868,7 +868,7 @@ void *iommu_alloc_coherent(struct device *dev, struct iommu_table *tbl,
 	io_order = get_iommu_order(size, tbl);
 	mapping = iommu_alloc(dev, tbl, ret, nio_pages, DMA_BIDIRECTIONAL,
 			      mask >> tbl->it_page_shift, io_order, 0);
-	if (mapping == IOMMU_MAPPING_ERROR) {
+	if (mapping == DMA_MAPPING_ERROR) {
 		free_pages((unsigned long)ret, order);
 		return NULL;
 	}
diff --git a/arch/powerpc/platforms/cell/iommu.c b/arch/powerpc/platforms/cell/iommu.c
index 12352a58072a..af2a3c15e0ec 100644
--- a/arch/powerpc/platforms/cell/iommu.c
+++ b/arch/powerpc/platforms/cell/iommu.c
@@ -654,7 +654,6 @@ static const struct dma_map_ops dma_iommu_fixed_ops = {
 	.dma_supported  = dma_suported_and_switch,
 	.map_page       = dma_fixed_map_page,
 	.unmap_page     = dma_fixed_unmap_page,
-	.mapping_error	= dma_iommu_mapping_error,
 };
 
 static void cell_dma_dev_setup(struct device *dev)
diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
index 88f1ad1d6309..a29ad7db918a 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -519,7 +519,7 @@ static dma_addr_t vio_dma_iommu_map_page(struct device *dev, struct page *page,
 {
 	struct vio_dev *viodev = to_vio_dev(dev);
 	struct iommu_table *tbl;
-	dma_addr_t ret = IOMMU_MAPPING_ERROR;
+	dma_addr_t ret = DMA_MAPPING_ERROR;
 
 	tbl = get_iommu_table_base(dev);
 	if (vio_cmo_alloc(viodev, roundup(size, IOMMU_PAGE_SIZE(tbl)))) {
@@ -625,7 +625,6 @@ static const struct dma_map_ops vio_dma_mapping_ops = {
 	.unmap_page        = vio_dma_iommu_unmap_page,
 	.dma_supported     = vio_dma_iommu_dma_supported,
 	.get_required_mask = vio_dma_get_required_mask,
-	.mapping_error	   = dma_iommu_mapping_error,
 };
 
 /**
-- 
2.19.1


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

* [PATCH 07/24] mips/jazz: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (5 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 06/24] powerpc/iommu: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 08/24] s390: " Christoph Hellwig
                   ` (17 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

The Jazz iommu code already returns (~(dma_addr_t)0x0) on mapping
failures, so we can switch over to returning DMA_MAPPING_ERROR and
let the core dma-mapping code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/mips/include/asm/jazzdma.h |  6 ------
 arch/mips/jazz/jazzdma.c        | 16 +++++-----------
 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/arch/mips/include/asm/jazzdma.h b/arch/mips/include/asm/jazzdma.h
index d913439c738c..d13f940022d5 100644
--- a/arch/mips/include/asm/jazzdma.h
+++ b/arch/mips/include/asm/jazzdma.h
@@ -39,12 +39,6 @@ extern int vdma_get_enable(int channel);
 #define VDMA_PAGE(a)		((unsigned int)(a) >> 12)
 #define VDMA_OFFSET(a)		((unsigned int)(a) & (VDMA_PAGESIZE-1))
 
-/*
- * error code returned by vdma_alloc()
- * (See also arch/mips/kernel/jazzdma.c)
- */
-#define VDMA_ERROR		0xffffffff
-
 /*
  * VDMA pagetable entry description
  */
diff --git a/arch/mips/jazz/jazzdma.c b/arch/mips/jazz/jazzdma.c
index 4c41ed0a637e..6256d35dbf4d 100644
--- a/arch/mips/jazz/jazzdma.c
+++ b/arch/mips/jazz/jazzdma.c
@@ -104,12 +104,12 @@ unsigned long vdma_alloc(unsigned long paddr, unsigned long size)
 		if (vdma_debug)
 			printk("vdma_alloc: Invalid physical address: %08lx\n",
 			       paddr);
-		return VDMA_ERROR;	/* invalid physical address */
+		return DMA_MAPPING_ERROR;	/* invalid physical address */
 	}
 	if (size > 0x400000 || size == 0) {
 		if (vdma_debug)
 			printk("vdma_alloc: Invalid size: %08lx\n", size);
-		return VDMA_ERROR;	/* invalid physical address */
+		return DMA_MAPPING_ERROR;	/* invalid physical address */
 	}
 
 	spin_lock_irqsave(&vdma_lock, flags);
@@ -123,7 +123,7 @@ unsigned long vdma_alloc(unsigned long paddr, unsigned long size)
 		       first < VDMA_PGTBL_ENTRIES) first++;
 		if (first + pages > VDMA_PGTBL_ENTRIES) {	/* nothing free */
 			spin_unlock_irqrestore(&vdma_lock, flags);
-			return VDMA_ERROR;
+			return DMA_MAPPING_ERROR;
 		}
 
 		last = first + 1;
@@ -569,7 +569,7 @@ static void *jazz_dma_alloc(struct device *dev, size_t size,
 		return NULL;
 
 	*dma_handle = vdma_alloc(virt_to_phys(ret), size);
-	if (*dma_handle == VDMA_ERROR) {
+	if (*dma_handle == DMA_MAPPING_ERROR) {
 		dma_direct_free_pages(dev, size, ret, *dma_handle, attrs);
 		return NULL;
 	}
@@ -620,7 +620,7 @@ static int jazz_dma_map_sg(struct device *dev, struct scatterlist *sglist,
 			arch_sync_dma_for_device(dev, sg_phys(sg), sg->length,
 				dir);
 		sg->dma_address = vdma_alloc(sg_phys(sg), sg->length);
-		if (sg->dma_address == VDMA_ERROR)
+		if (sg->dma_address == DMA_MAPPING_ERROR)
 			return 0;
 		sg_dma_len(sg) = sg->length;
 	}
@@ -674,11 +674,6 @@ static void jazz_dma_sync_sg_for_cpu(struct device *dev,
 		arch_sync_dma_for_cpu(dev, sg_phys(sg), sg->length, dir);
 }
 
-static int jazz_dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == VDMA_ERROR;
-}
-
 const struct dma_map_ops jazz_dma_ops = {
 	.alloc			= jazz_dma_alloc,
 	.free			= jazz_dma_free,
@@ -692,6 +687,5 @@ const struct dma_map_ops jazz_dma_ops = {
 	.sync_sg_for_device	= jazz_dma_sync_sg_for_device,
 	.dma_supported		= dma_direct_supported,
 	.cache_sync		= arch_dma_cache_sync,
-	.mapping_error		= jazz_dma_mapping_error,
 };
 EXPORT_SYMBOL(jazz_dma_ops);
-- 
2.19.1


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

* [PATCH 08/24] s390: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (6 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 07/24] mips/jazz: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 09/24] sparc: " Christoph Hellwig
                   ` (16 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

S390 already returns (~(dma_addr_t)0x0) on mapping failures, so we can
switch over to returning DMA_MAPPING_ERROR and let the core dma-mapping
code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/s390/pci/pci_dma.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
index d387a0fbdd7e..346ba382193a 100644
--- a/arch/s390/pci/pci_dma.c
+++ b/arch/s390/pci/pci_dma.c
@@ -15,8 +15,6 @@
 #include <linux/pci.h>
 #include <asm/pci_dma.h>
 
-#define S390_MAPPING_ERROR		(~(dma_addr_t) 0x0)
-
 static struct kmem_cache *dma_region_table_cache;
 static struct kmem_cache *dma_page_table_cache;
 static int s390_iommu_strict;
@@ -301,7 +299,7 @@ static dma_addr_t dma_alloc_address(struct device *dev, int size)
 
 out_error:
 	spin_unlock_irqrestore(&zdev->iommu_bitmap_lock, flags);
-	return S390_MAPPING_ERROR;
+	return DMA_MAPPING_ERROR;
 }
 
 static void dma_free_address(struct device *dev, dma_addr_t dma_addr, int size)
@@ -349,7 +347,7 @@ static dma_addr_t s390_dma_map_pages(struct device *dev, struct page *page,
 	/* This rounds up number of pages based on size and offset */
 	nr_pages = iommu_num_pages(pa, size, PAGE_SIZE);
 	dma_addr = dma_alloc_address(dev, nr_pages);
-	if (dma_addr == S390_MAPPING_ERROR) {
+	if (dma_addr == DMA_MAPPING_ERROR) {
 		ret = -ENOSPC;
 		goto out_err;
 	}
@@ -372,7 +370,7 @@ static dma_addr_t s390_dma_map_pages(struct device *dev, struct page *page,
 out_err:
 	zpci_err("map error:\n");
 	zpci_err_dma(ret, pa);
-	return S390_MAPPING_ERROR;
+	return DMA_MAPPING_ERROR;
 }
 
 static void s390_dma_unmap_pages(struct device *dev, dma_addr_t dma_addr,
@@ -449,7 +447,7 @@ static int __s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
 	int ret;
 
 	dma_addr_base = dma_alloc_address(dev, nr_pages);
-	if (dma_addr_base == S390_MAPPING_ERROR)
+	if (dma_addr_base == DMA_MAPPING_ERROR)
 		return -ENOMEM;
 
 	dma_addr = dma_addr_base;
@@ -496,7 +494,7 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
 	for (i = 1; i < nr_elements; i++) {
 		s = sg_next(s);
 
-		s->dma_address = S390_MAPPING_ERROR;
+		s->dma_address = DMA_MAPPING_ERROR;
 		s->dma_length = 0;
 
 		if (s->offset || (size & ~PAGE_MASK) ||
@@ -546,11 +544,6 @@ static void s390_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
 	}
 }
 	
-static int s390_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == S390_MAPPING_ERROR;
-}
-
 int zpci_dma_init_device(struct zpci_dev *zdev)
 {
 	int rc;
@@ -675,7 +668,6 @@ const struct dma_map_ops s390_pci_dma_ops = {
 	.unmap_sg	= s390_dma_unmap_sg,
 	.map_page	= s390_dma_map_pages,
 	.unmap_page	= s390_dma_unmap_pages,
-	.mapping_error	= s390_mapping_error,
 	/* dma_supported is unconditionally true without a callback */
 };
 EXPORT_SYMBOL_GPL(s390_pci_dma_ops);
-- 
2.19.1


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

* [PATCH 09/24] sparc: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (7 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 08/24] s390: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 10/24] parisc/ccio: " Christoph Hellwig
                   ` (15 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Sparc already returns (~(dma_addr_t)0x0) on mapping failures, so we can
switch over to returning DMA_MAPPING_ERROR and let the core dma-mapping
code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/sparc/kernel/iommu.c        | 12 +++---------
 arch/sparc/kernel/iommu_common.h |  2 --
 arch/sparc/kernel/pci_sun4v.c    | 14 ++++----------
 3 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/arch/sparc/kernel/iommu.c b/arch/sparc/kernel/iommu.c
index 40d008b0bd3e..0626bae5e3da 100644
--- a/arch/sparc/kernel/iommu.c
+++ b/arch/sparc/kernel/iommu.c
@@ -315,7 +315,7 @@ static dma_addr_t dma_4u_map_page(struct device *dev, struct page *page,
 bad_no_ctx:
 	if (printk_ratelimit())
 		WARN_ON(1);
-	return SPARC_MAPPING_ERROR;
+	return DMA_MAPPING_ERROR;
 }
 
 static void strbuf_flush(struct strbuf *strbuf, struct iommu *iommu,
@@ -548,7 +548,7 @@ static int dma_4u_map_sg(struct device *dev, struct scatterlist *sglist,
 
 	if (outcount < incount) {
 		outs = sg_next(outs);
-		outs->dma_address = SPARC_MAPPING_ERROR;
+		outs->dma_address = DMA_MAPPING_ERROR;
 		outs->dma_length = 0;
 	}
 
@@ -574,7 +574,7 @@ static int dma_4u_map_sg(struct device *dev, struct scatterlist *sglist,
 			iommu_tbl_range_free(&iommu->tbl, vaddr, npages,
 					     IOMMU_ERROR_CODE);
 
-			s->dma_address = SPARC_MAPPING_ERROR;
+			s->dma_address = DMA_MAPPING_ERROR;
 			s->dma_length = 0;
 		}
 		if (s == outs)
@@ -742,11 +742,6 @@ static void dma_4u_sync_sg_for_cpu(struct device *dev,
 	spin_unlock_irqrestore(&iommu->lock, flags);
 }
 
-static int dma_4u_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == SPARC_MAPPING_ERROR;
-}
-
 static int dma_4u_supported(struct device *dev, u64 device_mask)
 {
 	struct iommu *iommu = dev->archdata.iommu;
@@ -772,7 +767,6 @@ static const struct dma_map_ops sun4u_dma_ops = {
 	.sync_single_for_cpu	= dma_4u_sync_single_for_cpu,
 	.sync_sg_for_cpu	= dma_4u_sync_sg_for_cpu,
 	.dma_supported		= dma_4u_supported,
-	.mapping_error		= dma_4u_mapping_error,
 };
 
 const struct dma_map_ops *dma_ops = &sun4u_dma_ops;
diff --git a/arch/sparc/kernel/iommu_common.h b/arch/sparc/kernel/iommu_common.h
index e3c02ba32500..d62ed9c5682d 100644
--- a/arch/sparc/kernel/iommu_common.h
+++ b/arch/sparc/kernel/iommu_common.h
@@ -48,6 +48,4 @@ static inline int is_span_boundary(unsigned long entry,
 	return iommu_is_span_boundary(entry, nr, shift, boundary_size);
 }
 
-#define SPARC_MAPPING_ERROR	(~(dma_addr_t)0x0)
-
 #endif /* _IOMMU_COMMON_H */
diff --git a/arch/sparc/kernel/pci_sun4v.c b/arch/sparc/kernel/pci_sun4v.c
index 565d9ac883d0..fa0e42b4cbfb 100644
--- a/arch/sparc/kernel/pci_sun4v.c
+++ b/arch/sparc/kernel/pci_sun4v.c
@@ -414,12 +414,12 @@ static dma_addr_t dma_4v_map_page(struct device *dev, struct page *page,
 bad:
 	if (printk_ratelimit())
 		WARN_ON(1);
-	return SPARC_MAPPING_ERROR;
+	return DMA_MAPPING_ERROR;
 
 iommu_map_fail:
 	local_irq_restore(flags);
 	iommu_tbl_range_free(tbl, bus_addr, npages, IOMMU_ERROR_CODE);
-	return SPARC_MAPPING_ERROR;
+	return DMA_MAPPING_ERROR;
 }
 
 static void dma_4v_unmap_page(struct device *dev, dma_addr_t bus_addr,
@@ -592,7 +592,7 @@ static int dma_4v_map_sg(struct device *dev, struct scatterlist *sglist,
 
 	if (outcount < incount) {
 		outs = sg_next(outs);
-		outs->dma_address = SPARC_MAPPING_ERROR;
+		outs->dma_address = DMA_MAPPING_ERROR;
 		outs->dma_length = 0;
 	}
 
@@ -609,7 +609,7 @@ static int dma_4v_map_sg(struct device *dev, struct scatterlist *sglist,
 			iommu_tbl_range_free(tbl, vaddr, npages,
 					     IOMMU_ERROR_CODE);
 			/* XXX demap? XXX */
-			s->dma_address = SPARC_MAPPING_ERROR;
+			s->dma_address = DMA_MAPPING_ERROR;
 			s->dma_length = 0;
 		}
 		if (s == outs)
@@ -688,11 +688,6 @@ static int dma_4v_supported(struct device *dev, u64 device_mask)
 	return pci64_dma_supported(to_pci_dev(dev), device_mask);
 }
 
-static int dma_4v_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == SPARC_MAPPING_ERROR;
-}
-
 static const struct dma_map_ops sun4v_dma_ops = {
 	.alloc				= dma_4v_alloc_coherent,
 	.free				= dma_4v_free_coherent,
@@ -701,7 +696,6 @@ static const struct dma_map_ops sun4v_dma_ops = {
 	.map_sg				= dma_4v_map_sg,
 	.unmap_sg			= dma_4v_unmap_sg,
 	.dma_supported			= dma_4v_supported,
-	.mapping_error			= dma_4v_mapping_error,
 };
 
 static void pci_sun4v_scan_bus(struct pci_pbm_info *pbm, struct device *parent)
-- 
2.19.1


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

* [PATCH 10/24] parisc/ccio: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (8 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 09/24] sparc: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 11/24] parisc/sba_iommu: " Christoph Hellwig
                   ` (14 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

The CCIO iommu code already returns (~(dma_addr_t)0x0) on mapping
failures, so we can switch over to returning DMA_MAPPING_ERROR and let
the core dma-mapping code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/parisc/ccio-dma.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c
index 701a7d6a74d5..714aac72df0e 100644
--- a/drivers/parisc/ccio-dma.c
+++ b/drivers/parisc/ccio-dma.c
@@ -110,8 +110,6 @@
 #define CMD_TLB_DIRECT_WRITE 35         /* IO_COMMAND for I/O TLB Writes     */
 #define CMD_TLB_PURGE        33         /* IO_COMMAND to Purge I/O TLB entry */
 
-#define CCIO_MAPPING_ERROR    (~(dma_addr_t)0)
-
 struct ioa_registers {
         /* Runway Supervisory Set */
         int32_t    unused1[12];
@@ -740,7 +738,7 @@ ccio_map_single(struct device *dev, void *addr, size_t size,
 	BUG_ON(!dev);
 	ioc = GET_IOC(dev);
 	if (!ioc)
-		return CCIO_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 
 	BUG_ON(size <= 0);
 
@@ -1021,11 +1019,6 @@ ccio_unmap_sg(struct device *dev, struct scatterlist *sglist, int nents,
 	DBG_RUN_SG("%s() DONE (nents %d)\n", __func__, nents);
 }
 
-static int ccio_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == CCIO_MAPPING_ERROR;
-}
-
 static const struct dma_map_ops ccio_ops = {
 	.dma_supported =	ccio_dma_supported,
 	.alloc =		ccio_alloc,
@@ -1034,7 +1027,6 @@ static const struct dma_map_ops ccio_ops = {
 	.unmap_page =		ccio_unmap_page,
 	.map_sg = 		ccio_map_sg,
 	.unmap_sg = 		ccio_unmap_sg,
-	.mapping_error =	ccio_mapping_error,
 };
 
 #ifdef CONFIG_PROC_FS
-- 
2.19.1


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

* [PATCH 11/24] parisc/sba_iommu: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (9 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 10/24] parisc/ccio: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 12/24] arm64: remove the dummy_dma_ops mapping_error method Christoph Hellwig
                   ` (13 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

The SBA iommu code already returns (~(dma_addr_t)0x0) on mapping
failures, so we can switch over to returning DMA_MAPPING_ERROR and let
the core dma-mapping code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/parisc/sba_iommu.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
index c1e599a429af..452d306ce5cb 100644
--- a/drivers/parisc/sba_iommu.c
+++ b/drivers/parisc/sba_iommu.c
@@ -93,8 +93,6 @@
 
 #define DEFAULT_DMA_HINT_REG	0
 
-#define SBA_MAPPING_ERROR    (~(dma_addr_t)0)
-
 struct sba_device *sba_list;
 EXPORT_SYMBOL_GPL(sba_list);
 
@@ -725,7 +723,7 @@ sba_map_single(struct device *dev, void *addr, size_t size,
 
 	ioc = GET_IOC(dev);
 	if (!ioc)
-		return SBA_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 
 	/* save offset bits */
 	offset = ((dma_addr_t) (long) addr) & ~IOVP_MASK;
@@ -1080,11 +1078,6 @@ sba_unmap_sg(struct device *dev, struct scatterlist *sglist, int nents,
 
 }
 
-static int sba_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == SBA_MAPPING_ERROR;
-}
-
 static const struct dma_map_ops sba_ops = {
 	.dma_supported =	sba_dma_supported,
 	.alloc =		sba_alloc,
@@ -1093,7 +1086,6 @@ static const struct dma_map_ops sba_ops = {
 	.unmap_page =		sba_unmap_page,
 	.map_sg =		sba_map_sg,
 	.unmap_sg =		sba_unmap_sg,
-	.mapping_error =	sba_mapping_error,
 };
 
 
-- 
2.19.1


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

* [PATCH 12/24] arm64: remove the dummy_dma_ops mapping_error method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (10 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 11/24] parisc/sba_iommu: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 13/24] alpha: remove the mapping_error dma_map_ops method Christoph Hellwig
                   ` (12 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Just return DMA_MAPPING_ERROR from __dummy_map_page and let the core
dma-mapping code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/arm64/mm/dma-mapping.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index a3ac26284845..fdc26ea5036c 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -282,7 +282,7 @@ static dma_addr_t __dummy_map_page(struct device *dev, struct page *page,
 				   enum dma_data_direction dir,
 				   unsigned long attrs)
 {
-	return 0;
+	return DMA_MAPPING_ERROR;
 }
 
 static void __dummy_unmap_page(struct device *dev, dma_addr_t dev_addr,
@@ -317,11 +317,6 @@ static void __dummy_sync_sg(struct device *dev,
 {
 }
 
-static int __dummy_mapping_error(struct device *hwdev, dma_addr_t dma_addr)
-{
-	return 1;
-}
-
 static int __dummy_dma_supported(struct device *hwdev, u64 mask)
 {
 	return 0;
@@ -339,7 +334,6 @@ const struct dma_map_ops dummy_dma_ops = {
 	.sync_single_for_device = __dummy_sync_single,
 	.sync_sg_for_cpu        = __dummy_sync_sg,
 	.sync_sg_for_device     = __dummy_sync_sg,
-	.mapping_error          = __dummy_mapping_error,
 	.dma_supported          = __dummy_dma_supported,
 };
 EXPORT_SYMBOL(dummy_dma_ops);
-- 
2.19.1


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

* [PATCH 13/24] alpha: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (11 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 12/24] arm64: remove the dummy_dma_ops mapping_error method Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 14/24] ia64/sba_iommu: improve internal map_page users Christoph Hellwig
                   ` (11 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Return DMA_MAPPING_ERROR instead of 0 on a dma mapping failure and let
the core dma-mapping code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/alpha/kernel/pci_iommu.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index 46e08e0d9181..e1716e0d92fd 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -291,7 +291,7 @@ pci_map_single_1(struct pci_dev *pdev, void *cpu_addr, size_t size,
 	   use direct_map above, it now must be considered an error. */
 	if (! alpha_mv.mv_pci_tbi) {
 		printk_once(KERN_WARNING "pci_map_single: no HW sg\n");
-		return 0;
+		return DMA_MAPPING_ERROR;
 	}
 
 	arena = hose->sg_pci;
@@ -307,7 +307,7 @@ pci_map_single_1(struct pci_dev *pdev, void *cpu_addr, size_t size,
 	if (dma_ofs < 0) {
 		printk(KERN_WARNING "pci_map_single failed: "
 		       "could not allocate dma page tables\n");
-		return 0;
+		return DMA_MAPPING_ERROR;
 	}
 
 	paddr &= PAGE_MASK;
@@ -455,7 +455,7 @@ static void *alpha_pci_alloc_coherent(struct device *dev, size_t size,
 	memset(cpu_addr, 0, size);
 
 	*dma_addrp = pci_map_single_1(pdev, cpu_addr, size, 0);
-	if (*dma_addrp == 0) {
+	if (*dma_addrp == DMA_MAPPING_ERROR) {
 		free_pages((unsigned long)cpu_addr, order);
 		if (alpha_mv.mv_pci_tbi || (gfp & GFP_DMA))
 			return NULL;
@@ -671,7 +671,7 @@ static int alpha_pci_map_sg(struct device *dev, struct scatterlist *sg,
 		sg->dma_address
 		  = pci_map_single_1(pdev, SG_ENT_VIRT_ADDRESS(sg),
 				     sg->length, dac_allowed);
-		return sg->dma_address != 0;
+		return sg->dma_address != DMA_MAPPING_ERROR;
 	}
 
 	start = sg;
@@ -935,11 +935,6 @@ iommu_unbind(struct pci_iommu_arena *arena, long pg_start, long pg_count)
 	return 0;
 }
 
-static int alpha_pci_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == 0;
-}
-
 const struct dma_map_ops alpha_pci_ops = {
 	.alloc			= alpha_pci_alloc_coherent,
 	.free			= alpha_pci_free_coherent,
@@ -947,7 +942,6 @@ const struct dma_map_ops alpha_pci_ops = {
 	.unmap_page		= alpha_pci_unmap_page,
 	.map_sg			= alpha_pci_map_sg,
 	.unmap_sg		= alpha_pci_unmap_sg,
-	.mapping_error		= alpha_pci_mapping_error,
 	.dma_supported		= alpha_pci_supported,
 };
 EXPORT_SYMBOL(alpha_pci_ops);
-- 
2.19.1


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

* [PATCH 14/24] ia64/sba_iommu: improve internal map_page users
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (12 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 13/24] alpha: remove the mapping_error dma_map_ops method Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 15/24] ia64/sba_iommu: remove the mapping_error dma_map_ops method Christoph Hellwig
                   ` (10 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Remove the odd sba_{un,}map_single_attrs wrappers, check errors
everywhere, and remove the completly bogus alloc_pages_node call that
uses the dma attributes argument as the node id.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/ia64/hp/common/sba_iommu.c | 71 ++++++++++++---------------------
 1 file changed, 26 insertions(+), 45 deletions(-)

diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
index e8a93b07283e..b953282d4704 100644
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -907,11 +907,12 @@ sba_mark_invalid(struct ioc *ioc, dma_addr_t iova, size_t byte_cnt)
 }
 
 /**
- * sba_map_single_attrs - map one buffer and return IOVA for DMA
+ * sba_map_page - map one buffer and return IOVA for DMA
  * @dev: instance of PCI owned by the driver that's asking.
- * @addr:  driver buffer to map.
- * @size:  number of bytes to map in driver buffer.
- * @dir:  R/W or both.
+ * @page: page to map
+ * @poff: offset into page
+ * @size: number of bytes to map
+ * @dir: dma direction
  * @attrs: optional dma attributes
  *
  * See Documentation/DMA-API-HOWTO.txt
@@ -944,7 +945,7 @@ static dma_addr_t sba_map_page(struct device *dev, struct page *page,
  		** Device is bit capable of DMA'ing to the buffer...
 		** just return the PCI address of ptr
  		*/
-		DBG_BYPASS("sba_map_single_attrs() bypass mask/addr: "
+		DBG_BYPASS("sba_map_page() bypass mask/addr: "
 			   "0x%lx/0x%lx\n",
 		           to_pci_dev(dev)->dma_mask, pci_addr);
 		return pci_addr;
@@ -966,7 +967,7 @@ static dma_addr_t sba_map_page(struct device *dev, struct page *page,
 
 #ifdef ASSERT_PDIR_SANITY
 	spin_lock_irqsave(&ioc->res_lock, flags);
-	if (sba_check_pdir(ioc,"Check before sba_map_single_attrs()"))
+	if (sba_check_pdir(ioc,"Check before sba_map_page()"))
 		panic("Sanity check failed");
 	spin_unlock_irqrestore(&ioc->res_lock, flags);
 #endif
@@ -997,20 +998,12 @@ static dma_addr_t sba_map_page(struct device *dev, struct page *page,
 	/* form complete address */
 #ifdef ASSERT_PDIR_SANITY
 	spin_lock_irqsave(&ioc->res_lock, flags);
-	sba_check_pdir(ioc,"Check after sba_map_single_attrs()");
+	sba_check_pdir(ioc,"Check after sba_map_page()");
 	spin_unlock_irqrestore(&ioc->res_lock, flags);
 #endif
 	return SBA_IOVA(ioc, iovp, offset);
 }
 
-static dma_addr_t sba_map_single_attrs(struct device *dev, void *addr,
-				       size_t size, enum dma_data_direction dir,
-				       unsigned long attrs)
-{
-	return sba_map_page(dev, virt_to_page(addr),
-			    (unsigned long)addr & ~PAGE_MASK, size, dir, attrs);
-}
-
 #ifdef ENABLE_MARK_CLEAN
 static SBA_INLINE void
 sba_mark_clean(struct ioc *ioc, dma_addr_t iova, size_t size)
@@ -1036,7 +1029,7 @@ sba_mark_clean(struct ioc *ioc, dma_addr_t iova, size_t size)
 #endif
 
 /**
- * sba_unmap_single_attrs - unmap one IOVA and free resources
+ * sba_unmap_page - unmap one IOVA and free resources
  * @dev: instance of PCI owned by the driver that's asking.
  * @iova:  IOVA of driver buffer previously mapped.
  * @size:  number of bytes mapped in driver buffer.
@@ -1063,7 +1056,7 @@ static void sba_unmap_page(struct device *dev, dma_addr_t iova, size_t size,
 		/*
 		** Address does not fall w/in IOVA, must be bypassing
 		*/
-		DBG_BYPASS("sba_unmap_single_attrs() bypass addr: 0x%lx\n",
+		DBG_BYPASS("sba_unmap_page() bypass addr: 0x%lx\n",
 			   iova);
 
 #ifdef ENABLE_MARK_CLEAN
@@ -1114,12 +1107,6 @@ static void sba_unmap_page(struct device *dev, dma_addr_t iova, size_t size,
 #endif /* DELAYED_RESOURCE_CNT == 0 */
 }
 
-void sba_unmap_single_attrs(struct device *dev, dma_addr_t iova, size_t size,
-			    enum dma_data_direction dir, unsigned long attrs)
-{
-	sba_unmap_page(dev, iova, size, dir, attrs);
-}
-
 /**
  * sba_alloc_coherent - allocate/map shared mem for DMA
  * @dev: instance of PCI owned by the driver that's asking.
@@ -1132,30 +1119,20 @@ static void *
 sba_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
 		   gfp_t flags, unsigned long attrs)
 {
+	struct page *page;
 	struct ioc *ioc;
 	void *addr;
 
 	ioc = GET_IOC(dev);
 	ASSERT(ioc);
 
-#ifdef CONFIG_NUMA
-	{
-		struct page *page;
-
-		page = alloc_pages_node(ioc->node, flags, get_order(size));
-		if (unlikely(!page))
-			return NULL;
-
-		addr = page_address(page);
-	}
-#else
-	addr = (void *) __get_free_pages(flags, get_order(size));
-#endif
-	if (unlikely(!addr))
+	page = alloc_pages(ioc->node, get_order(size));
+	if (unlikely(!page))
 		return NULL;
 
+	addr = page_address(page);
 	memset(addr, 0, size);
-	*dma_handle = virt_to_phys(addr);
+	*dma_handle = page_to_phys(page);
 
 #ifdef ALLOW_IOV_BYPASS
 	ASSERT(dev->coherent_dma_mask);
@@ -1174,9 +1151,10 @@ sba_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
 	 * If device can't bypass or bypass is disabled, pass the 32bit fake
 	 * device to map single to get an iova mapping.
 	 */
-	*dma_handle = sba_map_single_attrs(&ioc->sac_only_dev->dev, addr,
-					   size, 0, 0);
-
+	*dma_handle = sba_map_page(&ioc->sac_only_dev->dev, page, 0, size,
+			DMA_BIDIRECTIONAL, 0);
+	if (dma_mapping_error(dev, *dma_handle))
+		return NULL;
 	return addr;
 }
 
@@ -1193,7 +1171,7 @@ sba_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
 static void sba_free_coherent(struct device *dev, size_t size, void *vaddr,
 			      dma_addr_t dma_handle, unsigned long attrs)
 {
-	sba_unmap_single_attrs(dev, dma_handle, size, 0, 0);
+	sba_unmap_page(dev, dma_handle, size, 0, 0);
 	free_pages((unsigned long) vaddr, get_order(size));
 }
 
@@ -1483,7 +1461,10 @@ static int sba_map_sg_attrs(struct device *dev, struct scatterlist *sglist,
 	/* Fast path single entry scatterlists. */
 	if (nents == 1) {
 		sglist->dma_length = sglist->length;
-		sglist->dma_address = sba_map_single_attrs(dev, sba_sg_address(sglist), sglist->length, dir, attrs);
+		sglist->dma_address = sba_map_page(dev, sg_page(sglist),
+				sglist->offset, sglist->length, dir, attrs);
+		if (dma_mapping_error(dev, sglist->dma_address))
+			return 0;
 		return 1;
 	}
 
@@ -1572,8 +1553,8 @@ static void sba_unmap_sg_attrs(struct device *dev, struct scatterlist *sglist,
 
 	while (nents && sglist->dma_length) {
 
-		sba_unmap_single_attrs(dev, sglist->dma_address,
-				       sglist->dma_length, dir, attrs);
+		sba_unmap_page(dev, sglist->dma_address, sglist->dma_length,
+			       dir, attrs);
 		sglist = sg_next(sglist);
 		nents--;
 	}
-- 
2.19.1


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

* [PATCH 15/24] ia64/sba_iommu: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (13 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 14/24] ia64/sba_iommu: improve internal map_page users Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 16/24] ia64/sn: " Christoph Hellwig
                   ` (9 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Return DMA_MAPPING_ERROR instead of 0 on a dma mapping failure and let
the core dma-mapping code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/ia64/hp/common/sba_iommu.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
index b953282d4704..0316af2cb426 100644
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -974,7 +974,7 @@ static dma_addr_t sba_map_page(struct device *dev, struct page *page,
 
 	pide = sba_alloc_range(ioc, dev, size);
 	if (pide < 0)
-		return 0;
+		return DMA_MAPPING_ERROR;
 
 	iovp = (dma_addr_t) pide << iovp_shift;
 
@@ -2151,11 +2151,6 @@ static int sba_dma_supported (struct device *dev, u64 mask)
 	return ((mask & 0xFFFFFFFFUL) == 0xFFFFFFFFUL);
 }
 
-static int sba_dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return 0;
-}
-
 __setup("nosbagart", nosbagart);
 
 static int __init
@@ -2189,7 +2184,6 @@ const struct dma_map_ops sba_dma_ops = {
 	.map_sg			= sba_map_sg_attrs,
 	.unmap_sg		= sba_unmap_sg_attrs,
 	.dma_supported		= sba_dma_supported,
-	.mapping_error		= sba_dma_mapping_error,
 };
 
 void sba_dma_init(void)
-- 
2.19.1


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

* [PATCH 16/24] ia64/sn: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (14 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 15/24] ia64/sba_iommu: remove the mapping_error dma_map_ops method Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 17/24] x86/amd_gart: " Christoph Hellwig
                   ` (8 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Return DMA_MAPPING_ERROR instead of 0 on a dma mapping failure and let
the core dma-mapping code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/ia64/sn/pci/pci_dma.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/ia64/sn/pci/pci_dma.c b/arch/ia64/sn/pci/pci_dma.c
index 4ce4ee4ef9f2..b7d42e4edc1f 100644
--- a/arch/ia64/sn/pci/pci_dma.c
+++ b/arch/ia64/sn/pci/pci_dma.c
@@ -196,7 +196,7 @@ static dma_addr_t sn_dma_map_page(struct device *dev, struct page *page,
 
 	if (!dma_addr) {
 		printk(KERN_ERR "%s: out of ATEs\n", __func__);
-		return 0;
+		return DMA_MAPPING_ERROR;
 	}
 	return dma_addr;
 }
@@ -314,11 +314,6 @@ static int sn_dma_map_sg(struct device *dev, struct scatterlist *sgl,
 	return nhwentries;
 }
 
-static int sn_dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return 0;
-}
-
 static u64 sn_dma_get_required_mask(struct device *dev)
 {
 	return DMA_BIT_MASK(64);
@@ -441,7 +436,6 @@ static struct dma_map_ops sn_dma_ops = {
 	.unmap_page		= sn_dma_unmap_page,
 	.map_sg			= sn_dma_map_sg,
 	.unmap_sg		= sn_dma_unmap_sg,
-	.mapping_error		= sn_dma_mapping_error,
 	.dma_supported		= sn_dma_supported,
 	.get_required_mask	= sn_dma_get_required_mask,
 };
-- 
2.19.1


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

* [PATCH 17/24] x86/amd_gart: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (15 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 16/24] ia64/sn: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 18/24] x86/calgary: " Christoph Hellwig
                   ` (7 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Return DMA_MAPPING_ERROR instead of the magic bad_dma_addr on a dma
mapping failure and let the core dma-mapping code handle the rest.

Remove the magic EMERGENCY_PAGES that the bad_dma_addr gets redirected to.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/x86/kernel/amd_gart_64.c | 39 ++++++-----------------------------
 1 file changed, 6 insertions(+), 33 deletions(-)

diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c
index 3f9d1b4019bb..4e733de93f41 100644
--- a/arch/x86/kernel/amd_gart_64.c
+++ b/arch/x86/kernel/amd_gart_64.c
@@ -50,8 +50,6 @@ static unsigned long iommu_pages;	/* .. and in pages */
 
 static u32 *iommu_gatt_base;		/* Remapping table */
 
-static dma_addr_t bad_dma_addr;
-
 /*
  * If this is disabled the IOMMU will use an optimized flushing strategy
  * of only flushing when an mapping is reused. With it true the GART is
@@ -74,8 +72,6 @@ static u32 gart_unmapped_entry;
 	(((x) & 0xfffff000) | (((x) >> 32) << 4) | GPTE_VALID | GPTE_COHERENT)
 #define GPTE_DECODE(x) (((x) & 0xfffff000) | (((u64)(x) & 0xff0) << 28))
 
-#define EMERGENCY_PAGES 32 /* = 128KB */
-
 #ifdef CONFIG_AGP
 #define AGPEXTERN extern
 #else
@@ -184,14 +180,6 @@ static void iommu_full(struct device *dev, size_t size, int dir)
 	 */
 
 	dev_err(dev, "PCI-DMA: Out of IOMMU space for %lu bytes\n", size);
-
-	if (size > PAGE_SIZE*EMERGENCY_PAGES) {
-		if (dir == PCI_DMA_FROMDEVICE || dir == PCI_DMA_BIDIRECTIONAL)
-			panic("PCI-DMA: Memory would be corrupted\n");
-		if (dir == PCI_DMA_TODEVICE || dir == PCI_DMA_BIDIRECTIONAL)
-			panic(KERN_ERR
-				"PCI-DMA: Random memory would be DMAed\n");
-	}
 #ifdef CONFIG_IOMMU_LEAK
 	dump_leak();
 #endif
@@ -220,7 +208,7 @@ static dma_addr_t dma_map_area(struct device *dev, dma_addr_t phys_mem,
 	int i;
 
 	if (unlikely(phys_mem + size > GART_MAX_PHYS_ADDR))
-		return bad_dma_addr;
+		return DMA_MAPPING_ERROR;
 
 	iommu_page = alloc_iommu(dev, npages, align_mask);
 	if (iommu_page == -1) {
@@ -229,7 +217,7 @@ static dma_addr_t dma_map_area(struct device *dev, dma_addr_t phys_mem,
 		if (panic_on_overflow)
 			panic("dma_map_area overflow %lu bytes\n", size);
 		iommu_full(dev, size, dir);
-		return bad_dma_addr;
+		return DMA_MAPPING_ERROR;
 	}
 
 	for (i = 0; i < npages; i++) {
@@ -271,7 +259,7 @@ static void gart_unmap_page(struct device *dev, dma_addr_t dma_addr,
 	int npages;
 	int i;
 
-	if (dma_addr < iommu_bus_base + EMERGENCY_PAGES*PAGE_SIZE ||
+	if (dma_addr == DMA_MAPPING_ERROR ||
 	    dma_addr >= iommu_bus_base + iommu_size)
 		return;
 
@@ -315,7 +303,7 @@ static int dma_map_sg_nonforce(struct device *dev, struct scatterlist *sg,
 
 		if (nonforced_iommu(dev, addr, s->length)) {
 			addr = dma_map_area(dev, addr, s->length, dir, 0);
-			if (addr == bad_dma_addr) {
+			if (addr == DMA_MAPPING_ERROR) {
 				if (i > 0)
 					gart_unmap_sg(dev, sg, i, dir, 0);
 				nents = 0;
@@ -471,7 +459,7 @@ static int gart_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 
 	iommu_full(dev, pages << PAGE_SHIFT, dir);
 	for_each_sg(sg, s, nents, i)
-		s->dma_address = bad_dma_addr;
+		s->dma_address = DMA_MAPPING_ERROR;
 	return 0;
 }
 
@@ -490,7 +478,7 @@ gart_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_addr,
 	*dma_addr = dma_map_area(dev, virt_to_phys(vaddr), size,
 			DMA_BIDIRECTIONAL, (1UL << get_order(size)) - 1);
 	flush_gart();
-	if (unlikely(*dma_addr == bad_dma_addr))
+	if (unlikely(*dma_addr == DMA_MAPPING_ERROR))
 		goto out_free;
 	return vaddr;
 out_free:
@@ -507,11 +495,6 @@ gart_free_coherent(struct device *dev, size_t size, void *vaddr,
 	dma_direct_free_pages(dev, size, vaddr, dma_addr, attrs);
 }
 
-static int gart_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return (dma_addr == bad_dma_addr);
-}
-
 static int no_agp;
 
 static __init unsigned long check_iommu_size(unsigned long aper, u64 aper_size)
@@ -695,7 +678,6 @@ static const struct dma_map_ops gart_dma_ops = {
 	.unmap_page			= gart_unmap_page,
 	.alloc				= gart_alloc_coherent,
 	.free				= gart_free_coherent,
-	.mapping_error			= gart_mapping_error,
 	.dma_supported			= dma_direct_supported,
 };
 
@@ -784,19 +766,12 @@ int __init gart_iommu_init(void)
 	}
 #endif
 
-	/*
-	 * Out of IOMMU space handling.
-	 * Reserve some invalid pages at the beginning of the GART.
-	 */
-	bitmap_set(iommu_gart_bitmap, 0, EMERGENCY_PAGES);
-
 	pr_info("PCI-DMA: Reserving %luMB of IOMMU area in the AGP aperture\n",
 	       iommu_size >> 20);
 
 	agp_memory_reserved	= iommu_size;
 	iommu_start		= aper_size - iommu_size;
 	iommu_bus_base		= info.aper_base + iommu_start;
-	bad_dma_addr		= iommu_bus_base;
 	iommu_gatt_base		= agp_gatt_table + (iommu_start>>PAGE_SHIFT);
 
 	/*
@@ -838,8 +813,6 @@ int __init gart_iommu_init(void)
 	if (!scratch)
 		panic("Cannot allocate iommu scratch page");
 	gart_unmapped_entry = GPTE_ENCODE(__pa(scratch));
-	for (i = EMERGENCY_PAGES; i < iommu_pages; i++)
-		iommu_gatt_base[i] = gart_unmapped_entry;
 
 	flush_gart();
 	dma_ops = &gart_dma_ops;
-- 
2.19.1


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

* [PATCH 18/24] x86/calgary: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (16 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 17/24] x86/amd_gart: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 19/24] iommu: " Christoph Hellwig
                   ` (6 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Return DMA_MAPPING_ERROR instead of the magic bad_dma_addr on a dma
mapping failure and let the core dma-mapping code handle the rest.

Remove the magic EMERGENCY_PAGES that the bad_dma_addr gets redirected to.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/x86/kernel/pci-calgary_64.c | 29 +++++++----------------------
 1 file changed, 7 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kernel/pci-calgary_64.c b/arch/x86/kernel/pci-calgary_64.c
index bbfc8b1e9104..e76ec1b8ed1f 100644
--- a/arch/x86/kernel/pci-calgary_64.c
+++ b/arch/x86/kernel/pci-calgary_64.c
@@ -51,8 +51,6 @@
 #include <asm/x86_init.h>
 #include <asm/iommu_table.h>
 
-#define CALGARY_MAPPING_ERROR	0
-
 #ifdef CONFIG_CALGARY_IOMMU_ENABLED_BY_DEFAULT
 int use_calgary __read_mostly = 1;
 #else
@@ -157,8 +155,6 @@ static const unsigned long phb_debug_offsets[] = {
 
 #define PHB_DEBUG_STUFF_OFFSET	0x0020
 
-#define EMERGENCY_PAGES 32 /* = 128KB */
-
 unsigned int specified_table_size = TCE_TABLE_SIZE_UNSPECIFIED;
 static int translate_empty_slots __read_mostly = 0;
 static int calgary_detected __read_mostly = 0;
@@ -255,7 +251,7 @@ static unsigned long iommu_range_alloc(struct device *dev,
 			if (panic_on_overflow)
 				panic("Calgary: fix the allocator.\n");
 			else
-				return CALGARY_MAPPING_ERROR;
+				return DMA_MAPPING_ERROR;
 		}
 	}
 
@@ -274,11 +270,10 @@ static dma_addr_t iommu_alloc(struct device *dev, struct iommu_table *tbl,
 	dma_addr_t ret;
 
 	entry = iommu_range_alloc(dev, tbl, npages);
-
-	if (unlikely(entry == CALGARY_MAPPING_ERROR)) {
+	if (unlikely(entry == DMA_MAPPING_ERROR)) {
 		pr_warn("failed to allocate %u pages in iommu %p\n",
 			npages, tbl);
-		return CALGARY_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 	}
 
 	/* set the return dma address */
@@ -298,8 +293,7 @@ static void iommu_free(struct iommu_table *tbl, dma_addr_t dma_addr,
 	unsigned long flags;
 
 	/* were we called with bad_dma_address? */
-	badend = CALGARY_MAPPING_ERROR + (EMERGENCY_PAGES * PAGE_SIZE);
-	if (unlikely(dma_addr < badend)) {
+	if (unlikely(dma_addr == DMA_MAPPING_ERROR)) {
 		WARN(1, KERN_ERR "Calgary: driver tried unmapping bad DMA "
 		       "address 0x%Lx\n", dma_addr);
 		return;
@@ -383,7 +377,7 @@ static int calgary_map_sg(struct device *dev, struct scatterlist *sg,
 		npages = iommu_num_pages(vaddr, s->length, PAGE_SIZE);
 
 		entry = iommu_range_alloc(dev, tbl, npages);
-		if (entry == CALGARY_MAPPING_ERROR) {
+		if (entry == DMA_MAPPING_ERROR) {
 			/* makes sure unmap knows to stop */
 			s->dma_length = 0;
 			goto error;
@@ -401,7 +395,7 @@ static int calgary_map_sg(struct device *dev, struct scatterlist *sg,
 error:
 	calgary_unmap_sg(dev, sg, nelems, dir, 0);
 	for_each_sg(sg, s, nelems, i) {
-		sg->dma_address = CALGARY_MAPPING_ERROR;
+		sg->dma_address = DMA_MAPPING_ERROR;
 		sg->dma_length = 0;
 	}
 	return 0;
@@ -454,7 +448,7 @@ static void* calgary_alloc_coherent(struct device *dev, size_t size,
 
 	/* set up tces to cover the allocated range */
 	mapping = iommu_alloc(dev, tbl, ret, npages, DMA_BIDIRECTIONAL);
-	if (mapping == CALGARY_MAPPING_ERROR)
+	if (mapping == DMA_MAPPING_ERROR)
 		goto free;
 	*dma_handle = mapping;
 	return ret;
@@ -479,11 +473,6 @@ static void calgary_free_coherent(struct device *dev, size_t size,
 	free_pages((unsigned long)vaddr, get_order(size));
 }
 
-static int calgary_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == CALGARY_MAPPING_ERROR;
-}
-
 static const struct dma_map_ops calgary_dma_ops = {
 	.alloc = calgary_alloc_coherent,
 	.free = calgary_free_coherent,
@@ -491,7 +480,6 @@ static const struct dma_map_ops calgary_dma_ops = {
 	.unmap_sg = calgary_unmap_sg,
 	.map_page = calgary_map_page,
 	.unmap_page = calgary_unmap_page,
-	.mapping_error = calgary_mapping_error,
 	.dma_supported = dma_direct_supported,
 };
 
@@ -739,9 +727,6 @@ static void __init calgary_reserve_regions(struct pci_dev *dev)
 	u64 start;
 	struct iommu_table *tbl = pci_iommu(dev->bus);
 
-	/* reserve EMERGENCY_PAGES from bad_dma_address and up */
-	iommu_range_reserve(tbl, CALGARY_MAPPING_ERROR, EMERGENCY_PAGES);
-
 	/* avoid the BIOS/VGA first 640KB-1MB region */
 	/* for CalIOC2 - avoid the entire first MB */
 	if (is_calgary(dev->device)) {
-- 
2.19.1


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

* [PATCH 19/24] iommu: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (17 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 18/24] x86/calgary: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 20/24] iommu/intel: small map_page cleanup Christoph Hellwig
                   ` (5 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Return DMA_MAPPING_ERROR instead of 0 on a dma mapping failure and let
the core dma-mapping code handle the rest.

Note that the existing code used AMD_IOMMU_MAPPING_ERROR to check from
a 0 return from the IOVA allocator, which is replaced with an explicit
0 as in the implementation and other users of that interface.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/iommu/amd_iommu.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 1167ff0416cf..c5d6c7c42b0a 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -55,8 +55,6 @@
 #include "amd_iommu_types.h"
 #include "irq_remapping.h"
 
-#define AMD_IOMMU_MAPPING_ERROR	0
-
 #define CMD_SET_TYPE(cmd, t) ((cmd)->data[1] |= ((t) << 28))
 
 #define LOOP_TIMEOUT	100000
@@ -2339,7 +2337,7 @@ static dma_addr_t __map_single(struct device *dev,
 	paddr &= PAGE_MASK;
 
 	address = dma_ops_alloc_iova(dev, dma_dom, pages, dma_mask);
-	if (address == AMD_IOMMU_MAPPING_ERROR)
+	if (!address)
 		goto out;
 
 	prot = dir2prot(direction);
@@ -2376,7 +2374,7 @@ static dma_addr_t __map_single(struct device *dev,
 
 	dma_ops_free_iova(dma_dom, address, pages);
 
-	return AMD_IOMMU_MAPPING_ERROR;
+	return DMA_MAPPING_ERROR;
 }
 
 /*
@@ -2427,7 +2425,7 @@ static dma_addr_t map_page(struct device *dev, struct page *page,
 	if (PTR_ERR(domain) == -EINVAL)
 		return (dma_addr_t)paddr;
 	else if (IS_ERR(domain))
-		return AMD_IOMMU_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 
 	dma_mask = *dev->dma_mask;
 	dma_dom = to_dma_ops_domain(domain);
@@ -2504,7 +2502,7 @@ static int map_sg(struct device *dev, struct scatterlist *sglist,
 	npages = sg_num_pages(dev, sglist, nelems);
 
 	address = dma_ops_alloc_iova(dev, dma_dom, npages, dma_mask);
-	if (address == AMD_IOMMU_MAPPING_ERROR)
+	if (address == DMA_MAPPING_ERROR)
 		goto out_err;
 
 	prot = dir2prot(direction);
@@ -2627,7 +2625,7 @@ static void *alloc_coherent(struct device *dev, size_t size,
 	*dma_addr = __map_single(dev, dma_dom, page_to_phys(page),
 				 size, DMA_BIDIRECTIONAL, dma_mask);
 
-	if (*dma_addr == AMD_IOMMU_MAPPING_ERROR)
+	if (*dma_addr == DMA_MAPPING_ERROR)
 		goto out_free;
 
 	return page_address(page);
@@ -2678,11 +2676,6 @@ static int amd_iommu_dma_supported(struct device *dev, u64 mask)
 	return check_device(dev);
 }
 
-static int amd_iommu_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == AMD_IOMMU_MAPPING_ERROR;
-}
-
 static const struct dma_map_ops amd_iommu_dma_ops = {
 	.alloc		= alloc_coherent,
 	.free		= free_coherent,
@@ -2691,7 +2684,6 @@ static const struct dma_map_ops amd_iommu_dma_ops = {
 	.map_sg		= map_sg,
 	.unmap_sg	= unmap_sg,
 	.dma_supported	= amd_iommu_dma_supported,
-	.mapping_error	= amd_iommu_mapping_error,
 };
 
 static int init_reserved_iova_ranges(void)
-- 
2.19.1


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

* [PATCH 20/24] iommu/intel: small map_page cleanup
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (18 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 19/24] iommu: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 21/24] iommu/vt-d: remove the mapping_error dma_map_ops method Christoph Hellwig
                   ` (4 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Pass the page + offset to the low-level __iommu_map_single helper
(which gets renamed to fit the new calling conventions) as both
callers have the page at hand.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/iommu/intel-iommu.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index f3ccf025108b..3b8a7acac7a1 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3597,9 +3597,11 @@ static int iommu_no_mapping(struct device *dev)
 	return 0;
 }
 
-static dma_addr_t __intel_map_single(struct device *dev, phys_addr_t paddr,
-				     size_t size, int dir, u64 dma_mask)
+static dma_addr_t __intel_map_page(struct device *dev, struct page *page,
+				   unsigned long offset, size_t size, int dir,
+				   u64 dma_mask)
 {
+	phys_addr_t paddr = page_to_phys(page) + offset;
 	struct dmar_domain *domain;
 	phys_addr_t start_paddr;
 	unsigned long iova_pfn;
@@ -3661,8 +3663,7 @@ static dma_addr_t intel_map_page(struct device *dev, struct page *page,
 				 enum dma_data_direction dir,
 				 unsigned long attrs)
 {
-	return __intel_map_single(dev, page_to_phys(page) + offset, size,
-				  dir, *dev->dma_mask);
+	return __intel_map_page(dev, page, offset, size, dir, *dev->dma_mask);
 }
 
 static void intel_unmap(struct device *dev, dma_addr_t dev_addr, size_t size)
@@ -3753,9 +3754,8 @@ static void *intel_alloc_coherent(struct device *dev, size_t size,
 		return NULL;
 	memset(page_address(page), 0, size);
 
-	*dma_handle = __intel_map_single(dev, page_to_phys(page), size,
-					 DMA_BIDIRECTIONAL,
-					 dev->coherent_dma_mask);
+	*dma_handle = __intel_map_page(dev, page, 0, size, DMA_BIDIRECTIONAL,
+				       dev->coherent_dma_mask);
 	if (*dma_handle)
 		return page_address(page);
 	if (!dma_release_from_contiguous(dev, page, size >> PAGE_SHIFT))
-- 
2.19.1


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

* [PATCH 21/24] iommu/vt-d: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (19 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 20/24] iommu/intel: small map_page cleanup Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 22/24] iommu/dma-iommu: " Christoph Hellwig
                   ` (3 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Return DMA_MAPPING_ERROR instead of 0 on a dma mapping failure and let
the core dma-mapping code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/iommu/intel-iommu.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 3b8a7acac7a1..a5704155932d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3617,7 +3617,7 @@ static dma_addr_t __intel_map_page(struct device *dev, struct page *page,
 
 	domain = get_valid_domain_for_dev(dev);
 	if (!domain)
-		return 0;
+		return DMA_MAPPING_ERROR;
 
 	iommu = domain_get_iommu(domain);
 	size = aligned_nrpages(paddr, size);
@@ -3655,7 +3655,7 @@ static dma_addr_t __intel_map_page(struct device *dev, struct page *page,
 		free_iova_fast(&domain->iovad, iova_pfn, dma_to_mm_pfn(size));
 	pr_err("Device %s request: %zx@%llx dir %d --- failed\n",
 		dev_name(dev), size, (unsigned long long)paddr, dir);
-	return 0;
+	return DMA_MAPPING_ERROR;
 }
 
 static dma_addr_t intel_map_page(struct device *dev, struct page *page,
@@ -3756,7 +3756,7 @@ static void *intel_alloc_coherent(struct device *dev, size_t size,
 
 	*dma_handle = __intel_map_page(dev, page, 0, size, DMA_BIDIRECTIONAL,
 				       dev->coherent_dma_mask);
-	if (*dma_handle)
+	if (*dma_handle != DMA_MAPPING_ERROR)
 		return page_address(page);
 	if (!dma_release_from_contiguous(dev, page, size >> PAGE_SHIFT))
 		__free_pages(page, order);
@@ -3865,11 +3865,6 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
 	return nelems;
 }
 
-static int intel_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return !dma_addr;
-}
-
 static const struct dma_map_ops intel_dma_ops = {
 	.alloc = intel_alloc_coherent,
 	.free = intel_free_coherent,
@@ -3877,7 +3872,6 @@ static const struct dma_map_ops intel_dma_ops = {
 	.unmap_sg = intel_unmap_sg,
 	.map_page = intel_map_page,
 	.unmap_page = intel_unmap_page,
-	.mapping_error = intel_mapping_error,
 	.dma_supported = dma_direct_supported,
 };
 
-- 
2.19.1


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

* [PATCH 22/24] iommu/dma-iommu: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (20 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 21/24] iommu/vt-d: remove the mapping_error dma_map_ops method Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 23/24] xen-swiotlb: " Christoph Hellwig
                   ` (2 subsequent siblings)
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Return DMA_MAPPING_ERROR instead of 0 on a dma mapping failure and let
the core dma-mapping code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/arm64/mm/dma-mapping.c |  7 +++----
 drivers/iommu/dma-iommu.c   | 23 ++++++++---------------
 include/linux/dma-iommu.h   |  1 -
 3 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index fdc26ea5036c..4cc70029cf8d 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -397,7 +397,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
 			return NULL;
 
 		*handle = iommu_dma_map_page(dev, page, 0, iosize, ioprot);
-		if (iommu_dma_mapping_error(dev, *handle)) {
+		if (*handle == DMA_MAPPING_ERROR) {
 			if (coherent)
 				__free_pages(page, get_order(size));
 			else
@@ -414,7 +414,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
 			return NULL;
 
 		*handle = iommu_dma_map_page(dev, page, 0, iosize, ioprot);
-		if (iommu_dma_mapping_error(dev, *handle)) {
+		if (*handle == DMA_MAPPING_ERROR) {
 			dma_release_from_contiguous(dev, page,
 						    size >> PAGE_SHIFT);
 			return NULL;
@@ -574,7 +574,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
 	dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
 
 	if (!coherent && !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
-	    !iommu_dma_mapping_error(dev, dev_addr))
+	    dev_addr != DMA_MAPPING_ERROR)
 		__dma_map_area(page_address(page) + offset, size, dir);
 
 	return dev_addr;
@@ -657,7 +657,6 @@ static const struct dma_map_ops iommu_dma_ops = {
 	.sync_sg_for_device = __iommu_sync_sg_for_device,
 	.map_resource = iommu_dma_map_resource,
 	.unmap_resource = iommu_dma_unmap_resource,
-	.mapping_error = iommu_dma_mapping_error,
 };
 
 static int __init __iommu_dma_init(void)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d1b04753b204..60c7e9e9901e 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -32,8 +32,6 @@
 #include <linux/scatterlist.h>
 #include <linux/vmalloc.h>
 
-#define IOMMU_MAPPING_ERROR	0
-
 struct iommu_dma_msi_page {
 	struct list_head	list;
 	dma_addr_t		iova;
@@ -523,7 +521,7 @@ void iommu_dma_free(struct device *dev, struct page **pages, size_t size,
 {
 	__iommu_dma_unmap(iommu_get_dma_domain(dev), *handle, size);
 	__iommu_dma_free_pages(pages, PAGE_ALIGN(size) >> PAGE_SHIFT);
-	*handle = IOMMU_MAPPING_ERROR;
+	*handle = DMA_MAPPING_ERROR;
 }
 
 /**
@@ -556,7 +554,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
 	dma_addr_t iova;
 	unsigned int count, min_size, alloc_sizes = domain->pgsize_bitmap;
 
-	*handle = IOMMU_MAPPING_ERROR;
+	*handle = DMA_MAPPING_ERROR;
 
 	min_size = alloc_sizes & -alloc_sizes;
 	if (min_size < PAGE_SIZE) {
@@ -649,11 +647,11 @@ static dma_addr_t __iommu_dma_map(struct device *dev, phys_addr_t phys,
 
 	iova = iommu_dma_alloc_iova(domain, size, dma_get_mask(dev), dev);
 	if (!iova)
-		return IOMMU_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 
 	if (iommu_map(domain, iova, phys - iova_off, size, prot)) {
 		iommu_dma_free_iova(cookie, iova, size);
-		return IOMMU_MAPPING_ERROR;
+		return DMA_MAPPING_ERROR;
 	}
 	return iova + iova_off;
 }
@@ -694,7 +692,7 @@ static int __finalise_sg(struct device *dev, struct scatterlist *sg, int nents,
 
 		s->offset += s_iova_off;
 		s->length = s_length;
-		sg_dma_address(s) = IOMMU_MAPPING_ERROR;
+		sg_dma_address(s) = DMA_MAPPING_ERROR;
 		sg_dma_len(s) = 0;
 
 		/*
@@ -737,11 +735,11 @@ static void __invalidate_sg(struct scatterlist *sg, int nents)
 	int i;
 
 	for_each_sg(sg, s, nents, i) {
-		if (sg_dma_address(s) != IOMMU_MAPPING_ERROR)
+		if (sg_dma_address(s) != DMA_MAPPING_ERROR)
 			s->offset += sg_dma_address(s);
 		if (sg_dma_len(s))
 			s->length = sg_dma_len(s);
-		sg_dma_address(s) = IOMMU_MAPPING_ERROR;
+		sg_dma_address(s) = DMA_MAPPING_ERROR;
 		sg_dma_len(s) = 0;
 	}
 }
@@ -858,11 +856,6 @@ void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
 	__iommu_dma_unmap(iommu_get_dma_domain(dev), handle, size);
 }
 
-int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == IOMMU_MAPPING_ERROR;
-}
-
 static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
 		phys_addr_t msi_addr, struct iommu_domain *domain)
 {
@@ -882,7 +875,7 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
 		return NULL;
 
 	iova = __iommu_dma_map(dev, msi_addr, size, prot, domain);
-	if (iommu_dma_mapping_error(dev, iova))
+	if (iova == DMA_MAPPING_ERROR)
 		goto out_free_page;
 
 	INIT_LIST_HEAD(&msi_page->list);
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index e8ca5e654277..e760dc5d1fa8 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -69,7 +69,6 @@ dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys,
 		size_t size, enum dma_data_direction dir, unsigned long attrs);
 void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
 		size_t size, enum dma_data_direction dir, unsigned long attrs);
-int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
 
 /* The DMA API isn't _quite_ the whole story, though... */
 void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
-- 
2.19.1


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

* [PATCH 23/24] xen-swiotlb: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (21 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 22/24] iommu/dma-iommu: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 14:03 ` [PATCH 24/24] dma-mapping: " Christoph Hellwig
  2018-11-22 16:50 ` remove the ->mapping_error method from dma_map_ops V2 Linus Torvalds
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

Return DMA_MAPPING_ERROR instead of 0 on a dma mapping failure and let
the core dma-mapping code handle the rest.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/xen/swiotlb-xen.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 2a7f545bd0b5..6dc969d5ea2f 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -53,8 +53,6 @@
  * API.
  */
 
-#define XEN_SWIOTLB_ERROR_CODE	(~(dma_addr_t)0x0)
-
 static char *xen_io_tlb_start, *xen_io_tlb_end;
 static unsigned long xen_io_tlb_nslabs;
 /*
@@ -406,7 +404,7 @@ static dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
 	map = swiotlb_tbl_map_single(dev, start_dma_addr, phys, size, dir,
 				     attrs);
 	if (map == SWIOTLB_MAP_ERROR)
-		return XEN_SWIOTLB_ERROR_CODE;
+		return DMA_MAPPING_ERROR;
 
 	dev_addr = xen_phys_to_bus(map);
 	xen_dma_map_page(dev, pfn_to_page(map >> PAGE_SHIFT),
@@ -421,7 +419,7 @@ static dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
 	attrs |= DMA_ATTR_SKIP_CPU_SYNC;
 	swiotlb_tbl_unmap_single(dev, map, size, dir, attrs);
 
-	return XEN_SWIOTLB_ERROR_CODE;
+	return DMA_MAPPING_ERROR;
 }
 
 /*
@@ -700,11 +698,6 @@ xen_swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt,
 	return dma_common_get_sgtable(dev, sgt, cpu_addr, handle, size, attrs);
 }
 
-static int xen_swiotlb_mapping_error(struct device *dev, dma_addr_t dma_addr)
-{
-	return dma_addr == XEN_SWIOTLB_ERROR_CODE;
-}
-
 const struct dma_map_ops xen_swiotlb_dma_ops = {
 	.alloc = xen_swiotlb_alloc_coherent,
 	.free = xen_swiotlb_free_coherent,
@@ -719,5 +712,4 @@ const struct dma_map_ops xen_swiotlb_dma_ops = {
 	.dma_supported = xen_swiotlb_dma_supported,
 	.mmap = xen_swiotlb_dma_mmap,
 	.get_sgtable = xen_swiotlb_get_sgtable,
-	.mapping_error	= xen_swiotlb_mapping_error,
 };
-- 
2.19.1


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

* [PATCH 24/24] dma-mapping: remove the mapping_error dma_map_ops method
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (22 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 23/24] xen-swiotlb: " Christoph Hellwig
@ 2018-11-22 14:03 ` Christoph Hellwig
  2018-11-22 16:50 ` remove the ->mapping_error method from dma_map_ops V2 Linus Torvalds
  24 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-22 14:03 UTC (permalink / raw)
  To: iommu
  Cc: Linus Torvalds, Jon Mason, Joerg Roedel, David Woodhouse,
	Marek Szyprowski, Robin Murphy, x86, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, linux-kernel

No users left except for vmd which just forwards it.  Also switch
dma_mapping_error to an explicit bool return value.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/vmd.c |  6 ------
 include/linux/dma-mapping.h  | 13 ++-----------
 2 files changed, 2 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index e50b0b5815ff..98ce79eac128 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -394,11 +394,6 @@ static void vmd_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
 	vmd_dma_ops(dev)->sync_sg_for_device(to_vmd_dev(dev), sg, nents, dir);
 }
 
-static int vmd_mapping_error(struct device *dev, dma_addr_t addr)
-{
-	return vmd_dma_ops(dev)->mapping_error(to_vmd_dev(dev), addr);
-}
-
 static int vmd_dma_supported(struct device *dev, u64 mask)
 {
 	return vmd_dma_ops(dev)->dma_supported(to_vmd_dev(dev), mask);
@@ -446,7 +441,6 @@ static void vmd_setup_dma_ops(struct vmd_dev *vmd)
 	ASSIGN_VMD_DMA_OPS(source, dest, sync_single_for_device);
 	ASSIGN_VMD_DMA_OPS(source, dest, sync_sg_for_cpu);
 	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);
 	ASSIGN_VMD_DMA_OPS(source, dest, get_required_mask);
 	add_dma_domain(domain);
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index c323c539b7cb..c250e2ca6dfc 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -128,7 +128,6 @@ struct dma_map_ops {
 				   enum dma_data_direction dir);
 	void (*cache_sync)(struct device *dev, void *vaddr, size_t size,
 			enum dma_data_direction direction);
-	int (*mapping_error)(struct device *dev, dma_addr_t dma_addr);
 	int (*dma_supported)(struct device *dev, u64 mask);
 	u64 (*get_required_mask)(struct device *dev);
 };
@@ -573,18 +572,10 @@ static inline void dma_free_coherent(struct device *dev, size_t size,
 	return dma_free_attrs(dev, size, cpu_addr, dma_handle, 0);
 }
 
-static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
+static inline bool dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
 {
-	const struct dma_map_ops *ops = get_dma_ops(dev);
-
 	debug_dma_mapping_error(dev, dma_addr);
-
-	if (dma_addr == DMA_MAPPING_ERROR)
-		return 1;
-
-	if (ops->mapping_error)
-		return ops->mapping_error(dev, dma_addr);
-	return 0;
+	return dma_addr == DMA_MAPPING_ERROR;
 }
 
 static inline void dma_check_mask(struct device *dev, u64 mask)
-- 
2.19.1


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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
                   ` (23 preceding siblings ...)
  2018-11-22 14:03 ` [PATCH 24/24] dma-mapping: " Christoph Hellwig
@ 2018-11-22 16:50 ` Linus Torvalds
  2018-11-22 17:07   ` Russell King - ARM Linux
  24 siblings, 1 reply; 53+ messages in thread
From: Linus Torvalds @ 2018-11-22 16:50 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: iommu, jdmason, joro, David Woodhouse, m.szyprowski,
	robin.murphy, the arch/x86 maintainers, linux-alpha,
	linux-arm-kernel, linux-ia64, linux-parisc, xen-devel,
	linux-arch, Linux List Kernel Mailing

On Thu, Nov 22, 2018 at 6:03 AM Christoph Hellwig <hch@lst.de> wrote:
>
> The 0 return doesn't work for direct mappings that have ram at address
> zero and a lot of IOMMUs that start allocating bus space from address
> zero, so we can't consolidate on that, but I think we can move everyone
> to all-Fs, which the patch here does.

Hmm. Maybe not limit it to just all ones, but actually use the
(standard, for the kernel) IS_ERR_VALUE()?

That basically reserves the last 4095 values in an unsigned long for
error values.

Then those functions could actually return *what* error they
encountered, using just plain

        return -ENOMEM;

or whatever?

               Linus

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-22 16:50 ` remove the ->mapping_error method from dma_map_ops V2 Linus Torvalds
@ 2018-11-22 17:07   ` Russell King - ARM Linux
  2018-11-22 17:09     ` Linus Torvalds
  0 siblings, 1 reply; 53+ messages in thread
From: Russell King - ARM Linux @ 2018-11-22 17:07 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Christoph Hellwig, linux-arch, linux-alpha, linux-ia64,
	linux-parisc, David Woodhouse, joro, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, jdmason, xen-devel,
	robin.murphy, linux-arm-kernel, m.szyprowski

On Thu, Nov 22, 2018 at 08:50:47AM -0800, Linus Torvalds wrote:
> On Thu, Nov 22, 2018 at 6:03 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > The 0 return doesn't work for direct mappings that have ram at address
> > zero and a lot of IOMMUs that start allocating bus space from address
> > zero, so we can't consolidate on that, but I think we can move everyone
> > to all-Fs, which the patch here does.
> 
> Hmm. Maybe not limit it to just all ones, but actually use the
> (standard, for the kernel) IS_ERR_VALUE()?
> 
> That basically reserves the last 4095 values in an unsigned long for
> error values.
> 
> Then those functions could actually return *what* error they
> encountered, using just plain
> 
>         return -ENOMEM;
> 
> or whatever?

Linus,

I'm afraid that won't work very well - 32 bit platforms with 64-bit
addresses (LPAE) would have dma_addr_t as a 64-bit value, which
wouldn't fit into an unsigned long.

IS_ERR_VALUE() would cast a 64-bit DMA address down to a 32-bit
pointer (effectively masking with 0xffffffff).  It would have the
effect of making (eg) 0xXXXXXXXX_fffffVVV an error, where XXXXXXXX
are any of the top 32-bits of a 64-bit bus address, and VVV is the
error code value.

That could be a problem if you hit it in several places throughout
your available RAM... we'd have to mark every top page of RAM in
a naturally aligned 4GB as unusable, as well as block the top page
in natually aligned 4GB blocks from IOMMUs... and then what about
buses that have weird offsets...

So, I don't think the IS_ERR_VALUE() would work very well.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-22 17:07   ` Russell King - ARM Linux
@ 2018-11-22 17:09     ` Linus Torvalds
  2018-11-22 17:14       ` Russell King - ARM Linux
  2018-11-22 17:52       ` Robin Murphy
  0 siblings, 2 replies; 53+ messages in thread
From: Linus Torvalds @ 2018-11-22 17:09 UTC (permalink / raw)
  To: linux
  Cc: Christoph Hellwig, linux-arch, linux-alpha, linux-ia64,
	linux-parisc, David Woodhouse, joro, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, jdmason, xen-devel,
	robin.murphy, linux-arm-kernel, m.szyprowski

On Thu, Nov 22, 2018 at 9:07 AM Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
>
> I'm afraid that won't work very well - 32 bit platforms with 64-bit
> addresses (LPAE) would have dma_addr_t as a 64-bit value, which
> wouldn't fit into an unsigned long.

Good point. So we'd have to have a special IS_DMA_ERR() function that
takes a dma_addr_t and checks the same "is it the top 4095 values".

Because I'd still prefer to allow people to return the _actual_ error,
and to have "return -Exyz" as the error case. That part still DTRT
even with dma_addr_t.

              Linus

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-22 17:09     ` Linus Torvalds
@ 2018-11-22 17:14       ` Russell King - ARM Linux
  2018-11-22 17:52       ` Robin Murphy
  1 sibling, 0 replies; 53+ messages in thread
From: Russell King - ARM Linux @ 2018-11-22 17:14 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Christoph Hellwig, linux-arch, linux-alpha, linux-ia64,
	linux-parisc, David Woodhouse, joro, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, jdmason, xen-devel,
	robin.murphy, linux-arm-kernel, m.szyprowski

On Thu, Nov 22, 2018 at 09:09:47AM -0800, Linus Torvalds wrote:
> On Thu, Nov 22, 2018 at 9:07 AM Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> >
> > I'm afraid that won't work very well - 32 bit platforms with 64-bit
> > addresses (LPAE) would have dma_addr_t as a 64-bit value, which
> > wouldn't fit into an unsigned long.
> 
> Good point. So we'd have to have a special IS_DMA_ERR() function that
> takes a dma_addr_t and checks the same "is it the top 4095 values".

No problem with that.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-22 17:09     ` Linus Torvalds
  2018-11-22 17:14       ` Russell King - ARM Linux
@ 2018-11-22 17:52       ` Robin Murphy
  2018-11-22 17:55         ` Linus Torvalds
  2018-11-23 10:49         ` Joerg Roedel
  1 sibling, 2 replies; 53+ messages in thread
From: Robin Murphy @ 2018-11-22 17:52 UTC (permalink / raw)
  To: Linus Torvalds, linux
  Cc: Christoph Hellwig, linux-arch, linux-alpha, linux-ia64,
	linux-parisc, David Woodhouse, joro, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, jdmason, xen-devel,
	linux-arm-kernel, m.szyprowski

On 22/11/2018 17:09, Linus Torvalds wrote:
> On Thu, Nov 22, 2018 at 9:07 AM Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>>
>> I'm afraid that won't work very well - 32 bit platforms with 64-bit
>> addresses (LPAE) would have dma_addr_t as a 64-bit value, which
>> wouldn't fit into an unsigned long.
> 
> Good point. So we'd have to have a special IS_DMA_ERR() function that
> takes a dma_addr_t and checks the same "is it the top 4095 values".
> 
> Because I'd still prefer to allow people to return the _actual_ error,
> and to have "return -Exyz" as the error case. That part still DTRT
> even with dma_addr_t.

Unfortunately, with things like the top-down IOVA allocator, and 32-bit 
systems in general, "the top 4095" values may well still be valid 
addresses - we're relying on a 1-byte mapping of the very top byte of 
memory/IOVA space being sufficiently ridiculous that no real code would 
ever do that, but even a 4-byte mapping of the top 4 bytes is within the 
realms of the plausible (I've definitely seen the USB layer make 8-byte 
mappings from any old offset within a page, for example).

Thus we'd have to go with the extra complication of detecting and 
carving out problematic memory maps in those corner cases as Russell 
alluded to, for the sake of better error reporting in places where, in 
the majority of cases, there's not really all that many ways to go 
wrong. Off the top of my head, I guess:

-EINVAL if the address is outside the device's DMA mask (and there is no 
IOMMU or bounce buffer).

-ENOSPC if there is an IOMMU or bounce buffer, but no free IOVA/buffer 
space currently.

-ESOMETHINGWENTWRONGWITHIOMMUPAGETABLES or similar, because it's not 
like the caller can treat that as anything other than an opaque failure 
anyway.

The only immediate benefit I can see is that we could distinguish cases 
like the first which can never possibly succeed, and thus callers could 
actually give up instead of doing what various subsystems currently do 
and retry the exact same mapping indefinitely on the apparent assumption 
that errors must be transient.

Robin.

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-22 17:52       ` Robin Murphy
@ 2018-11-22 17:55         ` Linus Torvalds
  2018-11-22 18:05           ` Russell King - ARM Linux
  2018-11-23  6:55           ` Christoph Hellwig
  2018-11-23 10:49         ` Joerg Roedel
  1 sibling, 2 replies; 53+ messages in thread
From: Linus Torvalds @ 2018-11-22 17:55 UTC (permalink / raw)
  To: robin.murphy
  Cc: linux, Christoph Hellwig, linux-arch, linux-alpha, linux-ia64,
	linux-parisc, David Woodhouse, joro, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, jdmason, xen-devel,
	linux-arm-kernel, m.szyprowski

On Thu, Nov 22, 2018 at 9:52 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> Unfortunately, with things like the top-down IOVA allocator, and 32-bit
> systems in general, "the top 4095" values may well still be valid
> addresses -

Ugh.

> The only immediate benefit I can see is that we could distinguish cases
> like the first which can never possibly succeed, and thus callers could
> actually give up instead of doing what various subsystems currently do
> and retry the exact same mapping indefinitely on the apparent assumption
> that errors must be transient.

No, the big immediate benefit of allowing "return -EINVAL" etc is
simply legibility and error avoidance.

It's basically how pretty much all the rest of the kernel returns
errors, so not only is it very obvious, it's also what people do
without even thinking.. So it would be good to work.

                       Linus

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-22 17:55         ` Linus Torvalds
@ 2018-11-22 18:05           ` Russell King - ARM Linux
  2018-11-23  6:57             ` Christoph Hellwig
  2018-11-23  6:55           ` Christoph Hellwig
  1 sibling, 1 reply; 53+ messages in thread
From: Russell King - ARM Linux @ 2018-11-22 18:05 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: robin.murphy, linux-arch, linux-ia64, linux-parisc, joro,
	the arch/x86 maintainers, Linux List Kernel Mailing, iommu,
	linux-alpha, xen-devel, jdmason, David Woodhouse,
	Christoph Hellwig, linux-arm-kernel, m.szyprowski

On Thu, Nov 22, 2018 at 09:55:25AM -0800, Linus Torvalds wrote:
> On Thu, Nov 22, 2018 at 9:52 AM Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > Unfortunately, with things like the top-down IOVA allocator, and 32-bit
> > systems in general, "the top 4095" values may well still be valid
> > addresses -
> 
> Ugh.
> 
> > The only immediate benefit I can see is that we could distinguish cases
> > like the first which can never possibly succeed, and thus callers could
> > actually give up instead of doing what various subsystems currently do
> > and retry the exact same mapping indefinitely on the apparent assumption
> > that errors must be transient.
> 
> No, the big immediate benefit of allowing "return -EINVAL" etc is
> simply legibility and error avoidance.
> 
> It's basically how pretty much all the rest of the kernel returns
> errors, so not only is it very obvious, it's also what people do
> without even thinking.. So it would be good to work.

An alternative idea would be to migrate away from the
dma_map_single() and dma_map_page() interfaces that return a
dma_addr_t, and instead have them return an error code or zero
on success.

I've thought for some time that our DMA interfaces aren't particularly
friendly, especially after we had the stuff with PCI DMA which migrated
its way into the DMA API:

DEFINE_DMA_UNMAP_ADDR
DEFINE_DMA_UNMAP_LEN
dma_unmap_*

When coupled that with the requirement that dma_unmap_*() should be
called with the same parameters as dma_map_*(), I wonder why we never
did:

struct dma_map_state {
	dma_addr_t dma_addr;
	whatever's needed;
}

int dma_map_single(struct device *dev, struct dma_map_state *state,
		   void *cpu, size_t len, enum dma_data_direction dir);
void dma_unmap_single(struct device *dev, struct dma_map_state *state);

note the simpler unmap API, which inherently guarantees that the
parameters to the map could be carried over to the unmap - without
our many driver authors having to think about it.

That also paves the way for dma_map_single() to return an error code
or zero.

However, I fear that boat sailed long ago - but maybe its worth
thinking along these lines if we want to sanitise the API now?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-22 17:55         ` Linus Torvalds
  2018-11-22 18:05           ` Russell King - ARM Linux
@ 2018-11-23  6:55           ` Christoph Hellwig
  2018-11-28  7:41             ` Christoph Hellwig
  1 sibling, 1 reply; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-23  6:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: robin.murphy, linux, Christoph Hellwig, linux-arch, linux-alpha,
	linux-ia64, linux-parisc, David Woodhouse, joro,
	the arch/x86 maintainers, Linux List Kernel Mailing, iommu,
	jdmason, xen-devel, linux-arm-kernel, m.szyprowski

On Thu, Nov 22, 2018 at 09:55:25AM -0800, Linus Torvalds wrote:
> No, the big immediate benefit of allowing "return -EINVAL" etc is
> simply legibility and error avoidance.

Well, I can tweak the last patch to return -EINVAL from dma_mapping_error
instead of the old 1 is as bool true.  The callers should all be fine,
although I'd have to audit them.  Still wouldn't help with being able to
return different errors.

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-22 18:05           ` Russell King - ARM Linux
@ 2018-11-23  6:57             ` Christoph Hellwig
  0 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-23  6:57 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Linus Torvalds, robin.murphy, linux-arch, linux-ia64,
	linux-parisc, joro, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, linux-alpha, xen-devel,
	jdmason, David Woodhouse, Christoph Hellwig, linux-arm-kernel,
	m.szyprowski

On Thu, Nov 22, 2018 at 06:05:26PM +0000, Russell King - ARM Linux wrote:
> An alternative idea would be to migrate away from the
> dma_map_single() and dma_map_page() interfaces that return a
> dma_addr_t, and instead have them return an error code or zero
> on success.

See here for a proposal:

https://lists.linuxfoundation.org/pipermail/iommu/2018-November/030912.html

That is just the attr variants, but that would be a start.  Dave didn't
particularly like it, though.

> note the simpler unmap API, which inherently guarantees that the
> parameters to the map could be carried over to the unmap - without
> our many driver authors having to think about it.

The problem is that we can often derive some or all parameters from
field already inherent in the upper layer or hardware interface.  So
for these cases your version would bloat the required data structures.

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-22 17:52       ` Robin Murphy
  2018-11-22 17:55         ` Linus Torvalds
@ 2018-11-23 10:49         ` Joerg Roedel
  2018-11-23 11:01           ` Russell King - ARM Linux
  1 sibling, 1 reply; 53+ messages in thread
From: Joerg Roedel @ 2018-11-23 10:49 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Linus Torvalds, linux, Christoph Hellwig, linux-arch,
	linux-alpha, linux-ia64, linux-parisc, David Woodhouse,
	the arch/x86 maintainers, Linux List Kernel Mailing, iommu,
	jdmason, xen-devel, linux-arm-kernel, m.szyprowski

On Thu, Nov 22, 2018 at 05:52:15PM +0000, Robin Murphy wrote:
> Unfortunately, with things like the top-down IOVA allocator, and 32-bit
> systems in general, "the top 4095" values may well still be valid addresses
> - we're relying on a 1-byte mapping of the very top byte of memory/IOVA
> space being sufficiently ridiculous that no real code would ever do that,
> but even a 4-byte mapping of the top 4 bytes is within the realms of the
> plausible (I've definitely seen the USB layer make 8-byte mappings from any
> old offset within a page, for example).

But we can easily work around that by reserving the top 4k of the first
4GB of IOVA address space in the allocator, no? Then these values are
never returned as valid DMA handles.


Regards,

	Joerg


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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-23 10:49         ` Joerg Roedel
@ 2018-11-23 11:01           ` Russell King - ARM Linux
  2018-11-23 13:03             ` Joerg Roedel
  0 siblings, 1 reply; 53+ messages in thread
From: Russell King - ARM Linux @ 2018-11-23 11:01 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Robin Murphy, Linus Torvalds, Christoph Hellwig, linux-arch,
	linux-alpha, linux-ia64, linux-parisc, David Woodhouse,
	the arch/x86 maintainers, Linux List Kernel Mailing, iommu,
	jdmason, xen-devel, linux-arm-kernel, m.szyprowski

On Fri, Nov 23, 2018 at 11:49:18AM +0100, Joerg Roedel wrote:
> On Thu, Nov 22, 2018 at 05:52:15PM +0000, Robin Murphy wrote:
> > Unfortunately, with things like the top-down IOVA allocator, and 32-bit
> > systems in general, "the top 4095" values may well still be valid addresses
> > - we're relying on a 1-byte mapping of the very top byte of memory/IOVA
> > space being sufficiently ridiculous that no real code would ever do that,
> > but even a 4-byte mapping of the top 4 bytes is within the realms of the
> > plausible (I've definitely seen the USB layer make 8-byte mappings from any
> > old offset within a page, for example).
> 
> But we can easily work around that by reserving the top 4k of the first
> 4GB of IOVA address space in the allocator, no? Then these values are
> never returned as valid DMA handles.

Yuck.  So, if we have a 4GB non-PAE 32-bit system, or a PAE system
where we have valid memory across the 4GB boundary and no IOMMU,
we have to reserve the top 4K page in the first 4GB of RAM?

Linus' IS_DMA_ERR() solution is way more preferable, but unfortunately
it still falls short, because it knocks out the top 4K of every DMA
capable bus.

A DMA capable bus may involve some translation of the address (eg, by
simple offsetting) which means that we'd need to potentially knock out
multiple pages from the page allocator for each of those pages that
correspond to the top 4K of any DMA capable bus.  Knowing that at the
right time at boot will be fun!  It also sounds wasteful to me.

Rather than inventing magic cookies like this, I'd much rather we
sanitised the API so that we have functions that return success or
an error code, rather than trying to shoe-horn some kind of magic
error codes into dma_addr_t and subtly break systems in the process.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-23 11:01           ` Russell King - ARM Linux
@ 2018-11-23 13:03             ` Joerg Roedel
  2018-11-23 13:20               ` Russell King - ARM Linux
  0 siblings, 1 reply; 53+ messages in thread
From: Joerg Roedel @ 2018-11-23 13:03 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Robin Murphy, Linus Torvalds, Christoph Hellwig, linux-arch,
	linux-alpha, linux-ia64, linux-parisc, David Woodhouse,
	the arch/x86 maintainers, Linux List Kernel Mailing, iommu,
	jdmason, xen-devel, linux-arm-kernel, m.szyprowski

On Fri, Nov 23, 2018 at 11:01:55AM +0000, Russell King - ARM Linux wrote:
> Yuck.  So, if we have a 4GB non-PAE 32-bit system, or a PAE system
> where we have valid memory across the 4GB boundary and no IOMMU,
> we have to reserve the top 4K page in the first 4GB of RAM?

But that is only needed when dma_addr_t is 32bit anyway, no?

> Rather than inventing magic cookies like this, I'd much rather we
> sanitised the API so that we have functions that return success or
> an error code, rather than trying to shoe-horn some kind of magic
> error codes into dma_addr_t and subtly break systems in the process.

Sure, but is has the obvious downside that we need to touch every driver
that uses these functions, and that are probably a lot of drivers.


Regards,

	Joerg

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-23 13:03             ` Joerg Roedel
@ 2018-11-23 13:20               ` Russell King - ARM Linux
  0 siblings, 0 replies; 53+ messages in thread
From: Russell King - ARM Linux @ 2018-11-23 13:20 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Robin Murphy, Linus Torvalds, Christoph Hellwig, linux-arch,
	linux-alpha, linux-ia64, linux-parisc, David Woodhouse,
	the arch/x86 maintainers, Linux List Kernel Mailing, iommu,
	jdmason, xen-devel, linux-arm-kernel, m.szyprowski

On Fri, Nov 23, 2018 at 02:03:13PM +0100, Joerg Roedel wrote:
> On Fri, Nov 23, 2018 at 11:01:55AM +0000, Russell King - ARM Linux wrote:
> > Yuck.  So, if we have a 4GB non-PAE 32-bit system, or a PAE system
> > where we have valid memory across the 4GB boundary and no IOMMU,
> > we have to reserve the top 4K page in the first 4GB of RAM?
> 
> But that is only needed when dma_addr_t is 32bit anyway, no?

You said:

  But we can easily work around that by reserving the top 4k of the first
  4GB of IOVA address space in the allocator, no? Then these values are
  never returned as valid DMA handles.

To me, your proposal equates to this in code:

int dma_error(dma_addr_t addr)
{
	return (addr & ~(dma_addr_t)0xfff) == 0xfffff000 ? (s32)addr : 0; 
}

Hence, the size of dma_addr_t would be entirely irrelevant.  This
makes dma_addr_t values in the range of 0xfffff000 to 0xffffffff special,
irrespective of whether dma_addr_t is 32-bit or 64-bit.

If that's not what you meant, then I'm afraid your statement wasn't
worded very well - so please can you re-word to state more precisely
what your proposal is?

> > Rather than inventing magic cookies like this, I'd much rather we
> > sanitised the API so that we have functions that return success or
> > an error code, rather than trying to shoe-horn some kind of magic
> > error codes into dma_addr_t and subtly break systems in the process.
> 
> Sure, but is has the obvious downside that we need to touch every driver
> that uses these functions, and that are probably a lot of drivers.

So we have two options:
- change the interface
- subtly break drivers

In any case, I disagree that we need to touch every driver.  Today,
drivers just do:

	if (dma_mapping_error(dev, addr))

and, because dma_mapping_error() returns a boolean, they only care about
the true/falseness.  If we're going to start returning error codes, then
there's no point just returning error codes unless we have some way for
drivers to use them.  Yes, the simple way would be to make
dma_mapping_error() translate addr to an error code, and that maintains
compatibility with existing drivers.

If, instead, we revamped the DMA API, and had the *new* mapping functions
which returned an error code, then the existing mapping functions can be
implemented as part of compatibility rather trivially:

dma_addr_t dma_map_single(...)
{
	dma_addr_t addr;
	int error;

	error = dma_map_single_error(..., &addr);
	if (error)
		addr = DMA_MAPPING_ERROR;
	return addr;
}

which means that if drivers want access to the error code, they use
dma_map_single_error(), meanwhile existing drivers just get on with life
as it _currently_ is, with the cookie-based all-ones error code - without
introducing any of this potential breakage.

Remember, existing drivers would need modification in _any_ case to make
use of a returned error code, so the argument that we'd need to touch
every driver doesn't really stand up.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-23  6:55           ` Christoph Hellwig
@ 2018-11-28  7:41             ` Christoph Hellwig
  2018-11-28 16:47               ` Linus Torvalds
  0 siblings, 1 reply; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-28  7:41 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-arch, linux-ia64, linux-parisc, David Woodhouse,
	the arch/x86 maintainers, linux, Linux List Kernel Mailing,
	iommu, linux-alpha, xen-devel, robin.murphy, Christoph Hellwig,
	linux-arm-kernel

On Fri, Nov 23, 2018 at 07:55:11AM +0100, Christoph Hellwig wrote:
> On Thu, Nov 22, 2018 at 09:55:25AM -0800, Linus Torvalds wrote:
> > No, the big immediate benefit of allowing "return -EINVAL" etc is
> > simply legibility and error avoidance.
> 
> Well, I can tweak the last patch to return -EINVAL from dma_mapping_error
> instead of the old 1 is as bool true.  The callers should all be fine,
> although I'd have to audit them.  Still wouldn't help with being able to
> return different errors.

Any opinions?  I'd really like to make some forward progress on this
series.

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-28  7:41             ` Christoph Hellwig
@ 2018-11-28 16:47               ` Linus Torvalds
  2018-11-28 17:45                 ` Russell King - ARM Linux
  0 siblings, 1 reply; 53+ messages in thread
From: Linus Torvalds @ 2018-11-28 16:47 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arch, linux-ia64, linux-parisc, David Woodhouse,
	the arch/x86 maintainers, linux, Linux List Kernel Mailing,
	iommu, linux-alpha, xen-devel, robin.murphy, linux-arm-kernel

On Tue, Nov 27, 2018 at 11:41 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Fri, Nov 23, 2018 at 07:55:11AM +0100, Christoph Hellwig wrote:
> > Well, I can tweak the last patch to return -EINVAL from dma_mapping_error
> > instead of the old 1 is as bool true.  The callers should all be fine,
> > although I'd have to audit them.  Still wouldn't help with being able to
> > return different errors.
>
> Any opinions?  I'd really like to make some forward progress on this
> series.

So I do think that yes, dma_mapping_error() should return an error
code, not 0/1.

But I was really hoping that the individual drivers themselves could
return error codes. Right now the patch-series has code like this:

      ret = needs_bounce(dev, dma_addr, size);
      if (ret < 0)
-         return ARM_MAPPING_ERROR;
+         return DMA_MAPPING_ERROR;

which while it all makes sense in the context of this patch-series, I
*really* think it would have been so much nicer to return the error
code 'ret' instead (which in this case is -E2BIG).

I don't think this is a huge deal, but ERR_PTR() has been hugely
successful elsewhere. And I'm not hugely convinced about all these
"any address can be valid" arguments. How the hell do you generate a
random dma address in the last page that isn't even page-aligned?

                     Linus

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-28 16:47               ` Linus Torvalds
@ 2018-11-28 17:45                 ` Russell King - ARM Linux
  2018-11-28 18:00                   ` Linus Torvalds
  0 siblings, 1 reply; 53+ messages in thread
From: Russell King - ARM Linux @ 2018-11-28 17:45 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Christoph Hellwig, linux-arch, linux-ia64, linux-parisc,
	robin.murphy, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, linux-alpha, xen-devel,
	David Woodhouse, linux-arm-kernel

On Wed, Nov 28, 2018 at 08:47:05AM -0800, Linus Torvalds wrote:
> On Tue, Nov 27, 2018 at 11:41 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > On Fri, Nov 23, 2018 at 07:55:11AM +0100, Christoph Hellwig wrote:
> > > Well, I can tweak the last patch to return -EINVAL from dma_mapping_error
> > > instead of the old 1 is as bool true.  The callers should all be fine,
> > > although I'd have to audit them.  Still wouldn't help with being able to
> > > return different errors.
> >
> > Any opinions?  I'd really like to make some forward progress on this
> > series.
> 
> So I do think that yes, dma_mapping_error() should return an error
> code, not 0/1.
> 
> But I was really hoping that the individual drivers themselves could
> return error codes. Right now the patch-series has code like this:
> 
>       ret = needs_bounce(dev, dma_addr, size);
>       if (ret < 0)
> -         return ARM_MAPPING_ERROR;
> +         return DMA_MAPPING_ERROR;
> 
> which while it all makes sense in the context of this patch-series, I
> *really* think it would have been so much nicer to return the error
> code 'ret' instead (which in this case is -E2BIG).
> 
> I don't think this is a huge deal, but ERR_PTR() has been hugely
> successful elsewhere. And I'm not hugely convinced about all these
> "any address can be valid" arguments. How the hell do you generate a
> random dma address in the last page that isn't even page-aligned?

kmalloc() a 64-byte buffer, dma_map_single() that buffer.  If you
have RAM that maps to a _bus_ address in the top page of 4GB of a
32-bit bus address, then you lose.  Simples.

Subsystems like I2C, SPI, USB etc all deal with small kmalloc'd
buffers and their drivers make use of DMA.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-28 17:45                 ` Russell King - ARM Linux
@ 2018-11-28 18:00                   ` Linus Torvalds
  2018-11-28 18:08                     ` Russell King - ARM Linux
  2018-11-28 19:27                     ` David Miller
  0 siblings, 2 replies; 53+ messages in thread
From: Linus Torvalds @ 2018-11-28 18:00 UTC (permalink / raw)
  To: linux
  Cc: Christoph Hellwig, linux-arch, linux-ia64, linux-parisc,
	robin.murphy, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, linux-alpha, xen-devel,
	David Woodhouse, linux-arm-kernel

On Wed, Nov 28, 2018 at 9:45 AM Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
>
> > I don't think this is a huge deal, but ERR_PTR() has been hugely
> > successful elsewhere. And I'm not hugely convinced about all these
> > "any address can be valid" arguments. How the hell do you generate a
> > random dma address in the last page that isn't even page-aligned?
>
> kmalloc() a 64-byte buffer, dma_map_single() that buffer.

No.

You already cannot do that kmalloc(), exactly because of ERR_PTR().

Not all memory is accessible even to the kernel. If you have memory
that shows up in the last page of phys_addr_t, you just mark it
reserved at boot-time.

Which is what we ALREADY do for these exact reasons. If the DMA
mappings means that you'd need to add one more page to that list of
reserved pages, then so be it.

The whole argument of "every possible piece of memory is DMA'able" is
just wrong.

                Linus

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-28 18:00                   ` Linus Torvalds
@ 2018-11-28 18:08                     ` Russell King - ARM Linux
  2018-11-28 19:23                       ` Russell King - ARM Linux
       [not found]                       ` <CAHk-=whcbiSxSUprsKjVPEdN5-+o8WnTGiKxEV-+HbKNDs=iNA@mail.gmail.com>
  2018-11-28 19:27                     ` David Miller
  1 sibling, 2 replies; 53+ messages in thread
From: Russell King - ARM Linux @ 2018-11-28 18:08 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Christoph Hellwig, linux-arch, linux-ia64, linux-parisc,
	robin.murphy, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, linux-alpha, xen-devel,
	David Woodhouse, linux-arm-kernel

On Wed, Nov 28, 2018 at 10:00:06AM -0800, Linus Torvalds wrote:
> On Wed, Nov 28, 2018 at 9:45 AM Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> >
> > > I don't think this is a huge deal, but ERR_PTR() has been hugely
> > > successful elsewhere. And I'm not hugely convinced about all these
> > > "any address can be valid" arguments. How the hell do you generate a
> > > random dma address in the last page that isn't even page-aligned?
> >
> > kmalloc() a 64-byte buffer, dma_map_single() that buffer.
> 
> No.
> 
> You already cannot do that kmalloc(), exactly because of ERR_PTR().

I'm very sorry, but I think you are confused.

kmalloc() returns a _virtual_ address, which quite rightly must not be
in the top 4K of 4GB, exactly due to ERR_PTR().  That is fine.

However, that is a completely different kettle of fish from a physical
or DMA address - neither of which are virtual addresses.

Now, say we have 1GB of RAM which starts at 0xc0000000 _physical_.
The kernel is configured with a 2GB/2GB user/kernel split, which means
all 1GB of RAM is mapped as lowmem from 0x80000000 to 0xbfffffff
inclusive.  This means kmalloc() can return any address in that range.

ERR_PTR() will work correctly on any of those pointers, meaning that
none of them will be seen as an error.

However, map any virtual address in the range of 0xbffff000 to
0xbfffffff into DMA space, and the resulting DMA address could well
be in the range of 0xfffff000 to 0xffffffff - precisely the range
of addresses that you are advocating to be used for error codes.

> The whole argument of "every possible piece of memory is DMA'able" is
> just wrong.

I'm very sorry, but I do not buy your argument - you are conflating
virtual addresses which ERR_PTR() deals in with physical and bus
addresses - and if you persist down this route, you will cause
regressions.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-28 18:08                     ` Russell King - ARM Linux
@ 2018-11-28 19:23                       ` Russell King - ARM Linux
       [not found]                       ` <CAHk-=whcbiSxSUprsKjVPEdN5-+o8WnTGiKxEV-+HbKNDs=iNA@mail.gmail.com>
  1 sibling, 0 replies; 53+ messages in thread
From: Russell King - ARM Linux @ 2018-11-28 19:23 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-arch, linux-ia64, linux-parisc, David Woodhouse,
	the arch/x86 maintainers, Linux List Kernel Mailing, iommu,
	linux-alpha, xen-devel, robin.murphy, Christoph Hellwig,
	linux-arm-kernel

On Wed, Nov 28, 2018 at 06:08:41PM +0000, Russell King - ARM Linux wrote:
> On Wed, Nov 28, 2018 at 10:00:06AM -0800, Linus Torvalds wrote:
> > On Wed, Nov 28, 2018 at 9:45 AM Russell King - ARM Linux
> > <linux@armlinux.org.uk> wrote:
> > >
> > > > I don't think this is a huge deal, but ERR_PTR() has been hugely
> > > > successful elsewhere. And I'm not hugely convinced about all these
> > > > "any address can be valid" arguments. How the hell do you generate a
> > > > random dma address in the last page that isn't even page-aligned?
> > >
> > > kmalloc() a 64-byte buffer, dma_map_single() that buffer.
> > 
> > No.
> > 
> > You already cannot do that kmalloc(), exactly because of ERR_PTR().
> 
> I'm very sorry, but I think you are confused.
> 
> kmalloc() returns a _virtual_ address, which quite rightly must not be
> in the top 4K of 4GB, exactly due to ERR_PTR().  That is fine.
> 
> However, that is a completely different kettle of fish from a physical
> or DMA address - neither of which are virtual addresses.
> 
> Now, say we have 1GB of RAM which starts at 0xc0000000 _physical_.
> The kernel is configured with a 2GB/2GB user/kernel split, which means
> all 1GB of RAM is mapped as lowmem from 0x80000000 to 0xbfffffff
> inclusive.  This means kmalloc() can return any address in that range.
> 
> ERR_PTR() will work correctly on any of those pointers, meaning that
> none of them will be seen as an error.
> 
> However, map any virtual address in the range of 0xbffff000 to
> 0xbfffffff into DMA space, and the resulting DMA address could well
> be in the range of 0xfffff000 to 0xffffffff - precisely the range
> of addresses that you are advocating to be used for error codes.
> 
> > The whole argument of "every possible piece of memory is DMA'able" is
> > just wrong.
> 
> I'm very sorry, but I do not buy your argument - you are conflating
> virtual addresses which ERR_PTR() deals in with physical and bus
> addresses - and if you persist down this route, you will cause
> regressions.

Here's another case:

i.MX6 with 4GB of RAM.  Devices are mapped to 0-0x0fffffff physical,
RAM is mapped to 0x10000000-0xffffffff physical.  The last 256MB of
RAM is not accessible as this is a 32-bit device.  DMA addresses are
the same as physical addresses.

While the final physical page will be highmem in a normal kernel,
and thus will not be available for kmalloc(), that doesn't mean it
can't happen.  A crashdump kernel loaded high in physical memory
(eg, last 512MB and given the last 512MB to play around in) would
have the top 512MB as lowmem, and therefore available for kmalloc().

If a page is available in lowmem, it's available for kmalloc(), and
we can't say that we will never allocate memory from such a page for
DMA - if we do and we're using an IS_ERR_VALUE() scheme, it _will_
break if that happens as memory will end up being mapped by the DMA
API but dma_mapping_error() will see it as a failure.

It won't be an obvious breakage, because it depends on the right
conditions happening - a kmalloc() from the top page of physical
RAM and that being passed to dma_map_single().  IOW, it's not something
that a quick boot test would find, it's something that is likely to
cause failures after a system has been running for a period of time.

There are other situations where there are possibilities - such as:

	dma_map_page(dev, page, offset, size, direction)

If 'page' is a highmem page which happens to be the top page in the
4GB space, and offset is non-zero, and there's a 1:1 mapping between
physical address and DMA address, the returned value will be
0xfffff000 + offset - within the "last 4095 values are errors"
range.

Networking uses this for fragments - the packet fragment list is
a list of pages, offsets and sizes - we have sendpage() that may
end up finding that last page, and TCP-sized packets may be
generated from it which would certianly result in non-zero offsets
being passed to dma_map_page().

So, whatever way _I_ look at it, I find your proposal to be unsafe
and potentially regression causing, and I *completely* and strongly
oppose it in its current form.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-28 18:00                   ` Linus Torvalds
  2018-11-28 18:08                     ` Russell King - ARM Linux
@ 2018-11-28 19:27                     ` David Miller
  2018-11-28 19:47                       ` Russell King - ARM Linux
  1 sibling, 1 reply; 53+ messages in thread
From: David Miller @ 2018-11-28 19:27 UTC (permalink / raw)
  To: torvalds
  Cc: linux, hch, linux-arch, linux-ia64, linux-parisc, robin.murphy,
	x86, linux-kernel, iommu, linux-alpha, xen-devel, dwmw2,
	linux-arm-kernel

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 28 Nov 2018 10:00:06 -0800

> Not all memory is accessible even to the kernel. If you have memory
> that shows up in the last page of phys_addr_t, you just mark it
> reserved at boot-time.

It's not the physical memory at the end that needs to be reserved.

It's the IOMMU mapping arena.

That basically requires changes to every IOMMU implementation in
the tree.  Not saying it's hard or impossible, but it is real
meticulous work that needs to be done in order to realize this.


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

* Re: remove the ->mapping_error method from dma_map_ops V2
       [not found]                       ` <CAHk-=whcbiSxSUprsKjVPEdN5-+o8WnTGiKxEV-+HbKNDs=iNA@mail.gmail.com>
@ 2018-11-28 19:31                         ` Russell King - ARM Linux
  2018-11-29 16:23                         ` Christoph Hellwig
  1 sibling, 0 replies; 53+ messages in thread
From: Russell King - ARM Linux @ 2018-11-28 19:31 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Christoph Hellwig, linux-arch, linux-ia64, linux-parisc,
	robin.murphy, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, linux-alpha, xen-devel,
	David Woodhouse, linux-arm-kernel

On Wed, Nov 28, 2018 at 11:19:15AM -0800, Linus Torvalds wrote:
> On Wed, Nov 28, 2018, 10:08 Russell King - ARM Linux <linux@armlinux.org.uk
> wrote:
> 
> > >
> > > You already cannot do that kmalloc(), exactly because of ERR_PTR().
> >
> > I'm very sorry, but I think you are confused.
> >
> > kmalloc() returns a _virtual_ address, which quite rightly must not be
> > in the top 4K of 4GB, exactly due to ERR_PTR().  That is fine.
> >
> > However, that is a completely different kettle of fish from a physical
> > or DMA address - neither of which are virtual addresses.
> >
> 
> You cut away the part that showed that I'm not in the least confused.
> 
> Let me just paste it back in here:
> 
>   "Which is what we ALREADY do for these exact reasons. If the DMA
> mappings means that you'd need to add one more page to that list of
> reserved pages, then so be it."

We're not talking about just one more page.  We're talking about
_every_ page that _may_ map to a bus address in the range of 4GB - 4K
(a point I've already made earlier in this discussion.)

It's not just about physical addresses, it's about bus addresses,
and there are buses out there that have a translation between physical
address and bus address without IOMMUs and the like.

Can we know ahead of time about all these translations?  It means
moving that information out of various bus drivers into core
architecture code (yuck, when you have multiple different platforms)
or into DT (which means effectively breaking every damn platform
that's using older DT files) - something that we don't have to do
today.

I remain 100% opposed to your idea.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-28 19:27                     ` David Miller
@ 2018-11-28 19:47                       ` Russell King - ARM Linux
  2018-11-28 23:01                         ` Shuah Khan
  0 siblings, 1 reply; 53+ messages in thread
From: Russell King - ARM Linux @ 2018-11-28 19:47 UTC (permalink / raw)
  To: David Miller
  Cc: torvalds, hch, linux-arch, linux-ia64, linux-parisc,
	robin.murphy, x86, linux-kernel, iommu, linux-alpha, xen-devel,
	dwmw2, linux-arm-kernel

On Wed, Nov 28, 2018 at 11:27:17AM -0800, David Miller wrote:
> From: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Wed, 28 Nov 2018 10:00:06 -0800
> 
> > Not all memory is accessible even to the kernel. If you have memory
> > that shows up in the last page of phys_addr_t, you just mark it
> > reserved at boot-time.
> 
> It's not the physical memory at the end that needs to be reserved.
> 
> It's the IOMMU mapping arena.

True, if and only if you have an IOMMU.

Where there isn't an IOMMU, then we'd have to reserve every page that
that translates to a bus address in the top 4K of dma_addr_t on any
bus in the system - that means knowing early in the kernel
initialisation about all buses in the system so we can detect and
reserve these pages.

I don't think that's trivial to do.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-28 19:47                       ` Russell King - ARM Linux
@ 2018-11-28 23:01                         ` Shuah Khan
  0 siblings, 0 replies; 53+ messages in thread
From: Shuah Khan @ 2018-11-28 23:01 UTC (permalink / raw)
  To: linux
  Cc: David Miller, Linus Torvalds, hch, linux-arch, linux-ia64,
	linux-parisc, robin.murphy, x86, LKML,
	open list:INTEL IOMMU (VT-d),
	linux-alpha, xen-devel, dwmw2, linux-arm-kernel, Shuah Khan

On Wed, Nov 28, 2018 at 12:48 PM Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
>
> On Wed, Nov 28, 2018 at 11:27:17AM -0800, David Miller wrote:
> > From: Linus Torvalds <torvalds@linux-foundation.org>
> > Date: Wed, 28 Nov 2018 10:00:06 -0800
> >
> > > Not all memory is accessible even to the kernel. If you have memory
> > > that shows up in the last page of phys_addr_t, you just mark it
> > > reserved at boot-time.
> >
> > It's not the physical memory at the end that needs to be reserved.
> >
> > It's the IOMMU mapping arena.
>
> True, if and only if you have an IOMMU.
>
> Where there isn't an IOMMU, then we'd have to reserve every page that
> that translates to a bus address in the top 4K of dma_addr_t on any
> bus in the system - that means knowing early in the kernel
> initialisation about all buses in the system so we can detect and
> reserve these pages.
>

The arch and platform differences/confusion as to "what is DMA error
value" is the reason why dma_mapping_error is part dma ops. I
understand that iy would make sense to reduce the overhead of an
additional call, however, I am not sure if would be trivial change to
address this.

I was down this path of trying to address the missing mapping error
checks a few years ago and introduced dma_mapping_error checks in the
DMA DEBUG API. As you might already know that there is no common
definition for the mapping error.

Quick look at the defines shows:

#define CALGARY_MAPPING_ERROR 0
#define S390_MAPPING_ERROR  (~(dma_addr_t) 0x0)
#define SPARC_MAPPING_ERROR (~(dma_addr_t)0x0)

This is the reason why there is a arch or in some cases bus specific
mapping_error ops is needed.
We could unify this and fix all these first. I haven't looked at the
patch set closely, maybe you are already doing this.

thanks,
-- Shuah

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

* Re: remove the ->mapping_error method from dma_map_ops V2
       [not found]                       ` <CAHk-=whcbiSxSUprsKjVPEdN5-+o8WnTGiKxEV-+HbKNDs=iNA@mail.gmail.com>
  2018-11-28 19:31                         ` Russell King - ARM Linux
@ 2018-11-29 16:23                         ` Christoph Hellwig
  2018-11-29 17:44                           ` Linus Torvalds
  1 sibling, 1 reply; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-29 16:23 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Russell King - ARM Linux, Christoph Hellwig, linux-arch,
	linux-ia64, linux-parisc, robin.murphy, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, linux-alpha, xen-devel,
	David Woodhouse, linux-arm-kernel

On Wed, Nov 28, 2018 at 11:19:15AM -0800, Linus Torvalds wrote:
> Let me just paste it back in here:
> 
>   "Which is what we ALREADY do for these exact reasons. If the DMA
> mappings means that you'd need to add one more page to that list of
> reserved pages, then so be it."
> 
> So no, I'm not at all confused.
> 
> Let me re-iterate: the argument that all addresses have to be dma'able is
> garbage.
> 
> *Exactly* as with kmalloc and limited virtual addresses, we can limit
> physical addresses.

We can.  At least in theory.  The problem is that depending on the
crazy mapping from physical and kernel virtual address to dma addresses
these might be pages at pretty random places.  Look at fun like
arch/x86/pci/sta2x11-fixup.c for how ugly these mappings could look.

It also means that we might have setup swiotlb on just about every
32-bit architecture, even if it has no real addressing limit except for
the one we imposed.  I don't really see how this is a win for us just
to be able to report more detailed error codes, which would be nice
to have, but the lack of which hasn't really harmed us.

So as far as I'm concerned I'd go either with the series that we are
discussing here, or change the map_page method to return an errno
and the dma_addr_t in the argument.  As Davem pointed out that can lead
to less optimal code, but it would still be better than the indirect
call we have.  But then again I think the series as posted here might
and up much simpler and good enough without opening up this rathole.

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-29 16:23                         ` Christoph Hellwig
@ 2018-11-29 17:44                           ` Linus Torvalds
  2018-11-29 18:31                             ` Christoph Hellwig
  0 siblings, 1 reply; 53+ messages in thread
From: Linus Torvalds @ 2018-11-29 17:44 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Russell King - ARM Linux, linux-arch, linux-ia64, linux-parisc,
	robin.murphy, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, linux-alpha, xen-devel,
	David Woodhouse, linux-arm-kernel

On Thu, Nov 29, 2018 at 8:23 AM Christoph Hellwig <hch@lst.de> wrote:
>
> We can.  At least in theory.  The problem is that depending on the
> crazy mapping from physical and kernel virtual address to dma addresses
> these might be pages at pretty random places.  Look at fun like
> arch/x86/pci/sta2x11-fixup.c for how ugly these mappings could look.
>
> It also means that we might have setup swiotlb on just about every
> 32-bit architecture, even if it has no real addressing limit except for
> the one we imposed.

No. Really. If there's no iotlb, then you just mark that one page
reserved. It simply doesn't get used. It doesn't mean you suddenly
need a swiotlb.

If there *is* a iotlb, none of this should matter, because you'd just
never map anything into that page.

But whatever. It's independent from the patch series under discussion.
Make dma_mapping_error() at least return a real error (eg -EINVAL, or
whatever is the common error), and we can maybe do this later.

Or, better yet, plan on removing the single-page dma mappign entirely
at a later date, and make the issue moot.

              Linus

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-29 17:44                           ` Linus Torvalds
@ 2018-11-29 18:31                             ` Christoph Hellwig
  2018-11-29 18:53                               ` Linus Torvalds
  0 siblings, 1 reply; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-29 18:31 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Christoph Hellwig, Russell King - ARM Linux, linux-arch,
	linux-ia64, linux-parisc, robin.murphy, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, linux-alpha, xen-devel,
	David Woodhouse, linux-arm-kernel

On Thu, Nov 29, 2018 at 09:44:05AM -0800, Linus Torvalds wrote:
> No. Really. If there's no iotlb, then you just mark that one page
> reserved. It simply doesn't get used. It doesn't mean you suddenly
> need a swiotlb.

Sure, we could just skip that page entirely based on dma_to_phys.

> But whatever. It's independent from the patch series under discussion.
> Make dma_mapping_error() at least return a real error (eg -EINVAL, or
> whatever is the common error), and we can maybe do this later.

Ok, I'll do that.

> Or, better yet, plan on removing the single-page dma mappign entirely
> at a later date, and make the issue moot.

What would be the replacement?  Build a S/G list for every single page
mapping?  Not sure that would create a lot of happy campers..

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-29 18:31                             ` Christoph Hellwig
@ 2018-11-29 18:53                               ` Linus Torvalds
  2018-11-29 18:55                                 ` Christoph Hellwig
  0 siblings, 1 reply; 53+ messages in thread
From: Linus Torvalds @ 2018-11-29 18:53 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Russell King - ARM Linux, linux-arch, linux-ia64, linux-parisc,
	robin.murphy, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, linux-alpha, xen-devel,
	David Woodhouse, linux-arm-kernel

On Thu, Nov 29, 2018 at 10:31 AM Christoph Hellwig <hch@lst.de> wrote:
>
> > Or, better yet, plan on removing the single-page dma mappign entirely
> > at a later date, and make the issue moot.
>
> What would be the replacement?  Build a S/G list for every single page
> mapping?  Not sure that would create a lot of happy campers..

It's what we ended up doing with some other cases, and it didn't
really end up hurting as much as I thought it would.

I'm thinking of the vfs functions that end up turning "buf, len" into

        struct iovec iov = { .iov_base = (void __user *)buf, .iov_len = len };

and then passing it around as a single-entry iov instead (not even
that - they end up being an iov_iter, which is not just the iov, but
the whole "what _kind_ of iov" indirection)

Maybe a very similar model could be used for just simplifying the core
dma mapping setup: sure, people will want to do single-area dma, but
how bad would it be to just turn them into single-entry SG lists on
stack, and then the dma-maping internally would just always see that?

Most of the high-performance IO is already using SG lists anyway, no?
Disk/networking/whatever.

But just an idea. And the "map_sg()" error handling isn't actually any
better, I think. It returns zero on error, no? So it's not improving
the error handling.

The whole dma-mapping layer seems full of those kinds of "inspired
error handling choices" ;)

                  Linus

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

* Re: remove the ->mapping_error method from dma_map_ops V2
  2018-11-29 18:53                               ` Linus Torvalds
@ 2018-11-29 18:55                                 ` Christoph Hellwig
  0 siblings, 0 replies; 53+ messages in thread
From: Christoph Hellwig @ 2018-11-29 18:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Christoph Hellwig, Russell King - ARM Linux, linux-arch,
	linux-ia64, linux-parisc, robin.murphy, the arch/x86 maintainers,
	Linux List Kernel Mailing, iommu, linux-alpha, xen-devel,
	David Woodhouse, linux-arm-kernel

On Thu, Nov 29, 2018 at 10:53:32AM -0800, Linus Torvalds wrote:
> Most of the high-performance IO is already using SG lists anyway, no?
> Disk/networking/whatever.

Networking basically never uses S/G lists.  Block I/O mostly uses it,
and graphics / media seems to have a fair amount of S/G uses, including
very, erm special ones.

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

end of thread, other threads:[~2018-11-29 18:55 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-22 14:02 remove the ->mapping_error method from dma_map_ops V2 Christoph Hellwig
2018-11-22 14:02 ` [PATCH 01/24] dma-direct: Make DIRECT_MAPPING_ERROR viable for SWIOTLB Christoph Hellwig
2018-11-22 14:02 ` [PATCH 02/24] swiotlb: Skip cache maintenance on map error Christoph Hellwig
2018-11-22 14:02 ` [PATCH 03/24] dma-mapping: provide a generic DMA_MAPPING_ERROR Christoph Hellwig
2018-11-22 14:03 ` [PATCH 04/24] dma-direct: remove the mapping_error dma_map_ops method Christoph Hellwig
2018-11-22 14:03 ` [PATCH 05/24] arm: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 06/24] powerpc/iommu: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 07/24] mips/jazz: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 08/24] s390: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 09/24] sparc: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 10/24] parisc/ccio: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 11/24] parisc/sba_iommu: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 12/24] arm64: remove the dummy_dma_ops mapping_error method Christoph Hellwig
2018-11-22 14:03 ` [PATCH 13/24] alpha: remove the mapping_error dma_map_ops method Christoph Hellwig
2018-11-22 14:03 ` [PATCH 14/24] ia64/sba_iommu: improve internal map_page users Christoph Hellwig
2018-11-22 14:03 ` [PATCH 15/24] ia64/sba_iommu: remove the mapping_error dma_map_ops method Christoph Hellwig
2018-11-22 14:03 ` [PATCH 16/24] ia64/sn: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 17/24] x86/amd_gart: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 18/24] x86/calgary: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 19/24] iommu: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 20/24] iommu/intel: small map_page cleanup Christoph Hellwig
2018-11-22 14:03 ` [PATCH 21/24] iommu/vt-d: remove the mapping_error dma_map_ops method Christoph Hellwig
2018-11-22 14:03 ` [PATCH 22/24] iommu/dma-iommu: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 23/24] xen-swiotlb: " Christoph Hellwig
2018-11-22 14:03 ` [PATCH 24/24] dma-mapping: " Christoph Hellwig
2018-11-22 16:50 ` remove the ->mapping_error method from dma_map_ops V2 Linus Torvalds
2018-11-22 17:07   ` Russell King - ARM Linux
2018-11-22 17:09     ` Linus Torvalds
2018-11-22 17:14       ` Russell King - ARM Linux
2018-11-22 17:52       ` Robin Murphy
2018-11-22 17:55         ` Linus Torvalds
2018-11-22 18:05           ` Russell King - ARM Linux
2018-11-23  6:57             ` Christoph Hellwig
2018-11-23  6:55           ` Christoph Hellwig
2018-11-28  7:41             ` Christoph Hellwig
2018-11-28 16:47               ` Linus Torvalds
2018-11-28 17:45                 ` Russell King - ARM Linux
2018-11-28 18:00                   ` Linus Torvalds
2018-11-28 18:08                     ` Russell King - ARM Linux
2018-11-28 19:23                       ` Russell King - ARM Linux
     [not found]                       ` <CAHk-=whcbiSxSUprsKjVPEdN5-+o8WnTGiKxEV-+HbKNDs=iNA@mail.gmail.com>
2018-11-28 19:31                         ` Russell King - ARM Linux
2018-11-29 16:23                         ` Christoph Hellwig
2018-11-29 17:44                           ` Linus Torvalds
2018-11-29 18:31                             ` Christoph Hellwig
2018-11-29 18:53                               ` Linus Torvalds
2018-11-29 18:55                                 ` Christoph Hellwig
2018-11-28 19:27                     ` David Miller
2018-11-28 19:47                       ` Russell King - ARM Linux
2018-11-28 23:01                         ` Shuah Khan
2018-11-23 10:49         ` Joerg Roedel
2018-11-23 11:01           ` Russell King - ARM Linux
2018-11-23 13:03             ` Joerg Roedel
2018-11-23 13:20               ` Russell King - ARM Linux

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