amd-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] drm/amdkfd: Remove legacy code not acquiring VMs
@ 2021-04-07 23:12 Felix Kuehling
  2021-04-07 23:12 ` [PATCH 2/4] drm/amdkfd: Use drm_priv to pass VM from KFD to amdgpu Felix Kuehling
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Felix Kuehling @ 2021-04-07 23:12 UTC (permalink / raw)
  To: amd-gfx, dri-devel; +Cc: christian.koenig, tzimmermann

ROCm user mode has acquired VMs from DRM file descriptors for as long
as it supported the upstream KFD. Legacy code to support older versions
of ROCm is not needed any more.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  4 --
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 50 -------------------
 drivers/gpu/drm/amd/amdkfd/kfd_process.c      | 27 ++++------
 3 files changed, 10 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 14f68c028126..5ffb07b02810 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -234,14 +234,10 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev *dst, struct kgd_dev *s
 	})
 
 /* GPUVM API */
-int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, u32 pasid,
-					void **vm, void **process_info,
-					struct dma_fence **ef);
 int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
 					struct file *filp, u32 pasid,
 					void **vm, void **process_info,
 					struct dma_fence **ef);
-void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm);
 void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm);
 uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm);
 int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index e93850f2f3b1..36012229ccc1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1037,41 +1037,6 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
 	return ret;
 }
 
-int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, u32 pasid,
-					  void **vm, void **process_info,
-					  struct dma_fence **ef)
-{
-	struct amdgpu_device *adev = get_amdgpu_device(kgd);
-	struct amdgpu_vm *new_vm;
-	int ret;
-
-	new_vm = kzalloc(sizeof(*new_vm), GFP_KERNEL);
-	if (!new_vm)
-		return -ENOMEM;
-
-	/* Initialize AMDGPU part of the VM */
-	ret = amdgpu_vm_init(adev, new_vm, AMDGPU_VM_CONTEXT_COMPUTE, pasid);
-	if (ret) {
-		pr_err("Failed init vm ret %d\n", ret);
-		goto amdgpu_vm_init_fail;
-	}
-
-	/* Initialize KFD part of the VM and process info */
-	ret = init_kfd_vm(new_vm, process_info, ef);
-	if (ret)
-		goto init_kfd_vm_fail;
-
-	*vm = (void *) new_vm;
-
-	return 0;
-
-init_kfd_vm_fail:
-	amdgpu_vm_fini(adev, new_vm);
-amdgpu_vm_init_fail:
-	kfree(new_vm);
-	return ret;
-}
-
 int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
 					   struct file *filp, u32 pasid,
 					   void **vm, void **process_info,
@@ -1138,21 +1103,6 @@ void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
 	}
 }
 
-void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm)
-{
-	struct amdgpu_device *adev = get_amdgpu_device(kgd);
-	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
-
-	if (WARN_ON(!kgd || !vm))
-		return;
-
-	pr_debug("Destroying process vm %p\n", vm);
-
-	/* Release the VM context */
-	amdgpu_vm_fini(adev, avm);
-	kfree(vm);
-}
-
 void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm)
 {
 	struct amdgpu_device *adev = get_amdgpu_device(kgd);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index d4241d29ea94..d97e330a5022 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -935,9 +935,6 @@ static void kfd_process_destroy_pdds(struct kfd_process *p)
 					pdd->dev->kgd, pdd->vm);
 			fput(pdd->drm_file);
 		}
-		else if (pdd->vm)
-			amdgpu_amdkfd_gpuvm_destroy_process_vm(
-				pdd->dev->kgd, pdd->vm);
 
 		if (pdd->qpd.cwsr_kaddr && !pdd->qpd.cwsr_base)
 			free_pages((unsigned long)pdd->qpd.cwsr_kaddr,
@@ -1375,19 +1372,18 @@ int kfd_process_device_init_vm(struct kfd_process_device *pdd,
 	struct kfd_dev *dev;
 	int ret;
 
+	if (!drm_file)
+		return -EINVAL;
+
 	if (pdd->vm)
-		return drm_file ? -EBUSY : 0;
+		return -EBUSY;
 
 	p = pdd->process;
 	dev = pdd->dev;
 
-	if (drm_file)
-		ret = amdgpu_amdkfd_gpuvm_acquire_process_vm(
-			dev->kgd, drm_file, p->pasid,
-			&pdd->vm, &p->kgd_process_info, &p->ef);
-	else
-		ret = amdgpu_amdkfd_gpuvm_create_process_vm(dev->kgd, p->pasid,
-			&pdd->vm, &p->kgd_process_info, &p->ef);
+	ret = amdgpu_amdkfd_gpuvm_acquire_process_vm(
+		dev->kgd, drm_file, p->pasid,
+		&pdd->vm, &p->kgd_process_info, &p->ef);
 	if (ret) {
 		pr_err("Failed to create process VM object\n");
 		return ret;
@@ -1409,8 +1405,6 @@ int kfd_process_device_init_vm(struct kfd_process_device *pdd,
 err_init_cwsr:
 err_reserve_ib_mem:
 	kfd_process_device_free_bos(pdd);
-	if (!drm_file)
-		amdgpu_amdkfd_gpuvm_destroy_process_vm(dev->kgd, pdd->vm);
 	pdd->vm = NULL;
 
 	return ret;
@@ -1435,6 +1429,9 @@ struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev,
 		return ERR_PTR(-ENOMEM);
 	}
 
+	if (!pdd->vm)
+		return ERR_PTR(-ENODEV);
+
 	/*
 	 * signal runtime-pm system to auto resume and prevent
 	 * further runtime suspend once device pdd is created until
@@ -1452,10 +1449,6 @@ struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev,
 	if (err)
 		goto out;
 
-	err = kfd_process_device_init_vm(pdd, NULL);
-	if (err)
-		goto out;
-
 	/*
 	 * make sure that runtime_usage counter is incremented just once
 	 * per pdd
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] drm/amdkfd: Use drm_priv to pass VM from KFD to amdgpu
  2021-04-07 23:12 [PATCH 1/4] drm/amdkfd: Remove legacy code not acquiring VMs Felix Kuehling
@ 2021-04-07 23:12 ` Felix Kuehling
  2021-04-14 15:21   ` philip yang
  2021-04-07 23:12 ` [PATCH 3/4] drm/amdkfd: Allow access for mmapping KFD BOs Felix Kuehling
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Felix Kuehling @ 2021-04-07 23:12 UTC (permalink / raw)
  To: amd-gfx, dri-devel; +Cc: christian.koenig, tzimmermann

amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu needs the drm_priv to allow mmap
to access the BO through the corresponding file descriptor.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    | 14 ++--
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 69 +++++++++++--------
 drivers/gpu/drm/amd/amdkfd/kfd_process.c      |  5 +-
 3 files changed, 50 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 5ffb07b02810..0d59bebd92af 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -236,20 +236,20 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev *dst, struct kgd_dev *s
 /* GPUVM API */
 int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
 					struct file *filp, u32 pasid,
-					void **vm, void **process_info,
+					void **process_info,
 					struct dma_fence **ef);
-void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm);
-uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm);
+void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *drm_priv);
+uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *drm_priv);
 int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 		struct kgd_dev *kgd, uint64_t va, uint64_t size,
-		void *vm, struct kgd_mem **mem,
+		void *drm_priv, struct kgd_mem **mem,
 		uint64_t *offset, uint32_t flags);
 int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 		struct kgd_dev *kgd, struct kgd_mem *mem, uint64_t *size);
 int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
-		struct kgd_dev *kgd, struct kgd_mem *mem, void *vm);
+		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv);
 int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
-		struct kgd_dev *kgd, struct kgd_mem *mem, void *vm);
+		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv);
 int amdgpu_amdkfd_gpuvm_sync_memory(
 		struct kgd_dev *kgd, struct kgd_mem *mem, bool intr);
 int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct kgd_dev *kgd,
@@ -260,7 +260,7 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
 					      struct kfd_vm_fault_info *info);
 int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
 				      struct dma_buf *dmabuf,
-				      uint64_t va, void *vm,
+				      uint64_t va, void *drm_priv,
 				      struct kgd_mem **mem, uint64_t *size,
 				      uint64_t *mmap_offset);
 int amdgpu_amdkfd_get_tile_config(struct kgd_dev *kgd,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 36012229ccc1..95442bcd60fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -951,6 +951,13 @@ static int process_update_pds(struct amdkfd_process_info *process_info,
 	return 0;
 }
 
+static struct amdgpu_vm *drm_priv_to_vm(struct drm_file *drm_priv)
+{
+	struct amdgpu_fpriv *fpriv = drm_priv->driver_priv;
+
+	return &fpriv->vm;
+}
+
 static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
 		       struct dma_fence **ef)
 {
@@ -1039,15 +1046,19 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
 
 int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
 					   struct file *filp, u32 pasid,
-					   void **vm, void **process_info,
+					   void **process_info,
 					   struct dma_fence **ef)
 {
 	struct amdgpu_device *adev = get_amdgpu_device(kgd);
-	struct drm_file *drm_priv = filp->private_data;
-	struct amdgpu_fpriv *drv_priv = drm_priv->driver_priv;
-	struct amdgpu_vm *avm = &drv_priv->vm;
+	struct amdgpu_fpriv *drv_priv;
+	struct amdgpu_vm *avm;
 	int ret;
 
+	ret = amdgpu_file_to_fpriv(filp, &drv_priv);
+	if (ret)
+		return ret;
+	avm = &drv_priv->vm;
+
 	/* Already a compute VM? */
 	if (avm->process_info)
 		return -EINVAL;
@@ -1062,7 +1073,7 @@ int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
 	if (ret)
 		return ret;
 
-	*vm = (void *)avm;
+	amdgpu_vm_set_task_info(avm);
 
 	return 0;
 }
@@ -1103,15 +1114,17 @@ void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
 	}
 }
 
-void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm)
+void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *drm_priv)
 {
 	struct amdgpu_device *adev = get_amdgpu_device(kgd);
-	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
+	struct amdgpu_vm *avm;
 
-	if (WARN_ON(!kgd || !vm))
+	if (WARN_ON(!kgd || !drm_priv))
 		return;
 
-	pr_debug("Releasing process vm %p\n", vm);
+	avm = drm_priv_to_vm(drm_priv);
+
+	pr_debug("Releasing process vm %p\n", avm);
 
 	/* The original pasid of amdgpu vm has already been
 	 * released during making a amdgpu vm to a compute vm
@@ -1122,9 +1135,9 @@ void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm)
 	amdgpu_vm_release_compute(adev, avm);
 }
 
-uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm)
+uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *drm_priv)
 {
-	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
+	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
 	struct amdgpu_bo *pd = avm->root.base.bo;
 	struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev);
 
@@ -1135,11 +1148,11 @@ uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm)
 
 int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 		struct kgd_dev *kgd, uint64_t va, uint64_t size,
-		void *vm, struct kgd_mem **mem,
+		void *drm_priv, struct kgd_mem **mem,
 		uint64_t *offset, uint32_t flags)
 {
 	struct amdgpu_device *adev = get_amdgpu_device(kgd);
-	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
+	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
 	enum ttm_bo_type bo_type = ttm_bo_type_device;
 	struct sg_table *sg = NULL;
 	uint64_t user_addr = 0;
@@ -1350,10 +1363,10 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 }
 
 int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
-		struct kgd_dev *kgd, struct kgd_mem *mem, void *vm)
+		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv)
 {
 	struct amdgpu_device *adev = get_amdgpu_device(kgd);
-	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
+	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
 	int ret;
 	struct amdgpu_bo *bo;
 	uint32_t domain;
@@ -1394,9 +1407,9 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
 	pr_debug("Map VA 0x%llx - 0x%llx to vm %p domain %s\n",
 			mem->va,
 			mem->va + bo_size * (1 + mem->aql_queue),
-			vm, domain_string(domain));
+			avm, domain_string(domain));
 
-	ret = reserve_bo_and_vm(mem, vm, &ctx);
+	ret = reserve_bo_and_vm(mem, avm, &ctx);
 	if (unlikely(ret))
 		goto out;
 
@@ -1440,7 +1453,7 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
 	}
 
 	list_for_each_entry(entry, &mem->bo_va_list, bo_list) {
-		if (entry->bo_va->base.vm == vm && !entry->is_mapped) {
+		if (entry->bo_va->base.vm == avm && !entry->is_mapped) {
 			pr_debug("\t map VA 0x%llx - 0x%llx in entry %p\n",
 					entry->va, entry->va + bo_size,
 					entry);
@@ -1452,7 +1465,7 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
 				goto map_bo_to_gpuvm_failed;
 			}
 
-			ret = vm_update_pds(vm, ctx.sync);
+			ret = vm_update_pds(avm, ctx.sync);
 			if (ret) {
 				pr_err("Failed to update page directories\n");
 				goto map_bo_to_gpuvm_failed;
@@ -1488,11 +1501,11 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
 }
 
 int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
-		struct kgd_dev *kgd, struct kgd_mem *mem, void *vm)
+		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv)
 {
 	struct amdgpu_device *adev = get_amdgpu_device(kgd);
-	struct amdkfd_process_info *process_info =
-		((struct amdgpu_vm *)vm)->process_info;
+	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
+	struct amdkfd_process_info *process_info = avm->process_info;
 	unsigned long bo_size = mem->bo->tbo.base.size;
 	struct kfd_bo_va_list *entry;
 	struct bo_vm_reservation_context ctx;
@@ -1500,7 +1513,7 @@ int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
 
 	mutex_lock(&mem->lock);
 
-	ret = reserve_bo_and_cond_vms(mem, vm, BO_VM_MAPPED, &ctx);
+	ret = reserve_bo_and_cond_vms(mem, avm, BO_VM_MAPPED, &ctx);
 	if (unlikely(ret))
 		goto out;
 	/* If no VMs were reserved, it means the BO wasn't actually mapped */
@@ -1509,17 +1522,17 @@ int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
 		goto unreserve_out;
 	}
 
-	ret = vm_validate_pt_pd_bos((struct amdgpu_vm *)vm);
+	ret = vm_validate_pt_pd_bos(avm);
 	if (unlikely(ret))
 		goto unreserve_out;
 
 	pr_debug("Unmap VA 0x%llx - 0x%llx from vm %p\n",
 		mem->va,
 		mem->va + bo_size * (1 + mem->aql_queue),
-		vm);
+		avm);
 
 	list_for_each_entry(entry, &mem->bo_va_list, bo_list) {
-		if (entry->bo_va->base.vm == vm && entry->is_mapped) {
+		if (entry->bo_va->base.vm == avm && entry->is_mapped) {
 			pr_debug("\t unmap VA 0x%llx - 0x%llx from entry %p\n",
 					entry->va,
 					entry->va + bo_size,
@@ -1645,14 +1658,14 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
 
 int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
 				      struct dma_buf *dma_buf,
-				      uint64_t va, void *vm,
+				      uint64_t va, void *drm_priv,
 				      struct kgd_mem **mem, uint64_t *size,
 				      uint64_t *mmap_offset)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)kgd;
+	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
 	struct drm_gem_object *obj;
 	struct amdgpu_bo *bo;
-	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
 
 	if (dma_buf->ops != &amdgpu_dmabuf_ops)
 		/* Can't handle non-graphics buffers */
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index d97e330a5022..bad0ecd6ef87 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1383,13 +1383,12 @@ int kfd_process_device_init_vm(struct kfd_process_device *pdd,
 
 	ret = amdgpu_amdkfd_gpuvm_acquire_process_vm(
 		dev->kgd, drm_file, p->pasid,
-		&pdd->vm, &p->kgd_process_info, &p->ef);
+		&p->kgd_process_info, &p->ef);
 	if (ret) {
 		pr_err("Failed to create process VM object\n");
 		return ret;
 	}
-
-	amdgpu_vm_set_task_info(pdd->vm);
+	pdd->vm = drm_file->private_data;
 
 	ret = kfd_process_device_reserve_ib_mem(pdd);
 	if (ret)
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] drm/amdkfd: Allow access for mmapping KFD BOs
  2021-04-07 23:12 [PATCH 1/4] drm/amdkfd: Remove legacy code not acquiring VMs Felix Kuehling
  2021-04-07 23:12 ` [PATCH 2/4] drm/amdkfd: Use drm_priv to pass VM from KFD to amdgpu Felix Kuehling
@ 2021-04-07 23:12 ` Felix Kuehling
  2021-04-14  6:43   ` Christian König
  2021-04-14 15:37   ` philip yang
  2021-04-07 23:12 ` [PATCH 4/4] drm/amdgpu: Remove verify_access shortcut for " Felix Kuehling
  2021-04-14 15:15 ` [PATCH 1/4] drm/amdkfd: Remove legacy code not acquiring VMs philip yang
  3 siblings, 2 replies; 11+ messages in thread
From: Felix Kuehling @ 2021-04-07 23:12 UTC (permalink / raw)
  To: amd-gfx, dri-devel; +Cc: christian.koenig, tzimmermann

DRM allows access automatically when it creates a GEM handle for a BO.
KFD BOs don't have GEM handles, so KFD needs to manage access manually.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  3 ++-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 19 ++++++++++++++++++-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |  8 +++++---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c      |  7 ++++---
 4 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 0d59bebd92af..7c8c5e469707 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -245,7 +245,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 		void *drm_priv, struct kgd_mem **mem,
 		uint64_t *offset, uint32_t flags);
 int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
-		struct kgd_dev *kgd, struct kgd_mem *mem, uint64_t *size);
+		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv,
+		uint64_t *size);
 int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
 		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv);
 int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 95442bcd60fb..e7d61ec966b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1232,6 +1232,12 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 			 domain_string(alloc_domain), ret);
 		goto err_bo_create;
 	}
+	ret = drm_vma_node_allow(&gobj->vma_node, drm_priv);
+	if (ret) {
+		pr_debug("Failed to allow vma node access. ret %d\n",
+			 ret);
+		goto err_node_allow;
+	}
 	bo = gem_to_amdgpu_bo(gobj);
 	if (bo_type == ttm_bo_type_sg) {
 		bo->tbo.sg = sg;
@@ -1261,6 +1267,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 
 allocate_init_user_pages_failed:
 	remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
+	drm_vma_node_revoke(&gobj->vma_node, drm_priv);
+err_node_allow:
 	amdgpu_bo_unref(&bo);
 	/* Don't unreserve system mem limit twice */
 	goto err_reserve_limit;
@@ -1278,7 +1286,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 }
 
 int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
-		struct kgd_dev *kgd, struct kgd_mem *mem, uint64_t *size)
+		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv,
+		uint64_t *size)
 {
 	struct amdkfd_process_info *process_info = mem->process_info;
 	unsigned long bo_size = mem->bo->tbo.base.size;
@@ -1355,6 +1364,7 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 	}
 
 	/* Free the BO*/
+	drm_vma_node_revoke(&mem->bo->tbo.base.vma_node, drm_priv);
 	drm_gem_object_put(&mem->bo->tbo.base);
 	mutex_destroy(&mem->lock);
 	kfree(mem);
@@ -1666,6 +1676,7 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
 	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
 	struct drm_gem_object *obj;
 	struct amdgpu_bo *bo;
+	int ret;
 
 	if (dma_buf->ops != &amdgpu_dmabuf_ops)
 		/* Can't handle non-graphics buffers */
@@ -1686,6 +1697,12 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
 	if (!*mem)
 		return -ENOMEM;
 
+	ret = drm_vma_node_allow(&obj->vma_node, drm_priv);
+	if (ret) {
+		kfree(mem);
+		return ret;
+	}
+
 	if (size)
 		*size = amdgpu_bo_size(bo);
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 43de260b2230..8fc18de7cff4 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1328,7 +1328,8 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
 	return 0;
 
 err_free:
-	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
+	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem,
+					       pdd->vm, NULL);
 err_unlock:
 	mutex_unlock(&p->mutex);
 	return err;
@@ -1365,7 +1366,7 @@ static int kfd_ioctl_free_memory_of_gpu(struct file *filep,
 	}
 
 	ret = amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd,
-						(struct kgd_mem *)mem, &size);
+					(struct kgd_mem *)mem, pdd->vm, &size);
 
 	/* If freeing the buffer failed, leave the handle in place for
 	 * clean-up during process tear-down.
@@ -1721,7 +1722,8 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
 	return 0;
 
 err_free:
-	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
+	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem,
+					       pdd->vm, NULL);
 err_unlock:
 	mutex_unlock(&p->mutex);
 	dma_buf_put(dmabuf);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index bad0ecd6ef87..da452407c4e5 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -648,7 +648,7 @@ static void kfd_process_free_gpuvm(struct kgd_mem *mem,
 	struct kfd_dev *dev = pdd->dev;
 
 	amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(dev->kgd, mem, pdd->vm);
-	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, mem, NULL);
+	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, mem, pdd->vm, NULL);
 }
 
 /* kfd_process_alloc_gpuvm - Allocate GPU VM for the KFD process
@@ -712,7 +712,7 @@ static int kfd_process_alloc_gpuvm(struct kfd_process_device *pdd,
 	return err;
 
 err_map_mem:
-	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->kgd, mem, NULL);
+	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->kgd, mem, pdd->vm, NULL);
 err_alloc_mem:
 	*kptr = NULL;
 	return err;
@@ -907,7 +907,8 @@ static void kfd_process_device_free_bos(struct kfd_process_device *pdd)
 				peer_pdd->dev->kgd, mem, peer_pdd->vm);
 		}
 
-		amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->kgd, mem, NULL);
+		amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->kgd, mem,
+						       pdd->vm, NULL);
 		kfd_process_device_remove_obj_handle(pdd, id);
 	}
 }
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] drm/amdgpu: Remove verify_access shortcut for KFD BOs
  2021-04-07 23:12 [PATCH 1/4] drm/amdkfd: Remove legacy code not acquiring VMs Felix Kuehling
  2021-04-07 23:12 ` [PATCH 2/4] drm/amdkfd: Use drm_priv to pass VM from KFD to amdgpu Felix Kuehling
  2021-04-07 23:12 ` [PATCH 3/4] drm/amdkfd: Allow access for mmapping KFD BOs Felix Kuehling
@ 2021-04-07 23:12 ` Felix Kuehling
  2021-04-14 15:38   ` philip yang
  2021-04-14 15:15 ` [PATCH 1/4] drm/amdkfd: Remove legacy code not acquiring VMs philip yang
  3 siblings, 1 reply; 11+ messages in thread
From: Felix Kuehling @ 2021-04-07 23:12 UTC (permalink / raw)
  To: amd-gfx, dri-devel; +Cc: christian.koenig, tzimmermann

This shortcut is no longer needed with access managed progerly by KFD.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 936b3cfdde55..4947dfe9aa70 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -165,13 +165,6 @@ static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
 	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
 
-	/*
-	 * Don't verify access for KFD BOs. They don't have a GEM
-	 * object associated with them.
-	 */
-	if (abo->kfd_bo)
-		return 0;
-
 	if (amdgpu_ttm_tt_get_usermm(bo->ttm))
 		return -EPERM;
 	return drm_vma_node_verify_access(&abo->tbo.base.vma_node,
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] drm/amdkfd: Allow access for mmapping KFD BOs
  2021-04-07 23:12 ` [PATCH 3/4] drm/amdkfd: Allow access for mmapping KFD BOs Felix Kuehling
@ 2021-04-14  6:43   ` Christian König
  2021-04-14 15:37   ` philip yang
  1 sibling, 0 replies; 11+ messages in thread
From: Christian König @ 2021-04-14  6:43 UTC (permalink / raw)
  To: Felix Kuehling, amd-gfx, dri-devel; +Cc: christian.koenig, tzimmermann

Am 08.04.21 um 01:12 schrieb Felix Kuehling:
> DRM allows access automatically when it creates a GEM handle for a BO.
> KFD BOs don't have GEM handles, so KFD needs to manage access manually.

Ok, double checking the code that makes sense.

> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  3 ++-
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 19 ++++++++++++++++++-
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |  8 +++++---
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c      |  7 ++++---
>   4 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 0d59bebd92af..7c8c5e469707 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -245,7 +245,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   		void *drm_priv, struct kgd_mem **mem,
>   		uint64_t *offset, uint32_t flags);
>   int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> -		struct kgd_dev *kgd, struct kgd_mem *mem, uint64_t *size);
> +		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv,
> +		uint64_t *size);
>   int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>   		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv);
>   int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 95442bcd60fb..e7d61ec966b6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1232,6 +1232,12 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   			 domain_string(alloc_domain), ret);
>   		goto err_bo_create;
>   	}
> +	ret = drm_vma_node_allow(&gobj->vma_node, drm_priv);
> +	if (ret) {
> +		pr_debug("Failed to allow vma node access. ret %d\n",
> +			 ret);
> +		goto err_node_allow;
> +	}
>   	bo = gem_to_amdgpu_bo(gobj);
>   	if (bo_type == ttm_bo_type_sg) {
>   		bo->tbo.sg = sg;
> @@ -1261,6 +1267,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   
>   allocate_init_user_pages_failed:
>   	remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
> +	drm_vma_node_revoke(&gobj->vma_node, drm_priv);
> +err_node_allow:
>   	amdgpu_bo_unref(&bo);
>   	/* Don't unreserve system mem limit twice */
>   	goto err_reserve_limit;
> @@ -1278,7 +1286,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   }
>   
>   int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> -		struct kgd_dev *kgd, struct kgd_mem *mem, uint64_t *size)
> +		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv,
> +		uint64_t *size)
>   {
>   	struct amdkfd_process_info *process_info = mem->process_info;
>   	unsigned long bo_size = mem->bo->tbo.base.size;
> @@ -1355,6 +1364,7 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
>   	}
>   
>   	/* Free the BO*/
> +	drm_vma_node_revoke(&mem->bo->tbo.base.vma_node, drm_priv);
>   	drm_gem_object_put(&mem->bo->tbo.base);
>   	mutex_destroy(&mem->lock);
>   	kfree(mem);
> @@ -1666,6 +1676,7 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
>   	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
>   	struct drm_gem_object *obj;
>   	struct amdgpu_bo *bo;
> +	int ret;
>   
>   	if (dma_buf->ops != &amdgpu_dmabuf_ops)
>   		/* Can't handle non-graphics buffers */
> @@ -1686,6 +1697,12 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
>   	if (!*mem)
>   		return -ENOMEM;
>   
> +	ret = drm_vma_node_allow(&obj->vma_node, drm_priv);
> +	if (ret) {
> +		kfree(mem);
> +		return ret;
> +	}
> +
>   	if (size)
>   		*size = amdgpu_bo_size(bo);
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 43de260b2230..8fc18de7cff4 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1328,7 +1328,8 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
>   	return 0;
>   
>   err_free:
> -	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
> +	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem,
> +					       pdd->vm, NULL);
>   err_unlock:
>   	mutex_unlock(&p->mutex);
>   	return err;
> @@ -1365,7 +1366,7 @@ static int kfd_ioctl_free_memory_of_gpu(struct file *filep,
>   	}
>   
>   	ret = amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd,
> -						(struct kgd_mem *)mem, &size);
> +					(struct kgd_mem *)mem, pdd->vm, &size);
>   
>   	/* If freeing the buffer failed, leave the handle in place for
>   	 * clean-up during process tear-down.
> @@ -1721,7 +1722,8 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
>   	return 0;
>   
>   err_free:
> -	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
> +	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem,
> +					       pdd->vm, NULL);
>   err_unlock:
>   	mutex_unlock(&p->mutex);
>   	dma_buf_put(dmabuf);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index bad0ecd6ef87..da452407c4e5 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -648,7 +648,7 @@ static void kfd_process_free_gpuvm(struct kgd_mem *mem,
>   	struct kfd_dev *dev = pdd->dev;
>   
>   	amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(dev->kgd, mem, pdd->vm);
> -	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, mem, NULL);
> +	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, mem, pdd->vm, NULL);
>   }
>   
>   /* kfd_process_alloc_gpuvm - Allocate GPU VM for the KFD process
> @@ -712,7 +712,7 @@ static int kfd_process_alloc_gpuvm(struct kfd_process_device *pdd,
>   	return err;
>   
>   err_map_mem:
> -	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->kgd, mem, NULL);
> +	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->kgd, mem, pdd->vm, NULL);
>   err_alloc_mem:
>   	*kptr = NULL;
>   	return err;
> @@ -907,7 +907,8 @@ static void kfd_process_device_free_bos(struct kfd_process_device *pdd)
>   				peer_pdd->dev->kgd, mem, peer_pdd->vm);
>   		}
>   
> -		amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->kgd, mem, NULL);
> +		amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->kgd, mem,
> +						       pdd->vm, NULL);
>   		kfd_process_device_remove_obj_handle(pdd, id);
>   	}
>   }

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] drm/amdkfd: Remove legacy code not acquiring VMs
  2021-04-07 23:12 [PATCH 1/4] drm/amdkfd: Remove legacy code not acquiring VMs Felix Kuehling
                   ` (2 preceding siblings ...)
  2021-04-07 23:12 ` [PATCH 4/4] drm/amdgpu: Remove verify_access shortcut for " Felix Kuehling
@ 2021-04-14 15:15 ` philip yang
  3 siblings, 0 replies; 11+ messages in thread
From: philip yang @ 2021-04-14 15:15 UTC (permalink / raw)
  To: Felix Kuehling, amd-gfx, dri-devel; +Cc: christian.koenig, tzimmermann

[-- Attachment #1: Type: text/html, Size: 6429 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] drm/amdkfd: Use drm_priv to pass VM from KFD to amdgpu
  2021-04-07 23:12 ` [PATCH 2/4] drm/amdkfd: Use drm_priv to pass VM from KFD to amdgpu Felix Kuehling
@ 2021-04-14 15:21   ` philip yang
  2021-04-14 15:47     ` Felix Kuehling
  0 siblings, 1 reply; 11+ messages in thread
From: philip yang @ 2021-04-14 15:21 UTC (permalink / raw)
  To: Felix Kuehling, amd-gfx, dri-devel; +Cc: christian.koenig, tzimmermann

[-- Attachment #1: Type: text/html, Size: 11298 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] drm/amdkfd: Allow access for mmapping KFD BOs
  2021-04-07 23:12 ` [PATCH 3/4] drm/amdkfd: Allow access for mmapping KFD BOs Felix Kuehling
  2021-04-14  6:43   ` Christian König
@ 2021-04-14 15:37   ` philip yang
  2021-04-14 15:46     ` Felix Kuehling
  1 sibling, 1 reply; 11+ messages in thread
From: philip yang @ 2021-04-14 15:37 UTC (permalink / raw)
  To: Felix Kuehling, amd-gfx, dri-devel; +Cc: christian.koenig, tzimmermann

[-- Attachment #1: Type: text/html, Size: 7503 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] drm/amdgpu: Remove verify_access shortcut for KFD BOs
  2021-04-07 23:12 ` [PATCH 4/4] drm/amdgpu: Remove verify_access shortcut for " Felix Kuehling
@ 2021-04-14 15:38   ` philip yang
  0 siblings, 0 replies; 11+ messages in thread
From: philip yang @ 2021-04-14 15:38 UTC (permalink / raw)
  To: Felix Kuehling, amd-gfx, dri-devel; +Cc: christian.koenig, tzimmermann

[-- Attachment #1: Type: text/html, Size: 1621 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] drm/amdkfd: Allow access for mmapping KFD BOs
  2021-04-14 15:37   ` philip yang
@ 2021-04-14 15:46     ` Felix Kuehling
  0 siblings, 0 replies; 11+ messages in thread
From: Felix Kuehling @ 2021-04-14 15:46 UTC (permalink / raw)
  To: philip yang, amd-gfx, dri-devel; +Cc: christian.koenig, tzimmermann

Am 2021-04-14 um 11:37 a.m. schrieb philip yang:
>
>
> On 2021-04-07 7:12 p.m., Felix Kuehling wrote:
>> DRM allows access automatically when it creates a GEM handle for a BO.
>> KFD BOs don't have GEM handles, so KFD needs to manage access manually.
>
> After reading drm vma manager, I understand it uses rbtree to store
> all GPU drm file handle when calling drm_vma_node_allow,
> drm_vma_node_is_allowed/drm_vma_node_verify_access is checked when
> creating mapping, I don't understand how application uses this, but
> seems we need call drm_vma_node_allow when
> amdgpu_amdkfd_gpuvm_map_memory_to_gpu, to add mapping GPUs drm file
> handle to vma manager.
>
The drm file handle is used for CPU mapping of BOs using mmap by the
Thunk. It uses on the file handle of the GPU where the BO was allocated.
It doesn't matter what other GPUs map the BO in their device page table.
Therefore we don't need to call drm_vma_node_allow in
amdgpu_amdkfd_gpuvm_map_memory_to_gpu.

I will add an explanation in the commit description about how user mode
depends on this access permission.

Regards,
  Felix


> Regards,
>
> Philip
>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  3 ++-
>>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 19 ++++++++++++++++++-
>>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |  8 +++++---
>>  drivers/gpu/drm/amd/amdkfd/kfd_process.c      |  7 ++++---
>>  4 files changed, 29 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>> index 0d59bebd92af..7c8c5e469707 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>> @@ -245,7 +245,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>  		void *drm_priv, struct kgd_mem **mem,
>>  		uint64_t *offset, uint32_t flags);
>>  int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
>> -		struct kgd_dev *kgd, struct kgd_mem *mem, uint64_t *size);
>> +		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv,
>> +		uint64_t *size);
>>  int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>>  		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv);
>>  int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> index 95442bcd60fb..e7d61ec966b6 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -1232,6 +1232,12 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>  			 domain_string(alloc_domain), ret);
>>  		goto err_bo_create;
>>  	}
>> +	ret = drm_vma_node_allow(&gobj->vma_node, drm_priv);
>> +	if (ret) {
>> +		pr_debug("Failed to allow vma node access. ret %d\n",
>> +			 ret);
>
> pr_debug("Failed to allow vma node access. ret %d\n", ret);
>
> It's within 80 columns.
>
> Philip
>
>> +		goto err_node_allow;
>> +	}
>>  	bo = gem_to_amdgpu_bo(gobj);
>>  	if (bo_type == ttm_bo_type_sg) {
>>  		bo->tbo.sg = sg;
>> @@ -1261,6 +1267,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>  
>>  allocate_init_user_pages_failed:
>>  	remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
>> +	drm_vma_node_revoke(&gobj->vma_node, drm_priv);
>> +err_node_allow:
>>  	amdgpu_bo_unref(&bo);
>>  	/* Don't unreserve system mem limit twice */
>>  	goto err_reserve_limit;
>> @@ -1278,7 +1286,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>  }
>>  
>>  int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
>> -		struct kgd_dev *kgd, struct kgd_mem *mem, uint64_t *size)
>> +		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv,
>> +		uint64_t *size)
>>  {
>>  	struct amdkfd_process_info *process_info = mem->process_info;
>>  	unsigned long bo_size = mem->bo->tbo.base.size;
>> @@ -1355,6 +1364,7 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
>>  	}
>>  
>>  	/* Free the BO*/
>> +	drm_vma_node_revoke(&mem->bo->tbo.base.vma_node, drm_priv);
>>  	drm_gem_object_put(&mem->bo->tbo.base);
>>  	mutex_destroy(&mem->lock);
>>  	kfree(mem);
>> @@ -1666,6 +1676,7 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
>>  	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
>>  	struct drm_gem_object *obj;
>>  	struct amdgpu_bo *bo;
>> +	int ret;
>>  
>>  	if (dma_buf->ops != &amdgpu_dmabuf_ops)
>>  		/* Can't handle non-graphics buffers */
>> @@ -1686,6 +1697,12 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
>>  	if (!*mem)
>>  		return -ENOMEM;
>>  
>> +	ret = drm_vma_node_allow(&obj->vma_node, drm_priv);
>> +	if (ret) {
>> +		kfree(mem);
>> +		return ret;
>> +	}
>> +
>>  	if (size)
>>  		*size = amdgpu_bo_size(bo);
>>  
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> index 43de260b2230..8fc18de7cff4 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> @@ -1328,7 +1328,8 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
>>  	return 0;
>>  
>>  err_free:
>> -	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
>> +	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem,
>> +					       pdd->vm, NULL);
>>  err_unlock:
>>  	mutex_unlock(&p->mutex);
>>  	return err;
>> @@ -1365,7 +1366,7 @@ static int kfd_ioctl_free_memory_of_gpu(struct file *filep,
>>  	}
>>  
>>  	ret = amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd,
>> -						(struct kgd_mem *)mem, &size);
>> +					(struct kgd_mem *)mem, pdd->vm, &size);
>>  
>>  	/* If freeing the buffer failed, leave the handle in place for
>>  	 * clean-up during process tear-down.
>> @@ -1721,7 +1722,8 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
>>  	return 0;
>>  
>>  err_free:
>> -	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
>> +	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem,
>> +					       pdd->vm, NULL);
>>  err_unlock:
>>  	mutex_unlock(&p->mutex);
>>  	dma_buf_put(dmabuf);
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> index bad0ecd6ef87..da452407c4e5 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> @@ -648,7 +648,7 @@ static void kfd_process_free_gpuvm(struct kgd_mem *mem,
>>  	struct kfd_dev *dev = pdd->dev;
>>  
>>  	amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(dev->kgd, mem, pdd->vm);
>> -	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, mem, NULL);
>> +	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, mem, pdd->vm, NULL);
>>  }
>>  
>>  /* kfd_process_alloc_gpuvm - Allocate GPU VM for the KFD process
>> @@ -712,7 +712,7 @@ static int kfd_process_alloc_gpuvm(struct kfd_process_device *pdd,
>>  	return err;
>>  
>>  err_map_mem:
>> -	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->kgd, mem, NULL);
>> +	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->kgd, mem, pdd->vm, NULL);
>>  err_alloc_mem:
>>  	*kptr = NULL;
>>  	return err;
>> @@ -907,7 +907,8 @@ static void kfd_process_device_free_bos(struct kfd_process_device *pdd)
>>  				peer_pdd->dev->kgd, mem, peer_pdd->vm);
>>  		}
>>  
>> -		amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->kgd, mem, NULL);
>> +		amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->kgd, mem,
>> +						       pdd->vm, NULL);
>>  		kfd_process_device_remove_obj_handle(pdd, id);
>>  	}
>>  }
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] drm/amdkfd: Use drm_priv to pass VM from KFD to amdgpu
  2021-04-14 15:21   ` philip yang
@ 2021-04-14 15:47     ` Felix Kuehling
  0 siblings, 0 replies; 11+ messages in thread
From: Felix Kuehling @ 2021-04-14 15:47 UTC (permalink / raw)
  To: philip yang, amd-gfx, dri-devel; +Cc: christian.koenig, tzimmermann

Am 2021-04-14 um 11:21 a.m. schrieb philip yang:
>
>
> On 2021-04-07 7:12 p.m., Felix Kuehling wrote:
>> amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu needs the drm_priv to allow mmap
>> to access the BO through the corresponding file descriptor.
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    | 14 ++--
>>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 69 +++++++++++--------
>>  drivers/gpu/drm/amd/amdkfd/kfd_process.c      |  5 +-
>>  3 files changed, 50 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>> index 5ffb07b02810..0d59bebd92af 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>> @@ -236,20 +236,20 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev *dst, struct kgd_dev *s
>>  /* GPUVM API */
>>  int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
>>  					struct file *filp, u32 pasid,
>> -					void **vm, void **process_info,
>> +					void **process_info,
>>  					struct dma_fence **ef);
>> -void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm);
>> -uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm);
>> +void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *drm_priv);
>> +uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *drm_priv);
>>  int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>  		struct kgd_dev *kgd, uint64_t va, uint64_t size,
>> -		void *vm, struct kgd_mem **mem,
>> +		void *drm_priv, struct kgd_mem **mem,
>>  		uint64_t *offset, uint32_t flags);
>>  int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
>>  		struct kgd_dev *kgd, struct kgd_mem *mem, uint64_t *size);
>>  int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>> -		struct kgd_dev *kgd, struct kgd_mem *mem, void *vm);
>> +		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv);
>>  int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
>> -		struct kgd_dev *kgd, struct kgd_mem *mem, void *vm);
>> +		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv);
>>  int amdgpu_amdkfd_gpuvm_sync_memory(
>>  		struct kgd_dev *kgd, struct kgd_mem *mem, bool intr);
>>  int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct kgd_dev *kgd,
>> @@ -260,7 +260,7 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
>>  					      struct kfd_vm_fault_info *info);
>>  int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
>>  				      struct dma_buf *dmabuf,
>> -				      uint64_t va, void *vm,
>> +				      uint64_t va, void *drm_priv,
>>  				      struct kgd_mem **mem, uint64_t *size,
>>  				      uint64_t *mmap_offset);
>>  int amdgpu_amdkfd_get_tile_config(struct kgd_dev *kgd,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> index 36012229ccc1..95442bcd60fb 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -951,6 +951,13 @@ static int process_update_pds(struct amdkfd_process_info *process_info,
>>  	return 0;
>>  }
>>  
>> +static struct amdgpu_vm *drm_priv_to_vm(struct drm_file *drm_priv)
>> +{
>> +	struct amdgpu_fpriv *fpriv = drm_priv->driver_priv;
>> +
>> +	return &fpriv->vm;
>> +}
>> +
>>  static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
>>  		       struct dma_fence **ef)
>>  {
>> @@ -1039,15 +1046,19 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
>>  
>>  int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
>>  					   struct file *filp, u32 pasid,
>> -					   void **vm, void **process_info,
>> +					   void **process_info,
>>  					   struct dma_fence **ef)
>>  {
>>  	struct amdgpu_device *adev = get_amdgpu_device(kgd);
>> -	struct drm_file *drm_priv = filp->private_data;
>> -	struct amdgpu_fpriv *drv_priv = drm_priv->driver_priv;
>> -	struct amdgpu_vm *avm = &drv_priv->vm;
>> +	struct amdgpu_fpriv *drv_priv;
>> +	struct amdgpu_vm *avm;
>>  	int ret;
>>  
>> +	ret = amdgpu_file_to_fpriv(filp, &drv_priv);
>> +	if (ret)
>> +		return ret;
>> +	avm = &drv_priv->vm;
>> +
>>  	/* Already a compute VM? */
>>  	if (avm->process_info)
>>  		return -EINVAL;
>> @@ -1062,7 +1073,7 @@ int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
>>  	if (ret)
>>  		return ret;
>>  
>> -	*vm = (void *)avm;
>> +	amdgpu_vm_set_task_info(avm);
>>  
>>  	return 0;
>>  }
>> @@ -1103,15 +1114,17 @@ void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
>>  	}
>>  }
>>  
>> -void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm)
>> +void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *drm_priv)
>>  {
>>  	struct amdgpu_device *adev = get_amdgpu_device(kgd);
>> -	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
>> +	struct amdgpu_vm *avm;
>>  
>> -	if (WARN_ON(!kgd || !vm))
>> +	if (WARN_ON(!kgd || !drm_priv))
>>  		return;
>>  
>> -	pr_debug("Releasing process vm %p\n", vm);
>> +	avm = drm_priv_to_vm(drm_priv);
>> +
>> +	pr_debug("Releasing process vm %p\n", avm);
>>  
>>  	/* The original pasid of amdgpu vm has already been
>>  	 * released during making a amdgpu vm to a compute vm
>> @@ -1122,9 +1135,9 @@ void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm)
>>  	amdgpu_vm_release_compute(adev, avm);
>>  }
>>  
>> -uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm)
>> +uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *drm_priv)
>>  {
>> -	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
>> +	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
>>  	struct amdgpu_bo *pd = avm->root.base.bo;
>>  	struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev);
>>  
>> @@ -1135,11 +1148,11 @@ uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm)
>>  
>>  int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>  		struct kgd_dev *kgd, uint64_t va, uint64_t size,
>> -		void *vm, struct kgd_mem **mem,
>> +		void *drm_priv, struct kgd_mem **mem,
>>  		uint64_t *offset, uint32_t flags)
>>  {
>>  	struct amdgpu_device *adev = get_amdgpu_device(kgd);
>> -	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
>> +	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
>>  	enum ttm_bo_type bo_type = ttm_bo_type_device;
>>  	struct sg_table *sg = NULL;
>>  	uint64_t user_addr = 0;
>> @@ -1350,10 +1363,10 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
>>  }
>>  
>>  int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>> -		struct kgd_dev *kgd, struct kgd_mem *mem, void *vm)
>> +		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv)
>>  {
>>  	struct amdgpu_device *adev = get_amdgpu_device(kgd);
>> -	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
>> +	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
>>  	int ret;
>>  	struct amdgpu_bo *bo;
>>  	uint32_t domain;
>> @@ -1394,9 +1407,9 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>>  	pr_debug("Map VA 0x%llx - 0x%llx to vm %p domain %s\n",
>>  			mem->va,
>>  			mem->va + bo_size * (1 + mem->aql_queue),
>> -			vm, domain_string(domain));
>> +			avm, domain_string(domain));
>>  
>> -	ret = reserve_bo_and_vm(mem, vm, &ctx);
>> +	ret = reserve_bo_and_vm(mem, avm, &ctx);
>>  	if (unlikely(ret))
>>  		goto out;
>>  
>> @@ -1440,7 +1453,7 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>>  	}
>>  
>>  	list_for_each_entry(entry, &mem->bo_va_list, bo_list) {
>> -		if (entry->bo_va->base.vm == vm && !entry->is_mapped) {
>> +		if (entry->bo_va->base.vm == avm && !entry->is_mapped) {
>>  			pr_debug("\t map VA 0x%llx - 0x%llx in entry %p\n",
>>  					entry->va, entry->va + bo_size,
>>  					entry);
>> @@ -1452,7 +1465,7 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>>  				goto map_bo_to_gpuvm_failed;
>>  			}
>>  
>> -			ret = vm_update_pds(vm, ctx.sync);
>> +			ret = vm_update_pds(avm, ctx.sync);
>>  			if (ret) {
>>  				pr_err("Failed to update page directories\n");
>>  				goto map_bo_to_gpuvm_failed;
>> @@ -1488,11 +1501,11 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>>  }
>>  
>>  int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
>> -		struct kgd_dev *kgd, struct kgd_mem *mem, void *vm)
>> +		struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv)
>>  {
>>  	struct amdgpu_device *adev = get_amdgpu_device(kgd);
>> -	struct amdkfd_process_info *process_info =
>> -		((struct amdgpu_vm *)vm)->process_info;
>> +	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
>> +	struct amdkfd_process_info *process_info = avm->process_info;
>>  	unsigned long bo_size = mem->bo->tbo.base.size;
>>  	struct kfd_bo_va_list *entry;
>>  	struct bo_vm_reservation_context ctx;
>> @@ -1500,7 +1513,7 @@ int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
>>  
>>  	mutex_lock(&mem->lock);
>>  
>> -	ret = reserve_bo_and_cond_vms(mem, vm, BO_VM_MAPPED, &ctx);
>> +	ret = reserve_bo_and_cond_vms(mem, avm, BO_VM_MAPPED, &ctx);
>>  	if (unlikely(ret))
>>  		goto out;
>>  	/* If no VMs were reserved, it means the BO wasn't actually mapped */
>> @@ -1509,17 +1522,17 @@ int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
>>  		goto unreserve_out;
>>  	}
>>  
>> -	ret = vm_validate_pt_pd_bos((struct amdgpu_vm *)vm);
>> +	ret = vm_validate_pt_pd_bos(avm);
>>  	if (unlikely(ret))
>>  		goto unreserve_out;
>>  
>>  	pr_debug("Unmap VA 0x%llx - 0x%llx from vm %p\n",
>>  		mem->va,
>>  		mem->va + bo_size * (1 + mem->aql_queue),
>> -		vm);
>> +		avm);
>>  
>>  	list_for_each_entry(entry, &mem->bo_va_list, bo_list) {
>> -		if (entry->bo_va->base.vm == vm && entry->is_mapped) {
>> +		if (entry->bo_va->base.vm == avm && entry->is_mapped) {
>>  			pr_debug("\t unmap VA 0x%llx - 0x%llx from entry %p\n",
>>  					entry->va,
>>  					entry->va + bo_size,
>> @@ -1645,14 +1658,14 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
>>  
>>  int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
>>  				      struct dma_buf *dma_buf,
>> -				      uint64_t va, void *vm,
>> +				      uint64_t va, void *drm_priv,
>>  				      struct kgd_mem **mem, uint64_t *size,
>>  				      uint64_t *mmap_offset)
>>  {
>>  	struct amdgpu_device *adev = (struct amdgpu_device *)kgd;
>> +	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
>>  	struct drm_gem_object *obj;
>>  	struct amdgpu_bo *bo;
>> -	struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
>>  
>>  	if (dma_buf->ops != &amdgpu_dmabuf_ops)
>>  		/* Can't handle non-graphics buffers */
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> index d97e330a5022..bad0ecd6ef87 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> @@ -1383,13 +1383,12 @@ int kfd_process_device_init_vm(struct kfd_process_device *pdd,
>>  
>>  	ret = amdgpu_amdkfd_gpuvm_acquire_process_vm(
>>  		dev->kgd, drm_file, p->pasid,
>> -		&pdd->vm, &p->kgd_process_info, &p->ef);
>> +		&p->kgd_process_info, &p->ef);
>>  	if (ret) {
>>  		pr_err("Failed to create process VM object\n");
>>  		return ret;
>>  	}
>> -
>> -	amdgpu_vm_set_task_info(pdd->vm);
>> +	pdd->vm = drm_file->private_data;
>>  
>
> Maybe it is better to read, rename pdd->vm to pdd->drm_priv as well?
>
I agree. I'll send out an update with that fixed.

Regards,
  Felix


> Philip
>
>>  	ret = kfd_process_device_reserve_ib_mem(pdd);
>>  	if (ret)
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-04-14 15:47 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-07 23:12 [PATCH 1/4] drm/amdkfd: Remove legacy code not acquiring VMs Felix Kuehling
2021-04-07 23:12 ` [PATCH 2/4] drm/amdkfd: Use drm_priv to pass VM from KFD to amdgpu Felix Kuehling
2021-04-14 15:21   ` philip yang
2021-04-14 15:47     ` Felix Kuehling
2021-04-07 23:12 ` [PATCH 3/4] drm/amdkfd: Allow access for mmapping KFD BOs Felix Kuehling
2021-04-14  6:43   ` Christian König
2021-04-14 15:37   ` philip yang
2021-04-14 15:46     ` Felix Kuehling
2021-04-07 23:12 ` [PATCH 4/4] drm/amdgpu: Remove verify_access shortcut for " Felix Kuehling
2021-04-14 15:38   ` philip yang
2021-04-14 15:15 ` [PATCH 1/4] drm/amdkfd: Remove legacy code not acquiring VMs philip yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).