Re: [PATCH 11/16] drm/xe/multi_queue: Handle CGP context error

[Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>]

On Sun, Nov 02, 2025 at 10:29:32AM -0800, Matthew Brost wrote:
>On Fri, Oct 31, 2025 at 11:29:31AM -0700, Niranjana Vishwanathapura wrote:
>> Trigger multi-queue context cleanup upon CGP context error
>> notification from GuC.
>>
>> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>> ---
>>  drivers/gpu/drm/xe/abi/guc_actions_abi.h |  1 +
>>  drivers/gpu/drm/xe/xe_guc_ct.c           |  4 +++
>>  drivers/gpu/drm/xe/xe_guc_submit.c       | 33 ++++++++++++++++++++++++
>>  drivers/gpu/drm/xe/xe_guc_submit.h       |  2 ++
>>  drivers/gpu/drm/xe/xe_trace.h            |  5 ++++
>>  5 files changed, 45 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/abi/guc_actions_abi.h b/drivers/gpu/drm/xe/abi/guc_actions_abi.h
>> index 3e9fbed9cda6..8af3691626bf 100644
>> --- a/drivers/gpu/drm/xe/abi/guc_actions_abi.h
>> +++ b/drivers/gpu/drm/xe/abi/guc_actions_abi.h
>> @@ -142,6 +142,7 @@ enum xe_guc_action {
>>  	XE_GUC_ACTION_REGISTER_CONTEXT_MULTI_QUEUE = 0x4602,
>>  	XE_GUC_ACTION_MULTI_QUEUE_CONTEXT_CGP_SYNC = 0x4603,
>>  	XE_GUC_ACTION_NOTIFY_MULTI_QUEUE_CONTEXT_CGP_SYNC_DONE = 0x4604,
>> +	XE_GUC_ACTION_NOTIFY_MULTI_QUEUE_CGP_CONTEXT_ERROR = 0x4605,
>>  	XE_GUC_ACTION_CLIENT_SOFT_RESET = 0x5507,
>>  	XE_GUC_ACTION_SET_ENG_UTIL_BUFF = 0x550A,
>>  	XE_GUC_ACTION_SET_DEVICE_ENGINE_ACTIVITY_BUFFER = 0x550C,
>> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
>> index 48b5006eb080..d0e19af0b4d2 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
>> @@ -1574,6 +1574,10 @@ static int process_g2h_msg(struct xe_guc_ct *ct, u32 *msg, u32 len)
>>  	case XE_GUC_ACTION_NOTIFY_MULTI_QUEUE_CONTEXT_CGP_SYNC_DONE:
>>  		ret = xe_guc_exec_queue_cgp_sync_done_handler(guc, payload, adj_len);
>>  		break;
>> +	case XE_GUC_ACTION_NOTIFY_MULTI_QUEUE_CGP_CONTEXT_ERROR:
>> +		ret = xe_guc_exec_queue_cgp_context_error_handler(guc, payload,
>> +								  adj_len);
>> +		break;
>>  	default:
>>  		xe_gt_err(gt, "unexpected G2H action 0x%04x\n", action);
>>  	}
>> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
>> index 87c13feb2cef..605352145d76 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
>> @@ -48,6 +48,8 @@
>>  #include "xe_vm.h"
>>  #include "xe_bo.h"
>>
>> +#define XE_GUC_EXEC_QUEUE_CGP_CONTEXT_ERROR_LEN		6
>> +
>>  static struct xe_guc *
>>  exec_queue_to_guc(struct xe_exec_queue *q)
>>  {
>> @@ -3001,6 +3003,37 @@ int xe_guc_exec_queue_reset_failure_handler(struct xe_guc *guc, u32 *msg, u32 le
>>  	return 0;
>>  }
>>
>> +int xe_guc_exec_queue_cgp_context_error_handler(struct xe_guc *guc, u32 *msg,
>> +						u32 len)
>> +{
>> +	struct xe_gt *gt = guc_to_gt(guc);
>> +	struct xe_device *xe = guc_to_xe(guc);
>> +	struct xe_exec_queue *q;
>> +	u32 guc_id = msg[2];
>> +
>> +	if (unlikely(len != XE_GUC_EXEC_QUEUE_CGP_CONTEXT_ERROR_LEN)) {
>> +		drm_err(&xe->drm, "Invalid length %u", len);
>> +		return -EPROTO;
>> +	}
>> +
>> +	q = g2h_exec_queue_lookup(guc, guc_id);
>> +	if (unlikely(!q))
>> +		return -EPROTO;
>> +
>> +	xe_gt_dbg(gt,
>> +		  "CGP context error: region=%s err=0x%x, context=0x%x LRCA=0x%x:0x%x SgId=0x%x",
>> +		  msg[0] & 1 ? "uc" : "kmd", msg[1], msg[2], msg[4], msg[3], msg[5]);
>> +
>> +	trace_xe_exec_queue_cgp_context_error(q);
>> +
>> +	/* Treat the same as engine reset */
>> +	set_exec_queue_reset(q);
>> +	if (!exec_queue_banned(q) && !exec_queue_check_timeout(q))
>
>I don't think you need the exec_queue_check_timeout check.
>

The check here is same as in other guc error handlers like
xe_guc_exec_queue_reset_handler() and xe_guc_exec_queue_memory_cat_error_handler().
Hence the reason to keep it here also. Doesn't exec_queue_check_timeout()
mean TDR is already underway?

Niranjana

>Otherwise LGTM.
>
>Matt
>
>> +		xe_guc_exec_queue_trigger_cleanup(q);
>> +
>> +	return 0;
>> +}
>> +
>>  /**
>>   * xe_guc_exec_queue_cgp_sync_done_handler - CGP synchronization done handler
>>   * @guc: guc
>> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.h b/drivers/gpu/drm/xe/xe_guc_submit.h
>> index abfa94bce391..01b013a90b1b 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_submit.h
>> +++ b/drivers/gpu/drm/xe/xe_guc_submit.h
>> @@ -35,6 +35,8 @@ int xe_guc_exec_queue_memory_cat_error_handler(struct xe_guc *guc, u32 *msg,
>>  int xe_guc_exec_queue_reset_failure_handler(struct xe_guc *guc, u32 *msg, u32 len);
>>  int xe_guc_error_capture_handler(struct xe_guc *guc, u32 *msg, u32 len);
>>  int xe_guc_exec_queue_cgp_sync_done_handler(struct xe_guc *guc, u32 *msg, u32 len);
>> +int xe_guc_exec_queue_cgp_context_error_handler(struct xe_guc *guc, u32 *msg,
>> +						u32 len);
>>
>>  struct xe_guc_submit_exec_queue_snapshot *
>>  xe_guc_exec_queue_snapshot_capture(struct xe_exec_queue *q);
>> diff --git a/drivers/gpu/drm/xe/xe_trace.h b/drivers/gpu/drm/xe/xe_trace.h
>> index 79a97b086cb2..c9d0748dae9d 100644
>> --- a/drivers/gpu/drm/xe/xe_trace.h
>> +++ b/drivers/gpu/drm/xe/xe_trace.h
>> @@ -172,6 +172,11 @@ DEFINE_EVENT(xe_exec_queue, xe_exec_queue_memory_cat_error,
>>  	     TP_ARGS(q)
>>  );
>>
>> +DEFINE_EVENT(xe_exec_queue, xe_exec_queue_cgp_context_error,
>> +	     TP_PROTO(struct xe_exec_queue *q),
>> +	     TP_ARGS(q)
>> +);
>> +
>>  DEFINE_EVENT(xe_exec_queue, xe_exec_queue_stop,
>>  	     TP_PROTO(struct xe_exec_queue *q),
>>  	     TP_ARGS(q)
>> --
>> 2.43.0
>>