Re: [PATCH v10 26/27] drm/xe/eudebug: Introduce EU pagefault handling interface

[Maciej Patelczyk <maciej.patelczyk@intel.com>]

On 03/09/2026 17:43, sashiko-bot@kernel.org wrote:
 > Thank you for your contribution! Sashiko AI review found 6 potential 
issue(s) to consider:
 > - [Critical] Race condition in `queue_pagefault` when accessing 
`epf->d` after unlocking causes Use-After-Free and double-put.
True. Addressed.
 >
 > - [High] Unverified ASID in `xe_eudebug_pagefault_start` groups 
unrelated pagefaults, causing VMA destruction under the wrong VM lock 
and triggering lockdep failures.
Addressed, but not sure it's valid.
 >
 > - [High] Missing NULL check for `vm->xef` in 
`xe_eudebug_pagefault_start` and `xe_eudebug_pagefault_creatable` causes 
a kernel panic for kernel/migrate VMs.
Addressed.
 >
 > - [High] Non-recoverable error in `send_queued_pagefaults` triggers a 
full GPU reset, enabling a Denial of Service via queue destruction.
If EU Debugger fails at later stages of processing given page fault, a 
GPU reset is actually required.
 >
 > - [High] Deferring finalization in `xe_eudebug_pagefault_end` hangs 
the state machine if subsequent pagefaults fail to initialize.
Not really sure if this is an issue.
 >
 > - [Medium] Failure to clear `TD_CTL_FORCE_EXCEPTION` in 
`eudebug_pagefault_finalize` ignores a hardware timeout without 
triggering a recovery reset.

Addressed.


 > [ ... ]
 >
 >> +void xe_eudebug_pagefault_end(void *private, int err)
 >> +{
 >> +    struct xe_eudebug_pagefault *epf = _get_epf(private);
 >> +    struct xe_eudebug_pagefault *first_epf = NULL;
 >> +    struct xe_pagefault_queue *pf_queue;
 >> +    struct xe_device *xe;
 >> +    struct xe_eudebug *d;
 >> +    struct xe_vm *vm;
 >> +    int resume_attn = 0, destroy_pf = 0;
 >> +
 >> +    if (!epf)
 >> +        return;
 >> +
 >> +    xe = epf->q->xef->xe;
 >> +    pf_queue = &xe->usm.pf_queue;
 >> +
 >> +    d = epf->d;
 >> +    vm = xe_vm_get(epf->q->vm);
 >> +    /* Need to have outher vm lock for eudebug lock as for pf start */
 >> +    down_write(&vm->lock);
 >> +    mutex_lock(&d->xe->eudebug.lock);
 >> +    if (!--d->pf.active) {
 >> +        /*
 >> +         * This will also catch entries that are popped from pagefault
 >> +         * queue and moved to worker but not yet processed.
 >> +         * xe_eudebug_pagefault_start() will catch later it and 
increase
 >> +         * the pf.active counter.
 >> +         */
 >> +        if (!xe_eudebug_pagefault_queue_has_asid(pf_queue, 
d->pf.asid)) {
 >> +            if (!err)
 >> + xe_eudebug_pagefault_process(d->pf.first);
 >> +
 >> +            destroy_pf = eudebug_pagefault_finalize(d->pf.first, err);
 >> +            /* If d->pf.first was queued the epf shall be removed. */
 >> +            if (epf != d->pf.first) {
 >> +                if (destroy_pf)
 >> +                    first_epf = d->pf.first;
 >> +                else
 >> +                    destroy_pf = 1;
 >> +            }
 >> +            d->pf.first = NULL;
 >> +            d->pf.asid = 0;
 >> +            resume_attn = 1;
 >> +        } else {
 >> +            /* page faults still queued */
 >> +            if (epf != d->pf.first)
 >> +                destroy_pf = 1;
 >> +        }
 >
 > [Severity: High]
 > Does deferring finalization when page faults are still queued risk 
deadlocking
 > the state machine?
 >
 > If xe_eudebug_pagefault_queue_has_asid() is true, 
eudebug_pagefault_finalize()
 > is deferred on the assumption that the next queued page fault will 
successfully
 > attach to the eudebug state machine.
 >
 > If the next page fault fails in xe_eudebug_pagefault_start() (for 
example, if
 > the exec queue is no longer debuggable), eudebug setup is skipped.  
When that
 > fault completes, xe_eudebug_pagefault_end() will receive a NULL 
private pointer
 > and return early.
 >
 > Could this leave pf.active at 0 while eudebug_pagefault_finalize() is 
never
 > called, permanently hanging the GPU with TD_CTL_FORCE_EXCEPTION still 
active
 > and deadlocking EU_CONTROL ioctls waiting on the unsignaled pf.fence?
 >
 > [ ... ]

Exec queue cannot stop being debuggable.

VMs without 'xef' don't have 'asid'. They will not be detected by 
xe_eudebug_pagefault_queue_has_asid().

And debugging a workload on GPU is a specific job which has severe side 
effects, including GPU reprogramming and reset. So only a workload under 
debug should be on GT.

  Therefore, despite the fact that issue is technically correct, should 
not appear.


Maciej