[PATCH v2 0/1] drm/xe: Report GT TDR and VM rebind faults using SIGID

[PATCH v2 0/1] drm/xe: Report GT TDR and VM rebind faults using SIGID

[Arvind Yadav]

This patch is a small follow-up on top of the structured SIGID error
logging infrastructure series:

https://patchwork.freedesktop.org/series/171022/

That series adds SIGID-based error reporting and the xe_log_*() helpers.
This patch converts a few existing driver-detected GT TDR and VM rebind
fault logs to the structured SIGID logging path.

v2:
 - Rebased on the latest structured SIGID logging series.
 - Switched from the old xe_ras_log_() helpers to the new xe_log_()
   helpers.
 - Dropped paths already covered by the base SIGID series.

Arvind Yadav (1):
  drm/xe: Report GT TDR and VM rebind faults using SIGID

 drivers/gpu/drm/xe/xe_guc_submit.c | 12 ++++++++----
 drivers/gpu/drm/xe/xe_vm.c         |  5 ++++-
 2 files changed, 12 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH v2 1/1] drm/xe: Report GT TDR and VM rebind faults using SIGID

[Arvind Yadav]

Route a few existing GT TDR and VM rebind failure logs through the
structured SIGID logging helpers.

Use the GT component for GuC job-timeout checks and engine reset-request
failure, which reports them with XE_SIGID_GT_TDR. Use XE_SIGID_MEM_FAULT
for the terminal preempt rebind worker failure, since the VM is killed
immediately afterwards.

v2:
 - Rebased on the latest structured SIGID logging series.
 - Switched from the old xe_ras_log_() helpers to the new xe_log_()
   helpers.
 - Dropped paths already covered by the base SIGID series.

Cc: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
Cc: Badal Nilawar <badal.nilawar@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
---
 drivers/gpu/drm/xe/xe_guc_submit.c | 12 ++++++++----
 drivers/gpu/drm/xe/xe_vm.c         |  5 ++++-
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
index 8aaed4fd13ea..79bd0e46640f 100644
--- a/drivers/gpu/drm/xe/xe_guc_submit.c
+++ b/drivers/gpu/drm/xe/xe_guc_submit.c
@@ -34,6 +34,7 @@
 #include "xe_guc_klv_helpers.h"
 #include "xe_guc_submit_types.h"
 #include "xe_hw_engine.h"
+#include "xe_log.h"
 #include "xe_lrc.h"
 #include "xe_macros.h"
 #include "xe_map.h"
@@ -1375,7 +1376,8 @@ static bool check_timeout(struct xe_exec_queue *q, struct xe_sched_job *job)
 	u64 running_time_ms;
 
 	if (!xe_sched_job_started(job)) {
-		xe_gt_warn(gt, "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, not started",
+		xe_log_err(gt, GT, -ETIMEDOUT,
+			   "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, not started\n",
 			   xe_sched_job_seqno(job), xe_sched_job_lrc_seqno(job),
 			   q->guc->id);
 
@@ -1390,7 +1392,8 @@ static bool check_timeout(struct xe_exec_queue *q, struct xe_sched_job *job)
 				     xe_sched_job_seqno(job),
 				     xe_sched_job_lrc_seqno(job), q->guc->id);
 		else
-			xe_gt_warn(gt, "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, timestamp stuck",
+			xe_log_err(gt, GT, -ETIMEDOUT,
+				   "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, timestamp stuck\n",
 				   xe_sched_job_seqno(job),
 				   xe_sched_job_lrc_seqno(job), q->guc->id);
 
@@ -3354,8 +3357,9 @@ int xe_guc_exec_queue_reset_failure_handler(struct xe_guc *guc, u32 *msg, u32 le
 	reason = msg[2];
 
 	/* Unexpected failure of a hardware feature, log an actual error */
-	xe_gt_err(gt, "GuC engine reset request failed on %d:%d because 0x%08X",
-		  guc_class, instance, reason);
+	xe_log_err(gt, GT, -EIO,
+		   "GuC engine reset request failed on %d:%d because 0x%08X\n",
+		   guc_class, instance, reason);
 
 	xe_gt_reset_async(gt);
 
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 9e0176861cb6..7c70ea23045a 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -28,6 +28,7 @@
 #include "xe_drm_client.h"
 #include "xe_exec_queue.h"
 #include "xe_gt.h"
+#include "xe_log.h"
 #include "xe_migrate.h"
 #include "xe_pat.h"
 #include "xe_pm.h"
@@ -591,7 +592,9 @@ static void preempt_rebind_work_func(struct work_struct *w)
 	}
 
 	if (err) {
-		drm_warn(&vm->xe->drm, "VM worker error: %d\n", err);
+		xe_log_from_recoverable(vm->xe, XE_SIGID_MEM_FAULT,
+					XE_LOG_COMPONENT_NONE, ERR_PTR(err), 0,
+					"Preempt rebind worker failed\n");
 		xe_vm_kill(vm, true);
 	}
 	up_write(&vm->lock);
-- 
2.43.0

✗ CI.KUnit: failure for drm/xe: Report GT TDR and VM rebind faults using SIGID (rev2)

[Patchwork]

== Series Details ==

Series: drm/xe: Report GT TDR and VM rebind faults using SIGID (rev2)
URL   : https://patchwork.freedesktop.org/series/171571/
State : failure

== Summary ==

+ trap cleanup EXIT
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig
ERROR:root:../drivers/gpu/drm/xe/xe_guc_submit.c:37:10: fatal error: xe_log.h: No such file or directory
   37 | #include "xe_log.h"
      |          ^~~~~~~~~~
compilation terminated.
make[7]: *** [../scripts/Makefile.build:289: drivers/gpu/drm/xe/xe_guc_submit.o] Error 1
make[7]: *** Waiting for unfinished jobs....
make[6]: *** [../scripts/Makefile.build:549: drivers/gpu/drm/xe] Error 2
make[5]: *** [../scripts/Makefile.build:549: drivers/gpu/drm] Error 2
make[4]: *** [../scripts/Makefile.build:549: drivers/gpu] Error 2
make[3]: *** [../scripts/Makefile.build:549: drivers] Error 2
make[2]: *** [/kernel/Makefile:2187: .] Error 2
make[1]: *** [/kernel/Makefile:248: __sub-make] Error 2
make: *** [Makefile:248: __sub-make] Error 2

[13:47:55] Configuring KUnit Kernel ...
Generating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[13:48:00] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=48
+ cleanup
++ stat -c %u:%g /kernel
+ chown -R 1003:1003 /kernel

Re: [PATCH v2 1/1] drm/xe: Report GT TDR and VM rebind faults using SIGID

[Michal Wajdeczko]

On 8/4/2026 11:36 AM, Arvind Yadav wrote:
> Route a few existing GT TDR and VM rebind failure logs through the
> structured SIGID logging helpers.

above is a good candidate for the cover-letter
then IMO there should be separate patches per each new SIGID usage (see
examples in SIGID introduction series)
> 
> Use the GT component for GuC job-timeout checks and engine reset-request
> failure, which reports them with XE_SIGID_GT_TDR. Use XE_SIGID_MEM_FAULT
> for the terminal preempt rebind worker failure, since the VM is killed
> immediately afterwards.
> 
> v2:
>  - Rebased on the latest structured SIGID logging series.
>  - Switched from the old xe_ras_log_() helpers to the new xe_log_()
>    helpers.
>  - Dropped paths already covered by the base SIGID series.

you can keep change log under ---

> 
> Cc: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
> Cc: Badal Nilawar <badal.nilawar@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_submit.c | 12 ++++++++----
>  drivers/gpu/drm/xe/xe_vm.c         |  5 ++++-
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 8aaed4fd13ea..79bd0e46640f 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -34,6 +34,7 @@
>  #include "xe_guc_klv_helpers.h"
>  #include "xe_guc_submit_types.h"
>  #include "xe_hw_engine.h"
> +#include "xe_log.h"
>  #include "xe_lrc.h"
>  #include "xe_macros.h"
>  #include "xe_map.h"
> @@ -1375,7 +1376,8 @@ static bool check_timeout(struct xe_exec_queue *q, struct xe_sched_job *job)
>  	u64 running_time_ms;
>  
>  	if (!xe_sched_job_started(job)) {
> -		xe_gt_warn(gt, "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, not started",
> +		xe_log_err(gt, GT, -ETIMEDOUT,
> +			   "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, not started\n",
>  			   xe_sched_job_seqno(job), xe_sched_job_lrc_seqno(job),
>  			   q->guc->id);
>  
> @@ -1390,7 +1392,8 @@ static bool check_timeout(struct xe_exec_queue *q, struct xe_sched_job *job)
>  				     xe_sched_job_seqno(job),
>  				     xe_sched_job_lrc_seqno(job), q->guc->id);
>  		else
> -			xe_gt_warn(gt, "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, timestamp stuck",
> +			xe_log_err(gt, GT, -ETIMEDOUT,
> +				   "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, timestamp stuck\n",
>  				   xe_sched_job_seqno(job),
>  				   xe_sched_job_lrc_seqno(job), q->guc->id);

in both above cases you are promoting from warn to err level
is this expected/required ?

if we want to log them with SIGID but not with an error level,
we can use xe_log_err_info() instead

@Matthew, @Rodrigo ?

also, early documentation was suggesting that each SIGID should
be selected based on the source file; now since we have components
are are more relaxed, but still we have to follow some guidelines

@Aravind, @Rodrigo :

is it ok to use GT in the GuC file?
or maybe we should introduce GUCSUBMIT/SUBMISSION component with
associated GT_TDR instead of RUNTIME_FW ?

>  
> @@ -3354,8 +3357,9 @@ int xe_guc_exec_queue_reset_failure_handler(struct xe_guc *guc, u32 *msg, u32 le
>  	reason = msg[2];
>  
>  	/* Unexpected failure of a hardware feature, log an actual error */
> -	xe_gt_err(gt, "GuC engine reset request failed on %d:%d because 0x%08X",
> -		  guc_class, instance, reason);
> +	xe_log_err(gt, GT, -EIO,
> +		   "GuC engine reset request failed on %d:%d because 0x%08X\n",
> +		   guc_class, instance, reason);

separate patch ?
>  
>  	xe_gt_reset_async(gt);
>  
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 9e0176861cb6..7c70ea23045a 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -28,6 +28,7 @@
>  #include "xe_drm_client.h"
>  #include "xe_exec_queue.h"
>  #include "xe_gt.h"
> +#include "xe_log.h"
>  #include "xe_migrate.h"
>  #include "xe_pat.h"
>  #include "xe_pm.h"
> @@ -591,7 +592,9 @@ static void preempt_rebind_work_func(struct work_struct *w)
>  	}
>  
>  	if (err) {
> -		drm_warn(&vm->xe->drm, "VM worker error: %d\n", err);
> +		xe_log_from_recoverable(vm->xe, XE_SIGID_MEM_FAULT,
> +					XE_LOG_COMPONENT_NONE, ERR_PTR(err), 0,
> +					"Preempt rebind worker failed\n");

separate patch ?

maybe we should just introduce new Xe component named VM *)
and assign it the SIGID MEM_FAULT ?

*) or maybe better VMA to avoid clash with Virtual Machine?

btw, there is also other existing warn that we may want
promote to SIGID:

	"Could not allocate memory for fault!\n"

>  		xe_vm_kill(vm, true);
>  	}
>  	up_write(&vm->lock);

Re: [PATCH v2 1/1] drm/xe: Report GT TDR and VM rebind faults using SIGID

[Yadav, Arvind]

On 17-08-2026 15:50, Michal Wajdeczko wrote:
>
> On 8/4/2026 11:36 AM, Arvind Yadav wrote:
>> Route a few existing GT TDR and VM rebind failure logs through the
>> structured SIGID logging helpers.
> above is a good candidate for the cover-letter
> then IMO there should be separate patches per each new SIGID usage (see
> examples in SIGID introduction series)


Agree. I will move the high-level text to the cover letter and and split 
this into smaller patches.

>> Use the GT component for GuC job-timeout checks and engine reset-request
>> failure, which reports them with XE_SIGID_GT_TDR. Use XE_SIGID_MEM_FAULT
>> for the terminal preempt rebind worker failure, since the VM is killed
>> immediately afterwards.
>>
>> v2:
>>   - Rebased on the latest structured SIGID logging series.
>>   - Switched from the old xe_ras_log_() helpers to the new xe_log_()
>>     helpers.
>>   - Dropped paths already covered by the base SIGID series.
> you can keep change log under ---


Noted,

>
>> Cc: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
>> Cc: Badal Nilawar <badal.nilawar@intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_guc_submit.c | 12 ++++++++----
>>   drivers/gpu/drm/xe/xe_vm.c         |  5 ++++-
>>   2 files changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
>> index 8aaed4fd13ea..79bd0e46640f 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
>> @@ -34,6 +34,7 @@
>>   #include "xe_guc_klv_helpers.h"
>>   #include "xe_guc_submit_types.h"
>>   #include "xe_hw_engine.h"
>> +#include "xe_log.h"
>>   #include "xe_lrc.h"
>>   #include "xe_macros.h"
>>   #include "xe_map.h"
>> @@ -1375,7 +1376,8 @@ static bool check_timeout(struct xe_exec_queue *q, struct xe_sched_job *job)
>>   	u64 running_time_ms;
>>   
>>   	if (!xe_sched_job_started(job)) {
>> -		xe_gt_warn(gt, "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, not started",
>> +		xe_log_err(gt, GT, -ETIMEDOUT,
>> +			   "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, not started\n",
>>   			   xe_sched_job_seqno(job), xe_sched_job_lrc_seqno(job),
>>   			   q->guc->id);
>>   
>> @@ -1390,7 +1392,8 @@ static bool check_timeout(struct xe_exec_queue *q, struct xe_sched_job *job)
>>   				     xe_sched_job_seqno(job),
>>   				     xe_sched_job_lrc_seqno(job), q->guc->id);
>>   		else
>> -			xe_gt_warn(gt, "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, timestamp stuck",
>> +			xe_log_err(gt, GT, -ETIMEDOUT,
>> +				   "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, timestamp stuck\n",
>>   				   xe_sched_job_seqno(job),
>>   				   xe_sched_job_lrc_seqno(job), q->guc->id);
> in both above cases you are promoting from warn to err level
> is this expected/required ?
>
> if we want to log them with SIGID but not with an error level,
> we can use xe_log_err_info() instead
>
> @Matthew, @Rodrigo ?


Good point. I did not intend to silently change the log level. Since 
these two paths were xe_gt_warn(), I will drop these conversions from 
the next revision.

>
> also, early documentation was suggesting that each SIGID should
> be selected based on the source file; now since we have components
> are are more relaxed, but still we have to follow some guidelines
>
> @Aravind, @Rodrigo :
>
> is it ok to use GT in the GuC file?
> or maybe we should introduce GUCSUBMIT/SUBMISSION component with
> associated GT_TDR instead of RUNTIME_FW ?


My intent was to classify the fault, not just the source file. Since 
these are timeout and reset recovery paths, GT_TDR seemed more suitable.
I agree we should follow the existing guidelines, so I will avoid adding 
a new component in this patch.

>>   
>> @@ -3354,8 +3357,9 @@ int xe_guc_exec_queue_reset_failure_handler(struct xe_guc *guc, u32 *msg, u32 le
>>   	reason = msg[2];
>>   
>>   	/* Unexpected failure of a hardware feature, log an actual error */
>> -	xe_gt_err(gt, "GuC engine reset request failed on %d:%d because 0x%08X",
>> -		  guc_class, instance, reason);
>> +	xe_log_err(gt, GT, -EIO,
>> +		   "GuC engine reset request failed on %d:%d because 0x%08X\n",
>> +		   guc_class, instance, reason);
> separate patch ?


Noted,

>>   
>>   	xe_gt_reset_async(gt);
>>   
>> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>> index 9e0176861cb6..7c70ea23045a 100644
>> --- a/drivers/gpu/drm/xe/xe_vm.c
>> +++ b/drivers/gpu/drm/xe/xe_vm.c
>> @@ -28,6 +28,7 @@
>>   #include "xe_drm_client.h"
>>   #include "xe_exec_queue.h"
>>   #include "xe_gt.h"
>> +#include "xe_log.h"
>>   #include "xe_migrate.h"
>>   #include "xe_pat.h"
>>   #include "xe_pm.h"
>> @@ -591,7 +592,9 @@ static void preempt_rebind_work_func(struct work_struct *w)
>>   	}
>>   
>>   	if (err) {
>> -		drm_warn(&vm->xe->drm, "VM worker error: %d\n", err);
>> +		xe_log_from_recoverable(vm->xe, XE_SIGID_MEM_FAULT,
>> +					XE_LOG_COMPONENT_NONE, ERR_PTR(err), 0,
>> +					"Preempt rebind worker failed\n");
> separate patch ?
>
> maybe we should just introduce new Xe component named VM *)
> and assign it the SIGID MEM_FAULT ?


Agree. I will split the preempt rebind worker conversion into a 
separate patch.
A VM/VMA component mapped to MEM_FAULT would be cleaner than using 
XE_LOG_COMPONENT_NONE, but that looks like an infrastructure change.
I will do the changes accordingly.

>
> *) or maybe better VMA to avoid clash with Virtual Machine?
>
> btw, there is also other existing warn that we may want
> promote to SIGID:


Yes, that looks like a good follow-up candidate. But I will follow the 
current guideline.

>
> 	"Could not allocate memory for fault!\n"
>
>>   		xe_vm_kill(vm, true);
>>   	}
>>   	up_write(&vm->lock);