Re: [PATCH v4 04/13] drm/xe: Skip device access during PCI error recovery

["Tauro, Riana" <riana.tauro@intel.com>]

On 4/30/2026 6:28 PM, Anshuman Gupta wrote:
> On 2026-04-17 at 14:28:16 +0530, Riana Tauro wrote:
>> When a fatal error occurs and the error_detected callback is
>> invoked the device is inaccessible. The error_detected callback
>> wedges the device causing the jobs to timeout.
>>
>> The timedout handler acquires forcewake to dump devcoredump and
>> triggers a GT reset. Since the device is inacessible this causes
>> errors. Skip all mmio accesses and gt reset when the device
>> is in recovery.
>>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> ---
>> v2: add check in worker (Mallesh)
>> ---
>>   drivers/gpu/drm/xe/xe_gt.c         | 14 +++++++++++---
>>   drivers/gpu/drm/xe/xe_guc_submit.c |  9 +++++----
>>   2 files changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
>> index 8a31c963c372..5ea5524d83af 100644
>> --- a/drivers/gpu/drm/xe/xe_gt.c
>> +++ b/drivers/gpu/drm/xe/xe_gt.c
>> @@ -917,6 +917,9 @@ static void gt_reset_worker(struct work_struct *w)
>>   	if (xe_device_wedged(gt_to_xe(gt)))
>>   		goto err_pm_put;
>>   
>> +	if (xe_device_is_in_recovery(gt_to_xe(gt)))
>> +		goto err_pm_put;
>> +
>>   	/* We only support GT resets with GuC submission */
>>   	if (!xe_device_uc_enabled(gt_to_xe(gt)))
>>   		goto err_pm_put;
>> @@ -977,18 +980,23 @@ static void gt_reset_worker(struct work_struct *w)
>>   
>>   void xe_gt_reset_async(struct xe_gt *gt)
>>   {
>> -	xe_gt_info(gt, "trying reset from %ps\n", __builtin_return_address(0));
>> +	struct xe_device *xe = gt_to_xe(gt);
>> +
>> +	if (xe_device_is_in_recovery(xe))
>> +		return;
> How is this synchronize with xe_device_set_in_recovery() a mid flight
> reset can still hit the device which already passed the check
> xe_device_is_in_recovery() check.


There might be a race when gt reset is already in progress and there is 
an AER.
Since this is an error path, some -ECANCELLED errors should be 
acceptable. Currently we
see a similar behaviour if device gets wedged and gt reset is in progress.

A better approach would be to cancel all workers which will be done in
[PATCH v6 0/8] Introduce Xe PCIe FLR - Raag Jadav 
<https://lore.kernel.org/intel-xe/20260423100017.1051587-1-raag.jadav@intel.com/> . 
This series will be integrated with
existing PCI error handlers once it is merged. Will add a TODO in the 
cover letter
and the first patch.

A similar TODO exists in the FLR series "TODO: Add PCIe error handling 
callbacks using similar flow."

Thanks
Riana

>
> Thanks,
> Anshuman
>>   
>>   	/* Don't do a reset while one is already in flight */
>>   	if (!xe_fault_inject_gt_reset() && xe_uc_reset_prepare(&gt->uc))
>>   		return;
>>   
>> +	xe_gt_info(gt, "trying reset from %ps\n", __builtin_return_address(0));
>> +
>>   	xe_gt_info(gt, "reset queued\n");
>>   
>>   	/* Pair with put in gt_reset_worker() if work is enqueued */
>> -	xe_pm_runtime_get_noresume(gt_to_xe(gt));
>> +	xe_pm_runtime_get_noresume(xe);
>>   	if (!queue_work(gt->ordered_wq, &gt->reset.worker))
>> -		xe_pm_runtime_put(gt_to_xe(gt));
>> +		xe_pm_runtime_put(xe);
>>   }
>>   
>>   void xe_gt_suspend_prepare(struct xe_gt *gt)
>> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
>> index 10556156eaad..1f32fb14a5c1 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
>> @@ -1522,7 +1522,7 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job)
>>   	 * If devcoredump not captured and GuC capture for the job is not ready
>>   	 * do manual capture first and decide later if we need to use it
>>   	 */
>> -	if (!exec_queue_killed(q) && !xe->devcoredump.captured &&
>> +	if (!xe_device_is_in_recovery(xe) && !exec_queue_killed(q) && !xe->devcoredump.captured &&
>>   	    !xe_guc_capture_get_matching_and_lock(q)) {
>>   		/* take force wake before engine register manual capture */
>>   		CLASS(xe_force_wake, fw_ref)(gt_to_fw(q->gt), XE_FORCEWAKE_ALL);
>> @@ -1544,8 +1544,8 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job)
>>   	set_exec_queue_banned(q);
>>   
>>   	/* Kick job / queue off hardware */
>> -	if (!wedged && (exec_queue_enabled(primary) ||
>> -			exec_queue_pending_disable(primary))) {
>> +	if (!xe_device_is_in_recovery(xe) && !wedged &&
>> +	    (exec_queue_enabled(primary) || exec_queue_pending_disable(primary))) {
>>   		int ret;
>>   
>>   		if (exec_queue_reset(primary))
>> @@ -1613,7 +1613,8 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job)
>>   
>>   	trace_xe_sched_job_timedout(job);
>>   
>> -	if (!exec_queue_killed(q))
>> +	/* Do not access device if in recovery */
>> +	if (!xe_device_is_in_recovery(xe) && !exec_queue_killed(q))
>>   		xe_devcoredump(q, job,
>>   			       "Timedout job - seqno=%u, lrc_seqno=%u, guc_id=%d, flags=0x%lx",
>>   			       xe_sched_job_seqno(job), xe_sched_job_lrc_seqno(job),
>> -- 
>> 2.47.1
>>