From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:26:22
Hi all,
for a while we have a generic implementation of the dma mapping routines
that call into per-arch or per-device operations. But right now there
still are various bits in the interfaces where don't clearly operate
on these ops. This series tries to clean up a lot of those (but not all
yet, but the series is big enough). It gets rid of the DMA_ERROR_CODE
way of signaling failures of the mapping routines from the
implementations to the generic code (and cleans up various drivers that
were incorrectly using it), and gets rid of the ->set_dma_mask routine
in favor of relying on the ->dma_capable method that can be used in
the same way, but which requires less code duplication.
Btw, we don't seem to have a tree every-growing amount of common dma
mapping code, and given that I have a fair amount of all over the tree
work in that area in my plate I'd like to start one. Any good reason
to that? Anyone willing to volunteer as co maintainer?
The whole series is also available in git:
git://git.infradead.org/users/hch/misc.git dma-map
Gitweb:
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-map
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:26:26
DMA_ERROR_CODE is not supposed to be used by drivers.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/firmware/tegra/ivc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:26:32
That way the driver doesn't have to rely on DMA_ERROR_CODE, which
is not a public API and going away.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/net/ethernet/ibm/ibmveth.c | 159 +++++++++++++++++--------------------
1 file changed, 74 insertions(+), 85 deletions(-)
@@ -573,14 +523,17 @@ static int ibmveth_open(struct net_device *netdev)for(i=0;i<IBMVETH_NUM_BUFF_POOLS;i++)rxq_entries+=adapter->rx_buff_pool[i].size;+rc=-ENOMEM;adapter->buffer_list_addr=(void*)get_zeroed_page(GFP_KERNEL);-adapter->filter_list_addr=(void*)get_zeroed_page(GFP_KERNEL);+if(!adapter->buffer_list_addr){+netdev_err(netdev,"unable to allocate list pages\n");+gotoout;+}-if(!adapter->buffer_list_addr||!adapter->filter_list_addr){-netdev_err(netdev,"unable to allocate filter or buffer list "-"pages\n");-rc=-ENOMEM;-gotoerr_out;+adapter->filter_list_addr=(void*)get_zeroed_page(GFP_KERNEL);+if(!adapter->filter_list_addr){+netdev_err(netdev,"unable to allocate filter pages\n");+gotoout_free_buffer_list;}dev=&adapter->vdev->dev;
@@ -590,22 +543,21 @@ static int ibmveth_open(struct net_device *netdev)adapter->rx_queue.queue_addr=dma_alloc_coherent(dev,adapter->rx_queue.queue_len,&adapter->rx_queue.queue_dma,GFP_KERNEL);-if(!adapter->rx_queue.queue_addr){-rc=-ENOMEM;-gotoerr_out;-}+if(!adapter->rx_queue.queue_addr)+gotoout_free_filter_list;adapter->buffer_list_dma=dma_map_single(dev,adapter->buffer_list_addr,4096,DMA_BIDIRECTIONAL);+if(dma_mapping_error(dev,adapter->buffer_list_dma)){+netdev_err(netdev,"unable to map buffer list pages\n");+gotoout_free_queue_mem;+}+adapter->filter_list_dma=dma_map_single(dev,adapter->filter_list_addr,4096,DMA_BIDIRECTIONAL);--if((dma_mapping_error(dev,adapter->buffer_list_dma))||-(dma_mapping_error(dev,adapter->filter_list_dma))){-netdev_err(netdev,"unable to map filter or buffer list "-"pages\n");-rc=-ENOMEM;-gotoerr_out;+if(dma_mapping_error(dev,adapter->filter_list_dma)){+netdev_err(netdev,"unable to map filter list pages\n");+gotoout_unmap_buffer_list;}adapter->rx_queue.index=0;
@@ -636,7 +588,7 @@ static int ibmveth_open(struct net_device *netdev)rxq_desc.desc,mac_address);rc=-ENONET;-gotoerr_out;+gotoout_unmap_filter_list;}for(i=0;i<IBMVETH_NUM_BUFF_POOLS;i++){
@@ -646,7 +598,7 @@ static int ibmveth_open(struct net_device *netdev)netdev_err(netdev,"unable to alloc pool\n");adapter->rx_buff_pool[i].active=0;rc=-ENOMEM;-gotoerr_out;+gotoout_free_buffer_pools;}}
@@ -660,22 +612,21 @@ static int ibmveth_open(struct net_device *netdev)lpar_rc=h_free_logical_lan(adapter->vdev->unit_address);}while(H_IS_LONG_BUSY(lpar_rc)||(lpar_rc==H_BUSY));-gotoerr_out;+gotoout_free_buffer_pools;}+rc=-ENOMEM;adapter->bounce_buffer=kmalloc(netdev->mtu+IBMVETH_BUFF_OH,GFP_KERNEL);-if(!adapter->bounce_buffer){-rc=-ENOMEM;-gotoerr_out_free_irq;-}+if(!adapter->bounce_buffer)+gotoout_free_irq;+adapter->bounce_buffer_dma=dma_map_single(&adapter->vdev->dev,adapter->bounce_buffer,netdev->mtu+IBMVETH_BUFF_OH,DMA_BIDIRECTIONAL);if(dma_mapping_error(dev,adapter->bounce_buffer_dma)){netdev_err(netdev,"unable to map bounce buffer\n");-rc=-ENOMEM;-gotoerr_out_free_irq;+gotoout_free_bounce_buffer;}netdev_dbg(netdev,"initial replenish cycle\n");
@@ -687,10 +638,31 @@ static int ibmveth_open(struct net_device *netdev)return0;-err_out_free_irq:+out_free_bounce_buffer:+kfree(adapter->bounce_buffer);+out_free_irq:free_irq(netdev->irq,netdev);-err_out:-ibmveth_cleanup(adapter);+out_free_buffer_pools:+while(--i>=0){+if(adapter->rx_buff_pool[i].active)+ibmveth_free_buffer_pool(adapter,+&adapter->rx_buff_pool[i]);+}+out_unmap_filter_list:+dma_unmap_single(dev,adapter->filter_list_dma,4096,+DMA_BIDIRECTIONAL);+out_unmap_buffer_list:+dma_unmap_single(dev,adapter->buffer_list_dma,4096,+DMA_BIDIRECTIONAL);+out_free_queue_mem:+dma_free_coherent(dev,adapter->rx_queue.queue_len,+adapter->rx_queue.queue_addr,+adapter->rx_queue.queue_dma);+out_free_filter_list:+free_page((unsignedlong)adapter->filter_list_addr);+out_free_buffer_list:+free_page((unsignedlong)adapter->buffer_list_addr);+out:napi_disable(&adapter->napi);returnrc;}
@@ -698,7 +670,9 @@ static int ibmveth_open(struct net_device *netdev)staticintibmveth_close(structnet_device*netdev){structibmveth_adapter*adapter=netdev_priv(netdev);+structdevice*dev=&adapter->vdev->dev;longlpar_rc;+inti;netdev_dbg(netdev,"close starting\n");
@@ -722,7 +696,27 @@ static int ibmveth_close(struct net_device *netdev)ibmveth_update_rx_no_buffer(adapter);-ibmveth_cleanup(adapter);+dma_unmap_single(dev,adapter->buffer_list_dma,4096,+DMA_BIDIRECTIONAL);+free_page((unsignedlong)adapter->buffer_list_addr);++dma_unmap_single(dev,adapter->filter_list_dma,4096,+DMA_BIDIRECTIONAL);+free_page((unsignedlong)adapter->filter_list_addr);++dma_free_coherent(dev,adapter->rx_queue.queue_len,+adapter->rx_queue.queue_addr,+adapter->rx_queue.queue_dma);++for(i=0;i<IBMVETH_NUM_BUFF_POOLS;i++)+if(adapter->rx_buff_pool[i].active)+ibmveth_free_buffer_pool(adapter,+&adapter->rx_buff_pool[i]);++dma_unmap_single(&adapter->vdev->dev,adapter->bounce_buffer_dma,+adapter->netdev->mtu+IBMVETH_BUFF_OH,+DMA_BIDIRECTIONAL);+kfree(adapter->bounce_buffer);netdev_dbg(netdev,"close complete\n");
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:26:40
DMA_ERROR_CODE already isn't a valid API to user for drivers and will
go away soon. exynos_drm_fb_dma_addr uses it a an error return when
the passed in index is invalid, but the callers never check for it
but instead pass the address straight to the hardware.
Add a WARN_ON instead and just return 0.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/exynos/exynos_drm_fb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:26:53
DMA_ERROR_CODE is not a public API and will go away soon. dma dma-iommu
driver already implements a proper ->mapping_error method, so it's only
using the value internally. Add a new local define using the value
that arm64 which is the only current user of dma-iommu.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/iommu/dma-iommu.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:27:00
ARM and x86 had duplicated versions of the dma_ops structure, the
only difference is that x86 hasn't wired up the set_dma_mask,
mmap, and get_sgtable ops yet. On x86 all of them are identical
to the generic version, so they aren't needed but harmless.
All the symbols used only for xen_swiotlb_dma_ops can now be marked
static as well.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/arm/xen/mm.c | 17 --------
arch/x86/xen/pci-swiotlb-xen.c | 14 -------
drivers/xen/swiotlb-xen.c | 93 ++++++++++++++++++++++--------------------
include/xen/swiotlb-xen.h | 62 +---------------------------
4 files changed, 49 insertions(+), 137 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:27:10
DMA_ERROR_CODE is going to go away, so don't rely on it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/xen/swiotlb-xen.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:27:29
xtensa already implements the mapping_error method for its only
dma_map_ops instance.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/xtensa/include/asm/dma-mapping.h | 2 --
1 file changed, 2 deletions(-)
@@ -25,11 +25,11 @@#include<linux/module.h>#include<asm/page.h>+#define HEXAGON_MAPPING_ERROR 0+conststructdma_map_ops*dma_ops;EXPORT_SYMBOL(dma_ops);-intbad_dma_address;/* globals are automatically initialized to zero */-staticinlinevoid*dma_addr_to_virt(dma_addr_tdma_addr){returnphys_to_virt((unsignedlong)dma_addr);
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:27:42
s390 can also use noop_dma_ops, and while that currently does not return
errors it will so in the future. Implementing the mapping_error method
is the proper way to have per-ops error conditions.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/s390/include/asm/dma-mapping.h | 2 --
arch/s390/pci/pci_dma.c | 18 +++++++++++++-----
2 files changed, 13 insertions(+), 7 deletions(-)
@@ -329,7 +331,7 @@ static dma_addr_t s390_dma_map_pages(struct device *dev, struct page *page,/* This rounds up number of pages based on size and offset */nr_pages=iommu_num_pages(pa,size,PAGE_SIZE);dma_addr=dma_alloc_address(dev,nr_pages);-if(dma_addr==DMA_ERROR_CODE){+if(dma_addr==S390_MAPPING_ERROR){ret=-ENOSPC;gotoout_err;}
@@ -657,6 +664,7 @@ const struct dma_map_ops s390_pci_dma_ops = {.unmap_sg=s390_dma_unmap_sg,.map_page=s390_dma_map_pages,.unmap_page=s390_dma_unmap_pages,+.mapping_error=s390_mapping_error,/* if we support direct DMA this must be conditional */.is_phys=0,/* dma_supported is unconditionally true without a callback */
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:27:45
DMA_ERROR_CODE is going to go away, so don't rely on it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/iommu/amd_iommu.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:27:55
DMA_ERROR_CODE is going to go away, so don't rely on it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/x86/kernel/pci-calgary_64.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
@@ -252,7 +254,7 @@ static unsigned long iommu_range_alloc(struct device *dev,if(panic_on_overflow)panic("Calgary: fix the allocator.\n");else-returnDMA_ERROR_CODE;+returnCALGARY_MAPPING_ERROR;}}
@@ -272,10 +274,10 @@ static dma_addr_t iommu_alloc(struct device *dev, struct iommu_table *tbl,entry=iommu_range_alloc(dev,tbl,npages);-if(unlikely(entry==DMA_ERROR_CODE)){+if(unlikely(entry==CALGARY_MAPPING_ERROR)){pr_warn("failed to allocate %u pages in iommu %p\n",npages,tbl);-returnDMA_ERROR_CODE;+returnCALGARY_MAPPING_ERROR;}/* set the return dma address */
@@ -295,7 +297,7 @@ static void iommu_free(struct iommu_table *tbl, dma_addr_t dma_addr,unsignedlongflags;/* were we called with bad_dma_address? */-badend=DMA_ERROR_CODE+(EMERGENCY_PAGES*PAGE_SIZE);+badend=CALGARY_MAPPING_ERROR+(EMERGENCY_PAGES*PAGE_SIZE);if(unlikely(dma_addr<badend)){WARN(1,KERN_ERR"Calgary: driver tried unmapping bad DMA ""address 0x%Lx\n",dma_addr);
@@ -380,7 +382,7 @@ static int calgary_map_sg(struct device *dev, struct scatterlist *sg,npages=iommu_num_pages(vaddr,s->length,PAGE_SIZE);entry=iommu_range_alloc(dev,tbl,npages);-if(entry==DMA_ERROR_CODE){+if(entry==CALGARY_MAPPING_ERROR){/* makes sure unmap knows to stop */s->dma_length=0;gotoerror;
@@ -453,7 +455,7 @@ static void* calgary_alloc_coherent(struct device *dev, size_t size,/* set up tces to cover the allocated range */mapping=iommu_alloc(dev,tbl,ret,npages,DMA_BIDIRECTIONAL);-if(mapping==DMA_ERROR_CODE)+if(mapping==CALGARY_MAPPING_ERROR)gotofree;*dma_handle=mapping;returnret;
@@ -732,7 +740,7 @@ static void __init calgary_reserve_regions(struct pci_dev *dev)structiommu_table*tbl=pci_iommu(dev->bus);/* reserve EMERGENCY_PAGES from bad_dma_address and up */-iommu_range_reserve(tbl,DMA_ERROR_CODE,EMERGENCY_PAGES);+iommu_range_reserve(tbl,CALGARY_MAPPING_ERROR,EMERGENCY_PAGES);/* avoid the BIOS/VGA first 640KB-1MB region *//* for CalIOC2 - avoid the entire first MB */
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:28:01
All dma_map_ops instances now handle their errors through
->mapping_error.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/x86/include/asm/dma-mapping.h | 2 --
1 file changed, 2 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:28:12
We can just use pci32_dma_ops.
Btw, given that leon is 32-bit and appears to be PCI based, do even need
the special case for it in get_arch_dma_ops at all?
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/sparc/include/asm/dma-mapping.h | 3 +--
arch/sparc/kernel/ioport.c | 5 +----
2 files changed, 2 insertions(+), 6 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:28:17
Usually dma_supported decisions are done by the dma_map_ops instance.
Switch sparc to that model by providing a ->dma_supported instance for
sbus that always returns false, and implementations tailored to the sun4u
and sun4v cases for sparc64, and leave it unimplemented for PCI on
sparc32, which means always supported.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/sparc/include/asm/dma-mapping.h | 3 ---
arch/sparc/kernel/iommu.c | 40 +++++++++++++++---------------------
arch/sparc/kernel/ioport.c | 22 ++++++--------------
arch/sparc/kernel/pci_sun4v.c | 17 +++++++++++++++
4 files changed, 39 insertions(+), 43 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:28:27
And update the documentation - dma_mapping_error has been supported
everywhere for a long time.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
Documentation/DMA-API-HOWTO.txt | 31 +++++--------------------------
include/linux/dma-mapping.h | 5 -----
2 files changed, 5 insertions(+), 31 deletions(-)
@@ -550,32 +550,11 @@ and to unmap it: dma_unmap_single(dev, dma_handle, size, direction); You should call dma_mapping_error() as dma_map_single() could fail and return-error. Not all DMA implementations support the dma_mapping_error() interface.-However, it is a good practice to call dma_mapping_error() interface, which-will invoke the generic mapping error check interface. Doing so will ensure-that the mapping code will work correctly on all DMA implementations without-any dependency on the specifics of the underlying implementation. Using the-returned address without checking for errors could result in failures ranging-from panics to silent data corruption. A couple of examples of incorrect ways-to check for errors that make assumptions about the underlying DMA-implementation are as follows and these are applicable to dma_map_page() as-well.--Incorrect example 1:- dma_addr_t dma_handle;-- dma_handle = dma_map_single(dev, addr, size, direction);- if ((dma_handle & 0xffff != 0) || (dma_handle >= 0x1000000)) {- goto map_error;- }--Incorrect example 2:- dma_addr_t dma_handle;-- dma_handle = dma_map_single(dev, addr, size, direction);- if (dma_handle == DMA_ERROR_CODE) {- goto map_error;- }+error. Doing so will ensure that the mapping code will work correctly on all+DMA implementations without any dependency on the specifics of the underlying+implementation. Using the returned address without checking for errors could+result in failures ranging from panics to silent data corruption. The same+applies to dma_map_page() as well. You should call dma_unmap_single() when the DMA activity is finished, e.g., from the interrupt which told you that the DMA transfer is done.
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:28:29
These just duplicate the default behavior if no method is provided.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/dma-virt.c | 12 ------------
1 file changed, 12 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:28:35
This implementation is simply bogus - hexagon only has a simple
direct mapped DMA implementation and thus doesn't care about the
address.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/openrisc/include/asm/dma-mapping.h | 7 -------
1 file changed, 7 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:28:38
This implementation is simply bogus - hexagon only has a simple
direct mapped DMA implementation and thus doesn't care about the
address.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/hexagon/include/asm/dma-mapping.h | 2 --
arch/hexagon/kernel/dma.c | 9 ---------
2 files changed, 11 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:29:10
Besides removing the last instance of the set_dma_mask method this also
reduced the code duplication.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/powerpc/platforms/cell/iommu.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:29:12
By the time cell_pci_dma_dev_setup calls cell_dma_dev_setup no device can
have the fixed map_ops set yet as it's only set by the set_dma_mask
method. So move the setup for the fixed case to be only called in that
place instead of indirecting through cell_dma_dev_setup.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/powerpc/platforms/cell/iommu.c | 27 +++++++--------------------
1 file changed, 7 insertions(+), 20 deletions(-)
@@ -663,14 +663,9 @@ static const struct dma_map_ops dma_iommu_fixed_ops = {.mapping_error=dma_iommu_mapping_error,};-staticvoidcell_dma_dev_setup_fixed(structdevice*dev);-staticvoidcell_dma_dev_setup(structdevice*dev){-/* Order is important here, these are not mutually exclusive */-if(get_dma_ops(dev)==&dma_iommu_fixed_ops)-cell_dma_dev_setup_fixed(dev);-elseif(get_pci_dma_ops()==&dma_iommu_ops)+if(get_pci_dma_ops()==&dma_iommu_ops)set_iommu_table_base(dev,cell_get_iommu_table(dev));elseif(get_pci_dma_ops()==&dma_direct_ops)set_dma_offset(dev,cell_dma_direct_offset);
@@ -963,32 +958,24 @@ static int dma_set_mask_and_switch(struct device *dev, u64 dma_mask)return-EIO;if(dma_mask==DMA_BIT_MASK(64)&&-cell_iommu_get_fixed_address(dev)!=OF_BAD_ADDR)-{+cell_iommu_get_fixed_address(dev)!=OF_BAD_ADDR){+u64addr=cell_iommu_get_fixed_address(dev)++dma_iommu_fixed_base;dev_dbg(dev,"iommu: 64-bit OK, using fixed ops\n");+dev_dbg(dev,"iommu: fixed addr = %llx\n",addr);set_dma_ops(dev,&dma_iommu_fixed_ops);+set_dma_offset(dev,addr);}else{dev_dbg(dev,"iommu: not 64-bit, using default ops\n");set_dma_ops(dev,get_pci_dma_ops());+cell_dma_dev_setup(dev);}-cell_dma_dev_setup(dev);-*dev->dma_mask=dma_mask;return0;}-staticvoidcell_dma_dev_setup_fixed(structdevice*dev)-{-u64addr;--addr=cell_iommu_get_fixed_address(dev)+dma_iommu_fixed_base;-set_dma_offset(dev,addr);--dev_dbg(dev,"iommu: fixed addr = %llx\n",addr);-}-staticvoidinsert_16M_pte(unsignedlongaddr,unsignedlong*ptab,unsignedlongbase_pte){
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:30:29
These just duplicate the default behavior if no method is provided.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/tile/kernel/pci-dma.c | 30 ------------------------------
1 file changed, 30 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:31:05
This just duplicates the generic implementation.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/xen/swiotlb-xen.c | 12 ------------
1 file changed, 12 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:31:44
And instead wire it up as method for all the dma_map_ops instances.
Note that this also means the arch specific check will be fully instead
of partially applied in the AMD iommu driver.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/x86/include/asm/dma-mapping.h | 3 ---
arch/x86/include/asm/iommu.h | 2 ++
arch/x86/kernel/amd_gart_64.c | 1 +
arch/x86/kernel/pci-calgary_64.c | 1 +
arch/x86/kernel/pci-dma.c | 7 +------
arch/x86/kernel/pci-nommu.c | 1 +
arch/x86/pci/sta2x11-fixup.c | 3 ++-
drivers/iommu/amd_iommu.c | 2 ++
drivers/iommu/intel-iommu.c | 3 +++
9 files changed, 13 insertions(+), 10 deletions(-)
@@ -213,10 +213,8 @@ static __init int iommu_setup(char *p)}early_param("iommu",iommu_setup);-intdma_supported(structdevice*dev,u64mask)+intx86_dma_supported(structdevice*dev,u64mask){-conststructdma_map_ops*ops=get_dma_ops(dev);-#ifdef CONFIG_PCIif(mask>0xffffffff&&forbid_dac>0){dev_info(dev,"PCI: Disallowing DAC for device\n");
@@ -224,9 +222,6 @@ int dma_supported(struct device *dev, u64 mask)}#endif-if(ops->dma_supported)-returnops->dma_supported(dev,mask);-/* Copied from i386. Doesn't make much sense, because it willonlyworkforpci_alloc_coherent.ThecallerjusthastouseGFP_DMAinthiscase.*/
@@ -191,7 +192,7 @@ static const struct dma_map_ops sta2x11_dma_ops = {.sync_sg_for_cpu=swiotlb_sync_sg_for_cpu,.sync_sg_for_device=swiotlb_sync_sg_for_device,.mapping_error=swiotlb_dma_mapping_error,-.dma_supported=NULL,/* FIXME: we should use this instead! */+.dma_supported=x86_dma_supported,};/* At setup time, we use our own ops if the device is a ConneXt one */
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:33:07
And instead wire it up as method for all the dma_map_ops instances.
Note that the code seems a little fishy for dmabounce and iommu, but
for now I'd like to preserve the existing behavior 1:1.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/arm/common/dmabounce.c | 1 +
arch/arm/include/asm/dma-iommu.h | 2 ++
arch/arm/include/asm/dma-mapping.h | 3 ---
arch/arm/mm/dma-mapping.c | 7 +++++--
4 files changed, 8 insertions(+), 5 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:34:10
These just duplicate the default behavior if no method is provided.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/dma-noop.c | 12 ------------
1 file changed, 12 deletions(-)
@@ -9,6 +9,8 @@#include<linux/kmemcheck.h>#include<linux/kref.h>+#define ARM_MAPPING_ERROR (~(dma_addr_t)0x0)+structdma_iommu_mapping{/* iommu specific data */structiommu_domain*domain;
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:36:41
DMA_ERROR_CODE is going to go away, so don't rely on it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/x86/kernel/pci-nommu.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:37:42
DMA_ERROR_CODE is going to go away, so don't rely on it. Instead
define a ->mapping_error method for all IOMMU based dma operation
instances. The direct ops don't ever return an error and don't
need a ->mapping_error method.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/powerpc/include/asm/dma-mapping.h | 4 ----
arch/powerpc/include/asm/iommu.h | 4 ++++
arch/powerpc/kernel/dma-iommu.c | 6 ++++++
arch/powerpc/kernel/iommu.c | 28 ++++++++++++++--------------
arch/powerpc/platforms/cell/iommu.c | 1 +
arch/powerpc/platforms/pseries/vio.c | 3 ++-
6 files changed, 27 insertions(+), 19 deletions(-)
@@ -17,10 +17,6 @@#include<asm/io.h>#include<asm/swiotlb.h>-#ifdef CONFIG_PPC64-#define DMA_ERROR_CODE (~(dma_addr_t)0x0)-#endif-/* Some dma direct funcs must be visible for use in other dma_ops */externvoid*__dma_direct_alloc_coherent(structdevice*dev,size_tsize,dma_addr_t*dma_handle,gfp_tflag,
@@ -198,11 +198,11 @@ static unsigned long iommu_range_alloc(struct device *dev,if(unlikely(npages==0)){if(printk_ratelimit())WARN_ON(1);-returnDMA_ERROR_CODE;+returnIOMMU_MAPPING_ERROR;}if(should_fail_iommu(dev))-returnDMA_ERROR_CODE;+returnIOMMU_MAPPING_ERROR;/**Wedon'tneedtodisablepreemptionherebecauseanyCPUcan
@@ -278,7 +278,7 @@ static unsigned long iommu_range_alloc(struct device *dev,}else{/* Give up */spin_unlock_irqrestore(&(pool->lock),flags);-returnDMA_ERROR_CODE;+returnIOMMU_MAPPING_ERROR;}}
@@ -310,13 +310,13 @@ static dma_addr_t iommu_alloc(struct device *dev, struct iommu_table *tbl,unsignedlongattrs){unsignedlongentry;-dma_addr_tret=DMA_ERROR_CODE;+dma_addr_tret=IOMMU_MAPPING_ERROR;intbuild_fail;entry=iommu_range_alloc(dev,tbl,npages,NULL,mask,align_order);-if(unlikely(entry==DMA_ERROR_CODE))-returnDMA_ERROR_CODE;+if(unlikely(entry==IOMMU_MAPPING_ERROR))+returnIOMMU_MAPPING_ERROR;entry+=tbl->it_offset;/* Offset into real TCE table */ret=entry<<tbl->it_page_shift;/* Set the return dma address */
@@ -328,12 +328,12 @@ static dma_addr_t iommu_alloc(struct device *dev, struct iommu_table *tbl,/* tbl->it_ops->set() only returns non-zero for transient errors.*Cleanupthetablebitmapinthiscaseandreturn-*DMA_ERROR_CODE.Forallothererrorsthefunctionalityis+*IOMMU_MAPPING_ERROR.Forallothererrorsthefunctionalityis*notaltered.*/if(unlikely(build_fail)){__iommu_free(tbl,ret,npages);-returnDMA_ERROR_CODE;+returnIOMMU_MAPPING_ERROR;}/* Flush/invalidate TLB caches if necessary */
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:38:55
The dma alloc interface returns an error by return NULL, and the
mapping interfaces rely on the mapping_error method, which the dummy
ops already implement correctly.
Thus remove the DMA_ERROR_CODE define.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/arm64/include/asm/dma-mapping.h | 1 -
arch/arm64/mm/dma-mapping.c | 3 +--
2 files changed, 1 insertion(+), 3 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:40:30
sh does not return errors for dma_map_page.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/sh/include/asm/dma-mapping.h | 2 --
1 file changed, 2 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:40:59
dma-noop is the only dma_mapping_ops instance for m32r and does not return
errors.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/m32r/include/asm/dma-mapping.h | 2 --
1 file changed, 2 deletions(-)
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:44:48
dev_addr isn't even a dma_addr_t, and DMA_ERROR_CODE has never been
a valid driver API. Add a bool mapped flag instead.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/armada/armada_fb.c | 2 +-
drivers/gpu/drm/armada/armada_gem.c | 5 ++---
drivers/gpu/drm/armada/armada_gem.h | 1 +
3 files changed, 4 insertions(+), 4 deletions(-)
@@ -16,6 +16,7 @@ struct armada_gem_object {void*addr;phys_addr_tphys_addr;resource_size_tdev_addr;+boolmapped;structdrm_mm_node*linear;/* for linear backed */structpage*page;/* for page backed */structsg_table*sgt;/* for imported */
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-08 13:46:21
DMA_ERROR_CODE is not a public API and will go away. Instead properly
unwind based on the loop counter.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/dma/ioat/init.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2017-06-08 13:59:21
Hi Christoph,
On 08/06/17 14:25, Christoph Hellwig wrote:
DMA_ERROR_CODE is not a public API and will go away soon. dma dma-iommu
driver already implements a proper ->mapping_error method, so it's only
using the value internally. Add a new local define using the value
that arm64 which is the only current user of dma-iommu.
It would be fine to just use 0, since dma-iommu already makes sure that
that will never be allocated for a valid DMA address.
Otherwise, looks good!
Robin.
From: Robin Murphy <robin.murphy@arm.com> Date: 2017-06-08 14:02:10
On 08/06/17 14:25, Christoph Hellwig wrote:
The dma alloc interface returns an error by return NULL, and the
mapping interfaces rely on the mapping_error method, which the dummy
ops already implement correctly.
Thus remove the DMA_ERROR_CODE define.
From: David Miller <davem@davemloft.net> Date: 2017-06-08 14:21:45
From: Christoph Hellwig <hch@lst.de>
Date: Thu, 8 Jun 2017 15:25:25 +0200
for a while we have a generic implementation of the dma mapping routines
that call into per-arch or per-device operations. But right now there
still are various bits in the interfaces where don't clearly operate
on these ops. This series tries to clean up a lot of those (but not all
yet, but the series is big enough). It gets rid of the DMA_ERROR_CODE
way of signaling failures of the mapping routines from the
implementations to the generic code (and cleans up various drivers that
were incorrectly using it), and gets rid of the ->set_dma_mask routine
in favor of relying on the ->dma_capable method that can be used in
the same way, but which requires less code duplication.
There is unlikely to be conflicts for the sparc and net changes, so I
will simply ACK them.
Thanks Christoph.
From: David Miller <davem@davemloft.net> Date: 2017-06-08 14:22:41
From: Christoph Hellwig <hch@lst.de>
Date: Thu, 8 Jun 2017 15:25:52 +0200
We can just use pci32_dma_ops.
Btw, given that leon is 32-bit and appears to be PCI based, do even need
the special case for it in get_arch_dma_ops at all?
I would need to defer to the LEON developers on that, but they haven't
been very actively lately so whether you'll get a response or not is
hard to predict.
Hi Christoph,
On Thu, Jun 8, 2017 at 11:25 PM, Christoph Hellwig [off-list ref] wrote:
quoted hunk
Usually dma_supported decisions are done by the dma_map_ops instance.
Switch sparc to that model by providing a ->dma_supported instance for
sbus that always returns false, and implementations tailored to the sun4u
and sun4v cases for sparc64, and leave it unimplemented for PCI on
sparc32, which means always supported.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/sparc/include/asm/dma-mapping.h | 3 ---
arch/sparc/kernel/iommu.c | 40 +++++++++++++++---------------------
arch/sparc/kernel/ioport.c | 22 ++++++--------------
arch/sparc/kernel/pci_sun4v.c | 17 +++++++++++++++
4 files changed, 39 insertions(+), 43 deletions(-)
I'm guessing there's a few places that have DMA ops but DMA isn't
actually supported. Why not have a common method for this, maybe
"dma_not_supported"?
From: David Miller <davem@davemloft.net> Date: 2017-06-08 14:24:12
From: Christoph Hellwig <hch@lst.de>
Date: Thu, 8 Jun 2017 15:25:53 +0200
Usually dma_supported decisions are done by the dma_map_ops instance.
Switch sparc to that model by providing a ->dma_supported instance for
sbus that always returns false, and implementations tailored to the sun4u
and sun4v cases for sparc64, and leave it unimplemented for PCI on
sparc32, which means always supported.
Signed-off-by: Christoph Hellwig <hch@lst.de>
From: Russell King - ARM Linux <linux@armlinux.org.uk> Date: 2017-06-08 14:43:39
BOn Thu, Jun 08, 2017 at 03:25:50PM +0200, Christoph Hellwig wrote:
+static int dmabounce_mapping_error(struct device *dev, dma_addr_t dma_addr)
+{
+ if (dev->archdata.dmabounce)
+ return 0;
I'm not convinced that we need this check here:
dev->archdata.dmabounce = device_info;
set_dma_ops(dev, &dmabounce_ops);
There shouldn't be any chance of dev->archdata.dmabounce being NULL if
the dmabounce_ops has been set as the current device DMA ops. So I
think that test can be killed.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
On Thu, 8 Jun 2017 15:25:44 +0200
Christoph Hellwig [off-list ref] wrote:
s390 can also use noop_dma_ops, and while that currently does not return
errors it will so in the future. Implementing the mapping_error method
is the proper way to have per-ops error conditions.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Hi Christoph,
On Thu, Jun 8, 2017 at 3:25 PM, Christoph Hellwig [off-list ref] wrote:
This implementation is simply bogus - hexagon only has a simple
openrisc?
direct mapped DMA implementation and thus doesn't care about the
address.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/openrisc/include/asm/dma-mapping.h | 7 -------
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
From: Konrad Rzeszutek Wilk <hidden> Date: 2017-06-11 02:38:18
On Thu, Jun 08, 2017 at 03:25:32PM +0200, Christoph Hellwig wrote:
ARM and x86 had duplicated versions of the dma_ops structure, the
only difference is that x86 hasn't wired up the set_dma_mask,
mmap, and get_sgtable ops yet. On x86 all of them are identical
to the generic version, so they aren't needed but harmless.
All the symbols used only for xen_swiotlb_dma_ops can now be marked
static as well.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/arm/xen/mm.c | 17 --------
arch/x86/xen/pci-swiotlb-xen.c | 14 -------
drivers/xen/swiotlb-xen.c | 93 ++++++++++++++++++++++--------------------
include/xen/swiotlb-xen.h | 62 +---------------------------
4 files changed, 49 insertions(+), 137 deletions(-)
Yeeey!
Reviewed-by: Konrad Rzeszutek Wilk <redacted>
From: Andreas Larsson <andreas@gaisler.com> Date: 2017-06-12 08:33:45
On 2017-06-08 15:25, Christoph Hellwig wrote:
We can just use pci32_dma_ops.
Btw, given that leon is 32-bit and appears to be PCI based, do even need
the special case for it in get_arch_dma_ops at all?
Hi!
Yes, it is needed. LEON systems are AMBA bus based. The common case here
is DMA over AMBA buses. Some LEON systems have PCI bridges, but in
general CONFIG_PCI is not a given.
--
Andreas Larsson
Software Engineer
Cobham Gaisler
On Thu, Jun 08, 2017 at 03:25:26PM +0200, Christoph Hellwig wrote:
DMA_ERROR_CODE is not supposed to be used by drivers.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/firmware/tegra/ivc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2017-06-14 09:18:09
Christoph Hellwig [off-list ref] writes:
DMA_ERROR_CODE is going to go away, so don't rely on it. Instead
define a ->mapping_error method for all IOMMU based dma operation
instances. The direct ops don't ever return an error and don't
need a ->mapping_error method.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/powerpc/include/asm/dma-mapping.h | 4 ----
arch/powerpc/include/asm/iommu.h | 4 ++++
arch/powerpc/kernel/dma-iommu.c | 6 ++++++
arch/powerpc/kernel/iommu.c | 28 ++++++++++++++--------------
arch/powerpc/platforms/cell/iommu.c | 1 +
arch/powerpc/platforms/pseries/vio.c | 3 ++-
6 files changed, 27 insertions(+), 19 deletions(-)
I also see:
arch/powerpc/kernel/dma.c:const struct dma_map_ops dma_direct_ops = {
Which you mentioned can't fail.
arch/powerpc/platforms/pseries/ibmebus.c:static const struct dma_map_ops ibmebus_dma_ops = {
Which can't fail.
And:
arch/powerpc/platforms/powernv/npu-dma.c:static const struct dma_map_ops dma_npu_ops = {
arch/powerpc/platforms/ps3/system-bus.c:static const struct dma_map_ops ps3_sb_dma_ops = {
arch/powerpc/platforms/ps3/system-bus.c:static const struct dma_map_ops ps3_ioc0_dma_ops = {
All of which look like they definitely can fail, but return 0 on error
and don't implement ->mapping_error.
So I guess I'm acking this and adding a TODO to fix up the NPU code at
least, the ps3 code is probably better left alone these days.
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
Acked-by: Richard Kuo <redacted>
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
From: Richard Kuo <hidden> Date: 2017-06-16 00:20:30
On Thu, Jun 08, 2017 at 03:25:56PM +0200, Christoph Hellwig wrote:
This implementation is simply bogus - hexagon only has a simple
direct mapped DMA implementation and thus doesn't care about the
address.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/hexagon/include/asm/dma-mapping.h | 2 --
arch/hexagon/kernel/dma.c | 9 ---------
2 files changed, 11 deletions(-)
Acked-by: Richard Kuo <redacted>
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-16 08:37:06
On Thu, Jun 08, 2017 at 02:59:07PM +0100, Robin Murphy wrote:
Hi Christoph,
On 08/06/17 14:25, Christoph Hellwig wrote:
quoted
DMA_ERROR_CODE is not a public API and will go away soon. dma dma-iommu
driver already implements a proper ->mapping_error method, so it's only
using the value internally. Add a new local define using the value
that arm64 which is the only current user of dma-iommu.
It would be fine to just use 0, since dma-iommu already makes sure that
that will never be allocated for a valid DMA address.
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-16 08:43:35
On Thu, Jun 08, 2017 at 03:43:14PM +0100, Russell King - ARM Linux wrote:
BOn Thu, Jun 08, 2017 at 03:25:50PM +0200, Christoph Hellwig wrote:
quoted
+static int dmabounce_mapping_error(struct device *dev, dma_addr_t dma_addr)
+{
+ if (dev->archdata.dmabounce)
+ return 0;
I'm not convinced that we need this check here:
dev->archdata.dmabounce = device_info;
set_dma_ops(dev, &dmabounce_ops);
There shouldn't be any chance of dev->archdata.dmabounce being NULL if
the dmabounce_ops has been set as the current device DMA ops. So I
think that test can be killed.
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-16 08:45:48
On Mon, Jun 12, 2017 at 10:06:26AM +0200, Andreas Larsson wrote:
Yes, it is needed. LEON systems are AMBA bus based. The common case here is
DMA over AMBA buses. Some LEON systems have PCI bridges, but in general
CONFIG_PCI is not a given.
Ok, and even for AMBA we use the pci ops, so I'll leave it in and drop
the comment from the commit.
From: Christoph Hellwig <hch@lst.de> Date: 2017-06-16 08:47:07
On Fri, Jun 09, 2017 at 12:22:48AM +1000, Julian Calaby wrote:
I'm guessing there's a few places that have DMA ops but DMA isn't
actually supported. Why not have a common method for this, maybe
"dma_not_supported"?
It's not common at all. Except for sbus all dma API user first
call set_dma_mask which ends up in the dma_supported call. sbus
is the weird outlier here.
From: Daniel Vetter <hidden> Date: 2017-06-20 09:19:13
On Thu, Jun 08, 2017 at 03:25:25PM +0200, Christoph Hellwig wrote:
Hi all,
for a while we have a generic implementation of the dma mapping routines
that call into per-arch or per-device operations. But right now there
still are various bits in the interfaces where don't clearly operate
on these ops. This series tries to clean up a lot of those (but not all
yet, but the series is big enough). It gets rid of the DMA_ERROR_CODE
way of signaling failures of the mapping routines from the
implementations to the generic code (and cleans up various drivers that
were incorrectly using it), and gets rid of the ->set_dma_mask routine
in favor of relying on the ->dma_capable method that can be used in
the same way, but which requires less code duplication.
Btw, we don't seem to have a tree every-growing amount of common dma
mapping code, and given that I have a fair amount of all over the tree
work in that area in my plate I'd like to start one. Any good reason
to that? Anyone willing to volunteer as co maintainer?
The whole series is also available in git:
git://git.infradead.org/users/hch/misc.git dma-map
Ack for the 2 drm patches, but I can also pick them up through drm-misc if
you prefer that (but then it'll be 4.14).
-Daniel