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

["Poosa, Karthik" <karthik.poosa@intel.com>]

On 24-09-2026 16:53, sashiko-bot@kernel.org wrote:
> 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?
I shall return -EIO instead of 0 here in next revision.
>
>> +	} 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);