From: Robin Murphy <robin.murphy@arm.com>
To: Laurentiu Tudor <laurentiu.tudor@nxp.com>,
"hch@lst.de" <hch@lst.de>, "joro@8bytes.org" <joro@8bytes.org>,
Ioana Ciocoi Radulescu <ruxandra.radulescu@nxp.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"iommu@lists.linux-foundation.org"
<iommu@lists.linux-foundation.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
Ioana Ciornei <ioana.ciornei@nxp.com>
Cc: Madalin Bucur <madalin.bucur@nxp.com>,
"davem@davemloft.net" <davem@davemloft.net>,
Leo Li <leoyang.li@nxp.com>
Subject: Re: [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants
Date: Mon, 28 Oct 2019 13:42:43 +0000 [thread overview]
Message-ID: <ffcdb378-85df-f662-4961-49ff280f39d9@arm.com> (raw)
In-Reply-To: <20191024124130.16871-2-laurentiu.tudor@nxp.com>
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)
>
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2019-10-28 13:42 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ffcdb378-85df-f662-4961-49ff280f39d9@arm.com \
--to=robin.murphy@arm.com \
--cc=davem@davemloft.net \
--cc=hch@lst.de \
--cc=ioana.ciornei@nxp.com \
--cc=iommu@lists.linux-foundation.org \
--cc=joro@8bytes.org \
--cc=laurentiu.tudor@nxp.com \
--cc=leoyang.li@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=madalin.bucur@nxp.com \
--cc=netdev@vger.kernel.org \
--cc=ruxandra.radulescu@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).