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 <[email protected]> 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/[email protected]/ > 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/[email protected]?part=4
