Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <f0dbdcc0-13b5-c484-0bf3-a1f8c3e48954@amd.com>
Date:   Thu, 27 Jan 2022 08:27:11 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH 02/19] dma-buf-map: Add helper to initialize second map
Content-Language: en-US
To:     Lucas De Marchi <lucas.demarchi@intel.com>,
        intel-gfx@lists.freedesktop.org
Cc:     dri-devel@lists.freedesktop.org,
        Matt Roper <matthew.d.roper@intel.com>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org,
        linux-kernel@vger.kernel.org
References: <20220126203702.1784589-1-lucas.demarchi@intel.com>
 <20220126203702.1784589-3-lucas.demarchi@intel.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20220126203702.1784589-3-lucas.demarchi@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?L1BNS0xoR2lDM0pLSUFhRkxWK0R6R0xKUGVnL0tvWU5OTndtQXNINVZVTm1D?=
 =?utf-8?B?QkM2MnUrdjVWaS9rVVJHbDIxV0ZaTUtJZnNUeDU5cVl3UGdTZjlYdEhhQlU3?=
 =?utf-8?B?TlhzZm1VSU94SXVUNm1UY0tKcU45V2k4amhINWVoUzVWaml5aHRudTJ0QW8r?=
 =?utf-8?B?dGFhSWhjcElUNXRPOUYzZEN6TndwTjJxTmFMMGcxN0gzQkRLeWxKSnBVNjBo?=
 =?utf-8?B?V24yNkpPbG9mMmJsaFBrL0phYWUwWW1EMkl2RThlbFI1TFYvZ1UzTGd6UXdE?=
 =?utf-8?B?QklOeGsrMWZ0ejRSVWE4WEtzb3JOejVZb25Ta2c1c0JpZCt6MHgwWjFMQU9I?=
 =?utf-8?B?M3M4Q1R2UXhKTFJkVDZqcE5hYWdOOFBYRW9FeWhOUkdMb0oxWHdBVnNXdlNr?=
 =?utf-8?B?Q3ZKR1JLK05JTDl2bitDL1JRWXAzNmJFODg1STY1SkVuYm9laVJOUzJ6MnNY?=
 =?utf-8?B?cmMwRFk4SklMVlVKM1VQRWlCd24rVVErLzN2ZWNxQ2V1U3pKUGRyeDZESjZm?=
 =?utf-8?B?OWMwVDJyVTlZNFY1VVJJWVlIOFdjVE0weXpiRGtZTS90QVIrQmQ5MHpUeS9C?=
 =?utf-8?B?Mk96eGovR0d0SjdnWldGVWxtUWdheDhQeURYMUJYbG94eUZmR1ZyVFJ1V3pG?=
 =?utf-8?B?Ui9VR1ZBa3hoS29vcEJicVRqOE5URzhzYWdVbWtIb2FyeTlGbS9VRnljZUgr?=
 =?utf-8?B?dXIwN0lxQlZCRVdsSlFGNHg3V0lPeGh0djVnaW5SUWJlNWtVNkpDYmZsYTZu?=
 =?utf-8?B?bHBUT2x6ekxFQXZNVTB1dDAxR1ZJaS95ZkZNT2pqZCtqTWlockhyYXpyMlZF?=
 =?utf-8?B?WVZVcGxPN2tRYXZ2N2R0MFFIaXhROERUa2dNZzQxL2NzdFVsRnpoZDJ4a1Mv?=
 =?utf-8?B?YUg3WlhpZjhzZGtKNXZvbVpyQTMwQmI5YUhwWnY1Tmp6eHlCYXlHWG8xQW55?=
 =?utf-8?B?NysvejlJZUwxZFF4ZlpSMEpOanRKUmttbWpxQnJJZVJJejJURys2L29wQmpv?=
 =?utf-8?B?S0c5MU9OZUJQbk5zek13VEtBcmZGY2U4UG9ON2xUNnBEUDVLNHlleE13c05j?=
 =?utf-8?B?NUNtVllmejJXZnc0SkRwMk8rbWhQZkI2VnhGRm5HVU5sTUdORVJjeXk5NjY2?=
 =?utf-8?B?Q0EwOTMyNmc5MXNiVUMwK0JXOXRqczEwbUkyUlJiOFQwc3VwU2dDWEF0VjRu?=
 =?utf-8?B?YnRtZ2kwWG1zRWR2QlZwNUJ3dERPWXdoMGZ0SUsza1A5ZGpnQ0JuaExjTnRG?=
 =?utf-8?B?VE5KWnhZVm02TGVHVzUwbys2aWlGZS83UHlnUzBsY1lwb2dyYlBrcGk3RjR0?=
 =?utf-8?B?L2I5ZFBSM2NGUzFNdGZPQm9PTjdHeFRzOVp0Wk5PY0ZaWmlST1lwM0hTWEds?=
 =?utf-8?B?ejlhQnpVUHdRaS8yNDY0QStlUTlUKy9Vd2xUYkZ1ZmtTL3h6b1lQSVZnMDNJ?=
 =?utf-8?B?OWwyWERXblZOUFV1a0d2SHgxZ0RwZGsyWXM0Njg2a0JUOW5CaEFGbTFkTDE0?=
 =?utf-8?B?MjQybzM4YkN2VXpUUU4rdnNhQUdXdDJKNUFGV3Y4MUlxN0VlSlZyWjZINkpU?=
 =?utf-8?B?WjE4ZnV3cmY2Uk5VVER5NnhyaGEwVGFkdHZwUmtIenZPdHlsNnhvMDlFUTZj?=
 =?utf-8?B?ck1MaFo5ay81REdkUlR2OXVuVm9BRFlrSW5NRjh1QlZBZUtvNER5NUFWak5E?=
 =?utf-8?B?STNJRE5mT3l1cFIrVlZRSTVsYXgvRWZLVThrRlNzaldsbVU1Y0ZYdTNSVWg3?=
 =?utf-8?B?MjBOZjFqbm5iS2gzWkZ6NTFoL21iT3hZdVFHaUtFTit0YlJmN3FTcnhvTi8r?=
 =?utf-8?B?V3Nrc1cxYWU4RkdKQnF2L0wzWkw4THR4OVJXdVVDMEh6aThvSXRaeXVTSTRi?=
 =?utf-8?B?aW42OSt6cldjVlpLck5zMXlJblpFVUNaRlgxb1VWVnhycjBrSExLRURVZ0FD?=
 =?utf-8?B?U3lrTDVYZEhicXZJNVdWUEVpdVlZQ1NZOTJXeU1DRzJDUzRaZ2lpUGl0ZlJG?=
 =?utf-8?B?Um1YK2Q4eGJGbzduNHpDOHM1VStrRHZmZ3BvTUVBZk9YZnRHYXc2NlEyU00v?=
 =?utf-8?B?eTVRTmlBMFRuejJRS3hjMW1zc2lRWjdTcWZRMVJLM1lVaTZzNWRDM1hMTjdW?=
 =?utf-8?Q?DcAI=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d481c12f-3a39-425a-5287-08d9e1667206
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jan 2022 07:27:16.7285
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: qTeNZA25bCsp6F6Bu4QZ6LPuOXo8IuwLb5ZyzhycE+6kwzYXhSwxy9ifbedTjLrC
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB2603
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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.

Well what you propose here can lead to all kind of problems and is 
rather bad design as far as I can see.

The struct dma_buf_map is only to be filled in by the exporter and 
should not be modified in this way by the importer.

If you need to copy only a certain subset of the mapping use the 
functions you added in patch #1.

Regards,
Christian.

>
> 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,				\
> +	}
> +
>   /**
>    * dma_buf_map_set_vaddr - Sets a dma-buf mapping structure to an address in system memory
>    * @map:	The dma-buf mapping structure