From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4DC20C352A1 for ; Wed, 7 Dec 2022 11:04:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=fivoKQg5vTzXlAxSGNMBCg+lj9qsRwAtwr+DUAW2w9I=; b=TpE+rE6Y4tuy7u pB2VkFG+N1hnZJYihmOqjCrw2CUnK1Fx3Ft/LVxdhpcmhuKpwk8n1mq9lbRAZBpLO94Wu0LtGzNjR cYDW+Tp5A0FqloGEdYLevg8PNIlRKYpohWh6w2bM2nnqXaOylZAveUTGSD2YnzLrH9DSxpgbZ5wS5 pBjMjj6zXdjAzFMDDMcvE++9TIHefp5R9rhXlJxYh16nAdXR1B68oHHLq5kGuiM8tdkP7k7j4yT9O qFDsHK4trFtiXM+h3xn4kxKBwixCrZkLxvg5g6LKUYPV40IBh5TtwZ8U0inZf1MtgM1AUGBmTK1yp SNT6rfy+huoXK2y1AUyw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p2sDB-000Nxf-Ne; Wed, 07 Dec 2022 11:03:37 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1p2sD6-000Nrh-7J for linux-arm-kernel@lists.infradead.org; Wed, 07 Dec 2022 11:03:34 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id CA454B81BB7; Wed, 7 Dec 2022 11:03:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 68814C433D6; Wed, 7 Dec 2022 11:03:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1670411009; bh=xmexmDtxA4l+jjA3C9dNMASb9eHFC+CM3rUn77Z0q30=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=mGuPrrBNV0iwRGGniJJMkIHmp5Fl3718hsctWxLA/0htEMlDa+L2EM3G/OT1FIKqS CcHHm++NDBhPsX0Am0azZHYDcK7SsYnLsVmMwkRVQsW1XZ24qqnGkG4y+7MX93Hpp3 RYWTN3oattzZnWvVPAvbuIRlHhduGyMOTjgjgmcY1cT6PFXfzrtYUNQihOQS+ec4x9 TfpVpFtQsUT1WYvwUjkeNC0e7f+2KgZPl6xl32O7Q2wIrh3AQCFU6iYYCtnSBsHH60 9rUUoJFnuarsNZzCCSbqxBhXaRhBwUBWOUU9BMwu3BRcMaO1QqMc1BkjMy3RNKS2Lk 7J1ogEV0iF22A== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1p2sD1-00B4Rc-3J; Wed, 07 Dec 2022 11:03:27 +0000 Date: Wed, 07 Dec 2022 11:03:26 +0000 Message-ID: <86k033lblt.wl-maz@kernel.org> From: Marc Zyngier To: Mark Brown Cc: Catalin Marinas , Will Deacon , Lorenzo Pieralisi , Mark Rutland , Sami Mujawar , Thomas Gleixner , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev Subject: Re: [PATCH v2 12/14] arm64/nmi: Add handling of superpriority interrupts as NMIs In-Reply-To: <20221112151708.175147-13-broonie@kernel.org> References: <20221112151708.175147-1-broonie@kernel.org> <20221112151708.175147-13-broonie@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: broonie@kernel.org, catalin.marinas@arm.com, will@kernel.org, lpieralisi@kernel.org, mark.rutland@arm.com, Sami.Mujawar@arm.com, tglx@linutronix.de, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221207_030332_619599_E33EE0A8 X-CRM114-Status: GOOD ( 38.88 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sat, 12 Nov 2022 15:17:06 +0000, Mark Brown 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 > --- > 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. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel