Re: [PATCH 10/18] arm64: Introduce FIQ support
From: Marc Zyngier <maz@kernel.org>
Date: 2021-02-06 15:39:50
Subsystem:
arm64 port (aarch64 architecture), the rest · Maintainers:
Catalin Marinas, Will Deacon, Linus Torvalds
On Thu, 04 Feb 2021 20:39:43 +0000, Hector Martin [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Apple SoCs (A11 and newer) have some interrupt sources hardwired to the FIQ line. Implement support for this by simply treating IRQs and FIQs the same way in the interrupt vectors. This is conditional on the ARM64_NEEDS_FIQ CPU feature flag, and thus will not affect other systems. Root irqchip drivers can discriminate between IRQs and FIQs by checking the ISR_EL1 system register. Signed-off-by: Hector Martin <redacted> --- arch/arm64/include/asm/assembler.h | 4 ++++ arch/arm64/include/asm/daifflags.h | 7 +++++++ arch/arm64/include/asm/irqflags.h | 17 +++++++++++++---- arch/arm64/kernel/entry.S | 27 +++++++++++++++++++++++---- 4 files changed, 47 insertions(+), 8 deletions(-)diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index bf125c591116..6acfc372dc76 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h@@ -42,7 +42,11 @@ /* IRQ is the lowest priority flag, unconditionally unmask the rest. */ .macro enable_da_f +alternative_if ARM64_NEEDS_FIQ + msr daifclr, #(8 | 4) +alternative_else msr daifclr, #(8 | 4 | 1) +alternative_endif
See my digression in patch 8. I really wonder what the benefit is to treat FIQ independently of IRQ, and we might as well generalise this. We could always panic on getting a FIQ on platforms that don't expect one. It'd be good to rope in the other interested parties (Mark for the early entry code, James for RAS and SError handling).
quoted hunk ↗ jump to hunk
.endm /*diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h index 1c26d7baa67f..228a6039c701 100644 --- a/arch/arm64/include/asm/daifflags.h +++ b/arch/arm64/include/asm/daifflags.h@@ -112,6 +112,13 @@ static inline void local_daif_restore(unsigned long flags) * So we don't need additional synchronization here. */ gic_write_pmr(pmr); + } else if (system_uses_fiqs()) { + /* + * On systems that use FIQs, disable FIQs if IRQs are disabled. + * This can happen if the DAIF_* flags at the top of this file + * are used to set DAIF directly. + */ + flags |= PSR_F_BIT; } write_sysreg(flags, daif);diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h index ff328e5bbb75..689c573c4b47 100644 --- a/arch/arm64/include/asm/irqflags.h +++ b/arch/arm64/include/asm/irqflags.h@@ -19,8 +19,9 @@ * side effects for other flags. Keeping to this order makes it easier for * entry.S to know which exceptions should be unmasked. * - * 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 mask it when we disable + * debug exceptions, and unmask it at all other times. On platforms that + * require FIQs, it tracks IRQ masking. */ /*@@ -34,8 +35,14 @@ static inline void arch_local_irq_enable(void) WARN_ON_ONCE(pmr != GIC_PRIO_IRQON && pmr != GIC_PRIO_IRQOFF); } - asm volatile(ALTERNATIVE( + /* + * Yes, ALTERNATIVE() nests properly... only one of these should be + * active on any given platform. + */ + asm volatile(ALTERNATIVE(ALTERNATIVE( "msr daifclr, #2 // arch_local_irq_enable", + "msr daifclr, #3 // arch_local_irq_enable", + ARM64_NEEDS_FIQ),
Err... no. Please. It may be a cool hack, but that's an unmaintainable one in the long run. If you *really* have to have a special case here, consider using a callback instead, and generate the right instruction directly.
quoted hunk ↗ jump to hunk
__msr_s(SYS_ICC_PMR_EL1, "%0"), ARM64_HAS_IRQ_PRIO_MASKING) :@@ -53,8 +60,10 @@ static inline void arch_local_irq_disable(void) WARN_ON_ONCE(pmr != GIC_PRIO_IRQON && pmr != GIC_PRIO_IRQOFF); } - asm volatile(ALTERNATIVE( + asm volatile(ALTERNATIVE(ALTERNATIVE( "msr daifset, #2 // arch_local_irq_disable", + "msr daifset, #3 // arch_local_irq_disable", + ARM64_NEEDS_FIQ), __msr_s(SYS_ICC_PMR_EL1, "%0"), ARM64_HAS_IRQ_PRIO_MASKING) :diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S index c9bae73f2621..81ca04ebe37b 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S@@ -60,7 +60,7 @@ #define BAD_FIQ 2 #define BAD_ERROR 3 - .macro kernel_ventry, el, label, regsize = 64 + .macro kernel_ventry, el, label, regsize = 64, altlabel = 0, alt = 0 .align 7 #ifdef CONFIG_UNMAP_KERNEL_AT_EL0 .if \el == 0@@ -87,7 +87,15 @@ alternative_else_nop_endif tbnz x0, #THREAD_SHIFT, 0f sub x0, sp, x0 // x0'' = sp' - x0' = (sp + x0) - sp = x0 sub sp, sp, x0 // sp'' = sp' - x0 = (sp + x0) - x0 = sp + .if \altlabel != 0 + alternative_if \alt + b el\()\el\()_\altlabel + alternative_else b el\()\el\()_\label + alternative_endif + .else + b el\()\el\()_\label + .endif 0: /*@@ -119,7 +127,15 @@ alternative_else_nop_endif sub sp, sp, x0 mrs x0, tpidrro_el0 #endif + .if \altlabel != 0 + alternative_if \alt + b el\()\el\()_\altlabel + alternative_else b el\()\el\()_\label + alternative_endif + .else + b el\()\el\()_\label + .endif .endm .macro tramp_alias, dst, sym@@ -547,18 +563,21 @@ SYM_CODE_START(vectors) kernel_ventry 1, sync // Synchronous EL1h kernel_ventry 1, irq // IRQ EL1h - kernel_ventry 1, fiq_invalid // FIQ EL1h + // FIQ EL1h + kernel_ventry 1, fiq_invalid, 64, irq, ARM64_NEEDS_FIQ
It could be better to create a set of first class FIQ handlers rather than this alternative target macro. I quickly hacked this instead, which I find more readable. Thanks, M.
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index a8c3e7aaca74..dc65b56626ab 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S@@ -547,18 +547,18 @@ SYM_CODE_START(vectors) kernel_ventry 1, sync // Synchronous EL1h kernel_ventry 1, irq // IRQ EL1h - kernel_ventry 1, fiq_invalid // FIQ EL1h + kernel_ventry 1, fiq // FIQ EL1h kernel_ventry 1, error // Error EL1h kernel_ventry 0, sync // Synchronous 64-bit EL0 kernel_ventry 0, irq // IRQ 64-bit EL0 - kernel_ventry 0, fiq_invalid // FIQ 64-bit EL0 + kernel_ventry 0, fiq // FIQ 64-bit EL0 kernel_ventry 0, error // Error 64-bit EL0 #ifdef CONFIG_COMPAT kernel_ventry 0, sync_compat, 32 // Synchronous 32-bit EL0 kernel_ventry 0, irq_compat, 32 // IRQ 32-bit EL0 - kernel_ventry 0, fiq_invalid_compat, 32 // FIQ 32-bit EL0 + kernel_ventry 0, fiq, 32 // FIQ 32-bit EL0 kernel_ventry 0, error_compat, 32 // Error 32-bit EL0 #else kernel_ventry 0, sync_invalid, 32 // Synchronous 32-bit EL0
@@ -658,6 +658,10 @@ SYM_CODE_START_LOCAL_NOALIGN(el1_sync) SYM_CODE_END(el1_sync) .align 6 +SYM_CODE_START_LOCAL_NOALIGN(el1_fiq) +alternative_if_not ARM64_NEEDS_FIQ + b el1_fiq_invalid +alternative_else_nop_endif SYM_CODE_START_LOCAL_NOALIGN(el1_irq) kernel_entry 1 gic_prio_irq_setup pmr=x20, tmp=x1
@@ -688,6 +692,7 @@ alternative_else_nop_endif kernel_exit 1 SYM_CODE_END(el1_irq) +SYM_CODE_END(el1_fiq) /* * EL0 mode handlers.
@@ -710,10 +715,15 @@ SYM_CODE_START_LOCAL_NOALIGN(el0_sync_compat) SYM_CODE_END(el0_sync_compat) .align 6 +SYM_CODE_START_LOCAL_NOALIGN(el0_fiq_compat) +alternative_if_not ARM64_NEEDS_FIQ + b el0_fiq_invalid +alternative_else_nop_endif SYM_CODE_START_LOCAL_NOALIGN(el0_irq_compat) kernel_entry 0, 32 b el0_irq_naked SYM_CODE_END(el0_irq_compat) +SYM_CODE_END(el0_fiq_compat) SYM_CODE_START_LOCAL_NOALIGN(el0_error_compat) kernel_entry 0, 32
@@ -722,6 +732,10 @@ SYM_CODE_END(el0_error_compat) #endif .align 6 +SYM_CODE_START_LOCAL_NOALIGN(el0_fiq) +alternative_if_not ARM64_NEEDS_FIQ + b el0_fiq_invalid +alternative_else_nop_endif SYM_CODE_START_LOCAL_NOALIGN(el0_irq) kernel_entry 0 el0_irq_naked:
@@ -736,6 +750,7 @@ el0_irq_naked: b ret_to_user SYM_CODE_END(el0_irq) +SYM_CODE_END(el0_fiq) SYM_CODE_START_LOCAL(el1_error) kernel_entry 1
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel