Skip to content

Commit 465f1db

Browse files
baluvivekrodrigovivi
authored andcommitted
drm/xe/devcoredump: Defer devcoredump initialization during probe
Doing devcoredump initializing before GT though look harmless, it leads to problem during driver unbind. Because of this order, GT/Engine release functions will be called before xe devcoredump release function (xe_driver_devcoredump_fini) leading to the following kernel crash[1] because the devcoredump functions might still use GT/Engine datastructures after those are freed. The following crash is observed while running the IGT xe_wedged@wedged-at-any-timeout. The test forces a wedged state by submitting a workload which hangs. Then does a unbind/rebind of the driver to recover from the wedged state. The hanged workload leads to a devcoredump. The following crash is noticed when the devcoredump capture races with the driver unbind. During driver unbind, the release function hw_engine_fini() will be called which assigns NULL to hwe->gt. But the same data structure is accessed during the coredump capture in the function xe_engine_snapshot_print by reading snapshot->hwe->gt. With this patch, we make sure the devcoredump is stopped before deinitializing the core driver functions. [1]: BUG: kernel NULL pointer dereference, address: 0000000000000000 Workqueue: events_unbound xe_devcoredump_deferred_snap_work [xe] RIP: 0010:xe_engine_snapshot_print+0x47/0x420 [xe] Call Trace: <TASK> ? drm_printf+0x64/0x90 __xe_devcoredump_read+0x23f/0x2d0 [xe] ? __pfx___drm_printfn_coredump+0x10/0x10 ? __pfx___drm_puts_coredump+0x10/0x10 xe_devcoredump_deferred_snap_work+0x17a/0x190 [xe] process_one_work+0x22e/0x6f0 worker_thread+0x1e8/0x3d0 ? __pfx_worker_thread+0x10/0x10 kthread+0x11f/0x250 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x47/0x70 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1a/0x30 v2: Detailed commit description (Rodrigo) v3: FIXME added (Rodrigo, Stuart) Fixes: 4209d63 ("drm/xe: Remove devcoredump during driver release") Reviewed-by: Stuart Summers <stuart.summers@intel.com> Link: https://lore.kernel.org/r/20250731061300.14320-1-balasubramani.vivekanandan@intel.com Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com> Link: https://lore.kernel.org/r/20250801052356.21885-1-balasubramani.vivekanandan@intel.com Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> (cherry picked from commit 1fdc4c3) Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
1 parent df9bdd4 commit 465f1db

File tree

2 files changed

+10
-4
lines changed

2 files changed

+10
-4
lines changed

drivers/gpu/drm/xe/xe_device.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -802,10 +802,6 @@ int xe_device_probe(struct xe_device *xe)
802802
return err;
803803
}
804804

805-
err = xe_devcoredump_init(xe);
806-
if (err)
807-
return err;
808-
809805
/*
810806
* From here on, if a step fails, make sure a Driver-FLR is triggereed
811807
*/
@@ -870,6 +866,10 @@ int xe_device_probe(struct xe_device *xe)
870866
XE_WA(xe->tiles->media_gt, 15015404425_disable))
871867
XE_DEVICE_WA_DISABLE(xe, 15015404425);
872868

869+
err = xe_devcoredump_init(xe);
870+
if (err)
871+
return err;
872+
873873
xe_nvm_init(xe);
874874

875875
err = xe_heci_gsc_init(xe);

drivers/gpu/drm/xe/xe_guc_capture.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1817,6 +1817,12 @@ void xe_engine_snapshot_print(struct xe_hw_engine_snapshot *snapshot, struct drm
18171817
str_yes_no(snapshot->kernel_reserved));
18181818

18191819
for (type = GUC_STATE_CAPTURE_TYPE_GLOBAL; type < GUC_STATE_CAPTURE_TYPE_MAX; type++) {
1820+
/*
1821+
* FIXME: During devcoredump print we should avoid accessing the
1822+
* driver pointers for gt or engine. Printing should be done only
1823+
* using the snapshot captured. Here we are accessing the gt
1824+
* pointer. It should be fixed.
1825+
*/
18201826
list = xe_guc_capture_get_reg_desc_list(gt, GUC_CAPTURE_LIST_INDEX_PF, type,
18211827
capture_class, false);
18221828
snapshot_print_by_list_order(snapshot, p, type, list);

0 commit comments

Comments
 (0)