Message-ID: <5eb593b8-3cd3-af93-d1ce-59fea6b879b9@amd.com>
Date:   Fri, 3 Mar 2023 13:50:26 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH] drm/amdgpu: Implement mmap of imported dma-bufs
Content-Language: en-US
To:     Marek Maslanka <mmaslanka@chromium.org>,
        dri-devel@lists.freedesktop.org, Daniel Vetter <daniel@ffwll.ch>
Cc:     Dominik Behr <dbehr@chromium.org>,
        Alex Deucher <alexander.deucher@amd.com>,
        "Pan, Xinhui" <Xinhui.Pan@amd.com>,
        David Airlie <airlied@gmail.com>,
        Lijo Lazar <lijo.lazar@amd.com>,
        Guchun Chen <guchun.chen@amd.com>,
        Felix Kuehling <Felix.Kuehling@amd.com>,
        Marek Maslanka <mm@semihalf.com>,
        Evan Quan <evan.quan@amd.com>,
        Hawking Zhang <Hawking.Zhang@amd.com>,
        Mario Limonciello <mario.limonciello@amd.com>,
        YiPeng Chai <YiPeng.Chai@amd.com>,
        =?UTF-8?Q?Michel_D=c3=a4nzer?= <mdaenzer@redhat.com>,
        Kai-Heng Feng <kai.heng.feng@canonical.com>,
        Luben Tuikov <luben.tuikov@amd.com>,
        Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>,
        Tong Liu01 <Tong.Liu01@amd.com>,
        Rajneesh Bhardwaj <rajneesh.bhardwaj@amd.com>,
        Zack Rusin <zackr@vmware.com>, amd-gfx@lists.freedesktop.org,
        linux-kernel@vger.kernel.org
References: <20230303110951.3777850-1-mmaslanka@chromium.org>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20230303110951.3777850-1-mmaslanka@chromium.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N3RTeVNkYjZhSko1QkRJMkNqZEFqUURiSkRpZU1oR0pGUytFdjk1bnA1Z2pR?=
 =?utf-8?B?MUxYSzZkTHA3WDd1SDdzTUsyaXVKNnh5SWdHcG5jRU51a2h2UFdHY0dxWVd0?=
 =?utf-8?B?VDYzSTA5WGMwaGFHa0xkRjV5Wk1ITWpKL2krYXhsOVR3dXIvMGt4UktWQlB6?=
 =?utf-8?B?dTJEVHdyZEplTCt1WHl6UWVLOTBBUUhqbDZHY1dGUWczSmhkWVVOZWluSjRl?=
 =?utf-8?B?a08wZmZWaFlhTkQ1Y05zNWx1MnhmdHM2bVpxazc1UHU0VHlsbm05UkhBOU5T?=
 =?utf-8?B?dkRsZndCcW1EUXdSNzNNODVrdTVlUE43UHhUS0Mzbm5BY09ZcHh3clNvWHRR?=
 =?utf-8?B?TnhSbU45ejNjMkh1Qi95bjFacnRqM1czNlhWMy9VeW1TZEpuNXJySXVEQlha?=
 =?utf-8?B?RHpkalJ4ek8zQmN4L0Z5MkFHSi9VVWNrKzFickZyR0JOeFpRYXRLUmhLUzE4?=
 =?utf-8?B?M1BpR3BCWndvcmxwY25MMk9ncHBLdWlXUThiVGNDcVBzZlp6NThxd2xBZUJQ?=
 =?utf-8?B?ZE9kU0pIMUdlUW40RmFqTWFYZHFYdzE2dUVyVXJxVDdEbEF5WGxLSjFJWmtW?=
 =?utf-8?B?V1R3aGxLRTVnUHp0TjVEMGFJdXVaU1pzbzJIRTRRdEhLNWRsUVRqT1pFOTRN?=
 =?utf-8?B?M2wxWmlMVFJRMDRvTndTNldLdXVpMEpCek9UWDZQNERNMkxhNnNEeS92NWNI?=
 =?utf-8?B?b3RrVFV4UEg5a1pMZmMvVEdMdVUzckx6d2lXNTk1TUZRNGhzQ2d0bUxENENh?=
 =?utf-8?B?MzhYWnozbmFyVjJHK3BVZ1NaZi9aT21qbVlXbElXTyt4bERCbUEwS3BEdXJr?=
 =?utf-8?B?cE5leVhOYXgxNStFQXFWZW0zQ0JTSGtkRjJCN0lDN25FRWNuVWFQdHQ4Y0dm?=
 =?utf-8?B?cG9mejdaTTJTT3dmbkVGYU9mOEYydTF1QlZYUmZrSWhsUGx2cTVMTTZlb3Vj?=
 =?utf-8?B?c0VKc2phdDVQcVFHTDQ1WVByazRZOEZFWGF5Tjk0UUVvUDhtdno4eUQzZDFi?=
 =?utf-8?B?dDU3QXFjRlU0aXN4L2t6WTBaQUMvWkFFTm1yNVlwbVpFdjRrQzJtNnNKQ2VG?=
 =?utf-8?B?OU1XN01mZWhoc1lPVW5DeXFVdmNaRWxVL2dDeCsyV3dnd3V2N0VOZC9uRktk?=
 =?utf-8?B?R0ZOdDNxeVNFOGRtblFteUNrVkNBS3I3eWhtdlhlMW4zQXZwcVExYmJBYURE?=
 =?utf-8?B?RTRXME1zU1N5T2RGZ21SZEVjTmtMaUZheUg2Ym5SdG5uTGo4MDRMRWlmbksr?=
 =?utf-8?B?aE1VNEY2TEVMaThudC9lU2p6VHJvalRPMjNHVytOdVpCSC9tQlkyQlA1OHlM?=
 =?utf-8?B?RTVMZE9PMVRCK1lVLzB0NWl6ckR2S3gyc1IvOHZXbi9LRVh6elA1ZjJqTnhG?=
 =?utf-8?B?OGZ0N09iaSt6a2ttWllJY2ZzMEFhcU1Ja2J4TURsSnhlTTdvdmVKMGR5Qjhh?=
 =?utf-8?B?Ry8zQW9EN1Z1TVYrL1U1bG5yMWV6cE1iazhIZUZEa2xnT0t5R2xrd2RVZTV3?=
 =?utf-8?B?R0tNTmw5aEhzdDMvckVCS3E1ZnVyeEdEdFlsbHF2RmIvbmpLVU5jc3BkRStT?=
 =?utf-8?B?MzArNWdTTUFQQWJBTUlNdjkrY0MxNnB4dDZQbUlBNGNEUlJvRlk4cXF6TEY2?=
 =?utf-8?B?a2tBMGhrL0lna05QZW9jS1Y4YnRjTFY5RExnbnBUSlk3azhrNjhaRUVibFNu?=
 =?utf-8?B?RndmT0RCbGNnTzEzbW9GcXhsYjJwWnd1SHl6cllrbHVJSENQNkE0Z3AzL0t4?=
 =?utf-8?B?WHVMYUVKOE5CQ3dKS2FscUlqWXErQjNYNUlBT1pNeWo4TUdrVG03TzFhcTNs?=
 =?utf-8?B?Y3VlUEYrK0NOWWpBbXZnWXRORERpWlFGa2pjZGRnZTEwUHZockJTcmduY1dw?=
 =?utf-8?B?NmJVbjVtckpubVRHaHNnNjJEUTNoWWhnU3d3VUoyajhnUnlFWHBWOGVlaFlR?=
 =?utf-8?B?MVpHQnNjeXN5VVJ5T1RaSkJWL25rZ0ViUjNhLzFLMWZtdlI1bGVLMjFSQ3Q5?=
 =?utf-8?B?QkZ5Y2g4NEhRRFBkYWF6N3lNSThPS0I0eGdiNXBVaFU2eXZxWGpKMjlEc0hh?=
 =?utf-8?B?U2UzbndvQ0JROHFSa21PS0M2OG5qbjExb3BDV2NxdmcxbTk3dVlxa2dxeTVV?=
 =?utf-8?B?UFpnSnNkM2p6cHBhMTlvZHgxaDdsK0tUNGdiWkpUZUU3ajE4NEM2eW1OK280?=
 =?utf-8?Q?L2AEAFxg+2OqdmZ9NrFj3mXeaYUat/2SPot16sKiCNob?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8d200b2a-c4b3-4de0-b5cd-08db1be5e14d
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Mar 2023 12:50:34.7609
 (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: smY8mp08iZj1zoUlsXQj2tRWIM7sn4PeRWMCfbV34pQsYFWc75jn49JNJU2smSWO
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4353
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Well big NAK to this!

We have discussed this extensively on the mailing list and absolutely 
don't want to allow that.

Only for some ARM drivers we failed to block that soon enough and will 
keep the functionality around to not break userspace.

Regards,
Christian.

Am 03.03.23 um 12:09 schrieb Marek Maslanka:
> Use dmabuf mmap from exporting driver to do the mapping.
>
> Signed-off-by: Marek Maslanka <mmaslanka@chromium.org>
> Signed-off-by: Dominik Behr <dbehr@chromium.org>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 +++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h |  2 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  9 ++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  1 +
>   5 files changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> index 0c001bb8fc2b..8f22d29ba077 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> @@ -37,6 +37,7 @@
>   #include "amdgpu_dma_buf.h"
>   #include "amdgpu_xgmi.h"
>   #include <drm/amdgpu_drm.h>
> +#include <drm/drm_drv.h>
>   #include <drm/ttm/ttm_tt.h>
>   #include <linux/dma-buf.h>
>   #include <linux/dma-fence-array.h>
> @@ -275,6 +276,51 @@ const struct dma_buf_ops amdgpu_dmabuf_ops = {
>   	.vunmap = drm_gem_dmabuf_vunmap,
>   };
>   
> +int amdgpu_try_dma_buf_mmap(struct file *filp, struct vm_area_struct *vma)
> +{
> +	struct drm_file *priv = filp->private_data;
> +	struct drm_device *dev = priv->minor->dev;
> +	struct amdgpu_device *adev = drm_to_adev(dev);
> +	struct ttm_device *bdev = &adev->mman.bdev;
> +	struct ttm_buffer_object *tbo = NULL;
> +	struct amdgpu_bo *bo = NULL;
> +	struct drm_gem_object *obj = NULL;
> +	struct drm_vma_offset_node *node;
> +	int ret;
> +
> +	if (drm_dev_is_unplugged(dev))
> +		return -ENODEV;
> +
> +	drm_vma_offset_lock_lookup(bdev->vma_manager);
> +	node = drm_vma_offset_exact_lookup_locked(bdev->vma_manager,
> +						  vma->vm_pgoff,
> +						  vma_pages(vma));
> +
> +	if (likely(node)) {
> +		tbo = container_of(node, struct ttm_buffer_object,
> +				   base.vma_node);
> +		tbo = ttm_bo_get_unless_zero(tbo);
> +	}
> +	drm_vma_offset_unlock_lookup(bdev->vma_manager);
> +
> +	if (!tbo)
> +		return -EINVAL;
> +
> +	bo = ttm_to_amdgpu_bo(tbo);
> +	obj = &tbo->base;
> +
> +	if (!obj->import_attach) {
> +		ret = -EINVAL;
> +		goto done;
> +	}
> +
> +	ret = dma_buf_mmap(obj->import_attach->dmabuf, vma, 0);
> +
> +done:
> +	ttm_bo_put(tbo);
> +	return ret;
> +}
> +
>   /**
>    * amdgpu_gem_prime_export - &drm_driver.gem_prime_export implementation
>    * @gobj: GEM BO
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
> index 3e93b9b407a9..ecf1dc32eec4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
> @@ -25,6 +25,8 @@
>   
>   #include <drm/drm_gem.h>
>   
> +int amdgpu_try_dma_buf_mmap(struct file *filp, struct vm_area_struct *vma);
> +
>   struct dma_buf *amdgpu_gem_prime_export(struct drm_gem_object *gobj,
>   					int flags);
>   struct drm_gem_object *amdgpu_gem_prime_import(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 86fbb4138285..91e94342d48e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2737,7 +2737,7 @@ static const struct file_operations amdgpu_driver_kms_fops = {
>   	.flush = amdgpu_flush,
>   	.release = drm_release,
>   	.unlocked_ioctl = amdgpu_drm_ioctl,
> -	.mmap = drm_gem_mmap,
> +	.mmap = amdgpu_mmap,
>   	.poll = drm_poll,
>   	.read = drm_read,
>   #ifdef CONFIG_COMPAT
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index c5ef7f7bdc15..41944439cd6c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -61,6 +61,7 @@
>   #include "amdgpu_hmm.h"
>   #include "amdgpu_atomfirmware.h"
>   #include "amdgpu_res_cursor.h"
> +#include "amdgpu_dma_buf.h"
>   #include "bif/bif_4_1_d.h"
>   
>   MODULE_IMPORT_NS(DMA_BUF);
> @@ -1994,6 +1995,14 @@ static int amdgpu_ttm_prepare_job(struct amdgpu_device *adev,
>   						   DMA_RESV_USAGE_BOOKKEEP);
>   }
>   
> +int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma)
> +{
> +	if (amdgpu_try_dma_buf_mmap(filp, vma) == 0)
> +		return 0;
> +
> +	return drm_gem_mmap(filp, vma);
> +}
> +
>   int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset,
>   		       uint64_t dst_offset, uint32_t byte_count,
>   		       struct dma_resv *resv,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index e2cd5894afc9..e4cd1bda7a2b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -152,6 +152,7 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
>   			struct dma_resv *resv,
>   			struct dma_fence **fence);
>   
> +int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
>   int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
>   void amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
>   uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, uint32_t type);