iommu/dma: Implement dma_{map,unmap}_resource()
Subsystems (3)
Robin Murphy
maintainerPATCH 1/2 iommu/dma: Implement dma_{map,unmap}_resource()
With the new dma_{map,unmap}_resource() functions added to the DMA API for the benefit of cases like slave DMA, add suitable implementations to the arsenal of our generic layer. Since cache maintenance should not be a concern, these can both be standalone versions without the need for architecture-specific wrappers.
---
Since patch 2 has a build dependency on this one, they should probably go together through either the arm64 tree or the iommu tree, but I can't make up my mind which one seems more appropriate...
Robin.
drivers/iommu/dma-iommu.c | 13 +++++++++++++ include/linux/dma-iommu.h | 4 ++++ 2 files changed, 17 insertions(+)
drivers/iommu/dma-iommu.c +13 -0
@@ -624,6 +624,19 @@ void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,__iommu_dma_unmap (iommu_get_domain_for_dev (dev),sg_dma_address (sg)); } +dma_addr_t iommu_dma_map_resource (struct device *dev,phys_addr_t phys, +size_t size,enum dma_data_direction dir,unsigned long attrs) +{ +return iommu_dma_map_page (dev,phys_to_page (phys),offset_in_page (phys), + size,dma_direction_to_prot (dir, false) |IOMMU_MMIO ); +} + +void iommu_dma_unmap_resource (struct device *dev,dma_addr_t handle, +size_t size,enum dma_data_direction dir,unsigned long attrs) +{ +__iommu_dma_unmap (iommu_get_domain_for_dev (dev), handle); +} +int iommu_dma_supported (struct device *dev,u64 mask) {/*
include/linux/dma-iommu.h +4 -0
@@ -61,6 +61,10 @@ void iommu_dma_unmap_page(struct device *dev, dma_addr_t handle, size_t size, enum dma_data_direction dir, unsigned long attrs);void iommu_dma_unmap_sg (struct device *dev,struct scatterlist *sg,int nents,enum dma_data_direction dir,unsigned long attrs); +dma_addr_t iommu_dma_map_resource (struct device *dev,phys_addr_t phys, +size_t size,enum dma_data_direction dir,unsigned long attrs); +void iommu_dma_unmap_resource (struct device *dev,dma_addr_t handle, +size_t size,enum dma_data_direction dir,unsigned long attrs);int iommu_dma_supported (struct device *dev,u64 mask);int iommu_dma_mapping_error (struct device *dev,dma_addr_t dma_addr);
Signature · 1 line
1.9.1
Robin Murphy
maintainerPATCH 2/2 arm64: Wire up iommu_dma_{map, unmap}_resource()
With no coherency to worry about, just plug'em straight in.
--- arch/arm64/mm/dma-mapping.c | 2 ++ 1 file changed, 2 insertions(+)
arch/arm64/mm/dma-mapping.c +2 -0
@@ -796,6 +796,8 @@ static void __iommu_unmap_sg_attrs(struct device *dev, .sync_single_for_device = __iommu_sync_single_for_device, .sync_sg_for_cpu = __iommu_sync_sg_for_cpu, .sync_sg_for_device = __iommu_sync_sg_for_device, + .map_resource = iommu_dma_map_resource, + .unmap_resource = iommu_dma_unmap_resource, .dma_supported = iommu_dma_supported, .mapping_error = iommu_dma_mapping_error, };
Signature · 1 line
1.9.1
PATCH 1/2 iommu/dma: Implement dma_{map,unmap}_resource()
In reply to Robin MurphyQuoted 6 lines · Robin Murphy
With the new dma_{map,unmap}_resource() functions added to the DMA API for the benefit of cases like slave DMA, add suitable implementations to the arsenal of our generic layer. Since cache maintenance should not be a concern, these can both be standalone versions without the need for architecture-specific wrappers.
Quoted 7 lines
CC Joerg Roedel <[email protected]>Signed-off-by Robin Murphy <[email protected]>---
Since patch 2 has a build dependency on this one, they should probably go together through either the arm64 tree or the iommu tree, but I can't make up my mind which one seems more appropriate...
I can take it via the smmu tree, if you like. However, comment below.
Quoted 14 lines
drivers/iommu/dma-iommu.c | 13 +++++++++++++ include/linux/dma-iommu.h | 4 ++++ 2 files changed, 17 insertions(+)drivers/iommu/dma-iommu.c +2
-0@@ -624,6 +624,19 @@ void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,__iommu_dma_unmap (iommu_get_domain_for_dev (dev),sg_dma_address (sg)); } +dma_addr_t iommu_dma_map_resource (struct device *dev,phys_addr_t phys, +size_t size,enum dma_data_direction dir,unsigned long attrs)
drivers/iommu/dma-iommu.c void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents, · line 638
Quoted 10 lines
drivers/iommu/dma-iommu.c +9
-0+{ +return iommu_dma_map_page (dev,phys_to_page (phys),offset_in_page (phys), + size,dma_direction_to_prot (dir, false) |IOMMU_MMIO ); +} +void iommu_dma_unmap_resource (struct device *dev,dma_addr_t handle, +size_t size,enum dma_data_direction dir,unsigned long attrs) +{ +__iommu_dma_unmap (iommu_get_domain_for_dev (dev), handle); +}
I think it's better to call iommu_dma_unmap_page instead. That said, are you sure it's safe to ignore the "size" parameter here? Is it permitted to unmap part of a region? If not, why does that parameter exist?
Will
Robin Murphy
maintainerPATCH 1/2 iommu/dma: Implement dma_{map,unmap}_resource()
Quoted 11 lines · Will Deacon
Quoted 10 lines · Robin Murphy
With the new dma_{map,unmap}_resource() functions added to the DMA API for the benefit of cases like slave DMA, add suitable implementations to the arsenal of our generic layer. Since cache maintenance should not be a concern, these can both be standalone versions without the need for architecture-specific wrappers.
CC Joerg Roedel <[email protected]>Signed-off-by Robin Murphy <[email protected]>---
Quoted 5 lines
Quoted 3 lines
Since patch 2 has a build dependency on this one, they should probably go together through either the arm64 tree or the iommu tree, but I can't make up my mind which one seems more appropriate...
I can take it via the smmu tree, if you like. However, comment below.
I'm surprised that didn't occur to me - makes sense, thanks.
Quoted 19 lines
Quoted 19 lines
drivers/iommu/dma-iommu.c | 13 +++++++++++++ include/linux/dma-iommu.h | 4 ++++ 2 files changed, 17 insertions(+)drivers/iommu/dma-iommu.c +6
-0@@ -624,6 +624,19 @@ void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,__iommu_dma_unmap (iommu_get_domain_for_dev (dev),sg_dma_address (sg)); } +dma_addr_t iommu_dma_map_resource (struct device *dev,phys_addr_t phys, +size_t size,enum dma_data_direction dir,unsigned long attrs) +{ +return iommu_dma_map_page (dev,phys_to_page (phys),offset_in_page (phys), + size,dma_direction_to_prot (dir, false) |IOMMU_MMIO ); +}
Quoted 9 lines
drivers/iommu/dma-iommu.c void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents, · line 638
Quoted 5 lines
drivers/iommu/dma-iommu.c +5
-0+void iommu_dma_unmap_resource (struct device *dev,dma_addr_t handle, +size_t size,enum dma_data_direction dir,unsigned long attrs) +{ +__iommu_dma_unmap (iommu_get_domain_for_dev (dev), handle); +}I think it's better to call
iommu_dma_unmap_pageinstead. That said, are you sure it's safe to ignore the "size" parameter here? Is it permitted to unmap part of a region? If not, why does that parameter exist?
Actually, "#define iommu_dma_unmap_resource iommu_dma_unmap_page" in the header would make life even simpler. Or is that too evil?
Glossing over the size parameter is a detail of this particular implementation. Equivalently to iommu_dma_unmap_page(), it's there (along with dir and attrs) to match the signature of dma_unmap_resource(), so that arch code doesn't need yet another wrapper for its .unmap_resource callback in dma_ops (see patch 2). As it happens, ignoring the size is effectively extra-safe in the sense that we're enforcing the API and not even trusting the caller - since we have to look up the iova to free it, and that iova records the size originally mapped, we simply unmap that original size because it makes for simpler code and can't be wrong. See the __iommu_dma_unmap() implementation itself.
Robin.
Quoted 3 lines
Will