Re: [PATCH v5 01/13] drm/xe/hwmon: Handle pcode read failures of xe_hwmon_pcode_rmw_power_limit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] sysfs: returning 0 on error in store callback causes userspace infinite loop
--

commit fdbe8069cbe324980fc35768d7993da6a046117b
Author: Karthik Poosa <karthik.poosa@intel.com>
drm/xe/hwmon: Handle pcode read failures of xe_hwmon_pcode_rmw_power_limit

This commit adds error handling for pcode read failures in
xe_hwmon_pcode_rmw_power_limit() and checks the returned error in callers
to prevent updating power limit settings with invalid data.

> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
[ ... ]
> @@ -679,11 +683,13 @@ xe_hwmon_power_max_interval_store(struct device *dev, struct device_attribute *a
>  
>  	mutex_lock(&hwmon->hwmon_lock);
>  
> -	if (hwmon->xe->info.has_mbx_power_limits)
> -		xe_hwmon_pcode_rmw_power_limit(hwmon, power_attr, channel, PWR_LIM_TIME, rxy);
> -	else
> +	if (hwmon->xe->info.has_mbx_power_limits) {
> +		if (xe_hwmon_pcode_rmw_power_limit(hwmon, power_attr, channel, PWR_LIM_TIME, rxy))
> +			count = 0;

[Severity: Medium]
In xe_hwmon_power_max_interval_store() within drivers/gpu/drm/xe/xe_hwmon.c,
will setting count to 0 here cause userspace tools to enter an infinite loop?

When a sysfs store callback returns 0 instead of a negative error code, it
indicates to the virtual file system that 0 bytes were written and swallows
the error. Standard userspace utilities (like echo or dd) often treat a
0-byte write as a short write and retry infinitely.

If an administrative user or script writes to the sysfs attribute and the
underlying pcode mailbox operation fails, could this cause an infinite loop
that consumes maximum processor time while simultaneously flooding the kernel
log with error messages?

Should this return the negative error code from
xe_hwmon_pcode_rmw_power_limit() instead of setting count to 0?

> +	} else {
>  		r = xe_mmio_rmw32(mmio, xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT, channel),
>  				  PWR_LIM_TIME, rxy);
> +	}
>  
>  	mutex_unlock(&hwmon->hwmon_lock);

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