Re: [PATCH v9 07/16] drm/panfrost: Explicitly enable MMU interrupts at device init

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the GPU reset sequence (`panfrost_reset`) synchronizes the Job Management (JM) IRQ but fails to mask or synchronize the MMU IRQ, leading to a TOCTOU data race that can corrupt the driver state for newly submitted jobs.
- [High] This problem wasn't introduced by this patch, but during driver unbind (`panfrost_remove`) or probe failure, clocks and power domains are disabled while shared IRQ handlers remain active, which can lead to kernel crashes if the shared IRQ line is asserted.
--

commit 8161749d65dccd8c767c11da6233beaf16346d71
Author: Adrián Larumbe <adrian.larumbe@collabora.com>

drm/panfrost: Explicitly enable MMU interrupts at device init

Because the device must be in a position to accept jobs between the time
drm_dev_register() is called and autosuspend first kicks in, this commit
enables MMU interrupts at device initialisation time. This avoids a narrow
window where jobs targeting the tiler buffer object could time out.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index 5c393ed6e3101..faa62643caf1d 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
[ ... ]
> @@ -355,8 +361,7 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev)
>  
>  	spin_unlock(&pfdev->as_lock);
>  
> -	mmu_write(pfdev, MMU_INT_CLEAR, ~0);
> -	mmu_write(pfdev, MMU_INT_MASK, ~0);
> +	panfrost_mmu_enable_interrupts(pfdev);

[Severity: High]
This isn't a bug introduced by this patch, but does the GPU reset sequence
have a data race here?

When panfrost_reset() in drivers/gpu/drm/panfrost/panfrost_job.c triggers
a reset, it synchronizes the JM IRQ but fails to mask or synchronize the
MMU IRQ:

drivers/gpu/drm/panfrost/panfrost_job.c:panfrost_reset() {
	...
	job_write(pfdev, JOB_INT_MASK, 0);
	synchronize_irq(pfdev->js->irq);
	...
}

Because the MMU IRQ isn't synchronized, panfrost_mmu_irq_handler_thread()
can execute concurrently with panfrost_device_reset() and
panfrost_mmu_reset().

Since panfrost_mmu_enable_interrupts() modifies the MMU_INT_MASK hardware
register outside of pfdev->as_lock, could this race cause pfdev->as_faulty_mask
and the hardware register to become inconsistent if a fault occurs exactly
during reset, leading to timeouts for newly submitted jobs?

>  }
>  
[ ... ]
> @@ -973,6 +978,8 @@ int panfrost_mmu_init(struct panfrost_device *pfdev)
>  		return err;
>  	}
>  
> +	panfrost_mmu_enable_interrupts(pfdev);
> +

[Severity: High]
This is a pre-existing issue, but could enabling interrupts here cause a bus
error if the probe fails or during driver unbind?

If probe fails later in panfrost_device_init() in 
drivers/gpu/drm/panfrost/panfrost_device.c, or during module unload in
panfrost_remove() in drivers/gpu/drm/panfrost/panfrost_drv.c,
panfrost_mmu_fini() is called to clear the mask:

drivers/gpu/drm/panfrost/panfrost_mmu.c:panfrost_mmu_fini() {
	mmu_write(pfdev, MMU_INT_MASK, 0);
}

However, this doesn't set PANFROST_COMP_BIT_MMU in pfdev->is_suspended or
call synchronize_irq(). The clocks and power domains are then disabled, but
the devm-managed shared IRQ remains registered until the probe or remove
function fully returns.

If the shared IRQ line is asserted during this teardown window, won't
panfrost_mmu_irq_handler() bypass the is_suspended check and read
MMU_INT_STAT without active clocks, resulting in a synchronous external abort?

>  	return 0;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912-claude-fixes-v9-0-e588feaa61ef@collabora.com?part=7