Re: [PATCH v2 09/25] arm64: entry: Map the FIQ vector to IRQ on NEEDS_FIQ platforms

Mark Rutland
2021-02-17T11:52:58Z

Hi Hector,

On Mon, Feb 15, 2021 at 09:16:57PM +0900, Hector Martin wrote:
> From: Marc Zyngier <maz@kernel.org>
>
> By default, FIQ exceptions trigger a panic. On platforms that need to
> deliver interrupts via FIQ, this gets redirected via an alternative to
> instead handle FIQ the same way as IRQ. It is up to the irqchip handler
> to discriminate between the two.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Hector Martin <marcan@marcan.st>

Since the use of FIQ is a platform integration detail rather than a CPU
implementation detail (and e.g. can differ across bare-metal and VM),
I'd prefer to always have separate registered handlers for IRQ/FIQ (also
avoiding the need for patching). That way we can explicitly opt-in to
FIQ when required, and avoid edge-cases where an unexpected FIQ could
livelock an unaware IRQ handler.

Marc and I had a quick play with that, and I have a series of patches
I've pushed to:

https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/fiq
git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arm64/fiq

... which I'll post out shortly.

Thanks,
Mark.

> ---
> arch/arm64/kernel/entry.S | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index ba5f9aa379ce..bcfd1ac72636 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_compat, 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_compat
> +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
> --
> 2.30.0