Re: [PATCH 10/18] arm64: Introduce FIQ support

From: Arnd Bergmann <arnd@kernel.org>
To: "Hector Martin 'marcan'" <marcan@marcan.st>
Cc: DTML <devicetree@vger.kernel.org>, Marc Zyngier <maz@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	SoC Team <soc@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Olof Johansson <olof@lixom.net>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 10/18] arm64: Introduce FIQ support
Date: Sun, 7 Feb 2021 13:25:03 +0100	[thread overview]
Message-ID: <CAK8P3a1R51_nqfMWG7SxScJNJEQ3qvp-cynABKEDaQ4O9REM=Q@mail.gmail.com> (raw)
In-Reply-To: <cb721f28-d5e9-3381-2d04-746c0aa2a0d3@marcan.st>

On Sun, Feb 7, 2021 at 9:36 AM Hector Martin 'marcan' <marcan@marcan.st> wrote:
> On 07/02/2021 01.22, Arnd Bergmann wrote:
> > * In the fiq handler code, check if normal interrupts were enabled
> >    when the fiq hit. Normally they are enabled, so just proceed to
> >    handle the timer and ipi directly
> >
> > * if irq was disabled, defer the handling by doing a self-ipi
> >    through the aic's ipi method, and handle it from there
> >    when dealing with the next interrupt once interrupts get
> >    enabled.
> >
> > This would be similar to the soft-disable feature on powerpc, which
> > never actually turns off interrupts from regular kernel code but
> > just checks a flag in local_irq_enable that gets set when a
> > hardirq happened.
>
> Case #2 seems messy. In AIC, we'd have to either:
>
> * Disable FIQs, and hope that doesn't confuse any save/restore code
> going on, then set a flag and check it in *both* the IRQ and FIQ path
> since either might trigger depending on what happens next, or
> * Mask the relevant timer, which we'd then need to make sure does not
> confuse the timer code (Unmask it again when we fire the interrupt? But
> what if the timer code intended to mask it in the interim?)

I'm not quite following here. The IRQ should be disabled the entire time
while handling that self-IPI and the timer top half code, so if we get
another FIQ while handling the timer from the IRQ path, it will lead
either yet another self-IPI or it will be ignored in case the previous timer
event has not been Acked yet. I would expect that both cases are
race-free here, the only time that the FIQ needs to be disabled is
while actually handling the FIQ. Did I miss something?

> Plus I don't know if the vector entry code and other scaffolding between
> the vector and the AIC driver would be happy with, effectively,
> recursive interrupts. This could work with a carefully controlled path
> to make sure it doesn't break things, but I'm not so sure about the
> current "just point FIQ and IRQ to the same place" approach here.

If we do what I described above, the FIQ and IRQ entry would have
to be separate and only arrive in the same code path when calling
into drivers/clocksource/arm_arch_timer.c. It's not recursive there
because that part is only called when IRQ is disabled, and no IRQ
is being executed while the FIQ hits.

       Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel