Re: [PATCH v2] drm/xe/guc: Handle GuC semaphore instruction field check error

["Ceraolo Spurio, Daniele" <daniele.ceraolospurio@intel.com>]

On 10/26/2026 11:25 AM, Jesus Narvaez wrote:
> Starting on NVL-P and newer platforms which support 64 bit semaphores,
> opt in to enable GuC to check the semaphore instruction field when it
> receives it from HW.
>
> The 64 bit semaphore instruction includes a field that is passed through
> the CSB to the GuC. The GuC then checks for any invalid values in this
> field and sends a G2H message back if the check failed, so add a
> handler to deal with the context that has been marked as unrunnable.
>
> Rejecting the unsupported values now means later on, if the values are
> used for something else like sending hints about semaphore behavior,
> there won't be any conflicts due to the change in behavior.
>
> v2:
> - Updated commit message to explain why GuC checks the instruction field
>    (Daniele)
> - Use "instruction field" in comments, but kept KLV name to match GuC
>    specs (Daniele)
> - Removed LRCA variable and print out in handler (Daniele)
>
> Signed-off-by: Jesus Narvaez <jesus.narvaez@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
>   drivers/gpu/drm/xe/abi/guc_actions_abi.h |  1 +
>   drivers/gpu/drm/xe/abi/guc_klvs_abi.h    |  9 +++++
>   drivers/gpu/drm/xe/xe_guc.c              | 10 ++++++
>   drivers/gpu/drm/xe/xe_guc_ct.c           |  4 +++
>   drivers/gpu/drm/xe/xe_guc_submit.c       | 46 ++++++++++++++++++++++++
>   drivers/gpu/drm/xe/xe_guc_submit.h       |  2 ++
>   drivers/gpu/drm/xe/xe_trace.h            |  5 +++
>   7 files changed, 77 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/abi/guc_actions_abi.h b/drivers/gpu/drm/xe/abi/guc_actions_abi.h
> index f5c9b37038d4..520b771906cd 100644
> --- a/drivers/gpu/drm/xe/abi/guc_actions_abi.h
> +++ b/drivers/gpu/drm/xe/abi/guc_actions_abi.h
> @@ -143,6 +143,7 @@ enum xe_guc_action {
>   	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_NOTIFY_CONTEXT_INVALID_SEMAPHORE_INSTRUCTION_FIELD = 0x460B,
>   	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/abi/guc_klvs_abi.h b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> index 4bcdc1a96785..dd34d61ebf3a 100644
> --- a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> +++ b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
> @@ -169,6 +169,12 @@ enum  {
>    *      This flag will enable notification from GuC to KMD via G2H message
>    *      GUC_ACTION_GUC2HOST_NOTIFY_UNCORRECTABLE_LOCAL_ERROR upon receiving the
>    *      same interrupt from the CS.
> + *
> + * _`GUC_KLV_OPT_IN_FEATURE_SEMAPHORE_INSTRUCTION_FIELDS_CHECK` : 0x400A
> + *      This flag will enable the GuC to check the semaphore instruction field

IMO this is still not clear in regards to what we're checking. Maybe say 
something like "... GuC to check for unsupported values in the semaphore 
instruction token info metadata field when GuC receives it from the HW 
via the CSB". The commit message could also use a bit more refining. 
With that:

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

> + *      when GuC receives it from HW. If the check fails the GuC removes the
> + *      context, marks it as not runnable and notifies the driver via G2H
> + *      message GUC_ACTION_GUC2HOST_CONTEXT_INVALID_SEMAPHORE_INSTRUCTION_FIELD.
>    */
>   
>   #define GUC_KLV_OPT_IN_FEATURE_EXT_CAT_ERR_TYPE_KEY 0x4001
> @@ -180,6 +186,9 @@ enum  {
>   #define GUC_KLV_OPT_IN_FEATURE_UNCORRECTABLE_LOCAL_ERROR_NOTIFICATION_KEY 0x4004
>   #define GUC_KLV_OPT_IN_FEATURE_UNCORRECTABLE_LOCAL_ERROR_NOTIFICATION_LEN 0u
>   
> +#define GUC_KLV_OPT_IN_FEATURE_SEMAPHORE_INSTRUCTION_FIELDS_CHECK_KEY 0x400A
> +#define GUC_KLV_OPT_IN_FEATURE_SEMAPHORE_INSTRUCTION_FIELDS_CHECK_LEN 0u
> +
>   /**
>    * DOC: GuC Scheduling Policies KLVs
>    *
> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> index b1f4e132fa0c..c96c02fc924a 100644
> --- a/drivers/gpu/drm/xe/xe_guc.c
> +++ b/drivers/gpu/drm/xe/xe_guc.c
> @@ -663,6 +663,16 @@ int xe_guc_opt_in_features_enable(struct xe_guc *guc)
>   	if (supports_dynamic_ics(guc))
>   		klvs[count++] = PREP_GUC_KLV_TAG(OPT_IN_FEATURE_DYNAMIC_INHIBIT_CONTEXT_SWITCH);
>   
> +	/*
> +	 * 64 bit semaphore instruction field check support was added in GuC
> +	 * v70.64.0, which maps to compatibility version v1.33.0.
> +	 * Support starts with platforms that have 64 bit semaphores, which
> +	 * is NVL-P and newer.
> +	 */
> +	if ((GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 33, 0)) &&
> +	    xe->info.platform >= XE_NOVALAKE_P)
> +		klvs[count++] = PREP_GUC_KLV_TAG(OPT_IN_FEATURE_SEMAPHORE_INSTRUCTION_FIELDS_CHECK);
> +
>   	if (count) {
>   		xe_assert(xe, count <= OPT_IN_MAX_DWORDS);
>   
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index d42f0133c1eb..27afe95c4ce7 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -1909,6 +1909,10 @@ static int process_g2h_msg(struct xe_guc_ct *ct, u32 *msg, u32 len)
>   		ret = xe_guc_exec_queue_cgp_context_error_handler(guc, payload,
>   								  adj_len);
>   		break;
> +	case XE_GUC_ACTION_NOTIFY_CONTEXT_INVALID_SEMAPHORE_INSTRUCTION_FIELD:
> +		ret = xe_guc_context_invalid_semaphore_instruction_field_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 7e2ce1a79a75..179ad7287ffd 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -3723,6 +3723,52 @@ int xe_guc_exec_queue_cgp_sync_done_handler(struct xe_guc *guc, u32 *msg, u32 le
>   	return 0;
>   }
>   
> +/**
> + * xe_guc_context_invalid_semaphore_instruction_field_handler - Invalid semaphore field handler
> + * @guc: guc
> + * @msg: message indicating which context failed the field check
> + * @len: length of message
> + *
> + * The GuC has detected an unsupported SW Token Info value in a 64 bit semaphore
> + * instruction and has already removed the offending context and marked it as
> + * not runnable, so trigger reset-cleanup on the queue to match the state GuC
> + * already applied.
> + *
> + * Return: 0 on success, -EPROTO for malformed messages.
> + */
> +int xe_guc_context_invalid_semaphore_instruction_field_handler(struct xe_guc *guc,
> +							       u32 *msg, u32 len)
> +{
> +	struct xe_gt *gt = guc_to_gt(guc);
> +	struct xe_exec_queue *q;
> +	u32 guc_id, type;
> +	u64 token, data;
> +
> +	if (unlikely(len != 8))
> +		return -EPROTO;
> +
> +	guc_id = msg[0];
> +
> +	token = (u64)msg[4] << 32 | msg[3];
> +	type  = msg[5];
> +	data  = (u64)msg[7] << 32 | msg[6];
> +
> +	q = g2h_exec_queue_lookup(guc, guc_id);
> +	if (unlikely(!q))
> +		return -EPROTO;
> +
> +	xe_gt_info(gt,
> +		   "GuC: invalid semaphore instruction field! guc_id=%d, type=%u, token=0x%llx, data=0x%llx",
> +		   guc_id, type, token, data);
> +
> +	trace_xe_guc_invalid_semaphore_instruction_field(q);
> +
> +	/* Treat the same as engine reset */
> +	xe_guc_exec_queue_reset_trigger_cleanup(q);
> +
> +	return 0;
> +}
> +
>   static void
>   guc_exec_queue_wq_snapshot_capture(struct xe_exec_queue *q,
>   				   struct xe_guc_submit_exec_queue_snapshot *snapshot)
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.h b/drivers/gpu/drm/xe/xe_guc_submit.h
> index 7a81d0aa19bb..3eef8784a1a5 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.h
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.h
> @@ -41,6 +41,8 @@ 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);
> +int xe_guc_context_invalid_semaphore_instruction_field_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 d4e9d91f6f7f..e52054eccc8d 100644
> --- a/drivers/gpu/drm/xe/xe_trace.h
> +++ b/drivers/gpu/drm/xe/xe_trace.h
> @@ -223,6 +223,11 @@ DEFINE_EVENT(xe_exec_queue, xe_exec_queue_cgp_context_error,
>   	     TP_ARGS(q)
>   );
>   
> +DEFINE_EVENT(xe_exec_queue, xe_guc_invalid_semaphore_instruction_field,
> +	     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)