On 6/24/26 05:26, Geoffrey McRae wrote:
> MES ADD_QUEUE programs the firmware with the queue type from the driver
> input, but MES REMOVE_QUEUE leaves queue_type at the zero-initialized
> value. Zero decodes as GFX in the MES REMOVE_QUEUE packet.
>
> That means removing a KFD compute queue can be submitted to MES as a GFX
> queue. In a debug-trap suspend/remove sequence this can leave MES
> looking for the doorbell in the wrong queue class and the REMOVE_QUEUE
> command may never complete. The observed failing packet removed
> doorbell 0x1002 with queue_type=GFX even though the corresponding
> ADD_QUEUE for the same doorbell was queue_type=COMPUTE.
>
> Populate REMOVE_QUEUE.queue_type the same way ADD_QUEUE does.
>
> Signed-off-by: Geoffrey McRae <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: Christian König <[email protected]>
Of hand looks correct to me, but Sunil can you take a look as well.
Thanks,
Christian.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h | 1 +
> drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 1 +
> drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 2 ++
> drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 2 ++
> drivers/gpu/drm/amd/amdgpu/mes_v12_1.c | 3 +++
> drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 ++
> 6 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> index 5255360353f4..dbedb1e47c3f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
> @@ -274,6 +274,7 @@ struct mes_remove_queue_input {
> uint32_t xcc_id;
> uint32_t doorbell_offset;
> uint64_t gang_context_addr;
> + uint32_t queue_type;
> bool remove_queue_after_reset;
> };
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> index 5ad8dd18dc67..f0f12c81dd5d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> @@ -170,6 +170,7 @@ static int mes_userq_unmap(struct amdgpu_usermode_queue
> *queue)
> memset(&queue_input, 0x0, sizeof(struct mes_remove_queue_input));
> queue_input.doorbell_offset = queue->doorbell_index;
> queue_input.gang_context_addr = ctx->gpu_addr;
> + queue_input.queue_type = queue->queue_type;
>
> amdgpu_mes_lock(&adev->mes);
> r = adev->mes.funcs->remove_hw_queue(&adev->mes, &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 9e27d01cbfa3..76e6769cf7ac 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> @@ -383,6 +383,8 @@ static int mes_v11_0_remove_hw_queue(struct amdgpu_mes
> *mes,
>
> mes_remove_queue_pkt.doorbell_offset = input->doorbell_offset;
> mes_remove_queue_pkt.gang_context_addr = input->gang_context_addr;
> + mes_remove_queue_pkt.queue_type =
> + convert_to_mes_queue_type(input->queue_type);
>
> if (mes_rev >= 0x60)
> mes_remove_queue_pkt.remove_queue_after_reset =
> input->remove_queue_after_reset;
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> index 20f4fd57b1da..1b0c649d97a2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> @@ -371,6 +371,8 @@ static int mes_v12_0_remove_hw_queue(struct amdgpu_mes
> *mes,
>
> mes_remove_queue_pkt.doorbell_offset = input->doorbell_offset;
> mes_remove_queue_pkt.gang_context_addr = input->gang_context_addr;
> + mes_remove_queue_pkt.queue_type =
> + convert_to_mes_queue_type(input->queue_type);
>
> if (mes_rev >= 0x5a)
> mes_remove_queue_pkt.remove_queue_after_reset =
> input->remove_queue_after_reset;
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v12_1.c
> b/drivers/gpu/drm/amd/amdgpu/mes_v12_1.c
> index 8007a6e69305..c449efa70b60 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_v12_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v12_1.c
> @@ -362,6 +362,8 @@ static int mes_v12_1_remove_hw_queue(struct amdgpu_mes
> *mes,
>
> mes_remove_queue_pkt.doorbell_offset = input->doorbell_offset;
> mes_remove_queue_pkt.gang_context_addr = input->gang_context_addr;
> + mes_remove_queue_pkt.queue_type =
> + convert_to_mes_queue_type(input->queue_type);
>
> return mes_v12_1_submit_pkt_and_poll_completion(mes,
> xcc_id, AMDGPU_MES_SCHED_PIPE,
> @@ -2270,6 +2272,7 @@ static int mes_v12_1_test_queue(struct amdgpu_device
> *adev, int xcc_id,
> remove_queue.xcc_id = xcc_id;
> remove_queue.doorbell_offset = doorbell_idx;
> remove_queue.gang_context_addr = add_queue.gang_context_addr;
> + remove_queue.queue_type = queue_type;
> r = mes_v12_1_remove_hw_queue(&adev->mes, &remove_queue);
>
> error:
> 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 9f28974f25b4..74cdaa8636c9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -299,6 +299,7 @@ static int remove_queue_mes_on_reset_option(struct
> device_queue_manager *dqm, st
> memset(&queue_input, 0x0, sizeof(struct mes_remove_queue_input));
> queue_input.doorbell_offset = q->properties.doorbell_off;
> queue_input.gang_context_addr = q->gang_ctx_gpu_addr;
> + queue_input.queue_type = convert_to_mes_queue_type(q->properties.type);
> queue_input.remove_queue_after_reset = flush_mes_queue;
> queue_input.xcc_id = ffs(dqm->dev->xcc_mask) - 1;
>
> @@ -467,6 +468,7 @@ static int reset_queues_mes(struct device_queue_manager
> *dqm, struct queue *q)
> memset(&queue_input, 0x0, sizeof(struct mes_remove_queue_input));
> queue_input.doorbell_offset = q->properties.doorbell_off;
> queue_input.gang_context_addr = q->gang_ctx_gpu_addr;
> + queue_input.queue_type = convert_to_mes_queue_type(q->properties.type);
> queue_input.remove_queue_after_reset = false;
> queue_input.xcc_id = ffs(dqm->dev->xcc_mask) - 1;
> /* pass the known bad queue info to the reset function */
