Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()

[Marc Zyngier <maz@kernel.org>]

On Mon, 09 May 2022 09:54:21 +0100,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Fri, May 06, 2022 at 10:32:42PM +0200, Lukas Wunner wrote:
> > On Tue, Oct 26, 2021 at 10:25:04AM +0100, Mark Rutland wrote:
> > > Now that entry code handles IRQ entry (including setting the IRQ regs)
> > > before calling irqchip code, irqchip code can safely call
> > > generic_handle_domain_irq(), and there's no functional reason for it to
> > > call handle_domain_irq().
> > > 
> > > Let's cement this split of responsibility and remove handle_domain_irq()
> > > entirely, updating irqchip drivers to call generic_handle_domain_irq().
> > > 
> > > For consistency, handle_domain_nmi() is similarly removed and replaced
> > > with a generic_handle_domain_nmi() function which also does not perform
> > > any entry logic.
> > > 
> > > Previously handle_domain_{irq,nmi}() had a WARN_ON() which would fire
> > > when they were called in an inappropriate context. So that we can
> > > identify similar issues going forward, similar WARN_ON_ONCE() logic is
> > > added to the generic_handle_*() functions, and comments are updated for
> > > clarity and consistency.
> > [...]
> > >  int generic_handle_domain_irq(struct irq_domain *domain, unsigned int hwirq)
> > >  {
> > > +	WARN_ON_ONCE(!in_irq());
> > >  	return handle_irq_desc(irq_resolve_mapping(domain, hwirq));
> > >  }
> > >  EXPORT_SYMBOL_GPL(generic_handle_domain_irq);
> > 
> > Why isn't the WARN_ON_ONCE() conditional on handle_enforce_irqctx()?
> > (See handle_irq_desc() and c16816acd086.)
> 
> I did this for consistency with the in_nmi() check in
> generic_handle_domain_nmi(); I was unaware of commit c16816acd086 and
> IRQD_HANDLE_ENFORCE_IRQCTX.
> 
> I'll have ot leave it to Marc and Thomas as to what we should do there.

My preference would be to not introduce things that result in
different behaviours for drivers, specially for things that are
evidently cross-architecture such as USB drivers (which seems to be
the case here).

I'd rather do something that allows these to be handled in the right
context such as a self-IPI. This would certainly work for the GIC. No
idea whether this is valid for x86, which is the other user.

Thanks,

	M.


-- 
Without deviation from the norm, progress is not possible.