Re: [PATCH i-g-t v4 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 xe_hwmon_power_max_interval_store() causes infinite userspace loops
--

commit 719c9331149239878e8f9d385268d89b8d17624f
Author: Karthik Poosa <karthik.poosa@intel.com>

drm/xe/hwmon: Handle pcode read failures of xe_hwmon_pcode_rmw_power_limit

This patch handles pcode read failures in xe_hwmon_pcode_rmw_power_limit() by returning early and checking the error in callers. This prevents power limit settings from being updated with invalid data when the hardware read fails.

> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index 5edeac961ec33..514573f172ebb 100644
> --- 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]
Could setting count to 0 here instead of returning a negative error code 
cause an infinite loop in userspace tools?

Returning 0 from a sysfs store callback like 
xe_hwmon_power_max_interval_store() tells the VFS that 0 bytes were written. 
Many userspace writers, including bash loops and standard library buffered 
writers, will interpret a 0-byte write as a signal to retry the write 
indefinitely. 

If the underlying pcode mailbox hardware operation fails, would it be better 
to propagate the negative error code (such as the return value of 
xe_hwmon_pcode_rmw_power_limit()) so the userspace write operation properly 
fails?

> +	} 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/20260924105000.2627929-1-karthik.poosa@intel.com?part=1