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/20210513223203.5542-1-logang@deltatee.com/
[2] https://lore.kernel.org/linux-mips/20210715164544.6827-1-logang@deltatee.com/
--
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
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 <logang@deltatee.com>
---
kernel/dma/direct.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Martin Oliveira <redacted>
The .map_sg() op now expects an error code instead of zero on failure.
The only errno to return is -EINVAL in the case when DMA is not
supported.
Signed-off-by: Martin Oliveira <redacted>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
kernel/dma/dummy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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>
---
kernel/dma/mapping.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
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 <logang@deltatee.com>
---
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(-)
From: Martin Oliveira <redacted>
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 <redacted>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Konrad Rzeszutek Wilk <redacted>
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(-)
From: Martin Oliveira <redacted>
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 <redacted>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Thomas Gleixner <redacted>
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(-)
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/20210716063241.GC13345@lst.de/
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Thomas Gleixner <redacted>
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 | 2 --
1 file changed, 2 deletions(-)
From: Martin Oliveira <redacted>
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 <redacted>
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(-)
From: Martin Oliveira <redacted>
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 <redacted>
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(-)
@@ -580,7 +580,7 @@ static int dma_4u_map_sg(struct device *dev, struct scatterlist *sglist,}spin_unlock_irqrestore(&iommu->lock,flags);-return0;+return-EINVAL;}/* If contexts are being used, they are the same in all of the mappings
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/20210716063241.GC13345@lst.de/
Suggested-by: Christoph Hellwig <hch@lst.de>
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 | 2 --
arch/sparc/kernel/pci_sun4v.c | 2 --
2 files changed, 4 deletions(-)
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 <logang@deltatee.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Will Deacon <will@kernel.org>
---
drivers/iommu/dma-iommu.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
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/20210716063241.GC13345@lst.de/
Suggested-by: Christoph Hellwig <hch@lst.de>
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 | 1 -
1 file changed, 1 deletion(-)
From: Martin Oliveira <redacted>
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 <redacted>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Richard Henderson <redacted>
Cc: Ivan Kokshaysky <redacted>
Cc: Matt Turner <mattst88@gmail.com>
---
arch/alpha/kernel/pci_iommu.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
@@ -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);returnout-start;
@@ -699,7 +703,7 @@ static int alpha_pci_map_sg(struct device *dev, struct scatterlist *sg,entries.Unmapthemnow.*/if(out>start)pci_unmap_sg(pdev,start,out-start,dir);-return0;+return-ENOMEM;}/* Unmap a set of streaming mode DMA translations. Again, cpu read
From: Martin Oliveira <redacted>
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 <redacted>
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 | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
From: Martin Oliveira <redacted>
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 <redacted>
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 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
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/20210716063241.GC13345@lst.de/
Suggested-by: Christoph Hellwig <hch@lst.de>
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 <redacted>
Cc: Geoff Levand <geoff@infradead.org>
---
arch/powerpc/kernel/iommu.c | 2 --
1 file changed, 2 deletions(-)
From: Martin Oliveira <redacted>
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 <redacted>
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 <redacted>
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(-)
From: Martin Oliveira <redacted>
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 <redacted>
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(-)
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(-)
From: Martin Oliveira <redacted>
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 <redacted>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Acked-by: 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 <redacted>
---
arch/s390/pci/pci_dma.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)