* [PATCH v2 0/3] dma-mapping: introduce new dma unmap and sync variants @ 2019-10-24 12:41 Laurentiu Tudor 2019-10-24 12:41 ` [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants Laurentiu Tudor ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Laurentiu Tudor @ 2019-10-24 12:41 UTC (permalink / raw) To: hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei Cc: Leo Li, robin.murphy, Diana Madalina Craciun, davem, Madalin Bucur, Laurentiu Tudor From: Laurentiu Tudor <laurentiu.tudor@nxp.com> This series introduces a few new dma unmap and sync api variants that, on top of what the originals do, return the virtual address corresponding to the input dma address. In order to do that a new dma map op is added, .get_virt_addr that takes the input dma address and returns the virtual address backing it up. The second patch adds an implementation for this new dma map op in the generic iommu dma glue code and wires it in. The third patch updates the dpaa2-eth driver to use the new apis. Context: https://lkml.org/lkml/2019/5/31/684 Changes in v2 * use "dma_unmap_*_desc" names (Robin) * return va/page instead of phys addr (Robin) Changes since RFC * completely changed the approach: added unmap and sync variants that return the phys addr instead of adding a new dma to phys conversion function Laurentiu Tudor (3): dma-mapping: introduce new dma unmap and sync api variants iommu/dma: wire-up new dma map op .get_virt_addr dpaa2_eth: use new unmap and sync dma api variants drivers/iommu/dma-iommu.c | 16 ++++++ .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 40 +++++--------- .../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 1 - include/linux/dma-mapping.h | 55 +++++++++++++++++++ 4 files changed, 86 insertions(+), 26 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants 2019-10-24 12:41 [PATCH v2 0/3] dma-mapping: introduce new dma unmap and sync variants Laurentiu Tudor @ 2019-10-24 12:41 ` Laurentiu Tudor 2019-10-28 12:38 ` hch ` (2 more replies) 2019-10-24 12:41 ` [PATCH v2 2/3] iommu/dma: wire-up new dma map op .get_virt_addr Laurentiu Tudor 2019-10-24 12:41 ` [PATCH v2 3/3] dpaa2_eth: use new unmap and sync dma api variants Laurentiu Tudor 2 siblings, 3 replies; 15+ messages in thread From: Laurentiu Tudor @ 2019-10-24 12:41 UTC (permalink / raw) To: hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei Cc: Leo Li, robin.murphy, Diana Madalina Craciun, davem, Madalin Bucur, Laurentiu Tudor From: Laurentiu Tudor <laurentiu.tudor@nxp.com> Introduce a few new dma unmap and sync variants that, on top of the original variants, return the virtual address corresponding to the input dma address. In order to implement this a new dma map op is added and used: void *get_virt_addr(dev, dma_handle); It does the actual conversion of an input dma address to the output virtual address. Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com> --- include/linux/dma-mapping.h | 55 +++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index 4a1c4fca475a..ae7bb8a84b9d 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -132,6 +132,7 @@ struct dma_map_ops { u64 (*get_required_mask)(struct device *dev); size_t (*max_mapping_size)(struct device *dev); unsigned long (*get_merge_boundary)(struct device *dev); + void *(*get_virt_addr)(struct device *dev, dma_addr_t dma_handle); }; #define DMA_MAPPING_ERROR (~(dma_addr_t)0) @@ -304,6 +305,21 @@ static inline void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, debug_dma_unmap_page(dev, addr, size, dir); } +static inline struct page * +dma_unmap_page_attrs_desc(struct device *dev, dma_addr_t addr, size_t size, + enum dma_data_direction dir, unsigned long attrs) +{ + const struct dma_map_ops *ops = get_dma_ops(dev); + void *ptr = NULL; + + if (ops && ops->get_virt_addr) + ptr = ops->get_virt_addr(dev, addr); + + dma_unmap_page_attrs(dev, addr, size, dir, attrs); + + return ptr ? virt_to_page(ptr) : NULL; +} + /* * dma_maps_sg_attrs returns 0 on error and > 0 on success. * It should never return a value < 0. @@ -390,6 +406,21 @@ static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr, debug_dma_sync_single_for_cpu(dev, addr, size, dir); } +static inline void * +dma_sync_single_for_cpu_desc(struct device *dev, dma_addr_t addr, size_t size, + enum dma_data_direction dir) +{ + const struct dma_map_ops *ops = get_dma_ops(dev); + void *ptr = NULL; + + if (ops && ops->get_virt_addr) + ptr = ops->get_virt_addr(dev, addr); + + dma_sync_single_for_cpu(dev, addr, size, dir); + + return ptr; +} + static inline void dma_sync_single_for_device(struct device *dev, dma_addr_t addr, size_t size, enum dma_data_direction dir) @@ -500,6 +531,12 @@ static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr, size_t size, enum dma_data_direction dir) { } + +static inline void * +dma_sync_single_for_cpu_desc(struct device *dev, dma_addr_t addr, size_t size, + enum dma_data_direction dir) +{ +} static inline void dma_sync_single_for_device(struct device *dev, dma_addr_t addr, size_t size, enum dma_data_direction dir) { @@ -594,6 +631,21 @@ static inline void dma_unmap_single_attrs(struct device *dev, dma_addr_t addr, return dma_unmap_page_attrs(dev, addr, size, dir, attrs); } +static inline void * +dma_unmap_single_attrs_desc(struct device *dev, dma_addr_t addr, size_t size, + enum dma_data_direction dir, unsigned long attrs) +{ + const struct dma_map_ops *ops = get_dma_ops(dev); + void *ptr = NULL; + + if (ops && ops->get_virt_addr) + ptr = ops->get_virt_addr(dev, addr); + + dma_unmap_single_attrs(dev, addr, size, dir, attrs); + + return ptr; +} + static inline void dma_sync_single_range_for_cpu(struct device *dev, dma_addr_t addr, unsigned long offset, size_t size, enum dma_data_direction dir) @@ -610,10 +662,13 @@ static inline void dma_sync_single_range_for_device(struct device *dev, #define dma_map_single(d, a, s, r) dma_map_single_attrs(d, a, s, r, 0) #define dma_unmap_single(d, a, s, r) dma_unmap_single_attrs(d, a, s, r, 0) +#define dma_unmap_single_desc(d, a, s, r) \ + dma_unmap_single_attrs_desc(d, a, s, r, 0) #define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0) #define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0) #define dma_map_page(d, p, o, s, r) dma_map_page_attrs(d, p, o, s, r, 0) #define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, 0) +#define dma_unmap_page_desc(d, a, s, r) dma_unmap_page_attrs_desc(d, a, s, r, 0) #define dma_get_sgtable(d, t, v, h, s) dma_get_sgtable_attrs(d, t, v, h, s, 0) #define dma_mmap_coherent(d, v, c, h, s) dma_mmap_attrs(d, v, c, h, s, 0) -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants 2019-10-24 12:41 ` [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants Laurentiu Tudor @ 2019-10-28 12:38 ` hch 2019-10-29 7:05 ` Laurentiu Tudor 2019-10-28 13:42 ` Robin Murphy 2019-10-30 9:48 ` kbuild test robot 2 siblings, 1 reply; 15+ messages in thread From: hch @ 2019-10-28 12:38 UTC (permalink / raw) To: Laurentiu Tudor Cc: hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei, Leo Li, robin.murphy, Diana Madalina Craciun, davem, Madalin Bucur On Thu, Oct 24, 2019 at 12:41:41PM +0000, Laurentiu Tudor wrote: > From: Laurentiu Tudor <laurentiu.tudor@nxp.com> > > Introduce a few new dma unmap and sync variants that, on top of the > original variants, return the virtual address corresponding to the > input dma address. > In order to implement this a new dma map op is added and used: > void *get_virt_addr(dev, dma_handle); > It does the actual conversion of an input dma address to the output > virtual address. We'll definitively need an implementation for dma-direct at least as well. Also as said previously we need a dma_can_unmap_by_dma_addr() or similar helper that tells the driver beforehand if this works, so that the driver can either use a sub-optimal workaround or fail the probe if this functionality isn't implemented. ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants 2019-10-28 12:38 ` hch @ 2019-10-29 7:05 ` Laurentiu Tudor 0 siblings, 0 replies; 15+ messages in thread From: Laurentiu Tudor @ 2019-10-29 7:05 UTC (permalink / raw) To: hch Cc: joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei, Leo Li, robin.murphy, Diana Madalina Craciun, davem, Madalin Bucur > -----Original Message----- > From: hch@lst.de <hch@lst.de> > Sent: Monday, October 28, 2019 2:38 PM > > On Thu, Oct 24, 2019 at 12:41:41PM +0000, Laurentiu Tudor wrote: > > From: Laurentiu Tudor <laurentiu.tudor@nxp.com> > > > > Introduce a few new dma unmap and sync variants that, on top of the > > original variants, return the virtual address corresponding to the > > input dma address. > > In order to implement this a new dma map op is added and used: > > void *get_virt_addr(dev, dma_handle); > > It does the actual conversion of an input dma address to the output > > virtual address. > > We'll definitively need an implementation for dma-direct at least as > well. Also as said previously we need a dma_can_unmap_by_dma_addr() > or similar helper that tells the driver beforehand if this works, so > that the driver can either use a sub-optimal workaround or fail the > probe if this functionality isn't implemented. Alright. On top of that I need to make this work on booke ppc as we have one driver that runs both on arm and ppc and will use these APIs. --- Best Regards, Laurentiu ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants 2019-10-24 12:41 ` [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants Laurentiu Tudor 2019-10-28 12:38 ` hch @ 2019-10-28 13:42 ` Robin Murphy 2019-11-06 11:32 ` Laurentiu Tudor 2019-11-07 12:30 ` Laurentiu Tudor 2019-10-30 9:48 ` kbuild test robot 2 siblings, 2 replies; 15+ messages in thread From: Robin Murphy @ 2019-10-28 13:42 UTC (permalink / raw) To: Laurentiu Tudor, hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei Cc: Leo Li, Diana Madalina Craciun, davem, Madalin Bucur On 24/10/2019 13:41, Laurentiu Tudor wrote: > From: Laurentiu Tudor <laurentiu.tudor@nxp.com> > > Introduce a few new dma unmap and sync variants that, on top of the > original variants, return the virtual address corresponding to the > input dma address. > In order to implement this a new dma map op is added and used: > void *get_virt_addr(dev, dma_handle); > It does the actual conversion of an input dma address to the output > virtual address. At this point, I think it might be better to just change the prototype of the .unmap_page/.sync_single_for_cpu callbacks themselves. In cases where .get_virt_addr would be non-trivial, it's most likely duplicating work that the relevant callback has to do anyway (i.e. where the virtual and/or physical address is needed internally for a cache maintenance or bounce buffer operation). It would also help avoid any possible ambiguity about whether .get_virt_addr returns the VA corresponding dma_handle (if one exists) rather than the VA of the buffer *mapped to* dma_handle, which for a bounce-buffering implementation would be different, and the one you actually need - a naive phys_to_virt(dma_to_phys(dma_handle)) would lead you to the wrong place (in fact it looks like DPAA2 would currently go wrong with "swiotlb=force" and the SMMU disabled or in passthrough). One question there is whether we'd want careful special-casing to avoid introducing overhead where unmap/sync are currently complete no-ops, or whether an extra phys_to_virt() or so in those paths would be tolerable. > Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com> > --- > include/linux/dma-mapping.h | 55 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h > index 4a1c4fca475a..ae7bb8a84b9d 100644 > --- a/include/linux/dma-mapping.h > +++ b/include/linux/dma-mapping.h > @@ -132,6 +132,7 @@ struct dma_map_ops { > u64 (*get_required_mask)(struct device *dev); > size_t (*max_mapping_size)(struct device *dev); > unsigned long (*get_merge_boundary)(struct device *dev); > + void *(*get_virt_addr)(struct device *dev, dma_addr_t dma_handle); > }; > > #define DMA_MAPPING_ERROR (~(dma_addr_t)0) > @@ -304,6 +305,21 @@ static inline void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, > debug_dma_unmap_page(dev, addr, size, dir); > } > > +static inline struct page * > +dma_unmap_page_attrs_desc(struct device *dev, dma_addr_t addr, size_t size, > + enum dma_data_direction dir, unsigned long attrs) > +{ > + const struct dma_map_ops *ops = get_dma_ops(dev); > + void *ptr = NULL; > + > + if (ops && ops->get_virt_addr) > + ptr = ops->get_virt_addr(dev, addr); Note that this doesn't work for dma-direct, but for the sake of arm64 at least it almost certainly wants to. Robin. > + dma_unmap_page_attrs(dev, addr, size, dir, attrs); > + > + return ptr ? virt_to_page(ptr) : NULL; > +} > + > /* > * dma_maps_sg_attrs returns 0 on error and > 0 on success. > * It should never return a value < 0. > @@ -390,6 +406,21 @@ static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr, > debug_dma_sync_single_for_cpu(dev, addr, size, dir); > } > > +static inline void * > +dma_sync_single_for_cpu_desc(struct device *dev, dma_addr_t addr, size_t size, > + enum dma_data_direction dir) > +{ > + const struct dma_map_ops *ops = get_dma_ops(dev); > + void *ptr = NULL; > + > + if (ops && ops->get_virt_addr) > + ptr = ops->get_virt_addr(dev, addr); > + > + dma_sync_single_for_cpu(dev, addr, size, dir); > + > + return ptr; > +} > + > static inline void dma_sync_single_for_device(struct device *dev, > dma_addr_t addr, size_t size, > enum dma_data_direction dir) > @@ -500,6 +531,12 @@ static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr, > size_t size, enum dma_data_direction dir) > { > } > + > +static inline void * > +dma_sync_single_for_cpu_desc(struct device *dev, dma_addr_t addr, size_t size, > + enum dma_data_direction dir) > +{ > +} > static inline void dma_sync_single_for_device(struct device *dev, > dma_addr_t addr, size_t size, enum dma_data_direction dir) > { > @@ -594,6 +631,21 @@ static inline void dma_unmap_single_attrs(struct device *dev, dma_addr_t addr, > return dma_unmap_page_attrs(dev, addr, size, dir, attrs); > } > > +static inline void * > +dma_unmap_single_attrs_desc(struct device *dev, dma_addr_t addr, size_t size, > + enum dma_data_direction dir, unsigned long attrs) > +{ > + const struct dma_map_ops *ops = get_dma_ops(dev); > + void *ptr = NULL; > + > + if (ops && ops->get_virt_addr) > + ptr = ops->get_virt_addr(dev, addr); > + > + dma_unmap_single_attrs(dev, addr, size, dir, attrs); > + > + return ptr; > +} > + > static inline void dma_sync_single_range_for_cpu(struct device *dev, > dma_addr_t addr, unsigned long offset, size_t size, > enum dma_data_direction dir) > @@ -610,10 +662,13 @@ static inline void dma_sync_single_range_for_device(struct device *dev, > > #define dma_map_single(d, a, s, r) dma_map_single_attrs(d, a, s, r, 0) > #define dma_unmap_single(d, a, s, r) dma_unmap_single_attrs(d, a, s, r, 0) > +#define dma_unmap_single_desc(d, a, s, r) \ > + dma_unmap_single_attrs_desc(d, a, s, r, 0) > #define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0) > #define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0) > #define dma_map_page(d, p, o, s, r) dma_map_page_attrs(d, p, o, s, r, 0) > #define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, 0) > +#define dma_unmap_page_desc(d, a, s, r) dma_unmap_page_attrs_desc(d, a, s, r, 0) > #define dma_get_sgtable(d, t, v, h, s) dma_get_sgtable_attrs(d, t, v, h, s, 0) > #define dma_mmap_coherent(d, v, c, h, s) dma_mmap_attrs(d, v, c, h, s, 0) > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants 2019-10-28 13:42 ` Robin Murphy @ 2019-11-06 11:32 ` Laurentiu Tudor 2019-11-07 12:30 ` Laurentiu Tudor 1 sibling, 0 replies; 15+ messages in thread From: Laurentiu Tudor @ 2019-11-06 11:32 UTC (permalink / raw) To: Robin Murphy, hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei Cc: Leo Li, Diana Madalina Craciun, davem, Madalin Bucur On 28.10.2019 15:42, Robin Murphy wrote: > On 24/10/2019 13:41, Laurentiu Tudor wrote: >> From: Laurentiu Tudor <laurentiu.tudor@nxp.com> >> >> Introduce a few new dma unmap and sync variants that, on top of the >> original variants, return the virtual address corresponding to the >> input dma address. >> In order to implement this a new dma map op is added and used: >> void *get_virt_addr(dev, dma_handle); >> It does the actual conversion of an input dma address to the output >> virtual address. > > At this point, I think it might be better to just change the prototype > of the .unmap_page/.sync_single_for_cpu callbacks themselves. I could give this a try. At a first sight, looks like it will be quite an intrusive change. > In cases > where .get_virt_addr would be non-trivial, it's most likely duplicating > work that the relevant callback has to do anyway (i.e. where the virtual > and/or physical address is needed internally for a cache maintenance or > bounce buffer operation). It would also help avoid any possible > ambiguity about whether .get_virt_addr returns the VA corresponding > dma_handle (if one exists) rather than the VA of the buffer *mapped to* > dma_handle, which for a bounce-buffering implementation would be > different, and the one you actually need - a naive > phys_to_virt(dma_to_phys(dma_handle)) would lead you to the wrong place > (in fact it looks like DPAA2 would currently go wrong with > "swiotlb=force" and the SMMU disabled or in passthrough). Yes, most likely. > One question there is whether we'd want careful special-casing to avoid > introducing overhead where unmap/sync are currently complete no-ops, or > whether an extra phys_to_virt() or so in those paths would be tolerable. > >> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com> >> --- >> include/linux/dma-mapping.h | 55 +++++++++++++++++++++++++++++++++++++ >> 1 file changed, 55 insertions(+) >> >> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h >> index 4a1c4fca475a..ae7bb8a84b9d 100644 >> --- a/include/linux/dma-mapping.h >> +++ b/include/linux/dma-mapping.h >> @@ -132,6 +132,7 @@ struct dma_map_ops { >> u64 (*get_required_mask)(struct device *dev); >> size_t (*max_mapping_size)(struct device *dev); >> unsigned long (*get_merge_boundary)(struct device *dev); >> + void *(*get_virt_addr)(struct device *dev, dma_addr_t dma_handle); >> }; >> #define DMA_MAPPING_ERROR (~(dma_addr_t)0) >> @@ -304,6 +305,21 @@ static inline void dma_unmap_page_attrs(struct >> device *dev, dma_addr_t addr, >> debug_dma_unmap_page(dev, addr, size, dir); >> } >> +static inline struct page * >> +dma_unmap_page_attrs_desc(struct device *dev, dma_addr_t addr, size_t >> size, >> + enum dma_data_direction dir, unsigned long attrs) >> +{ >> + const struct dma_map_ops *ops = get_dma_ops(dev); >> + void *ptr = NULL; >> + >> + if (ops && ops->get_virt_addr) >> + ptr = ops->get_virt_addr(dev, addr); > > Note that this doesn't work for dma-direct, but for the sake of arm64 at > least it almost certainly wants to. > Will take care of it. --- Best Regards, Laurentiu > >> + dma_unmap_page_attrs(dev, addr, size, dir, attrs); >> + >> + return ptr ? virt_to_page(ptr) : NULL; >> +} >> + >> /* >> * dma_maps_sg_attrs returns 0 on error and > 0 on success. >> * It should never return a value < 0. >> @@ -390,6 +406,21 @@ static inline void dma_sync_single_for_cpu(struct >> device *dev, dma_addr_t addr, >> debug_dma_sync_single_for_cpu(dev, addr, size, dir); >> } >> +static inline void * >> +dma_sync_single_for_cpu_desc(struct device *dev, dma_addr_t addr, >> size_t size, >> + enum dma_data_direction dir) >> +{ >> + const struct dma_map_ops *ops = get_dma_ops(dev); >> + void *ptr = NULL; >> + >> + if (ops && ops->get_virt_addr) >> + ptr = ops->get_virt_addr(dev, addr); >> + >> + dma_sync_single_for_cpu(dev, addr, size, dir); >> + >> + return ptr; >> +} >> + >> static inline void dma_sync_single_for_device(struct device *dev, >> dma_addr_t addr, size_t size, >> enum dma_data_direction dir) >> @@ -500,6 +531,12 @@ static inline void dma_sync_single_for_cpu(struct >> device *dev, dma_addr_t addr, >> size_t size, enum dma_data_direction dir) >> { >> } >> + >> +static inline void * >> +dma_sync_single_for_cpu_desc(struct device *dev, dma_addr_t addr, >> size_t size, >> + enum dma_data_direction dir) >> +{ >> +} >> static inline void dma_sync_single_for_device(struct device *dev, >> dma_addr_t addr, size_t size, enum dma_data_direction dir) >> { >> @@ -594,6 +631,21 @@ static inline void dma_unmap_single_attrs(struct >> device *dev, dma_addr_t addr, >> return dma_unmap_page_attrs(dev, addr, size, dir, attrs); >> } >> +static inline void * >> +dma_unmap_single_attrs_desc(struct device *dev, dma_addr_t addr, >> size_t size, >> + enum dma_data_direction dir, unsigned long attrs) >> +{ >> + const struct dma_map_ops *ops = get_dma_ops(dev); >> + void *ptr = NULL; >> + >> + if (ops && ops->get_virt_addr) >> + ptr = ops->get_virt_addr(dev, addr); >> + >> + dma_unmap_single_attrs(dev, addr, size, dir, attrs); >> + >> + return ptr; >> +} >> + >> static inline void dma_sync_single_range_for_cpu(struct device *dev, >> dma_addr_t addr, unsigned long offset, size_t size, >> enum dma_data_direction dir) >> @@ -610,10 +662,13 @@ static inline void >> dma_sync_single_range_for_device(struct device *dev, >> #define dma_map_single(d, a, s, r) dma_map_single_attrs(d, a, s, r, 0) >> #define dma_unmap_single(d, a, s, r) dma_unmap_single_attrs(d, a, s, >> r, 0) >> +#define dma_unmap_single_desc(d, a, s, r) \ >> + dma_unmap_single_attrs_desc(d, a, s, r, 0) >> #define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0) >> #define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0) >> #define dma_map_page(d, p, o, s, r) dma_map_page_attrs(d, p, o, s, >> r, 0) >> #define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, 0) >> +#define dma_unmap_page_desc(d, a, s, r) dma_unmap_page_attrs_desc(d, >> a, s, r, 0) >> #define dma_get_sgtable(d, t, v, h, s) dma_get_sgtable_attrs(d, t, >> v, h, s, 0) >> #define dma_mmap_coherent(d, v, c, h, s) dma_mmap_attrs(d, v, c, h, >> s, 0) >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants 2019-10-28 13:42 ` Robin Murphy 2019-11-06 11:32 ` Laurentiu Tudor @ 2019-11-07 12:30 ` Laurentiu Tudor 1 sibling, 0 replies; 15+ messages in thread From: Laurentiu Tudor @ 2019-11-07 12:30 UTC (permalink / raw) To: Robin Murphy, hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei Cc: Leo Li, Diana Madalina Craciun, davem, Madalin Bucur Hi Robin, On 28.10.2019 15:42, Robin Murphy wrote: > On 24/10/2019 13:41, Laurentiu Tudor wrote: >> From: Laurentiu Tudor <laurentiu.tudor@nxp.com> >> >> Introduce a few new dma unmap and sync variants that, on top of the >> original variants, return the virtual address corresponding to the >> input dma address. >> In order to implement this a new dma map op is added and used: >> void *get_virt_addr(dev, dma_handle); >> It does the actual conversion of an input dma address to the output >> virtual address. > > At this point, I think it might be better to just change the prototype > of the .unmap_page/.sync_single_for_cpu callbacks themselves. In cases > where .get_virt_addr would be non-trivial, it's most likely duplicating > work that the relevant callback has to do anyway (i.e. where the virtual > and/or physical address is needed internally for a cache maintenance or > bounce buffer operation). Looking in the generic dma-iommu, I didn't see any mean of freely getting the pa or va bqcking the iova so I can't think of a way of doing this without adding a call to iommu_iova_to_phys() somewhere in the unmap op implementation. Obviously, this would come with an overhead that will probably upset people. At the moment I can't think at an option other than the initial one, that is adding the .get_virt_addr op. Please let me know your opinions on this. --- Thanks & Best Regards, Laurentiu > It would also help avoid any possible > ambiguity about whether .get_virt_addr returns the VA corresponding > dma_handle (if one exists) rather than the VA of the buffer *mapped to* > dma_handle, which for a bounce-buffering implementation would be > different, and the one you actually need - a naive > phys_to_virt(dma_to_phys(dma_handle)) would lead you to the wrong place > (in fact it looks like DPAA2 would currently go wrong with > "swiotlb=force" and the SMMU disabled or in passthrough). > > One question there is whether we'd want careful special-casing to avoid > introducing overhead where unmap/sync are currently complete no-ops, or > whether an extra phys_to_virt() or so in those paths would be tolerable. > >> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com> >> --- >> include/linux/dma-mapping.h | 55 +++++++++++++++++++++++++++++++++++++ >> 1 file changed, 55 insertions(+) >> >> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h >> index 4a1c4fca475a..ae7bb8a84b9d 100644 >> --- a/include/linux/dma-mapping.h >> +++ b/include/linux/dma-mapping.h >> @@ -132,6 +132,7 @@ struct dma_map_ops { >> u64 (*get_required_mask)(struct device *dev); >> size_t (*max_mapping_size)(struct device *dev); >> unsigned long (*get_merge_boundary)(struct device *dev); >> + void *(*get_virt_addr)(struct device *dev, dma_addr_t dma_handle); >> }; >> #define DMA_MAPPING_ERROR (~(dma_addr_t)0) >> @@ -304,6 +305,21 @@ static inline void dma_unmap_page_attrs(struct >> device *dev, dma_addr_t addr, >> debug_dma_unmap_page(dev, addr, size, dir); >> } >> +static inline struct page * >> +dma_unmap_page_attrs_desc(struct device *dev, dma_addr_t addr, size_t >> size, >> + enum dma_data_direction dir, unsigned long attrs) >> +{ >> + const struct dma_map_ops *ops = get_dma_ops(dev); >> + void *ptr = NULL; >> + >> + if (ops && ops->get_virt_addr) >> + ptr = ops->get_virt_addr(dev, addr); > > Note that this doesn't work for dma-direct, but for the sake of arm64 at > least it almost certainly wants to. > > Robin. > >> + dma_unmap_page_attrs(dev, addr, size, dir, attrs); >> + >> + return ptr ? virt_to_page(ptr) : NULL; >> +} >> + >> /* >> * dma_maps_sg_attrs returns 0 on error and > 0 on success. >> * It should never return a value < 0. >> @@ -390,6 +406,21 @@ static inline void dma_sync_single_for_cpu(struct >> device *dev, dma_addr_t addr, >> debug_dma_sync_single_for_cpu(dev, addr, size, dir); >> } >> +static inline void * >> +dma_sync_single_for_cpu_desc(struct device *dev, dma_addr_t addr, >> size_t size, >> + enum dma_data_direction dir) >> +{ >> + const struct dma_map_ops *ops = get_dma_ops(dev); >> + void *ptr = NULL; >> + >> + if (ops && ops->get_virt_addr) >> + ptr = ops->get_virt_addr(dev, addr); >> + >> + dma_sync_single_for_cpu(dev, addr, size, dir); >> + >> + return ptr; >> +} >> + >> static inline void dma_sync_single_for_device(struct device *dev, >> dma_addr_t addr, size_t size, >> enum dma_data_direction dir) >> @@ -500,6 +531,12 @@ static inline void dma_sync_single_for_cpu(struct >> device *dev, dma_addr_t addr, >> size_t size, enum dma_data_direction dir) >> { >> } >> + >> +static inline void * >> +dma_sync_single_for_cpu_desc(struct device *dev, dma_addr_t addr, >> size_t size, >> + enum dma_data_direction dir) >> +{ >> +} >> static inline void dma_sync_single_for_device(struct device *dev, >> dma_addr_t addr, size_t size, enum dma_data_direction dir) >> { >> @@ -594,6 +631,21 @@ static inline void dma_unmap_single_attrs(struct >> device *dev, dma_addr_t addr, >> return dma_unmap_page_attrs(dev, addr, size, dir, attrs); >> } >> +static inline void * >> +dma_unmap_single_attrs_desc(struct device *dev, dma_addr_t addr, >> size_t size, >> + enum dma_data_direction dir, unsigned long attrs) >> +{ >> + const struct dma_map_ops *ops = get_dma_ops(dev); >> + void *ptr = NULL; >> + >> + if (ops && ops->get_virt_addr) >> + ptr = ops->get_virt_addr(dev, addr); >> + >> + dma_unmap_single_attrs(dev, addr, size, dir, attrs); >> + >> + return ptr; >> +} >> + >> static inline void dma_sync_single_range_for_cpu(struct device *dev, >> dma_addr_t addr, unsigned long offset, size_t size, >> enum dma_data_direction dir) >> @@ -610,10 +662,13 @@ static inline void >> dma_sync_single_range_for_device(struct device *dev, >> #define dma_map_single(d, a, s, r) dma_map_single_attrs(d, a, s, r, 0) >> #define dma_unmap_single(d, a, s, r) dma_unmap_single_attrs(d, a, s, >> r, 0) >> +#define dma_unmap_single_desc(d, a, s, r) \ >> + dma_unmap_single_attrs_desc(d, a, s, r, 0) >> #define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0) >> #define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0) >> #define dma_map_page(d, p, o, s, r) dma_map_page_attrs(d, p, o, s, >> r, 0) >> #define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, 0) >> +#define dma_unmap_page_desc(d, a, s, r) dma_unmap_page_attrs_desc(d, >> a, s, r, 0) >> #define dma_get_sgtable(d, t, v, h, s) dma_get_sgtable_attrs(d, t, >> v, h, s, 0) >> #define dma_mmap_coherent(d, v, c, h, s) dma_mmap_attrs(d, v, c, h, >> s, 0) >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants 2019-10-24 12:41 ` [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants Laurentiu Tudor 2019-10-28 12:38 ` hch 2019-10-28 13:42 ` Robin Murphy @ 2019-10-30 9:48 ` kbuild test robot 2 siblings, 0 replies; 15+ messages in thread From: kbuild test robot @ 2019-10-30 9:48 UTC (permalink / raw) To: Laurentiu Tudor Cc: kbuild-all, hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei, Leo Li, robin.murphy, Diana Madalina Craciun, davem, Madalin Bucur, Laurentiu Tudor [-- Attachment #1: Type: text/plain, Size: 6190 bytes --] Hi Laurentiu, I love your patch! Yet something to improve: [auto build test ERROR on net-next/master] [also build test ERROR on v5.4-rc5 next-20191029] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Laurentiu-Tudor/dma-mapping-introduce-new-dma-unmap-and-sync-variants/20191027-173418 base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 503a64635d5ef7351657c78ad77f8b5ff658d5fc config: um-x86_64_defconfig (attached as .config) compiler: gcc-7 (Debian 7.4.0-14) 7.4.0 reproduce: # save the attached .config to linux build tree make ARCH=um SUBARCH=x86_64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from include/linux/skbuff.h:31:0, from include/net/net_namespace.h:38, from include/linux/inet.h:42, from include/linux/sunrpc/msg_prot.h:204, from include/linux/sunrpc/auth.h:16, from include/linux/nfs_fs.h:31, from init/do_mounts.c:23: include/linux/dma-mapping.h: In function 'dma_sync_single_for_cpu_desc': include/linux/dma-mapping.h:539:1: warning: no return statement in function returning non-void [-Wreturn-type] } ^ include/linux/dma-mapping.h: In function 'dma_unmap_single_attrs_desc': >> include/linux/dma-mapping.h:638:34: error: implicit declaration of function 'get_dma_ops'; did you mean 'get_mm_rss'? [-Werror=implicit-function-declaration] const struct dma_map_ops *ops = get_dma_ops(dev); ^~~~~~~~~~~ get_mm_rss include/linux/dma-mapping.h:638:34: warning: initialization makes pointer from integer without a cast [-Wint-conversion] cc1: some warnings being treated as errors vim +638 include/linux/dma-mapping.h 534 535 static inline void * 536 dma_sync_single_for_cpu_desc(struct device *dev, dma_addr_t addr, size_t size, 537 enum dma_data_direction dir) 538 { > 539 } 540 static inline void dma_sync_single_for_device(struct device *dev, 541 dma_addr_t addr, size_t size, enum dma_data_direction dir) 542 { 543 } 544 static inline void dma_sync_sg_for_cpu(struct device *dev, 545 struct scatterlist *sg, int nelems, enum dma_data_direction dir) 546 { 547 } 548 static inline void dma_sync_sg_for_device(struct device *dev, 549 struct scatterlist *sg, int nelems, enum dma_data_direction dir) 550 { 551 } 552 static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr) 553 { 554 return -ENOMEM; 555 } 556 static inline void *dma_alloc_attrs(struct device *dev, size_t size, 557 dma_addr_t *dma_handle, gfp_t flag, unsigned long attrs) 558 { 559 return NULL; 560 } 561 static void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr, 562 dma_addr_t dma_handle, unsigned long attrs) 563 { 564 } 565 static inline void *dmam_alloc_attrs(struct device *dev, size_t size, 566 dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs) 567 { 568 return NULL; 569 } 570 static inline void dmam_free_coherent(struct device *dev, size_t size, 571 void *vaddr, dma_addr_t dma_handle) 572 { 573 } 574 static inline void dma_cache_sync(struct device *dev, void *vaddr, size_t size, 575 enum dma_data_direction dir) 576 { 577 } 578 static inline int dma_get_sgtable_attrs(struct device *dev, 579 struct sg_table *sgt, void *cpu_addr, dma_addr_t dma_addr, 580 size_t size, unsigned long attrs) 581 { 582 return -ENXIO; 583 } 584 static inline int dma_mmap_attrs(struct device *dev, struct vm_area_struct *vma, 585 void *cpu_addr, dma_addr_t dma_addr, size_t size, 586 unsigned long attrs) 587 { 588 return -ENXIO; 589 } 590 static inline bool dma_can_mmap(struct device *dev) 591 { 592 return false; 593 } 594 static inline int dma_supported(struct device *dev, u64 mask) 595 { 596 return 0; 597 } 598 static inline int dma_set_mask(struct device *dev, u64 mask) 599 { 600 return -EIO; 601 } 602 static inline int dma_set_coherent_mask(struct device *dev, u64 mask) 603 { 604 return -EIO; 605 } 606 static inline u64 dma_get_required_mask(struct device *dev) 607 { 608 return 0; 609 } 610 static inline size_t dma_max_mapping_size(struct device *dev) 611 { 612 return 0; 613 } 614 static inline unsigned long dma_get_merge_boundary(struct device *dev) 615 { 616 return 0; 617 } 618 #endif /* CONFIG_HAS_DMA */ 619 620 static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr, 621 size_t size, enum dma_data_direction dir, unsigned long attrs) 622 { 623 debug_dma_map_single(dev, ptr, size); 624 return dma_map_page_attrs(dev, virt_to_page(ptr), offset_in_page(ptr), 625 size, dir, attrs); 626 } 627 628 static inline void dma_unmap_single_attrs(struct device *dev, dma_addr_t addr, 629 size_t size, enum dma_data_direction dir, unsigned long attrs) 630 { 631 return dma_unmap_page_attrs(dev, addr, size, dir, attrs); 632 } 633 634 static inline void * 635 dma_unmap_single_attrs_desc(struct device *dev, dma_addr_t addr, size_t size, 636 enum dma_data_direction dir, unsigned long attrs) 637 { > 638 const struct dma_map_ops *ops = get_dma_ops(dev); 639 void *ptr = NULL; 640 641 if (ops && ops->get_virt_addr) 642 ptr = ops->get_virt_addr(dev, addr); 643 644 dma_unmap_single_attrs(dev, addr, size, dir, attrs); 645 646 return ptr; 647 } 648 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 8323 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/3] iommu/dma: wire-up new dma map op .get_virt_addr 2019-10-24 12:41 [PATCH v2 0/3] dma-mapping: introduce new dma unmap and sync variants Laurentiu Tudor 2019-10-24 12:41 ` [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants Laurentiu Tudor @ 2019-10-24 12:41 ` Laurentiu Tudor 2019-10-28 13:52 ` Robin Murphy 2019-10-24 12:41 ` [PATCH v2 3/3] dpaa2_eth: use new unmap and sync dma api variants Laurentiu Tudor 2 siblings, 1 reply; 15+ messages in thread From: Laurentiu Tudor @ 2019-10-24 12:41 UTC (permalink / raw) To: hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei Cc: Leo Li, robin.murphy, Diana Madalina Craciun, davem, Madalin Bucur, Laurentiu Tudor From: Laurentiu Tudor <laurentiu.tudor@nxp.com> Add an implementation of the newly introduced dma map op in the generic DMA IOMMU generic glue layer and wire it up. Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com> --- drivers/iommu/dma-iommu.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index f321279baf9e..15e76232d697 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -1091,6 +1091,21 @@ static unsigned long iommu_dma_get_merge_boundary(struct device *dev) return (1UL << __ffs(domain->pgsize_bitmap)) - 1; } +static void *iommu_dma_get_virt_addr(struct device *dev, dma_addr_t dma_handle) +{ + struct iommu_domain *domain = iommu_get_domain_for_dev(dev); + + if (domain) { + phys_addr_t phys; + + phys = iommu_iova_to_phys(domain, dma_handle); + if (phys) + return phys_to_virt(phys); + } + + return NULL; +} + static const struct dma_map_ops iommu_dma_ops = { .alloc = iommu_dma_alloc, .free = iommu_dma_free, @@ -1107,6 +1122,7 @@ static const struct dma_map_ops iommu_dma_ops = { .map_resource = iommu_dma_map_resource, .unmap_resource = iommu_dma_unmap_resource, .get_merge_boundary = iommu_dma_get_merge_boundary, + .get_virt_addr = iommu_dma_get_virt_addr, }; /* -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/3] iommu/dma: wire-up new dma map op .get_virt_addr 2019-10-24 12:41 ` [PATCH v2 2/3] iommu/dma: wire-up new dma map op .get_virt_addr Laurentiu Tudor @ 2019-10-28 13:52 ` Robin Murphy 0 siblings, 0 replies; 15+ messages in thread From: Robin Murphy @ 2019-10-28 13:52 UTC (permalink / raw) To: Laurentiu Tudor, hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei Cc: Leo Li, Diana Madalina Craciun, davem, Madalin Bucur On 24/10/2019 13:41, Laurentiu Tudor wrote: > From: Laurentiu Tudor <laurentiu.tudor@nxp.com> > > Add an implementation of the newly introduced dma map op in the > generic DMA IOMMU generic glue layer and wire it up. > > Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com> > --- > drivers/iommu/dma-iommu.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index f321279baf9e..15e76232d697 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -1091,6 +1091,21 @@ static unsigned long iommu_dma_get_merge_boundary(struct device *dev) > return (1UL << __ffs(domain->pgsize_bitmap)) - 1; > } > > +static void *iommu_dma_get_virt_addr(struct device *dev, dma_addr_t dma_handle) > +{ > + struct iommu_domain *domain = iommu_get_domain_for_dev(dev); Note that we'd generally use the iommu_get_dma_domain() fastpath... > + > + if (domain) { ...wherein we can also assume that the device having iommu_dma_ops assigned at all implies that a DMA ops domain is in place. Robin. > + phys_addr_t phys; > + > + phys = iommu_iova_to_phys(domain, dma_handle); > + if (phys) > + return phys_to_virt(phys); > + } > + > + return NULL; > +} > + > static const struct dma_map_ops iommu_dma_ops = { > .alloc = iommu_dma_alloc, > .free = iommu_dma_free, > @@ -1107,6 +1122,7 @@ static const struct dma_map_ops iommu_dma_ops = { > .map_resource = iommu_dma_map_resource, > .unmap_resource = iommu_dma_unmap_resource, > .get_merge_boundary = iommu_dma_get_merge_boundary, > + .get_virt_addr = iommu_dma_get_virt_addr, > }; > > /* > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] dpaa2_eth: use new unmap and sync dma api variants 2019-10-24 12:41 [PATCH v2 0/3] dma-mapping: introduce new dma unmap and sync variants Laurentiu Tudor 2019-10-24 12:41 ` [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants Laurentiu Tudor 2019-10-24 12:41 ` [PATCH v2 2/3] iommu/dma: wire-up new dma map op .get_virt_addr Laurentiu Tudor @ 2019-10-24 12:41 ` Laurentiu Tudor 2019-10-25 16:12 ` Jonathan Lemon 2 siblings, 1 reply; 15+ messages in thread From: Laurentiu Tudor @ 2019-10-24 12:41 UTC (permalink / raw) To: hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei Cc: Leo Li, robin.murphy, Diana Madalina Craciun, davem, Madalin Bucur, Laurentiu Tudor From: Laurentiu Tudor <laurentiu.tudor@nxp.com> Convert this driver to usage of the newly introduced dma unmap and sync DMA APIs. This will get rid of the unsupported direct usage of iommu_iova_to_phys() API. Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com> --- .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 40 +++++++------------ .../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 1 - 2 files changed, 15 insertions(+), 26 deletions(-) diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c index 19379bae0144..8c3391e6e598 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c @@ -29,16 +29,6 @@ MODULE_LICENSE("Dual BSD/GPL"); MODULE_AUTHOR("Freescale Semiconductor, Inc"); MODULE_DESCRIPTION("Freescale DPAA2 Ethernet Driver"); -static void *dpaa2_iova_to_virt(struct iommu_domain *domain, - dma_addr_t iova_addr) -{ - phys_addr_t phys_addr; - - phys_addr = domain ? iommu_iova_to_phys(domain, iova_addr) : iova_addr; - - return phys_to_virt(phys_addr); -} - static void validate_rx_csum(struct dpaa2_eth_priv *priv, u32 fd_status, struct sk_buff *skb) @@ -85,9 +75,10 @@ static void free_rx_fd(struct dpaa2_eth_priv *priv, sgt = vaddr + dpaa2_fd_get_offset(fd); for (i = 1; i < DPAA2_ETH_MAX_SG_ENTRIES; i++) { addr = dpaa2_sg_get_addr(&sgt[i]); - sg_vaddr = dpaa2_iova_to_virt(priv->iommu_domain, addr); - dma_unmap_page(dev, addr, DPAA2_ETH_RX_BUF_SIZE, - DMA_BIDIRECTIONAL); + sg_vaddr = page_to_virt + (dma_unmap_page_desc(dev, addr, + DPAA2_ETH_RX_BUF_SIZE, + DMA_BIDIRECTIONAL)); free_pages((unsigned long)sg_vaddr, 0); if (dpaa2_sg_is_final(&sgt[i])) @@ -143,9 +134,10 @@ static struct sk_buff *build_frag_skb(struct dpaa2_eth_priv *priv, /* Get the address and length from the S/G entry */ sg_addr = dpaa2_sg_get_addr(sge); - sg_vaddr = dpaa2_iova_to_virt(priv->iommu_domain, sg_addr); - dma_unmap_page(dev, sg_addr, DPAA2_ETH_RX_BUF_SIZE, - DMA_BIDIRECTIONAL); + sg_vaddr = page_to_virt + (dma_unmap_page_desc(dev, sg_addr, + DPAA2_ETH_RX_BUF_SIZE, + DMA_BIDIRECTIONAL)); sg_length = dpaa2_sg_get_len(sge); @@ -210,9 +202,9 @@ static void free_bufs(struct dpaa2_eth_priv *priv, u64 *buf_array, int count) int i; for (i = 0; i < count; i++) { - vaddr = dpaa2_iova_to_virt(priv->iommu_domain, buf_array[i]); - dma_unmap_page(dev, buf_array[i], DPAA2_ETH_RX_BUF_SIZE, - DMA_BIDIRECTIONAL); + vaddr = page_to_virt(dma_unmap_page_desc(dev, buf_array[i], + DPAA2_ETH_RX_BUF_SIZE, + DMA_BIDIRECTIONAL)); free_pages((unsigned long)vaddr, 0); } } @@ -369,9 +361,8 @@ static void dpaa2_eth_rx(struct dpaa2_eth_priv *priv, /* Tracing point */ trace_dpaa2_rx_fd(priv->net_dev, fd); - vaddr = dpaa2_iova_to_virt(priv->iommu_domain, addr); - dma_sync_single_for_cpu(dev, addr, DPAA2_ETH_RX_BUF_SIZE, - DMA_BIDIRECTIONAL); + vaddr = dma_sync_single_for_cpu_desc(dev, addr, DPAA2_ETH_RX_BUF_SIZE, + DMA_BIDIRECTIONAL); fas = dpaa2_get_fas(vaddr, false); prefetch(fas); @@ -682,7 +673,8 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv, u32 fd_len = dpaa2_fd_get_len(fd); fd_addr = dpaa2_fd_get_addr(fd); - buffer_start = dpaa2_iova_to_virt(priv->iommu_domain, fd_addr); + buffer_start = dma_sync_single_for_cpu_desc(dev, fd_addr, sizeof(*swa), + DMA_BIDIRECTIONAL); swa = (struct dpaa2_eth_swa *)buffer_start; if (fd_format == dpaa2_fd_single) { @@ -3448,8 +3440,6 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev) priv = netdev_priv(net_dev); priv->net_dev = net_dev; - priv->iommu_domain = iommu_get_domain_for_dev(dev); - /* Obtain a MC portal */ err = fsl_mc_portal_allocate(dpni_dev, FSL_MC_IO_ATOMIC_CONTEXT_PORTAL, &priv->mc_io); diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h index 8a0e65b3267f..4e5183617ebd 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h @@ -374,7 +374,6 @@ struct dpaa2_eth_priv { struct fsl_mc_device *dpbp_dev; u16 bpid; - struct iommu_domain *iommu_domain; bool tx_tstamp; /* Tx timestamping enabled */ bool rx_tstamp; /* Rx timestamping enabled */ -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] dpaa2_eth: use new unmap and sync dma api variants 2019-10-24 12:41 ` [PATCH v2 3/3] dpaa2_eth: use new unmap and sync dma api variants Laurentiu Tudor @ 2019-10-25 16:12 ` Jonathan Lemon 2019-10-28 10:55 ` Laurentiu Tudor 0 siblings, 1 reply; 15+ messages in thread From: Jonathan Lemon @ 2019-10-25 16:12 UTC (permalink / raw) To: Laurentiu Tudor Cc: hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei, Leo Li, robin.murphy, Diana Madalina Craciun, davem, Madalin Bucur On 24 Oct 2019, at 5:41, Laurentiu Tudor wrote: > From: Laurentiu Tudor <laurentiu.tudor@nxp.com> > > Convert this driver to usage of the newly introduced dma unmap and > sync DMA APIs. This will get rid of the unsupported direct usage of > iommu_iova_to_phys() API. > > Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com> > --- > .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 40 > +++++++------------ > .../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 1 - > 2 files changed, 15 insertions(+), 26 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c > b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c > index 19379bae0144..8c3391e6e598 100644 > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c > @@ -29,16 +29,6 @@ MODULE_LICENSE("Dual BSD/GPL"); > MODULE_AUTHOR("Freescale Semiconductor, Inc"); > MODULE_DESCRIPTION("Freescale DPAA2 Ethernet Driver"); > > -static void *dpaa2_iova_to_virt(struct iommu_domain *domain, > - dma_addr_t iova_addr) > -{ > - phys_addr_t phys_addr; > - > - phys_addr = domain ? iommu_iova_to_phys(domain, iova_addr) : > iova_addr; > - > - return phys_to_virt(phys_addr); > -} > - > static void validate_rx_csum(struct dpaa2_eth_priv *priv, > u32 fd_status, > struct sk_buff *skb) > @@ -85,9 +75,10 @@ static void free_rx_fd(struct dpaa2_eth_priv *priv, > sgt = vaddr + dpaa2_fd_get_offset(fd); > for (i = 1; i < DPAA2_ETH_MAX_SG_ENTRIES; i++) { > addr = dpaa2_sg_get_addr(&sgt[i]); > - sg_vaddr = dpaa2_iova_to_virt(priv->iommu_domain, addr); > - dma_unmap_page(dev, addr, DPAA2_ETH_RX_BUF_SIZE, > - DMA_BIDIRECTIONAL); > + sg_vaddr = page_to_virt > + (dma_unmap_page_desc(dev, addr, > + DPAA2_ETH_RX_BUF_SIZE, > + DMA_BIDIRECTIONAL)); This is doing virt -> page -> virt. Why not just have the new function return the VA corresponding to the addr, which would match the other functions? -- Jonathan > > free_pages((unsigned long)sg_vaddr, 0); > if (dpaa2_sg_is_final(&sgt[i])) > @@ -143,9 +134,10 @@ static struct sk_buff *build_frag_skb(struct > dpaa2_eth_priv *priv, > > /* Get the address and length from the S/G entry */ > sg_addr = dpaa2_sg_get_addr(sge); > - sg_vaddr = dpaa2_iova_to_virt(priv->iommu_domain, sg_addr); > - dma_unmap_page(dev, sg_addr, DPAA2_ETH_RX_BUF_SIZE, > - DMA_BIDIRECTIONAL); > + sg_vaddr = page_to_virt > + (dma_unmap_page_desc(dev, sg_addr, > + DPAA2_ETH_RX_BUF_SIZE, > + DMA_BIDIRECTIONAL)); > > sg_length = dpaa2_sg_get_len(sge); > > @@ -210,9 +202,9 @@ static void free_bufs(struct dpaa2_eth_priv *priv, > u64 *buf_array, int count) > int i; > > for (i = 0; i < count; i++) { > - vaddr = dpaa2_iova_to_virt(priv->iommu_domain, buf_array[i]); > - dma_unmap_page(dev, buf_array[i], DPAA2_ETH_RX_BUF_SIZE, > - DMA_BIDIRECTIONAL); > + vaddr = page_to_virt(dma_unmap_page_desc(dev, buf_array[i], > + DPAA2_ETH_RX_BUF_SIZE, > + DMA_BIDIRECTIONAL)); > free_pages((unsigned long)vaddr, 0); > } > } > @@ -369,9 +361,8 @@ static void dpaa2_eth_rx(struct dpaa2_eth_priv > *priv, > /* Tracing point */ > trace_dpaa2_rx_fd(priv->net_dev, fd); > > - vaddr = dpaa2_iova_to_virt(priv->iommu_domain, addr); > - dma_sync_single_for_cpu(dev, addr, DPAA2_ETH_RX_BUF_SIZE, > - DMA_BIDIRECTIONAL); > + vaddr = dma_sync_single_for_cpu_desc(dev, addr, > DPAA2_ETH_RX_BUF_SIZE, > + DMA_BIDIRECTIONAL); > > fas = dpaa2_get_fas(vaddr, false); > prefetch(fas); > @@ -682,7 +673,8 @@ static void free_tx_fd(const struct dpaa2_eth_priv > *priv, > u32 fd_len = dpaa2_fd_get_len(fd); > > fd_addr = dpaa2_fd_get_addr(fd); > - buffer_start = dpaa2_iova_to_virt(priv->iommu_domain, fd_addr); > + buffer_start = dma_sync_single_for_cpu_desc(dev, fd_addr, > sizeof(*swa), > + DMA_BIDIRECTIONAL); > swa = (struct dpaa2_eth_swa *)buffer_start; > > if (fd_format == dpaa2_fd_single) { > @@ -3448,8 +3440,6 @@ static int dpaa2_eth_probe(struct fsl_mc_device > *dpni_dev) > priv = netdev_priv(net_dev); > priv->net_dev = net_dev; > > - priv->iommu_domain = iommu_get_domain_for_dev(dev); > - > /* Obtain a MC portal */ > err = fsl_mc_portal_allocate(dpni_dev, > FSL_MC_IO_ATOMIC_CONTEXT_PORTAL, > &priv->mc_io); > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h > b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h > index 8a0e65b3267f..4e5183617ebd 100644 > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h > @@ -374,7 +374,6 @@ struct dpaa2_eth_priv { > > struct fsl_mc_device *dpbp_dev; > u16 bpid; > - struct iommu_domain *iommu_domain; > > bool tx_tstamp; /* Tx timestamping enabled */ > bool rx_tstamp; /* Rx timestamping enabled */ > -- > 2.17.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] dpaa2_eth: use new unmap and sync dma api variants 2019-10-25 16:12 ` Jonathan Lemon @ 2019-10-28 10:55 ` Laurentiu Tudor 2019-10-28 11:38 ` hch 0 siblings, 1 reply; 15+ messages in thread From: Laurentiu Tudor @ 2019-10-28 10:55 UTC (permalink / raw) To: Jonathan Lemon Cc: hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei, Leo Li, robin.murphy, Diana Madalina Craciun, davem, Madalin Bucur Hi Jonathan, On 25.10.2019 19:12, Jonathan Lemon wrote: > > > On 24 Oct 2019, at 5:41, Laurentiu Tudor wrote: > >> From: Laurentiu Tudor <laurentiu.tudor@nxp.com> >> >> Convert this driver to usage of the newly introduced dma unmap and >> sync DMA APIs. This will get rid of the unsupported direct usage of >> iommu_iova_to_phys() API. >> >> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com> >> --- >> .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 40 +++++++------------ >> .../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 1 - >> 2 files changed, 15 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c >> b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c >> index 19379bae0144..8c3391e6e598 100644 >> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c >> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c >> @@ -29,16 +29,6 @@ MODULE_LICENSE("Dual BSD/GPL"); >> MODULE_AUTHOR("Freescale Semiconductor, Inc"); >> MODULE_DESCRIPTION("Freescale DPAA2 Ethernet Driver"); >> >> -static void *dpaa2_iova_to_virt(struct iommu_domain *domain, >> - dma_addr_t iova_addr) >> -{ >> - phys_addr_t phys_addr; >> - >> - phys_addr = domain ? iommu_iova_to_phys(domain, iova_addr) : >> iova_addr; >> - >> - return phys_to_virt(phys_addr); >> -} >> - >> static void validate_rx_csum(struct dpaa2_eth_priv *priv, >> u32 fd_status, >> struct sk_buff *skb) >> @@ -85,9 +75,10 @@ static void free_rx_fd(struct dpaa2_eth_priv *priv, >> sgt = vaddr + dpaa2_fd_get_offset(fd); >> for (i = 1; i < DPAA2_ETH_MAX_SG_ENTRIES; i++) { >> addr = dpaa2_sg_get_addr(&sgt[i]); >> - sg_vaddr = dpaa2_iova_to_virt(priv->iommu_domain, addr); >> - dma_unmap_page(dev, addr, DPAA2_ETH_RX_BUF_SIZE, >> - DMA_BIDIRECTIONAL); >> + sg_vaddr = page_to_virt >> + (dma_unmap_page_desc(dev, addr, >> + DPAA2_ETH_RX_BUF_SIZE, >> + DMA_BIDIRECTIONAL)); > > This is doing virt -> page -> virt. Why not just have the new > function return the VA corresponding to the addr, which would > match the other functions? I'd really like that as it would get rid of the page_to_virt() calls but it will break the symmetry with the dma_map_page() API. I'll let the maintainers decide. --- Best Regards, Laurentiu ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] dpaa2_eth: use new unmap and sync dma api variants 2019-10-28 10:55 ` Laurentiu Tudor @ 2019-10-28 11:38 ` hch 2019-11-06 11:17 ` Laurentiu Tudor 0 siblings, 1 reply; 15+ messages in thread From: hch @ 2019-10-28 11:38 UTC (permalink / raw) To: Laurentiu Tudor Cc: Jonathan Lemon, hch, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei, Leo Li, robin.murphy, Diana Madalina Craciun, davem, Madalin Bucur On Mon, Oct 28, 2019 at 10:55:05AM +0000, Laurentiu Tudor wrote: > >> @@ -85,9 +75,10 @@ static void free_rx_fd(struct dpaa2_eth_priv *priv, > >> sgt = vaddr + dpaa2_fd_get_offset(fd); > >> for (i = 1; i < DPAA2_ETH_MAX_SG_ENTRIES; i++) { > >> addr = dpaa2_sg_get_addr(&sgt[i]); > >> - sg_vaddr = dpaa2_iova_to_virt(priv->iommu_domain, addr); > >> - dma_unmap_page(dev, addr, DPAA2_ETH_RX_BUF_SIZE, > >> - DMA_BIDIRECTIONAL); > >> + sg_vaddr = page_to_virt > >> + (dma_unmap_page_desc(dev, addr, > >> + DPAA2_ETH_RX_BUF_SIZE, > >> + DMA_BIDIRECTIONAL)); > > > > This is doing virt -> page -> virt. Why not just have the new > > function return the VA corresponding to the addr, which would > > match the other functions? > > I'd really like that as it would get rid of the page_to_virt() calls but > it will break the symmetry with the dma_map_page() API. I'll let the > maintainers decide. It would be symmetric with dma_map_single, though. Maybe we need both variants? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] dpaa2_eth: use new unmap and sync dma api variants 2019-10-28 11:38 ` hch @ 2019-11-06 11:17 ` Laurentiu Tudor 0 siblings, 0 replies; 15+ messages in thread From: Laurentiu Tudor @ 2019-11-06 11:17 UTC (permalink / raw) To: hch Cc: Jonathan Lemon, joro, Ioana Ciocoi Radulescu, linux-kernel, iommu, netdev, Ioana Ciornei, Leo Li, robin.murphy, Diana Madalina Craciun, davem, Madalin Bucur On 28.10.2019 13:38, hch@lst.de wrote: > On Mon, Oct 28, 2019 at 10:55:05AM +0000, Laurentiu Tudor wrote: >>>> @@ -85,9 +75,10 @@ static void free_rx_fd(struct dpaa2_eth_priv *priv, >>>> sgt = vaddr + dpaa2_fd_get_offset(fd); >>>> for (i = 1; i < DPAA2_ETH_MAX_SG_ENTRIES; i++) { >>>> addr = dpaa2_sg_get_addr(&sgt[i]); >>>> - sg_vaddr = dpaa2_iova_to_virt(priv->iommu_domain, addr); >>>> - dma_unmap_page(dev, addr, DPAA2_ETH_RX_BUF_SIZE, >>>> - DMA_BIDIRECTIONAL); >>>> + sg_vaddr = page_to_virt >>>> + (dma_unmap_page_desc(dev, addr, >>>> + DPAA2_ETH_RX_BUF_SIZE, >>>> + DMA_BIDIRECTIONAL)); >>> >>> This is doing virt -> page -> virt. Why not just have the new >>> function return the VA corresponding to the addr, which would >>> match the other functions? >> >> I'd really like that as it would get rid of the page_to_virt() calls but >> it will break the symmetry with the dma_map_page() API. I'll let the >> maintainers decide. > > It would be symmetric with dma_map_single, though. Maybe we need > both variants? Patch 1/3 also adds an dma_unmap_single_desc(). Would it be legal to just use dma_unmap_single_desc() in the driver even if the driver does it's mappings with dma_map_page()? --- Best Regards, Laurentiu ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-11-07 12:30 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-24 12:41 [PATCH v2 0/3] dma-mapping: introduce new dma unmap and sync variants Laurentiu Tudor 2019-10-24 12:41 ` [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants Laurentiu Tudor 2019-10-28 12:38 ` hch 2019-10-29 7:05 ` Laurentiu Tudor 2019-10-28 13:42 ` Robin Murphy 2019-11-06 11:32 ` Laurentiu Tudor 2019-11-07 12:30 ` Laurentiu Tudor 2019-10-30 9:48 ` kbuild test robot 2019-10-24 12:41 ` [PATCH v2 2/3] iommu/dma: wire-up new dma map op .get_virt_addr Laurentiu Tudor 2019-10-28 13:52 ` Robin Murphy 2019-10-24 12:41 ` [PATCH v2 3/3] dpaa2_eth: use new unmap and sync dma api variants Laurentiu Tudor 2019-10-25 16:12 ` Jonathan Lemon 2019-10-28 10:55 ` Laurentiu Tudor 2019-10-28 11:38 ` hch 2019-11-06 11:17 ` Laurentiu Tudor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).