DMA IOMMU static calls
Subsystems (4)
v3 cover DMA IOMMU static calls
Changelog:
v3:
* Changed coding style as Christoph asked
* Moved debug WARN_ON check to dma_set_mask.
* Slightly updated commit messages
* Added Greg to the CC list
v2: https://lore.kernel.org/all/[email protected]/
* Ditched dma_ops flag in favor of field in struct device (suggested by Christoph)
* Removed CONFIG_DMA_OPS select from dma-iommu.c Kconfig
* Removed flags field which exist only in default IOMMU
v1: https://lore.kernel.org/all/[email protected]/
* Dropped extra layer and called directly to iommu_dma_* functions
* Added unmap_page and unmap_sg to dummy ops
* Converted all dma-mapping calls to use iommu directly
* Updated commit messages
v0:
https://lore.kernel.org/all/98d1821780028434ff55b5d2f1feea287409fbc4.1720693745.git.leon@kernel.org/
Leon Romanovsky (2):
dma: call unconditionally to unmap_page and unmap_sg callbacks
dma: add IOMMU static calls with clear default ops
MAINTAINERS | 1 + drivers/iommu/Kconfig | 1 - drivers/iommu/dma-iommu.c | 127 +++++++++++---------------- include/linux/device.h | 5 ++ include/linux/dma-map-ops.h | 33 ++++--- include/linux/iommu-dma.h | 169 ++++++++++++++++++++++++++++++++++++ kernel/dma/dummy.c | 23 +++++ kernel/dma/mapping.c | 84 +++++++++++++++--- 8 files changed, 338 insertions(+), 105 deletions(-) create mode 100644 include/linux/iommu-dma.h
Signature · 1 line
2.45.2
PATCH v3 1/2 dma: call unconditionally to unmap_page and unmap_sg callbacks
From: Leon Romanovsky <[email protected]>
Almost all users of ->map_page()/map_sg() callbacks implement ->unmap_page()/unmap_sg() callbacks too. One user which doesn't do it, is dummy DMA ops interface, while the use of this interface is to fail the operation. In such case, there won't be any call to ->unmap_page()/unmap_sg() at all.
This patch removes the existence checks of ->unmap_page()/unmap_sg() and calls to it directly to create symmetrical interface to ->map_page()/map_sg().
--- kernel/dma/dummy.c | 23 +++++++++++++++++++++++ kernel/dma/mapping.c | 4 ++-- 2 files changed, 25 insertions(+), 2 deletions(-)
kernel/dma/dummy.c +23 -0
@@ -18,6 +18,17 @@ static dma_addr_t dma_dummy_map_page(struct device *dev, struct page *page,return DMA_MAPPING_ERROR ; } +static void dma_dummy_unmap_page (struct device *dev,dma_addr_t dma_handle, +size_t size,enum dma_data_direction dir, +unsigned long attrs) +{ +/* +* Dummy ops doesn't support map_page, so unmap_page should never be +* called. +*/ +WARN_ON_ONCE (true); +} +static int dma_dummy_map_sg (struct device *dev,struct scatterlist *sgl,int nelems,enum dma_data_direction dir,unsigned long attrs)
@@ -25,6 +36,16 @@ static int dma_dummy_map_sg(struct device *dev, struct scatterlist *sgl,return -EINVAL ; } +static void dma_dummy_unmap_sg (struct device *dev,struct scatterlist *sgl, +int nelems,enum dma_data_direction dir, +unsigned long attrs) +{ +/* +* Dummy ops doesn't support map_sg, so unmap_sg should never be called. +*/ +WARN_ON_ONCE (true); +} +static int dma_dummy_supported (struct device *hwdev,u64 mask) {return 0 ;
@@ -33,6 +54,8 @@ static int dma_dummy_supported(struct device *hwdev, u64 mask)const struct dma_map_ops dma_dummy_ops = { .mmap = dma_dummy_mmap, .map_page = dma_dummy_map_page, + .unmap_page = dma_dummy_unmap_page, .map_sg = dma_dummy_map_sg, + .unmap_sg = dma_dummy_unmap_sg, .dma_supported = dma_dummy_supported, };
kernel/dma/mapping.c +2 -2
@@ -177,7 +177,7 @@ void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, size_t size,if (dma_map_direct (dev, ops) ||arch_dma_unmap_page_direct (dev, addr + size))dma_direct_unmap_page (dev, addr, size, dir, attrs); -else if (ops->unmap_page) +else ops->unmap_page (dev, addr, size, dir, attrs);debug_dma_unmap_page (dev, addr, size, dir); }
@@ -291,7 +291,7 @@ void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,if (dma_map_direct (dev, ops) ||arch_dma_unmap_sg_direct (dev, sg, nents))dma_direct_unmap_sg (dev, sg, nents, dir, attrs); -else if (ops->unmap_sg) +else ops->unmap_sg (dev, sg, nents, dir, attrs); }EXPORT_SYMBOL (dma_unmap_sg_attrs);
Signature · 1 line
2.45.2
PATCH v3 2/2 dma: add IOMMU static calls with clear default ops
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
Most of the arch DMA ops (which often, but not always, involve some sort of IOMMU) are using the same DMA operations, but for all modern platforms dma-iommu implementation is really matters.
So let's make sure to call them directly without need to perform function pointers dereference.
During system initialization, the arch can set its own DMA and in such case, the default DMA operations will be overridden.
--- MAINTAINERS | 1 + drivers/iommu/Kconfig | 1 - drivers/iommu/dma-iommu.c | 127 +++++++++++---------------- include/linux/device.h | 5 ++ include/linux/dma-map-ops.h | 33 ++++--- include/linux/iommu-dma.h | 169 ++++++++++++++++++++++++++++++++++++ kernel/dma/mapping.c | 84 +++++++++++++++--- 7 files changed, 315 insertions(+), 105 deletions(-) create mode 100644 include/linux/iommu-dma.h
MAINTAINERS +1 -0
@@ -11525,6 +11525,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git F: Documentation/devicetree/bindings/iommu/ F: Documentation/userspace-api/iommu.rst F: drivers/iommu/ +F: include/linux/iommu-dma.h F: include/linux/iommu.h F: include/linux/iova.h F: include/linux/of_iommu.h
drivers/iommu/Kconfig +0 -1
@@ -151,7 +151,6 @@ config OF_IOMMU # IOMMU-agnostic DMA-mapping layer config IOMMU_DMA def_bool ARM64 || X86 || S390 - select DMA_OPS select IOMMU_API select IOMMU_IOVA select IRQ_MSI_IOMMU
drivers/iommu/dma-iommu.c +50 -77
260 lines not on this page · raw message
include/linux/device.h +5 -0
@@ -707,6 +707,8 @@ struct device_physical_location {* for dma allocations. This flag is managed by the dma ops * instance from ->dma_supported. * @dma_skip_sync: DMA sync operations can be skipped for coherent buffers. +* @dma_iommu: Device is using default IOMMU implementation for DMA and +* doesn't rely on dma_ops structure. * * At the lowest level, every device in a Linux system is represented by an * instance of struct device. The device structure contains the information
@@ -822,6 +824,9 @@ struct device {#ifdef CONFIG_DMA_NEED_SYNC bool dma_skip_sync:1;#endif +#ifdef CONFIG_IOMMU_DMA + bool dma_iommu : 1; +#endif };/**
include/linux/dma-map-ops.h +16 -17
@@ -13,20 +13,7 @@struct cma ;struct iommu_ops ; -/* -* Values for struct dma_map_ops.flags: -* -* DMA_F_PCI_P2PDMA_SUPPORTED: Indicates the dma_map_ops implementation can -* handle PCI P2PDMA pages in the map_sg/unmap_sg operation. -* DMA_F_CAN_SKIP_SYNC: DMA sync operations can be skipped if the device is -* coherent and it's not an SWIOTLB buffer. -*/ -#define DMA_F_PCI_P2PDMA_SUPPORTED (1 << 0) -#define DMA_F_CAN_SKIP_SYNC (1 << 1) -struct dma_map_ops { -unsigned int flags; -void *(*alloc)(struct device *dev,size_t size,dma_addr_t *dma_handle,gfp_t gfp,unsigned long attrs);
@@ -103,6 +90,10 @@ static inline void set_dma_ops(struct device *dev, { dev->dma_ops = dma_ops; } +struct page *dma_common_alloc_pages (struct device *dev,size_t size, +dma_addr_t *dma_handle,enum dma_data_direction dir,gfp_t gfp); +void dma_common_free_pages (struct device *dev,size_t size,struct page *vaddr, +dma_addr_t dma_handle,enum dma_data_direction dir);#else /* CONFIG_DMA_OPS */ static inline const struct dma_map_ops *get_dma_ops (struct device *dev) {
@@ -112,6 +103,18 @@ static inline void set_dma_ops(struct device *dev, const struct dma_map_ops *dma_ops) { } +static inline struct page * +dma_common_alloc_pages (struct device *dev,size_t size,dma_addr_t *dma_handle, +enum dma_data_direction dir,gfp_t gfp) +{ +return NULL ; +} +static inline void dma_common_free_pages (struct device *dev,size_t size, +struct page *vaddr, +dma_addr_t dma_handle, +enum dma_data_direction dir) +{ +}#endif /* CONFIG_DMA_OPS */ #ifdef CONFIG_DMA_CMA
@@ -239,10 +242,6 @@ int dma_common_get_sgtable(struct device *dev, struct sg_table *sgt,int dma_common_mmap (struct device *dev,struct vm_area_struct *vma,void *cpu_addr,dma_addr_t dma_addr,size_t size,unsigned long attrs); -struct page *dma_common_alloc_pages (struct device *dev,size_t size, -dma_addr_t *dma_handle,enum dma_data_direction dir,gfp_t gfp); -void dma_common_free_pages (struct device *dev,size_t size,struct page *vaddr, -dma_addr_t dma_handle,enum dma_data_direction dir);struct page **dma_common_find_pages (void *cpu_addr); void *dma_common_contiguous_remap(struct page *page, size_t size, pgprot_t prot,
include/linux/iommu-dma.h new file +169 -0
169 lines not on this page · raw message
kernel/dma/mapping.c +74 -10
258 lines not on this page · raw message
Signature · 1 line
2.45.2
Re: [PATCH v3 2/2] dma: add IOMMU static calls with clear default ops
drivers/iommu/dma-iommu.c static struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev, · line 1064
Quoted 5 lines · Leon Romanovsky
drivers/iommu/dma-iommu.c +3
-2-static void iommu_dma_free_noncontiguous (struct device *dev,size_t size, -struct sg_table *sgt,enum dma_data_direction dir) +void iommu_dma_free_noncontiguous (struct device *dev,size_t size, +struct sg_table *sgt, +enum dma_data_direction dir)
Why are you reformatting these to a much less readable coding style?
include/linux/dma-map-ops.h static inline void set_dma_ops(struct device *dev, · line 117
Quoted 12 lines
include/linux/dma-map-ops.h +12
-0+static inline struct page * +dma_common_alloc_pages (struct device *dev,size_t size,dma_addr_t *dma_handle, +enum dma_data_direction dir,gfp_t gfp) +{ +return NULL ; +} +static inline void dma_common_free_pages (struct device *dev,size_t size, +struct page *vaddr, +dma_addr_t dma_handle, +enum dma_data_direction dir) +{ +}
dma-iommu very much depends on these. So we'll need to also build ops_helpers.o if dma-iommu is enabled. So we'll probably need a separate Kconfig option for it that the various users select.
Re: [PATCH v3 2/2] dma: add IOMMU static calls with clear default ops
Quoted 8 lines · Christoph Hellwig
drivers/iommu/dma-iommu.c static struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev, · line 1064
Quoted 5 lines · Leon Romanovsky
drivers/iommu/dma-iommu.c +3
-2-static void iommu_dma_free_noncontiguous (struct device *dev,size_t size, -struct sg_table *sgt,enum dma_data_direction dir) +void iommu_dma_free_noncontiguous (struct device *dev,size_t size, +struct sg_table *sgt, +enum dma_data_direction dir)Why are you reformatting these to a much less readable coding style?
I'm relying on clang-formatter to format the code, see .clang-format file in the kernel root directory.
Quoted 18 lines
include/linux/dma-map-ops.h static inline void set_dma_ops(struct device *dev, · line 117
Quoted 13 lines
include/linux/dma-map-ops.h +12
-0+static inline struct page * +dma_common_alloc_pages (struct device *dev,size_t size,dma_addr_t *dma_handle, +enum dma_data_direction dir,gfp_t gfp) +{ +return NULL ; +} +static inline void dma_common_free_pages (struct device *dev,size_t size, +struct page *vaddr, +dma_addr_t dma_handle, +enum dma_data_direction dir) +{ +}dma-iommu very much depends on these. So we'll need to also build ops_helpers.o if dma-iommu is enabled. So we'll probably need a separate
Kconfigoption for it that the various users select.
I removed them from dma-iommu.c and there are no calls to them in that file. ➜ kernel git:(m/dma-v2) git grep dma_common_alloc_pages drivers/iommu/ | wc -l 0
Thanks
Re: [PATCH v3 2/2] dma: add IOMMU static calls with clear default ops
Quoted 10 lines · Leon Romanovsky
Quoted 8 lines · Christoph Hellwig
drivers/iommu/dma-iommu.c static struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev, · line 1064
Quoted 5 lines · Leon Romanovsky
drivers/iommu/dma-iommu.c +3
-2-static void iommu_dma_free_noncontiguous (struct device *dev,size_t size, -struct sg_table *sgt,enum dma_data_direction dir) +void iommu_dma_free_noncontiguous (struct device *dev,size_t size, +struct sg_table *sgt, +enum dma_data_direction dir)Why are you reformatting these to a much less readable coding style?
Quoted 2 lines
I'm relying on clang-formatter to format the code, see .clang-format file in the kernel root directory.
Well, that uses a really horrible default then. Aligning to the opening brace not only is hard to read, but requireѕ reformatting with every addition and removal of a static like here, or when trivially changing the function name.
Quoted 12 lines
Quoted 11 lines
include/linux/dma-map-ops.h static inline void set_dma_ops(struct device *dev, · line 117
Quoted 6 lines
include/linux/dma-map-ops.h +6
-0+static inline void dma_common_free_pages (struct device *dev,size_t size, +struct page *vaddr, +dma_addr_t dma_handle, +enum dma_data_direction dir) +{ +}dma-iommu very much depends on these. So we'll need to also build ops_helpers.o if dma-iommu is enabled. So we'll probably need a separate
Kconfigoption for it that the various users select.
Quoted 2 lines
I removed them from
dma-iommu.cand there are no calls to them in that file. ➜ kernel git:(m/dma-v2) git grepdma_common_alloc_pagesdrivers/iommu/| wc -l
Well, you replaced them with direct calls from mapping.c, which now needs them if dma-iommu is supported. So while strictly speaking dma-iommu doesn't need them, proper operation with dma-iommu still needs them.
Re: [PATCH v3 2/2] dma: add IOMMU static calls with clear default ops
Quoted 11 lines · Christoph Hellwig
Quoted 10 lines · Leon Romanovsky
Quoted 8 lines · Christoph Hellwig
drivers/iommu/dma-iommu.c static struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev, · line 1064
Quoted 5 lines · Leon Romanovsky
drivers/iommu/dma-iommu.c +3
-2-static void iommu_dma_free_noncontiguous (struct device *dev,size_t size, -struct sg_table *sgt,enum dma_data_direction dir) +void iommu_dma_free_noncontiguous (struct device *dev,size_t size, +struct sg_table *sgt, +enum dma_data_direction dir)Why are you reformatting these to a much less readable coding style?
Quoted 7 lines
Quoted 2 lines
I'm relying on clang-formatter to format the code, see .clang-format file in the kernel root directory.
Well, that uses a really horrible default then. Aligning to the opening brace not only is hard to read, but requireѕ reformatting with every addition and removal of a static like here, or when trivially changing the function name.
Should I need to reformat the code to your preferred style? I can do it, but it will be manual work :(.
Quoted 13 lines
Quoted 13 lines
Quoted 12 lines
include/linux/dma-map-ops.h static inline void set_dma_ops(struct device *dev, · line 117
Quoted 7 lines
include/linux/dma-map-ops.h +6
-0+static inline void dma_common_free_pages (struct device *dev,size_t size, +struct page *vaddr, +dma_addr_t dma_handle, +enum dma_data_direction dir) +{ +}dma-iommu very much depends on these. So we'll need to also build ops_helpers.o if dma-iommu is enabled. So we'll probably need a separate
Kconfigoption for it that the various users select.
Quoted 7 lines
Quoted 2 lines
I removed them from
dma-iommu.cand there are no calls to them in that file. ➜ kernel git:(m/dma-v2) git grepdma_common_alloc_pagesdrivers/iommu/| wc -lWell, you replaced them with direct calls from
mapping.c, which now needs them if dma-iommu is supported. So while strictly speaking dma-iommu doesn't need them, proper operation with dma-iommu still needs them.
No problem, I can add.
Thanks
Quoted 1 line
Re: [PATCH v3 2/2] dma: add IOMMU static calls with clear default ops
Quoted 2 lines · Leon Romanovsky
Should I need to reformat the code to your preferred style? I can do it, but it will be manual work :(.
Just keep it as it was. And preferably also follow the style in dma-iommu.h. In the worst case I can just fix it up when applying the patch.