All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] drm/amdgpu: Fetch MES scheduler/KIQ versions
@ 2022-06-10 17:13 Graham Sider
  2022-06-10 17:13 ` [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription Graham Sider
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Graham Sider @ 2022-06-10 17:13 UTC (permalink / raw)
  To: amd-gfx; +Cc: Mukul.Joshi, Felix.Kuehling, Graham Sider, Philip.Yang

Store MES scheduler and MES KIQ version numbers in amdgpu_mes for GFX11.

Signed-off-by: Graham Sider <Graham.Sider@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h |  3 +++
 drivers/gpu/drm/amd/amdgpu/mes_v11_0.c  | 12 ++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
index 92ddee5e33db..aa06c8396ee0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
@@ -64,6 +64,9 @@ struct amdgpu_mes {
 
 	spinlock_t                      queue_id_lock;
 
+	uint32_t			sched_version;
+	uint32_t			kiq_version;
+
 	uint32_t                        total_max_queue;
 	uint32_t                        doorbell_id_offset;
 	uint32_t                        max_doorbell_slices;
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
index e4eb87689f7f..2a9ef308e71c 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
@@ -890,6 +890,18 @@ static int mes_v11_0_queue_init(struct amdgpu_device *adev,
 		mes_v11_0_queue_init_register(ring);
 	}
 
+	/* get MES scheduler/KIQ versions */
+	mutex_lock(&adev->srbm_mutex);
+	soc21_grbm_select(adev, 3, pipe, 0, 0);
+
+	if (pipe == AMDGPU_MES_SCHED_PIPE)
+		adev->mes.sched_version = RREG32_SOC15(GC, 0, regCP_MES_GP3_LO);
+	else if (pipe == AMDGPU_MES_KIQ_PIPE && adev->enable_mes_kiq)
+		adev->mes.kiq_version = RREG32_SOC15(GC, 0, regCP_MES_GP3_LO);
+
+	soc21_grbm_select(adev, 0, 0, 0, 0);
+	mutex_unlock(&adev->srbm_mutex);
+
 	return 0;
 }
 
-- 
2.25.1


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

* [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription
  2022-06-10 17:13 [PATCH v2 1/3] drm/amdgpu: Fetch MES scheduler/KIQ versions Graham Sider
@ 2022-06-10 17:13 ` Graham Sider
  2022-06-10 23:06   ` Felix Kuehling
  2022-06-11  2:37   ` Lang Yu
  2022-06-10 17:13 ` [PATCH v2 3/3] drm/amdgpu: Update mes_v11_api_def.h Graham Sider
  2022-06-10 23:07 ` [PATCH v2 1/3] drm/amdgpu: Fetch MES scheduler/KIQ versions Felix Kuehling
  2 siblings, 2 replies; 9+ messages in thread
From: Graham Sider @ 2022-06-10 17:13 UTC (permalink / raw)
  To: amd-gfx; +Cc: Mukul.Joshi, Felix.Kuehling, Graham Sider, Philip.Yang

Starting with GFX11, MES requires wptr BOs to be GTT allocated/mapped to
GART for usermode queues in order to support oversubscription. In the
case that work is submitted to an unmapped queue, MES must have a GART
wptr address to determine whether the queue should be mapped.

This change is accompanied with changes in MES and is applicable for
MES_VERSION >= 3.

v2: Update MES_VERSION check from 2 to 3.

Signed-off-by: Graham Sider <Graham.Sider@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  1 +
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 39 ++++++++++++++++
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      | 45 ++++++++++++++++++-
 .../drm/amd/amdkfd/kfd_device_queue_manager.c |  9 +++-
 .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c  |  2 +
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  3 ++
 .../amd/amdkfd/kfd_process_queue_manager.c    | 19 +++++---
 7 files changed, 110 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 429b16ba10bf..dba26d1e3be9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -301,6 +301,7 @@ int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct amdgpu_device *adev,
 		struct kgd_mem *mem, void **kptr, uint64_t *size);
 void amdgpu_amdkfd_gpuvm_unmap_gtt_bo_from_kernel(struct amdgpu_device *adev,
 		struct kgd_mem *mem);
+int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_device *adev, struct amdgpu_bo *bo);
 
 int amdgpu_amdkfd_gpuvm_restore_process_bos(void *process_info,
 					    struct dma_fence **ef);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index efab923056f4..2d452655eb04 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -2030,6 +2030,45 @@ int amdgpu_amdkfd_gpuvm_sync_memory(
 	return ret;
 }
 
+int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_device *adev, struct amdgpu_bo *bo)
+{
+	int ret;
+
+	ret = amdgpu_bo_reserve(bo, true);
+	if (ret) {
+		pr_err("Failed to reserve bo. ret %d\n", ret);
+		goto err_reserve_bo_failed;
+	}
+
+	ret = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT);
+	if (ret) {
+		pr_err("Failed to pin bo. ret %d\n", ret);
+		goto err_pin_bo_failed;
+	}
+
+	ret = amdgpu_ttm_alloc_gart(&bo->tbo);
+	if (ret) {
+		pr_err("Failed to bind bo to GART. ret %d\n", ret);
+		goto err_map_bo_gart_failed;
+	}
+
+	amdgpu_amdkfd_remove_eviction_fence(
+		bo, bo->kfd_bo->process_info->eviction_fence);
+	list_del_init(&bo->kfd_bo->validate_list.head);
+
+	amdgpu_bo_unreserve(bo);
+
+	return 0;
+
+err_map_bo_gart_failed:
+	amdgpu_bo_unpin(bo);
+err_pin_bo_failed:
+	amdgpu_bo_unreserve(bo);
+err_reserve_bo_failed:
+
+	return ret;
+}
+
 int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct amdgpu_device *adev,
 		struct kgd_mem *mem, void **kptr, uint64_t *size)
 {
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index e9766e165c38..58d5ebed1b32 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -290,6 +290,11 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
 	struct queue_properties q_properties;
 	uint32_t doorbell_offset_in_process = 0;
 
+	struct amdgpu_bo_va_mapping *wptr_mapping;
+	struct interval_tree_node *wptr_node;
+	struct amdgpu_vm *wptr_vm;
+	struct amdgpu_bo *wptr_bo = NULL;
+
 	memset(&q_properties, 0, sizeof(struct queue_properties));
 
 	pr_debug("Creating queue ioctl\n");
@@ -316,12 +321,44 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
 		goto err_bind_process;
 	}
 
+	/* Starting with GFX11, MES requires wptr BOs to be GTT allocated/mapped to
+	 * GART for usermode queues in order to support oversubscription. In the
+	 * case that work is submitted to an unmapped queue, MES must have a GART
+	 * wptr address to determine whether the queue should be mapped.
+	 */
+	if (dev->shared_resources.enable_mes && (dev->adev->mes.sched_version & 0xff) >= 3) {
+		wptr_vm = drm_priv_to_vm(pdd->drm_priv);
+		err = amdgpu_bo_reserve(wptr_vm->root.bo, false);
+		if (err)
+			goto err_wptr_bo_reserve;
+
+		wptr_node = interval_tree_iter_first(&wptr_vm->va,
+				args->write_pointer_address >> PAGE_SHIFT,
+				args->write_pointer_address >> PAGE_SHIFT);
+		if (!wptr_node) {
+			pr_err("Failed to lookup wptr bo\n");
+			err = -EINVAL;
+			goto err_wptr_bo_lookup;
+		}
+
+		wptr_mapping = container_of((struct rb_node *)wptr_node, struct amdgpu_bo_va_mapping, rb);
+		wptr_bo = wptr_mapping->bo_va->base.bo;
+
+		amdgpu_bo_unreserve(wptr_vm->root.bo);
+
+		err = amdgpu_amdkfd_map_gtt_bo_to_gart(dev->adev, wptr_bo);
+		if (err) {
+			pr_err("Failed to map wptr bo to GART\n");
+			goto err_wptr_bo_gart_map;
+		}
+	}
+
 	pr_debug("Creating queue for PASID 0x%x on gpu 0x%x\n",
 			p->pasid,
 			dev->id);
 
-	err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id, NULL, NULL, NULL,
-			&doorbell_offset_in_process);
+	err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id, wptr_bo,
+			NULL, NULL, NULL, &doorbell_offset_in_process);
 	if (err != 0)
 		goto err_create_queue;
 
@@ -353,7 +390,11 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
 	kfd_dbg_ev_raise(KFD_EC_MASK(EC_QUEUE_NEW), p, dev, queue_id, false, NULL, 0);
 	return 0;
 
+err_wptr_bo_lookup:
+	amdgpu_bo_unreserve(wptr_vm->root.bo);
 err_create_queue:
+err_wptr_bo_gart_map:
+err_wptr_bo_reserve:
 err_bind_process:
 err_pdd:
 	mutex_unlock(&p->mutex);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index b39d89c52887..d8de2fbdfc7d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -208,6 +208,7 @@ static int add_queue_mes(struct device_queue_manager *dqm, struct queue *q,
 	struct kfd_process_device *pdd = qpd_to_pdd(qpd);
 	struct mes_add_queue_input queue_input;
 	int r, queue_type;
+	uint64_t wptr_addr_off;
 
 	if (dqm->is_hws_hang)
 		return -EIO;
@@ -227,7 +228,13 @@ static int add_queue_mes(struct device_queue_manager *dqm, struct queue *q,
 					AMDGPU_MES_PRIORITY_LEVEL_NORMAL;
 	queue_input.doorbell_offset = q->properties.doorbell_off;
 	queue_input.mqd_addr = q->gart_mqd_addr;
-	queue_input.wptr_addr = (uint64_t)q->properties.write_ptr;
+
+	if (q->wptr_bo) {
+		wptr_addr_off = (uint64_t)q->properties.write_ptr - (uint64_t)q->wptr_bo->kfd_bo->va;
+		queue_input.wptr_addr = ((uint64_t)q->wptr_bo->tbo.resource->start << PAGE_SHIFT) + wptr_addr_off;
+	} else
+		queue_input.wptr_addr = (uint64_t)q->properties.write_ptr;
+
 	queue_input.paging = false;
 	queue_input.tba_addr = qpd->tba_addr;
 	queue_input.tma_addr = qpd->tma_addr;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
index f1654b4da856..35e74bdd81da 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
@@ -377,6 +377,8 @@ static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
 	m->sdmax_rlcx_rb_base_hi = upper_32_bits(q->queue_address >> 8);
 	m->sdmax_rlcx_rb_rptr_addr_lo = lower_32_bits((uint64_t)q->read_ptr);
 	m->sdmax_rlcx_rb_rptr_addr_hi = upper_32_bits((uint64_t)q->read_ptr);
+	m->sdmax_rlcx_rb_wptr_poll_addr_lo = lower_32_bits((uint64_t)q->write_ptr);
+	m->sdmax_rlcx_rb_wptr_poll_addr_hi = upper_32_bits((uint64_t)q->write_ptr);
 	m->sdmax_rlcx_doorbell_offset =
 		q->doorbell_off << SDMA0_QUEUE0_DOORBELL_OFFSET__OFFSET__SHIFT;
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index a5d3963537d7..dcddee0d6f06 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -639,6 +639,8 @@ struct queue {
 	void *gang_ctx_bo;
 	uint64_t gang_ctx_gpu_addr;
 	void *gang_ctx_cpu_ptr;
+
+	struct amdgpu_bo *wptr_bo;
 };
 
 enum KFD_MQD_TYPE {
@@ -1404,6 +1406,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
 			    struct file *f,
 			    struct queue_properties *properties,
 			    unsigned int *qid,
+			    struct amdgpu_bo *wptr_bo,
 			    const struct kfd_criu_queue_priv_data *q_data,
 			    const void *restore_mqd,
 			    const void *restore_ctl_stack,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index f99e09dc43ea..ede1462a55c8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -190,7 +190,8 @@ void pqm_uninit(struct process_queue_manager *pqm)
 static int init_user_queue(struct process_queue_manager *pqm,
 				struct kfd_dev *dev, struct queue **q,
 				struct queue_properties *q_properties,
-				struct file *f, unsigned int qid)
+				struct file *f, struct amdgpu_bo *wptr_bo,
+				unsigned int qid)
 {
 	int retval;
 
@@ -221,6 +222,9 @@ static int init_user_queue(struct process_queue_manager *pqm,
 			goto cleanup;
 		}
 		memset((*q)->gang_ctx_cpu_ptr, 0, AMDGPU_MES_GANG_CTX_SIZE);
+
+		if (wptr_bo)
+			(*q)->wptr_bo = amdgpu_bo_ref(wptr_bo);
 	}
 
 	pr_debug("PQM After init queue");
@@ -237,6 +241,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
 			    struct file *f,
 			    struct queue_properties *properties,
 			    unsigned int *qid,
+			    struct amdgpu_bo *wptr_bo,
 			    const struct kfd_criu_queue_priv_data *q_data,
 			    const void *restore_mqd,
 			    const void *restore_ctl_stack,
@@ -299,7 +304,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
 		 * allocate_sdma_queue() in create_queue() has the
 		 * corresponding check logic.
 		 */
-		retval = init_user_queue(pqm, dev, &q, properties, f, *qid);
+		retval = init_user_queue(pqm, dev, &q, properties, f, wptr_bo, *qid);
 		if (retval != 0)
 			goto err_create_queue;
 		pqn->q = q;
@@ -320,7 +325,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
 			goto err_create_queue;
 		}
 
-		retval = init_user_queue(pqm, dev, &q, properties, f, *qid);
+		retval = init_user_queue(pqm, dev, &q, properties, f, wptr_bo, *qid);
 		if (retval != 0)
 			goto err_create_queue;
 		pqn->q = q;
@@ -457,9 +462,13 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid)
 			pdd->qpd.num_gws = 0;
 		}
 
-		if (dev->shared_resources.enable_mes)
+		if (dev->shared_resources.enable_mes) {
 			amdgpu_amdkfd_free_gtt_mem(dev->adev,
 						   pqn->q->gang_ctx_bo);
+			if (pqn->q->wptr_bo)
+				amdgpu_amdkfd_free_gtt_mem(dev->adev, pqn->q->wptr_bo);
+
+		}
 		uninit_queue(pqn->q);
 	}
 
@@ -900,7 +909,7 @@ int kfd_criu_restore_queue(struct kfd_process *p,
 
 	print_queue_properties(&qp);
 
-	ret = pqm_create_queue(&p->pqm, pdd->dev, NULL, &qp, &queue_id, q_data, mqd, ctl_stack,
+	ret = pqm_create_queue(&p->pqm, pdd->dev, NULL, &qp, &queue_id, NULL, q_data, mqd, ctl_stack,
 				NULL);
 	if (ret) {
 		pr_err("Failed to create new queue err:%d\n", ret);
-- 
2.25.1


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

* [PATCH v2 3/3] drm/amdgpu: Update mes_v11_api_def.h
  2022-06-10 17:13 [PATCH v2 1/3] drm/amdgpu: Fetch MES scheduler/KIQ versions Graham Sider
  2022-06-10 17:13 ` [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription Graham Sider
@ 2022-06-10 17:13 ` Graham Sider
  2022-06-10 23:06   ` Felix Kuehling
  2022-06-10 23:07 ` [PATCH v2 1/3] drm/amdgpu: Fetch MES scheduler/KIQ versions Felix Kuehling
  2 siblings, 1 reply; 9+ messages in thread
From: Graham Sider @ 2022-06-10 17:13 UTC (permalink / raw)
  To: amd-gfx; +Cc: Mukul.Joshi, Felix.Kuehling, Graham Sider, Philip.Yang

Update MES API to support oversubscription without aggregated doorbell
for usermode queues.

Signed-off-by: Graham Sider <Graham.Sider@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c               | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h               | 1 +
 drivers/gpu/drm/amd/amdgpu/mes_v11_0.c                | 3 +++
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 ++
 drivers/gpu/drm/amd/include/mes_v11_api_def.h         | 4 +++-
 5 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index 2e86baa32c55..3d9a81a8fa1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -681,6 +681,7 @@ int amdgpu_mes_add_hw_queue(struct amdgpu_device *adev, int gang_id,
 	queue_input.wptr_addr = qprops->wptr_gpu_addr;
 	queue_input.queue_type = qprops->queue_type;
 	queue_input.paging = qprops->paging;
+	queue_input.oversubscription_no_aggregated_en = 0;
 
 	r = adev->mes.funcs->add_hw_queue(&adev->mes, &queue_input);
 	if (r) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
index aa06c8396ee0..26765a9946a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
@@ -207,6 +207,7 @@ struct mes_add_queue_input {
 	uint32_t        debug_vmid;
 	uint64_t	tba_addr;
 	uint64_t	tma_addr;
+	uint64_t	oversubscription_no_aggregated_en;
 };
 
 struct mes_remove_queue_input {
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
index 2a9ef308e71c..95a1394d3943 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
@@ -163,6 +163,8 @@ static int mes_v11_0_add_hw_queue(struct amdgpu_mes *mes,
 	mes_add_queue_pkt.gws_size = input->gws_size;
 	mes_add_queue_pkt.trap_handler_addr = input->tba_addr;
 	mes_add_queue_pkt.tma_addr = input->tma_addr;
+	mes_add_queue_pkt.oversubscription_no_aggregated_en =
+		input->oversubscription_no_aggregated_en;
 
 	mes_add_queue_pkt.api_status.api_completion_fence_addr =
 		mes->ring.fence_drv.gpu_addr;
@@ -341,6 +343,7 @@ static int mes_v11_0_set_hw_resources(struct amdgpu_mes *mes)
 	mes_set_hw_res_pkt.disable_reset = 1;
 	mes_set_hw_res_pkt.disable_mes_log = 1;
 	mes_set_hw_res_pkt.use_different_vmid_compute = 1;
+	mes_set_hw_res_pkt.oversubscription_timer = 50;
 
 	mes_set_hw_res_pkt.api_status.api_completion_fence_addr =
 		mes->ring.fence_drv.gpu_addr;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index d8de2fbdfc7d..762bc6059387 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -235,6 +235,8 @@ static int add_queue_mes(struct device_queue_manager *dqm, struct queue *q,
 	} else
 		queue_input.wptr_addr = (uint64_t)q->properties.write_ptr;
 
+	queue_input.oversubscription_no_aggregated_en = 1;
+
 	queue_input.paging = false;
 	queue_input.tba_addr = qpd->tba_addr;
 	queue_input.tma_addr = qpd->tma_addr;
diff --git a/drivers/gpu/drm/amd/include/mes_v11_api_def.h b/drivers/gpu/drm/amd/include/mes_v11_api_def.h
index f9d02d7bdf77..95f0246eb045 100644
--- a/drivers/gpu/drm/amd/include/mes_v11_api_def.h
+++ b/drivers/gpu/drm/amd/include/mes_v11_api_def.h
@@ -226,6 +226,7 @@ union MESAPI_SET_HW_RESOURCES {
 			};
 			uint32_t	uint32_t_all;
 		};
+		uint32_t	oversubscription_timer;
 	};
 
 	uint32_t	max_dwords_in_api[API_FRAME_SIZE_IN_DWORDS];
@@ -265,7 +266,8 @@ union MESAPI__ADD_QUEUE {
 			uint32_t is_gang_suspended	: 1;
 			uint32_t is_tmz_queue		: 1;
 			uint32_t map_kiq_utility_queue  : 1;
-			uint32_t reserved		: 23;
+			uint32_t oversubscription_no_aggregated_en : 1;
+			uint32_t reserved		: 22;
 		};
 		struct MES_API_STATUS		api_status;
 		uint64_t                        tma_addr;
-- 
2.25.1


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

* Re: [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription
  2022-06-10 17:13 ` [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription Graham Sider
@ 2022-06-10 23:06   ` Felix Kuehling
  2022-06-13 13:33     ` Sider, Graham
  2022-06-11  2:37   ` Lang Yu
  1 sibling, 1 reply; 9+ messages in thread
From: Felix Kuehling @ 2022-06-10 23:06 UTC (permalink / raw)
  To: Graham Sider, amd-gfx; +Cc: Mukul.Joshi, Philip.Yang

Am 2022-06-10 um 13:13 schrieb Graham Sider:
> Starting with GFX11, MES requires wptr BOs to be GTT allocated/mapped to
> GART for usermode queues in order to support oversubscription. In the
> case that work is submitted to an unmapped queue, MES must have a GART
> wptr address to determine whether the queue should be mapped.
>
> This change is accompanied with changes in MES and is applicable for
> MES_VERSION >= 3.
>
> v2: Update MES_VERSION check from 2 to 3.
>
> Signed-off-by: Graham Sider <Graham.Sider@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  1 +
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 39 ++++++++++++++++
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      | 45 ++++++++++++++++++-
>   .../drm/amd/amdkfd/kfd_device_queue_manager.c |  9 +++-
>   .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c  |  2 +
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  3 ++
>   .../amd/amdkfd/kfd_process_queue_manager.c    | 19 +++++---
>   7 files changed, 110 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 429b16ba10bf..dba26d1e3be9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -301,6 +301,7 @@ int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct amdgpu_device *adev,
>   		struct kgd_mem *mem, void **kptr, uint64_t *size);
>   void amdgpu_amdkfd_gpuvm_unmap_gtt_bo_from_kernel(struct amdgpu_device *adev,
>   		struct kgd_mem *mem);
> +int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_device *adev, struct amdgpu_bo *bo);
>   
>   int amdgpu_amdkfd_gpuvm_restore_process_bos(void *process_info,
>   					    struct dma_fence **ef);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index efab923056f4..2d452655eb04 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -2030,6 +2030,45 @@ int amdgpu_amdkfd_gpuvm_sync_memory(
>   	return ret;
>   }
>   
> +int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_device *adev, struct amdgpu_bo *bo)

I think this function should take a reference count on the bo to ensure 
the pinned BO is not freed prematurely (even if broken user mode frees 
the BO before destroying the queue). Add a comment that the correct way 
to release the reference and unpin/unmap the BO is to call 
amdgpu_amdkfd_free_gtt_mem. See another comment below about moving the 
amdgpu_bo_ref here.


> +{
> +	int ret;
> +
> +	ret = amdgpu_bo_reserve(bo, true);
> +	if (ret) {
> +		pr_err("Failed to reserve bo. ret %d\n", ret);
> +		goto err_reserve_bo_failed;
> +	}
> +
> +	ret = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT);
> +	if (ret) {
> +		pr_err("Failed to pin bo. ret %d\n", ret);
> +		goto err_pin_bo_failed;
> +	}
> +
> +	ret = amdgpu_ttm_alloc_gart(&bo->tbo);
> +	if (ret) {
> +		pr_err("Failed to bind bo to GART. ret %d\n", ret);
> +		goto err_map_bo_gart_failed;
> +	}
> +
> +	amdgpu_amdkfd_remove_eviction_fence(
> +		bo, bo->kfd_bo->process_info->eviction_fence);
> +	list_del_init(&bo->kfd_bo->validate_list.head);

Please see Lang, Yu's patch "drm/amdkfd: add pinned BOs to kfd_bo_list". 
We realized that pinned BOs still need to be on the validate list. So 
please remove the list_del_init here.


> +
> +	amdgpu_bo_unreserve(bo);
> +
> +	return 0;
> +
> +err_map_bo_gart_failed:
> +	amdgpu_bo_unpin(bo);
> +err_pin_bo_failed:
> +	amdgpu_bo_unreserve(bo);
> +err_reserve_bo_failed:
> +
> +	return ret;
> +}
> +
>   int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct amdgpu_device *adev,
>   		struct kgd_mem *mem, void **kptr, uint64_t *size)
>   {
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index e9766e165c38..58d5ebed1b32 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -290,6 +290,11 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>   	struct queue_properties q_properties;
>   	uint32_t doorbell_offset_in_process = 0;
>   
> +	struct amdgpu_bo_va_mapping *wptr_mapping;
> +	struct interval_tree_node *wptr_node;
> +	struct amdgpu_vm *wptr_vm;

It's good practice to minimize the scope of local variables. The above 
three variables are only used inside the "if" below. So they should be 
declared there.


> +	struct amdgpu_bo *wptr_bo = NULL;
> +
>   	memset(&q_properties, 0, sizeof(struct queue_properties));
>   
>   	pr_debug("Creating queue ioctl\n");
> @@ -316,12 +321,44 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>   		goto err_bind_process;
>   	}
>   
> +	/* Starting with GFX11, MES requires wptr BOs to be GTT allocated/mapped to
> +	 * GART for usermode queues in order to support oversubscription. In the
> +	 * case that work is submitted to an unmapped queue, MES must have a GART
> +	 * wptr address to determine whether the queue should be mapped.
> +	 */
> +	if (dev->shared_resources.enable_mes && (dev->adev->mes.sched_version & 0xff) >= 3) {
> +		wptr_vm = drm_priv_to_vm(pdd->drm_priv);
> +		err = amdgpu_bo_reserve(wptr_vm->root.bo, false);
> +		if (err)
> +			goto err_wptr_bo_reserve;
> +
> +		wptr_node = interval_tree_iter_first(&wptr_vm->va,
> +				args->write_pointer_address >> PAGE_SHIFT,
> +				args->write_pointer_address >> PAGE_SHIFT);
> +		if (!wptr_node) {
> +			pr_err("Failed to lookup wptr bo\n");
> +			err = -EINVAL;
> +			goto err_wptr_bo_lookup;
> +		}
> +
> +		wptr_mapping = container_of((struct rb_node *)wptr_node, struct amdgpu_bo_va_mapping, rb);
> +		wptr_bo = wptr_mapping->bo_va->base.bo;
> +
> +		amdgpu_bo_unreserve(wptr_vm->root.bo);

You could move this to just after the interval_tree_iter_first call, 
even before it checks for errors. That way you don't need to deal with 
the unreserve conditionally on the error handling code path. And you can 
probably just use one error label err_wptr_map_gart that handles all 3 
failure cases.


> +
> +		err = amdgpu_amdkfd_map_gtt_bo_to_gart(dev->adev, wptr_bo);
> +		if (err) {
> +			pr_err("Failed to map wptr bo to GART\n");
> +			goto err_wptr_bo_gart_map;
> +		}
> +	}
> +
>   	pr_debug("Creating queue for PASID 0x%x on gpu 0x%x\n",
>   			p->pasid,
>   			dev->id);
>   
> -	err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id, NULL, NULL, NULL,
> -			&doorbell_offset_in_process);
> +	err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id, wptr_bo,
> +			NULL, NULL, NULL, &doorbell_offset_in_process);
>   	if (err != 0)
>   		goto err_create_queue;

The error handling for this and all later failures should gart-unmap the 
wptr_bo again.


>   
> @@ -353,7 +390,11 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>   	kfd_dbg_ev_raise(KFD_EC_MASK(EC_QUEUE_NEW), p, dev, queue_id, false, NULL, 0);
>   	return 0;
>   
> +err_wptr_bo_lookup:
> +	amdgpu_bo_unreserve(wptr_vm->root.bo);
>   err_create_queue:
> +err_wptr_bo_gart_map:
> +err_wptr_bo_reserve:
>   err_bind_process:
>   err_pdd:
>   	mutex_unlock(&p->mutex);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index b39d89c52887..d8de2fbdfc7d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -208,6 +208,7 @@ static int add_queue_mes(struct device_queue_manager *dqm, struct queue *q,
>   	struct kfd_process_device *pdd = qpd_to_pdd(qpd);
>   	struct mes_add_queue_input queue_input;
>   	int r, queue_type;
> +	uint64_t wptr_addr_off;
>   
>   	if (dqm->is_hws_hang)
>   		return -EIO;
> @@ -227,7 +228,13 @@ static int add_queue_mes(struct device_queue_manager *dqm, struct queue *q,
>   					AMDGPU_MES_PRIORITY_LEVEL_NORMAL;
>   	queue_input.doorbell_offset = q->properties.doorbell_off;
>   	queue_input.mqd_addr = q->gart_mqd_addr;
> -	queue_input.wptr_addr = (uint64_t)q->properties.write_ptr;
> +
> +	if (q->wptr_bo) {
> +		wptr_addr_off = (uint64_t)q->properties.write_ptr - (uint64_t)q->wptr_bo->kfd_bo->va;
> +		queue_input.wptr_addr = ((uint64_t)q->wptr_bo->tbo.resource->start << PAGE_SHIFT) + wptr_addr_off;
> +	} else
> +		queue_input.wptr_addr = (uint64_t)q->properties.write_ptr;
> +
>   	queue_input.paging = false;
>   	queue_input.tba_addr = qpd->tba_addr;
>   	queue_input.tma_addr = qpd->tma_addr;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> index f1654b4da856..35e74bdd81da 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> @@ -377,6 +377,8 @@ static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
>   	m->sdmax_rlcx_rb_base_hi = upper_32_bits(q->queue_address >> 8);
>   	m->sdmax_rlcx_rb_rptr_addr_lo = lower_32_bits((uint64_t)q->read_ptr);
>   	m->sdmax_rlcx_rb_rptr_addr_hi = upper_32_bits((uint64_t)q->read_ptr);
> +	m->sdmax_rlcx_rb_wptr_poll_addr_lo = lower_32_bits((uint64_t)q->write_ptr);
> +	m->sdmax_rlcx_rb_wptr_poll_addr_hi = upper_32_bits((uint64_t)q->write_ptr);
>   	m->sdmax_rlcx_doorbell_offset =
>   		q->doorbell_off << SDMA0_QUEUE0_DOORBELL_OFFSET__OFFSET__SHIFT;
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index a5d3963537d7..dcddee0d6f06 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -639,6 +639,8 @@ struct queue {
>   	void *gang_ctx_bo;
>   	uint64_t gang_ctx_gpu_addr;
>   	void *gang_ctx_cpu_ptr;
> +
> +	struct amdgpu_bo *wptr_bo;
>   };
>   
>   enum KFD_MQD_TYPE {
> @@ -1404,6 +1406,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>   			    struct file *f,
>   			    struct queue_properties *properties,
>   			    unsigned int *qid,
> +			    struct amdgpu_bo *wptr_bo,
>   			    const struct kfd_criu_queue_priv_data *q_data,
>   			    const void *restore_mqd,
>   			    const void *restore_ctl_stack,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index f99e09dc43ea..ede1462a55c8 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -190,7 +190,8 @@ void pqm_uninit(struct process_queue_manager *pqm)
>   static int init_user_queue(struct process_queue_manager *pqm,
>   				struct kfd_dev *dev, struct queue **q,
>   				struct queue_properties *q_properties,
> -				struct file *f, unsigned int qid)
> +				struct file *f, struct amdgpu_bo *wptr_bo,
> +				unsigned int qid)
>   {
>   	int retval;
>   
> @@ -221,6 +222,9 @@ static int init_user_queue(struct process_queue_manager *pqm,
>   			goto cleanup;
>   		}
>   		memset((*q)->gang_ctx_cpu_ptr, 0, AMDGPU_MES_GANG_CTX_SIZE);
> +
> +		if (wptr_bo)
> +			(*q)->wptr_bo = amdgpu_bo_ref(wptr_bo);

I think it would be clearer if you took that reference in 
amdgpu_amdkfd_map_gtt_bo_to_gart. That way it would be a simple 
assignment here with no "if". And all the BO reference handling would be 
more neatly contained in amdgpu_amdkfd code.

Regards,
   Felix


>   	}
>   
>   	pr_debug("PQM After init queue");
> @@ -237,6 +241,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>   			    struct file *f,
>   			    struct queue_properties *properties,
>   			    unsigned int *qid,
> +			    struct amdgpu_bo *wptr_bo,
>   			    const struct kfd_criu_queue_priv_data *q_data,
>   			    const void *restore_mqd,
>   			    const void *restore_ctl_stack,
> @@ -299,7 +304,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>   		 * allocate_sdma_queue() in create_queue() has the
>   		 * corresponding check logic.
>   		 */
> -		retval = init_user_queue(pqm, dev, &q, properties, f, *qid);
> +		retval = init_user_queue(pqm, dev, &q, properties, f, wptr_bo, *qid);
>   		if (retval != 0)
>   			goto err_create_queue;
>   		pqn->q = q;
> @@ -320,7 +325,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>   			goto err_create_queue;
>   		}
>   
> -		retval = init_user_queue(pqm, dev, &q, properties, f, *qid);
> +		retval = init_user_queue(pqm, dev, &q, properties, f, wptr_bo, *qid);
>   		if (retval != 0)
>   			goto err_create_queue;
>   		pqn->q = q;
> @@ -457,9 +462,13 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid)
>   			pdd->qpd.num_gws = 0;
>   		}
>   
> -		if (dev->shared_resources.enable_mes)
> +		if (dev->shared_resources.enable_mes) {
>   			amdgpu_amdkfd_free_gtt_mem(dev->adev,
>   						   pqn->q->gang_ctx_bo);
> +			if (pqn->q->wptr_bo)
> +				amdgpu_amdkfd_free_gtt_mem(dev->adev, pqn->q->wptr_bo);
> +
> +		}
>   		uninit_queue(pqn->q);
>   	}
>   
> @@ -900,7 +909,7 @@ int kfd_criu_restore_queue(struct kfd_process *p,
>   
>   	print_queue_properties(&qp);
>   
> -	ret = pqm_create_queue(&p->pqm, pdd->dev, NULL, &qp, &queue_id, q_data, mqd, ctl_stack,
> +	ret = pqm_create_queue(&p->pqm, pdd->dev, NULL, &qp, &queue_id, NULL, q_data, mqd, ctl_stack,
>   				NULL);
>   	if (ret) {
>   		pr_err("Failed to create new queue err:%d\n", ret);

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

* Re: [PATCH v2 3/3] drm/amdgpu: Update mes_v11_api_def.h
  2022-06-10 17:13 ` [PATCH v2 3/3] drm/amdgpu: Update mes_v11_api_def.h Graham Sider
@ 2022-06-10 23:06   ` Felix Kuehling
  0 siblings, 0 replies; 9+ messages in thread
From: Felix Kuehling @ 2022-06-10 23:06 UTC (permalink / raw)
  To: Graham Sider, amd-gfx; +Cc: Mukul.Joshi, Philip.Yang

Am 2022-06-10 um 13:13 schrieb Graham Sider:
> Update MES API to support oversubscription without aggregated doorbell
> for usermode queues.
>
> Signed-off-by: Graham Sider <Graham.Sider@amd.com>

Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c               | 1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h               | 1 +
>   drivers/gpu/drm/amd/amdgpu/mes_v11_0.c                | 3 +++
>   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 ++
>   drivers/gpu/drm/amd/include/mes_v11_api_def.h         | 4 +++-
>   5 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> index 2e86baa32c55..3d9a81a8fa1c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> @@ -681,6 +681,7 @@ int amdgpu_mes_add_hw_queue(struct amdgpu_device *adev, int gang_id,
>   	queue_input.wptr_addr = qprops->wptr_gpu_addr;
>   	queue_input.queue_type = qprops->queue_type;
>   	queue_input.paging = qprops->paging;
> +	queue_input.oversubscription_no_aggregated_en = 0;
>   
>   	r = adev->mes.funcs->add_hw_queue(&adev->mes, &queue_input);
>   	if (r) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> index aa06c8396ee0..26765a9946a9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> @@ -207,6 +207,7 @@ struct mes_add_queue_input {
>   	uint32_t        debug_vmid;
>   	uint64_t	tba_addr;
>   	uint64_t	tma_addr;
> +	uint64_t	oversubscription_no_aggregated_en;
>   };
>   
>   struct mes_remove_queue_input {
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> index 2a9ef308e71c..95a1394d3943 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> @@ -163,6 +163,8 @@ static int mes_v11_0_add_hw_queue(struct amdgpu_mes *mes,
>   	mes_add_queue_pkt.gws_size = input->gws_size;
>   	mes_add_queue_pkt.trap_handler_addr = input->tba_addr;
>   	mes_add_queue_pkt.tma_addr = input->tma_addr;
> +	mes_add_queue_pkt.oversubscription_no_aggregated_en =
> +		input->oversubscription_no_aggregated_en;
>   
>   	mes_add_queue_pkt.api_status.api_completion_fence_addr =
>   		mes->ring.fence_drv.gpu_addr;
> @@ -341,6 +343,7 @@ static int mes_v11_0_set_hw_resources(struct amdgpu_mes *mes)
>   	mes_set_hw_res_pkt.disable_reset = 1;
>   	mes_set_hw_res_pkt.disable_mes_log = 1;
>   	mes_set_hw_res_pkt.use_different_vmid_compute = 1;
> +	mes_set_hw_res_pkt.oversubscription_timer = 50;
>   
>   	mes_set_hw_res_pkt.api_status.api_completion_fence_addr =
>   		mes->ring.fence_drv.gpu_addr;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index d8de2fbdfc7d..762bc6059387 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -235,6 +235,8 @@ static int add_queue_mes(struct device_queue_manager *dqm, struct queue *q,
>   	} else
>   		queue_input.wptr_addr = (uint64_t)q->properties.write_ptr;
>   
> +	queue_input.oversubscription_no_aggregated_en = 1;
> +
>   	queue_input.paging = false;
>   	queue_input.tba_addr = qpd->tba_addr;
>   	queue_input.tma_addr = qpd->tma_addr;
> diff --git a/drivers/gpu/drm/amd/include/mes_v11_api_def.h b/drivers/gpu/drm/amd/include/mes_v11_api_def.h
> index f9d02d7bdf77..95f0246eb045 100644
> --- a/drivers/gpu/drm/amd/include/mes_v11_api_def.h
> +++ b/drivers/gpu/drm/amd/include/mes_v11_api_def.h
> @@ -226,6 +226,7 @@ union MESAPI_SET_HW_RESOURCES {
>   			};
>   			uint32_t	uint32_t_all;
>   		};
> +		uint32_t	oversubscription_timer;
>   	};
>   
>   	uint32_t	max_dwords_in_api[API_FRAME_SIZE_IN_DWORDS];
> @@ -265,7 +266,8 @@ union MESAPI__ADD_QUEUE {
>   			uint32_t is_gang_suspended	: 1;
>   			uint32_t is_tmz_queue		: 1;
>   			uint32_t map_kiq_utility_queue  : 1;
> -			uint32_t reserved		: 23;
> +			uint32_t oversubscription_no_aggregated_en : 1;
> +			uint32_t reserved		: 22;
>   		};
>   		struct MES_API_STATUS		api_status;
>   		uint64_t                        tma_addr;

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

* Re: [PATCH v2 1/3] drm/amdgpu: Fetch MES scheduler/KIQ versions
  2022-06-10 17:13 [PATCH v2 1/3] drm/amdgpu: Fetch MES scheduler/KIQ versions Graham Sider
  2022-06-10 17:13 ` [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription Graham Sider
  2022-06-10 17:13 ` [PATCH v2 3/3] drm/amdgpu: Update mes_v11_api_def.h Graham Sider
@ 2022-06-10 23:07 ` Felix Kuehling
  2 siblings, 0 replies; 9+ messages in thread
From: Felix Kuehling @ 2022-06-10 23:07 UTC (permalink / raw)
  To: Graham Sider, amd-gfx; +Cc: Mukul.Joshi, Philip.Yang

Am 2022-06-10 um 13:13 schrieb Graham Sider:
> Store MES scheduler and MES KIQ version numbers in amdgpu_mes for GFX11.
>
> Signed-off-by: Graham Sider <Graham.Sider@amd.com>

Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h |  3 +++
>   drivers/gpu/drm/amd/amdgpu/mes_v11_0.c  | 12 ++++++++++++
>   2 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> index 92ddee5e33db..aa06c8396ee0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> @@ -64,6 +64,9 @@ struct amdgpu_mes {
>   
>   	spinlock_t                      queue_id_lock;
>   
> +	uint32_t			sched_version;
> +	uint32_t			kiq_version;
> +
>   	uint32_t                        total_max_queue;
>   	uint32_t                        doorbell_id_offset;
>   	uint32_t                        max_doorbell_slices;
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> index e4eb87689f7f..2a9ef308e71c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> @@ -890,6 +890,18 @@ static int mes_v11_0_queue_init(struct amdgpu_device *adev,
>   		mes_v11_0_queue_init_register(ring);
>   	}
>   
> +	/* get MES scheduler/KIQ versions */
> +	mutex_lock(&adev->srbm_mutex);
> +	soc21_grbm_select(adev, 3, pipe, 0, 0);
> +
> +	if (pipe == AMDGPU_MES_SCHED_PIPE)
> +		adev->mes.sched_version = RREG32_SOC15(GC, 0, regCP_MES_GP3_LO);
> +	else if (pipe == AMDGPU_MES_KIQ_PIPE && adev->enable_mes_kiq)
> +		adev->mes.kiq_version = RREG32_SOC15(GC, 0, regCP_MES_GP3_LO);
> +
> +	soc21_grbm_select(adev, 0, 0, 0, 0);
> +	mutex_unlock(&adev->srbm_mutex);
> +
>   	return 0;
>   }
>   

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

* Re: [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription
  2022-06-10 17:13 ` [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription Graham Sider
  2022-06-10 23:06   ` Felix Kuehling
@ 2022-06-11  2:37   ` Lang Yu
  2022-06-13 13:35     ` Sider, Graham
  1 sibling, 1 reply; 9+ messages in thread
From: Lang Yu @ 2022-06-11  2:37 UTC (permalink / raw)
  To: Graham Sider; +Cc: Mukul.Joshi, Felix.Kuehling, amd-gfx, Philip.Yang

On 06/10/ , Graham Sider wrote:
> Starting with GFX11, MES requires wptr BOs to be GTT allocated/mapped to
> GART for usermode queues in order to support oversubscription. In the
> case that work is submitted to an unmapped queue, MES must have a GART
> wptr address to determine whether the queue should be mapped.
> 
> This change is accompanied with changes in MES and is applicable for
> MES_VERSION >= 3.
> 
> v2: Update MES_VERSION check from 2 to 3.
> 
> Signed-off-by: Graham Sider <Graham.Sider@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  1 +
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 39 ++++++++++++++++
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      | 45 ++++++++++++++++++-
>  .../drm/amd/amdkfd/kfd_device_queue_manager.c |  9 +++-
>  .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c  |  2 +
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  3 ++
>  .../amd/amdkfd/kfd_process_queue_manager.c    | 19 +++++---
>  7 files changed, 110 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 429b16ba10bf..dba26d1e3be9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -301,6 +301,7 @@ int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct amdgpu_device *adev,
>  		struct kgd_mem *mem, void **kptr, uint64_t *size);
>  void amdgpu_amdkfd_gpuvm_unmap_gtt_bo_from_kernel(struct amdgpu_device *adev,
>  		struct kgd_mem *mem);
> +int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_device *adev, struct amdgpu_bo *bo);
>  
>  int amdgpu_amdkfd_gpuvm_restore_process_bos(void *process_info,
>  					    struct dma_fence **ef);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index efab923056f4..2d452655eb04 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -2030,6 +2030,45 @@ int amdgpu_amdkfd_gpuvm_sync_memory(
>  	return ret;
>  }
>  
> +int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_device *adev, struct amdgpu_bo *bo)
> +{
> +	int ret;
> +
> +	ret = amdgpu_bo_reserve(bo, true);
> +	if (ret) {
> +		pr_err("Failed to reserve bo. ret %d\n", ret);
> +		goto err_reserve_bo_failed;
> +	}
> +
> +	ret = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT);
> +	if (ret) {
> +		pr_err("Failed to pin bo. ret %d\n", ret);
> +		goto err_pin_bo_failed;
> +	}
> +
> +	ret = amdgpu_ttm_alloc_gart(&bo->tbo);
> +	if (ret) {
> +		pr_err("Failed to bind bo to GART. ret %d\n", ret);
> +		goto err_map_bo_gart_failed;
> +	}
> +
> +	amdgpu_amdkfd_remove_eviction_fence(
> +		bo, bo->kfd_bo->process_info->eviction_fence);
> +	list_del_init(&bo->kfd_bo->validate_list.head);
> +
> +	amdgpu_bo_unreserve(bo);
> +
> +	return 0;
> +
> +err_map_bo_gart_failed:
> +	amdgpu_bo_unpin(bo);
> +err_pin_bo_failed:
> +	amdgpu_bo_unreserve(bo);
> +err_reserve_bo_failed:
> +
> +	return ret;
> +}
> +
>  int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct amdgpu_device *adev,
>  		struct kgd_mem *mem, void **kptr, uint64_t *size)
>  {
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index e9766e165c38..58d5ebed1b32 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -290,6 +290,11 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>  	struct queue_properties q_properties;
>  	uint32_t doorbell_offset_in_process = 0;
>  
> +	struct amdgpu_bo_va_mapping *wptr_mapping;
> +	struct interval_tree_node *wptr_node;
> +	struct amdgpu_vm *wptr_vm;
> +	struct amdgpu_bo *wptr_bo = NULL;
> +
>  	memset(&q_properties, 0, sizeof(struct queue_properties));
>  
>  	pr_debug("Creating queue ioctl\n");
> @@ -316,12 +321,44 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>  		goto err_bind_process;
>  	}
>  
> +	/* Starting with GFX11, MES requires wptr BOs to be GTT allocated/mapped to
> +	 * GART for usermode queues in order to support oversubscription. In the
> +	 * case that work is submitted to an unmapped queue, MES must have a GART
> +	 * wptr address to determine whether the queue should be mapped.
> +	 */
> +	if (dev->shared_resources.enable_mes && (dev->adev->mes.sched_version & 0xff) >= 3) {
> +		wptr_vm = drm_priv_to_vm(pdd->drm_priv);
> +		err = amdgpu_bo_reserve(wptr_vm->root.bo, false);
> +		if (err)
> +			goto err_wptr_bo_reserve;
> +
> +		wptr_node = interval_tree_iter_first(&wptr_vm->va,
> +				args->write_pointer_address >> PAGE_SHIFT,
> +				args->write_pointer_address >> PAGE_SHIFT);
> +		if (!wptr_node) {
> +			pr_err("Failed to lookup wptr bo\n");
> +			err = -EINVAL;
> +			goto err_wptr_bo_lookup;
> +		}
> +
> +		wptr_mapping = container_of((struct rb_node *)wptr_node, struct amdgpu_bo_va_mapping, rb);

You can use amdgpu_vm_bo_lookup_mapping() to find a mapping by address.

Regards,
Lang

> +		wptr_bo = wptr_mapping->bo_va->base.bo;
> +
> +		amdgpu_bo_unreserve(wptr_vm->root.bo);
> +
> +		err = amdgpu_amdkfd_map_gtt_bo_to_gart(dev->adev, wptr_bo);
> +		if (err) {
> +			pr_err("Failed to map wptr bo to GART\n");
> +			goto err_wptr_bo_gart_map;
> +		}
> +	}
> +
>  	pr_debug("Creating queue for PASID 0x%x on gpu 0x%x\n",
>  			p->pasid,
>  			dev->id);
>  
> -	err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id, NULL, NULL, NULL,
> -			&doorbell_offset_in_process);
> +	err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id, wptr_bo,
> +			NULL, NULL, NULL, &doorbell_offset_in_process);
>  	if (err != 0)
>  		goto err_create_queue;
>  
> @@ -353,7 +390,11 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>  	kfd_dbg_ev_raise(KFD_EC_MASK(EC_QUEUE_NEW), p, dev, queue_id, false, NULL, 0);
>  	return 0;
>  
> +err_wptr_bo_lookup:
> +	amdgpu_bo_unreserve(wptr_vm->root.bo);
>  err_create_queue:
> +err_wptr_bo_gart_map:
> +err_wptr_bo_reserve:
>  err_bind_process:
>  err_pdd:
>  	mutex_unlock(&p->mutex);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index b39d89c52887..d8de2fbdfc7d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -208,6 +208,7 @@ static int add_queue_mes(struct device_queue_manager *dqm, struct queue *q,
>  	struct kfd_process_device *pdd = qpd_to_pdd(qpd);
>  	struct mes_add_queue_input queue_input;
>  	int r, queue_type;
> +	uint64_t wptr_addr_off;
>  
>  	if (dqm->is_hws_hang)
>  		return -EIO;
> @@ -227,7 +228,13 @@ static int add_queue_mes(struct device_queue_manager *dqm, struct queue *q,
>  					AMDGPU_MES_PRIORITY_LEVEL_NORMAL;
>  	queue_input.doorbell_offset = q->properties.doorbell_off;
>  	queue_input.mqd_addr = q->gart_mqd_addr;
> -	queue_input.wptr_addr = (uint64_t)q->properties.write_ptr;
> +
> +	if (q->wptr_bo) {
> +		wptr_addr_off = (uint64_t)q->properties.write_ptr - (uint64_t)q->wptr_bo->kfd_bo->va;
> +		queue_input.wptr_addr = ((uint64_t)q->wptr_bo->tbo.resource->start << PAGE_SHIFT) + wptr_addr_off;
> +	} else
> +		queue_input.wptr_addr = (uint64_t)q->properties.write_ptr;
> +
>  	queue_input.paging = false;
>  	queue_input.tba_addr = qpd->tba_addr;
>  	queue_input.tma_addr = qpd->tma_addr;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> index f1654b4da856..35e74bdd81da 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> @@ -377,6 +377,8 @@ static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
>  	m->sdmax_rlcx_rb_base_hi = upper_32_bits(q->queue_address >> 8);
>  	m->sdmax_rlcx_rb_rptr_addr_lo = lower_32_bits((uint64_t)q->read_ptr);
>  	m->sdmax_rlcx_rb_rptr_addr_hi = upper_32_bits((uint64_t)q->read_ptr);
> +	m->sdmax_rlcx_rb_wptr_poll_addr_lo = lower_32_bits((uint64_t)q->write_ptr);
> +	m->sdmax_rlcx_rb_wptr_poll_addr_hi = upper_32_bits((uint64_t)q->write_ptr);
>  	m->sdmax_rlcx_doorbell_offset =
>  		q->doorbell_off << SDMA0_QUEUE0_DOORBELL_OFFSET__OFFSET__SHIFT;
>  
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index a5d3963537d7..dcddee0d6f06 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -639,6 +639,8 @@ struct queue {
>  	void *gang_ctx_bo;
>  	uint64_t gang_ctx_gpu_addr;
>  	void *gang_ctx_cpu_ptr;
> +
> +	struct amdgpu_bo *wptr_bo;
>  };
>  
>  enum KFD_MQD_TYPE {
> @@ -1404,6 +1406,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>  			    struct file *f,
>  			    struct queue_properties *properties,
>  			    unsigned int *qid,
> +			    struct amdgpu_bo *wptr_bo,
>  			    const struct kfd_criu_queue_priv_data *q_data,
>  			    const void *restore_mqd,
>  			    const void *restore_ctl_stack,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index f99e09dc43ea..ede1462a55c8 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -190,7 +190,8 @@ void pqm_uninit(struct process_queue_manager *pqm)
>  static int init_user_queue(struct process_queue_manager *pqm,
>  				struct kfd_dev *dev, struct queue **q,
>  				struct queue_properties *q_properties,
> -				struct file *f, unsigned int qid)
> +				struct file *f, struct amdgpu_bo *wptr_bo,
> +				unsigned int qid)
>  {
>  	int retval;
>  
> @@ -221,6 +222,9 @@ static int init_user_queue(struct process_queue_manager *pqm,
>  			goto cleanup;
>  		}
>  		memset((*q)->gang_ctx_cpu_ptr, 0, AMDGPU_MES_GANG_CTX_SIZE);
> +
> +		if (wptr_bo)
> +			(*q)->wptr_bo = amdgpu_bo_ref(wptr_bo);
>  	}
>  
>  	pr_debug("PQM After init queue");
> @@ -237,6 +241,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>  			    struct file *f,
>  			    struct queue_properties *properties,
>  			    unsigned int *qid,
> +			    struct amdgpu_bo *wptr_bo,
>  			    const struct kfd_criu_queue_priv_data *q_data,
>  			    const void *restore_mqd,
>  			    const void *restore_ctl_stack,
> @@ -299,7 +304,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>  		 * allocate_sdma_queue() in create_queue() has the
>  		 * corresponding check logic.
>  		 */
> -		retval = init_user_queue(pqm, dev, &q, properties, f, *qid);
> +		retval = init_user_queue(pqm, dev, &q, properties, f, wptr_bo, *qid);
>  		if (retval != 0)
>  			goto err_create_queue;
>  		pqn->q = q;
> @@ -320,7 +325,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>  			goto err_create_queue;
>  		}
>  
> -		retval = init_user_queue(pqm, dev, &q, properties, f, *qid);
> +		retval = init_user_queue(pqm, dev, &q, properties, f, wptr_bo, *qid);
>  		if (retval != 0)
>  			goto err_create_queue;
>  		pqn->q = q;
> @@ -457,9 +462,13 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid)
>  			pdd->qpd.num_gws = 0;
>  		}
>  
> -		if (dev->shared_resources.enable_mes)
> +		if (dev->shared_resources.enable_mes) {
>  			amdgpu_amdkfd_free_gtt_mem(dev->adev,
>  						   pqn->q->gang_ctx_bo);
> +			if (pqn->q->wptr_bo)
> +				amdgpu_amdkfd_free_gtt_mem(dev->adev, pqn->q->wptr_bo);
> +
> +		}
>  		uninit_queue(pqn->q);
>  	}
>  
> @@ -900,7 +909,7 @@ int kfd_criu_restore_queue(struct kfd_process *p,
>  
>  	print_queue_properties(&qp);
>  
> -	ret = pqm_create_queue(&p->pqm, pdd->dev, NULL, &qp, &queue_id, q_data, mqd, ctl_stack,
> +	ret = pqm_create_queue(&p->pqm, pdd->dev, NULL, &qp, &queue_id, NULL, q_data, mqd, ctl_stack,
>  				NULL);
>  	if (ret) {
>  		pr_err("Failed to create new queue err:%d\n", ret);
> -- 
> 2.25.1
> 

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

* RE: [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription
  2022-06-10 23:06   ` Felix Kuehling
@ 2022-06-13 13:33     ` Sider, Graham
  0 siblings, 0 replies; 9+ messages in thread
From: Sider, Graham @ 2022-06-13 13:33 UTC (permalink / raw)
  To: Kuehling, Felix, amd-gfx; +Cc: Joshi, Mukul, Yang, Philip

[AMD Official Use Only - General]

Thanks for the great comments Felix--will apply these.

Best,
Graham

-----Original Message-----
From: Kuehling, Felix <Felix.Kuehling@amd.com> 
Sent: Friday, June 10, 2022 7:06 PM
To: Sider, Graham <Graham.Sider@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Joshi, Mukul <Mukul.Joshi@amd.com>; Yang, Philip <Philip.Yang@amd.com>
Subject: Re: [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription

Am 2022-06-10 um 13:13 schrieb Graham Sider:
> Starting with GFX11, MES requires wptr BOs to be GTT allocated/mapped 
> to GART for usermode queues in order to support oversubscription. In 
> the case that work is submitted to an unmapped queue, MES must have a 
> GART wptr address to determine whether the queue should be mapped.
>
> This change is accompanied with changes in MES and is applicable for 
> MES_VERSION >= 3.
>
> v2: Update MES_VERSION check from 2 to 3.
>
> Signed-off-by: Graham Sider <Graham.Sider@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  1 +
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 39 ++++++++++++++++
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      | 45 ++++++++++++++++++-
>   .../drm/amd/amdkfd/kfd_device_queue_manager.c |  9 +++-
>   .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c  |  2 +
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  3 ++
>   .../amd/amdkfd/kfd_process_queue_manager.c    | 19 +++++---
>   7 files changed, 110 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 429b16ba10bf..dba26d1e3be9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -301,6 +301,7 @@ int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct amdgpu_device *adev,
>   		struct kgd_mem *mem, void **kptr, uint64_t *size);
>   void amdgpu_amdkfd_gpuvm_unmap_gtt_bo_from_kernel(struct amdgpu_device *adev,
>   		struct kgd_mem *mem);
> +int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_device *adev, 
> +struct amdgpu_bo *bo);
>   
>   int amdgpu_amdkfd_gpuvm_restore_process_bos(void *process_info,
>   					    struct dma_fence **ef);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index efab923056f4..2d452655eb04 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -2030,6 +2030,45 @@ int amdgpu_amdkfd_gpuvm_sync_memory(
>   	return ret;
>   }
>   
> +int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_device *adev, 
> +struct amdgpu_bo *bo)

I think this function should take a reference count on the bo to ensure the pinned BO is not freed prematurely (even if broken user mode frees the BO before destroying the queue). Add a comment that the correct way to release the reference and unpin/unmap the BO is to call amdgpu_amdkfd_free_gtt_mem. See another comment below about moving the amdgpu_bo_ref here.


> +{
> +	int ret;
> +
> +	ret = amdgpu_bo_reserve(bo, true);
> +	if (ret) {
> +		pr_err("Failed to reserve bo. ret %d\n", ret);
> +		goto err_reserve_bo_failed;
> +	}
> +
> +	ret = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT);
> +	if (ret) {
> +		pr_err("Failed to pin bo. ret %d\n", ret);
> +		goto err_pin_bo_failed;
> +	}
> +
> +	ret = amdgpu_ttm_alloc_gart(&bo->tbo);
> +	if (ret) {
> +		pr_err("Failed to bind bo to GART. ret %d\n", ret);
> +		goto err_map_bo_gart_failed;
> +	}
> +
> +	amdgpu_amdkfd_remove_eviction_fence(
> +		bo, bo->kfd_bo->process_info->eviction_fence);
> +	list_del_init(&bo->kfd_bo->validate_list.head);

Please see Lang, Yu's patch "drm/amdkfd: add pinned BOs to kfd_bo_list". 
We realized that pinned BOs still need to be on the validate list. So please remove the list_del_init here.


> +
> +	amdgpu_bo_unreserve(bo);
> +
> +	return 0;
> +
> +err_map_bo_gart_failed:
> +	amdgpu_bo_unpin(bo);
> +err_pin_bo_failed:
> +	amdgpu_bo_unreserve(bo);
> +err_reserve_bo_failed:
> +
> +	return ret;
> +}
> +
>   int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct amdgpu_device *adev,
>   		struct kgd_mem *mem, void **kptr, uint64_t *size)
>   {
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index e9766e165c38..58d5ebed1b32 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -290,6 +290,11 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>   	struct queue_properties q_properties;
>   	uint32_t doorbell_offset_in_process = 0;
>   
> +	struct amdgpu_bo_va_mapping *wptr_mapping;
> +	struct interval_tree_node *wptr_node;
> +	struct amdgpu_vm *wptr_vm;

It's good practice to minimize the scope of local variables. The above 
three variables are only used inside the "if" below. So they should be 
declared there.


> +	struct amdgpu_bo *wptr_bo = NULL;
> +
>   	memset(&q_properties, 0, sizeof(struct queue_properties));
>   
>   	pr_debug("Creating queue ioctl\n");
> @@ -316,12 +321,44 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>   		goto err_bind_process;
>   	}
>   
> +	/* Starting with GFX11, MES requires wptr BOs to be GTT allocated/mapped to
> +	 * GART for usermode queues in order to support oversubscription. In the
> +	 * case that work is submitted to an unmapped queue, MES must have a GART
> +	 * wptr address to determine whether the queue should be mapped.
> +	 */
> +	if (dev->shared_resources.enable_mes && (dev->adev->mes.sched_version & 0xff) >= 3) {
> +		wptr_vm = drm_priv_to_vm(pdd->drm_priv);
> +		err = amdgpu_bo_reserve(wptr_vm->root.bo, false);
> +		if (err)
> +			goto err_wptr_bo_reserve;
> +
> +		wptr_node = interval_tree_iter_first(&wptr_vm->va,
> +				args->write_pointer_address >> PAGE_SHIFT,
> +				args->write_pointer_address >> PAGE_SHIFT);
> +		if (!wptr_node) {
> +			pr_err("Failed to lookup wptr bo\n");
> +			err = -EINVAL;
> +			goto err_wptr_bo_lookup;
> +		}
> +
> +		wptr_mapping = container_of((struct rb_node *)wptr_node, struct amdgpu_bo_va_mapping, rb);
> +		wptr_bo = wptr_mapping->bo_va->base.bo;
> +
> +		amdgpu_bo_unreserve(wptr_vm->root.bo);

You could move this to just after the interval_tree_iter_first call, 
even before it checks for errors. That way you don't need to deal with 
the unreserve conditionally on the error handling code path. And you can 
probably just use one error label err_wptr_map_gart that handles all 3 
failure cases.


> +
> +		err = amdgpu_amdkfd_map_gtt_bo_to_gart(dev->adev, wptr_bo);
> +		if (err) {
> +			pr_err("Failed to map wptr bo to GART\n");
> +			goto err_wptr_bo_gart_map;
> +		}
> +	}
> +
>   	pr_debug("Creating queue for PASID 0x%x on gpu 0x%x\n",
>   			p->pasid,
>   			dev->id);
>   
> -	err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id, NULL, NULL, NULL,
> -			&doorbell_offset_in_process);
> +	err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, &queue_id, wptr_bo,
> +			NULL, NULL, NULL, &doorbell_offset_in_process);
>   	if (err != 0)
>   		goto err_create_queue;

The error handling for this and all later failures should gart-unmap the 
wptr_bo again.


>   
> @@ -353,7 +390,11 @@ static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
>   	kfd_dbg_ev_raise(KFD_EC_MASK(EC_QUEUE_NEW), p, dev, queue_id, false, NULL, 0);
>   	return 0;
>   
> +err_wptr_bo_lookup:
> +	amdgpu_bo_unreserve(wptr_vm->root.bo);
>   err_create_queue:
> +err_wptr_bo_gart_map:
> +err_wptr_bo_reserve:
>   err_bind_process:
>   err_pdd:
>   	mutex_unlock(&p->mutex);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index b39d89c52887..d8de2fbdfc7d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -208,6 +208,7 @@ static int add_queue_mes(struct device_queue_manager *dqm, struct queue *q,
>   	struct kfd_process_device *pdd = qpd_to_pdd(qpd);
>   	struct mes_add_queue_input queue_input;
>   	int r, queue_type;
> +	uint64_t wptr_addr_off;
>   
>   	if (dqm->is_hws_hang)
>   		return -EIO;
> @@ -227,7 +228,13 @@ static int add_queue_mes(struct device_queue_manager *dqm, struct queue *q,
>   					AMDGPU_MES_PRIORITY_LEVEL_NORMAL;
>   	queue_input.doorbell_offset = q->properties.doorbell_off;
>   	queue_input.mqd_addr = q->gart_mqd_addr;
> -	queue_input.wptr_addr = (uint64_t)q->properties.write_ptr;
> +
> +	if (q->wptr_bo) {
> +		wptr_addr_off = (uint64_t)q->properties.write_ptr - (uint64_t)q->wptr_bo->kfd_bo->va;
> +		queue_input.wptr_addr = ((uint64_t)q->wptr_bo->tbo.resource->start << PAGE_SHIFT) + wptr_addr_off;
> +	} else
> +		queue_input.wptr_addr = (uint64_t)q->properties.write_ptr;
> +
>   	queue_input.paging = false;
>   	queue_input.tba_addr = qpd->tba_addr;
>   	queue_input.tma_addr = qpd->tma_addr;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> index f1654b4da856..35e74bdd81da 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> @@ -377,6 +377,8 @@ static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
>   	m->sdmax_rlcx_rb_base_hi = upper_32_bits(q->queue_address >> 8);
>   	m->sdmax_rlcx_rb_rptr_addr_lo = lower_32_bits((uint64_t)q->read_ptr);
>   	m->sdmax_rlcx_rb_rptr_addr_hi = upper_32_bits((uint64_t)q->read_ptr);
> +	m->sdmax_rlcx_rb_wptr_poll_addr_lo = lower_32_bits((uint64_t)q->write_ptr);
> +	m->sdmax_rlcx_rb_wptr_poll_addr_hi = upper_32_bits((uint64_t)q->write_ptr);
>   	m->sdmax_rlcx_doorbell_offset =
>   		q->doorbell_off << SDMA0_QUEUE0_DOORBELL_OFFSET__OFFSET__SHIFT;
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index a5d3963537d7..dcddee0d6f06 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -639,6 +639,8 @@ struct queue {
>   	void *gang_ctx_bo;
>   	uint64_t gang_ctx_gpu_addr;
>   	void *gang_ctx_cpu_ptr;
> +
> +	struct amdgpu_bo *wptr_bo;
>   };
>   
>   enum KFD_MQD_TYPE {
> @@ -1404,6 +1406,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>   			    struct file *f,
>   			    struct queue_properties *properties,
>   			    unsigned int *qid,
> +			    struct amdgpu_bo *wptr_bo,
>   			    const struct kfd_criu_queue_priv_data *q_data,
>   			    const void *restore_mqd,
>   			    const void *restore_ctl_stack,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index f99e09dc43ea..ede1462a55c8 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -190,7 +190,8 @@ void pqm_uninit(struct process_queue_manager *pqm)
>   static int init_user_queue(struct process_queue_manager *pqm,
>   				struct kfd_dev *dev, struct queue **q,
>   				struct queue_properties *q_properties,
> -				struct file *f, unsigned int qid)
> +				struct file *f, struct amdgpu_bo *wptr_bo,
> +				unsigned int qid)
>   {
>   	int retval;
>   
> @@ -221,6 +222,9 @@ static int init_user_queue(struct process_queue_manager *pqm,
>   			goto cleanup;
>   		}
>   		memset((*q)->gang_ctx_cpu_ptr, 0, AMDGPU_MES_GANG_CTX_SIZE);
> +
> +		if (wptr_bo)
> +			(*q)->wptr_bo = amdgpu_bo_ref(wptr_bo);

I think it would be clearer if you took that reference in 
amdgpu_amdkfd_map_gtt_bo_to_gart. That way it would be a simple 
assignment here with no "if". And all the BO reference handling would be 
more neatly contained in amdgpu_amdkfd code.

Regards,
   Felix


>   	}
>   
>   	pr_debug("PQM After init queue");
> @@ -237,6 +241,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>   			    struct file *f,
>   			    struct queue_properties *properties,
>   			    unsigned int *qid,
> +			    struct amdgpu_bo *wptr_bo,
>   			    const struct kfd_criu_queue_priv_data *q_data,
>   			    const void *restore_mqd,
>   			    const void *restore_ctl_stack,
> @@ -299,7 +304,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>   		 * allocate_sdma_queue() in create_queue() has the
>   		 * corresponding check logic.
>   		 */
> -		retval = init_user_queue(pqm, dev, &q, properties, f, *qid);
> +		retval = init_user_queue(pqm, dev, &q, properties, f, wptr_bo, *qid);
>   		if (retval != 0)
>   			goto err_create_queue;
>   		pqn->q = q;
> @@ -320,7 +325,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>   			goto err_create_queue;
>   		}
>   
> -		retval = init_user_queue(pqm, dev, &q, properties, f, *qid);
> +		retval = init_user_queue(pqm, dev, &q, properties, f, wptr_bo, *qid);
>   		if (retval != 0)
>   			goto err_create_queue;
>   		pqn->q = q;
> @@ -457,9 +462,13 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid)
>   			pdd->qpd.num_gws = 0;
>   		}
>   
> -		if (dev->shared_resources.enable_mes)
> +		if (dev->shared_resources.enable_mes) {
>   			amdgpu_amdkfd_free_gtt_mem(dev->adev,
>   						   pqn->q->gang_ctx_bo);
> +			if (pqn->q->wptr_bo)
> +				amdgpu_amdkfd_free_gtt_mem(dev->adev, pqn->q->wptr_bo);
> +
> +		}
>   		uninit_queue(pqn->q);
>   	}
>   
> @@ -900,7 +909,7 @@ int kfd_criu_restore_queue(struct kfd_process *p,
>   
>   	print_queue_properties(&qp);
>   
> -	ret = pqm_create_queue(&p->pqm, pdd->dev, NULL, &qp, &queue_id, q_data, mqd, ctl_stack,
> +	ret = pqm_create_queue(&p->pqm, pdd->dev, NULL, &qp, &queue_id, NULL, q_data, mqd, ctl_stack,
>   				NULL);
>   	if (ret) {
>   		pr_err("Failed to create new queue err:%d\n", ret);

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

* RE: [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription
  2022-06-11  2:37   ` Lang Yu
@ 2022-06-13 13:35     ` Sider, Graham
  0 siblings, 0 replies; 9+ messages in thread
From: Sider, Graham @ 2022-06-13 13:35 UTC (permalink / raw)
  To: Yu, Lang; +Cc: Joshi, Mukul, Kuehling, Felix, amd-gfx, Yang, Philip

[Public]

> -----Original Message-----
> From: Yu, Lang <Lang.Yu@amd.com>
> Sent: Friday, June 10, 2022 10:38 PM
> To: Sider, Graham <Graham.Sider@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; Joshi, Mukul <Mukul.Joshi@amd.com>;
> Kuehling, Felix <Felix.Kuehling@amd.com>; Yang, Philip
> <Philip.Yang@amd.com>
> Subject: Re: [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue
> oversubscription
> 
> On 06/10/ , Graham Sider wrote:
> > Starting with GFX11, MES requires wptr BOs to be GTT allocated/mapped
> > to GART for usermode queues in order to support oversubscription. In
> > the case that work is submitted to an unmapped queue, MES must have a
> > GART wptr address to determine whether the queue should be mapped.
> >
> > This change is accompanied with changes in MES and is applicable for
> > MES_VERSION >= 3.
> >
> > v2: Update MES_VERSION check from 2 to 3.
> >
> > Signed-off-by: Graham Sider <Graham.Sider@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  1 +
> >  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 39
> ++++++++++++++++
> >  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      | 45
> ++++++++++++++++++-
> >  .../drm/amd/amdkfd/kfd_device_queue_manager.c |  9 +++-
> > .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c  |  2 +
> >  drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  3 ++
> >  .../amd/amdkfd/kfd_process_queue_manager.c    | 19 +++++---
> >  7 files changed, 110 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > index 429b16ba10bf..dba26d1e3be9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > @@ -301,6 +301,7 @@ int
> amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct amdgpu_device
> *adev,
> >  		struct kgd_mem *mem, void **kptr, uint64_t *size);  void
> > amdgpu_amdkfd_gpuvm_unmap_gtt_bo_from_kernel(struct
> amdgpu_device *adev,
> >  		struct kgd_mem *mem);
> > +int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_device *adev,
> > +struct amdgpu_bo *bo);
> >
> >  int amdgpu_amdkfd_gpuvm_restore_process_bos(void *process_info,
> >  					    struct dma_fence **ef);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > index efab923056f4..2d452655eb04 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > @@ -2030,6 +2030,45 @@ int amdgpu_amdkfd_gpuvm_sync_memory(
> >  	return ret;
> >  }
> >
> > +int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_device *adev,
> > +struct amdgpu_bo *bo) {
> > +	int ret;
> > +
> > +	ret = amdgpu_bo_reserve(bo, true);
> > +	if (ret) {
> > +		pr_err("Failed to reserve bo. ret %d\n", ret);
> > +		goto err_reserve_bo_failed;
> > +	}
> > +
> > +	ret = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT);
> > +	if (ret) {
> > +		pr_err("Failed to pin bo. ret %d\n", ret);
> > +		goto err_pin_bo_failed;
> > +	}
> > +
> > +	ret = amdgpu_ttm_alloc_gart(&bo->tbo);
> > +	if (ret) {
> > +		pr_err("Failed to bind bo to GART. ret %d\n", ret);
> > +		goto err_map_bo_gart_failed;
> > +	}
> > +
> > +	amdgpu_amdkfd_remove_eviction_fence(
> > +		bo, bo->kfd_bo->process_info->eviction_fence);
> > +	list_del_init(&bo->kfd_bo->validate_list.head);
> > +
> > +	amdgpu_bo_unreserve(bo);
> > +
> > +	return 0;
> > +
> > +err_map_bo_gart_failed:
> > +	amdgpu_bo_unpin(bo);
> > +err_pin_bo_failed:
> > +	amdgpu_bo_unreserve(bo);
> > +err_reserve_bo_failed:
> > +
> > +	return ret;
> > +}
> > +
> >  int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct
> amdgpu_device *adev,
> >  		struct kgd_mem *mem, void **kptr, uint64_t *size)  { diff --
> git
> > a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > index e9766e165c38..58d5ebed1b32 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > @@ -290,6 +290,11 @@ static int kfd_ioctl_create_queue(struct file *filep,
> struct kfd_process *p,
> >  	struct queue_properties q_properties;
> >  	uint32_t doorbell_offset_in_process = 0;
> >
> > +	struct amdgpu_bo_va_mapping *wptr_mapping;
> > +	struct interval_tree_node *wptr_node;
> > +	struct amdgpu_vm *wptr_vm;
> > +	struct amdgpu_bo *wptr_bo = NULL;
> > +
> >  	memset(&q_properties, 0, sizeof(struct queue_properties));
> >
> >  	pr_debug("Creating queue ioctl\n");
> > @@ -316,12 +321,44 @@ static int kfd_ioctl_create_queue(struct file
> *filep, struct kfd_process *p,
> >  		goto err_bind_process;
> >  	}
> >
> > +	/* Starting with GFX11, MES requires wptr BOs to be GTT
> allocated/mapped to
> > +	 * GART for usermode queues in order to support oversubscription.
> In the
> > +	 * case that work is submitted to an unmapped queue, MES must
> have a GART
> > +	 * wptr address to determine whether the queue should be mapped.
> > +	 */
> > +	if (dev->shared_resources.enable_mes && (dev->adev-
> >mes.sched_version & 0xff) >= 3) {
> > +		wptr_vm = drm_priv_to_vm(pdd->drm_priv);
> > +		err = amdgpu_bo_reserve(wptr_vm->root.bo, false);
> > +		if (err)
> > +			goto err_wptr_bo_reserve;
> > +
> > +		wptr_node = interval_tree_iter_first(&wptr_vm->va,
> > +				args->write_pointer_address >>
> PAGE_SHIFT,
> > +				args->write_pointer_address >>
> PAGE_SHIFT);
> > +		if (!wptr_node) {
> > +			pr_err("Failed to lookup wptr bo\n");
> > +			err = -EINVAL;
> > +			goto err_wptr_bo_lookup;
> > +		}
> > +
> > +		wptr_mapping = container_of((struct rb_node *)wptr_node,
> struct
> > +amdgpu_bo_va_mapping, rb);
> 
> You can use amdgpu_vm_bo_lookup_mapping() to find a mapping by
> address.
> 
> Regards,
> Lang

Good catch, will use this. Thanks!

Best,
Graham

> 
> > +		wptr_bo = wptr_mapping->bo_va->base.bo;
> > +
> > +		amdgpu_bo_unreserve(wptr_vm->root.bo);
> > +
> > +		err = amdgpu_amdkfd_map_gtt_bo_to_gart(dev->adev,
> wptr_bo);
> > +		if (err) {
> > +			pr_err("Failed to map wptr bo to GART\n");
> > +			goto err_wptr_bo_gart_map;
> > +		}
> > +	}
> > +
> >  	pr_debug("Creating queue for PASID 0x%x on gpu 0x%x\n",
> >  			p->pasid,
> >  			dev->id);
> >
> > -	err = pqm_create_queue(&p->pqm, dev, filep, &q_properties,
> &queue_id, NULL, NULL, NULL,
> > -			&doorbell_offset_in_process);
> > +	err = pqm_create_queue(&p->pqm, dev, filep, &q_properties,
> &queue_id, wptr_bo,
> > +			NULL, NULL, NULL, &doorbell_offset_in_process);
> >  	if (err != 0)
> >  		goto err_create_queue;
> >
> > @@ -353,7 +390,11 @@ static int kfd_ioctl_create_queue(struct file *filep,
> struct kfd_process *p,
> >  	kfd_dbg_ev_raise(KFD_EC_MASK(EC_QUEUE_NEW), p, dev,
> queue_id, false, NULL, 0);
> >  	return 0;
> >
> > +err_wptr_bo_lookup:
> > +	amdgpu_bo_unreserve(wptr_vm->root.bo);
> >  err_create_queue:
> > +err_wptr_bo_gart_map:
> > +err_wptr_bo_reserve:
> >  err_bind_process:
> >  err_pdd:
> >  	mutex_unlock(&p->mutex);
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > index b39d89c52887..d8de2fbdfc7d 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > @@ -208,6 +208,7 @@ static int add_queue_mes(struct
> device_queue_manager *dqm, struct queue *q,
> >  	struct kfd_process_device *pdd = qpd_to_pdd(qpd);
> >  	struct mes_add_queue_input queue_input;
> >  	int r, queue_type;
> > +	uint64_t wptr_addr_off;
> >
> >  	if (dqm->is_hws_hang)
> >  		return -EIO;
> > @@ -227,7 +228,13 @@ static int add_queue_mes(struct
> device_queue_manager *dqm, struct queue *q,
> >
> 	AMDGPU_MES_PRIORITY_LEVEL_NORMAL;
> >  	queue_input.doorbell_offset = q->properties.doorbell_off;
> >  	queue_input.mqd_addr = q->gart_mqd_addr;
> > -	queue_input.wptr_addr = (uint64_t)q->properties.write_ptr;
> > +
> > +	if (q->wptr_bo) {
> > +		wptr_addr_off = (uint64_t)q->properties.write_ptr -
> (uint64_t)q->wptr_bo->kfd_bo->va;
> > +		queue_input.wptr_addr = ((uint64_t)q->wptr_bo-
> >tbo.resource->start << PAGE_SHIFT) + wptr_addr_off;
> > +	} else
> > +		queue_input.wptr_addr = (uint64_t)q-
> >properties.write_ptr;
> > +
> >  	queue_input.paging = false;
> >  	queue_input.tba_addr = qpd->tba_addr;
> >  	queue_input.tma_addr = qpd->tma_addr; diff --git
> > a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> > index f1654b4da856..35e74bdd81da 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
> > @@ -377,6 +377,8 @@ static void update_mqd_sdma(struct mqd_manager
> *mm, void *mqd,
> >  	m->sdmax_rlcx_rb_base_hi = upper_32_bits(q->queue_address >>
> 8);
> >  	m->sdmax_rlcx_rb_rptr_addr_lo = lower_32_bits((uint64_t)q-
> >read_ptr);
> >  	m->sdmax_rlcx_rb_rptr_addr_hi =
> > upper_32_bits((uint64_t)q->read_ptr);
> > +	m->sdmax_rlcx_rb_wptr_poll_addr_lo = lower_32_bits((uint64_t)q-
> >write_ptr);
> > +	m->sdmax_rlcx_rb_wptr_poll_addr_hi =
> > +upper_32_bits((uint64_t)q->write_ptr);
> >  	m->sdmax_rlcx_doorbell_offset =
> >  		q->doorbell_off <<
> SDMA0_QUEUE0_DOORBELL_OFFSET__OFFSET__SHIFT;
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > index a5d3963537d7..dcddee0d6f06 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > @@ -639,6 +639,8 @@ struct queue {
> >  	void *gang_ctx_bo;
> >  	uint64_t gang_ctx_gpu_addr;
> >  	void *gang_ctx_cpu_ptr;
> > +
> > +	struct amdgpu_bo *wptr_bo;
> >  };
> >
> >  enum KFD_MQD_TYPE {
> > @@ -1404,6 +1406,7 @@ int pqm_create_queue(struct
> process_queue_manager *pqm,
> >  			    struct file *f,
> >  			    struct queue_properties *properties,
> >  			    unsigned int *qid,
> > +			    struct amdgpu_bo *wptr_bo,
> >  			    const struct kfd_criu_queue_priv_data *q_data,
> >  			    const void *restore_mqd,
> >  			    const void *restore_ctl_stack, diff --git
> > a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> > index f99e09dc43ea..ede1462a55c8 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> > @@ -190,7 +190,8 @@ void pqm_uninit(struct process_queue_manager
> *pqm)
> > static int init_user_queue(struct process_queue_manager *pqm,
> >  				struct kfd_dev *dev, struct queue **q,
> >  				struct queue_properties *q_properties,
> > -				struct file *f, unsigned int qid)
> > +				struct file *f, struct amdgpu_bo *wptr_bo,
> > +				unsigned int qid)
> >  {
> >  	int retval;
> >
> > @@ -221,6 +222,9 @@ static int init_user_queue(struct
> process_queue_manager *pqm,
> >  			goto cleanup;
> >  		}
> >  		memset((*q)->gang_ctx_cpu_ptr, 0,
> AMDGPU_MES_GANG_CTX_SIZE);
> > +
> > +		if (wptr_bo)
> > +			(*q)->wptr_bo = amdgpu_bo_ref(wptr_bo);
> >  	}
> >
> >  	pr_debug("PQM After init queue");
> > @@ -237,6 +241,7 @@ int pqm_create_queue(struct
> process_queue_manager *pqm,
> >  			    struct file *f,
> >  			    struct queue_properties *properties,
> >  			    unsigned int *qid,
> > +			    struct amdgpu_bo *wptr_bo,
> >  			    const struct kfd_criu_queue_priv_data *q_data,
> >  			    const void *restore_mqd,
> >  			    const void *restore_ctl_stack, @@ -299,7 +304,7
> @@ int
> > pqm_create_queue(struct process_queue_manager *pqm,
> >  		 * allocate_sdma_queue() in create_queue() has the
> >  		 * corresponding check logic.
> >  		 */
> > -		retval = init_user_queue(pqm, dev, &q, properties, f, *qid);
> > +		retval = init_user_queue(pqm, dev, &q, properties, f,
> wptr_bo,
> > +*qid);
> >  		if (retval != 0)
> >  			goto err_create_queue;
> >  		pqn->q = q;
> > @@ -320,7 +325,7 @@ int pqm_create_queue(struct
> process_queue_manager *pqm,
> >  			goto err_create_queue;
> >  		}
> >
> > -		retval = init_user_queue(pqm, dev, &q, properties, f, *qid);
> > +		retval = init_user_queue(pqm, dev, &q, properties, f,
> wptr_bo,
> > +*qid);
> >  		if (retval != 0)
> >  			goto err_create_queue;
> >  		pqn->q = q;
> > @@ -457,9 +462,13 @@ int pqm_destroy_queue(struct
> process_queue_manager *pqm, unsigned int qid)
> >  			pdd->qpd.num_gws = 0;
> >  		}
> >
> > -		if (dev->shared_resources.enable_mes)
> > +		if (dev->shared_resources.enable_mes) {
> >  			amdgpu_amdkfd_free_gtt_mem(dev->adev,
> >  						   pqn->q->gang_ctx_bo);
> > +			if (pqn->q->wptr_bo)
> > +				amdgpu_amdkfd_free_gtt_mem(dev-
> >adev, pqn->q->wptr_bo);
> > +
> > +		}
> >  		uninit_queue(pqn->q);
> >  	}
> >
> > @@ -900,7 +909,7 @@ int kfd_criu_restore_queue(struct kfd_process *p,
> >
> >  	print_queue_properties(&qp);
> >
> > -	ret = pqm_create_queue(&p->pqm, pdd->dev, NULL, &qp,
> &queue_id, q_data, mqd, ctl_stack,
> > +	ret = pqm_create_queue(&p->pqm, pdd->dev, NULL, &qp,
> &queue_id,
> > +NULL, q_data, mqd, ctl_stack,
> >  				NULL);
> >  	if (ret) {
> >  		pr_err("Failed to create new queue err:%d\n", ret);
> > --
> > 2.25.1
> >

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

end of thread, other threads:[~2022-06-13 13:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-10 17:13 [PATCH v2 1/3] drm/amdgpu: Fetch MES scheduler/KIQ versions Graham Sider
2022-06-10 17:13 ` [PATCH v2 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription Graham Sider
2022-06-10 23:06   ` Felix Kuehling
2022-06-13 13:33     ` Sider, Graham
2022-06-11  2:37   ` Lang Yu
2022-06-13 13:35     ` Sider, Graham
2022-06-10 17:13 ` [PATCH v2 3/3] drm/amdgpu: Update mes_v11_api_def.h Graham Sider
2022-06-10 23:06   ` Felix Kuehling
2022-06-10 23:07 ` [PATCH v2 1/3] drm/amdgpu: Fetch MES scheduler/KIQ versions Felix Kuehling

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.