Re: [PATCH v13 04/14] accel/rocket: let the core suspend after a reset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition in reset path prevents required IOMMU hardware recovery
--

commit b227f4b369396fb45d834eeade730dbc33623b84
Author: Jiaxing Hu <gahing@gahingwoo.com>

    accel/rocket: let the core suspend after a reset

This commit replaces pm_runtime_put_noidle() with pm_runtime_put_autosuspend()
in the rocket_reset() path to allow the core to suspend. Cycling the power
domain issues a bus interface reset that is needed to recover the IOMMU.

Link: https://lore.kernel.org/all/20260819073530.6087-1-royalnet026@gmail.com/

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index dfe9135d86af5..2a0b8af6f67da 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -437,12 +437,12 @@ rocket_reset(struct rocket_core *core, struct drm_sched_job *bad)
>  
>  	/*
>  	 * No handler is running now, but we might still have stuck jobs. Let's
> -	 * make sure the PM counters stay balanced by manually calling
> -	 * pm_runtime_put_noidle().
> +	 * make sure the PM counters stay balanced by putting the reference the
> +	 * job took, and request idle while doing it so the core can suspend.
>  	 */
>  	scoped_guard(mutex, &core->job_lock) {
>  		if (core->in_flight_job)
> -			pm_runtime_put_noidle(core->dev);
> +			pm_runtime_put_autosuspend(core->dev);

[Severity: High]
Does using pm_runtime_put_autosuspend() here guarantee the synchronous hardware
power cycle required to recover the IOMMU?

Since pm_runtime_put_autosuspend() is asynchronous, it merely queues a suspend
with a delay. A few lines below, rocket_reset() calls drm_sched_start():

drivers/accel/rocket/rocket_job.c:rocket_reset() {
	...
	/* Restart the scheduler */
	drm_sched_start(&core->sched, 0);
}

If the scheduler has jobs queued, it will immediately dequeue the next job and
execute rocket_job_run(). This function calls pm_runtime_resume_and_get(),
which increments the usage count and cancels the pending autosuspend before the
timer can expire.

Could this race condition prevent the power domain from cycling under continuous
load, leaving the NPU's IOMMU unresponsive and causing subsequent jobs to fail?

Would it be safer to use a synchronous put like pm_runtime_put_sync() when
hardware ordering constraints require the device to be powered down before the
next operation?

>  
>  		iommu_detach_group(NULL, core->iommu_group);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915104328.45901-1-gahing@gahingwoo.com?part=4