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) <[email protected]>

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.

> +
>       return 0;
>  }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to