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 X-Spam-Level: X-Spam-Status: No, score=-14.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DFBA0C43460 for ; Wed, 28 Apr 2021 15:31:20 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3CD09610A5 for ; Wed, 28 Apr 2021 15:31:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3CD09610A5 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=1sDArqxKqTrPbEeYm49hb0BeFWBEzRTHybG0vav8DTA=; b=ImI0PeDj6pz40NjF/+YSE8K+J F3o/jS2iG5XifNIh7jMCQMq7Boo1hNpxiwzEtbRfqByvfe+M9/JAZBy+3QbTGGpO/lBxvoSXBU78R JT72cO42bHp22/p8CPOODeVT85242+zvojfv7WB/4Locvgf4ZlIEmycrzWQFV6UDqx8l+bCEfZkMh VEF9oTe7D7IbhugRpQ63+TMr8iYYhCNxsg47v8YAkSdXWwfgSYI8uXtdZJfkFrBli9TzGerjdieTN XFWtVYoLICkQGLzf0woaxq0EUDNR+qJM8QeOxfXtQISYHcFov9naR5B7kBk4oK7/M+dgHYE4XLLNU MMOK47mvw==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lbm8Q-003kmG-VO; Wed, 28 Apr 2021 15:29:55 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lbm8P-003kmA-EM for linux-arm-kernel@desiato.infradead.org; Wed, 28 Apr 2021 15:29:53 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=DG4Y6VInYIby9M5UhbbdJDE1OP0Q0pexU8htQSTdURo=; b=0sKUg417in1+pm/jB58nN2Y2yJ M24DbgLh+v76yUYPqCJhbQOoSUwgRTxnbfRVeIfbXzZ+R3bODLrb2zgP+6nW6vhiQvnB3LOJZwcKK mm3dJOVfyaIvAMh17tNBEp5fxZTQe8mn+7fbrJg5w1lAMBswH/EpIAXunujTuW/1/QQ1eiLOzWjPT 80aRo7tOy3apbE0VKLdrxYOW+BmXEwbVFo2NRMjUrpOpAezGWxDDA/s30cY2BNy+WkgGZLotSzofY 7E75ItfXGuD/XBbPkKa1Tu9ed0guyKz5UJnqNSxWjVLR7fEjQupVVzSTMTbQ5obEW4BtcjG7HzSit Z1dCLq/g==; Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lbm8L-00HYR6-Jo for linux-arm-kernel@lists.infradead.org; Wed, 28 Apr 2021 15:29:52 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 90D191FB; Wed, 28 Apr 2021 08:29:42 -0700 (PDT) Received: from C02TD0UTHF1T.local (unknown [10.57.5.87]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 898113F73B; Wed, 28 Apr 2021 08:29:41 -0700 (PDT) Date: Wed, 28 Apr 2021 16:29:39 +0100 From: Mark Rutland To: Marc Zyngier Cc: linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com, will@kernel.org, yuzenghui@huawei.com Subject: Re: [PATCH] arm64: entry: always set GIC_PRIO_PSR_I_SET during entry Message-ID: <20210428152939.GB1343@C02TD0UTHF1T.local> References: <20210428111555.50880-1-mark.rutland@arm.com> <87im46o4j6.wl-maz@kernel.org> <20210428151439.GA1343@C02TD0UTHF1T.local> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210428151439.GA1343@C02TD0UTHF1T.local> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210428_082950_094752_33ADDFBB X-CRM114-Status: GOOD ( 44.86 ) 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 Wed, Apr 28, 2021 at 04:19:11PM +0100, Mark Rutland wrote: > On Wed, Apr 28, 2021 at 03:41:17PM +0100, Marc Zyngier wrote: > > Hi Mark, > > > > On Wed, 28 Apr 2021 12:15:55 +0100, > > Mark Rutland wrote: > > > > > > Zenghui reports that booting a kernel with "irqchip.gicv3_pseudo_nmi=1" > > > on the command line hits a warning during kernel entry, due to the way > > > we manipulate the PMR. > > > > > > Early in the entry sequence, we call lockdep_hardirqs_off() to inform > > > lockdep that interrupts have been masked (as the HW sets DAIF wqhen > > > entering an exception). Architecturally PMR_EL1 is not affected by > > > exception entry, and we don't set GIC_PRIO_PSR_I_SET in the PMR early in > > > the exception entry sequence, so early in exception entry the PMR can > > > indicate that interrupts are unmasked even though they are masked by > > > DAIF. > > > > > > If DEBUG_LOCKDEP is selected, lockdep_hardirqs_off() will check that > > > interrupts are masked, before we set GIC_PRIO_PSR_I_SET in any of the > > > exception entry paths, and hence lockdep_hardirqs_off() will WARN() that > > > something is amiss. > > > > > > We can avoid this by consistently setting GIC_PRIO_PSR_I_SET during > > > exception entry so that kernel code sees a consistent environment. We > > > must also update local_daif_inherit() to undo this, as currently only > > > touches DAIF. For other paths, local_daif_restore() will update both > > > DAIF and the PMR. With this done, we can remove the existing special > > > cases which set this later in the entry code. > > > > > > We always use (GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET) for consistency with > > > local_daif_save(), as this will warn if it ever encounters > > > (GIC_PRIO_IRQOFF | GIC_PRIO_PSR_I_SET), and never sets this itself. This > > > matches the gic_prio_kentry_setup that we have to retain for > > > ret_to_user. > > > > > > The original splat from Zenghui's report was: > > > > > > | DEBUG_LOCKS_WARN_ON(!irqs_disabled()) > > > | WARNING: CPU: 3 PID: 125 at kernel/locking/lockdep.c:4258 lockdep_hardirqs_off+0xd4/0xe8 > > > | Modules linked in: > > > | CPU: 3 PID: 125 Comm: modprobe Tainted: G W 5.12.0-rc8+ #463 > > > | Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015 > > > | pstate: 604003c5 (nZCv DAIF +PAN -UAO -TCO BTYPE=--) > > > | pc : lockdep_hardirqs_off+0xd4/0xe8 > > > | lr : lockdep_hardirqs_off+0xd4/0xe8 > > > | sp : ffff80002a39bad0 > > > | pmr_save: 000000e0 > > > | x29: ffff80002a39bad0 x28: ffff0000de214bc0 > > > | x27: ffff0000de1c0400 x26: 000000000049b328 > > > | x25: 0000000000406f30 x24: ffff0000de1c00a0 > > > | x23: 0000000020400005 x22: ffff8000105f747c > > > | x21: 0000000096000044 x20: 0000000000498ef9 > > > | x19: ffff80002a39bc88 x18: ffffffffffffffff > > > | x17: 0000000000000000 x16: ffff800011c61eb0 > > > | x15: ffff800011700a88 x14: 0720072007200720 > > > | x13: 0720072007200720 x12: 0720072007200720 > > > | x11: 0720072007200720 x10: 0720072007200720 > > > | x9 : ffff80002a39bad0 x8 : ffff80002a39bad0 > > > | x7 : ffff8000119f0800 x6 : c0000000ffff7fff > > > | x5 : ffff8000119f07a8 x4 : 0000000000000001 > > > | x3 : 9bcdab23f2432800 x2 : ffff800011730538 > > > | x1 : 9bcdab23f2432800 x0 : 0000000000000000 > > > | Call trace: > > > | lockdep_hardirqs_off+0xd4/0xe8 > > > | enter_from_kernel_mode.isra.5+0x7c/0xa8 > > > | el1_abort+0x24/0x100 > > > | el1_sync_handler+0x80/0xd0 > > > | el1_sync+0x6c/0x100 > > > | __arch_clear_user+0xc/0x90 > > > | load_elf_binary+0x9fc/0x1450 > > > | bprm_execve+0x404/0x880 > > > | kernel_execve+0x180/0x188 > > > | call_usermodehelper_exec_async+0xdc/0x158 > > > | ret_from_fork+0x10/0x18 > > > > > > Fixes: 23529049c6842382 ("arm64: entry: fix non-NMI user<->kernel transitions") > > > Fixes: 7cd1ea1010acbede ("arm64: entry: fix non-NMI kernel<->kernel transitions") > > > Fixes: f0cd5ac1e4c53cb6 ("arm64: entry: fix NMI {user, kernel}->kernel transitions") > > > Fixes: 2a9b3e6ac69a8bf1 ("arm64: entry: fix EL1 debug transitions") > > > Link: https://lore.kernel.org/r/f4012761-026f-4e51-3a0c-7524e434e8b3@huawei.com > > > Signed-off-by: Mark Rutland > > > Reported-by: Zenghui Yu > > > Cc: Catalin Marinas > > > Cc: Marc Zyngier > > > Cc: Will Deacon > > > --- > > > arch/arm64/include/asm/daifflags.h | 3 +++ > > > arch/arm64/kernel/entry-common.c | 17 ----------------- > > > arch/arm64/kernel/entry.S | 15 ++------------- > > > 3 files changed, 5 insertions(+), 30 deletions(-) > > > > > > diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h > > > index 1c26d7baa67f..cfdde3a56805 100644 > > > --- a/arch/arm64/include/asm/daifflags.h > > > +++ b/arch/arm64/include/asm/daifflags.h > > > @@ -131,6 +131,9 @@ static inline void local_daif_inherit(struct pt_regs *regs) > > > if (interrupts_enabled(regs)) > > > trace_hardirqs_on(); > > > > > > + if (system_uses_irq_prio_masking()) > > > + gic_write_pmr(regs->pmr_save); > > > + > > > /* > > > * We can't use local_daif_restore(regs->pstate) here as > > > * system_has_prio_mask_debugging() won't restore the I bit if it can > > > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c > > > index 9d3588450473..117412bae915 100644 > > > --- a/arch/arm64/kernel/entry-common.c > > > +++ b/arch/arm64/kernel/entry-common.c > > > @@ -226,14 +226,6 @@ static void noinstr el1_dbg(struct pt_regs *regs, unsigned long esr) > > > { > > > unsigned long far = read_sysreg(far_el1); > > > > > > - /* > > > - * The CPU masked interrupts, and we are leaving them masked during > > > - * do_debug_exception(). Update PMR as if we had called > > > - * local_daif_mask(). > > > - */ > > > - if (system_uses_irq_prio_masking()) > > > - gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET); > > > - > > > arm64_enter_el1_dbg(regs); > > > if (!cortex_a76_erratum_1463225_debug_handler(regs)) > > > do_debug_exception(far, esr, regs); > > > @@ -398,9 +390,6 @@ static void noinstr el0_dbg(struct pt_regs *regs, unsigned long esr) > > > /* Only watchpoints write FAR_EL1, otherwise its UNKNOWN */ > > > unsigned long far = read_sysreg(far_el1); > > > > > > - if (system_uses_irq_prio_masking()) > > > - gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET); > > > - > > > enter_from_user_mode(); > > > do_debug_exception(far, esr, regs); > > > local_daif_restore(DAIF_PROCCTX_NOIRQ); > > > @@ -408,9 +397,6 @@ static void noinstr el0_dbg(struct pt_regs *regs, unsigned long esr) > > > > > > static void noinstr el0_svc(struct pt_regs *regs) > > > { > > > - if (system_uses_irq_prio_masking()) > > > - gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET); > > > - > > > enter_from_user_mode(); > > > cortex_a76_erratum_1463225_svc_handler(); > > > do_el0_svc(regs); > > > @@ -486,9 +472,6 @@ static void noinstr el0_cp15(struct pt_regs *regs, unsigned long esr) > > > > > > static void noinstr el0_svc_compat(struct pt_regs *regs) > > > { > > > - if (system_uses_irq_prio_masking()) > > > - gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET); > > > - > > > enter_from_user_mode(); > > > cortex_a76_erratum_1463225_svc_handler(); > > > do_el0_svc_compat(regs); > > > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S > > > index 6acfc5e6b5e0..941bb52b5e21 100644 > > > --- a/arch/arm64/kernel/entry.S > > > +++ b/arch/arm64/kernel/entry.S > > > @@ -292,6 +292,8 @@ alternative_else_nop_endif > > > alternative_if ARM64_HAS_IRQ_PRIO_MASKING > > > mrs_s x20, SYS_ICC_PMR_EL1 > > > str x20, [sp, #S_PMR_SAVE] > > > + mov x20, #GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET > > > + msr_s SYS_ICC_PMR_EL1, x20 > > > > I'm a bit worried about forcing PMR to IRQON at this stage. We could > > have been in IRQOFF state and entering the kernel because of a > > NMI. Don't we risk losing track of how we made it here? > > I also worried about this, but after digging for a while I convinced > myself there wasn't a problem, as: > > * This is what local_daif_mask() does today. It sets the PMR to > GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET regardless of its original state. > > * For checking how we got here, we use either: > - regs::pmr_save, which is sampled (immediately) before we alter HW PMR > in the asm above. > - DAIF, which is unaffected by this. > > * This is transparent to local_irq_save() .. local_irq_restore(), as > that blindly saves/restores the PMR. > ... oh, and within the GIC driver: * for a pNMI, we'll leave both PMR and DAIF as-is * for an IRQ, we'll set the PMU to GIC_PRIO_IRQOFF (regardless of the original value), then unmask DAIF. ... so that'll do the right thing even if we enter with PMR set to (GIC_PRIO_IRQON | CIG_PRIO_PSR_I_SET). Thanks. Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel