On 9/11/26 10:22, Prike Liang wrote:
> When a queue encounters a reset error and fallback to device reset,
> mark both the queue and its owning process as guilty so that userspace
> is notified that the offending queue is no longer usable and should
> recreate the context before resubmitting work.
>
> Signed-off-by: Prike Liang <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 4 +++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 24 ++++++++++++++++++++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h | 5 ++++-
> 3 files changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index b15ed4a534f1..038d18faf344 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -589,7 +589,9 @@ static int amdgpu_ctx_query2(struct amdgpu_device *adev,
> if (ctx->generation != amdgpu_vm_generation(adev, &fpriv->vm))
> out->state.flags |= AMDGPU_CTX_QUERY2_FLAGS_VRAMLOST;
>
> - if (amdgpu_ctx_guilty(ctx))
> + if (amdgpu_ctx_guilty(ctx) ||
> + ((out->state.flags & AMDGPU_CTX_QUERY2_FLAGS_RESET) &&
> + fpriv->userq_mgr.guilty))
> out->state.flags |= AMDGPU_CTX_QUERY2_FLAGS_GUILTY;
>
> if (amdgpu_in_reset(adev))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 6c92575213c1..7098d550c0ee 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -173,8 +173,12 @@ static void amdgpu_userq_hang_detect_work(struct
> work_struct *work)
> * Don't schedule the work here! Scheduling or queue work from one reset
> * handler to another is illegal if you don't take extra precautions!
> */
> - if (gpu_reset)
> + if (gpu_reset) {
> + /* Block re-map until userspace recreates the queue. */
> + queue->guilty = true;
> + queue->userq_mgr->guilty = true;
Clear NAK to having a global guilty flag in the userq_mgr. Each queue should be
handled completely separated.
The only case where that makes sense would be gang submitted queues, but we
haven't implemented those.
Regards,
Christian.
> amdgpu_userq_mgr_reset_work(&queue->userq_mgr->reset_work);
> + }
> }
>
> /*
> @@ -389,8 +393,17 @@ static int amdgpu_userq_map_helper(struct
> amdgpu_usermode_queue *queue)
> int r;
>
> if (queue->state == AMDGPU_USERQ_STATE_UNMAPPED) {
> - trace_amdgpu_userq_state_start(queue);
>
> + if (queue->guilty) {
> + /*
> + * A queue that was reset for hanging must not be
> + * re-mapped: replaying its command stream faults
> + * again. Leave it unmapped for userspace to recreate.
> + */
> + return 0;
> + }
> +
> + trace_amdgpu_userq_state_start(queue);
> r = userq_funcs->map(queue);
> if (r) {
> trace_amdgpu_userq_state_changed(queue,
> AMDGPU_USERQ_STATE_HUNG);
> @@ -833,7 +846,8 @@ amdgpu_userq_query_status(struct drm_file *filp, union
> drm_amdgpu_userq *args)
> args->out_qs.flags = 0;
> if (queue->state == AMDGPU_USERQ_STATE_HUNG)
> args->out_qs.flags |= AMDGPU_USERQ_QUERY_STATUS_FLAGS_HUNG;
> - if (queue->generation != amdgpu_vm_generation(adev, &fpriv->vm))
> + if (queue->generation != amdgpu_vm_generation(adev, &fpriv->vm) ||
> + fpriv->userq_mgr.guilty)
> args->out_qs.flags |= AMDGPU_USERQ_QUERY_STATUS_FLAGS_VRAMLOST;
> if (queue->state == AMDGPU_USERQ_STATE_INVALID_VA)
> args->out_qs.flags |=
> AMDGPU_USERQ_QUERY_STATUS_FLAGS_INVALID_VA;
> @@ -1594,6 +1608,10 @@ int amdgpu_userq_post_reset(struct amdgpu_device
> *adev, bool vram_lost)
> int ret = 0, r;
>
> xa_for_each(&adev->userq_doorbell_xa, queue_id, queue) {
> + /* Guilty queues are not restored; userspace recreates them. */
> + if (queue->guilty)
> + continue;
> +
> if (queue->state == AMDGPU_USERQ_STATE_HUNG && !vram_lost) {
> trace_amdgpu_userq_state_start(queue);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> index 40228c60a1ea..f2eca744a3c5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> @@ -51,6 +51,8 @@ struct amdgpu_userq_obj {
> struct amdgpu_usermode_queue {
> int queue_type;
> enum amdgpu_userq_state state;
> + /* Reset due to a hang; must not be re-mapped, userspace recreates. */
> + bool guilty;
> uint64_t doorbell_handle;
> uint64_t doorbell_index;
> u32 doorbell_offset;
> @@ -132,7 +134,8 @@ struct amdgpu_userq_mgr {
> struct drm_file *file;
> struct mutex proc_ctx_lock;
> struct amdgpu_userq_obj proc_ctx_obj;
> -
> + /* Set when any queue owned by this process is reset guilty */
> + bool guilty;
> bool proc_ctx_allocated;
> uint32_t proc_ctx_array_index;
> /**
