Re: [PATCH v4 27/32] 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] Uncapped exponential backoff causes massive over-sleeping and potential kernel panic.
--

--- Patch [27]: [PATCH v4 27/32] drm/xe/pcode: Report 'initialization timedout' error using SIGID ---
commit 3feb217014d673b0d9d119f4debacacb0e357e77
Author: Michal Wajdeczko <[email protected]>

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

This commit updates the PCODE initialization timeout error to use the
xe_log_err() helper instead of drm_err(). It also refactors the 3 minute
timeout constant to use 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)
[ ... ]
>  	if (locked)
>  		mutex_lock(&tile->pcode.lock);
>  
>  	ret = pcode_try_request(tile, DGFX_PCODE_STATUS, request,
>  				DGFX_INIT_STATUS_COMPLETE,
>  				DGFX_INIT_STATUS_COMPLETE,
>  				&status, false, timeout_us, locked);

[Severity: High]
This is a pre-existing issue, but can the 3-minute timeout passed here
trigger a kernel panic via the Hung Task Detector if PCODE is unresponsive?

Looking at pcode_try_request() in drivers/gpu/drm/xe/xe_pcode.c, the wait
duration is exponentially increased without any cap:

	usleep_range(wait, wait << 1);
	wait <<= 1;

When the timeout is 180,000,000 us (3 minutes), the polling loop doubles the
wait variable every iteration. When the total slept time reaches ~167 seconds,
the wait variable becomes ~83.8 seconds. The loop then executes:

	usleep_range(83886080, 167772160);

Since usleep_range() sleeps in TASK_UNINTERRUPTIBLE, sleeping for up to 167.7
seconds exceeds the default 120-second hung task watchdog timeout, causing a
panic. Furthermore, if atomic=true was used with a large timeout, it would
result in a massive udelay(), violating architectural limits with preemption
disabled.

Could a cap be added to the maximum wait time in pcode_try_request() to
prevent massive over-sleeping?

>  
>  	if (locked)
>  		mutex_unlock(&tile->pcode.lock);
>  
>  	if (ret)
> -		drm_err(&xe->drm,
> -			"PCODE initialization timedout after: 3 min\n");
> +		xe_log_err(tile, PCODE, ret, "initialization timedout after %ld seconds\n",
> +			   timeout_us / USEC_PER_SEC);
>  
>  	return ret;
>  }

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