diff --git a/drivers/gpu/drm/xe/xe_hw_engine_group.c b/drivers/gpu/drm/xe/xe_hw_engine_group.c index 02cf32ae5aa9..0804b426b6f9 100644 --- a/drivers/gpu/drm/xe/xe_hw_engine_group.c +++ b/drivers/gpu/drm/xe/xe_hw_engine_group.c @@ -34,6 +34,15 @@ hw_engine_group_resume_lr_jobs_func(struct work_struct *w) if (!xe_vm_in_fault_mode(q->vm)) continue; + /* + * Only resume queues that were actually suspended. A queue whose + * suspend() failed (e.g. killed/banned/wedged) was never + * suspended, so it must not be resumed. + */ + if (!READ_ONCE(q->lr.suspended)) + continue; + + WRITE_ONCE(q->lr.suspended, false); q->ops->resume(q); } @@ -140,7 +149,18 @@ int xe_hw_engine_group_add_exec_queue(struct xe_hw_engine_group *group, struct x return err; if (xe_vm_in_fault_mode(q->vm) && group->cur_mode == EXEC_MODE_DMA_FENCE) { - q->ops->suspend(q); + /* + * suspend() can fail (e.g. killed/banned/wedged), leaving the + * queue un-suspended. Propagate the failure so the queue is not + * added; on failure nothing was suspended, so there is nothing to + * undo. Only record the queue as suspended (and later resume it) + * once suspend() has succeeded. + */ + err = q->ops->suspend(q); + if (err) + goto err_suspend; + + WRITE_ONCE(q->lr.suspended, true); err = q->ops->suspend_wait(q); if (err) goto err_suspend; @@ -216,8 +236,22 @@ static int xe_hw_engine_group_suspend_faulting_lr_jobs(struct xe_hw_engine_group return -EAGAIN; xe_gt_stats_incr(q->gt, XE_GT_STATS_ID_HW_ENGINE_GROUP_SUSPEND_LR_QUEUE_COUNT, 1); + /* + * suspend() only fails when the queue is killed/banned/wedged. + * Such a queue is being torn down (its removal from HW is handled + * by the kill/ban teardown), so it is not a live fault-mode + * context the mode switch must preempt. Skip it rather than + * failing the switch, otherwise one dying sibling would block a + * dma-fence submission on the healthy queues in the group. Only + * queues recorded as suspended below are later waited on and + * resumed. + */ + err = q->ops->suspend(q); + if (err) + continue; + + WRITE_ONCE(q->lr.suspended, true); need_resume = true; - q->ops->suspend(q); gt = q->gt; } @@ -225,9 +259,13 @@ static int xe_hw_engine_group_suspend_faulting_lr_jobs(struct xe_hw_engine_group if (!xe_vm_in_fault_mode(q->vm)) continue; + /* Only wait on queues that were actually suspended above. */ + if (!READ_ONCE(q->lr.suspended)) + continue; + err = q->ops->suspend_wait(q); if (err) - return err; + goto err_resume; } if (gt) { @@ -240,6 +278,47 @@ static int xe_hw_engine_group_suspend_faulting_lr_jobs(struct xe_hw_engine_group xe_hw_engine_group_resume_faulting_lr_jobs(group); return 0; + +err_resume: + /* + * A suspend_wait() failed partway through the mode switch. Resume the + * sibling queues that were already suspended in this call so they are + * not left suspended forever. + * + * resume() requires the suspend to have completed (suspend_pending + * cleared) or it trips the !suspend_pending assert. So skip the resume + * when either: + * - suspend_wait_blocking() fails: on a GuC timeout it bans the queue + * and triggers cleanup, so the queue is being torn down; or + * - reset_status() is true: the queue was reset/killed/banned/wedged. + * suspend_wait() can return success in this case via its killed/ + * stopped wait condition while suspend_pending is still set, and the + * queue is being torn down anyway, so its state is resolved by + * teardown rather than by a resume here. + * In either case leave the queue marked suspended. + * + * Use the *blocking* (uninterruptible) wait here: the queues resumed on + * this path may belong to a different process than the one that + * triggered the mode switch. An interruptible suspend_wait() would + * return -ERESTARTSYS if the triggering task is signalled, skip the + * resume, and leave the other process's queue suspended forever + * (cross-process DoS). + */ + list_for_each_entry(q, &group->exec_queue_list, hw_engine_group_link) { + if (!xe_vm_in_fault_mode(q->vm)) + continue; + + if (!READ_ONCE(q->lr.suspended)) + continue; + + if (q->ops->suspend_wait_blocking(q) || q->ops->reset_status(q)) + continue; + + WRITE_ONCE(q->lr.suspended, false); + q->ops->resume(q); + } + + return err; } /**