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

From: Thomas Gleixner <tglx@linutronix.de>
To: Marc Zyngier <maz@kernel.org>, Mark Rutland <mark.rutland@arm.com>
Cc: Lukas Wunner <lukas@wunner.de>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <brgl@bgdev.pl>,
	linux-gpio@vger.kernel.org,
	Octavian Purdila <octavian.purdila@nxp.com>,
	linux-kernel@vger.kernel.org, aou@eecs.berkeley.edu,
	catalin.marinas@arm.com, deanbo422@gmail.com, green.hu@gmail.com,
	guoren@kernel.org, jonas@southpole.se, kernelfans@gmail.com,
	linux-arm-kernel@lists.infradead.org, linux@armlinux.org.uk,
	nickhu@andestech.com, palmer@dabbelt.com,
	paul.walmsley@sifive.com, shorne@gmail.com,
	stefan.kristiansson@saunalahti.fi, tsbogend@alpha.franken.de,
	vgupta@kernel.org, vladimir.murzin@arm.com, will@kernel.org
Subject: Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
Date: Wed, 11 May 2022 01:36:15 +0200	[thread overview]
Message-ID: <877d6tot8w.ffs@tglx> (raw)
In-Reply-To: <87h75z6pix.wl-maz@kernel.org>

On Mon, May 09 2022 at 10:09, Marc Zyngier wrote:
> On Mon, 09 May 2022 09:54:21 +0100,
> Mark Rutland <mark.rutland@arm.com> wrote:
>> > 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).

No. USB drivers which synthesize their interrupts from a received packet
work perfectly fine on all architectures because the interrupt domain
and interrupt chip they are using are a software construct designed for
the purpose and have no hard interrupt context requirements.

The reason why this is done is to leverage the interrupt driven PHY
status changes instead of enforcing timer based polling. The charm is
that the phy code does not have to grow another wart and just uses the
offered synthetic interrupt. There are other places which do similar
things for the same reason. This also provides the beloved statistics in
/proc/interrupt, tracepoints etc. out of the box without having to add
extra muck into yet another subsystem.

> 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.

This interrupt is neither directly nor indirectly connected to GIC
or APIC. It's synthesized. So what would the self-IPI help?

And no, I don't want to create infrastructure to allocate a pseudo
device vector on x86 just to be able to self-IPI this USB synthesized
interrupt. That'd be yet another horrorshow and worse a horrorshow for
no reason and zero value.

IRQD_HANDLE_ENFORCE_IRQCTX was introduced to be able to differentiate
between interrupt chips which require a particular context and chips
which can handle it perfectly fine to have e.g. their software retrigger
handled by directly invoking the handler from an arbitrary context.

I wish this would be the case on x86. That would eliminate a boatload of
horrible code.

Thanks,

        tglx