Re: [PATCH v12 04/16] drm/xe/hwmon: Add helpers to validate thermal sensor readings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hwmon: Validation helpers fail to filter U32_MAX/0xFF on pre-CRI platforms
--

commit 3734bfcfc0e55f578f66176481a816cff2820aea
Author: Karthik Poosa <karthik.poosa@intel.com>
drm/xe/hwmon: Add helpers to validate thermal sensor readings

This commit adds mmio_temp_valid() and mbx_temp_valid() helpers to validate
temperature sensor readings. It introduces has_temp_sentinel to use U32_MAX
or 0xFF as invalid indicators on supported platforms, falling back to checking
for 0 on other platforms.

> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index b1d57f4910579..418e1c503eb5a 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c

[ ... ]

> @@ -825,6 +828,23 @@ static int xe_hwmon_pcode_read_thermal_info(struct xe_hwmon *hwmon)
>  	return ret;
>  }
>  
> +static inline bool mmio_temp_valid(const struct xe_hwmon *hwmon, u32 value)
> +{
> +	/*
> +	 * On platforms lacking temperature sentinel support, a value of 0 is treated
> +	 * as invalid. While 0°C is theoretically possible, it is highly unlikely for
> +	 * a discrete GPU in normal environments, making it a reasonable fallback
> +	 * indicator for invalid or uninitialized temperature readings.
> +	 */
> +	return hwmon->xe->info.has_temp_sentinel ? value != U32_MAX : value != 0;

[Severity: Medium]
Does this allow standard MMIO read failures (U32_MAX) to bypass the filter on
platforms lacking has_temp_sentinel?

If a device on a platform before CRI encounters a PCI read failure returning
U32_MAX, the ternary fallback (value != 0) evaluates to true. Any userspace
read of the hwmon sysfs nodes during such conditions would expose bogus
extreme temperatures instead of properly returning an error.

> +}
> +
> +/* Mailbox temperature is sign-magnitude (bit 7 sign, bits 6:0 magnitude). */
> +static inline bool mbx_temp_valid(const struct xe_hwmon *hwmon, u8 value)
> +{
> +	return hwmon->xe->info.has_temp_sentinel ? value != TEMP_MBX_INVALID : value != 0;

[Severity: Medium]
Does this allow 0xFF (TEMP_MBX_INVALID) to be treated as a valid reading on
platforms lacking has_temp_sentinel?

If a device reads an unpopulated or unavailable mailbox sensor returning 0xFF,
the fallback check (value != 0) evaluates to true. This would bypass the filter
and erroneously decode the reading to -127 degrees Celsius.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013184939.694945-1-karthik.poosa@intel.com?part=4