Re: [PATCH V1] accel/amdxdna: Remove mmap and export support for ubuf

[Lizhi Hou <lizhi.hou@amd.com>]

On 5/18/26 04:46, Christian König wrote:
>
> On 5/15/26 17:53, Lizhi Hou wrote:
>> Ubuf pages should not be mmaped or exported. Remove the ubuf mmap callback
>> and return -EOPNOTSUPP when exporting ubuf objects.
>>
>> ubuf vmap is also removed for there is not a real use case yet.
>>
>> Fixes: bd72d4acda10 ("accel/amdxdna: Support user space allocated buffer")
>> Cc: Christian Koenig <christian.koenig@amd.com>
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>> ---
>>   drivers/accel/amdxdna/amdxdna_gem.c  |  9 +++++++-
>>   drivers/accel/amdxdna/amdxdna_gem.h  |  2 ++
>>   drivers/accel/amdxdna/amdxdna_ubuf.c | 32 ----------------------------
>>   3 files changed, 10 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
>> index 319d2064fafa..6087264ba1b5 100644
>> --- a/drivers/accel/amdxdna/amdxdna_gem.c
>> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
>> @@ -492,6 +492,9 @@ static struct dma_buf *amdxdna_gem_prime_export(struct drm_gem_object *gobj, int
>>   	struct amdxdna_gem_obj *abo = to_xdna_obj(gobj);
>>   	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>>   
>> +	if (abo->pri)
>> +		return ERR_PTR(-EOPNOTSUPP);
>> +
>>   	if (abo->dma_buf) {
>>   		get_dma_buf(abo->dma_buf);
>>   		return abo->dma_buf;
>> @@ -716,6 +719,7 @@ amdxdna_gem_create_ubuf_object(struct drm_device *dev, struct amdxdna_drm_create
>>   {
>>   	struct amdxdna_dev *xdna = to_xdna_dev(dev);
>>   	struct amdxdna_drm_va_tbl va_tbl;
>> +	struct amdxdna_gem_obj *abo;
>>   	struct drm_gem_object *gobj;
>>   	struct dma_buf *dma_buf;
>>   
>> @@ -742,7 +746,10 @@ amdxdna_gem_create_ubuf_object(struct drm_device *dev, struct amdxdna_drm_create
>>   
>>   	dma_buf_put(dma_buf);
>>   
>> -	return to_xdna_obj(gobj);
>> +	abo = to_xdna_obj(gobj);
>> +	abo->pri = true;
>> +
>> +	return abo;
>>   }
>>   
>>   static struct amdxdna_gem_obj *
>> diff --git a/drivers/accel/amdxdna/amdxdna_gem.h b/drivers/accel/amdxdna/amdxdna_gem.h
>> index 4fc48a1189d2..162e5499f5e0 100644
>> --- a/drivers/accel/amdxdna/amdxdna_gem.h
>> +++ b/drivers/accel/amdxdna/amdxdna_gem.h
>> @@ -54,6 +54,8 @@ struct amdxdna_gem_obj {
>>   
>>   	/* True, if BO is managed by XRT, not application */
>>   	bool				internal;
>> +	/* True, if BO is not exportable */
>> +	bool				pri;
> That member name could be improved.
Sure, I will change it to private_buffer.
>
>>   };
>>   
>>   #define to_gobj(obj)    (&(obj)->base.base)
>> diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c b/drivers/accel/amdxdna/amdxdna_ubuf.c
>> index 3769210c55cc..df4ab225fbf9 100644
>> --- a/drivers/accel/amdxdna/amdxdna_ubuf.c
>> +++ b/drivers/accel/amdxdna/amdxdna_ubuf.c
>> @@ -87,42 +87,10 @@ static const struct vm_operations_struct amdxdna_ubuf_vm_ops = {
>>   	.fault = amdxdna_ubuf_vm_fault,
>>   };
> As pointed out by the kernel test robot the amdxdna_ubuf_vm_ops structure and amdxdna_ubuf_vm_fault function is now dead code, please remove as well.

Will fix this and post V2 patch.

Thanks for reviewing.

Lizhi

>
> Apart from that looks good to me,
> Christian.
>
>>   
>> -static int amdxdna_ubuf_mmap(struct dma_buf *dbuf, struct vm_area_struct *vma)
>> -{
>> -	struct amdxdna_ubuf_priv *ubuf = dbuf->priv;
>> -
>> -	vma->vm_ops = &amdxdna_ubuf_vm_ops;
>> -	vma->vm_private_data = ubuf;
>> -	vm_flags_set(vma, VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP);
>> -
>> -	return 0;
>> -}
>> -
>> -static int amdxdna_ubuf_vmap(struct dma_buf *dbuf, struct iosys_map *map)
>> -{
>> -	struct amdxdna_ubuf_priv *ubuf = dbuf->priv;
>> -	void *kva;
>> -
>> -	kva = vmap(ubuf->pages, ubuf->nr_pages, VM_MAP, PAGE_KERNEL);
>> -	if (!kva)
>> -		return -EINVAL;
>> -
>> -	iosys_map_set_vaddr(map, kva);
>> -	return 0;
>> -}
>> -
>> -static void amdxdna_ubuf_vunmap(struct dma_buf *dbuf, struct iosys_map *map)
>> -{
>> -	vunmap(map->vaddr);
>> -}
>> -
>>   static const struct dma_buf_ops amdxdna_ubuf_dmabuf_ops = {
>>   	.map_dma_buf = amdxdna_ubuf_map,
>>   	.unmap_dma_buf = amdxdna_ubuf_unmap,
>>   	.release = amdxdna_ubuf_release,
>> -	.mmap = amdxdna_ubuf_mmap,
>> -	.vmap = amdxdna_ubuf_vmap,
>> -	.vunmap = amdxdna_ubuf_vunmap,
>>   };
>>   
>>   static int readonly_va_entry(struct amdxdna_drm_va_entry *va_ent)