Re: [PATCH v9 01/12] iommu/arm-smmu-v3: Refactor arm_smmu_setup_irqs

[Pranjal Shrivastava <praan@google.com>]

On Tue, Aug 25, 2026 at 11:47:57AM -0700, Nicolin Chen wrote:
> On Tue, Aug 25, 2026 at 05:35:28PM +0000, Pranjal Shrivastava wrote:
> > On Tue, Aug 25, 2026 at 01:36:11PM -0300, Jason Gunthorpe wrote:
> > > > [ ... 84 lines skipped ... ]
> > > > +	/* Setup interrupt handlers */
> > > > +	ret = arm_smmu_setup_irqs(smmu);
> > > > +	if (ret) {
> > > > +		dev_err(smmu->dev, "failed to setup irqs\n");
> > > > +		return ret;
> > > > +	}
> > > > +
> > > 
> > > The sashiko remark makes sense to me, we should not enable interrupts
> > > until everything has been initialized and things are ready to go. So
> > > this is better done after the device reset call, which is the thing
> > > that is setting up the queue pointers in HW..
> > > 
> > > I'm not sure what to make of the MSI register ordering around kdump
> > > comment.. Nicolin?
> > > 
> > 
> > Ack. I agree with Sashiko's remarks, we'd need to call this within
> > device_reset right before the SMMU is enabled (same place as today).
> > I factored this out to avoid setting up irq handlers multiple times
> > across a suspend / resume cycle (for e.g. we can't request_irq multiple
> > times for the same irq). Otherwise, we'd have to de-register IRQ
> > handlers in the suspend callback. Maybe we could do this ONCE based on 
> > some smmu->inited bool?
> 
> Yea, setting MSI while IRQ and CMDQ are enabled might be an issue.
> 
> FWIW, my kdump series disables EVTQ and PRIQ, so the concern would
> only apply to GERROR.
> 

Hmm, I guess I'll move it into the reset handler with some smmu->inited?
The other thing could be somehow masking all interrupts till we *really* 
enable the SMMU. 

Which one sounds better to you? Any preference? 
(Will / Jason / Mostafa / Daniel?)

Thanks,
Praan