Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Yadav, Arvind" <arvind.yadav@intel.com>
To: Matthew Brost <matthew.brost@intel.com>
Cc: <intel-xe@lists.freedesktop.org>,
	<himal.prasad.ghimiray@intel.com>,
	<thomas.hellstrom@linux.intel.com>, <tejas.upadhyay@intel.com>
Subject: Re: [PATCH] drm/xe/debugfs: Add knob to wedge on first page fault
Date: Thu, 16 Jul 2026 19:04:10 +0530	[thread overview]
Message-ID: <78e97f99-ecb6-4e32-8cd5-dc522cbcac96@intel.com> (raw)
In-Reply-To: <alhxjBaQ4HBIea3y@gsse-cloud1.jf.intel.com>


On 16-07-2026 11:22, Matthew Brost wrote:
> On Thu, Jul 16, 2026 at 10:44:13AM +0530, Arvind Yadav wrote:
>> Add a debugfs knob to wedge the device on the first GPU page fault.
>>
>> This is intended for debug where preserving the faulting HW
>> state is more useful than servicing the fault. When enabled,
>> the first page fault is printed, saved to the VM fault list,
>> and the device is declared wedged without servicing or acknowledging
>> the fault.
>>
>>    echo 1 > /sys/kernel/debug/dri/<card>/wedge_on_first_pagefault
>>
>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_debugfs.c      | 42 ++++++++++++++++++++++++++++
>>   drivers/gpu/drm/xe/xe_device_types.h | 12 ++++++++
>>   drivers/gpu/drm/xe/xe_pagefault.c    | 23 +++++++++++++++
>>   3 files changed, 77 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
>> index 8c391c7b017a..d22f73241d89 100644
>> --- a/drivers/gpu/drm/xe/xe_debugfs.c
>> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
>> @@ -564,6 +564,43 @@ static const struct file_operations disable_late_binding_fops = {
>>   	.write = disable_late_binding_set,
>>   };
>>   
>> +static ssize_t wedge_on_first_pagefault_show(struct file *f, char __user *ubuf,
>> +					     size_t size, loff_t *pos)
>> +{
>> +	struct xe_device *xe = file_inode(f)->i_private;
>> +	char buf[8];
>> +	int len;
>> +
>> +	len = scnprintf(buf, sizeof(buf), "%d\n",
>> +			READ_ONCE(xe->wedge_on_first_pagefault) ? 1 : 0);
>> +
>> +	return simple_read_from_buffer(ubuf, size, pos, buf, len);
>> +}
>> +
>> +static ssize_t wedge_on_first_pagefault_set(struct file *f,
>> +					    const char __user *ubuf,
>> +					    size_t size, loff_t *pos)
>> +{
>> +	struct xe_device *xe = file_inode(f)->i_private;
>> +	bool val;
>> +	int ret;
>> +
>> +	ret = kstrtobool_from_user(ubuf, size, &val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	WRITE_ONCE(xe->wedge_on_first_pagefault, val);
>> +	atomic_set(&xe->pf_wedge_triggered, 0);
>> +
>> +	return size;
>> +}
>> +
>> +static const struct file_operations wedge_on_first_pagefault_fops = {
>> +	.owner = THIS_MODULE,
>> +	.read = wedge_on_first_pagefault_show,
>> +	.write = wedge_on_first_pagefault_set,
>> +};
>> +
>>   void xe_debugfs_register(struct xe_device *xe)
>>   {
>>   	struct ttm_device *bdev = &xe->ttm;
>> @@ -617,6 +654,11 @@ void xe_debugfs_register(struct xe_device *xe)
>>   	debugfs_create_file("disable_late_binding", 0600, root, xe,
>>   			    &disable_late_binding_fops);
>>   
>> +	/* Debug: wedge on first page fault to freeze HW state for inspection. */
>> +	if (xe->info.has_usm)
>> +		debugfs_create_file("wedge_on_first_pagefault", 0600, root, xe,
>> +				     &wedge_on_first_pagefault_fops);
>> +
>>   	/*
>>   	 * Don't expose page reclaim configuration file if not supported by the
>>   	 * hardware initially.
>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>> index 022e08205897..c208b99fc577 100644
>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>> @@ -530,6 +530,18 @@ struct xe_device {
>>   	/** @min_run_period_pf_ms: LR VM (page fault mode) timeslice */
>>   	u32 min_run_period_pf_ms;
>>   
>> +	/**
>> +	 * @wedge_on_first_pagefault: Debug knob to wedge device on first GPU
>> +	 * page fault instead of recovery, freezing HW state for inspection.
>> +	 */
>> +	bool wedge_on_first_pagefault;
>> +
>> +	/**
>> +	 * @pf_wedge_triggered: One-shot latch preventing repeated wedge
>> +	 * handling.
>> +	 */
>> +	atomic_t pf_wedge_triggered;
> I'd scope these variables in sub-struct somewhere.
>
> We already have this in xe_device:
>
> 546         /** @wedged: Struct to control Wedged States and mode */
> 547         struct {
> 548                 /** @wedged.flag: Xe device faced a critical error and is now blocked. */
> 549                 atomic_t flag;
> 550                 /** @wedged.mode: Mode controlled by kernel parameter and debugfs */
> 551                 enum xe_wedged_mode mode;
> 552                 /** @wedged.method: Recovery method to be sent in the drm device wedged uevent */
> 553                 unsigned long method;
> 554                 /** @wedged.inconsistent_reset: Inconsistent reset policy state between GTs */
> 555                 bool inconsistent_reset;
> 556         } wedged;
>
> Seems like a good place.


Noted, I will move the debug fields under xe->wedged.


>
>> +
>>   #ifdef TEST_VM_OPS_ERROR
>>   	/**
>>   	 * @vm_inject_error_position: inject errors at different places in VM
>> diff --git a/drivers/gpu/drm/xe/xe_pagefault.c b/drivers/gpu/drm/xe/xe_pagefault.c
>> index dd3c068e1a39..82496c5511b3 100644
>> --- a/drivers/gpu/drm/xe/xe_pagefault.c
>> +++ b/drivers/gpu/drm/xe/xe_pagefault.c
>> @@ -299,11 +299,34 @@ static void xe_pagefault_queue_work(struct work_struct *w)
>>   	threshold = jiffies + msecs_to_jiffies(USM_QUEUE_MAX_RUNTIME_MS);
>>   
>>   	while (xe_pagefault_queue_pop(pf_queue, &pf)) {
>> +		struct xe_device *xe;
>>   		int err;
>>   
>>   		if (!pf.gt)	/* Fault squashed during reset */
>>   			continue;
>>   
>> +		xe = gt_to_xe(pf.gt);
>> +
>> +		/*
>> +		 * Debug mode: if armed via debugfs, wedge on first fault instead of
>> +		 * servicing. Dumps fault info and freezes HW for inspection.
>> +		 */
>> +		if (unlikely(READ_ONCE(xe->wedge_on_first_pagefault))) {
>> +			if (!atomic_xchg(&xe->pf_wedge_triggered, 1)) {
> Do we want want a devcoredump too?


No, I did not add devcoredump  since that wasn't part of the ask.

>
>> +				xe_pagefault_print(&pf);
>> +				xe_gt_err(pf.gt,
>> +					"wedge_on_first_pagefault: freezing HW after first fault\n");
>> +				xe_pagefault_save_to_vm(xe, &pf);
> xe_pagefault_save_to_vm isn't particularly useful, as it attaches a
> fault to the VM that can be retrieved via an ioctl, but wedging disables
> all ioctls, iirc.


Agreed, I will drop. Since wedging blocks ioctls, saving the fault to VM 
is not useful here. The dmesg PF print should be enough for this debug path.

>
>> +				xe_device_declare_wedged(xe);
> xe_device_declare_wedged has some issues with directly stopping and
> starting queues in the GuC backend. Those should probably be fixed as
> part of this series, as this change will likely make that bug more
> noticeable.
>
> See xe_guc_submit_wedge():
>
> 1353         } else {
> 1354                 /* Forcefully kill any remaining exec queues, signal fences */
> 1355                 guc_submit_reset_prepare(guc);
> 1356                 xe_guc_submit_stop(guc);
> 1357                 xe_guc_softreset(guc);
> 1358                 xe_uc_fw_sanitize(&guc->fw);
> 1359                 xe_guc_submit_pause_abort(guc);
> 1360         }
>
>
> The above code is only safe to run on the GT ordered workqueue. I have a
> Jira somewhere to clean this up and add some lockdep checks so we can
> catch cases where someone incorrectly stops or starts queues.
>
> As a quick fix, we could simply move this code into a work item and
> schedule it asynchronously on the GT ordered workqueue.


Agreed. I will move the actual wedge call out of the PF worker and queue 
it on the GT ordered workqueue. That keeps this debug path aligned with 
the GuC queue stop/start requirement.

Thanks,
Arvind

>
> Matt
>
>> +			}
>> +
>> +			/*
>> +			 * Do not service or ack. Stop processing this queue so the
>> +			 * captured state stays close to the first fault.
>> +			 */
>> +			return;
>> +		}
>> +
>>   		err = xe_pagefault_service(&pf);
>>   		if (err) {
>>   			xe_pagefault_save_to_vm(gt_to_xe(pf.gt), &pf);
>> -- 
>> 2.43.0
>>

  reply	other threads:[~2026-07-16 13:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-16  5:14 [PATCH] drm/xe/debugfs: Add knob to wedge on first page fault Arvind Yadav
2026-07-16  5:48 ` ✗ CI.checkpatch: warning for " Patchwork
2026-07-16  5:50 ` ✓ CI.KUnit: success " Patchwork
2026-07-16  5:52 ` [PATCH] " Matthew Brost
2026-07-16 13:34   ` Yadav, Arvind [this message]
2026-07-16  6:55 ` ✓ Xe.CI.BAT: success for " Patchwork
2026-07-16  8:14 ` ✓ Xe.CI.FULL: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=78e97f99-ecb6-4e32-8cd5-dc522cbcac96@intel.com \
    --to=arvind.yadav@intel.com \
    --cc=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.brost@intel.com \
    --cc=tejas.upadhyay@intel.com \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox