Re: [PATCH v2 08/25] arm64: Always keep DAIF.[IF] in sync

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux