Re: [PATCH 02/19] dma-buf-map: Add helper to initialize second map

Thomas Zimmermann
2022-01-27T14:33:18Z

[-- Attachment #1.1: Type: text/plain, Size: 2593 bytes --]

Am 26.01.22 um 21:36 schrieb Lucas De Marchi:
> When dma_buf_map struct is passed around, it's useful to be able to
> initialize a second map that takes care of reading/writing to an offset
> of the original map.
>
> Add a helper that copies the struct and add the offset to the proper
> address.
>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: linux-media@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linaro-mm-sig@lists.linaro.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> include/linux/dma-buf-map.h | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
> index 65e927d9ce33..3514a859f628 100644
> --- a/include/linux/dma-buf-map.h
> +++ b/include/linux/dma-buf-map.h
> @@ -131,6 +131,35 @@ struct dma_buf_map {
> .is_iomem = false, \
> }
>
> +/**
> + * DMA_BUF_MAP_INIT_OFFSET - Initializes struct dma_buf_map from another dma_buf_map
> + * @map_: The dma-buf mapping structure to copy from
> + * @offset: Offset to add to the other mapping
> + *
> + * Initializes a new dma_buf_struct based on another. This is the equivalent of doing:
> + *
> + * .. code-block: c
> + *
> + * dma_buf_map map = other_map;
> + * dma_buf_map_incr(&map, &offset);
> + *
> + * Example usage:
> + *
> + * .. code-block: c
> + *
> + * void foo(struct device *dev, struct dma_buf_map *base_map)
> + * {
> + * ...
> + * struct dma_buf_map = DMA_BUF_MAP_INIT_OFFSET(base_map, FIELD_OFFSET);
> + * ...
> + * }
> + */
> +#define DMA_BUF_MAP_INIT_OFFSET(map_, offset_) (struct dma_buf_map) \
> + { \
> + .vaddr = (map_)->vaddr + (offset_), \
> + .is_iomem = (map_)->is_iomem, \
> + }
> +

It's illegal to access .vaddr with raw pointer. Always use a
dma_buf_memcpy_() interface. So why would you need this macro when you
have dma_buf_memcpy_*() with an offset parameter?

I've also been very careful to distinguish between .vaddr and
.vaddr_iomem, even in places where I wouldn't have to. This macro breaks
the assumption.

Best regards
Thomas

> /**
> * dma_buf_map_set_vaddr - Sets a dma-buf mapping structure to an address in system memory
> * @map: The dma-buf mapping structure

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]