linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 00/16] .map_sg() error cleanup
@ 2021-07-15 16:45 Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 01/16] dma-mapping: Allow map_sg() ops to return negative error codes Logan Gunthorpe
                   ` (16 more replies)
  0 siblings, 17 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe

Hi,

This series is spun out and expanded from my work to add P2PDMA support
to DMA map operations[1].

The P2PDMA work requires distinguishing different error conditions in
a map_sg operation. dma_map_sgtable() already allows for returning an
error code (where as dma_map_sg() is only allowed to return zero)
however, it currently only returns -EINVAL when a .map_sg() call returns
zero.

This series cleans up all .map_sg() implementations to return appropriate
error codes. After the cleanup, dma_map_sg() will still return zero,
however dma_map_sgtable() will pass the error code from the .map_sg()
call. Thanks go to Martn Oliveira for doing a lot of the cleanup of the
obscure implementations.

The patch set is based off of v5.14-rc1 and a git repo can be found
here:

  https://github.com/sbates130272/linux-p2pmem map_sg_err_cleanup_v1

Thanks,

Logan

[1] https://lore.kernel.org/linux-block/20210513223203.5542-1-logang@deltatee.com/

--

Logan Gunthorpe (5):
  dma-mapping: Allow map_sg() ops to return negative error codes
  dma-direct: Return appropriate error code from dma_direct_map_sg()
  iommu: Return full error code from iommu_map_sg[_atomic]()
  dma-iommu: Return error code from iommu_dma_map_sg()
  dma-mapping: Disallow .map_sg operations from returning zero on error

Martin Oliveira (11):
  alpha: return error code from alpha_pci_map_sg()
  ARM/dma-mapping: return error code from .map_sg() ops
  ia64/sba_iommu: return error code from sba_map_sg_attrs()
  MIPS/jazzdma: return error code from jazz_dma_map_sg()
  powerpc/iommu: return error code from .map_sg() ops
  s390/pci: return error code from s390_dma_map_sg()
  sparc/iommu: return error codes from .map_sg() ops
  parisc: return error code from .map_sg() ops
  xen: swiotlb: return error code from xen_swiotlb_map_sg()
  x86/amd_gart: return error code from gart_map_sg()
  dma-mapping: return error code from dma_dummy_map_sg()

 arch/alpha/kernel/pci_iommu.c           | 10 +++-
 arch/arm/mm/dma-mapping.c               | 22 +++++---
 arch/ia64/hp/common/sba_iommu.c         |  9 +--
 arch/mips/jazz/jazzdma.c                |  2 +-
 arch/powerpc/kernel/iommu.c             |  4 +-
 arch/powerpc/platforms/ps3/system-bus.c |  2 +-
 arch/powerpc/platforms/pseries/vio.c    |  5 +-
 arch/s390/pci/pci_dma.c                 | 12 ++--
 arch/sparc/kernel/iommu.c               |  4 +-
 arch/sparc/kernel/pci_sun4v.c           |  4 +-
 arch/sparc/mm/iommu.c                   |  2 +-
 arch/x86/kernel/amd_gart_64.c           | 16 +++---
 drivers/iommu/dma-iommu.c               | 20 ++++---
 drivers/iommu/iommu.c                   | 15 +++--
 drivers/parisc/ccio-dma.c               |  2 +-
 drivers/parisc/sba_iommu.c              |  2 +-
 drivers/xen/swiotlb-xen.c               |  2 +-
 include/linux/dma-map-ops.h             |  6 +-
 include/linux/dma-mapping.h             | 35 +++---------
 include/linux/iommu.h                   | 22 ++++----
 kernel/dma/direct.c                     |  2 +-
 kernel/dma/dummy.c                      |  2 +-
 kernel/dma/mapping.c                    | 73 ++++++++++++++++++++++---
 23 files changed, 165 insertions(+), 108 deletions(-)


base-commit: e73f0f0ee7541171d89f2e2491130c7771ba58d3
--
2.20.1

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

* [PATCH v1 01/16] dma-mapping: Allow map_sg() ops to return negative error codes
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-16  6:29   ` Christoph Hellwig
  2021-07-15 16:45 ` [PATCH v1 02/16] dma-direct: Return appropriate error code from dma_direct_map_sg() Logan Gunthorpe
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe

Allow dma_map_sgtable() to pass errors from the map_sg() ops. This
will be required for returning appropriate error codes when mapping
P2PDMA memory.

Introduce __dma_map_sg_attrs() which will return the raw error code
from the map_sg operation (whether it be negative or zero). Then add a
dma_map_sg_attrs() wrapper to convert any negative errors to zero to
satisfy the existing calling convention.

dma_map_sgtable() will convert a zero error return for old map_sg() ops
into a -EINVAL return and return any negative errors as reported.

This allows map_sg implementations to start returning multiple
negative error codes. Legacy map_sg implementations can continue
to return zero until they are all converted.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 include/linux/dma-map-ops.h |  8 +++-
 include/linux/dma-mapping.h | 35 ++++--------------
 kernel/dma/mapping.c        | 73 +++++++++++++++++++++++++++++++++----
 3 files changed, 78 insertions(+), 38 deletions(-)

diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index 0d53a96a3d64..eaa969be8284 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -41,8 +41,12 @@ struct dma_map_ops {
 			size_t size, enum dma_data_direction dir,
 			unsigned long attrs);
 	/*
-	 * map_sg returns 0 on error and a value > 0 on success.
-	 * It should never return a value < 0.
+	 * map_sg should return a negative error code on error.
+	 * dma_map_sgtable() will return the error code returned and convert
+	 * a zero return (for legacy implementations) into -EINVAL.
+	 *
+	 * dma_map_sg() will always return zero on any negative or zero
+	 * return to satisfy its own calling convention.
 	 */
 	int (*map_sg)(struct device *dev, struct scatterlist *sg, int nents,
 			enum dma_data_direction dir, unsigned long attrs);
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 183e7103a66d..daa1e360f0ee 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -110,6 +110,8 @@ int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg, int nents,
 void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
 				      int nents, enum dma_data_direction dir,
 				      unsigned long attrs);
+int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
+		enum dma_data_direction dir, unsigned long attrs);
 dma_addr_t dma_map_resource(struct device *dev, phys_addr_t phys_addr,
 		size_t size, enum dma_data_direction dir, unsigned long attrs);
 void dma_unmap_resource(struct device *dev, dma_addr_t addr, size_t size,
@@ -174,6 +176,11 @@ static inline void dma_unmap_sg_attrs(struct device *dev,
 		unsigned long attrs)
 {
 }
+static inline int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
+		enum dma_data_direction dir, unsigned long attrs)
+{
+	return -EOPNOTSUPP;
+}
 static inline dma_addr_t dma_map_resource(struct device *dev,
 		phys_addr_t phys_addr, size_t size, enum dma_data_direction dir,
 		unsigned long attrs)
@@ -343,34 +350,6 @@ static inline void dma_sync_single_range_for_device(struct device *dev,
 	return dma_sync_single_for_device(dev, addr + offset, size, dir);
 }
 
-/**
- * dma_map_sgtable - Map the given buffer for DMA
- * @dev:	The device for which to perform the DMA operation
- * @sgt:	The sg_table object describing the buffer
- * @dir:	DMA direction
- * @attrs:	Optional DMA attributes for the map operation
- *
- * Maps a buffer described by a scatterlist stored in the given sg_table
- * object for the @dir DMA operation by the @dev device. After success the
- * ownership for the buffer is transferred to the DMA domain.  One has to
- * call dma_sync_sgtable_for_cpu() or dma_unmap_sgtable() to move the
- * ownership of the buffer back to the CPU domain before touching the
- * buffer by the CPU.
- *
- * Returns 0 on success or -EINVAL on error during mapping the buffer.
- */
-static inline int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
-		enum dma_data_direction dir, unsigned long attrs)
-{
-	int nents;
-
-	nents = dma_map_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
-	if (nents <= 0)
-		return -EINVAL;
-	sgt->nents = nents;
-	return 0;
-}
-
 /**
  * dma_unmap_sgtable - Unmap the given buffer for DMA
  * @dev:	The device for which to perform the DMA operation
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 2b06a809d0b9..30f89d244566 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -177,12 +177,8 @@ void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, size_t size,
 }
 EXPORT_SYMBOL(dma_unmap_page_attrs);
 
-/*
- * dma_maps_sg_attrs returns 0 on error and > 0 on success.
- * It should never return a value < 0.
- */
-int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg, int nents,
-		enum dma_data_direction dir, unsigned long attrs)
+static int __dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
+	 int nents, enum dma_data_direction dir, unsigned long attrs)
 {
 	const struct dma_map_ops *ops = get_dma_ops(dev);
 	int ents;
@@ -197,13 +193,74 @@ int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg, int nents,
 		ents = dma_direct_map_sg(dev, sg, nents, dir, attrs);
 	else
 		ents = ops->map_sg(dev, sg, nents, dir, attrs);
-	BUG_ON(ents < 0);
-	debug_dma_map_sg(dev, sg, nents, ents, dir);
+
+	if (ents > 0)
+		debug_dma_map_sg(dev, sg, nents, ents, dir);
 
 	return ents;
 }
+
+/**
+ * dma_map_sg_attrs - Map the given buffer for DMA
+ * @dev:	The device for which to perform the DMA operation
+ * @sg:	The sg_table object describing the buffer
+ * @dir:	DMA direction
+ * @attrs:	Optional DMA attributes for the map operation
+ *
+ * Maps a buffer described by a scatterlist passed in the sg argument with
+ * nents segments for the @dir DMA operation by the @dev device.
+ *
+ * Returns the number of mapped entries (which can be less than nents)
+ * on success. Zero is returned for any error.
+ *
+ * dma_unmap_sg_attrs() should be used to unmap the buffer with the
+ * original sg and original nents (not the value returned by this funciton).
+ */
+int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
+		    int nents, enum dma_data_direction dir, unsigned long attrs)
+{
+	int ret;
+
+	ret = __dma_map_sg_attrs(dev, sg, nents, dir, attrs);
+	if (ret < 0)
+		ret = 0;
+
+	return ret;
+}
 EXPORT_SYMBOL(dma_map_sg_attrs);
 
+/**
+ * dma_map_sgtable - Map the given buffer for DMA
+ * @dev:	The device for which to perform the DMA operation
+ * @sgt:	The sg_table object describing the buffer
+ * @dir:	DMA direction
+ * @attrs:	Optional DMA attributes for the map operation
+ *
+ * Maps a buffer described by a scatterlist stored in the given sg_table
+ * object for the @dir DMA operation by the @dev device. After success, the
+ * ownership for the buffer is transferred to the DMA domain.  One has to
+ * call dma_sync_sgtable_for_cpu() or dma_unmap_sgtable() to move the
+ * ownership of the buffer back to the CPU domain before touching the
+ * buffer by the CPU.
+ *
+ * Returns 0 on success or a negative error code on error
+ */
+int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
+		    enum dma_data_direction dir, unsigned long attrs)
+{
+	int nents;
+
+	nents = __dma_map_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
+	if (nents == 0)
+		return -EINVAL;
+	else if (nents < 0)
+		return nents;
+
+	sgt->nents = nents;
+	return 0;
+}
+EXPORT_SYMBOL(dma_map_sgtable);
+
 void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
 				      int nents, enum dma_data_direction dir,
 				      unsigned long attrs)
-- 
2.20.1


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

* [PATCH v1 02/16] dma-direct: Return appropriate error code from dma_direct_map_sg()
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 01/16] dma-mapping: Allow map_sg() ops to return negative error codes Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 03/16] iommu: Return full error code from iommu_map_sg[_atomic]() Logan Gunthorpe
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe

Now that the map_sg() op expects error codes instead of return zero on
error, convert dma_direct_map_sg() to return an error code. The
only error to return presently is EINVAL if a page could not
be mapped.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 kernel/dma/direct.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index f737e3347059..803ee9321170 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -411,7 +411,7 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
 
 out_unmap:
 	dma_direct_unmap_sg(dev, sgl, i, dir, attrs | DMA_ATTR_SKIP_CPU_SYNC);
-	return 0;
+	return -EINVAL;
 }
 
 dma_addr_t dma_direct_map_resource(struct device *dev, phys_addr_t paddr,
-- 
2.20.1


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

* [PATCH v1 03/16] iommu: Return full error code from iommu_map_sg[_atomic]()
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 01/16] dma-mapping: Allow map_sg() ops to return negative error codes Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 02/16] dma-direct: Return appropriate error code from dma_direct_map_sg() Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 04/16] dma-iommu: Return error code from iommu_dma_map_sg() Logan Gunthorpe
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe, Joerg Roedel, Will Deacon

Convert to ssize_t return code so the return code from __iommu_map()
can be returned all the way down through dma_iommu_map_sg().

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Will Deacon <will@kernel.org>
---
 drivers/iommu/iommu.c | 15 +++++++--------
 include/linux/iommu.h | 22 +++++++++++-----------
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5419c4b9f27a..bf971b4e34aa 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2567,9 +2567,9 @@ size_t iommu_unmap_fast(struct iommu_domain *domain,
 }
 EXPORT_SYMBOL_GPL(iommu_unmap_fast);
 
-static size_t __iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
-			     struct scatterlist *sg, unsigned int nents, int prot,
-			     gfp_t gfp)
+static ssize_t __iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
+		struct scatterlist *sg, unsigned int nents, int prot,
+		gfp_t gfp)
 {
 	const struct iommu_ops *ops = domain->ops;
 	size_t len = 0, mapped = 0;
@@ -2610,19 +2610,18 @@ static size_t __iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
 	/* undo mappings already done */
 	iommu_unmap(domain, iova, mapped);
 
-	return 0;
-
+	return ret;
 }
 
-size_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
-		    struct scatterlist *sg, unsigned int nents, int prot)
+ssize_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
+		     struct scatterlist *sg, unsigned int nents, int prot)
 {
 	might_sleep();
 	return __iommu_map_sg(domain, iova, sg, nents, prot, GFP_KERNEL);
 }
 EXPORT_SYMBOL_GPL(iommu_map_sg);
 
-size_t iommu_map_sg_atomic(struct iommu_domain *domain, unsigned long iova,
+ssize_t iommu_map_sg_atomic(struct iommu_domain *domain, unsigned long iova,
 		    struct scatterlist *sg, unsigned int nents, int prot)
 {
 	return __iommu_map_sg(domain, iova, sg, nents, prot, GFP_ATOMIC);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 32d448050bf7..9369458ba1bd 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -414,11 +414,11 @@ extern size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova,
 extern size_t iommu_unmap_fast(struct iommu_domain *domain,
 			       unsigned long iova, size_t size,
 			       struct iommu_iotlb_gather *iotlb_gather);
-extern size_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
-			   struct scatterlist *sg,unsigned int nents, int prot);
-extern size_t iommu_map_sg_atomic(struct iommu_domain *domain,
-				  unsigned long iova, struct scatterlist *sg,
-				  unsigned int nents, int prot);
+extern ssize_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
+		struct scatterlist *sg, unsigned int nents, int prot);
+extern ssize_t iommu_map_sg_atomic(struct iommu_domain *domain,
+				   unsigned long iova, struct scatterlist *sg,
+				   unsigned int nents, int prot);
 extern phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova);
 extern void iommu_set_fault_handler(struct iommu_domain *domain,
 			iommu_fault_handler_t handler, void *token);
@@ -679,18 +679,18 @@ static inline size_t iommu_unmap_fast(struct iommu_domain *domain,
 	return 0;
 }
 
-static inline size_t iommu_map_sg(struct iommu_domain *domain,
-				  unsigned long iova, struct scatterlist *sg,
-				  unsigned int nents, int prot)
+static inline ssize_t iommu_map_sg(struct iommu_domain *domain,
+				   unsigned long iova, struct scatterlist *sg,
+				   unsigned int nents, int prot)
 {
-	return 0;
+	return -ENODEV;
 }
 
-static inline size_t iommu_map_sg_atomic(struct iommu_domain *domain,
+static inline ssize_t iommu_map_sg_atomic(struct iommu_domain *domain,
 				  unsigned long iova, struct scatterlist *sg,
 				  unsigned int nents, int prot)
 {
-	return 0;
+	return -ENODEV;
 }
 
 static inline void iommu_flush_iotlb_all(struct iommu_domain *domain)
-- 
2.20.1


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

* [PATCH v1 04/16] dma-iommu: Return error code from iommu_dma_map_sg()
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (2 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 03/16] iommu: Return full error code from iommu_map_sg[_atomic]() Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-16  6:31   ` Christoph Hellwig
  2021-07-15 16:45 ` [PATCH v1 05/16] alpha: return error code from alpha_pci_map_sg() Logan Gunthorpe
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe, Joerg Roedel, Will Deacon

Pass through appropriate error codes from iommu_dma_map_sg() now that
the error code will be passed through dma_map_sgtable().

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Will Deacon <will@kernel.org>
---
 drivers/iommu/dma-iommu.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 98ba927aee1a..9d35e9994306 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -972,7 +972,7 @@ static int iommu_dma_map_sg_swiotlb(struct device *dev, struct scatterlist *sg,
 
 out_unmap:
 	iommu_dma_unmap_sg_swiotlb(dev, sg, i, dir, attrs | DMA_ATTR_SKIP_CPU_SYNC);
-	return 0;
+	return -EINVAL;
 }
 
 /*
@@ -993,11 +993,14 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
 	dma_addr_t iova;
 	size_t iova_len = 0;
 	unsigned long mask = dma_get_seg_boundary(dev);
+	ssize_t ret;
 	int i;
 
-	if (static_branch_unlikely(&iommu_deferred_attach_enabled) &&
-	    iommu_deferred_attach(dev, domain))
-		return 0;
+	if (static_branch_unlikely(&iommu_deferred_attach_enabled)) {
+		ret = iommu_deferred_attach(dev, domain);
+		if (ret)
+			return ret;
+	}
 
 	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
 		iommu_dma_sync_sg_for_device(dev, sg, nents, dir);
@@ -1045,14 +1048,17 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
 	}
 
 	iova = iommu_dma_alloc_iova(domain, iova_len, dma_get_mask(dev), dev);
-	if (!iova)
+	if (!iova) {
+		ret = -ENOMEM;
 		goto out_restore_sg;
+	}
 
 	/*
 	 * We'll leave any physical concatenation to the IOMMU driver's
 	 * implementation - it knows better than we do.
 	 */
-	if (iommu_map_sg_atomic(domain, iova, sg, nents, prot) < iova_len)
+	ret = iommu_map_sg_atomic(domain, iova, sg, nents, prot);
+	if (ret < iova_len)
 		goto out_free_iova;
 
 	return __finalise_sg(dev, sg, nents, iova);
@@ -1061,7 +1067,7 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg,
 	iommu_dma_free_iova(cookie, iova, iova_len, NULL);
 out_restore_sg:
 	__invalidate_sg(sg, nents);
-	return 0;
+	return ret;
 }
 
 static void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
-- 
2.20.1


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

* [PATCH v1 05/16] alpha: return error code from alpha_pci_map_sg()
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (3 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 04/16] dma-iommu: Return error code from iommu_dma_map_sg() Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 06/16] ARM/dma-mapping: return error code from .map_sg() ops Logan Gunthorpe
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe, Richard Henderson,
	Ivan Kokshaysky, Matt Turner

From: Martin Oliveira <martin.oliveira@eideticom.com>

The .map_sg() op now expects an error code instead of zero on failure.

pci_map_single_1() can fail for different reasons, but since the only
supported type of error return is DMA_MAPPING_ERROR, we coalesce those
errors into EINVAL.

ENOMEM is returned when no page tables can be allocated.

Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
---
 arch/alpha/kernel/pci_iommu.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index 35d7b3096d6e..72fc2465d13c 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -649,7 +649,9 @@ 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 != DMA_MAPPING_ERROR;
+		if (sg->dma_address == DMA_MAPPING_ERROR)
+			return -EINVAL;
+		return 1;
 	}
 
 	start = sg;
@@ -685,8 +687,10 @@ static int alpha_pci_map_sg(struct device *dev, struct scatterlist *sg,
 	if (out < end)
 		out->dma_length = 0;
 
-	if (out - start == 0)
+	if (out - start == 0) {
 		printk(KERN_WARNING "pci_map_sg failed: no entries?\n");
+		return -ENOMEM;
+	}
 	DBGA("pci_map_sg: %ld entries\n", out - start);
 
 	return out - start;
@@ -699,7 +703,7 @@ static int alpha_pci_map_sg(struct device *dev, struct scatterlist *sg,
 	   entries.  Unmap them now.  */
 	if (out > start)
 		pci_unmap_sg(pdev, start, out - start, dir);
-	return 0;
+	return -ENOMEM;
 }
 
 /* Unmap a set of streaming mode DMA translations.  Again, cpu read
-- 
2.20.1


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

* [PATCH v1 06/16] ARM/dma-mapping: return error code from .map_sg() ops
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (4 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 05/16] alpha: return error code from alpha_pci_map_sg() Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 07/16] ia64/sba_iommu: return error code from sba_map_sg_attrs() Logan Gunthorpe
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe, Russell King,
	Thomas Bogendoerfer

From: Martin Oliveira <martin.oliveira@eideticom.com>

The .map_sg() op now expects an error code instead of zero on failure,
so propagate any errors that may happen all the way up.

Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
---
 arch/arm/mm/dma-mapping.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index c4b8df2ad328..8c286e690756 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -980,7 +980,7 @@ int arm_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 {
 	const struct dma_map_ops *ops = get_dma_ops(dev);
 	struct scatterlist *s;
-	int i, j;
+	int i, j, ret;
 
 	for_each_sg(sg, s, nents, i) {
 #ifdef CONFIG_NEED_SG_DMA_LENGTH
@@ -988,7 +988,8 @@ int arm_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 #endif
 		s->dma_address = ops->map_page(dev, sg_page(s), s->offset,
 						s->length, dir, attrs);
-		if (dma_mapping_error(dev, s->dma_address))
+		ret = dma_mapping_error(dev, s->dma_address);
+		if (ret)
 			goto bad_mapping;
 	}
 	return nents;
@@ -996,7 +997,7 @@ int arm_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
  bad_mapping:
 	for_each_sg(sg, s, i, j)
 		ops->unmap_page(dev, sg_dma_address(s), sg_dma_len(s), dir, attrs);
-	return 0;
+	return ret;
 }
 
 /**
@@ -1622,7 +1623,7 @@ static int __iommu_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 		     bool is_coherent)
 {
 	struct scatterlist *s = sg, *dma = sg, *start = sg;
-	int i, count = 0;
+	int i, count = 0, ret;
 	unsigned int offset = s->offset;
 	unsigned int size = s->offset + s->length;
 	unsigned int max = dma_get_max_seg_size(dev);
@@ -1634,8 +1635,10 @@ static int __iommu_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 		s->dma_length = 0;
 
 		if (s->offset || (size & ~PAGE_MASK) || size + s->length > max) {
-			if (__map_sg_chunk(dev, start, size, &dma->dma_address,
-			    dir, attrs, is_coherent) < 0)
+			ret = __map_sg_chunk(dev, start, size,
+					     &dma->dma_address, dir, attrs,
+					     is_coherent);
+			if (ret < 0)
 				goto bad_mapping;
 
 			dma->dma_address += offset;
@@ -1648,8 +1651,9 @@ static int __iommu_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 		}
 		size += s->length;
 	}
-	if (__map_sg_chunk(dev, start, size, &dma->dma_address, dir, attrs,
-		is_coherent) < 0)
+	ret = __map_sg_chunk(dev, start, size, &dma->dma_address, dir, attrs,
+			     is_coherent);
+	if (ret < 0)
 		goto bad_mapping;
 
 	dma->dma_address += offset;
@@ -1660,7 +1664,7 @@ static int __iommu_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 bad_mapping:
 	for_each_sg(sg, s, count, i)
 		__iommu_remove_mapping(dev, sg_dma_address(s), sg_dma_len(s));
-	return 0;
+	return ret;
 }
 
 /**
-- 
2.20.1


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

* [PATCH v1 07/16] ia64/sba_iommu: return error code from sba_map_sg_attrs()
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (5 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 06/16] ARM/dma-mapping: return error code from .map_sg() ops Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 08/16] MIPS/jazzdma: return error code from jazz_dma_map_sg() Logan Gunthorpe
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe, Michael Ellerman,
	Niklas Schnelle, Thomas Bogendoerfer

From: Martin Oliveira <martin.oliveira@eideticom.com>

The .map_sg() op now expects an error code instead of zero on failure.

Propagate the return of dma_mapping_error() up, if it is an errno.

sba_coalesce_chunks() may only presently fail if sba_alloc_range()
fails, which in turn only fails if the iommu is out of mapping
resources, hence a -ENOMEM is used in that case.

Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Niklas Schnelle <schnelle@linux.ibm.com>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
---
 arch/ia64/hp/common/sba_iommu.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
index 9148ddbf02e5..09dbe07a18c1 100644
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -1431,7 +1431,7 @@ static int sba_map_sg_attrs(struct device *dev, struct scatterlist *sglist,
 			    unsigned long attrs)
 {
 	struct ioc *ioc;
-	int coalesced, filled = 0;
+	int coalesced, filled = 0, ret;
 #ifdef ASSERT_PDIR_SANITY
 	unsigned long flags;
 #endif
@@ -1458,8 +1458,9 @@ static int sba_map_sg_attrs(struct device *dev, struct scatterlist *sglist,
 		sglist->dma_length = sglist->length;
 		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;
+		ret = dma_mapping_error(dev, sglist->dma_address);
+		if (ret)
+			return ret;
 		return 1;
 	}
 
@@ -1486,7 +1487,7 @@ static int sba_map_sg_attrs(struct device *dev, struct scatterlist *sglist,
 	coalesced = sba_coalesce_chunks(ioc, dev, sglist, nents);
 	if (coalesced < 0) {
 		sba_unmap_sg_attrs(dev, sglist, nents, dir, attrs);
-		return 0;
+		return -ENOMEM;
 	}
 
 	/*
-- 
2.20.1


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

* [PATCH v1 08/16] MIPS/jazzdma: return error code from jazz_dma_map_sg()
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (6 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 07/16] ia64/sba_iommu: return error code from sba_map_sg_attrs() Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 09/16] powerpc/iommu: return error code from .map_sg() ops Logan Gunthorpe
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe, Thomas Bogendoerfer

From: Martin Oliveira <martin.oliveira@eideticom.com>

The .map_sg() op now expects an error code instead of zero on failure.

vdma_alloc() may fail for different reasons, but since it only supports
indicating an error via a return of DMA_MAPPING_ERROR, we coalesce the
different reasons into -EINVAL.

Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
---
 arch/mips/jazz/jazzdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/jazz/jazzdma.c b/arch/mips/jazz/jazzdma.c
index 461457b28982..3b99743435db 100644
--- a/arch/mips/jazz/jazzdma.c
+++ b/arch/mips/jazz/jazzdma.c
@@ -552,7 +552,7 @@ static int jazz_dma_map_sg(struct device *dev, struct scatterlist *sglist,
 				dir);
 		sg->dma_address = vdma_alloc(sg_phys(sg), sg->length);
 		if (sg->dma_address == DMA_MAPPING_ERROR)
-			return 0;
+			return -EINVAL;
 		sg_dma_len(sg) = sg->length;
 	}

--
2.20.1

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

* [PATCH v1 09/16] powerpc/iommu: return error code from .map_sg() ops
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (7 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 08/16] MIPS/jazzdma: return error code from jazz_dma_map_sg() Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 10/16] s390/pci: return error code from s390_dma_map_sg() Logan Gunthorpe
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Geoff Levand

From: Martin Oliveira <martin.oliveira@eideticom.com>

The .map_sg() op now expects an error code instead of zero on failure.

Propagate the error up if vio_dma_iommu_map_sg() fails.

ppc_iommu_map_sg() may fail either because of iommu_range_alloc() or
because of tbl->it_ops->set(). The former only supports returning an
error with DMA_MAPPING_ERROR and an examination of the latter indicates
that it may return arch-specific errors (for example,
tce_buildmulti_pSeriesLP()). Hence, coalesce all of those errors into
-EINVAL;

Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Geoff Levand <geoff@infradead.org>
---
 arch/powerpc/kernel/iommu.c             | 4 ++--
 arch/powerpc/platforms/ps3/system-bus.c | 2 +-
 arch/powerpc/platforms/pseries/vio.c    | 5 +++--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 2af89a5e379f..bd0ed618bfa5 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -473,7 +473,7 @@ int ppc_iommu_map_sg(struct device *dev, struct iommu_table *tbl,
 	BUG_ON(direction == DMA_NONE);
 
 	if ((nelems == 0) || !tbl)
-		return 0;
+		return -EINVAL;
 
 	outs = s = segstart = &sglist[0];
 	outcount = 1;
@@ -599,7 +599,7 @@ int ppc_iommu_map_sg(struct device *dev, struct iommu_table *tbl,
 		if (s == outs)
 			break;
 	}
-	return 0;
+	return -EINVAL;
 }
 
 
diff --git a/arch/powerpc/platforms/ps3/system-bus.c b/arch/powerpc/platforms/ps3/system-bus.c
index 1a5665875165..c54eb46f0cfb 100644
--- a/arch/powerpc/platforms/ps3/system-bus.c
+++ b/arch/powerpc/platforms/ps3/system-bus.c
@@ -663,7 +663,7 @@ static int ps3_ioc0_map_sg(struct device *_dev, struct scatterlist *sg,
 			   unsigned long attrs)
 {
 	BUG();
-	return 0;
+	return -EINVAL;
 }
 
 static void ps3_sb_unmap_sg(struct device *_dev, struct scatterlist *sg,
diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
index e00f3725ec96..e31e59c54f30 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -560,7 +560,8 @@ static int vio_dma_iommu_map_sg(struct device *dev, struct scatterlist *sglist,
 	for_each_sg(sglist, sgl, nelems, count)
 		alloc_size += roundup(sgl->length, IOMMU_PAGE_SIZE(tbl));
 
-	if (vio_cmo_alloc(viodev, alloc_size))
+	ret = vio_cmo_alloc(viodev, alloc_size);
+	if (ret)
 		goto out_fail;
 	ret = ppc_iommu_map_sg(dev, tbl, sglist, nelems, dma_get_mask(dev),
 			direction, attrs);
@@ -577,7 +578,7 @@ static int vio_dma_iommu_map_sg(struct device *dev, struct scatterlist *sglist,
 	vio_cmo_dealloc(viodev, alloc_size);
 out_fail:
 	atomic_inc(&viodev->cmo.allocs_failed);
-	return 0;
+	return ret;
 }
 
 static void vio_dma_iommu_unmap_sg(struct device *dev,
-- 
2.20.1


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

* [PATCH v1 10/16] s390/pci: return error code from s390_dma_map_sg()
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (8 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 09/16] powerpc/iommu: return error code from .map_sg() ops Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-16  8:24   ` Niklas Schnelle
  2021-07-15 16:45 ` [PATCH v1 11/16] sparc/iommu: return error codes from .map_sg() ops Logan Gunthorpe
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe, Niklas Schnelle,
	Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger

From: Martin Oliveira <martin.oliveira@eideticom.com>

The .map_sg() op now expects an error code instead of zero on failure.

So propagate the error from __s390_dma_map_sg() up.

Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Niklas Schnelle <schnelle@linux.ibm.com>
Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/pci/pci_dma.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
index ebc9a49523aa..c78b02012764 100644
--- a/arch/s390/pci/pci_dma.c
+++ b/arch/s390/pci/pci_dma.c
@@ -487,7 +487,7 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
 	unsigned int max = dma_get_max_seg_size(dev);
 	unsigned int size = s->offset + s->length;
 	unsigned int offset = s->offset;
-	int count = 0, i;
+	int count = 0, i, ret;
 
 	for (i = 1; i < nr_elements; i++) {
 		s = sg_next(s);
@@ -497,8 +497,9 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
 
 		if (s->offset || (size & ~PAGE_MASK) ||
 		    size + s->length > max) {
-			if (__s390_dma_map_sg(dev, start, size,
-					      &dma->dma_address, dir))
+			ret = __s390_dma_map_sg(dev, start, size,
+						&dma->dma_address, dir);
+			if (ret)
 				goto unmap;
 
 			dma->dma_address += offset;
@@ -511,7 +512,8 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
 		}
 		size += s->length;
 	}
-	if (__s390_dma_map_sg(dev, start, size, &dma->dma_address, dir))
+	ret = __s390_dma_map_sg(dev, start, size, &dma->dma_address, dir);
+	if (ret)
 		goto unmap;
 
 	dma->dma_address += offset;
@@ -523,7 +525,7 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
 		s390_dma_unmap_pages(dev, sg_dma_address(s), sg_dma_len(s),
 				     dir, attrs);
 
-	return 0;
+	return ret;
 }
 
 static void s390_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
-- 
2.20.1


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

* [PATCH v1 11/16] sparc/iommu: return error codes from .map_sg() ops
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (9 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 10/16] s390/pci: return error code from s390_dma_map_sg() Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 12/16] parisc: return error code " Logan Gunthorpe
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe, David S. Miller,
	Niklas Schnelle, Michael Ellerman

From: Martin Oliveira <martin.oliveira@eideticom.com>

The .map_sg() op now expects an error code instead of zero on failure.

Returning an errno from __sbus_iommu_map_sg() results in
sbus_iommu_map_sg_gflush() and sbus_iommu_map_sg_pflush() returning an
errno, as those functions are wrappers around __sbus_iommu_map_sg().

Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Niklas Schnelle <schnelle@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/sparc/kernel/iommu.c     | 4 ++--
 arch/sparc/kernel/pci_sun4v.c | 4 ++--
 arch/sparc/mm/iommu.c         | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/sparc/kernel/iommu.c b/arch/sparc/kernel/iommu.c
index a034f571d869..0589acd34201 100644
--- a/arch/sparc/kernel/iommu.c
+++ b/arch/sparc/kernel/iommu.c
@@ -448,7 +448,7 @@ static int dma_4u_map_sg(struct device *dev, struct scatterlist *sglist,
 	iommu = dev->archdata.iommu;
 	strbuf = dev->archdata.stc;
 	if (nelems == 0 || !iommu)
-		return 0;
+		return -EINVAL;
 
 	spin_lock_irqsave(&iommu->lock, flags);
 
@@ -580,7 +580,7 @@ static int dma_4u_map_sg(struct device *dev, struct scatterlist *sglist,
 	}
 	spin_unlock_irqrestore(&iommu->lock, flags);
 
-	return 0;
+	return -EINVAL;
 }
 
 /* If contexts are being used, they are the same in all of the mappings
diff --git a/arch/sparc/kernel/pci_sun4v.c b/arch/sparc/kernel/pci_sun4v.c
index 9de57e88f7a1..d90e80fa5705 100644
--- a/arch/sparc/kernel/pci_sun4v.c
+++ b/arch/sparc/kernel/pci_sun4v.c
@@ -486,7 +486,7 @@ static int dma_4v_map_sg(struct device *dev, struct scatterlist *sglist,
 
 	iommu = dev->archdata.iommu;
 	if (nelems == 0 || !iommu)
-		return 0;
+		return -EINVAL;
 	atu = iommu->atu;
 
 	prot = HV_PCI_MAP_ATTR_READ;
@@ -619,7 +619,7 @@ static int dma_4v_map_sg(struct device *dev, struct scatterlist *sglist,
 	}
 	local_irq_restore(flags);
 
-	return 0;
+	return -EINVAL;
 }
 
 static void dma_4v_unmap_sg(struct device *dev, struct scatterlist *sglist,
diff --git a/arch/sparc/mm/iommu.c b/arch/sparc/mm/iommu.c
index 0c0342e5b10d..01ffcedd159c 100644
--- a/arch/sparc/mm/iommu.c
+++ b/arch/sparc/mm/iommu.c
@@ -256,7 +256,7 @@ static int __sbus_iommu_map_sg(struct device *dev, struct scatterlist *sgl,
 		sg->dma_address =__sbus_iommu_map_page(dev, sg_page(sg),
 				sg->offset, sg->length, per_page_flush);
 		if (sg->dma_address == DMA_MAPPING_ERROR)
-			return 0;
+			return -EINVAL;
 		sg->dma_length = sg->length;
 	}
 
-- 
2.20.1


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

* [PATCH v1 12/16] parisc: return error code from .map_sg() ops
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (10 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 11/16] sparc/iommu: return error codes from .map_sg() ops Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 13/16] xen: swiotlb: return error code from xen_swiotlb_map_sg() Logan Gunthorpe
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe, James E.J. Bottomley,
	Helge Deller

From: Martin Oliveira <martin.oliveira@eideticom.com>

The .map_sg() op now expects an error code instead of zero on failure.

Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
---
 drivers/parisc/ccio-dma.c  | 2 +-
 drivers/parisc/sba_iommu.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c
index b5f9ee81a46c..a3a5cfda3d93 100644
--- a/drivers/parisc/ccio-dma.c
+++ b/drivers/parisc/ccio-dma.c
@@ -918,7 +918,7 @@ ccio_map_sg(struct device *dev, struct scatterlist *sglist, int nents,
 	BUG_ON(!dev);
 	ioc = GET_IOC(dev);
 	if (!ioc)
-		return 0;
+		return -ENODEV;
 	
 	DBG_RUN_SG("%s() START %d entries\n", __func__, nents);
 
diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
index dce4cdf786cd..9a6671a230ee 100644
--- a/drivers/parisc/sba_iommu.c
+++ b/drivers/parisc/sba_iommu.c
@@ -947,7 +947,7 @@ sba_map_sg(struct device *dev, struct scatterlist *sglist, int nents,
 
 	ioc = GET_IOC(dev);
 	if (!ioc)
-		return 0;
+		return -ENODEV;
 
 	/* Fast path single entry scatterlists. */
 	if (nents == 1) {
-- 
2.20.1


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

* [PATCH v1 13/16] xen: swiotlb: return error code from xen_swiotlb_map_sg()
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (11 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 12/16] parisc: return error code " Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-19 20:22   ` Boris Ostrovsky
  2021-07-15 16:45 ` [PATCH v1 14/16] x86/amd_gart: return error code from gart_map_sg() Logan Gunthorpe
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe, Konrad Rzeszutek Wilk,
	Boris Ostrovsky, Juergen Gross, Stefano Stabellini

From: Martin Oliveira <martin.oliveira@eideticom.com>

The .map_sg() op now expects an error code instead of zero on failure.

xen_swiotlb_map_sg() may only fail if xen_swiotlb_map_page() fails, but
xen_swiotlb_map_page() only supports returning errors as
DMA_MAPPING_ERROR. So coalesce all errors into EINVAL.

Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
---
 drivers/xen/swiotlb-xen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 24d11861ac7d..b5707127c9d7 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -509,7 +509,7 @@ xen_swiotlb_map_sg(struct device *dev, struct scatterlist *sgl, int nelems,
 out_unmap:
 	xen_swiotlb_unmap_sg(dev, sgl, i, dir, attrs | DMA_ATTR_SKIP_CPU_SYNC);
 	sg_dma_len(sgl) = 0;
-	return 0;
+	return -EINVAL;
 }
 
 static void
-- 
2.20.1


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

* [PATCH v1 14/16] x86/amd_gart: return error code from gart_map_sg()
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (12 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 13/16] xen: swiotlb: return error code from xen_swiotlb_map_sg() Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-16  6:32   ` Christoph Hellwig
  2021-07-15 16:45 ` [PATCH v1 15/16] dma-mapping: return error code from dma_dummy_map_sg() Logan Gunthorpe
                   ` (2 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, Niklas Schnelle,
	Thomas Bogendoerfer, Michael Ellerman

From: Martin Oliveira <martin.oliveira@eideticom.com>

The .map_sg() op now expects an error code instead of zero on failure.

So make __dma_map_cont() return a valid errno (which is then propagated
to gart_map_sg() via dma_map_cont()) and return it in case of failure.

Also, return -EINVAL in case of invalid nents.

Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Niklas Schnelle <schnelle@linux.ibm.com>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/x86/kernel/amd_gart_64.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c
index 9ac696487b13..46aea9a4f26b 100644
--- a/arch/x86/kernel/amd_gart_64.c
+++ b/arch/x86/kernel/amd_gart_64.c
@@ -331,7 +331,7 @@ static int __dma_map_cont(struct device *dev, struct scatterlist *start,
 	int i;
 
 	if (iommu_start == -1)
-		return -1;
+		return -ENOMEM;
 
 	for_each_sg(start, s, nelems, i) {
 		unsigned long pages, addr;
@@ -380,13 +380,13 @@ static int gart_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 		       enum dma_data_direction dir, unsigned long attrs)
 {
 	struct scatterlist *s, *ps, *start_sg, *sgmap;
-	int need = 0, nextneed, i, out, start;
+	int need = 0, nextneed, i, out, start, ret;
 	unsigned long pages = 0;
 	unsigned int seg_size;
 	unsigned int max_seg_size;
 
 	if (nents == 0)
-		return 0;
+		return -EINVAL;
 
 	out		= 0;
 	start		= 0;
@@ -414,8 +414,9 @@ static int gart_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 			if (!iommu_merge || !nextneed || !need || s->offset ||
 			    (s->length + seg_size > max_seg_size) ||
 			    (ps->offset + ps->length) % PAGE_SIZE) {
-				if (dma_map_cont(dev, start_sg, i - start,
-						 sgmap, pages, need) < 0)
+				ret = dma_map_cont(dev, start_sg, i - start,
+						   sgmap, pages, need);
+				if (ret < 0)
 					goto error;
 				out++;
 
@@ -432,7 +433,8 @@ static int gart_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 		pages += iommu_num_pages(s->offset, s->length, PAGE_SIZE);
 		ps = s;
 	}
-	if (dma_map_cont(dev, start_sg, i - start, sgmap, pages, need) < 0)
+	ret = dma_map_cont(dev, start_sg, i - start, sgmap, pages, need);
+	if (ret < 0)
 		goto error;
 	out++;
 	flush_gart();
@@ -458,7 +460,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 = DMA_MAPPING_ERROR;
-	return 0;
+	return ret;
 }
 
 /* allocate and map a coherent mapping */
-- 
2.20.1


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

* [PATCH v1 15/16] dma-mapping: return error code from dma_dummy_map_sg()
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (13 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 14/16] x86/amd_gart: return error code from gart_map_sg() Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-15 16:45 ` [PATCH v1 16/16] dma-mapping: Disallow .map_sg operations from returning zero on error Logan Gunthorpe
  2021-07-15 16:53 ` [PATCH v1 00/16] .map_sg() error cleanup Russell King (Oracle)
  16 siblings, 0 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe

From: Martin Oliveira <martin.oliveira@eideticom.com>

The .map_sg() op now expects an error code instead of zero on failure.

The only errno to return is -ENODEV in the case when DMA is not
supported.

Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 kernel/dma/dummy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/dma/dummy.c b/kernel/dma/dummy.c
index eacd4c5b10bf..ae9abebed0c4 100644
--- a/kernel/dma/dummy.c
+++ b/kernel/dma/dummy.c
@@ -22,7 +22,7 @@ static int dma_dummy_map_sg(struct device *dev, struct scatterlist *sgl,
 		int nelems, enum dma_data_direction dir,
 		unsigned long attrs)
 {
-	return 0;
+	return -ENODEV;
 }
 
 static int dma_dummy_supported(struct device *hwdev, u64 mask)
-- 
2.20.1


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

* [PATCH v1 16/16] dma-mapping: Disallow .map_sg operations from returning zero on error
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (14 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 15/16] dma-mapping: return error code from dma_dummy_map_sg() Logan Gunthorpe
@ 2021-07-15 16:45 ` Logan Gunthorpe
  2021-07-16  6:33   ` Christoph Hellwig
  2021-07-15 16:53 ` [PATCH v1 00/16] .map_sg() error cleanup Russell King (Oracle)
  16 siblings, 1 reply; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:45 UTC (permalink / raw)
  To: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Logan Gunthorpe

Now that all the .map_sg operations have been converted to returning
proper error codes, drop the code to handle a zero return value,
add a warning if a zero is returned and update the comment for the
map_sg operation.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 include/linux/dma-map-ops.h | 8 +++-----
 kernel/dma/mapping.c        | 6 +++---
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index eaa969be8284..f299bc1e317b 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -42,11 +42,9 @@ struct dma_map_ops {
 			unsigned long attrs);
 	/*
 	 * map_sg should return a negative error code on error.
-	 * dma_map_sgtable() will return the error code returned and convert
-	 * a zero return (for legacy implementations) into -EINVAL.
-	 *
-	 * dma_map_sg() will always return zero on any negative or zero
-	 * return to satisfy its own calling convention.
+	 * dma_map_sgtable() will return the error code returned by the
+	 * operation and dma_map_sg() will always convert any error to zero
+	 * to satisfy its own calling convention.
 	 */
 	int (*map_sg)(struct device *dev, struct scatterlist *sg, int nents,
 			enum dma_data_direction dir, unsigned long attrs);
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 30f89d244566..978a6a16aaf7 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -194,6 +194,8 @@ static int __dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
 	else
 		ents = ops->map_sg(dev, sg, nents, dir, attrs);
 
+	WARN_ON_ONCE(ents == 0);
+
 	if (ents > 0)
 		debug_dma_map_sg(dev, sg, nents, ents, dir);
 
@@ -251,9 +253,7 @@ int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
 	int nents;
 
 	nents = __dma_map_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
-	if (nents == 0)
-		return -EINVAL;
-	else if (nents < 0)
+	if (nents < 0)
 		return nents;
 
 	sgt->nents = nents;
-- 
2.20.1


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

* Re: [PATCH v1 00/16] .map_sg() error cleanup
  2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
                   ` (15 preceding siblings ...)
  2021-07-15 16:45 ` [PATCH v1 16/16] dma-mapping: Disallow .map_sg operations from returning zero on error Logan Gunthorpe
@ 2021-07-15 16:53 ` Russell King (Oracle)
  2021-07-15 16:56   ` Logan Gunthorpe
  16 siblings, 1 reply; 28+ messages in thread
From: Russell King (Oracle) @ 2021-07-15 16:53 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Stephen Bates, Martin Oliveira

On Thu, Jul 15, 2021 at 10:45:28AM -0600, Logan Gunthorpe wrote:
> Hi,
> 
> This series is spun out and expanded from my work to add P2PDMA support
> to DMA map operations[1].
> 
> The P2PDMA work requires distinguishing different error conditions in
> a map_sg operation. dma_map_sgtable() already allows for returning an
> error code (where as dma_map_sg() is only allowed to return zero)
> however, it currently only returns -EINVAL when a .map_sg() call returns
> zero.
> 
> This series cleans up all .map_sg() implementations to return appropriate
> error codes. After the cleanup, dma_map_sg() will still return zero,
> however dma_map_sgtable() will pass the error code from the .map_sg()
> call. Thanks go to Martn Oliveira for doing a lot of the cleanup of the
> obscure implementations.
> 
> The patch set is based off of v5.14-rc1 and a git repo can be found
> here:

Have all the callers for dma_map_sg() been updated to check for error
codes? If not, isn't that a pre-requisit to this patch set?

From what I see in Linus' current tree, we still have cases today
where the return value of dma_map_sg() is compared with zero to
detect failure, so I think that needs fixing before we start changing
the dma_map_sg() implementation to return negative numbers.

I also notice that there are various places that don't check the
return value - and returning a negative number instead of zero may
well cause random other bits to be set in fields.

So, I think there's a fair amount of work to do in all the drivers
before this change can be considered.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

* Re: [PATCH v1 00/16] .map_sg() error cleanup
  2021-07-15 16:53 ` [PATCH v1 00/16] .map_sg() error cleanup Russell King (Oracle)
@ 2021-07-15 16:56   ` Logan Gunthorpe
  0 siblings, 0 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-15 16:56 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Stephen Bates, Martin Oliveira




On 2021-07-15 10:53 a.m., Russell King (Oracle) wrote:
> On Thu, Jul 15, 2021 at 10:45:28AM -0600, Logan Gunthorpe wrote:
>> Hi,
>>
>> This series is spun out and expanded from my work to add P2PDMA support
>> to DMA map operations[1].
>>
>> The P2PDMA work requires distinguishing different error conditions in
>> a map_sg operation. dma_map_sgtable() already allows for returning an
>> error code (where as dma_map_sg() is only allowed to return zero)
>> however, it currently only returns -EINVAL when a .map_sg() call returns
>> zero.
>>
>> This series cleans up all .map_sg() implementations to return appropriate
>> error codes. After the cleanup, dma_map_sg() will still return zero,
>> however dma_map_sgtable() will pass the error code from the .map_sg()
>> call. Thanks go to Martn Oliveira for doing a lot of the cleanup of the
>> obscure implementations.
>>
>> The patch set is based off of v5.14-rc1 and a git repo can be found
>> here:
> 
> Have all the callers for dma_map_sg() been updated to check for error
> codes? If not, isn't that a pre-requisit to this patch set?

No. Perhaps I wasn't clear enough: This series is changing only
impelemntations of .map_sg(). It does *not* change the return code of
dma_map_sg(). dma_map_sg() will continue to return zero on error for the
foreseeable future. The dma_map_sgtable() call already allows returning
error codes and it will pass the new error code through. This is what
will be used in the P2PDMA work.

Logan

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

* Re: [PATCH v1 01/16] dma-mapping: Allow map_sg() ops to return negative error codes
  2021-07-15 16:45 ` [PATCH v1 01/16] dma-mapping: Allow map_sg() ops to return negative error codes Logan Gunthorpe
@ 2021-07-16  6:29   ` Christoph Hellwig
  0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2021-07-16  6:29 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Stephen Bates, Martin Oliveira

On Thu, Jul 15, 2021 at 10:45:29AM -0600, Logan Gunthorpe wrote:
> +	 * dma_map_sgtable() will return the error code returned and convert
> +	 * a zero return (for legacy implementations) into -EINVAL.
> +	 *
> +	 * dma_map_sg() will always return zero on any negative or zero
> +	 * return to satisfy its own calling convention.
>  	 */

I don't think this belongs here.

> +EXPORT_SYMBOL(dma_map_sgtable);

EXPORT_SYMBOL_GPL, please.

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

* Re: [PATCH v1 04/16] dma-iommu: Return error code from iommu_dma_map_sg()
  2021-07-15 16:45 ` [PATCH v1 04/16] dma-iommu: Return error code from iommu_dma_map_sg() Logan Gunthorpe
@ 2021-07-16  6:31   ` Christoph Hellwig
  0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2021-07-16  6:31 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Stephen Bates, Martin Oliveira, Joerg Roedel,
	Will Deacon

Careful here. What do all these errors from the low-level code mean
here?  I think we need to clearly standardize on what we actually
return from ->map_sg and possibly document what the callers expect and
can do, and enforce that only those error are reported.

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

* Re: [PATCH v1 14/16] x86/amd_gart: return error code from gart_map_sg()
  2021-07-15 16:45 ` [PATCH v1 14/16] x86/amd_gart: return error code from gart_map_sg() Logan Gunthorpe
@ 2021-07-16  6:32   ` Christoph Hellwig
  2021-07-16 12:11     ` Robin Murphy
  0 siblings, 1 reply; 28+ messages in thread
From: Christoph Hellwig @ 2021-07-16  6:32 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Stephen Bates, Martin Oliveira, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, Niklas Schnelle,
	Thomas Bogendoerfer, Michael Ellerman

On Thu, Jul 15, 2021 at 10:45:42AM -0600, Logan Gunthorpe wrote:
> @@ -458,7 +460,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 = DMA_MAPPING_ERROR;
> -	return 0;
> +	return ret;

While we're at it - setting the ->dma_address to DMA_MAPPING_ERROR is
not part of the ->map_sg calling convention.  Might be worth to fix
up while we're at it.

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

* Re: [PATCH v1 16/16] dma-mapping: Disallow .map_sg operations from returning zero on error
  2021-07-15 16:45 ` [PATCH v1 16/16] dma-mapping: Disallow .map_sg operations from returning zero on error Logan Gunthorpe
@ 2021-07-16  6:33   ` Christoph Hellwig
  2021-07-16 12:19     ` Robin Murphy
  2021-07-16 16:17     ` Logan Gunthorpe
  0 siblings, 2 replies; 28+ messages in thread
From: Christoph Hellwig @ 2021-07-16  6:33 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, Stephen Bates, Martin Oliveira

On Thu, Jul 15, 2021 at 10:45:44AM -0600, Logan Gunthorpe wrote:
> @@ -194,6 +194,8 @@ static int __dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
>  	else
>  		ents = ops->map_sg(dev, sg, nents, dir, attrs);
>  
> +	WARN_ON_ONCE(ents == 0);

Turns this into a negative error code while we're at it, just to keep
the callers sane?

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

* Re: [PATCH v1 10/16] s390/pci: return error code from s390_dma_map_sg()
  2021-07-15 16:45 ` [PATCH v1 10/16] s390/pci: return error code from s390_dma_map_sg() Logan Gunthorpe
@ 2021-07-16  8:24   ` Niklas Schnelle
  0 siblings, 0 replies; 28+ messages in thread
From: Niklas Schnelle @ 2021-07-16  8:24 UTC (permalink / raw)
  To: Logan Gunthorpe, linux-kernel, linux-alpha, linux-arm-kernel,
	linux-ia64, linux-mips, linuxppc-dev, linux-s390, sparclinux,
	iommu, linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger

On Thu, 2021-07-15 at 10:45 -0600, Logan Gunthorpe wrote:
> From: Martin Oliveira <martin.oliveira@eideticom.com>
> 
> The .map_sg() op now expects an error code instead of zero on failure.
> 
> So propagate the error from __s390_dma_map_sg() up.
> 
> Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Niklas Schnelle <schnelle@linux.ibm.com>
> Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/pci/pci_dma.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
> index ebc9a49523aa..c78b02012764 100644
> --- a/arch/s390/pci/pci_dma.c
> +++ b/arch/s390/pci/pci_dma.c
> @@ -487,7 +487,7 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
>  	unsigned int max = dma_get_max_seg_size(dev);
>  	unsigned int size = s->offset + s->length;
>  	unsigned int offset = s->offset;
> -	int count = 0, i;
> +	int count = 0, i, ret;
>  
>  	for (i = 1; i < nr_elements; i++) {
>  		s = sg_next(s);
> @@ -497,8 +497,9 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
>  
>  		if (s->offset || (size & ~PAGE_MASK) ||
>  		    size + s->length > max) {
> -			if (__s390_dma_map_sg(dev, start, size,
> -					      &dma->dma_address, dir))
> +			ret = __s390_dma_map_sg(dev, start, size,
> +						&dma->dma_address, dir);
> +			if (ret)
>  				goto unmap;
>  
>  			dma->dma_address += offset;
> @@ -511,7 +512,8 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
>  		}
>  		size += s->length;
>  	}
> -	if (__s390_dma_map_sg(dev, start, size, &dma->dma_address, dir))
> +	ret = __s390_dma_map_sg(dev, start, size, &dma->dma_address, dir);
> +	if (ret)
>  		goto unmap;
>  
>  	dma->dma_address += offset;
> @@ -523,7 +525,7 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
>  		s390_dma_unmap_pages(dev, sg_dma_address(s), sg_dma_len(s),
>  				     dir, attrs);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static void s390_dma_unmap_sg(struct device *dev, struct scatterlist *sg,

So the error codes we return are -ENOMEM if allocating a DMA
translation entry fails and -EINVAL if the DMA translation table hasn't
been initialized or the caller tries to map 0 sized memory. Are these
error codes that you would expect? If yes then this change looks good
to me.

Acked-by: Niklas Schnelle <schnelle@linux.ibm.com>


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

* Re: [PATCH v1 14/16] x86/amd_gart: return error code from gart_map_sg()
  2021-07-16  6:32   ` Christoph Hellwig
@ 2021-07-16 12:11     ` Robin Murphy
  0 siblings, 0 replies; 28+ messages in thread
From: Robin Murphy @ 2021-07-16 12:11 UTC (permalink / raw)
  To: Christoph Hellwig, Logan Gunthorpe
  Cc: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel, Marek Szyprowski, Stephen Bates,
	Martin Oliveira, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Niklas Schnelle, Thomas Bogendoerfer,
	Michael Ellerman

On 2021-07-16 07:32, Christoph Hellwig wrote:
> On Thu, Jul 15, 2021 at 10:45:42AM -0600, Logan Gunthorpe wrote:
>> @@ -458,7 +460,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 = DMA_MAPPING_ERROR;
>> -	return 0;
>> +	return ret;
> 
> While we're at it - setting the ->dma_address to DMA_MAPPING_ERROR is
> not part of the ->map_sg calling convention.  Might be worth to fix
> up while we're at it.

Especially since it's not even zeroing dma_length, which at least is a 
documented indicator of validity (even if it's not strictly defined for 
failure cases either).

Robin.

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

* Re: [PATCH v1 16/16] dma-mapping: Disallow .map_sg operations from returning zero on error
  2021-07-16  6:33   ` Christoph Hellwig
@ 2021-07-16 12:19     ` Robin Murphy
  2021-07-16 16:17     ` Logan Gunthorpe
  1 sibling, 0 replies; 28+ messages in thread
From: Robin Murphy @ 2021-07-16 12:19 UTC (permalink / raw)
  To: Christoph Hellwig, Logan Gunthorpe
  Cc: linux-s390, linux-ia64, linux-parisc, Martin Oliveira,
	linux-mips, linux-kernel, iommu, linux-alpha, sparclinux,
	xen-devel, Stephen Bates, linuxppc-dev, linux-arm-kernel

On 2021-07-16 07:33, Christoph Hellwig wrote:
> On Thu, Jul 15, 2021 at 10:45:44AM -0600, Logan Gunthorpe wrote:
>> @@ -194,6 +194,8 @@ static int __dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
>>   	else
>>   		ents = ops->map_sg(dev, sg, nents, dir, attrs);
>>   
>> +	WARN_ON_ONCE(ents == 0);
> 
> Turns this into a negative error code while we're at it, just to keep
> the callers sane?

Right, by this point returning the 0 would pass through 
dma_map_sg_attrs() OK, but AFAICS dma_map_sgtable() would now get 
confused and return success but with sgt->nents = 0. Coercing it to an 
error code (which dma_map_sg_attrs() would then just change right back) 
seems sensible for the sake of easy robustness.

Robin.

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

* Re: [PATCH v1 16/16] dma-mapping: Disallow .map_sg operations from returning zero on error
  2021-07-16  6:33   ` Christoph Hellwig
  2021-07-16 12:19     ` Robin Murphy
@ 2021-07-16 16:17     ` Logan Gunthorpe
  1 sibling, 0 replies; 28+ messages in thread
From: Logan Gunthorpe @ 2021-07-16 16:17 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-kernel, linux-alpha, linux-arm-kernel, linux-ia64,
	linux-mips, linuxppc-dev, linux-s390, sparclinux, iommu,
	linux-parisc, xen-devel, Marek Szyprowski, Robin Murphy,
	Stephen Bates, Martin Oliveira



On 2021-07-16 12:33 a.m., Christoph Hellwig wrote:
> On Thu, Jul 15, 2021 at 10:45:44AM -0600, Logan Gunthorpe wrote:
>> @@ -194,6 +194,8 @@ static int __dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
>>  	else
>>  		ents = ops->map_sg(dev, sg, nents, dir, attrs);
>>  
>> +	WARN_ON_ONCE(ents == 0);
> 
> Turns this into a negative error code while we're at it, just to keep
> the callers sane?
> 

Sure thing. All the feedback makes sense, we'll fix it up and send a v2
in due course.

Thanks,

Logan

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

* Re: [PATCH v1 13/16] xen: swiotlb: return error code from xen_swiotlb_map_sg()
  2021-07-15 16:45 ` [PATCH v1 13/16] xen: swiotlb: return error code from xen_swiotlb_map_sg() Logan Gunthorpe
@ 2021-07-19 20:22   ` Boris Ostrovsky
  0 siblings, 0 replies; 28+ messages in thread
From: Boris Ostrovsky @ 2021-07-19 20:22 UTC (permalink / raw)
  To: Logan Gunthorpe, linux-kernel, linux-alpha, linux-arm-kernel,
	linux-ia64, linux-mips, linuxppc-dev, linux-s390, sparclinux,
	iommu, linux-parisc, xen-devel
  Cc: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Stephen Bates,
	Martin Oliveira, Konrad Rzeszutek Wilk, Juergen Gross,
	Stefano Stabellini


On 7/15/21 12:45 PM, Logan Gunthorpe wrote:
> From: Martin Oliveira <martin.oliveira@eideticom.com>
>
> The .map_sg() op now expects an error code instead of zero on failure.
>
> xen_swiotlb_map_sg() may only fail if xen_swiotlb_map_page() fails, but
> xen_swiotlb_map_page() only supports returning errors as
> DMA_MAPPING_ERROR. So coalesce all errors into EINVAL.


Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>




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

end of thread, other threads:[~2021-07-19 22:47 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 16:45 [PATCH v1 00/16] .map_sg() error cleanup Logan Gunthorpe
2021-07-15 16:45 ` [PATCH v1 01/16] dma-mapping: Allow map_sg() ops to return negative error codes Logan Gunthorpe
2021-07-16  6:29   ` Christoph Hellwig
2021-07-15 16:45 ` [PATCH v1 02/16] dma-direct: Return appropriate error code from dma_direct_map_sg() Logan Gunthorpe
2021-07-15 16:45 ` [PATCH v1 03/16] iommu: Return full error code from iommu_map_sg[_atomic]() Logan Gunthorpe
2021-07-15 16:45 ` [PATCH v1 04/16] dma-iommu: Return error code from iommu_dma_map_sg() Logan Gunthorpe
2021-07-16  6:31   ` Christoph Hellwig
2021-07-15 16:45 ` [PATCH v1 05/16] alpha: return error code from alpha_pci_map_sg() Logan Gunthorpe
2021-07-15 16:45 ` [PATCH v1 06/16] ARM/dma-mapping: return error code from .map_sg() ops Logan Gunthorpe
2021-07-15 16:45 ` [PATCH v1 07/16] ia64/sba_iommu: return error code from sba_map_sg_attrs() Logan Gunthorpe
2021-07-15 16:45 ` [PATCH v1 08/16] MIPS/jazzdma: return error code from jazz_dma_map_sg() Logan Gunthorpe
2021-07-15 16:45 ` [PATCH v1 09/16] powerpc/iommu: return error code from .map_sg() ops Logan Gunthorpe
2021-07-15 16:45 ` [PATCH v1 10/16] s390/pci: return error code from s390_dma_map_sg() Logan Gunthorpe
2021-07-16  8:24   ` Niklas Schnelle
2021-07-15 16:45 ` [PATCH v1 11/16] sparc/iommu: return error codes from .map_sg() ops Logan Gunthorpe
2021-07-15 16:45 ` [PATCH v1 12/16] parisc: return error code " Logan Gunthorpe
2021-07-15 16:45 ` [PATCH v1 13/16] xen: swiotlb: return error code from xen_swiotlb_map_sg() Logan Gunthorpe
2021-07-19 20:22   ` Boris Ostrovsky
2021-07-15 16:45 ` [PATCH v1 14/16] x86/amd_gart: return error code from gart_map_sg() Logan Gunthorpe
2021-07-16  6:32   ` Christoph Hellwig
2021-07-16 12:11     ` Robin Murphy
2021-07-15 16:45 ` [PATCH v1 15/16] dma-mapping: return error code from dma_dummy_map_sg() Logan Gunthorpe
2021-07-15 16:45 ` [PATCH v1 16/16] dma-mapping: Disallow .map_sg operations from returning zero on error Logan Gunthorpe
2021-07-16  6:33   ` Christoph Hellwig
2021-07-16 12:19     ` Robin Murphy
2021-07-16 16:17     ` Logan Gunthorpe
2021-07-15 16:53 ` [PATCH v1 00/16] .map_sg() error cleanup Russell King (Oracle)
2021-07-15 16:56   ` Logan Gunthorpe

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