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 2A936C4332F for ; Mon, 5 Dec 2022 18:49:00 +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=3mUHxw7/40oulFHxCCd6CKMuvc84TF4J+NitSGlimg4=; b=mcmyDNlBok4L5j yTS7XRRzAmMWR7D6qPANd4EKaCpfR0aQVa/4C2n+eIdowCNCax1mMK+jo0qlh4hxz+FunMriFdoLr DIJmi5TTKwBbqb2o+XnmHS44wJo/3mKi0wwFk9l+8zplWr63vtouiAWiFy2dJli8lBVo9lFqeL/cI 7ZlHgBK5Fp+5sxpaZeJmL2gfK7YISA7Q+R2Ft/pXJjtFUa6yqAaBEaHSdMx/k3dLzDez5Se1qNBN4 17pntGvZvf4MyhUn3cdK5X4458KvbPi8cJVqHkHjjX3kW71sP8V9G3U/jrkXtcRO1rv+kaxZe/VnK 59h8esMiQsTRSIf6KN4g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p2GVN-008gdl-BX; Mon, 05 Dec 2022 18:47:53 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1p2GVJ-008gZC-LA for linux-arm-kernel@lists.infradead.org; Mon, 05 Dec 2022 18:47:51 +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 dfw.source.kernel.org (Postfix) with ESMTPS id F3D6C61228; Mon, 5 Dec 2022 18:47:48 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 58DAFC433C1; Mon, 5 Dec 2022 18:47:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1670266068; bh=XTZvOj8vo6tIv62zmeZrDtWlI9pNkDLDxVRzLsHegI8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=sB4FJnvnm6dxImhflMqKvqXninog1z3agib6mgeQGixwmluR4g5kKplCcsYwzdQNA EfL6Ega5sc42XslfHpl8NWr0fXTermm1Zc3i3M44W6u/f+U4/vpFgr4E5K4FaZ+9p3 2s7d/LxP/R2i6DsQ3DbQ6OqHjH+38/k8fhQaPQ0QMwMPgFBXAOiI/+nEwlYJoY1f5c hVUAym9z1B+1SHfdn1hMw52hP9yyIem1BhsvIFHe1pVlZBuTp5WU/FpkZSCjwZFES1 UlN725mCS6jz7IsNHverOxpHENPaGp7a7he0K3O3LFIpOffwY4gUmLa425rCQzz5vY O/YvSxrX9PC+g== 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 1p2GVG-00AeRe-4d; Mon, 05 Dec 2022 18:47:46 +0000 Date: Mon, 05 Dec 2022 18:47:45 +0000 Message-ID: <86o7shlmb2.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 10/14] arm64/nmi: Manage masking for superpriority interrupts along with DAIF In-Reply-To: <20221112151708.175147-11-broonie@kernel.org> References: <20221112151708.175147-1-broonie@kernel.org> <20221112151708.175147-11-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-20221205_104749_791279_2E3FE5D4 X-CRM114-Status: GOOD ( 38.49 ) 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:04 +0000, Mark Brown wrote: > > As we do for pseudo NMIs add code to our DAIF management which keeps , > superpriority interrupts unmasked when we have asynchronous exceptions > enabled. Since superpriority interrupts are not masked through DAIF like > pseduo NMIs are we also need to modify the assembler macros for managing , > DAIF to ensure that the masking is done in the assembly code. At present > users of the assembly macros always mask pseudo NMIs. In patch #5, you say: "This is not integrated into the existing DAIF macros since we do not always wish to manage ALLINT along with DAIF and the use of DAIF in the naming of the existing macros might lead to surprises if ALLINT is also managed." It isn't integrated, and yet it is. > > There is a difference to the actual handling between pseudo NMIs > and superpriority interrupts in the assembly save_and_disable_irq and > restore_irq macros, these cover both interrupts and FIQs using DAIF s/,/;/ > without regard for the use of pseudo NMIs so also mask those but are not , > updated here to mask superpriority interrupts. Given the names it is not > clear that the behaviour with pseudo NMIs is particularly intentional, Pseudo-NMIs are still compatible with the standard DAIF behaviour, where setting PSTATE.I is strictly equivalent to setting PSTATE.ALLINT when you have architected NMIs. So I don't really understand your concern here. > and in any case these macros are only used in the implementation of > alternatives for software PAN while hardware PAN has been mandatory > since v8.1 so it is not anticipated that practical systems with support , > for FEAT_NMI will ever execute the affected code. > > This should be a conservative set of masked regions, we may be able to > relax this in future, but this should represent a good starting point. > > Signed-off-by: Mark Brown > --- > arch/arm64/include/asm/assembler.h | 11 +++++++++++ > arch/arm64/include/asm/daifflags.h | 18 ++++++++++++++++++ > 2 files changed, 29 insertions(+) > > diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h > index 88d9779a83c0..e85a7e9af9ae 100644 > --- a/arch/arm64/include/asm/assembler.h > +++ b/arch/arm64/include/asm/assembler.h > @@ -52,19 +52,30 @@ alternative_else_nop_endif > > .macro save_and_disable_daif, flags > mrs \flags, daif > + disable_allint > msr daifset, #0xf > .endm > > .macro disable_daif > + disable_allint > msr daifset, #0xf > .endm > > .macro enable_daif > msr daifclr, #0xf > + enable_allint > .endm > > .macro restore_daif, flags:req > msr daif, \flags > +#ifdef CONFIG_ARM64_NMI > +alternative_if ARM64_HAS_NMI > + /* If async exceptions are unmasked we can take NMIs */ > + tbnz \flags, #8, 2004f > + msr_s SYS_ALLINT_CLR, xzr > +2004: Please use the usual blah\@ hack for these macros, as you have no control over the context in which they will expand. > +alternative_else_nop_endif > +#endif > .endm > > /* IRQ/FIQ are the lowest priority flags, unconditionally unmask the rest. */ > diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h > index b3bed2004342..fda73976068f 100644 > --- a/arch/arm64/include/asm/daifflags.h > +++ b/arch/arm64/include/asm/daifflags.h > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > #include > > #define DAIF_PROCCTX 0 > @@ -35,6 +36,9 @@ static inline void local_daif_mask(void) > if (system_uses_irq_prio_masking()) > gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET); > > + if (system_uses_nmi()) > + _allint_set(); > + > trace_hardirqs_off(); > } > > @@ -50,6 +54,12 @@ static inline unsigned long local_daif_save_flags(void) > flags |= PSR_I_BIT | PSR_F_BIT; > } > > + if (system_uses_nmi()) { > + /* If IRQs are masked with ALLINT, reflect in in the flags */ > + if (read_sysreg_s(SYS_ALLINT) & ALLINT_ALLINT) > + flags |= PSR_I_BIT | PSR_F_BIT; > + } > + > return flags; > } > > @@ -114,6 +124,10 @@ static inline void local_daif_restore(unsigned long flags) > gic_write_pmr(pmr); > } > > + /* If we can take asynchronous errors we can take NMIs */ > + if (system_uses_nmi() && !(flags & PSR_A_BIT)) > + _allint_clear(); > + > write_sysreg(flags, daif); This sequence feels odd. With pseudo-NMI, we only allow the NMI to fire *after* the write to DAIF. With architected NMI, a NMI can fire *before*. I think that for the time being, we should follow a similar ordering. > > if (irq_disabled) > @@ -131,6 +145,10 @@ static inline void local_daif_inherit(struct pt_regs *regs) > if (interrupts_enabled(regs)) > trace_hardirqs_on(); > > + /* If we can take asynchronous errors we can take NMIs */ > + if (system_uses_nmi() && !(flags & PSR_A_BIT)) > + _allint_clear(); > + Same remark about the ordering. Also, we don't check for PSTATE.A in the pseudo-NMI case. Why is this any different? > if (system_uses_irq_prio_masking()) > gic_write_pmr(regs->pmr_save); > 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