[PATCH v2 12/14] arm64/nmi: Add handling of superpriority interrupts as NMIs
Marc Zyngier
maz at kernel.org
Wed Dec 7 03:03:26 PST 2022
On Sat, 12 Nov 2022 15:17:06 +0000,
Mark Brown <broonie at kernel.org> wrote:
>
> Our goal with superpriority interrupts is to use them as NMIs, taking
> advantage of the much smaller regions where they are masked to allow
> prompt handling of the most time critical interrupts.
>
> When an interrupt configured with superpriority we will enter EL1 as
> normal for any interrupt, the presence of a superpriority interrupt is
> indicated with a status bit in ISR_EL1. We use this to check for the
> presence of a superpriority interrupt before we unmask anything in
> elX_interrupt(), reporting without unmasking any interrupts. If no
> superpriority interrupt is present then we handle normal interrupts as
> normal, superpriority interrupts will be unmasked while doing so as a
> result of setting DAIF_PROCCTX.
>
> Both IRQs and FIQs may be configured with superpriority so we handle
> both, passing an additional root handler into the elX_interrupt()
> function along with the mask for the bit in ISR_EL1 which indicates the
> presence of the relevant kind of superpriority interrupt. These root
> handlers can be configured by the interrupt controller similarly to the
> root handlers for normal interrupts using the newly added
> set_handle_nmi_irq() and set_handle_nmi_fiq() functions.
>
> Signed-off-by: Mark Brown <broonie at kernel.org>
> ---
> arch/arm64/include/asm/irq.h | 2 ++
> arch/arm64/kernel/entry-common.c | 55 +++++++++++++++++++++++++++-----
> arch/arm64/kernel/irq.c | 32 +++++++++++++++++++
> 3 files changed, 81 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
> index fac08e18bcd5..2ab05d899bf6 100644
> --- a/arch/arm64/include/asm/irq.h
> +++ b/arch/arm64/include/asm/irq.h
> @@ -8,6 +8,8 @@
>
> struct pt_regs;
>
> +int set_handle_nmi_irq(void (*handle_irq)(struct pt_regs *));
> +int set_handle_nmi_fiq(void (*handle_fiq)(struct pt_regs *));
I'm not overly keen on adding hooks that are not used, and I can't
really foresee a use case for a FIQ NMI at the moment (there is no
plan to use Group-0 interrupts in VMs when the GIC is enabled, and the
only interrupt controller we have that uses FIQ doesn't even have
priorities, let alone NMIs).
> int set_handle_irq(void (*handle_irq)(struct pt_regs *));
> #define set_handle_irq set_handle_irq
> int set_handle_fiq(void (*handle_fiq)(struct pt_regs *));
> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> index 9173fad279af..eb6fc718737e 100644
> --- a/arch/arm64/kernel/entry-common.c
> +++ b/arch/arm64/kernel/entry-common.c
> @@ -278,6 +278,8 @@ static void do_interrupt_handler(struct pt_regs *regs,
> set_irq_regs(old_regs);
> }
>
> +extern void (*handle_arch_nmi_irq)(struct pt_regs *);
> +extern void (*handle_arch_nmi_fiq)(struct pt_regs *);
> extern void (*handle_arch_irq)(struct pt_regs *);
> extern void (*handle_arch_fiq)(struct pt_regs *);
>
> @@ -453,6 +455,14 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
> }
> }
>
> +static __always_inline void __el1_nmi(struct pt_regs *regs,
> + void (*handler)(struct pt_regs *))
> +{
> + arm64_enter_nmi(regs);
> + do_interrupt_handler(regs, handler);
> + arm64_exit_nmi(regs);
> +}
> +
> static __always_inline void __el1_pnmi(struct pt_regs *regs,
> void (*handler)(struct pt_regs *))
> {
> @@ -474,9 +484,19 @@ static __always_inline void __el1_irq(struct pt_regs *regs,
>
> exit_to_kernel_mode(regs);
> }
> -static void noinstr el1_interrupt(struct pt_regs *regs,
> - void (*handler)(struct pt_regs *))
> +
> +static void noinstr el1_interrupt(struct pt_regs *regs, u64 nmi_flag,
> + void (*handler)(struct pt_regs *),
> + void (*nmi_handler)(struct pt_regs *))
> {
> + if (system_uses_nmi()) {
> + /* Is there a NMI to handle? */
> + if (read_sysreg(isr_el1) & nmi_flag) {
Better written as:
if (system_uses_nmi() && (read_sysreg(isr_el1) & nmi_flag)) {
> + __el1_nmi(regs, nmi_handler);
> + return;
> + }
> + }
> +
> write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
>
> if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
> @@ -487,12 +507,12 @@ static void noinstr el1_interrupt(struct pt_regs *regs,
>
> asmlinkage void noinstr el1h_64_irq_handler(struct pt_regs *regs)
> {
> - el1_interrupt(regs, handle_arch_irq);
> + el1_interrupt(regs, ISR_EL1_IS, handle_arch_irq, handle_arch_nmi_irq);
> }
>
> asmlinkage void noinstr el1h_64_fiq_handler(struct pt_regs *regs)
> {
> - el1_interrupt(regs, handle_arch_fiq);
> + el1_interrupt(regs, ISR_EL1_FS, handle_arch_fiq, handle_arch_nmi_fiq);
> }
>
> asmlinkage void noinstr el1h_64_error_handler(struct pt_regs *regs)
> @@ -701,11 +721,30 @@ asmlinkage void noinstr el0t_64_sync_handler(struct pt_regs *regs)
> }
> }
>
> -static void noinstr el0_interrupt(struct pt_regs *regs,
> - void (*handler)(struct pt_regs *))
> +static void noinstr el0_interrupt(struct pt_regs *regs, u64 nmi_flag,
> + void (*handler)(struct pt_regs *),
> + void (*nmi_handler)(struct pt_regs *))
> {
> enter_from_user_mode(regs);
>
> + if (system_uses_nmi()) {
> + /* Is there a NMI to handle? */
> + if (read_sysreg(isr_el1) & nmi_flag) {
Same thing.
> + /*
> + * Any system with FEAT_NMI should not be
> + * affected by Spectre v2 so we don't mitigate
> + * here.
> + */
Why? I don't see a good reason not to mitigate it, specially when the
mitigation is guarded by cpus_have_const_cap(ARM64_SPECTRE_V2). Maybe
you can explain what the rationale is for this.
> +
> + arm64_enter_nmi(regs);
> + do_interrupt_handler(regs, nmi_handler);
> + arm64_exit_nmi(regs);
> +
> + exit_to_user_mode(regs);
> + return;
> + }
> + }
> +
> write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
>
> if (regs->pc & BIT(55))
> @@ -720,7 +759,7 @@ static void noinstr el0_interrupt(struct pt_regs *regs,
>
> static void noinstr __el0_irq_handler_common(struct pt_regs *regs)
> {
> - el0_interrupt(regs, handle_arch_irq);
> + el0_interrupt(regs, ISR_EL1_IS, handle_arch_irq, handle_arch_nmi_irq);
> }
>
> asmlinkage void noinstr el0t_64_irq_handler(struct pt_regs *regs)
> @@ -730,7 +769,7 @@ asmlinkage void noinstr el0t_64_irq_handler(struct pt_regs *regs)
>
> static void noinstr __el0_fiq_handler_common(struct pt_regs *regs)
> {
> - el0_interrupt(regs, handle_arch_fiq);
> + el0_interrupt(regs, ISR_EL1_FS, handle_arch_fiq, handle_arch_nmi_fiq);
> }
>
> asmlinkage void noinstr el0t_64_fiq_handler(struct pt_regs *regs)
> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> index 38dbd3828f13..77a1ea90b244 100644
> --- a/arch/arm64/kernel/irq.c
> +++ b/arch/arm64/kernel/irq.c
> @@ -85,6 +85,16 @@ void do_softirq_own_stack(void)
> }
> #endif
>
> +static void default_handle_nmi_irq(struct pt_regs *regs)
> +{
> + panic("Superpriority IRQ taken without a root NMI IRQ handler\n");
> +}
> +
> +static void default_handle_nmi_fiq(struct pt_regs *regs)
> +{
> + panic("Superpriority FIQ taken without a root NMI FIQ handler\n");
> +}
> +
> static void default_handle_irq(struct pt_regs *regs)
> {
> panic("IRQ taken without a root IRQ handler\n");
> @@ -95,9 +105,31 @@ static void default_handle_fiq(struct pt_regs *regs)
> panic("FIQ taken without a root FIQ handler\n");
> }
>
> +void (*handle_arch_nmi_irq)(struct pt_regs *) __ro_after_init = default_handle_nmi_irq;
> +void (*handle_arch_nmi_fiq)(struct pt_regs *) __ro_after_init = default_handle_nmi_fiq;
> void (*handle_arch_irq)(struct pt_regs *) __ro_after_init = default_handle_irq;
> void (*handle_arch_fiq)(struct pt_regs *) __ro_after_init = default_handle_fiq;
>
> +int __init set_handle_nmi_irq(void (*handle_nmi_irq)(struct pt_regs *))
> +{
> + if (handle_arch_nmi_irq != default_handle_nmi_irq)
> + return -EBUSY;
> +
> + handle_arch_nmi_irq = handle_nmi_irq;
> + pr_info("Root superpriority IRQ handler: %ps\n", handle_nmi_irq);
> + return 0;
> +}
> +
> +int __init set_handle_nmi_fiq(void (*handle_nmi_fiq)(struct pt_regs *))
> +{
> + if (handle_arch_nmi_fiq != default_handle_nmi_fiq)
> + return -EBUSY;
> +
> + handle_arch_nmi_fiq = handle_nmi_fiq;
> + pr_info("Root superpriority FIQ handler: %ps\n", handle_nmi_fiq);
> + return 0;
> +}
> +
> int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
> {
> if (handle_arch_irq != default_handle_irq)
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
More information about the linux-arm-kernel
mailing list