All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.