[PATCH 10/18] arm64: Introduce FIQ support

[Arnd Bergmann]

On Sun, Feb 7, 2021 at 9:36 AM Hector Martin 'marcan' <marcan at 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