Re: [PATCH 22/22] drm/xe/eudebug: Enable EU pagefault handling

[Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>]

On 2/23/26 10:41 AM, Matthew Brost wrote:
> On Mon, Feb 23, 2026 at 04:03:17PM +0200, Mika Kuoppala wrote:
>> From: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>>
>> The XE2 (and PVC) HW has a limitation that the pagefault due to invalid
>> access will halt the corresponding EUs. To solve this problem, enable
>> EU pagefault handling functionality, which allows to unhalt pagefaulted
>> eu threads and to EU debugger to get inform about the eu attentions state
>> of EU threads during execution.
>>
>> If a pagefault occurs, send the DRM_XE_EUDEBUG_EVENT_PAGEFAULT event
>> after handling the pagefault.
>>
>> The pagefault handling is a mechanism that allows a stalled EU thread to
>> enter SIP mode by installing a temporal null page to the page table entry
>> where the pagefault happened.
>>
>> A brief description of the page fault handling mechanism flow between KMD
>> and the eu thread is as follows
>>
>> (1) eu thread accesses unallocated address
>> (2) pagefault happens and eu thread stalls
>> (3) XE kmd set an force eu thread exception to allow the running eu thread
>>      to enter SIP mode (kmd set ForceException / ForceExternalHalt bit of
>>      TD_CTL register)
>>      Not stalled (none-pagefaulted) eu threads enter SIP mode
>> (4) XE kmd installs temporal null page to the pagetable entry of the
>>      address where pagefault happened.
>> (5) XE kmd replies pagefault successful message to GUC
>> (6) stalled eu thread resumes as per pagefault condition has resolved
>> (7) resumed eu thread enters SIP mode due to force exception set by (3)
>> (8) adapted to consumer/produced pagefaults
>>
>> As designed this feature to only work when eudbug is enabled, it should
>> have no impact to regular recoverable pagefault code path.
>>
>> v2: - pf->q holds the vm ref so drop it (Mika)
>>      - streamline uapi (Mika)
>>      - cleanup the pagefault through producer if (Mika)
>>
>> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_guc_pagefault.c   |  8 +++++++
>>   drivers/gpu/drm/xe/xe_pagefault.c       | 31 ++++++++++++++++++++++++-
>>   drivers/gpu/drm/xe/xe_pagefault_types.h |  9 +++++++
>>   3 files changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_guc_pagefault.c b/drivers/gpu/drm/xe/xe_guc_pagefault.c
>> index d48f6ed103bb..6adf3bf73b1c 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_pagefault.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_pagefault.c
>> @@ -8,6 +8,7 @@
>>   #include "xe_guc_ct.h"
>>   #include "xe_guc_pagefault.h"
>>   #include "xe_pagefault.h"
>> +#include "xe_eudebug_pagefault.h"
>>   
>>   static void guc_ack_fault(struct xe_pagefault *pf, int err)
>>   {
>> @@ -37,8 +38,15 @@ static void guc_ack_fault(struct xe_pagefault *pf, int err)
>>   	xe_guc_ct_send(&guc->ct, action, ARRAY_SIZE(action), 0, 0);
>>   }
>>   
>> +static void guc_cleanup_fault(struct xe_pagefault *pf, int err)
>> +{
>> +	xe_eudebug_pagefault_service(pf);
>> +	xe_eudebug_pagefault_destroy(pf, 0);
>> +}
>> +
>>   static const struct xe_pagefault_ops guc_pagefault_ops = {
>>   	.ack_fault = guc_ack_fault,
>> +	.cleanup_fault = guc_cleanup_fault,
>>   };
>>   
>>   /**
>> diff --git a/drivers/gpu/drm/xe/xe_pagefault.c b/drivers/gpu/drm/xe/xe_pagefault.c
>> index 72f589fd2b64..9dcd854e99f9 100644
>> --- a/drivers/gpu/drm/xe/xe_pagefault.c
>> +++ b/drivers/gpu/drm/xe/xe_pagefault.c
>> @@ -10,6 +10,7 @@
>>   
>>   #include "xe_bo.h"
>>   #include "xe_device.h"
>> +#include "xe_eudebug_pagefault.h"
>>   #include "xe_gt_printk.h"
>>   #include "xe_gt_types.h"
>>   #include "xe_gt_stats.h"
>> @@ -171,6 +172,8 @@ static int xe_pagefault_service(struct xe_pagefault *pf)
>>   	if (IS_ERR(vm))
>>   		return PTR_ERR(vm);
>>   
>> +	xe_eudebug_pagefault_create(vm, pf);
>> +
>>   	/*
>>   	 * TODO: Change to read lock? Using write lock for simplicity.
>>   	 */
>> @@ -184,9 +187,28 @@ static int xe_pagefault_service(struct xe_pagefault *pf)
>>   	vma = xe_vm_find_vma_by_addr(vm, pf->consumer.page_addr);
> 
> I've mentioned this before - this fundamentally broken if SVM is
> enabled as the VMA lookup will never fail given VMA tree is completely
> populated in the SVM cases (i.e., when SVM is enabled the first thing
> the UMD does is bind the entire CPU address space with a CPU mirror
> VMA). What will fail in the SVM case is xe_svm_handle_pagefault will
> likely return -ENOENT. UMDs from my understanding will enable SVM by
> default so this likely needs to be rethought.
> 
Thank you for your reply. Yes, additional implementation of eudebug 
pagefault is required for cases where SVM is used.
As per your comment, in the SVM + eudebug pagefault scenario, if 
xe_svm_handle_pagefault() returns -ENOENT (i.e., when memory allocation 
via mmap etc. is not performed in userspace),
eudebug requires a temporary page install at the address where the page 
fault occurred to allow stalled EU threads to enter SIP.

Two methods come to mind for temporary page installation in the page table:
1) Temporary memory allocation by emulating the implementation of 
do_mmap() / __mmap_region() in an eudebug pagefault situation,
    similar to the general memory allocation scenario in an SVM scenario

    - Pros: Follows the do_mmap() flow for memory allocation; only 
requires one additional call to xe_svm_handle_pagefault() after 
temporary page installation (simple code implementation).
    - Cons: (1) If the installed temporary VMA is not removed, a page 
fault occurring at the same address on the CPU triggers migration to 
system memory,
                preventing the CPU debugger from causing a segmentation 
fault.
            (2) The GPU debugger may fail to handle page faults for 
low-address regions inaccessible to userspace (mmap_min_addr issue)

2) In the SVM scenario, perform a temporary page install only on the GPU 
page table where the page fault occurred during the EUDEBUG page fault 
situation
    : Updating the page table directly without using the 
xe_svm_handle_pagefault() function flow


Could I get your input on these two approaches? Or do you have 
additional thoughts?

G.G.

> Matt
> 
>>   	if (!vma) {
>>   		err = -EINVAL;
>> -		goto unlock_vm;
>> +		vma = xe_eudebug_create_vma(vm, pf);
>> +		if (IS_ERR(vma)) {
>> +			err = PTR_ERR(vma);
>> +			vma = NULL;
>> +		}
>>   	}
>>   
>> +	if (vma) {
>> +		/*
>> +		 * When creating an instance of eudebug_pagefault, there was
>> +		 * no vma containing the ppgtt address where the pagefault occurred,
>> +		 * but when reacquiring vm->lock, there is.
>> +		 * During not aquiring the vm->lock from this context,
>> +		 * but vma corresponding to the address where the pagefault occurred
>> +		 * in another context has allocated.
>> +		 */
>> +		err = 0;
>> +	}
>> +
>> +	if (err)
>> +		goto unlock_vm;
>> +
>>   	atomic = xe_pagefault_access_is_atomic(pf->consumer.access_type);
>>   
>>   	if (xe_vma_is_cpu_addr_mirror(vma))
>> @@ -198,6 +220,10 @@ static int xe_pagefault_service(struct xe_pagefault *pf)
>>   unlock_vm:
>>   	if (!err)
>>   		vm->usm.last_fault_vma = vma;
>> +
>> +	if (err)
>> +		xe_eudebug_pagefault_destroy(pf, err);
>> +
>>   	up_write(&vm->lock);
>>   	xe_vm_put(vm);
>>   
>> @@ -268,6 +294,9 @@ static void xe_pagefault_queue_work(struct work_struct *w)
>>   
>>   		pf.producer.ops->ack_fault(&pf, err);
>>   
>> +		if (pf.producer.ops->cleanup_fault)
>> +			pf.producer.ops->cleanup_fault(&pf, err);
>> +
>>   		if (time_after(jiffies, threshold)) {
>>   			queue_work(gt_to_xe(pf.gt)->usm.pf_wq, w);
>>   			break;
>> diff --git a/drivers/gpu/drm/xe/xe_pagefault_types.h b/drivers/gpu/drm/xe/xe_pagefault_types.h
>> index 2bee858da597..9d2d29d35a4b 100644
>> --- a/drivers/gpu/drm/xe/xe_pagefault_types.h
>> +++ b/drivers/gpu/drm/xe/xe_pagefault_types.h
>> @@ -43,6 +43,15 @@ struct xe_pagefault_ops {
>>   	 * sends the result to the HW/FW interface.
>>   	 */
>>   	void (*ack_fault)(struct xe_pagefault *pf, int err);
>> +
>> +	/**
>> +	 * @cleanup_fault: Cleanup for producer, if any
>> +	 * @pf: Page fault
>> +	 * @err: Error state of fault
>> +	 *
>> +	 * Page fault producer received cleanup request from consumer
>> +	 */
>> +	void (*cleanup_fault)(struct xe_pagefault *pf, int err);
>>   };
>>   
>>   /**
>> -- 
>> 2.43.0
>>