Re: [PATCH 2/2] media: ipu6: Only call the isys and psys ISRs for their own interrupts

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Felipe,

Thanks for the patch.

On Wed, Sep 23, 2026 at 08:42:24PM -0300, Felipe Calliari wrote:
> ipu6_buttress_isr() calls the isys and psys ISRs on every buttress
> interrupt, and only afterwards checks whether the interrupt was theirs.
> So each isys interrupt also runs the psys ISR, and each psys interrupt
> runs the isys ISR, only to have the result discarded.
> 
> Beyond the wasted work, this makes an interrupt for one device
> dereference the other device's hooks. After intel_ipu6_psys is unloaded,
> its adev->auxdrv_data still points into the unloaded module, and every
> isys interrupt, e.g. on the next stream, calls through it. This matches
> a hard lockup without a trace reported on the first stream after
> unloading the psys driver. On a Samsung Galaxy Book3 Ultra,
> instrumenting the ISR showed that a 60-frame capture after
> "rmmod intel_ipu6_psys" would have made at least ten calls through the
> stale psys hooks. The same capture made none with this change, and
> captured all 60 frames.

The problem here is a dangling pointer. Can you also post a patch to set
that drvdata to NULL?

The above paragraph isn't relevant in the context of this patch.

> 
> Check the interrupt status bit before calling the ISR.
> 
> Reported-by: Mars-Wave <tmorolias@gmail.com>
> Closes: https://lore.kernel.org/linux-media/20260922063507.690-1-tmorolias@gmail.com/
> Fixes: ab29a2478e70 ("media: intel/ipu6: add IPU6 buttress interface driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Felipe Calliari <calliarifelipe@gmail.com>
> ---
>  drivers/media/pci/intel/ipu6/ipu6-buttress.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-buttress.c b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
> index 63197f746..74c191d72 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-buttress.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
> @@ -369,11 +369,13 @@ irqreturn_t ipu6_buttress_isr(int irq, void *isp_ptr)
>  		writel(irq_status, isp->base + regs->irq_clear);
>  
>  		for (i = 0; i < ARRAY_SIZE(adev_irq_mask); i++) {
> -			irqreturn_t r = ipu6_buttress_call_isr(adev[i]);
> +			irqreturn_t r;
>  
>  			if (!(irq_status & adev_irq_mask[i]))
>  				continue;
>  
> +			r = ipu6_buttress_call_isr(adev[i]);
> +

No need for this newline.

>  			if (r == IRQ_WAKE_THREAD) {
>  				ret = IRQ_WAKE_THREAD;
>  				disable_irqs |= adev_irq_mask[i];

-- 
Regards,

Sakari Ailus