Re: [PATCH 1/1] drm/xe/survivability: broadcast dmesg when engaging survivability

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/xe: unreachable branch and wrong log in xe_device_probe_early()
--

commit 9d8c24e19d32b177027b35cc906adcdcf29823ef
Author: Alan Previn <alan.previn.teres.alexis@intel.com>

drm/xe/survivability: broadcast dmesg when engaging survivability

This commit adds dmesg logging in xe_device_probe_early() when the driver
encounters early pcode errors or when survivability mode is requested.

> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index e1979bbcf7bc6..49df992253a9a 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -702,7 +702,8 @@ static int xe_device_vram_alloc(struct xe_device *xe)
>   */
>  int xe_device_probe_early(struct xe_device *xe)
>  {
> -	int err;
> +	int err, pcode_err;
> +	bool survival_requested;
>  
>  	xe_wa_device_init(xe);
>  	xe_wa_process_device_oob(xe);
> @@ -726,20 +727,37 @@ int xe_device_probe_early(struct xe_device *xe)
>  	 * initialization is complete. Until done, any MMIO or lmem access from
>  	 * the driver will be blocked
>  	 */
> -	err = xe_pcode_probe_early(xe);
> -	if (err || xe_survivability_mode_is_requested(xe)) {
> -		int save_err = err;
> -
> +	pcode_err = xe_pcode_probe_early(xe);
> +	survival_requested = xe_survivability_mode_is_requested(xe);
> +	if (pcode_err || survival_requested) {
>  		/*
>  		 * Try to leave device in survivability mode if device is
>  		 * possible, but still return the previous error for error
>  		 * propagation
>  		 */
> -		err = xe_survivability_mode_boot_enable(xe);
> -		if (err)
> -			return err;
> -
> -		return save_err;
> +		int survive_err = xe_survivability_mode_boot_enable(xe);
> +
> +		if (survive_err) {
> +			if (pcode_err && survival_requested)
> +				xe_err(xe, "Early pcode err(%pe), survivability requested but unavailable(%pe)\n",
> +				       ERR_PTR(pcode_err), ERR_PTR(survive_err));
> +			else if (!pcode_err)
> +				xe_warn(xe, "Early pcode ok, survivability requested but unavailable(%pe)\n",
> +					ERR_PTR(survive_err));
> +			else
> +				xe_warn(xe, "Early pcode err(%pe), survivability not requested and unavailable(%pe)\n",
> +					ERR_PTR(pcode_err), ERR_PTR(survive_err));
> +			return survive_err;
> +		}
> +		if (pcode_err && survival_requested)
> +			xe_notice(xe, "Early pcode err(%pe), survivability requested\n",
> +				  ERR_PTR(pcode_err));
> +		else if (!pcode_err)
> +			xe_notice(xe, "Early pcode ok but survivability requested\n");
> +		else
> +			xe_warn(xe, "Early pcode err(%pe), survivability not requested. Consider xe_configfs\n",
> +				ERR_PTR(pcode_err));

[Severity: Medium]
Can the first else branch inside if (survive_err) ever be reached, and does
the second else branch log a misleading message on unsupported platforms?

In drivers/gpu/drm/xe/xe_survivability_mode.c,
xe_survivability_mode_boot_enable() immediately re-checks
xe_survivability_mode_is_requested(xe) and returns 0 when it is false:

int xe_survivability_mode_boot_enable(struct xe_device *xe)
{
	struct xe_survivability *survivability = &xe->survivability;
	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);

	if (!xe_survivability_mode_is_requested(xe))
		return 0;

And xe_survivability_mode_is_requested() returns false both when
survivability is not requested and when the platform or VF does not support
survivability mode:

bool xe_survivability_mode_is_requested(struct xe_device *xe)
{
	...
	if (!IS_DGFX(xe) || IS_SRIOV_VF(xe) || xe->info.platform < XE_BATTLEMAGE)
		return false;

Because xe_survivability_mode_boot_enable() returns 0 whenever
xe_survivability_mode_is_requested() is false:

1. In xe_device_probe_early(), when pcode_err is non-zero and
   survival_requested is false, survive_err will be 0 (unless the redundant
   second call to xe_survivability_mode_is_requested() reads a changed
   configfs or PCODE_SCRATCH(0) state, in which case survivability was
   actually requested). That makes the "Early pcode err(%pe), survivability
   not requested and unavailable(%pe)" branch unreachable under normal
   execution.

2. On integrated GPUs, SR-IOV VFs, or dGPUs prior to XE_BATTLEMAGE where
   survivability mode is unsupported (and where xe_config_device_is_visible()
   in drivers/gpu/drm/xe/xe_configfs.c hides attr_survivability_mode), an
   early pcode failure will have survive_err == 0 and survival_requested ==
   false, falling through to log "Consider xe_configfs" even though the
   survivability_mode configfs attribute is not available on that device.

Should xe_survivability_mode_boot_enable() or its helper distinguish between
"unsupported" and "not requested", and avoid calling
xe_survivability_mode_is_requested() twice?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023001827.170645-2-alan.previn.teres.alexis@intel.com?part=1