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 2 potential issue(s) to consider:
- [Medium] drm/xe/survivability: missing boot survivability log when pcode probe fails
- [Low] drm/xe/survivability: missing static on survivability_mode_required()
--

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

drm/xe/survivability: broadcast dmesg when engaging survivability

This commit renames xe_survivability_mode_is_requested() to
xe_survivability_mode_is_required() and moves boot survivability logging from
enable_boot_survivability_mode() into survivability_mode_required() when
log_on_action is true.

> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 7e6027f49a4ca..bb0ccae7f13a8 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -727,7 +727,7 @@ int xe_device_probe_early(struct xe_device *xe)
>  	 * the driver will be blocked
>  	 */
>  	err = xe_pcode_probe_early(xe);
> -	if (err || xe_survivability_mode_is_requested(xe)) {
> +	if (err || xe_survivability_mode_is_required(xe)) {
>  		int save_err = err;
>  
>  		/*
> diff --git a/drivers/gpu/drm/xe/xe_survivability_mode.c b/drivers/gpu/drm/xe/xe_survivability_mode.c
> index 45d44ebe288b4..50af47ac58cc7 100644
> --- a/drivers/gpu/drm/xe/xe_survivability_mode.c
> +++ b/drivers/gpu/drm/xe/xe_survivability_mode.c
> @@ -363,11 +363,6 @@ static int enable_boot_survivability_mode(struct pci_dev *pdev)
>  	if (ret)
>  		goto failed;
>  
> -	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:
> @@ -389,17 +384,7 @@ bool xe_survivability_mode_is_boot_enabled(struct xe_device *xe)
>  	return survivability->mode && survivability->type == XE_SURVIVABILITY_TYPE_BOOT;
>  }
>  
> -/**
> - * xe_survivability_mode_is_requested - check if it's possible to enable survivability
> - *					mode that was requested by firmware or userspace
> - * @xe: xe device instance
> - *
> - * This function reads configfs and  boot status from Pcode.
> - *
> - * Return: true if platform support is available and boot status indicates
> - * failure or if survivability mode is requested, false otherwise.
> - */
> -bool xe_survivability_mode_is_requested(struct xe_device *xe)
> +bool survivability_mode_required(struct xe_device *xe, bool log_on_action)

[Severity: Low]
Should survivability_mode_required() be marked static? It is only called
within drivers/gpu/drm/xe/xe_survivability_mode.c and has no prototype in
drivers/gpu/drm/xe/xe_survivability_mode.h, which will trigger a
-Wmissing-prototypes compiler warning (and fail the build when
CONFIG_DRM_XE_WERROR or CONFIG_WERROR is enabled).

>  {
>  	struct xe_survivability *survivability = &xe->survivability;
>  	struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> @@ -412,13 +397,36 @@ 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) {
> +		if (log_on_action)
> +			xe_log_info(xe, SURVIVABILITY, "Boot mode requested.\n");
>  		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)) {
> +		if (log_on_action)
> +			xe_log_err_fatal(pdev, SURVIVABILITY, 0, "Boot Mode required!\n");
> +		return true;
> +	}
> +	return false;
> +}
> +
> +/**
> + * xe_survivability_mode_is_required - check if survivability mode is required based on
> + *                                     state of firmware or userspace request.
> + * @xe: xe device instance
> + *
> + * This function reads configfs and boot status from Pcode.
> + *
> + * Return: true if platform support is available and boot status indicates
> + * failure or if survivability mode is requested, false otherwise.
> + */
> +bool xe_survivability_mode_is_required(struct xe_device *xe)
> +{
> +	return survivability_mode_required(xe, true);
>  }
>  
>  /**
> @@ -468,7 +476,7 @@ 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))
> +	if (!survivability_mode_required(xe, false))

[Severity: Medium]
Does this suppress all boot survivability dmesg messages when
xe_pcode_probe_early() returns an error?

In drivers/gpu/drm/xe/xe_device.c, xe_device_probe_early() checks:

	err = xe_pcode_probe_early(xe);
	if (err || xe_survivability_mode_is_required(xe)) {
		int save_err = err;

		err = xe_survivability_mode_boot_enable(xe);

When pcode initialization fails (err != 0), the || operator short-circuits
and skips calling xe_survivability_mode_is_required(xe). Execution then
enters xe_survivability_mode_boot_enable(xe), which calls
survivability_mode_required(xe, false) with log_on_action set to false.

Because the "Boot Mode enabled!" messages were also removed from
enable_boot_survivability_mode(), won't entering boot survivability mode on
a pcode probe failure result in no survivability message being logged at
all?

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