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

["Yadav, Sanjay Kumar" <sanjay.kumar.yadav@intel.com>]

On 18-09-2026 23:25, Matthew Brost wrote:
> 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
Hi Matt,

Thanks for the reply and review comments.
Arvind’s series[1] appears to implement similar handling. I will use 
that series as the fix instead.

[1] https://patchwork.freedesktop.org/series/174252/
FYI: Arvind
>
>>>>> +        }
>>>>>            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
>>>>>