From: Robin Murphy <robin.murphy@arm.com>
To: Marek Szyprowski <m.szyprowski@samsung.com>,
dri-devel@lists.freedesktop.org,
iommu@lists.linux-foundation.org, linaro-mm-sig@lists.linaro.org,
linux-kernel@vger.kernel.org
Cc: David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
Christoph Hellwig <hch@lst.de>,
linux-arm-kernel@lists.infradead.org,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects
Date: Wed, 13 May 2020 14:23:10 +0100 [thread overview]
Message-ID: <400501ec-c56b-edb7-7def-36ad43264123@arm.com> (raw)
In-Reply-To: <20200512090058.14910-1-m.szyprowski@samsung.com>
On 2020-05-12 10:00 am, Marek Szyprowski wrote:
> struct sg_table is a common structure used for describing a memory
> buffer. It consists of a scatterlist with memory pages and DMA addresses
> (sgl entry), as well as the number of scatterlist entries: CPU pages
> (orig_nents entry) and DMA mapped pages (nents entry).
>
> It turned out that it was a common mistake to misuse nents and orig_nents
> entries, calling DMA-mapping functions with a wrong number of entries or
> ignoring the number of mapped entries returned by the dma_map_sg
> function.
>
> To avoid such issues, lets introduce a common wrappers operating directly
Nit: "let's"
> on the struct sg_table objects, which take care of the proper use of
> the nents and orig_nents entries.
A few more documentation nitpicks below, but either way the
implementation itself (modulo Christoph's fixup) looks good;
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> For more information, see '[PATCH v4 00/38] DRM: fix struct sg_table nents
> vs. orig_nents misuse' thread:
> https://lore.kernel.org/dri-devel/20200512085710.14688-1-m.szyprowski@samsung.com/T/
> ---
> include/linux/dma-mapping.h | 79 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index b43116a..88f01cc 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -609,6 +609,85 @@ static inline void dma_sync_single_range_for_device(struct device *dev,
> return dma_sync_single_for_device(dev, addr + offset, size, dir);
> }
>
> +/**
> + * dma_map_sgtable - Map the given buffer for the DMA operations
Either "for DMA operations", "for the DMA operation", or "for a DMA
operation", depending on the exact context. Or at that point, perhaps
just "for DMA".
> + * @dev: The device to perform a DMA operation
That doesn't quite parse, maybe "the device performing the DMA
operation", or "the device for which to perform the DMA operation",
depending on whether "DMA operation" means the mapping or the actual
hardware access?
> + * @sgt: The sg_table object describing the buffer
> + * @dir: DMA direction
> + * @attrs: Optional DMA attributes for the map operation
> + *
> + * Maps a buffer described by a scatterlist stored in the given sg_table
> + * object for the @dir DMA operation by the @dev device. After success
> + * the ownership for the buffer is transferred to the DMA domain. One has
> + * to call dma_sync_sgtable_for_cpu() or dma_unmap_sgtable() to move the
> + * ownership of the buffer back to the CPU domain before touching the
> + * buffer by the CPU.
> + * Returns 0 on success or -EINVAL on error during mapping the buffer.
Maybe make that a proper "Return:" section?
> + */
> +static inline int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
> + enum dma_data_direction dir, unsigned long attrs)
> +{
> + int n = dma_map_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
> +
> + if (n > 0) {
> + sgt->nents = n;
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> +/**
> + * dma_unmap_sgtable - Unmap the given buffer for the DMA operations
> + * @dev: The device to perform a DMA operation
Same two points as before.
> + * @sgt: The sg_table object describing the buffer
> + * @dir: DMA direction
> + * @attrs: Optional DMA attributes for the map operation
Presumably "the unmap operation", although it *is* true that some
attributes are expected to match those originally passed to
dma_map_sgtable()... not sure if kerneldoc can can stretch to that level
of detail concisely ;)
> + *
> + * Unmaps a buffer described by a scatterlist stored in the given sg_table
> + * object for the @dir DMA operation by the @dev device. After this function
> + * the ownership of the buffer is transferred back to the CPU domain.
> + */
> +static inline void dma_unmap_sgtable(struct device *dev, struct sg_table *sgt,
> + enum dma_data_direction dir, unsigned long attrs)
> +{
> + dma_unmap_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
> +}
> +
> +/**
> + * dma_sync_sgtable_for_cpu - Synchronize the given buffer for the CPU access
s/the CPU/CPU/
> + * @dev: The device to perform a DMA operation
As before.
> + * @sgt: The sg_table object describing the buffer
> + * @dir: DMA direction
> + *
> + * Performs the needed cache synchronization and moves the ownership of the
> + * buffer back to the CPU domain, so it is safe to perform any access to it
> + * by the CPU. Before doing any further DMA operations, one has to transfer
> + * the ownership of the buffer back to the DMA domain by calling the
> + * dma_sync_sgtable_for_device().
> + */
> +static inline void dma_sync_sgtable_for_cpu(struct device *dev,
> + struct sg_table *sgt, enum dma_data_direction dir)
> +{
> + dma_sync_sg_for_cpu(dev, sgt->sgl, sgt->orig_nents, dir);
> +}
> +
> +/**
> + * dma_sync_sgtable_for_device - Synchronize the given buffer for the DMA
That one doesn't even
> + * @dev: The device to perform a DMA operation
As before.
But of course, many thanks for taking the effort to add such complete
documentation in the first place :)
Cheers,
Robin.
> + * @sgt: The sg_table object describing the buffer
> + * @dir: DMA direction
> + *
> + * Performs the needed cache synchronization and moves the ownership of the
> + * buffer back to the DMA domain, so it is safe to perform the DMA operation.
> + * Once finished, one has to call dma_sync_sgtable_for_cpu() or
> + * dma_unmap_sgtable().
> + */
> +static inline void dma_sync_sgtable_for_device(struct device *dev,
> + struct sg_table *sgt, enum dma_data_direction dir)
> +{
> + dma_sync_sg_for_device(dev, sgt->sgl, sgt->orig_nents, dir);
> +}
> +
> #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_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-05-13 13:23 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20200512085722eucas1p2fbaab30e49c9ddadc64b27db856e5921@eucas1p2.samsung.com>
2020-05-12 8:57 ` [PATCH v4 00/38] DRM: fix struct sg_table nents vs. orig_nents misuse Marek Szyprowski
[not found] ` <CGME20200512090107eucas1p13a38ce5ce4c15cd0033acaea7b26c9b0@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Marek Szyprowski
[not found] ` <CGME20200512090108eucas1p10a3571be3f60265daea3b3f1469b5e82@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 02/38] scatterlist: add generic wrappers for iterating over " Marek Szyprowski
2020-05-12 12:18 ` Christoph Hellwig
2020-05-13 13:24 ` Robin Murphy
[not found] ` <CGME20200512090108eucas1p2168167ab5e1de09df0d5def83f64dbfe@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 03/38] iommu: add generic helper for mapping " Marek Szyprowski
2020-05-12 12:18 ` Christoph Hellwig
2020-05-13 9:03 ` Joerg Roedel
2020-05-13 13:27 ` Robin Murphy
[not found] ` <CGME20200512090109eucas1p285ca10dceb29f43aae1c40814e2dec8d@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 04/38] drm: prime: add common helper to check scatterlist contiguity Marek Szyprowski
[not found] ` <CGME20200512090110eucas1p1fdf69509f401e425c45e958430a99b65@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 05/38] drm: prime: use sgtable iterators in drm_prime_sg_to_page_addr_arrays() Marek Szyprowski
[not found] ` <CGME20200512090110eucas1p1b8de7671df480c071718c96f8ebdbc42@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 06/38] drm: core: fix common struct sg_table related issues Marek Szyprowski
[not found] ` <CGME20200512090111eucas1p2fd703addaa7975c16a1ea2d7807cc6a6@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 07/38] drm: amdgpu: " Marek Szyprowski
[not found] ` <CGME20200512090111eucas1p29897737c262b467437d0775204129105@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 08/38] drm: armada: " Marek Szyprowski
[not found] ` <CGME20200512090112eucas1p225de9f54f7fd54346043fc8c31e7ea2d@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 09/38] drm: etnaviv: " Marek Szyprowski
[not found] ` <CGME20200512090112eucas1p280707473d14730b8d3054fe9b0781a05@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 10/38] drm: exynos: use common helper for a scatterlist contiguity check Marek Szyprowski
[not found] ` <CGME20200512090113eucas1p254a8b23dd0ee63411df200f66d193203@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 11/38] drm: exynos: fix common struct sg_table related issues Marek Szyprowski
[not found] ` <CGME20200512090114eucas1p1917c16e0312cfb191f327e6dad2f7808@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 12/38] drm: i915: " Marek Szyprowski
[not found] ` <CGME20200512090114eucas1p1bc4ab112b490205283e7d2f82a9713ee@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 13/38] drm: lima: " Marek Szyprowski
[not found] ` <CGME20200512090115eucas1p25e71b29fa935e53e4c04f9b3789a09fc@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 14/38] drm: mediatek: use common helper for a scatterlist contiguity check Marek Szyprowski
[not found] ` <CGME20200512090116eucas1p2089d6eb7aa6bad4d2cbc2875c175873f@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 15/38] drm: mediatek: use common helper for extracting pages array Marek Szyprowski
[not found] ` <CGME20200512090116eucas1p24662e01574c0700cfe6d474280bb8df5@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 16/38] drm: msm: fix common struct sg_table related issues Marek Szyprowski
[not found] ` <CGME20200512090117eucas1p1acf4ddfe65242d28eee247ab2ca21454@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 17/38] drm: omapdrm: use common helper for extracting pages array Marek Szyprowski
[not found] ` <CGME20200512090117eucas1p1179ea62b61b45fae70630e66e434ffb3@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 18/38] drm: omapdrm: fix common struct sg_table related issues Marek Szyprowski
[not found] ` <CGME20200512090118eucas1p19ed5cf76c6e1e3f3bcaaefaeff7cf333@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 19/38] drm: panfrost: " Marek Szyprowski
[not found] ` <CGME20200512090119eucas1p2c0db485fddf17f15135f8e69e46fc097@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 20/38] drm: radeon: " Marek Szyprowski
[not found] ` <CGME20200512090119eucas1p2b3e1a858d8893f8d209d5c19fcbab941@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 21/38] drm: rockchip: use common helper for a scatterlist contiguity check Marek Szyprowski
[not found] ` <CGME20200512090120eucas1p28cc382480b2e3298b59fb6bf5ffde80b@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 22/38] drm: rockchip: fix common struct sg_table related issues Marek Szyprowski
[not found] ` <CGME20200512090120eucas1p18ae5489153837bbf5b8baa5089234c40@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 23/38] drm: tegra: " Marek Szyprowski
[not found] ` <CGME20200512090121eucas1p2f20e300f70ff54da15fe49cc6690f608@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 24/38] drm: v3d: " Marek Szyprowski
[not found] ` <CGME20200512090122eucas1p10cc3f42cb0452a8a279fcc8702e50a7a@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 25/38] drm: virtio: " Marek Szyprowski
[not found] ` <CGME20200512090122eucas1p155db33deb51b4bbc34c0a012e4a7361d@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 26/38] drm: vmwgfx: " Marek Szyprowski
[not found] ` <CGME20200512090123eucas1p268736ef6e202c23e8be77c56873f415f@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 27/38] xen: gntdev: " Marek Szyprowski
[not found] ` <CGME20200512090123eucas1p25758ba07ba913e5a3eeca13c7f386fdb@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 28/38] drm: host1x: " Marek Szyprowski
[not found] ` <CGME20200512090124eucas1p1f96fac067834c139fe1095a63b4dc2f0@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 29/38] drm: rcar-du: " Marek Szyprowski
[not found] ` <CGME20200512090124eucas1p20509113bdbdd1070d8265aa1af80e64a@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 30/38] dmabuf: " Marek Szyprowski
[not found] ` <CGME20200512090125eucas1p1f9eae024a33e92bf1468f2af4e5a1b0a@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 31/38] staging: ion: remove dead code Marek Szyprowski
[not found] ` <CGME20200512090126eucas1p1ad8d5dfd09fce31d9a18691a76e9fa75@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 32/38] staging: ion: fix common struct sg_table related issues Marek Szyprowski
[not found] ` <CGME20200512090127eucas1p19889d83b1c750dcdc869323e8d1946a3@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 33/38] staging: tegra-vde: " Marek Szyprowski
[not found] ` <CGME20200512090128eucas1p2cf31bfdca3b096585ba9f2741ef08ce0@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 34/38] misc: fastrpc: " Marek Szyprowski
[not found] ` <CGME20200512090128eucas1p11ee8a5e72ca37dc6b3e8a07ea094bab6@eucas1p1.samsung.com>
2020-05-12 9:00 ` [PATCH v4 35/38] rapidio: " Marek Szyprowski
[not found] ` <CGME20200512090129eucas1p2e67c8a9adafb202970a59c3412cd887a@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 36/38] samples: vfio-mdev/mbochs: " Marek Szyprowski
[not found] ` <CGME20200512090129eucas1p24fa7e83acb8cde494f71ca5694279401@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 37/38] media: pci: fix common ALSA DMA-mapping related codes Marek Szyprowski
[not found] ` <CGME20200512090130eucas1p2eb86c5d34be56bbc81032bc0b6927d1e@eucas1p2.samsung.com>
2020-05-12 9:00 ` [PATCH v4 38/38] videobuf2: use sgtable-based scatterlist wrappers Marek Szyprowski
2020-05-12 17:52 ` Ruhl, Michael J
2020-05-12 20:33 ` Marek Szyprowski
2020-05-13 12:01 ` Ruhl, Michael J
2020-05-12 12:18 ` [PATCH v4 01/38] dma-mapping: add generic helpers for mapping sgtable objects Christoph Hellwig
2020-05-12 13:00 ` Marek Szyprowski
2020-05-12 13:09 ` Christoph Hellwig
2020-05-12 13:19 ` Marek Szyprowski
2020-05-13 13:23 ` Robin Murphy [this message]
2020-05-12 12:19 ` [PATCH v4 00/38] DRM: fix struct sg_table nents vs. orig_nents misuse Christoph Hellwig
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=400501ec-c56b-edb7-7def-36ad43264123@arm.com \
--to=robin.murphy@arm.com \
--cc=airlied@linux.ie \
--cc=b.zolnierkie@samsung.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=hch@lst.de \
--cc=iommu@lists.linux-foundation.org \
--cc=linaro-mm-sig@lists.linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=m.szyprowski@samsung.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).