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: duplicate and false logs in xe_survivability_mode_is_requested()
--

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

drm/xe/survivability: broadcast dmesg when engaging survivability

This commit adds kernel log messages to xe_survivability_mode_is_requested()
when boot survivability mode is requested via configfs or when PCODE_SCRATCH(0)
reports a boot failure.

> diff --git a/drivers/gpu/drm/xe/xe_survivability_mode.c b/drivers/gpu/drm/xe/xe_survivability_mode.c
> index 45d44ebe288b4..3eebb69728c3f 100644
> --- a/drivers/gpu/drm/xe/xe_survivability_mode.c
> +++ b/drivers/gpu/drm/xe/xe_survivability_mode.c
> @@ -412,13 +412,19 @@ bool xe_survivability_mode_is_requested(struct xe_device *xe)
>  
>  	survivability_mode = xe_configfs_get_survivability_mode(pdev);
>  	/* Enable survivability mode if set via configfs */
> -	if (survivability_mode)
> +	if (survivability_mode) {
> +		xe_log_info(xe, SURVIVABILITY, "Boot mode requested.");
>  		return true;
> +	}
>  
>  	data = xe_mmio_read32(mmio, PCODE_SCRATCH(0));
>  	survivability->boot_status = REG_FIELD_GET(BOOT_STATUS, data);
>  
> -	return check_boot_failure(xe);
> +	if (check_boot_failure(xe)) {
> +		xe_log_err_fatal(pdev, SURVIVABILITY, 0, "Boot Mode enabled!\n");
> +		return true;
> +	}
> +	return false;
>  }

[Severity: Medium]
Does logging inside xe_survivability_mode_is_requested() cause duplicate and
premature messages in dmesg?

During probe, xe_device_probe_early() calls
xe_survivability_mode_is_requested() in the condition and then calls
xe_survivability_mode_boot_enable(), which calls
xe_survivability_mode_is_requested() a second time:

drivers/gpu/drm/xe/xe_device.c:xe_device_probe_early() {
	...
	err = xe_pcode_probe_early(xe);
	if (err || xe_survivability_mode_is_requested(xe)) {
		int save_err = err;
		...
		err = xe_survivability_mode_boot_enable(xe);
}

drivers/gpu/drm/xe/xe_survivability_mode.c:xe_survivability_mode_boot_enable() {
	...
	if (!xe_survivability_mode_is_requested(xe))
		return 0;
}

When xe_pcode_probe_early() returns 0 (for example, when survivability mode is
enabled via configfs), xe_survivability_mode_is_requested() runs twice back to
back and emits the log message twice. Also, "Boot mode requested." is missing a
trailing newline.

For the check_boot_failure(xe) path, enable_boot_survivability_mode() already
logs the exact same message once boot survivability mode has actually been
initialized:

drivers/gpu/drm/xe/xe_survivability_mode.c:enable_boot_survivability_mode() {
	...
	if (check_boot_failure(xe))
		xe_log_err_fatal(pdev, SURVIVABILITY, 0, "Boot Mode enabled!\n");
	else
		xe_log_info(pdev, SURVIVABILITY, "Boot Mode enabled!\n");

	return 0;

failed:
	xe_log_err_fatal(pdev, SURVIVABILITY, ret, "Failed to enable Boot Mode!\n");
	survivability->mode = false;
	return ret;
}

Logging "Boot Mode enabled!\n" inside xe_survivability_mode_is_requested() not
only duplicates the message in enable_boot_survivability_mode(), but also
reports that boot mode is enabled before initialization has happened. If
xe_survivability_mode_boot_enable() aborts with -ENXIO (when
survivability->version < 2 and boot_status is CRITICAL_FAILURE) or if
enable_boot_survivability_mode() fails in create_survivability_sysfs(),
xe_nvm_init(), or xe_i2c_probe(), dmesg will falsely report "Boot Mode
enabled!" before failing.

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