[PATCH 1/6] drm/amdgpu: Generalize KFD dmabuf import

[Felix Kuehling]

On 2023-01-12 17:41, Chen, Xiaogang wrote:
>
> On 1/11/2023 7:31 PM, Felix Kuehling wrote:
>> Use proper amdgpu_gem_prime_import function to handle all kinds of
>> imports. Remember the dmabuf reference to enable proper multi-GPU
>> attachment to multiple VMs without erroneously re-exporting the
>> underlying BO multiple times.
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
>> ---
>>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 38 ++++++++++---------
>>   1 file changed, 21 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> index 6f236ded5f12..e13c3493b786 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -2209,30 +2209,27 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct 
>> amdgpu_device *adev,
>>       struct amdgpu_bo *bo;
>>       int ret;
>>   -    if (dma_buf->ops != &amdgpu_dmabuf_ops)
>> -        /* Can't handle non-graphics buffers */
>> -        return -EINVAL;
>> -
>> -    obj = dma_buf->priv;
>> -    if (drm_to_adev(obj->dev) != adev)
>> -        /* Can't handle buffers from other devices */
>> -        return -EINVAL;
>> +    obj = amdgpu_gem_prime_import(adev_to_drm(adev), dma_buf);
>> +    if (IS_ERR(obj))
>> +        return PTR_ERR(obj);
>>         bo = gem_to_amdgpu_bo(obj);
>>       if (!(bo->preferred_domains & (AMDGPU_GEM_DOMAIN_VRAM |
>> -                    AMDGPU_GEM_DOMAIN_GTT)))
>> +                    AMDGPU_GEM_DOMAIN_GTT))) {
>>           /* Only VRAM and GTT BOs are supported */
>> -        return -EINVAL;
>> +        ret = -EINVAL;
>> +        goto err_put_obj;
>> +    }
>>         *mem = kzalloc(sizeof(struct kgd_mem), GFP_KERNEL);
>> -    if (!*mem)
>> -        return -ENOMEM;
>> +    if (!*mem) {
>> +        ret = -ENOMEM;
>> +        goto err_put_obj;
>> +    }
>>         ret = drm_vma_node_allow(&obj->vma_node, drm_priv);
>> -    if (ret) {
>> -        kfree(*mem);
>> -        return ret;
>> -    }
>> +    if (ret)
>> +        goto err_free_mem;
>>         if (size)
>>           *size = amdgpu_bo_size(bo);
>
> Hi Felix:
>
> I have a question when using amdgpu_gem_prime_import. It will allow 
> importing a dmabuf to different gpus, then can we still call 
> amdgpu_bo_mmap_offset on the generated bo if 
> amdgpu_amdkfd_gpuvm_import_dmabuf also ask mmap_offset?

The mmap  offset comes from drm_vma_node_offset_addr. The DRM VMA 
address is allocated when ttm_bo_init_reserved calls drm_vma_offset_add, 
so there should be no problem querying the mmap_offset. Whether mmapping 
of an imported BO is actually supported is a different question. As far 
as I can see, it should be possible. That said, currently ROCm 
(libhsakmt) uses only original BOs for CPU mappings, not imported BOs.

Regards,
   Felix


>
>> @@ -2249,7 +2246,8 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct 
>> amdgpu_device *adev,
>>           | KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE
>>           | KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
>>   -    drm_gem_object_get(&bo->tbo.base);
>> +    get_dma_buf(dma_buf);
>> +    (*mem)->dmabuf = dma_buf;
>>       (*mem)->bo = bo;
>>       (*mem)->va = va;
>>       (*mem)->domain = (bo->preferred_domains & 
>> AMDGPU_GEM_DOMAIN_VRAM) ?
>> @@ -2261,6 +2259,12 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct 
>> amdgpu_device *adev,
>>       (*mem)->is_imported = true;
>>         return 0;
>> +
>> +err_free_mem:
>> +    kfree(*mem);
>> +err_put_obj:
>> +    drm_gem_object_put(obj);
>> +    return ret;
>>   }
>>     /* Evict a userptr BO by stopping the queues if necessary