On 17/02/2021 21.22, Mark Rutland wrote:
Root irqchip drivers can discriminate between IRQs and FIQs by checking
the ISR_EL1 system register.
I think we can remove this note for now. If we go with seperate handlers
this won't be necessary, and if not this would be better placed on a
commit adding the FIQ handling capability.
Indeed, this doesn't make sense any more. Changed for v3.
Judging by `git grep -Wi daif -- arch/arm64`, with this patch applied,
we'll also need fixups in:
* gic_arch_enable_irqs() in arch/arm64/include/asm/arch_gicv3.h
* save_and_disable_irq() in arch/arm64/include/asm/assembler.h (noted below)
* local_daif_save_flags() in arch/arm64/include/asm/daifflags.h
(the fake DAIF should have F set too)
* __cpu_do_idle_irqprio() in arch/arm64/kernel/process.c
Good catches. A few of those are irrelevant for M1 but need to be done
now that we're making this change globally, others I just missed from
the beginning.
There's also an incorrect comment in entry.S:
/*
* DA_F were cleared at start of handling. If anything is set in
* DAIF, we come back from an NMI, so skip preemption
*/
mrs x0, daif
orr x24, x24, x0
Now only DA__ are cleared. This actually pairs with
gic_arch_enable_irqs() and begs the question: in priority masking
systems, do we unmask both IRQ and FIQ (the gic_arch_enable_irqs
change), or do we leave FIQ masked (which instead would need an AND in
that part of entry.S so as to not consider FIQ masked as meaning we're
coming back from an NMI)?
And a minor related one: should init_gic_priority_masking() WARN if FIQ
is masked too? This probably goes with the above.
Either way, this was nontrivial to make sense of, so I'll make that
entry.S comment clearer while I'm touching it.
I think save_and_diable_irq below needs to be updated too, since it
only sets DAIF.I and leaves DAIF.F as-is.
Totally missed this one! Fixed for v3.
- * FIQ is never expected, but we mask it when we disable debug exceptions, and
- * unmask it at all other times.
+ * FIQ is never expected on most platforms, but we keep it synchronized
+ * with the IRQ mask status. On platforms that do not expect FIQ, that vector
+ * triggers a kernel panic. On platforms that do, the FIQ vector is unified
+ * with the IRQ vector.
*/
Can we please delete this bit, though? Now that we say IRQ and FIQ are
masked/unmasked together, I don't think the rest is necessary to
understand the masking logic, and it's one less thing to keep in sync
with changes to the entry code.
Gone :)
--
Hector Martin (marcan@xxxxxxxxx)
Public Key: https://mrcn.st/pub
