<html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> <div class="moz-cite-prefix">On 2022-06-15 10:06, Christian König wrote: </div> <blockquote type="cite" cite="mid:26836141-d84a-363e-32c8-bd65dc8cdd22@amd.com">Am 15.06.22 um 15:17 schrieb Sider, Graham: <blockquote type="cite">[AMD Official Use Only - General] <blockquote type="cite">-----Original Message----- From: Koenig, Christian <a class="moz-txt-link-rfc2396E" href="mailto:Christian.Koenig@amd.com"><Christian.Koenig@amd.com></a> Sent: Wednesday, June 15, 2022 3:29 AM To: Sider, Graham <a class="moz-txt-link-rfc2396E" href="mailto:Graham.Sider@amd.com"><Graham.Sider@amd.com></a>; amd- <a class="moz-txt-link-abbreviated" href="mailto:gfx@lists.freedesktop.org">gfx@lists.freedesktop.org</a> Cc: Joshi, Mukul <a class="moz-txt-link-rfc2396E" href="mailto:Mukul.Joshi@amd.com"><Mukul.Joshi@amd.com></a>; Kuehling, Felix <a class="moz-txt-link-rfc2396E" href="mailto:Felix.Kuehling@amd.com"><Felix.Kuehling@amd.com></a>; Yang, Philip <a class="moz-txt-link-rfc2396E" href="mailto:Philip.Yang@amd.com"><Philip.Yang@amd.com></a> Subject: Re: [PATCH v3 2/3] drm/amdkfd: Enable GFX11 usermode queue oversubscription Am 13.06.22 um 17:20 schrieb Graham Sider: <blockquote type="cite">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. v3: - Use amdgpu_vm_bo_lookup_mapping for wptr_bo mapping lookup - Move wptr_bo refcount increment to </blockquote> amdgpu_amdkfd_map_gtt_bo_to_gart <blockquote type="cite">- Remove list_del_init from amdgpu_amdkfd_map_gtt_bo_to_gart - Cleanup/fix create_queue wptr_bo error handling Signed-off-by: Graham Sider <a class="moz-txt-link-rfc2396E" href="mailto:Graham.Sider@amd.com"><Graham.Sider@amd.com></a> --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 1 + .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 49 </blockquote> +++++++++++++++++++ <blockquote type="cite"> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 37 +++++++++++++- .../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 | 17 +++++-- 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 </blockquote> amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct amdgpu_device *adev, <blockquote type="cite"> struct kgd_mem *mem, void **kptr, uint64_t *size); void amdgpu_amdkfd_gpuvm_unmap_gtt_bo_from_kernel(struct </blockquote> amdgpu_device *adev, <blockquote type="cite"> 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..888d08128a94 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -2030,6 +2030,55 @@ int amdgpu_amdkfd_gpuvm_sync_memory( return ret; } +/** + * amdgpu_amdkfd_map_gtt_bo_to_gart - Map BO to GART and </blockquote> increment <blockquote type="cite">+reference count + * @adev: Device to which allocated BO belongs + * @bo: Buffer object to be mapped + * + * Before return, bo reference count is incremented. To release the +reference and unpin/ + * unmap the BO, call amdgpu_amdkfd_free_gtt_mem. + */ +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; + } </blockquote> Oh! Is that something we do for every MQD? When yes that here is pretty much a NAK. We can't do this or create a trivial deny of service attack against the kernel driver. Regards, Christian. </blockquote> Hi Christian, could you elaborate on this? Right now this is only being used to pin the queue wptr BO. </blockquote> Well is this wptr BO per process, per queue or global? amdgpu_bo_pin() is only allowed if we pin global resources, otherwise I have to reject that. </blockquote> wptr BO is per queue, allocated as queue structure, 1 page size on system memory. KFD limit number of queues globally, max_queues = 127; /* HWS limit */, so this will pin max 508KB and take max 127 GART page mapping. wptr is updated by app and read by HWS, if we don't pin wptr, we have to evict queue when wptr bo is moved on system memory, then update GART mapping and restore queue. Regards, Philip <blockquote type="cite" cite="mid:26836141-d84a-363e-32c8-bd65dc8cdd22@amd.com"> Regards, Christian. <blockquote type="cite"> Best, Graham <blockquote type="cite"> <blockquote type="cite">+ + 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); + + bo = amdgpu_bo_ref(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 </blockquote> amdgpu_device *adev, <blockquote type="cite"> 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..1789ed8b79f5 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c @@ -289,6 +289,7 @@ static int kfd_ioctl_create_queue(struct file *filep, </blockquote> struct kfd_process *p, <blockquote type="cite"> struct kfd_process_device *pdd; struct queue_properties q_properties; uint32_t doorbell_offset_in_process = 0; + struct amdgpu_bo *wptr_bo = NULL; memset(&q_properties, 0, sizeof(struct queue_properties)); @@ -316,12 +317,41 @@ static int kfd_ioctl_create_queue(struct file </blockquote> *filep, struct kfd_process *p, <blockquote type="cite"> goto err_bind_process; } + /* Starting with GFX11, wptr BOs must be mapped to GART for MES </blockquote> to determine work <blockquote type="cite">+ * on unmapped queues for usermode queue oversubscription (no </blockquote> aggregated doorbell) <blockquote type="cite">+ */ + if (dev->shared_resources.enable_mes && (dev->adev- mes.sched_version & 0xff) >= 3) { + struct amdgpu_bo_va_mapping *wptr_mapping; + struct amdgpu_vm *wptr_vm; + + wptr_vm = drm_priv_to_vm(pdd->drm_priv); + err = amdgpu_bo_reserve(wptr_vm->root.bo, false); + if (err) + goto err_wptr_map_gart; + + wptr_mapping = amdgpu_vm_bo_lookup_mapping( + wptr_vm, args->write_pointer_address >> </blockquote> PAGE_SHIFT); <blockquote type="cite">+ amdgpu_bo_unreserve(wptr_vm->root.bo); + if (!wptr_mapping) { + pr_err("Failed to lookup wptr bo\n"); + err = -EINVAL; + goto err_wptr_map_gart; + } + + wptr_bo = wptr_mapping->bo_va->base.bo; + err = amdgpu_amdkfd_map_gtt_bo_to_gart(dev->adev, </blockquote> wptr_bo); <blockquote type="cite">+ if (err) { + pr_err("Failed to map wptr bo to GART\n"); + goto err_wptr_map_gart; + } + } + 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, </blockquote> &queue_id, NULL, NULL, NULL, <blockquote type="cite">- &doorbell_offset_in_process); + err = pqm_create_queue(&p->pqm, dev, filep, &q_properties, </blockquote> &queue_id, wptr_bo, <blockquote type="cite">+ NULL, NULL, NULL, &doorbell_offset_in_process); if (err != 0) goto err_create_queue; @@ -354,6 +384,9 @@ static int kfd_ioctl_create_queue(struct file *filep, </blockquote> struct kfd_process *p, <blockquote type="cite"> return 0; err_create_queue: + if (wptr_bo) + amdgpu_amdkfd_free_gtt_mem(dev->adev, wptr_bo); +err_wptr_map_gart: 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 </blockquote> device_queue_manager *dqm, struct queue *q, <blockquote type="cite"> 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 </blockquote> device_queue_manager *dqm, struct queue *q, AMDGPU_MES_PRIORITY_LEVEL_NORMAL; <blockquote type="cite"> 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 - </blockquote> (uint64_t)q->wptr_bo->kfd_bo->va; <blockquote type="cite">+ 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 </blockquote> *mm, void *mqd, <blockquote type="cite"> m->sdmax_rlcx_rb_base_hi = upper_32_bits(q->queue_address >> </blockquote> 8); <blockquote type="cite"> 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 << </blockquote> SDMA0_QUEUE0_DOORBELL_OFFSET__OFFSET__SHIFT; <blockquote type="cite">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 </blockquote> process_queue_manager *pqm, <blockquote type="cite"> 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..3a17c1ebc527 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 </blockquote> *pqm) <blockquote type="cite"> 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,7 @@ static int init_user_queue(struct </blockquote> process_queue_manager *pqm, <blockquote type="cite"> goto cleanup; } memset((*q)->gang_ctx_cpu_ptr, 0, </blockquote> AMDGPU_MES_GANG_CTX_SIZE); <blockquote type="cite">+ (*q)->wptr_bo = wptr_bo; } pr_debug("PQM After init queue"); @@ -237,6 +239,7 @@ int pqm_create_queue(struct </blockquote> process_queue_manager *pqm, <blockquote type="cite"> 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 +302,7 </blockquote> @@ int <blockquote type="cite">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, </blockquote> wptr_bo, <blockquote type="cite">+*qid); if (retval != 0) goto err_create_queue; pqn->q = q; @@ -320,7 +323,7 @@ int pqm_create_queue(struct </blockquote> process_queue_manager *pqm, <blockquote type="cite"> goto err_create_queue; } - retval = init_user_queue(pqm, dev, &q, properties, f, *qid); + retval = init_user_queue(pqm, dev, &q, properties, f, </blockquote> wptr_bo, <blockquote type="cite">+*qid); if (retval != 0) goto err_create_queue; pqn->q = q; @@ -457,9 +460,13 @@ int pqm_destroy_queue(struct </blockquote> process_queue_manager *pqm, unsigned int qid) <blockquote type="cite"> 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 +907,7 @@ int kfd_criu_restore_queue(struct kfd_process *p, print_queue_properties(&qp); - ret = pqm_create_queue(&p->pqm, pdd->dev, NULL, &qp, </blockquote> &queue_id, q_data, mqd, ctl_stack, <blockquote type="cite">+ ret = pqm_create_queue(&p->pqm, pdd->dev, NULL, &qp, </blockquote> &queue_id, <blockquote type="cite">+NULL, q_data, mqd, ctl_stack, NULL); if (ret) { pr_err("Failed to create new queue err:%d\n", ret); </blockquote> </blockquote> </blockquote> </blockquote> </body> </html>