Re: [PATCH] drm/xe: Don't wedge shared engine on stale faults from torn-down VMs

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Sep 18, 2026 at 10:45:24PM +0530, Yadav, Sanjay Kumar wrote:
> 
> On 14-09-2026 16:05, Yadav, Sanjay Kumar wrote:
> > 
> > On 12-09-2026 02:15, Matthew Brost wrote:
> > > On Sat, Sep 12, 2026 at 12:53:01AM +0530, Sanjay Yadav wrote:
> > > > A recoverable page fault generated by a process's completed migrate/BCS
> > > > work can arrive after that process's VM has been closed and its ASID
> > > > removed (abnormal exit while GPU work was in flight).
> > > > xe_pagefault_service() returned an error for such faults, which is
> > > > reported to the GuC/HW as an unsuccessful response. Repeated
> > > > unsuccessful responses eventually escalate to an engine memory CAT
> > > > error. Because the migrate/BCS engine is shared, that CAT error
> > > > wedges it for unrelated processes, which then hang on their own
> > > > copies.
> > > Yikes. Do you have a reproducer for this? I wouldn't expect this
> > > behavior from the hardware, as it appears to violate the expected
> > > process isolation contract.
> > Yes, there is a reproducer, "l0-abnormal-exit-repro", in GSD-13290 on
> > BMG. I have also attached the dmesg logs
> > in the GSD from the drm-tip kernel, as well as from the older 7.0
> > kernel, where the issue was originally reported.
> > > > Detect stale faults whose owning VM is closed or whose ASID no longer
> > > > maps to a fault-capable VM, and drain them instead of replying
> > > > unsuccessful.
> > > > 
> > > > Cc: Matthew Brost <matthew.brost@intel.com>
> > > > Cc: Stuart Summers <stuart.summers@intel.com>
> > > Fixes, CC stable if this is an issue.
> > sure
> > > 
> > > > Assisted-by: GitHub Copilot:claude-opus-4.8
> > > > Signed-off-by: Sanjay Yadav <sanjay.kumar.yadav@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/xe/xe_pagefault.c | 22 +++++++++++++++++++---
> > > >   1 file changed, 19 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/xe/xe_pagefault.c
> > > > b/drivers/gpu/drm/xe/xe_pagefault.c
> > > > index c82b8bc8bc70..2827652ab8a3 100644
> > > > --- a/drivers/gpu/drm/xe/xe_pagefault.c
> > > > +++ b/drivers/gpu/drm/xe/xe_pagefault.c
> > > > @@ -238,8 +238,10 @@ static struct xe_vm
> > > > *xe_pagefault_asid_to_vm(struct xe_device *xe, u32 asid)
> > > >       vm = xa_load(&xe->usm.asid_to_vm, asid);
> > > >       if (vm && xe_vm_in_fault_mode(vm))
> > > >           xe_vm_get(vm);
> > > > -    else
> > > > +    else if (vm)
> > > >           vm = ERR_PTR(-EINVAL);
> > > > +    else
> > > > +        vm = ERR_PTR(-ENOENT);
> > > >       up_read(&xe->usm.lock);
> > > >         return vm;
> > > > @@ -260,13 +262,27 @@ static int xe_pagefault_service(struct
> > > > xe_pagefault *pf)
> > > >           return -EFAULT;
> > > >         vm = xe_pagefault_asid_to_vm(xe, asid);
> > > > -    if (IS_ERR(vm))
> > > > +    if (IS_ERR(vm)) {
> > > > +        if (PTR_ERR(vm) == -ENOENT) {
> > > > +            drm_info(&xe->drm,
> > > > +                 "xe_pf_debug: drain stale fault (no VM)
> > > > asid=%u addr=0x%llx\n",
> > > > +                 asid, pf->consumer.page_addr);
> > > > +            xe_pagefault_set_start_addr(pf, pf->consumer.page_addr);
> > > > +            xe_pagefault_set_end_addr(pf, pf->consumer.page_addr);
> > > > +            return 0;
> > > Won't the page walker hotspin on faults until the executing queue is
> > > descheduled from the hardware if we successfully acknowledge them? That
> > > doesn't seem ideal.
> > > 
> > > Matt
> > agree with your point
> > also with this patch[with_patch-752845-fix_run.txt attached in GSD] from
> > a 1.5h run (3,373 torn-down contexts)
> > 95% of ASIDs re-faulted exactly once before being descheduled, 0 CAT
> > errors, 0 hangs.
> > So the existing async teardown already deschedules the context within a
> > fault cycle or two.
> > 
> > can I move it to teardown wait for the ASIDs queues to deregister and
> > drain its pending faults
> > in xe_vm_close_and_put() before erasing the ASID, but that a larger
> > change. Happy to change either way.
> > -Sanjay
> 
> 
> Gentle ping
> 

We have actually discussed this quite a bit, and you are not the first
to complain about it. I agreed on an acceptable approach in this
thread [1], particularly in this comment [2].

I haven't had enough time to implement and fully test this solution, but
if you or anyone else wants to take over, my latest version [3] can be
used as a starting point. The idea would be to implement [2] on top of
it to serialize queue teardown before tearing down the page tables
(xe_vm_close).

Matt

[1] https://patchwork.freedesktop.org/patch/732703/?series=168398&rev=4
[2] https://patchwork.freedesktop.org/patch/732703/?series=168398&rev=4#comment_1349875
[3] https://patchwork.freedesktop.org/patch/748380/?series=168398&rev=6

> > > > +        }
> > > >           return PTR_ERR(vm);
> > > > +    }
> > > >         down_read(&vm->lock);
> > > >         if (xe_vm_is_closed(vm)) {
> > > > -        err = -ENOENT;
> > > > +        drm_info(&xe->drm,
> > > > +             "xe_pf_debug: drain stale fault (closed VM)
> > > > asid=%u addr=0x%llx\n",
> > > > +             asid, pf->consumer.page_addr);
> > > > +        xe_pagefault_set_start_addr(pf, pf->consumer.page_addr);
> > > > +        xe_pagefault_set_end_addr(pf, pf->consumer.page_addr);
> > > > +        err = 0;
> > > >           goto unlock_vm;
> > > >       }
> > > >   --
> > > > 2.52.0
> > > >