Am 14.01.23 um 00:15 schrieb Felix Kuehling:
On 2023-01-13 18:00, Chen, Xiaogang wrote:
On 1/13/2023 4:26 PM, Felix Kuehling wrote:
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@xxxxxxx>
---
.../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
The mmap_offset is actually not returned to user space. I just wonder
if here we should get mmap_offset of imported vram buffer if allow bo
be imported to difference gpus. If a vram buffer is imported to same
gpu device amdgpu_bo_mmap_offset is ok, otherwise I think
amdgpu_bo_mmap_offset would not give correct mmap_offset for the
device that the buffer is imported to.
When the BO is imported into the same GPU, you get a reference to the
same BO, so the imported BO has the same mmap_offset as the original BO.
When the BO is imported into a different GPU, it is a new BO with a
new mmap_offset.
That won't work.
I don't think this is incorrect.
No, this is completely incorrect. It mixes up the reverse tracking of
mappings and might crash the system. This is the reason why we can't
mmap() imported BOs.
mmapping the memory with that new offset should still work. The
imported BO is created with ttm_bo_type_sg, and AFAICT ttm_bo_vm.c
supports mapping of SG BOs.
Actually it shouldn't. This can go boom really easily.
When you have imported a BO the only correct way of to mmap() it is to
do so on the original exporter.
Regards,
Christian.
Regards,
Felix
Maybe we should remove mmap_offset parameter of
amdgpu_amdkfd_gpuvm_import_dmabuf since we do not return it to user
space anyway. With that:
Reviewed-by: Xiaogang Chen <Xiaoganng.Chen@xxxxxxx>
Regards
Xiaogang
@@ -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
