2021-07-23 17:51:25

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 00/21] .map_sg() error cleanup

Hi,

This v2 of the series is spun out and expanded from my work to add
P2PDMA support to DMA map operations[1]. v1 is at [2]. The main changes
in v1 are to more carefully define the meaning of the error codes for
dma_map_sgtable().

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-rc2 and a git repo can be found
here:

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

Thanks,

Logan

[1] https://lore.kernel.org/linux-block/[email protected]/
[2] https://lore.kernel.org/linux-mips/[email protected]/

--

Changes in v2:
- Attempt to define the meanings of the errors returned by
dma_map_sgtable() and restrict the valid return codes of
.map_sg implementations. (Per Christoph)
- Change dma_map_sgtable() to EXPORT_SYMBOL_GPL() (Per Christoph)
- Add patches to remove the erroneous setting of sg->dma_address
to DMA_MAP_ERROR in a few .map_sg(0 implementations. (Per
Christoph).

--

Logan Gunthorpe (10):
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()
ARM/dma-mapping: don't set failed sg dma_address to DMA_MAPPING_ERROR
powerpc/iommu: don't set failed sg dma_address to DMA_MAPPING_ERROR
s390/pci: don't set failed sg dma_address to DMA_MAPPING_ERROR
sparc/iommu: don't set failed sg dma_address to DMA_MAPPING_ERROR
x86/amd_gart: don't set failed sg dma_address to DMA_MAPPING_ERROR
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 | 26 +++++---
arch/ia64/hp/common/sba_iommu.c | 6 +-
arch/mips/jazz/jazzdma.c | 2 +-
arch/powerpc/kernel/iommu.c | 6 +-
arch/powerpc/platforms/ps3/system-bus.c | 2 +-
arch/powerpc/platforms/pseries/vio.c | 5 +-
arch/s390/pci/pci_dma.c | 13 ++--
arch/sparc/kernel/iommu.c | 6 +-
arch/sparc/kernel/pci_sun4v.c | 6 +-
arch/sparc/mm/iommu.c | 2 +-
arch/x86/kernel/amd_gart_64.c | 18 +++---
drivers/iommu/dma-iommu.c | 23 +++++--
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 | 5 +-
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 | 86 ++++++++++++++++++++++---
23 files changed, 181 insertions(+), 117 deletions(-)


base-commit: 2734d6c1b1a089fb593ef6a23d4b70903526fe0c
--
2.20.1


2021-07-23 17:51:25

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 02/21] dma-direct: Return appropriate error code from dma_direct_map_sg()

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. Per the
documentation for dma_map_sgtable(), -EIO is returned due to an
DMA_MAPPING_ERROR with unknown cause.

Signed-off-by: Logan Gunthorpe <[email protected]>
---
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..f33ceb68aef2 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 -EIO;
}

dma_addr_t dma_direct_map_resource(struct device *dev, phys_addr_t paddr,
--
2.20.1

2021-07-23 17:51:30

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 01/21] dma-mapping: Allow map_sg() ops to return negative error codes

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() defines three error codes that .map_sg implementations
are allowed to return: -EINVAL, -ENOMEM and -EIO. The latter of which
is a generic return for cases that are passing DMA_MAPPING_ERROR
through.

dma_map_sgtable() will convert a zero error return for old map_sg() ops
into a -EIO 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 <[email protected]>
---
include/linux/dma-map-ops.h | 5 ++-
include/linux/dma-mapping.h | 35 +++------------
kernel/dma/mapping.c | 85 +++++++++++++++++++++++++++++++++----
3 files changed, 87 insertions(+), 38 deletions(-)

diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index 0d53a96a3d64..2f842498c448 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -41,8 +41,9 @@ 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. See
+ * dma_map_sgtable() for a list of appropriate error codes
+ * and their meanings.
*/
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..b8dc8b1cb402 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,86 @@ 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. The following
+ * error codes are supported with the given meaning:
+ *
+ * -EINVAL - An invalid argument, unaligned access or other error
+ * in usage. Will not succeed if retried.
+ * -ENOMEM - Insufficient resources (like memory or IOVA space) to
+ * complete the mapping. Should succeed if retried later.
+ * -EIO - Legacy error code with an unknown meaning. eg. this is
+ * returned if a lower level call returned DMA_MAPPING_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 -EIO;
+ else if (nents < 0) {
+ if (WARN_ON_ONCE(nents != -EINVAL && nents != -ENOMEM &&
+ nents != -EIO))
+ return -EIO;
+ return nents;
+ }
+
+ sgt->nents = nents;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(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

2021-07-23 17:51:36

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 17/21] xen: swiotlb: return error code from xen_swiotlb_map_sg()

From: Martin Oliveira <[email protected]>

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 EIO per the documentation
for dma_map_sgtable().

Signed-off-by: Martin Oliveira <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Boris Ostrovsky <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
Cc: Juergen Gross <[email protected]>
Cc: Stefano Stabellini <[email protected]>
---
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..85d58b720a24 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 -EIO;
}

static void
--
2.20.1

2021-07-23 17:51:37

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 18/21] x86/amd_gart: return error code from gart_map_sg()

From: Martin Oliveira <[email protected]>

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 <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Niklas Schnelle <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Michael Ellerman <[email protected]>
---
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

2021-07-23 17:51:44

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 19/21] x86/amd_gart: don't set failed sg dma_address to DMA_MAPPING_ERROR

Setting the ->dma_address to DMA_MAPPING_ERROR is not part of
the ->map_sg calling convention, so remove it.

Link: https://lore.kernel.org/linux-mips/[email protected]/
Suggested-by: Christoph Hellwig <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Niklas Schnelle <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Michael Ellerman <[email protected]>
---
arch/x86/kernel/amd_gart_64.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c
index 46aea9a4f26b..ed837383de5c 100644
--- a/arch/x86/kernel/amd_gart_64.c
+++ b/arch/x86/kernel/amd_gart_64.c
@@ -458,8 +458,6 @@ static int gart_map_sg(struct device *dev, struct scatterlist *sg, int nents,
panic("dma_map_sg: overflow on %lu pages\n", pages);

iommu_full(dev, pages << PAGE_SHIFT, dir);
- for_each_sg(sg, s, nents, i)
- s->dma_address = DMA_MAPPING_ERROR;
return ret;
}

--
2.20.1

2021-07-23 17:51:56

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 16/21] parisc: return error code from .map_sg() ops

From: Martin Oliveira <[email protected]>

The .map_sg() op now expects an error code instead of zero on failure.
Return -EINVAL if the ioc cannot be obtained.

Signed-off-by: Martin Oliveira <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: "James E.J. Bottomley" <[email protected]>
Cc: Helge Deller <[email protected]>
---
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..452e72b7bd01 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 -EINVAL;

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..e60690d38d67 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 -EINVAL;

/* Fast path single entry scatterlists. */
if (nents == 1) {
--
2.20.1

2021-07-23 17:52:02

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 13/21] s390/pci: don't set failed sg dma_address to DMA_MAPPING_ERROR

Setting the ->dma_address to DMA_MAPPING_ERROR is not part of
the ->map_sg calling convention, so remove it.

Link: https://lore.kernel.org/linux-mips/[email protected]/
Suggested-by: Christoph Hellwig <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: Niklas Schnelle <[email protected]>
Cc: Gerald Schaefer <[email protected]>
Cc: Heiko Carstens <[email protected]>
Cc: Vasily Gorbik <[email protected]>
Cc: Christian Borntraeger <[email protected]>
---
arch/s390/pci/pci_dma.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
index c78b02012764..be48e5b5bfcf 100644
--- a/arch/s390/pci/pci_dma.c
+++ b/arch/s390/pci/pci_dma.c
@@ -492,7 +492,6 @@ 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 = DMA_MAPPING_ERROR;
s->dma_length = 0;

if (s->offset || (size & ~PAGE_MASK) ||
--
2.20.1

2021-07-23 17:52:04

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 04/21] dma-iommu: Return error code from iommu_dma_map_sg()

Return appropriate error codes EINVAL or ENOMEM from
iommup_dma_map_sg(). If lower level code returns ENOMEM, then we
return it, other errors are coalesced into EINVAL.

iommu_dma_map_sg_swiotlb() returns -EIO as its an unknown error
from a call that returns DMA_MAPPING_ERROR.

Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Will Deacon <[email protected]>
---
drivers/iommu/dma-iommu.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 98ba927aee1a..d9aaed080e68 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 -EIO;
}

/*
@@ -993,11 +993,13 @@ 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);
+ goto out;
+ }

if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
iommu_dma_sync_sg_for_device(dev, sg, nents, dir);
@@ -1045,14 +1047,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 +1066,11 @@ 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;
+out:
+ if (ret == -ENOMEM)
+ return ret;
+ else
+ return -EINVAL;
}

static void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
--
2.20.1

2021-07-23 17:52:16

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 07/21] ARM/dma-mapping: don't set failed sg dma_address to DMA_MAPPING_ERROR

Setting the ->dma_address to DMA_MAPPING_ERROR is not part of the
->map_sg calling convention, so remove it.

Link: https://lore.kernel.org/linux-mips/[email protected]/
Suggested-by: Christoph Hellwig <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: Russell King <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
---
arch/arm/mm/dma-mapping.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 113b9cb3701b..4b61541853ea 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1632,7 +1632,6 @@ 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 = DMA_MAPPING_ERROR;
s->dma_length = 0;

if (s->offset || (size & ~PAGE_MASK) || size + s->length > max) {
--
2.20.1

2021-07-23 17:52:20

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 05/21] alpha: return error code from alpha_pci_map_sg()

From: Martin Oliveira <[email protected]>

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

ENOMEM is returned when no page tables can be allocated.

Signed-off-by: Martin Oliveira <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: Richard Henderson <[email protected]>
Cc: Ivan Kokshaysky <[email protected]>
Cc: Matt Turner <[email protected]>
---
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..21f9ac101324 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 -EIO;
+ 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

2021-07-23 17:52:21

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 08/21] ia64/sba_iommu: return error code from sba_map_sg_attrs()

From: Martin Oliveira <[email protected]>

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

In the case of a dma_mapping_error() return -EIO as the actual cause
is opaque here.

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 <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Niklas Schnelle <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
---
arch/ia64/hp/common/sba_iommu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
index 9148ddbf02e5..430c166b68cd 100644
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -1458,8 +1458,8 @@ 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;
+ if(dma_mapping_error(dev, sglist->dma_address))
+ return -EIO;
return 1;
}

@@ -1486,7 +1486,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

2021-07-23 17:52:28

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 10/21] powerpc/iommu: return error code from .map_sg() ops

From: Martin Oliveira <[email protected]>

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
-EIO, per the documentation on dma_map_sgtable().

Signed-off-by: Martin Oliveira <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Geoff Levand <[email protected]>
---
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..a8ec4fe42817 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 -EIO;
}


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

2021-07-23 17:52:28

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 03/21] iommu: Return full error code from iommu_map_sg[_atomic]()

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 <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Will Deacon <[email protected]>
---
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

2021-07-23 17:52:29

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 09/21] MIPS/jazzdma: return error code from jazz_dma_map_sg()

From: Martin Oliveira <[email protected]>

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 -EIO as is documented on dma_map_sgtable().

Signed-off-by: Martin Oliveira <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
---
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..eabddb89d221 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 -EIO;
sg_dma_len(sg) = sg->length;
}

--
2.20.1

2021-07-23 17:52:56

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 06/21] ARM/dma-mapping: return error code from .map_sg() ops

From: Martin Oliveira <[email protected]>

The .map_sg() op now expects an error code instead of zero on failure.
In the case of a DMA_MAPPING_ERROR, -EIO is returned. Otherwise,
-ENOMEM or -EINVAL is returned depending on the error from
__map_sg_chunk().

Signed-off-by: Martin Oliveira <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: Russell King <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
---
arch/arm/mm/dma-mapping.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index c4b8df2ad328..113b9cb3701b 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,15 +988,17 @@ 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))
+ if (dma_mapping_error(dev, s->dma_address)) {
+ ret = -EIO;
goto bad_mapping;
+ }
}
return 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 +1624,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 +1636,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 +1652,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 +1665,9 @@ 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;
+ if (ret == -ENOMEM)
+ return ret;
+ return -EINVAL;
}

/**
--
2.20.1

2021-07-23 17:52:57

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 15/21] sparc/iommu: don't set failed sg dma_address to DMA_MAPPING_ERROR

Setting the ->dma_address to DMA_MAPPING_ERROR is not part of
the ->map_sg calling convention, so remove it.

Link: https://lore.kernel.org/linux-mips/[email protected]/
Suggested-by: Christoph Hellwig <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Niklas Schnelle <[email protected]>
Cc: Michael Ellerman <[email protected]>
---
arch/sparc/kernel/iommu.c | 2 --
arch/sparc/kernel/pci_sun4v.c | 2 --
2 files changed, 4 deletions(-)

diff --git a/arch/sparc/kernel/iommu.c b/arch/sparc/kernel/iommu.c
index 0589acd34201..da0363692528 100644
--- a/arch/sparc/kernel/iommu.c
+++ b/arch/sparc/kernel/iommu.c
@@ -546,7 +546,6 @@ static int dma_4u_map_sg(struct device *dev, struct scatterlist *sglist,

if (outcount < incount) {
outs = sg_next(outs);
- outs->dma_address = DMA_MAPPING_ERROR;
outs->dma_length = 0;
}

@@ -572,7 +571,6 @@ 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 = DMA_MAPPING_ERROR;
s->dma_length = 0;
}
if (s == outs)
diff --git a/arch/sparc/kernel/pci_sun4v.c b/arch/sparc/kernel/pci_sun4v.c
index d90e80fa5705..384480971805 100644
--- a/arch/sparc/kernel/pci_sun4v.c
+++ b/arch/sparc/kernel/pci_sun4v.c
@@ -594,7 +594,6 @@ static int dma_4v_map_sg(struct device *dev, struct scatterlist *sglist,

if (outcount < incount) {
outs = sg_next(outs);
- outs->dma_address = DMA_MAPPING_ERROR;
outs->dma_length = 0;
}

@@ -611,7 +610,6 @@ 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 = DMA_MAPPING_ERROR;
s->dma_length = 0;
}
if (s == outs)
--
2.20.1

2021-07-23 17:53:05

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 14/21] sparc/iommu: return error codes from .map_sg() ops

From: Martin Oliveira <[email protected]>

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 <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Niklas Schnelle <[email protected]>
Cc: Michael Ellerman <[email protected]>
---
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..9e3f6933ca13 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 -EIO;
sg->dma_length = sg->length;
}

--
2.20.1

2021-07-23 17:53:05

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 12/21] s390/pci: return error code from s390_dma_map_sg()

From: Martin Oliveira <[email protected]>

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

So propagate the error from __s390_dma_map_sg() up. __s390_dma_map_sg()
returns either -ENOMEM on allocation failure or -EINVAL which is
the same as what's expected by dma_map_sgtable().

Signed-off-by: Martin Oliveira <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Acked-by: Niklas Schnelle <[email protected]>
Cc: Gerald Schaefer <[email protected]>
Cc: Heiko Carstens <[email protected]>
Cc: Vasily Gorbik <[email protected]>
Cc: Christian Borntraeger <[email protected]>
---
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

2021-07-23 17:53:30

by Logan Gunthorpe

[permalink] [raw]
Subject: [PATCH v2 11/21] powerpc/iommu: don't set failed sg dma_address to DMA_MAPPING_ERROR

Setting the ->dma_address to DMA_MAPPING_ERROR is not part of
the ->map_sg calling convention, so remove it.

Link: https://lore.kernel.org/linux-mips/[email protected]/
Suggested-by: Christoph Hellwig <[email protected]>
Signed-off-by: Logan Gunthorpe <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Geoff Levand <[email protected]>
---
arch/powerpc/kernel/iommu.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index a8ec4fe42817..30b7736f0896 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -575,7 +575,6 @@ int ppc_iommu_map_sg(struct device *dev, struct iommu_table *tbl,
*/
if (outcount < incount) {
outs = sg_next(outs);
- outs->dma_address = DMA_MAPPING_ERROR;
outs->dma_length = 0;
}

@@ -593,7 +592,6 @@ 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 = DMA_MAPPING_ERROR;
s->dma_length = 0;
}
if (s == outs)
--
2.20.1

2021-07-25 06:09:46

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2 01/21] dma-mapping: Allow map_sg() ops to return negative error codes

> +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 -EIO;
> + else if (nents < 0) {
> + if (WARN_ON_ONCE(nents != -EINVAL && nents != -ENOMEM &&
> + nents != -EIO))
> + return -EIO;

I think this validation of the errnos needs to go into __dma_map_sg_attrs,
so that we catch it for the classic dma_map_sg callers as well.

2021-08-02 10:35:54

by Niklas Schnelle

[permalink] [raw]
Subject: Re: [PATCH v2 13/21] s390/pci: don't set failed sg dma_address to DMA_MAPPING_ERROR

On Fri, 2021-07-23 at 11:50 -0600, Logan Gunthorpe wrote:
> Setting the ->dma_address to DMA_MAPPING_ERROR is not part of
> the ->map_sg calling convention, so remove it.
>
> Link: https://lore.kernel.org/linux-mips/[email protected]/
> Suggested-by: Christoph Hellwig <[email protected]>
> Signed-off-by: Logan Gunthorpe <[email protected]>
> Cc: Niklas Schnelle <[email protected]>
> Cc: Gerald Schaefer <[email protected]>
> Cc: Heiko Carstens <[email protected]>
> Cc: Vasily Gorbik <[email protected]>
> Cc: Christian Borntraeger <[email protected]>
> ---
> arch/s390/pci/pci_dma.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
> index c78b02012764..be48e5b5bfcf 100644
> --- a/arch/s390/pci/pci_dma.c
> +++ b/arch/s390/pci/pci_dma.c
> @@ -492,7 +492,6 @@ 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 = DMA_MAPPING_ERROR;
> s->dma_length = 0;
>
> if (s->offset || (size & ~PAGE_MASK) ||

Acked-by: Niklas Schnelle <[email protected]>

Thanks!