Re: [PATCH v5 28/33] drm/xe/pcode: Report 'initialization timedout' error using SIGID

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The polling loop in `pcode_try_request` uses unbounded exponential backoff, causing massive delay intervals, dangerous `udelay()` values, and incorrect timeout calculations.
--

--- Patch [28]: [PATCH v5 28/33] drm/xe/pcode: Report 'initialization timedout' error using SIGID ---
commit 37b63ecd447fce41ca2633f291eae720f6ea1ba1
Author: Michal Wajdeczko <[email protected]>

drm/xe/pcode: Report 'initialization timedout' error using SIGID

This commit updates xe_pcode_ready() to report initialization timeouts
using the xe_log_err helper with a SIGID. It also replaces the hardcoded
integer timeout with a calculated value using USEC_PER_SEC.

> diff --git a/drivers/gpu/drm/xe/xe_pcode.c b/drivers/gpu/drm/xe/xe_pcode.c
> index 14b13a11d1d6d..e1b8062541a91 100644
> --- a/drivers/gpu/drm/xe/xe_pcode.c
> +++ b/drivers/gpu/drm/xe/xe_pcode.c
> @@ -297,7 +297,7 @@ int xe_pcode_ready(struct xe_device *xe, bool locked)
>  {
>  	u32 status, request = DGFX_GET_INIT_STATUS;
>  	struct xe_tile *tile = xe_device_get_root_tile(xe);
> -	int timeout_us = 180000000; /* 3 min */
> +	long timeout_us = 3 * 60 * USEC_PER_SEC; /* 3 min */

[Severity: High]
This is a pre-existing issue, but does the polling loop in
pcode_try_request() have an uncapped exponential backoff that causes
dangerous wait intervals here?

With timeout_us set to 3 minutes (180,000,000 us), the backoff loop
in pcode_try_request() doubles the wait variable repeatedly:

	for (slept = 0; slept < timeout_us; slept += wait) {
...
		if (atomic)
			udelay(wait);
		else
			usleep_range(wait, wait << 1);
		wait <<= 1;
	}

Because there is no cap on wait, it will grow exponentially until it
reaches ~83 seconds, leading to a massive sleep interval that could
break hardware responsiveness and stall the kernel thread.

Additionally, when xe_pcode_request() polls with preemption disabled
(atomic = true), wait can grow up to ~32ms, leading to udelay(32000),
which exceeds architecture limits for udelay() and risks soft lockups.

Also, since slept adds wait before the actual sleep duration is completed,
and wait is then doubled, does this miscalculate the elapsed time and
effectively halve the actual timeout?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=28