Re: [PATCH v4 2/7] drm/xe/guc: Handle CRASH and EXCEPTION G2H with separate helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] xe: spurious PROCESS_FAILED on GuC crash due to -EHOSTDOWN return
--

commit 0030c089dd9bc1455460d3cc58697be48bcade56
Author: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

drm/xe/guc: Handle CRASH and EXCEPTION G2H with separate helpers

This commit refactors the handling of CRASH and EXCEPTION G2H messages
into separate helper functions. It also drops an unreachable code path
that existed in the earlier combined function.

> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index 5c4733da385c5..b3a6aa37808b4 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -1553,22 +1553,22 @@ static int parse_g2h_event(struct xe_guc_ct *ct, u32 *msg, u32 len)
>  	return 0;
>  }
>  
> -static int guc_crash_process_msg(struct xe_guc_ct *ct, u32 action)
> +static int guc_handle_crash_dump_msg(struct xe_guc_ct *ct, u32 action)
>  {
> -	struct xe_gt *gt = ct_to_gt(ct);
> +	xe_gt_err(ct_to_gt(ct), "GuC Crash dump notification\n");
> +	CT_DEAD(ct, NULL, CRASH);
> +	kick_reset(ct);
>  
> -	if (action == XE_GUC_ACTION_NOTIFY_CRASH_DUMP_POSTED)
> -		xe_gt_err(gt, "GuC Crash dump notification\n");
[ ... ]
> +	return -EHOSTDOWN;
> +}

[Severity: Medium]
Since this function now returns -EHOSTDOWN instead of 0, will this trigger the
failure path in process_g2h_msg()?

When process_g2h_msg() receives a non-zero return value, it logs a spurious
"G2H action failed" error and redundantly flags the CT as dead:

    if (ret) {
        xe_gt_err(gt, "G2H action %#04x failed (%pe) len %u msg %*ph\n", ...);
        CT_DEAD(ct, NULL, PROCESS_FAILED);
    }

Does this misrepresent a successfully processed firmware notification as a
host-side parsing failure?

>  
> +static int guc_handle_exception_msg(struct xe_guc_ct *ct, u32 action)
> +{
> +	xe_gt_err(ct_to_gt(ct), "GuC Exception notification\n");
>  	CT_DEAD(ct, NULL, CRASH);
> -
>  	kick_reset(ct);
>  
> -	return 0;
> +	return -EHOSTDOWN;
>  }

[Severity: Medium]
Similarly here, returning -EHOSTDOWN instead of 0 will cause process_g2h_msg()
to execute its error handling path. Should these helpers continue to return 0
to indicate the notification was successfully processed by the host?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917235923.1521112-9-umesh.nerlige.ramappa@intel.com?part=2