Re: [PATCH v5 2/4] drm/xe/guc: Report reset failure using SIGID

["Yadav, Arvind" <arvind.yadav@intel.com>]

On 31-08-2026 15:01, Michal Wajdeczko wrote:
>
> On 8/31/2026 10:05 AM, Arvind Yadav wrote:
>> Route the GuC submit engine reset failure through the structured SIGID
>> logging helper.
>>
>> Use the GUCSUBMIT component, which maps to XE_SIGID_GT_TDR, and pass -EIO
>> as the errno value.
>>
>> 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>
>> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
>> ---
>> v5:
>>   - Added GUCSUBMIT component and mapped it to XE_SIGID_GT_TDR.
>>     (Michal)
>>   - Used GUCSUBMIT instead of GT for the GuC submit reset failure.
>>     (Michal)
>>   - Dropped "request" from the reset failure message.
>>     (Michal)
>>
>> v4:
>>   - Clarified that the GT component maps to XE_SIGID_GT_TDR and -EIO is
>>     only the errno value. (Sashiko)
>>
>> v3:
>>   - Split the GuC reset-request failure conversion into a separate patch.
>>     (Michal)
>>   - Dropped GuC job-timeout warning conversions to avoid changing warn-level
>>     logs into error-level SIGID reports. (Michal)
>>   - Kept only the existing error path that maps naturally to GT_TDR. (Michal)
>>
>> 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.
>>
>>   drivers/gpu/drm/xe/xe_guc_submit.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
>> index 99d8c807ff05..543cf7baae96 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"
>> @@ -3494,8 +3495,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, GUCSUBMIT, -EIO,
> shouldn't we try to use different errno based on the 'reason' code?
> AFAICT there are just few reasons that GuC will use here


I used -EIO because this is a generic GuC engine reset failure, and the 
GuC reason is only reported as a raw firmware value.
I would prefer not to add errno mapping as part of this SIGID conversion 
patch. We can have separate patch for this.

>> +		   "engine reset failed on %d:%d because 0x%08X\n",
>> +		   guc_class, instance, reason);
> nit: as both guc_class and instance are u8, we should print them as:
>
> 		%u:%u
>
> nit: the reason could be printed with just:
>
> 		%#x


Noted, I will add this "engine reset failed on %u:%u, reason=%#x".


Thanks,
Arvind

>
>>   
>>   	xe_gt_reset_async(gt);
>>