Re: [PATCH rc v6 3/7] iommu/arm-smmu-v3: Do not enable EVTQ/PRIQ interrupts in kdump kernel

[Pranjal Shrivastava <praan@google.com>]

On Mon, Jun 29, 2026 at 09:14:48PM -0700, Nicolin Chen wrote:
> On Mon, Jun 29, 2026 at 08:48:11AM +0000, Pranjal Shrivastava wrote:
> > On Wed, May 20, 2026 at 10:03:20AM -0700, Nicolin Chen wrote:
> > > @@ -5020,19 +5029,30 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
> > >  		/*
> > >  		 * Cavium ThunderX2 implementation doesn't support unique irq
> > >  		 * lines. Use a single irq line for all the SMMUv3 interrupts.
> > > +		 *
> > > +		 * In kdump, EVTQ/PRIQ are disabled, so no threaded handling.
> > >  		 */
> > > -		ret = devm_request_threaded_irq(smmu->dev, irq,
> > > -					arm_smmu_combined_irq_handler,
> > > -					arm_smmu_combined_irq_thread,
> > > -					IRQF_ONESHOT,
> > > -					"arm-smmu-v3-combined-irq", smmu);
> > > +		if (is_kdump_kernel())
> > > +			ret = devm_request_irq(smmu->dev, irq,
> > > +					       arm_smmu_combined_irq_handler, 0,
> > > +					       "arm-smmu-v3-combined-irq",
> > > +					       smmu);
> > 
> > This `if` isn't needed, we can continue using devm_request_threaded_irq,
> > if you look at the doc for devm_request_threaded_irq [1] it says:
> [...]
> > So, we can pass handler() here while leaving the thread_fn == NULL:
> > 
> > ret = devm_request_threaded_irq(smmu->dev, irq,
> >          arm_smmu_combined_irq_handler,
> >          is_kdump_kernel() ? NULL : arm_smmu_combined_irq_thread,
> >          IRQF_ONESHOT,
> >          "arm-smmu-v3-combined-irq", smmu);
> 
> Are you sure?
> 
> __setup_irq():
> 1497-   /*
> 1498:    * IRQF_ONESHOT means the interrupt source in the IRQ chip will be
> 1499-    * masked until the threaded handled is done. If there is no thread
> 1500:    * handler then it makes no sense to have IRQF_ONESHOT.
> 1501-    */
> 1502:   WARN_ON_ONCE(new->flags & IRQF_ONESHOT && !new->thread_fn);

I meant without IRQF_ONESHOT: 

is_kdump_kernel() ? 0 : IRQF_ONESHOT, note that devm_request_irq is just:

static inline int __must_check
devm_request_irq(struct device *dev, unsigned int irq, irq_handler_t handler,
		 unsigned long irqflags, const char *devname, void *dev_id)
{
	return devm_request_threaded_irq(dev, irq, handler, NULL, irqflags | IRQF_COND_ONESHOT,
					 devname, dev_id);
}

Not a strong opinion though, just suggesting a way to remove the if.

> 
> > Additionally, the arm_smmu_combined_irq_handler() returns 
> > IRQ_WAKE_THREAD unconditionally, which causes us to hit the warn_on[3] in
> > __handle_irq_event_percpu.
> 
> arm_smmu_combined_irq_handler() does not return IRQ_WAKE_THREAD
> unconditionally.
> 
> This is the first part of PATCH-3 in v6:

Ahh I missed that, somehow.

Thanks,
Praan