linux-stable.git/drivers/gpu/drm/xe/xe_exec_queue.c, branch v6.11

drm/xe: prevent UAF around preempt fence

2024-08-19T14:39:28+00:00

The fence lock is part of the queue, therefore in the current design
anything locking the fence should then also hold a ref to the queue to
prevent the queue from being freed.

However, currently it looks like we signal the fence and then drop the
queue ref, but if something is waiting on the fence, the waiter is
kicked to wake up at some later point, where upon waking up it first
grabs the lock before checking the fence state. But if we have already
dropped the queue ref, then the lock might already be freed as part of
the queue, leading to uaf.

To prevent this, move the fence lock into the fence itself so we don't
run into lifetime issues. Alternative might be to have device level
lock, or only release the queue in the fence release callback, however
that might require pushing to another worker to avoid locking issues.

Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
References: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2454
References: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2342
References: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2020
Signed-off-by: Matthew Auld 
Cc: Matthew Brost 
Cc:  # v6.8+
Reviewed-by: Matthew Brost 
Link: https://patchwork.freedesktop.org/patch/msgid/20240814110129.825847-2-matthew.auld@intel.com
(cherry picked from commit 7116c35aacedc38be6d15bd21b2fc936eed0008b)
Signed-off-by: Rodrigo Vivi

drm/xe: Move VM dma-resv lock from xe_exec_queue_create to __xe_exec_queue_init

2024-08-19T14:39:04+00:00

The critical section which requires the VM dma-resv is the call
xe_lrc_create in __xe_exec_queue_init. Move this lock to
__xe_exec_queue_init holding it just around xe_lrc_create. Not only is
good practice, this also fixes a locking double of the VM dma-resv in
the error paths of __xe_exec_queue_init as xe_lrc_put tries to acquire
this too resulting in a deadlock.

Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
Cc: Maarten Lankhorst 
Signed-off-by: Matthew Brost 
Reviewed-by: Maarten Lankhorst 
Link: https://patchwork.freedesktop.org/patch/msgid/20240724152831.1848325-1-matthew.brost@intel.com
(cherry picked from commit 549dd786b61cd3db903f5d94d07fc5a89ccdbeb9)
Signed-off-by: Rodrigo Vivi

drm/xe: Fix use after free when client stats are captured

2024-08-15T13:44:22+00:00

xe_file_close triggers an asynchronous queue cleanup and then frees up
the xef object. Since queue cleanup flushes all pending jobs and the KMD
stores client usage stats into the xef object after jobs are flushed, we
see a use-after-free for the xef object. Resolve this by taking a
reference to xef from xe_exec_queue.

While at it, revert an earlier change that contained a partial work
around for this issue.

v2:
- Take a ref to xef even for the VM bind queue (Matt)
- Squash patches relevant to that fix and work around (Lucas)

v3: Fix typo (Lucas)

Fixes: ce62827bc294 ("drm/xe: Do not access xe file when updating exec queue run_ticks")
Fixes: 6109f24f87d7 ("drm/xe: Add helper to accumulate exec queue runtime")
Closes: https://gitlab.freedesktop.org/drm/xe/kernel/issues/1908
Signed-off-by: Umesh Nerlige Ramappa 
Reviewed-by: Matthew Brost 
Reviewed-by: Lucas De Marchi 
Link: https://patchwork.freedesktop.org/patch/msgid/20240718210548.3580382-5-umesh.nerlige.ramappa@intel.com
Signed-off-by: Lucas De Marchi 
(cherry picked from commit 2149ded63079449b8dddf9da38392632f155e6b5)
Signed-off-by: Rodrigo Vivi

drm/xe/exec_queue: Rename xe_exec_queue::compute to xe_exec_queue::lr

2024-06-17T20:46:52+00:00

The properties of this struct are used in long running context so
make that clear by renaming it to lr, in alignment with the rest
of the code.

Cc: Matthew Brost 
Signed-off-by: Francois Dugast 
Reviewed-by: Matthew Brost 
Link: https://patchwork.freedesktop.org/patch/msgid/20240613170348.723245-1-francois.dugast@intel.com
Signed-off-by: Rodrigo Vivi

drm/xe: Drop EXEC_QUEUE_FLAG_BANNED

2024-06-07T16:16:36+00:00

Clean up laying violation of setting q->flags EXEC_QUEUE_FLAG_BANNED bit
in GuC backend. Move banned to GuC owned bit and report banned status to
upper layers via reset_status vfunc. This is a slight change in behavior
as reset_status returns true if wedged or killed bits set too, but in
all of these cases submission to queue is no longer allowed.

Signed-off-by: Matthew Brost 
Reviewed-by: Jonathan Cavitt 
Link: https://patchwork.freedesktop.org/patch/msgid/20240604184700.1946918-1-matthew.brost@intel.com
Signed-off-by: Rodrigo Vivi

drm/xe: Decouple xe_exec_queue and xe_lrc

2024-05-30T06:44:41+00:00

Decouple xe_lrc from xe_exec_queue and reference count xe_lrc.
Removing hard coupling between xe_exec_queue and xe_lrc allows
flexible design where the user interface xe_exec_queue can be
destroyed independent of the hardware/firmware interface xe_lrc.

v2: Fix lrc indexing in wq_item_append()

Signed-off-by: Niranjana Vishwanathapura 
Reviewed-by: Matthew Brost 
Signed-off-by: Matthew Brost 
Link: https://patchwork.freedesktop.org/patch/msgid/20240530032211.29299-1-niranjana.vishwanathapura@intel.com

drm/xe: Do not access xe file when updating exec queue run_ticks

2024-05-27T21:07:46+00:00

The current code is running into a use after free case where xe file is
closed before the exec queue run_ticks can be updated. This is occurring
in the xe_file_close path. To fix that, do not access xe file when
updating the exec queue run_ticks. Instead store the exec queue run_ticks
locally in the exec queue object and accumulate it when the user dumps
the drm client stats. We know that the xe file is valid when user is
dumping the run_ticks for the drm client, so this effectively
removes the dependency on xe file object in
xe_exec_queue_update_run_ticks().

v2:
- Fix the accumulation of q->run_ticks delta into xe file run_ticks
- s/runtime/run_ticks/ (Rodrigo)

Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1908
Fixes: 6109f24f87d7 ("drm/xe: Add helper to accumulate exec queue runtime")
Signed-off-by: Umesh Nerlige Ramappa 
Reviewed-by: Rodrigo Vivi 
Link: https://patchwork.freedesktop.org/patch/msgid/20240524234744.1352543-2-umesh.nerlige.ramappa@intel.com

drm/xe: Use run_ticks instead of runtime for client stats

2024-05-27T21:07:44+00:00

Note that runtime is also used in the pm context, so it is confusing to
use the same name to denote run time of the drm client. Use a more
appropriate name for the client utilization.

While at it, drop the incorrect multi-lrc comment in the helper
description

v2: s/show_runtime/show_run_ticks/ (Rodrigo)

Signed-off-by: Umesh Nerlige Ramappa 
Reviewed-by: Rodrigo Vivi 
Link: https://patchwork.freedesktop.org/patch/msgid/20240524234744.1352543-1-umesh.nerlige.ramappa@intel.com

drm/xe: Don't initialize fences at xe_sched_job_create()

2024-05-27T19:26:03+00:00

Pre-allocate but don't initialize fences at xe_sched_job_create(),
and initialize / arm them instead at xe_sched_job_arm(). This
makes it possible to move xe_sched_job_create() with its memory
allocation out of any lock that is required for fence
initialization, and that may not allow memory allocation under it.

Replaces the struct dma_fence_array for parallell jobs with a
struct dma_fence_chain, since the former doesn't allow
a split-up between allocation and initialization.

v2:
- Rebase.
- Don't always use the first lrc when initializing parallel
  lrc fences.
- Use dma_fence_chain_contained() to access the lrc fences.

v4:
- Add an assert that job->lrc_seqno == fence->seqno.
  (Matthew Brost)

Signed-off-by: Thomas Hellström 
Reviewed-by: Matthew Brost 
Reviewed-by: Rodrigo Vivi  #v2
Link: https://patchwork.freedesktop.org/patch/msgid/20240527135912.152156-4-thomas.hellstrom@linux.intel.com

drm/xe: Decouple job seqno and lrc seqno

2024-05-27T19:25:59+00:00

Tightly coupling these seqno presents problems if alternative fences for
jobs are used. Decouple these for correctness.

v2:
- Slightly reword commit message (Thomas)
- Make sure the lrc fence ops are used in comparison (Thomas)
- Assume seqno is unsigned rather than signed in format string (Thomas)

Cc: Thomas Hellström 
Signed-off-by: Matthew Brost 
Signed-off-by: Thomas Hellström 
Reviewed-by: Rodrigo Vivi 
Link: https://patchwork.freedesktop.org/patch/msgid/20240527135912.152156-2-thomas.hellstrom@linux.intel.com