All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/amdgpu: drm/amdkfd: split amdgpu_mn_register
@ 2021-03-03  9:25 Nirmoy Das
  2021-03-03  9:25 ` [PATCH v2 2/3] drm/amdgpu: introduce kfd user flag for amdgpu_bo Nirmoy Das
  2021-03-03  9:25 ` [PATCH v2 3/3] drm/amdgpu: drm/amdkfd: add amdgpu_kfd_bo struct Nirmoy Das
  0 siblings, 2 replies; 11+ messages in thread
From: Nirmoy Das @ 2021-03-03  9:25 UTC (permalink / raw)
  To: Christian.Koenig, Felix.Kuehling; +Cc: Nirmoy Das, amd-gfx

Split amdgpu_mn_register() into two functions to avoid unnecessary
bo->kfd_bo check.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c        | 21 +++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h        |  8 +++++++
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 99ad4e1d0896..89d0e4f7c6a8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -571,7 +571,7 @@ static int init_user_pages(struct kgd_mem *mem, uint64_t user_addr)
 		goto out;
 	}
 
-	ret = amdgpu_mn_register(bo, user_addr);
+	ret = amdgpu_mn_register_hsa(bo, user_addr);
 	if (ret) {
 		pr_err("%s: Failed to register MMU notifier: %d\n",
 		       __func__, ret);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index 828b5167ff12..1da67cf812b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -132,15 +132,28 @@ static const struct mmu_interval_notifier_ops amdgpu_mn_hsa_ops = {
  */
 int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr)
 {
-	if (bo->kfd_bo)
-		return mmu_interval_notifier_insert(&bo->notifier, current->mm,
-						    addr, amdgpu_bo_size(bo),
-						    &amdgpu_mn_hsa_ops);
 	return mmu_interval_notifier_insert(&bo->notifier, current->mm, addr,
 					    amdgpu_bo_size(bo),
 					    &amdgpu_mn_gfx_ops);
 }
 
+/**
+ * amdgpu_mn_register_hsa - register a BO for notifier updates
+ *
+ * @bo: amdgpu buffer object
+ * @addr: userptr addr we should monitor
+ *
+ * Registers a mmu_notifier for the given kfd BO at the specified address.
+ * Returns 0 on success, -ERRNO if anything goes wrong.
+ */
+
+int amdgpu_mn_register_hsa(struct amdgpu_bo *bo, unsigned long addr)
+{
+	return mmu_interval_notifier_insert(&bo->notifier, current->mm, addr,
+					    amdgpu_bo_size(bo),
+					    &amdgpu_mn_hsa_ops);
+}
+
 /**
  * amdgpu_mn_unregister - unregister a BO for notifier updates
  *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
index a292238f75eb..565ee5a0a3ad 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
@@ -32,6 +32,7 @@
 
 #if defined(CONFIG_HMM_MIRROR)
 int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr);
+int amdgpu_mn_register_hsa(struct amdgpu_bo *bo, unsigned long addr);
 void amdgpu_mn_unregister(struct amdgpu_bo *bo);
 #else
 static inline int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr)
@@ -40,6 +41,13 @@ static inline int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr)
 		      "add CONFIG_ZONE_DEVICE=y in config file to fix this\n");
 	return -ENODEV;
 }
+
+static inline int amdgpu_mn_register_hsa(struct amdgpu_bo *bo, unsigned long addr)
+{
+	DRM_WARN_ONCE("HMM_MIRROR kernel config option is not enabled, "
+		      "add CONFIG_ZONE_DEVICE=y in config file to fix this\n");
+	return -ENODEV;
+}
 static inline void amdgpu_mn_unregister(struct amdgpu_bo *bo) {}
 #endif
 
-- 
2.30.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 v2 2/3] drm/amdgpu: introduce kfd user flag for amdgpu_bo
  2021-03-03  9:25 [PATCH 1/3] drm/amdgpu: drm/amdkfd: split amdgpu_mn_register Nirmoy Das
@ 2021-03-03  9:25 ` Nirmoy Das
  2021-03-03 12:01   ` Christian König
  2021-03-03  9:25 ` [PATCH v2 3/3] drm/amdgpu: drm/amdkfd: add amdgpu_kfd_bo struct Nirmoy Das
  1 sibling, 1 reply; 11+ messages in thread
From: Nirmoy Das @ 2021-03-03  9:25 UTC (permalink / raw)
  To: Christian.Koenig, Felix.Kuehling; +Cc: Nirmoy Das, amd-gfx

Introduce a new flag for amdgpu_bo->flags to identify if
a BO is created by KFD.

v2: rename AMDGPU_GEM_USER_KFD -> AMDGPU_GEM_CREATE_KFD

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c       |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    | 48 ++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c       |  2 +-
 include/uapi/drm/amdgpu_drm.h                 |  5 ++
 6 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 89d0e4f7c6a8..57798707cd5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1227,7 +1227,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 	bp.flags = alloc_flags;
 	bp.type = bo_type;
 	bp.resv = NULL;
-	ret = amdgpu_bo_create(adev, &bp, &bo);
+	ret = amdgpu_kfd_bo_create(adev, &bp, &bo);
 	if (ret) {
 		pr_debug("Failed to create BO on domain %s. ret %d\n",
 				domain_string(alloc_domain), ret);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 8e9b8a6e6ef0..e0ceeb32642c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -234,7 +234,8 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
 		      AMDGPU_GEM_CREATE_VRAM_CLEARED |
 		      AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
 		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
-		      AMDGPU_GEM_CREATE_ENCRYPTED))
+		      AMDGPU_GEM_CREATE_ENCRYPTED |
+		      AMDGPU_GEM_CREATE_KFD))

 		return -EINVAL;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 0bd22ed1dacf..1b41b4870c99 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -697,6 +697,52 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 	return r;
 }

+/**
+ * amdgpu_kfd_bo_create - create an &amdgpu_bo buffer object with kfd user flag
+ * @adev: amdgpu device object
+ * @bp: parameters to be used for the buffer object
+ * @bo_ptr: pointer to the buffer object pointer
+ *
+ * Creates an &amdgpu_bo buffer object; and if requested, also creates a
+ * shadow object.
+ * Shadow object is used to backup the original buffer object, and is always
+ * in GTT.
+ *
+ * Returns:
+ * 0 for success or a negative error code on failure.
+ */
+
+int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
+			 struct amdgpu_bo_param *bp,
+			 struct amdgpu_bo **bo_ptr)
+{
+	u64 flags = bp->flags;
+	int r;
+
+	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
+	bp->flags = bp->flags | AMDGPU_GEM_CREATE_KFD;
+	r = amdgpu_bo_do_create(adev, bp, bo_ptr);
+	if (r)
+		return r;
+
+	if ((flags & AMDGPU_GEM_CREATE_SHADOW) && !(adev->flags & AMD_IS_APU)) {
+		if (!bp->resv)
+			WARN_ON(dma_resv_lock((*bo_ptr)->tbo.base.resv,
+							NULL));
+
+		r = amdgpu_bo_create_shadow(adev, bp->size, *bo_ptr);
+
+		if (!bp->resv)
+			dma_resv_unlock((*bo_ptr)->tbo.base.resv);
+
+		if (r)
+			amdgpu_bo_unref(bo_ptr);
+	}
+
+	return r;
+}
+
+
 /**
  * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
  * @bo: pointer to the buffer object
@@ -1309,7 +1355,7 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo)

 	abo = ttm_to_amdgpu_bo(bo);

-	if (abo->kfd_bo)
+	if (abo->flags & AMDGPU_GEM_CREATE_KFD)
 		amdgpu_amdkfd_unreserve_memory_limit(abo);

 	/* We only remove the fence if the resv has individualized. */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 8cd96c9330dd..665ee0015f06 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -245,6 +245,9 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain);
 int amdgpu_bo_create(struct amdgpu_device *adev,
 		     struct amdgpu_bo_param *bp,
 		     struct amdgpu_bo **bo_ptr);
+int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
+			 struct amdgpu_bo_param *bp,
+			 struct amdgpu_bo **bo_ptr);
 int amdgpu_bo_create_reserved(struct amdgpu_device *adev,
 			      unsigned long size, int align,
 			      u32 domain, struct amdgpu_bo **bo_ptr,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 7b2db779f313..030bec382f54 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -164,7 +164,7 @@ static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 	 * Don't verify access for KFD BOs. They don't have a GEM
 	 * object associated with them.
 	 */
-	if (abo->kfd_bo)
+	if (abo->flags & AMDGPU_GEM_CREATE_KFD)
 		return 0;

 	if (amdgpu_ttm_tt_get_usermm(bo->ttm))
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 8b832f7458f2..f510e8302228 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -142,6 +142,11 @@ extern "C" {
  */
 #define AMDGPU_GEM_CREATE_ENCRYPTED		(1 << 10)

+/* Flag that the allocating BO's user is KFD. It should never be used by
+ * user space applications
+ */
+#define AMDGPU_GEM_CREATE_KFD			(1 << 20)
+
 struct drm_amdgpu_gem_create_in  {
 	/** the requested memory size */
 	__u64 bo_size;
--
2.30.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 v2 3/3] drm/amdgpu: drm/amdkfd: add amdgpu_kfd_bo struct
  2021-03-03  9:25 [PATCH 1/3] drm/amdgpu: drm/amdkfd: split amdgpu_mn_register Nirmoy Das
  2021-03-03  9:25 ` [PATCH v2 2/3] drm/amdgpu: introduce kfd user flag for amdgpu_bo Nirmoy Das
@ 2021-03-03  9:25 ` Nirmoy Das
  2021-03-03 12:04   ` Christian König
  1 sibling, 1 reply; 11+ messages in thread
From: Nirmoy Das @ 2021-03-03  9:25 UTC (permalink / raw)
  To: Christian.Koenig, Felix.Kuehling; +Cc: Nirmoy Das, amd-gfx

Implement a new struct based on amdgpu_bo base class
for BOs created by kfd device so that kfd related memeber
of amdgpu_bo can be moved there.

v2: rename AMDGPU_GEM_USER_KFD -> AMDGPU_GEM_CREATE_KFD

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 10 ++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c        |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    | 32 ++++++++++++++-----
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    |  8 ++++-
 4 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 57798707cd5f..1f52ae4de609 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1152,6 +1152,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 	struct sg_table *sg = NULL;
 	uint64_t user_addr = 0;
 	struct amdgpu_bo *bo;
+	struct amdgpu_kfd_bo *kbo;
 	struct amdgpu_bo_param bp;
 	u32 domain, alloc_domain;
 	u64 alloc_flags;
@@ -1227,17 +1228,20 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 	bp.flags = alloc_flags;
 	bp.type = bo_type;
 	bp.resv = NULL;
-	ret = amdgpu_kfd_bo_create(adev, &bp, &bo);
+	ret = amdgpu_kfd_bo_create(adev, &bp, &kbo);
 	if (ret) {
 		pr_debug("Failed to create BO on domain %s. ret %d\n",
 				domain_string(alloc_domain), ret);
 		goto err_bo_create;
 	}
+
+	bo = &kbo->bo;
 	if (bo_type == ttm_bo_type_sg) {
 		bo->tbo.sg = sg;
 		bo->tbo.ttm->sg = sg;
 	}
-	bo->kfd_bo = *mem;
+
+	kbo->kfd_bo = *mem;
 	(*mem)->bo = bo;
 	if (user_addr)
 		bo->flags |= AMDGPU_AMDKFD_USERPTR_BO;
@@ -1261,7 +1265,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(

 allocate_init_user_pages_failed:
 	remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
-	amdgpu_bo_unref(&bo);
+	amdgpu_kfd_bo_unref(&kbo);
 	/* Don't unreserve system mem limit twice */
 	goto err_reserve_limit;
 err_bo_create:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index 1da67cf812b1..eaaf4940abcb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -102,6 +102,7 @@ static bool amdgpu_mn_invalidate_hsa(struct mmu_interval_notifier *mni,
 				     unsigned long cur_seq)
 {
 	struct amdgpu_bo *bo = container_of(mni, struct amdgpu_bo, notifier);
+	struct amdgpu_kfd_bo *kbo = container_of(bo, struct amdgpu_kfd_bo, bo);
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);

 	if (!mmu_notifier_range_blockable(range))
@@ -111,7 +112,7 @@ static bool amdgpu_mn_invalidate_hsa(struct mmu_interval_notifier *mni,

 	mmu_interval_set_seq(mni, cur_seq);

-	amdgpu_amdkfd_evict_userptr(bo->kfd_bo, bo->notifier.mm);
+	amdgpu_amdkfd_evict_userptr(kbo->kfd_bo, bo->notifier.mm);
 	mutex_unlock(&adev->notifier_lock);

 	return true;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 1b41b4870c99..787eb99119a2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -551,8 +551,10 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,

 	acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
 				       sizeof(struct amdgpu_bo));
+	if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
+		bp->bo_ptr_size = sizeof(struct amdgpu_bo);

-	bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
+	bo = kzalloc(bp->bo_ptr_size, GFP_KERNEL);
 	if (bo == NULL)
 		return -ENOMEM;
 	drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
@@ -714,35 +716,37 @@ int amdgpu_bo_create(struct amdgpu_device *adev,

 int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
 			 struct amdgpu_bo_param *bp,
-			 struct amdgpu_bo **bo_ptr)
+			 struct amdgpu_kfd_bo **kfd_bo_ptr)
 {
+	struct amdgpu_bo *bo_ptr;
 	u64 flags = bp->flags;
 	int r;

 	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
 	bp->flags = bp->flags | AMDGPU_GEM_CREATE_KFD;
-	r = amdgpu_bo_do_create(adev, bp, bo_ptr);
+	bp->bo_ptr_size = sizeof(struct amdgpu_kfd_bo);
+	r = amdgpu_bo_do_create(adev, bp, &bo_ptr);
 	if (r)
 		return r;

+	*kfd_bo_ptr = (struct amdgpu_kfd_bo *)bo_ptr;
 	if ((flags & AMDGPU_GEM_CREATE_SHADOW) && !(adev->flags & AMD_IS_APU)) {
 		if (!bp->resv)
-			WARN_ON(dma_resv_lock((*bo_ptr)->tbo.base.resv,
+			WARN_ON(dma_resv_lock((*kfd_bo_ptr)->bo.tbo.base.resv,
 							NULL));

-		r = amdgpu_bo_create_shadow(adev, bp->size, *bo_ptr);
+		r = amdgpu_bo_create_shadow(adev, bp->size, &(*kfd_bo_ptr)->bo);

 		if (!bp->resv)
-			dma_resv_unlock((*bo_ptr)->tbo.base.resv);
+			dma_resv_unlock((*kfd_bo_ptr)->bo.tbo.base.resv);

 		if (r)
-			amdgpu_bo_unref(bo_ptr);
+			amdgpu_kfd_bo_unref(kfd_bo_ptr);
 	}

 	return r;
 }

-
 /**
  * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
  * @bo: pointer to the buffer object
@@ -910,6 +914,18 @@ void amdgpu_bo_unref(struct amdgpu_bo **bo)
 	*bo = NULL;
 }

+void amdgpu_kfd_bo_unref(struct amdgpu_kfd_bo **kbo)
+{
+	struct ttm_buffer_object *tbo;
+
+	if ((*kbo) == NULL)
+		return;
+
+	tbo = &((*kbo)->bo.tbo);
+	ttm_bo_put(tbo);
+	*kbo = NULL;
+}
+
 /**
  * amdgpu_bo_pin_restricted - pin an &amdgpu_bo buffer object
  * @bo: &amdgpu_bo buffer object to be pinned
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 665ee0015f06..fa98a1fe2574 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -43,6 +43,8 @@ struct amdgpu_bo_param {
 	u32				domain;
 	u32				preferred_domain;
 	u64				flags;
+	/* size of a subclass using amdgpu_bo as base class */
+	u32				bo_ptr_size;
 	enum ttm_bo_type		type;
 	bool				no_wait_gpu;
 	struct dma_resv	*resv;
@@ -109,7 +111,10 @@ struct amdgpu_bo {
 #endif

 	struct list_head		shadow_list;
+};

+struct amdgpu_kfd_bo {
+	struct amdgpu_bo		bo;
 	struct kgd_mem                  *kfd_bo;
 };

@@ -247,7 +252,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 		     struct amdgpu_bo **bo_ptr);
 int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
 			 struct amdgpu_bo_param *bp,
-			 struct amdgpu_bo **bo_ptr);
+			 struct amdgpu_kfd_bo **bo_ptr);
 int amdgpu_bo_create_reserved(struct amdgpu_device *adev,
 			      unsigned long size, int align,
 			      u32 domain, struct amdgpu_bo **bo_ptr,
@@ -266,6 +271,7 @@ void *amdgpu_bo_kptr(struct amdgpu_bo *bo);
 void amdgpu_bo_kunmap(struct amdgpu_bo *bo);
 struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo);
 void amdgpu_bo_unref(struct amdgpu_bo **bo);
+void amdgpu_kfd_bo_unref(struct amdgpu_kfd_bo **kbo);
 int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain);
 int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
 			     u64 min_offset, u64 max_offset);
--
2.30.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 v2 2/3] drm/amdgpu: introduce kfd user flag for amdgpu_bo
  2021-03-03  9:25 ` [PATCH v2 2/3] drm/amdgpu: introduce kfd user flag for amdgpu_bo Nirmoy Das
@ 2021-03-03 12:01   ` Christian König
  2021-03-03 12:41     ` Nirmoy
  0 siblings, 1 reply; 11+ messages in thread
From: Christian König @ 2021-03-03 12:01 UTC (permalink / raw)
  To: Nirmoy Das, Christian.Koenig, Felix.Kuehling; +Cc: amd-gfx

Am 03.03.21 um 10:25 schrieb Nirmoy Das:
> Introduce a new flag for amdgpu_bo->flags to identify if
> a BO is created by KFD.
>
> v2: rename AMDGPU_GEM_USER_KFD -> AMDGPU_GEM_CREATE_KFD
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c       |  3 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    | 48 ++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    |  3 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c       |  2 +-
>   include/uapi/drm/amdgpu_drm.h                 |  5 ++
>   6 files changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 89d0e4f7c6a8..57798707cd5f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1227,7 +1227,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   	bp.flags = alloc_flags;
>   	bp.type = bo_type;
>   	bp.resv = NULL;
> -	ret = amdgpu_bo_create(adev, &bp, &bo);
> +	ret = amdgpu_kfd_bo_create(adev, &bp, &bo);
>   	if (ret) {
>   		pr_debug("Failed to create BO on domain %s. ret %d\n",
>   				domain_string(alloc_domain), ret);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 8e9b8a6e6ef0..e0ceeb32642c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -234,7 +234,8 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>   		      AMDGPU_GEM_CREATE_VRAM_CLEARED |
>   		      AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
>   		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
> -		      AMDGPU_GEM_CREATE_ENCRYPTED))
> +		      AMDGPU_GEM_CREATE_ENCRYPTED |
> +		      AMDGPU_GEM_CREATE_KFD))
>
>   		return -EINVAL;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 0bd22ed1dacf..1b41b4870c99 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -697,6 +697,52 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   	return r;
>   }
>
> +/**
> + * amdgpu_kfd_bo_create - create an &amdgpu_bo buffer object with kfd user flag
> + * @adev: amdgpu device object
> + * @bp: parameters to be used for the buffer object
> + * @bo_ptr: pointer to the buffer object pointer
> + *
> + * Creates an &amdgpu_bo buffer object; and if requested, also creates a
> + * shadow object.
> + * Shadow object is used to backup the original buffer object, and is always
> + * in GTT.
> + *
> + * Returns:
> + * 0 for success or a negative error code on failure.
> + */
> +
> +int amdgpu_kfd_bo_create(struct amdgpu_device *adev,

Please name this amdgpu_bo_create_kfd instead.

> +			 struct amdgpu_bo_param *bp,
> +			 struct amdgpu_bo **bo_ptr)
> +{
> +	u64 flags = bp->flags;
> +	int r;
> +
> +	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
> +	bp->flags = bp->flags | AMDGPU_GEM_CREATE_KFD;
> +	r = amdgpu_bo_do_create(adev, bp, bo_ptr);
> +	if (r)
> +		return r;
> +
> +	if ((flags & AMDGPU_GEM_CREATE_SHADOW) && !(adev->flags & AMD_IS_APU)) {
> +		if (!bp->resv)
> +			WARN_ON(dma_resv_lock((*bo_ptr)->tbo.base.resv,
> +							NULL));
> +
> +		r = amdgpu_bo_create_shadow(adev, bp->size, *bo_ptr);
> +
> +		if (!bp->resv)
> +			dma_resv_unlock((*bo_ptr)->tbo.base.resv);
> +
> +		if (r)
> +			amdgpu_bo_unref(bo_ptr);
> +	}

I don't think the KFD should ever have a reason to use the shadow buffer 
functionality.

> +
> +	return r;
> +}
> +
> +
>   /**
>    * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
>    * @bo: pointer to the buffer object
> @@ -1309,7 +1355,7 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo)
>
>   	abo = ttm_to_amdgpu_bo(bo);
>
> -	if (abo->kfd_bo)
> +	if (abo->flags & AMDGPU_GEM_CREATE_KFD)
>   		amdgpu_amdkfd_unreserve_memory_limit(abo);
>
>   	/* We only remove the fence if the resv has individualized. */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 8cd96c9330dd..665ee0015f06 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -245,6 +245,9 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain);
>   int amdgpu_bo_create(struct amdgpu_device *adev,
>   		     struct amdgpu_bo_param *bp,
>   		     struct amdgpu_bo **bo_ptr);
> +int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
> +			 struct amdgpu_bo_param *bp,
> +			 struct amdgpu_bo **bo_ptr);
>   int amdgpu_bo_create_reserved(struct amdgpu_device *adev,
>   			      unsigned long size, int align,
>   			      u32 domain, struct amdgpu_bo **bo_ptr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 7b2db779f313..030bec382f54 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -164,7 +164,7 @@ static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp)
>   	 * Don't verify access for KFD BOs. They don't have a GEM
>   	 * object associated with them.
>   	 */
> -	if (abo->kfd_bo)
> +	if (abo->flags & AMDGPU_GEM_CREATE_KFD)
>   		return 0;
>
>   	if (amdgpu_ttm_tt_get_usermm(bo->ttm))
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 8b832f7458f2..f510e8302228 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -142,6 +142,11 @@ extern "C" {
>    */
>   #define AMDGPU_GEM_CREATE_ENCRYPTED		(1 << 10)
>
> +/* Flag that the allocating BO's user is KFD. It should never be used by
> + * user space applications
> + */
> +#define AMDGPU_GEM_CREATE_KFD			(1 << 20)

Why 20? 11 is the next one here.

Christian.

> +
>   struct drm_amdgpu_gem_create_in  {
>   	/** the requested memory size */
>   	__u64 bo_size;
> --
> 2.30.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
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 v2 3/3] drm/amdgpu: drm/amdkfd: add amdgpu_kfd_bo struct
  2021-03-03  9:25 ` [PATCH v2 3/3] drm/amdgpu: drm/amdkfd: add amdgpu_kfd_bo struct Nirmoy Das
@ 2021-03-03 12:04   ` Christian König
  2021-03-03 13:06     ` Nirmoy
  2021-03-05  0:37     ` Felix Kuehling
  0 siblings, 2 replies; 11+ messages in thread
From: Christian König @ 2021-03-03 12:04 UTC (permalink / raw)
  To: Nirmoy Das, Christian.Koenig, Felix.Kuehling; +Cc: amd-gfx

Am 03.03.21 um 10:25 schrieb Nirmoy Das:
> Implement a new struct based on amdgpu_bo base class
> for BOs created by kfd device so that kfd related memeber
> of amdgpu_bo can be moved there.

You should probably restructure which patch has which code in it here.

The first one adds the general infrastructure and makes the necessary 
modification to allow allocating BO structures with different structure 
size.

And the second then adds the amdgpu_kfd_bo structure so that the KFD can 
use it.

You should also double check with Felix if we don't support importing 
BOs from elsewhere here and if that approach is correct.

Regards,
Christian.

>
> v2: rename AMDGPU_GEM_USER_KFD -> AMDGPU_GEM_CREATE_KFD
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 10 ++++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c        |  3 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    | 32 ++++++++++++++-----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    |  8 ++++-
>   4 files changed, 40 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 57798707cd5f..1f52ae4de609 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1152,6 +1152,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   	struct sg_table *sg = NULL;
>   	uint64_t user_addr = 0;
>   	struct amdgpu_bo *bo;
> +	struct amdgpu_kfd_bo *kbo;
>   	struct amdgpu_bo_param bp;
>   	u32 domain, alloc_domain;
>   	u64 alloc_flags;
> @@ -1227,17 +1228,20 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   	bp.flags = alloc_flags;
>   	bp.type = bo_type;
>   	bp.resv = NULL;
> -	ret = amdgpu_kfd_bo_create(adev, &bp, &bo);
> +	ret = amdgpu_kfd_bo_create(adev, &bp, &kbo);
>   	if (ret) {
>   		pr_debug("Failed to create BO on domain %s. ret %d\n",
>   				domain_string(alloc_domain), ret);
>   		goto err_bo_create;
>   	}
> +
> +	bo = &kbo->bo;
>   	if (bo_type == ttm_bo_type_sg) {
>   		bo->tbo.sg = sg;
>   		bo->tbo.ttm->sg = sg;
>   	}
> -	bo->kfd_bo = *mem;
> +
> +	kbo->kfd_bo = *mem;
>   	(*mem)->bo = bo;
>   	if (user_addr)
>   		bo->flags |= AMDGPU_AMDKFD_USERPTR_BO;
> @@ -1261,7 +1265,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>
>   allocate_init_user_pages_failed:
>   	remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
> -	amdgpu_bo_unref(&bo);
> +	amdgpu_kfd_bo_unref(&kbo);
>   	/* Don't unreserve system mem limit twice */
>   	goto err_reserve_limit;
>   err_bo_create:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> index 1da67cf812b1..eaaf4940abcb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> @@ -102,6 +102,7 @@ static bool amdgpu_mn_invalidate_hsa(struct mmu_interval_notifier *mni,
>   				     unsigned long cur_seq)
>   {
>   	struct amdgpu_bo *bo = container_of(mni, struct amdgpu_bo, notifier);
> +	struct amdgpu_kfd_bo *kbo = container_of(bo, struct amdgpu_kfd_bo, bo);
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>
>   	if (!mmu_notifier_range_blockable(range))
> @@ -111,7 +112,7 @@ static bool amdgpu_mn_invalidate_hsa(struct mmu_interval_notifier *mni,
>
>   	mmu_interval_set_seq(mni, cur_seq);
>
> -	amdgpu_amdkfd_evict_userptr(bo->kfd_bo, bo->notifier.mm);
> +	amdgpu_amdkfd_evict_userptr(kbo->kfd_bo, bo->notifier.mm);
>   	mutex_unlock(&adev->notifier_lock);
>
>   	return true;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 1b41b4870c99..787eb99119a2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -551,8 +551,10 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
>
>   	acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
>   				       sizeof(struct amdgpu_bo));
> +	if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
> +		bp->bo_ptr_size = sizeof(struct amdgpu_bo);
>
> -	bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
> +	bo = kzalloc(bp->bo_ptr_size, GFP_KERNEL);
>   	if (bo == NULL)
>   		return -ENOMEM;
>   	drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
> @@ -714,35 +716,37 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>
>   int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
>   			 struct amdgpu_bo_param *bp,
> -			 struct amdgpu_bo **bo_ptr)
> +			 struct amdgpu_kfd_bo **kfd_bo_ptr)
>   {
> +	struct amdgpu_bo *bo_ptr;
>   	u64 flags = bp->flags;
>   	int r;
>
>   	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
>   	bp->flags = bp->flags | AMDGPU_GEM_CREATE_KFD;
> -	r = amdgpu_bo_do_create(adev, bp, bo_ptr);
> +	bp->bo_ptr_size = sizeof(struct amdgpu_kfd_bo);
> +	r = amdgpu_bo_do_create(adev, bp, &bo_ptr);
>   	if (r)
>   		return r;
>
> +	*kfd_bo_ptr = (struct amdgpu_kfd_bo *)bo_ptr;
>   	if ((flags & AMDGPU_GEM_CREATE_SHADOW) && !(adev->flags & AMD_IS_APU)) {
>   		if (!bp->resv)
> -			WARN_ON(dma_resv_lock((*bo_ptr)->tbo.base.resv,
> +			WARN_ON(dma_resv_lock((*kfd_bo_ptr)->bo.tbo.base.resv,
>   							NULL));
>
> -		r = amdgpu_bo_create_shadow(adev, bp->size, *bo_ptr);
> +		r = amdgpu_bo_create_shadow(adev, bp->size, &(*kfd_bo_ptr)->bo);
>
>   		if (!bp->resv)
> -			dma_resv_unlock((*bo_ptr)->tbo.base.resv);
> +			dma_resv_unlock((*kfd_bo_ptr)->bo.tbo.base.resv);
>
>   		if (r)
> -			amdgpu_bo_unref(bo_ptr);
> +			amdgpu_kfd_bo_unref(kfd_bo_ptr);
>   	}
>
>   	return r;
>   }
>
> -
>   /**
>    * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
>    * @bo: pointer to the buffer object
> @@ -910,6 +914,18 @@ void amdgpu_bo_unref(struct amdgpu_bo **bo)
>   	*bo = NULL;
>   }
>
> +void amdgpu_kfd_bo_unref(struct amdgpu_kfd_bo **kbo)
> +{
> +	struct ttm_buffer_object *tbo;
> +
> +	if ((*kbo) == NULL)
> +		return;
> +
> +	tbo = &((*kbo)->bo.tbo);
> +	ttm_bo_put(tbo);
> +	*kbo = NULL;
> +}
> +
>   /**
>    * amdgpu_bo_pin_restricted - pin an &amdgpu_bo buffer object
>    * @bo: &amdgpu_bo buffer object to be pinned
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 665ee0015f06..fa98a1fe2574 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -43,6 +43,8 @@ struct amdgpu_bo_param {
>   	u32				domain;
>   	u32				preferred_domain;
>   	u64				flags;
> +	/* size of a subclass using amdgpu_bo as base class */
> +	u32				bo_ptr_size;
>   	enum ttm_bo_type		type;
>   	bool				no_wait_gpu;
>   	struct dma_resv	*resv;
> @@ -109,7 +111,10 @@ struct amdgpu_bo {
>   #endif
>
>   	struct list_head		shadow_list;
> +};
>
> +struct amdgpu_kfd_bo {
> +	struct amdgpu_bo		bo;
>   	struct kgd_mem                  *kfd_bo;
>   };
>
> @@ -247,7 +252,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   		     struct amdgpu_bo **bo_ptr);
>   int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
>   			 struct amdgpu_bo_param *bp,
> -			 struct amdgpu_bo **bo_ptr);
> +			 struct amdgpu_kfd_bo **bo_ptr);
>   int amdgpu_bo_create_reserved(struct amdgpu_device *adev,
>   			      unsigned long size, int align,
>   			      u32 domain, struct amdgpu_bo **bo_ptr,
> @@ -266,6 +271,7 @@ void *amdgpu_bo_kptr(struct amdgpu_bo *bo);
>   void amdgpu_bo_kunmap(struct amdgpu_bo *bo);
>   struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo);
>   void amdgpu_bo_unref(struct amdgpu_bo **bo);
> +void amdgpu_kfd_bo_unref(struct amdgpu_kfd_bo **kbo);
>   int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain);
>   int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
>   			     u64 min_offset, u64 max_offset);
> --
> 2.30.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
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 v2 2/3] drm/amdgpu: introduce kfd user flag for amdgpu_bo
  2021-03-03 12:01   ` Christian König
@ 2021-03-03 12:41     ` Nirmoy
  2021-03-03 12:46       ` Christian König
  0 siblings, 1 reply; 11+ messages in thread
From: Nirmoy @ 2021-03-03 12:41 UTC (permalink / raw)
  To: Christian König, Nirmoy Das, Christian.Koenig, Felix.Kuehling
  Cc: amd-gfx


On 3/3/21 1:01 PM, Christian König wrote:
> Am 03.03.21 um 10:25 schrieb Nirmoy Das:
>> Introduce a new flag for amdgpu_bo->flags to identify if
>> a BO is created by KFD.
>>
>> v2: rename AMDGPU_GEM_USER_KFD -> AMDGPU_GEM_CREATE_KFD
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c       |  3 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    | 48 ++++++++++++++++++-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    |  3 ++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c       |  2 +-
>>   include/uapi/drm/amdgpu_drm.h                 |  5 ++
>>   6 files changed, 59 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> index 89d0e4f7c6a8..57798707cd5f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -1227,7 +1227,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>       bp.flags = alloc_flags;
>>       bp.type = bo_type;
>>       bp.resv = NULL;
>> -    ret = amdgpu_bo_create(adev, &bp, &bo);
>> +    ret = amdgpu_kfd_bo_create(adev, &bp, &bo);
>>       if (ret) {
>>           pr_debug("Failed to create BO on domain %s. ret %d\n",
>>                   domain_string(alloc_domain), ret);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index 8e9b8a6e6ef0..e0ceeb32642c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -234,7 +234,8 @@ int amdgpu_gem_create_ioctl(struct drm_device 
>> *dev, void *data,
>>                 AMDGPU_GEM_CREATE_VRAM_CLEARED |
>>                 AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
>>                 AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
>> -              AMDGPU_GEM_CREATE_ENCRYPTED))
>> +              AMDGPU_GEM_CREATE_ENCRYPTED |
>> +              AMDGPU_GEM_CREATE_KFD))
>>
>>           return -EINVAL;
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 0bd22ed1dacf..1b41b4870c99 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -697,6 +697,52 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>>       return r;
>>   }
>>
>> +/**
>> + * amdgpu_kfd_bo_create - create an &amdgpu_bo buffer object with 
>> kfd user flag
>> + * @adev: amdgpu device object
>> + * @bp: parameters to be used for the buffer object
>> + * @bo_ptr: pointer to the buffer object pointer
>> + *
>> + * Creates an &amdgpu_bo buffer object; and if requested, also 
>> creates a
>> + * shadow object.
>> + * Shadow object is used to backup the original buffer object, and 
>> is always
>> + * in GTT.
>> + *
>> + * Returns:
>> + * 0 for success or a negative error code on failure.
>> + */
>> +
>> +int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
>
> Please name this amdgpu_bo_create_kfd instead.


Ok I will rename it.


>
>> +             struct amdgpu_bo_param *bp,
>> +             struct amdgpu_bo **bo_ptr)
>> +{
>> +    u64 flags = bp->flags;
>> +    int r;
>> +
>> +    bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
>> +    bp->flags = bp->flags | AMDGPU_GEM_CREATE_KFD;
>> +    r = amdgpu_bo_do_create(adev, bp, bo_ptr);
>> +    if (r)
>> +        return r;
>> +
>> +    if ((flags & AMDGPU_GEM_CREATE_SHADOW) && !(adev->flags & 
>> AMD_IS_APU)) {
>> +        if (!bp->resv)
>> +            WARN_ON(dma_resv_lock((*bo_ptr)->tbo.base.resv,
>> +                            NULL));
>> +
>> +        r = amdgpu_bo_create_shadow(adev, bp->size, *bo_ptr);
>> +
>> +        if (!bp->resv)
>> +            dma_resv_unlock((*bo_ptr)->tbo.base.resv);
>> +
>> +        if (r)
>> +            amdgpu_bo_unref(bo_ptr);
>> +    }
>
> I don't think the KFD should ever have a reason to use the shadow 
> buffer functionality.


This is interesting, I didn't know. I will remove 
amdgpu_bo_create_shadow().


>
>> +
>> +    return r;
>> +}
>> +
>> +
>>   /**
>>    * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
>>    * @bo: pointer to the buffer object
>> @@ -1309,7 +1355,7 @@ void amdgpu_bo_release_notify(struct 
>> ttm_buffer_object *bo)
>>
>>       abo = ttm_to_amdgpu_bo(bo);
>>
>> -    if (abo->kfd_bo)
>> +    if (abo->flags & AMDGPU_GEM_CREATE_KFD)
>>           amdgpu_amdkfd_unreserve_memory_limit(abo);
>>
>>       /* We only remove the fence if the resv has individualized. */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> index 8cd96c9330dd..665ee0015f06 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> @@ -245,6 +245,9 @@ void amdgpu_bo_placement_from_domain(struct 
>> amdgpu_bo *abo, u32 domain);
>>   int amdgpu_bo_create(struct amdgpu_device *adev,
>>                struct amdgpu_bo_param *bp,
>>                struct amdgpu_bo **bo_ptr);
>> +int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
>> +             struct amdgpu_bo_param *bp,
>> +             struct amdgpu_bo **bo_ptr);
>>   int amdgpu_bo_create_reserved(struct amdgpu_device *adev,
>>                     unsigned long size, int align,
>>                     u32 domain, struct amdgpu_bo **bo_ptr,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 7b2db779f313..030bec382f54 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -164,7 +164,7 @@ static int amdgpu_verify_access(struct 
>> ttm_buffer_object *bo, struct file *filp)
>>        * Don't verify access for KFD BOs. They don't have a GEM
>>        * object associated with them.
>>        */
>> -    if (abo->kfd_bo)
>> +    if (abo->flags & AMDGPU_GEM_CREATE_KFD)
>>           return 0;
>>
>>       if (amdgpu_ttm_tt_get_usermm(bo->ttm))
>> diff --git a/include/uapi/drm/amdgpu_drm.h 
>> b/include/uapi/drm/amdgpu_drm.h
>> index 8b832f7458f2..f510e8302228 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -142,6 +142,11 @@ extern "C" {
>>    */
>>   #define AMDGPU_GEM_CREATE_ENCRYPTED        (1 << 10)
>>
>> +/* Flag that the allocating BO's user is KFD. It should never be 
>> used by
>> + * user space applications
>> + */
>> +#define AMDGPU_GEM_CREATE_KFD            (1 << 20)
>
> Why 20? 11 is the next one here.


I feel BO owner flag is different than others so wanted to reserve some 
bits for grouping.

I can assign it to 11 if that makes more sense.


Thanks,

Nirmoy



>
> Christian.
>
>> +
>>   struct drm_amdgpu_gem_create_in  {
>>       /** the requested memory size */
>>       __u64 bo_size;
>> -- 
>> 2.30.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cnirmoy.das%40amd.com%7C5c41ee9032df45e36f1508d8de3c0c57%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503696776437773%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=1Lbaor5CuBUsnxr%2BQgB6zDYbRQVPWogth7gpIOhYRFI%3D&amp;reserved=0 
>>
>
_______________________________________________
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 v2 2/3] drm/amdgpu: introduce kfd user flag for amdgpu_bo
  2021-03-03 12:41     ` Nirmoy
@ 2021-03-03 12:46       ` Christian König
  0 siblings, 0 replies; 11+ messages in thread
From: Christian König @ 2021-03-03 12:46 UTC (permalink / raw)
  To: Nirmoy, Christian König, Nirmoy Das, Felix.Kuehling; +Cc: amd-gfx



Am 03.03.21 um 13:41 schrieb Nirmoy:
> [SNIP]
>>
>>> +             struct amdgpu_bo_param *bp,
>>> +             struct amdgpu_bo **bo_ptr)
>>> +{
>>> +    u64 flags = bp->flags;
>>> +    int r;
>>> +
>>> +    bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
>>> +    bp->flags = bp->flags | AMDGPU_GEM_CREATE_KFD;
>>> +    r = amdgpu_bo_do_create(adev, bp, bo_ptr);
>>> +    if (r)
>>> +        return r;
>>> +
>>> +    if ((flags & AMDGPU_GEM_CREATE_SHADOW) && !(adev->flags & 
>>> AMD_IS_APU)) {
>>> +        if (!bp->resv)
>>> + WARN_ON(dma_resv_lock((*bo_ptr)->tbo.base.resv,
>>> +                            NULL));
>>> +
>>> +        r = amdgpu_bo_create_shadow(adev, bp->size, *bo_ptr);
>>> +
>>> +        if (!bp->resv)
>>> +            dma_resv_unlock((*bo_ptr)->tbo.base.resv);
>>> +
>>> +        if (r)
>>> +            amdgpu_bo_unref(bo_ptr);
>>> +    }
>>
>> I don't think the KFD should ever have a reason to use the shadow 
>> buffer functionality.
>
>
> This is interesting, I didn't know. I will remove 
> amdgpu_bo_create_shadow().

Well the VM code is using the shadow stuff.

>
>
>>
>>> +
>>> +    return r;
>>> +}
>>> +
>>> +
>>>   /**
>>>    * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
>>>    * @bo: pointer to the buffer object
>>> @@ -1309,7 +1355,7 @@ void amdgpu_bo_release_notify(struct 
>>> ttm_buffer_object *bo)
>>>
>>>       abo = ttm_to_amdgpu_bo(bo);
>>>
>>> -    if (abo->kfd_bo)
>>> +    if (abo->flags & AMDGPU_GEM_CREATE_KFD)
>>>           amdgpu_amdkfd_unreserve_memory_limit(abo);
>>>
>>>       /* We only remove the fence if the resv has individualized. */
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> index 8cd96c9330dd..665ee0015f06 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> @@ -245,6 +245,9 @@ void amdgpu_bo_placement_from_domain(struct 
>>> amdgpu_bo *abo, u32 domain);
>>>   int amdgpu_bo_create(struct amdgpu_device *adev,
>>>                struct amdgpu_bo_param *bp,
>>>                struct amdgpu_bo **bo_ptr);
>>> +int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
>>> +             struct amdgpu_bo_param *bp,
>>> +             struct amdgpu_bo **bo_ptr);
>>>   int amdgpu_bo_create_reserved(struct amdgpu_device *adev,
>>>                     unsigned long size, int align,
>>>                     u32 domain, struct amdgpu_bo **bo_ptr,
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 7b2db779f313..030bec382f54 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -164,7 +164,7 @@ static int amdgpu_verify_access(struct 
>>> ttm_buffer_object *bo, struct file *filp)
>>>        * Don't verify access for KFD BOs. They don't have a GEM
>>>        * object associated with them.
>>>        */
>>> -    if (abo->kfd_bo)
>>> +    if (abo->flags & AMDGPU_GEM_CREATE_KFD)
>>>           return 0;
>>>
>>>       if (amdgpu_ttm_tt_get_usermm(bo->ttm))
>>> diff --git a/include/uapi/drm/amdgpu_drm.h 
>>> b/include/uapi/drm/amdgpu_drm.h
>>> index 8b832f7458f2..f510e8302228 100644
>>> --- a/include/uapi/drm/amdgpu_drm.h
>>> +++ b/include/uapi/drm/amdgpu_drm.h
>>> @@ -142,6 +142,11 @@ extern "C" {
>>>    */
>>>   #define AMDGPU_GEM_CREATE_ENCRYPTED        (1 << 10)
>>>
>>> +/* Flag that the allocating BO's user is KFD. It should never be 
>>> used by
>>> + * user space applications
>>> + */
>>> +#define AMDGPU_GEM_CREATE_KFD            (1 << 20)
>>
>> Why 20? 11 is the next one here.
>
>
> I feel BO owner flag is different than others so wanted to reserve 
> some bits for grouping.
>
> I can assign it to 11 if that makes more sense.

We already have other flags which userspace is not allowed to use, so 
this is nothing special here.

Christian.

>
>
> Thanks,
>
> Nirmoy
>
>
>
>>
>> Christian.
>>
>>> +
>>>   struct drm_amdgpu_gem_create_in  {
>>>       /** the requested memory size */
>>>       __u64 bo_size;
>>> -- 
>>> 2.30.1
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cnirmoy.das%40amd.com%7C5c41ee9032df45e36f1508d8de3c0c57%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503696776437773%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=1Lbaor5CuBUsnxr%2BQgB6zDYbRQVPWogth7gpIOhYRFI%3D&amp;reserved=0 
>>>
>>

_______________________________________________
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 v2 3/3] drm/amdgpu: drm/amdkfd: add amdgpu_kfd_bo struct
  2021-03-03 12:04   ` Christian König
@ 2021-03-03 13:06     ` Nirmoy
  2021-03-05  0:37     ` Felix Kuehling
  1 sibling, 0 replies; 11+ messages in thread
From: Nirmoy @ 2021-03-03 13:06 UTC (permalink / raw)
  To: Christian König, Nirmoy Das, Christian.Koenig, Felix.Kuehling
  Cc: amd-gfx


On 3/3/21 1:04 PM, Christian König wrote:
> Am 03.03.21 um 10:25 schrieb Nirmoy Das:
>> Implement a new struct based on amdgpu_bo base class
>> for BOs created by kfd device so that kfd related memeber
>> of amdgpu_bo can be moved there.
>
> You should probably restructure which patch has which code in it here.
>
> The first one adds the general infrastructure and makes the necessary 
> modification to allow allocating BO structures with different 
> structure size.
>
> And the second then adds the amdgpu_kfd_bo structure so that the KFD 
> can use it.


Thanks, I will split this into two.


>
> You should also double check with Felix if we don't support importing 
> BOs from elsewhere here and if that approach is correct.


Waiting for Felix to come back from vacation.


Nirmoy




>
> Regards,
> Christian.
>
>>
>> v2: rename AMDGPU_GEM_USER_KFD -> AMDGPU_GEM_CREATE_KFD
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 10 ++++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c        |  3 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    | 32 ++++++++++++++-----
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    |  8 ++++-
>>   4 files changed, 40 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> index 57798707cd5f..1f52ae4de609 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -1152,6 +1152,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>       struct sg_table *sg = NULL;
>>       uint64_t user_addr = 0;
>>       struct amdgpu_bo *bo;
>> +    struct amdgpu_kfd_bo *kbo;
>>       struct amdgpu_bo_param bp;
>>       u32 domain, alloc_domain;
>>       u64 alloc_flags;
>> @@ -1227,17 +1228,20 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>       bp.flags = alloc_flags;
>>       bp.type = bo_type;
>>       bp.resv = NULL;
>> -    ret = amdgpu_kfd_bo_create(adev, &bp, &bo);
>> +    ret = amdgpu_kfd_bo_create(adev, &bp, &kbo);
>>       if (ret) {
>>           pr_debug("Failed to create BO on domain %s. ret %d\n",
>>                   domain_string(alloc_domain), ret);
>>           goto err_bo_create;
>>       }
>> +
>> +    bo = &kbo->bo;
>>       if (bo_type == ttm_bo_type_sg) {
>>           bo->tbo.sg = sg;
>>           bo->tbo.ttm->sg = sg;
>>       }
>> -    bo->kfd_bo = *mem;
>> +
>> +    kbo->kfd_bo = *mem;
>>       (*mem)->bo = bo;
>>       if (user_addr)
>>           bo->flags |= AMDGPU_AMDKFD_USERPTR_BO;
>> @@ -1261,7 +1265,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>
>>   allocate_init_user_pages_failed:
>>       remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
>> -    amdgpu_bo_unref(&bo);
>> +    amdgpu_kfd_bo_unref(&kbo);
>>       /* Don't unreserve system mem limit twice */
>>       goto err_reserve_limit;
>>   err_bo_create:
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>> index 1da67cf812b1..eaaf4940abcb 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>> @@ -102,6 +102,7 @@ static bool amdgpu_mn_invalidate_hsa(struct 
>> mmu_interval_notifier *mni,
>>                        unsigned long cur_seq)
>>   {
>>       struct amdgpu_bo *bo = container_of(mni, struct amdgpu_bo, 
>> notifier);
>> +    struct amdgpu_kfd_bo *kbo = container_of(bo, struct 
>> amdgpu_kfd_bo, bo);
>>       struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>
>>       if (!mmu_notifier_range_blockable(range))
>> @@ -111,7 +112,7 @@ static bool amdgpu_mn_invalidate_hsa(struct 
>> mmu_interval_notifier *mni,
>>
>>       mmu_interval_set_seq(mni, cur_seq);
>>
>> -    amdgpu_amdkfd_evict_userptr(bo->kfd_bo, bo->notifier.mm);
>> +    amdgpu_amdkfd_evict_userptr(kbo->kfd_bo, bo->notifier.mm);
>>       mutex_unlock(&adev->notifier_lock);
>>
>>       return true;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 1b41b4870c99..787eb99119a2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -551,8 +551,10 @@ static int amdgpu_bo_do_create(struct 
>> amdgpu_device *adev,
>>
>>       acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
>>                          sizeof(struct amdgpu_bo));
>> +    if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
>> +        bp->bo_ptr_size = sizeof(struct amdgpu_bo);
>>
>> -    bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
>> +    bo = kzalloc(bp->bo_ptr_size, GFP_KERNEL);
>>       if (bo == NULL)
>>           return -ENOMEM;
>>       drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, 
>> size);
>> @@ -714,35 +716,37 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>>
>>   int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
>>                struct amdgpu_bo_param *bp,
>> -             struct amdgpu_bo **bo_ptr)
>> +             struct amdgpu_kfd_bo **kfd_bo_ptr)
>>   {
>> +    struct amdgpu_bo *bo_ptr;
>>       u64 flags = bp->flags;
>>       int r;
>>
>>       bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
>>       bp->flags = bp->flags | AMDGPU_GEM_CREATE_KFD;
>> -    r = amdgpu_bo_do_create(adev, bp, bo_ptr);
>> +    bp->bo_ptr_size = sizeof(struct amdgpu_kfd_bo);
>> +    r = amdgpu_bo_do_create(adev, bp, &bo_ptr);
>>       if (r)
>>           return r;
>>
>> +    *kfd_bo_ptr = (struct amdgpu_kfd_bo *)bo_ptr;
>>       if ((flags & AMDGPU_GEM_CREATE_SHADOW) && !(adev->flags & 
>> AMD_IS_APU)) {
>>           if (!bp->resv)
>> -            WARN_ON(dma_resv_lock((*bo_ptr)->tbo.base.resv,
>> + WARN_ON(dma_resv_lock((*kfd_bo_ptr)->bo.tbo.base.resv,
>>                               NULL));
>>
>> -        r = amdgpu_bo_create_shadow(adev, bp->size, *bo_ptr);
>> +        r = amdgpu_bo_create_shadow(adev, bp->size, 
>> &(*kfd_bo_ptr)->bo);
>>
>>           if (!bp->resv)
>> -            dma_resv_unlock((*bo_ptr)->tbo.base.resv);
>> + dma_resv_unlock((*kfd_bo_ptr)->bo.tbo.base.resv);
>>
>>           if (r)
>> -            amdgpu_bo_unref(bo_ptr);
>> +            amdgpu_kfd_bo_unref(kfd_bo_ptr);
>>       }
>>
>>       return r;
>>   }
>>
>> -
>>   /**
>>    * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
>>    * @bo: pointer to the buffer object
>> @@ -910,6 +914,18 @@ void amdgpu_bo_unref(struct amdgpu_bo **bo)
>>       *bo = NULL;
>>   }
>>
>> +void amdgpu_kfd_bo_unref(struct amdgpu_kfd_bo **kbo)
>> +{
>> +    struct ttm_buffer_object *tbo;
>> +
>> +    if ((*kbo) == NULL)
>> +        return;
>> +
>> +    tbo = &((*kbo)->bo.tbo);
>> +    ttm_bo_put(tbo);
>> +    *kbo = NULL;
>> +}
>> +
>>   /**
>>    * amdgpu_bo_pin_restricted - pin an &amdgpu_bo buffer object
>>    * @bo: &amdgpu_bo buffer object to be pinned
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> index 665ee0015f06..fa98a1fe2574 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> @@ -43,6 +43,8 @@ struct amdgpu_bo_param {
>>       u32                domain;
>>       u32                preferred_domain;
>>       u64                flags;
>> +    /* size of a subclass using amdgpu_bo as base class */
>> +    u32                bo_ptr_size;
>>       enum ttm_bo_type        type;
>>       bool                no_wait_gpu;
>>       struct dma_resv    *resv;
>> @@ -109,7 +111,10 @@ struct amdgpu_bo {
>>   #endif
>>
>>       struct list_head        shadow_list;
>> +};
>>
>> +struct amdgpu_kfd_bo {
>> +    struct amdgpu_bo        bo;
>>       struct kgd_mem                  *kfd_bo;
>>   };
>>
>> @@ -247,7 +252,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>>                struct amdgpu_bo **bo_ptr);
>>   int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
>>                struct amdgpu_bo_param *bp,
>> -             struct amdgpu_bo **bo_ptr);
>> +             struct amdgpu_kfd_bo **bo_ptr);
>>   int amdgpu_bo_create_reserved(struct amdgpu_device *adev,
>>                     unsigned long size, int align,
>>                     u32 domain, struct amdgpu_bo **bo_ptr,
>> @@ -266,6 +271,7 @@ void *amdgpu_bo_kptr(struct amdgpu_bo *bo);
>>   void amdgpu_bo_kunmap(struct amdgpu_bo *bo);
>>   struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo);
>>   void amdgpu_bo_unref(struct amdgpu_bo **bo);
>> +void amdgpu_kfd_bo_unref(struct amdgpu_kfd_bo **kbo);
>>   int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain);
>>   int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
>>                    u64 min_offset, u64 max_offset);
>> -- 
>> 2.30.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cnirmoy.das%40amd.com%7C96a11e5487d445a1aed608d8de3c801c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503698719785279%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=5ZHeoPPXcqfNiEBe%2FM64jEnTUnnGS34PfblzkKIp%2FkE%3D&amp;reserved=0 
>>
>
_______________________________________________
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 v2 3/3] drm/amdgpu: drm/amdkfd: add amdgpu_kfd_bo struct
  2021-03-03 12:04   ` Christian König
  2021-03-03 13:06     ` Nirmoy
@ 2021-03-05  0:37     ` Felix Kuehling
  2021-03-05  9:13       ` Nirmoy
  1 sibling, 1 reply; 11+ messages in thread
From: Felix Kuehling @ 2021-03-05  0:37 UTC (permalink / raw)
  To: Christian König, Nirmoy Das, Christian.Koenig; +Cc: amd-gfx

Am 2021-03-03 um 7:04 a.m. schrieb Christian König:
> Am 03.03.21 um 10:25 schrieb Nirmoy Das:
>> Implement a new struct based on amdgpu_bo base class
>> for BOs created by kfd device so that kfd related memeber
>> of amdgpu_bo can be moved there.
>
> You should probably restructure which patch has which code in it here.
>
> The first one adds the general infrastructure and makes the necessary
> modification to allow allocating BO structures with different
> structure size.
>
> And the second then adds the amdgpu_kfd_bo structure so that the KFD
> can use it.
>
> You should also double check with Felix if we don't support importing
> BOs from elsewhere here and if that approach is correct.

We do support importing graphics BOs into KFD processes.

Regards,
  Felix


>
> Regards,
> Christian.
>
>>
>> v2: rename AMDGPU_GEM_USER_KFD -> AMDGPU_GEM_CREATE_KFD
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 10 ++++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c        |  3 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    | 32 ++++++++++++++-----
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    |  8 ++++-
>>   4 files changed, 40 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> index 57798707cd5f..1f52ae4de609 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -1152,6 +1152,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>       struct sg_table *sg = NULL;
>>       uint64_t user_addr = 0;
>>       struct amdgpu_bo *bo;
>> +    struct amdgpu_kfd_bo *kbo;
>>       struct amdgpu_bo_param bp;
>>       u32 domain, alloc_domain;
>>       u64 alloc_flags;
>> @@ -1227,17 +1228,20 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>       bp.flags = alloc_flags;
>>       bp.type = bo_type;
>>       bp.resv = NULL;
>> -    ret = amdgpu_kfd_bo_create(adev, &bp, &bo);
>> +    ret = amdgpu_kfd_bo_create(adev, &bp, &kbo);
>>       if (ret) {
>>           pr_debug("Failed to create BO on domain %s. ret %d\n",
>>                   domain_string(alloc_domain), ret);
>>           goto err_bo_create;
>>       }
>> +
>> +    bo = &kbo->bo;
>>       if (bo_type == ttm_bo_type_sg) {
>>           bo->tbo.sg = sg;
>>           bo->tbo.ttm->sg = sg;
>>       }
>> -    bo->kfd_bo = *mem;
>> +
>> +    kbo->kfd_bo = *mem;
>>       (*mem)->bo = bo;
>>       if (user_addr)
>>           bo->flags |= AMDGPU_AMDKFD_USERPTR_BO;
>> @@ -1261,7 +1265,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>
>>   allocate_init_user_pages_failed:
>>       remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
>> -    amdgpu_bo_unref(&bo);
>> +    amdgpu_kfd_bo_unref(&kbo);
>>       /* Don't unreserve system mem limit twice */
>>       goto err_reserve_limit;
>>   err_bo_create:
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>> index 1da67cf812b1..eaaf4940abcb 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>> @@ -102,6 +102,7 @@ static bool amdgpu_mn_invalidate_hsa(struct
>> mmu_interval_notifier *mni,
>>                        unsigned long cur_seq)
>>   {
>>       struct amdgpu_bo *bo = container_of(mni, struct amdgpu_bo,
>> notifier);
>> +    struct amdgpu_kfd_bo *kbo = container_of(bo, struct
>> amdgpu_kfd_bo, bo);
>>       struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>
>>       if (!mmu_notifier_range_blockable(range))
>> @@ -111,7 +112,7 @@ static bool amdgpu_mn_invalidate_hsa(struct
>> mmu_interval_notifier *mni,
>>
>>       mmu_interval_set_seq(mni, cur_seq);
>>
>> -    amdgpu_amdkfd_evict_userptr(bo->kfd_bo, bo->notifier.mm);
>> +    amdgpu_amdkfd_evict_userptr(kbo->kfd_bo, bo->notifier.mm);
>>       mutex_unlock(&adev->notifier_lock);
>>
>>       return true;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 1b41b4870c99..787eb99119a2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -551,8 +551,10 @@ static int amdgpu_bo_do_create(struct
>> amdgpu_device *adev,
>>
>>       acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
>>                          sizeof(struct amdgpu_bo));
>> +    if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
>> +        bp->bo_ptr_size = sizeof(struct amdgpu_bo);
>>
>> -    bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
>> +    bo = kzalloc(bp->bo_ptr_size, GFP_KERNEL);
>>       if (bo == NULL)
>>           return -ENOMEM;
>>       drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base,
>> size);
>> @@ -714,35 +716,37 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>>
>>   int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
>>                struct amdgpu_bo_param *bp,
>> -             struct amdgpu_bo **bo_ptr)
>> +             struct amdgpu_kfd_bo **kfd_bo_ptr)
>>   {
>> +    struct amdgpu_bo *bo_ptr;
>>       u64 flags = bp->flags;
>>       int r;
>>
>>       bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
>>       bp->flags = bp->flags | AMDGPU_GEM_CREATE_KFD;
>> -    r = amdgpu_bo_do_create(adev, bp, bo_ptr);
>> +    bp->bo_ptr_size = sizeof(struct amdgpu_kfd_bo);
>> +    r = amdgpu_bo_do_create(adev, bp, &bo_ptr);
>>       if (r)
>>           return r;
>>
>> +    *kfd_bo_ptr = (struct amdgpu_kfd_bo *)bo_ptr;
>>       if ((flags & AMDGPU_GEM_CREATE_SHADOW) && !(adev->flags &
>> AMD_IS_APU)) {
>>           if (!bp->resv)
>> -            WARN_ON(dma_resv_lock((*bo_ptr)->tbo.base.resv,
>> +            WARN_ON(dma_resv_lock((*kfd_bo_ptr)->bo.tbo.base.resv,
>>                               NULL));
>>
>> -        r = amdgpu_bo_create_shadow(adev, bp->size, *bo_ptr);
>> +        r = amdgpu_bo_create_shadow(adev, bp->size,
>> &(*kfd_bo_ptr)->bo);
>>
>>           if (!bp->resv)
>> -            dma_resv_unlock((*bo_ptr)->tbo.base.resv);
>> +            dma_resv_unlock((*kfd_bo_ptr)->bo.tbo.base.resv);
>>
>>           if (r)
>> -            amdgpu_bo_unref(bo_ptr);
>> +            amdgpu_kfd_bo_unref(kfd_bo_ptr);
>>       }
>>
>>       return r;
>>   }
>>
>> -
>>   /**
>>    * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
>>    * @bo: pointer to the buffer object
>> @@ -910,6 +914,18 @@ void amdgpu_bo_unref(struct amdgpu_bo **bo)
>>       *bo = NULL;
>>   }
>>
>> +void amdgpu_kfd_bo_unref(struct amdgpu_kfd_bo **kbo)
>> +{
>> +    struct ttm_buffer_object *tbo;
>> +
>> +    if ((*kbo) == NULL)
>> +        return;
>> +
>> +    tbo = &((*kbo)->bo.tbo);
>> +    ttm_bo_put(tbo);
>> +    *kbo = NULL;
>> +}
>> +
>>   /**
>>    * amdgpu_bo_pin_restricted - pin an &amdgpu_bo buffer object
>>    * @bo: &amdgpu_bo buffer object to be pinned
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> index 665ee0015f06..fa98a1fe2574 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> @@ -43,6 +43,8 @@ struct amdgpu_bo_param {
>>       u32                domain;
>>       u32                preferred_domain;
>>       u64                flags;
>> +    /* size of a subclass using amdgpu_bo as base class */
>> +    u32                bo_ptr_size;
>>       enum ttm_bo_type        type;
>>       bool                no_wait_gpu;
>>       struct dma_resv    *resv;
>> @@ -109,7 +111,10 @@ struct amdgpu_bo {
>>   #endif
>>
>>       struct list_head        shadow_list;
>> +};
>>
>> +struct amdgpu_kfd_bo {
>> +    struct amdgpu_bo        bo;
>>       struct kgd_mem                  *kfd_bo;
>>   };
>>
>> @@ -247,7 +252,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>>                struct amdgpu_bo **bo_ptr);
>>   int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
>>                struct amdgpu_bo_param *bp,
>> -             struct amdgpu_bo **bo_ptr);
>> +             struct amdgpu_kfd_bo **bo_ptr);
>>   int amdgpu_bo_create_reserved(struct amdgpu_device *adev,
>>                     unsigned long size, int align,
>>                     u32 domain, struct amdgpu_bo **bo_ptr,
>> @@ -266,6 +271,7 @@ void *amdgpu_bo_kptr(struct amdgpu_bo *bo);
>>   void amdgpu_bo_kunmap(struct amdgpu_bo *bo);
>>   struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo);
>>   void amdgpu_bo_unref(struct amdgpu_bo **bo);
>> +void amdgpu_kfd_bo_unref(struct amdgpu_kfd_bo **kbo);
>>   int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain);
>>   int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
>>                    u64 min_offset, u64 max_offset);
>> -- 
>> 2.30.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
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 v2 3/3] drm/amdgpu: drm/amdkfd: add amdgpu_kfd_bo struct
  2021-03-05  0:37     ` Felix Kuehling
@ 2021-03-05  9:13       ` Nirmoy
  0 siblings, 0 replies; 11+ messages in thread
From: Nirmoy @ 2021-03-05  9:13 UTC (permalink / raw)
  To: Felix Kuehling, Christian König, Nirmoy Das, Christian.Koenig
  Cc: amd-gfx


On 3/5/21 1:37 AM, Felix Kuehling wrote:
> Am 2021-03-03 um 7:04 a.m. schrieb Christian König:
>> Am 03.03.21 um 10:25 schrieb Nirmoy Das:
>>> Implement a new struct based on amdgpu_bo base class
>>> for BOs created by kfd device so that kfd related memeber
>>> of amdgpu_bo can be moved there.
>> You should probably restructure which patch has which code in it here.
>>
>> The first one adds the general infrastructure and makes the necessary
>> modification to allow allocating BO structures with different
>> structure size.
>>
>> And the second then adds the amdgpu_kfd_bo structure so that the KFD
>> can use it.
>>
>> You should also double check with Felix if we don't support importing
>> BOs from elsewhere here and if that approach is correct.
> We do support importing graphics BOs into KFD processes.


Ok so this approach is not going to work then.


Thanks,

Nirmoy


>
> Regards,
>    Felix
>
>
>> Regards,
>> Christian.
>>
>>> v2: rename AMDGPU_GEM_USER_KFD -> AMDGPU_GEM_CREATE_KFD
>>>
>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>> ---
>>>    .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 10 ++++--
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c        |  3 +-
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    | 32 ++++++++++++++-----
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    |  8 ++++-
>>>    4 files changed, 40 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> index 57798707cd5f..1f52ae4de609 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> @@ -1152,6 +1152,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>>        struct sg_table *sg = NULL;
>>>        uint64_t user_addr = 0;
>>>        struct amdgpu_bo *bo;
>>> +    struct amdgpu_kfd_bo *kbo;
>>>        struct amdgpu_bo_param bp;
>>>        u32 domain, alloc_domain;
>>>        u64 alloc_flags;
>>> @@ -1227,17 +1228,20 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>>        bp.flags = alloc_flags;
>>>        bp.type = bo_type;
>>>        bp.resv = NULL;
>>> -    ret = amdgpu_kfd_bo_create(adev, &bp, &bo);
>>> +    ret = amdgpu_kfd_bo_create(adev, &bp, &kbo);
>>>        if (ret) {
>>>            pr_debug("Failed to create BO on domain %s. ret %d\n",
>>>                    domain_string(alloc_domain), ret);
>>>            goto err_bo_create;
>>>        }
>>> +
>>> +    bo = &kbo->bo;
>>>        if (bo_type == ttm_bo_type_sg) {
>>>            bo->tbo.sg = sg;
>>>            bo->tbo.ttm->sg = sg;
>>>        }
>>> -    bo->kfd_bo = *mem;
>>> +
>>> +    kbo->kfd_bo = *mem;
>>>        (*mem)->bo = bo;
>>>        if (user_addr)
>>>            bo->flags |= AMDGPU_AMDKFD_USERPTR_BO;
>>> @@ -1261,7 +1265,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>>>
>>>    allocate_init_user_pages_failed:
>>>        remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
>>> -    amdgpu_bo_unref(&bo);
>>> +    amdgpu_kfd_bo_unref(&kbo);
>>>        /* Don't unreserve system mem limit twice */
>>>        goto err_reserve_limit;
>>>    err_bo_create:
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>>> index 1da67cf812b1..eaaf4940abcb 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>>> @@ -102,6 +102,7 @@ static bool amdgpu_mn_invalidate_hsa(struct
>>> mmu_interval_notifier *mni,
>>>                         unsigned long cur_seq)
>>>    {
>>>        struct amdgpu_bo *bo = container_of(mni, struct amdgpu_bo,
>>> notifier);
>>> +    struct amdgpu_kfd_bo *kbo = container_of(bo, struct
>>> amdgpu_kfd_bo, bo);
>>>        struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>>
>>>        if (!mmu_notifier_range_blockable(range))
>>> @@ -111,7 +112,7 @@ static bool amdgpu_mn_invalidate_hsa(struct
>>> mmu_interval_notifier *mni,
>>>
>>>        mmu_interval_set_seq(mni, cur_seq);
>>>
>>> -    amdgpu_amdkfd_evict_userptr(bo->kfd_bo, bo->notifier.mm);
>>> +    amdgpu_amdkfd_evict_userptr(kbo->kfd_bo, bo->notifier.mm);
>>>        mutex_unlock(&adev->notifier_lock);
>>>
>>>        return true;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> index 1b41b4870c99..787eb99119a2 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> @@ -551,8 +551,10 @@ static int amdgpu_bo_do_create(struct
>>> amdgpu_device *adev,
>>>
>>>        acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
>>>                           sizeof(struct amdgpu_bo));
>>> +    if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
>>> +        bp->bo_ptr_size = sizeof(struct amdgpu_bo);
>>>
>>> -    bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
>>> +    bo = kzalloc(bp->bo_ptr_size, GFP_KERNEL);
>>>        if (bo == NULL)
>>>            return -ENOMEM;
>>>        drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base,
>>> size);
>>> @@ -714,35 +716,37 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>>>
>>>    int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
>>>                 struct amdgpu_bo_param *bp,
>>> -             struct amdgpu_bo **bo_ptr)
>>> +             struct amdgpu_kfd_bo **kfd_bo_ptr)
>>>    {
>>> +    struct amdgpu_bo *bo_ptr;
>>>        u64 flags = bp->flags;
>>>        int r;
>>>
>>>        bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
>>>        bp->flags = bp->flags | AMDGPU_GEM_CREATE_KFD;
>>> -    r = amdgpu_bo_do_create(adev, bp, bo_ptr);
>>> +    bp->bo_ptr_size = sizeof(struct amdgpu_kfd_bo);
>>> +    r = amdgpu_bo_do_create(adev, bp, &bo_ptr);
>>>        if (r)
>>>            return r;
>>>
>>> +    *kfd_bo_ptr = (struct amdgpu_kfd_bo *)bo_ptr;
>>>        if ((flags & AMDGPU_GEM_CREATE_SHADOW) && !(adev->flags &
>>> AMD_IS_APU)) {
>>>            if (!bp->resv)
>>> -            WARN_ON(dma_resv_lock((*bo_ptr)->tbo.base.resv,
>>> +            WARN_ON(dma_resv_lock((*kfd_bo_ptr)->bo.tbo.base.resv,
>>>                                NULL));
>>>
>>> -        r = amdgpu_bo_create_shadow(adev, bp->size, *bo_ptr);
>>> +        r = amdgpu_bo_create_shadow(adev, bp->size,
>>> &(*kfd_bo_ptr)->bo);
>>>
>>>            if (!bp->resv)
>>> -            dma_resv_unlock((*bo_ptr)->tbo.base.resv);
>>> +            dma_resv_unlock((*kfd_bo_ptr)->bo.tbo.base.resv);
>>>
>>>            if (r)
>>> -            amdgpu_bo_unref(bo_ptr);
>>> +            amdgpu_kfd_bo_unref(kfd_bo_ptr);
>>>        }
>>>
>>>        return r;
>>>    }
>>>
>>> -
>>>    /**
>>>     * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
>>>     * @bo: pointer to the buffer object
>>> @@ -910,6 +914,18 @@ void amdgpu_bo_unref(struct amdgpu_bo **bo)
>>>        *bo = NULL;
>>>    }
>>>
>>> +void amdgpu_kfd_bo_unref(struct amdgpu_kfd_bo **kbo)
>>> +{
>>> +    struct ttm_buffer_object *tbo;
>>> +
>>> +    if ((*kbo) == NULL)
>>> +        return;
>>> +
>>> +    tbo = &((*kbo)->bo.tbo);
>>> +    ttm_bo_put(tbo);
>>> +    *kbo = NULL;
>>> +}
>>> +
>>>    /**
>>>     * amdgpu_bo_pin_restricted - pin an &amdgpu_bo buffer object
>>>     * @bo: &amdgpu_bo buffer object to be pinned
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> index 665ee0015f06..fa98a1fe2574 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> @@ -43,6 +43,8 @@ struct amdgpu_bo_param {
>>>        u32                domain;
>>>        u32                preferred_domain;
>>>        u64                flags;
>>> +    /* size of a subclass using amdgpu_bo as base class */
>>> +    u32                bo_ptr_size;
>>>        enum ttm_bo_type        type;
>>>        bool                no_wait_gpu;
>>>        struct dma_resv    *resv;
>>> @@ -109,7 +111,10 @@ struct amdgpu_bo {
>>>    #endif
>>>
>>>        struct list_head        shadow_list;
>>> +};
>>>
>>> +struct amdgpu_kfd_bo {
>>> +    struct amdgpu_bo        bo;
>>>        struct kgd_mem                  *kfd_bo;
>>>    };
>>>
>>> @@ -247,7 +252,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>>>                 struct amdgpu_bo **bo_ptr);
>>>    int amdgpu_kfd_bo_create(struct amdgpu_device *adev,
>>>                 struct amdgpu_bo_param *bp,
>>> -             struct amdgpu_bo **bo_ptr);
>>> +             struct amdgpu_kfd_bo **bo_ptr);
>>>    int amdgpu_bo_create_reserved(struct amdgpu_device *adev,
>>>                      unsigned long size, int align,
>>>                      u32 domain, struct amdgpu_bo **bo_ptr,
>>> @@ -266,6 +271,7 @@ void *amdgpu_bo_kptr(struct amdgpu_bo *bo);
>>>    void amdgpu_bo_kunmap(struct amdgpu_bo *bo);
>>>    struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo);
>>>    void amdgpu_bo_unref(struct amdgpu_bo **bo);
>>> +void amdgpu_kfd_bo_unref(struct amdgpu_kfd_bo **kbo);
>>>    int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain);
>>>    int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
>>>                     u64 min_offset, u64 max_offset);
>>> -- 
>>> 2.30.1
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
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

* [PATCH 1/3] drm/amdgpu: drm/amdkfd: split amdgpu_mn_register
@ 2021-03-02 11:33 Nirmoy Das
  0 siblings, 0 replies; 11+ messages in thread
From: Nirmoy Das @ 2021-03-02 11:33 UTC (permalink / raw)
  To: Christian.Koenig, Felix.Kuehling; +Cc: Nirmoy Das, amd-gfx

Split amdgpu_mn_register() into two functions to avoid unnecessary
bo->kfd_bo check.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c        | 21 +++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h        |  8 +++++++
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 99ad4e1d0896..89d0e4f7c6a8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -571,7 +571,7 @@ static int init_user_pages(struct kgd_mem *mem, uint64_t user_addr)
 		goto out;
 	}
 
-	ret = amdgpu_mn_register(bo, user_addr);
+	ret = amdgpu_mn_register_hsa(bo, user_addr);
 	if (ret) {
 		pr_err("%s: Failed to register MMU notifier: %d\n",
 		       __func__, ret);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index 828b5167ff12..1da67cf812b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -132,15 +132,28 @@ static const struct mmu_interval_notifier_ops amdgpu_mn_hsa_ops = {
  */
 int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr)
 {
-	if (bo->kfd_bo)
-		return mmu_interval_notifier_insert(&bo->notifier, current->mm,
-						    addr, amdgpu_bo_size(bo),
-						    &amdgpu_mn_hsa_ops);
 	return mmu_interval_notifier_insert(&bo->notifier, current->mm, addr,
 					    amdgpu_bo_size(bo),
 					    &amdgpu_mn_gfx_ops);
 }
 
+/**
+ * amdgpu_mn_register_hsa - register a BO for notifier updates
+ *
+ * @bo: amdgpu buffer object
+ * @addr: userptr addr we should monitor
+ *
+ * Registers a mmu_notifier for the given kfd BO at the specified address.
+ * Returns 0 on success, -ERRNO if anything goes wrong.
+ */
+
+int amdgpu_mn_register_hsa(struct amdgpu_bo *bo, unsigned long addr)
+{
+	return mmu_interval_notifier_insert(&bo->notifier, current->mm, addr,
+					    amdgpu_bo_size(bo),
+					    &amdgpu_mn_hsa_ops);
+}
+
 /**
  * amdgpu_mn_unregister - unregister a BO for notifier updates
  *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
index a292238f75eb..565ee5a0a3ad 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
@@ -32,6 +32,7 @@
 
 #if defined(CONFIG_HMM_MIRROR)
 int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr);
+int amdgpu_mn_register_hsa(struct amdgpu_bo *bo, unsigned long addr);
 void amdgpu_mn_unregister(struct amdgpu_bo *bo);
 #else
 static inline int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr)
@@ -40,6 +41,13 @@ static inline int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr)
 		      "add CONFIG_ZONE_DEVICE=y in config file to fix this\n");
 	return -ENODEV;
 }
+
+static inline int amdgpu_mn_register_hsa(struct amdgpu_bo *bo, unsigned long addr)
+{
+	DRM_WARN_ONCE("HMM_MIRROR kernel config option is not enabled, "
+		      "add CONFIG_ZONE_DEVICE=y in config file to fix this\n");
+	return -ENODEV;
+}
 static inline void amdgpu_mn_unregister(struct amdgpu_bo *bo) {}
 #endif
 
-- 
2.30.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

end of thread, other threads:[~2021-03-05  9:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-03  9:25 [PATCH 1/3] drm/amdgpu: drm/amdkfd: split amdgpu_mn_register Nirmoy Das
2021-03-03  9:25 ` [PATCH v2 2/3] drm/amdgpu: introduce kfd user flag for amdgpu_bo Nirmoy Das
2021-03-03 12:01   ` Christian König
2021-03-03 12:41     ` Nirmoy
2021-03-03 12:46       ` Christian König
2021-03-03  9:25 ` [PATCH v2 3/3] drm/amdgpu: drm/amdkfd: add amdgpu_kfd_bo struct Nirmoy Das
2021-03-03 12:04   ` Christian König
2021-03-03 13:06     ` Nirmoy
2021-03-05  0:37     ` Felix Kuehling
2021-03-05  9:13       ` Nirmoy
  -- strict thread matches above, loose matches on Subject: below --
2021-03-02 11:33 [PATCH 1/3] drm/amdgpu: drm/amdkfd: split amdgpu_mn_register Nirmoy Das

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.