Re: [PATCH v2 01/19] accel: ethosu: Suspend after initialization

[Rob Herring <robh@kernel.org>]

On Sat, Sep 05, 2026 at 12:57:34AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] IRQ handler accesses hardware registers while device may be suspended.
> --
> 
> commit db83f58315d24bc892c6b008ce7d120f57ecaff1
> Author: Rob Herring (Arm) <robh@kernel.org>
> 
> accel: ethosu: Suspend after initialization
> 
> The initial runtime-PM reference is held only while initializing the NPU.
> Release it synchronously from ethosu_init() after the final hardware
> access, before registering the DRM device.
> 
> > diff --git a/drivers/accel/ethosu/ethosu_drv.c b/drivers/accel/ethosu/ethosu_drv.c
> > index 8108622de258f..df76253d01a64 100644
> > --- a/drivers/accel/ethosu/ethosu_drv.c
> > +++ b/drivers/accel/ethosu/ethosu_drv.c
> > @@ -338,6 +338,8 @@ static int ethosu_init(struct ethosu_device *ethosudev)
> >  		 ethosudev->npu_info.sram_size / 1024,
> >  		 ethosudev->npu_info.pmu_counters);
> >  
> > +	pm_runtime_put_sync_suspend(ethosudev->base.dev);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does putting the device into
> an immediate synchronous suspend here expose a vulnerability in the interrupt
> handler?
> 
> Looking at ethosu_job_irq_handler() in drivers/accel/ethosu/ethosu_job.c:
> 
> static irqreturn_t ethosu_job_irq_handler(int irq, void *data)
> {
> 	struct ethosu_device *dev = data;
> 	u32 status = readl_relaxed(dev->regs + NPU_REG_STATUS);
> 	...
> }
> 
> The hardirq handler accesses hardware registers unconditionally without
> verifying the device's PM state.
> 
> If a spurious interrupt occurs while the device is in the RPM_SUSPENDED
> state (where clocks are disabled via ethosu_device_suspend()), could
> reading NPU_REG_STATUS cause a Synchronous External Abort?
> 
> The PM subsystem guidelines mandate using pm_runtime_get_if_active() in
> IRQ handlers to prevent accessing powered-off hardware registers.

Not an issue.

The h/w will only generate an interrupt if a job has been submitted in 
which case we are not suspended.

Rob