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.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,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 B2248C433B4 for ; Wed, 28 Apr 2021 14:43:14 +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 EE4426142A for ; Wed, 28 Apr 2021 14:43:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EE4426142A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org 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: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=hBloOf3v7RQn4THKHu5ij5TmCXrgLGgScQuo8dRFVFU=; b=i9aYg1aOSqkP1ZrOdOOcDwN3L DoaJKBgEpq4EXe1uiZjqugxXFOulrR7l7DZOEHpxjNDAXIdhUjgASUCwMUkp81tgDj7LjRP31gq/g mvs37PzkiiV22TleRpdElMwSPDmsMZYYucXhZHFK+9o/7Ej8sKix3N8Xze26XDTdlRSYH0wP0xoQc FxX/VPQrkcS33NbCwanmuldZXSPER6No5QmkDxfeJv4OmzCyumcMVTa3FpyLpjJlQxnDVVEq6ZPX3 HjOsHlHkNA4MUyvTfExHyB8NjrNafCGqudVwLdaYjkLQsHygVKTIjX4472pekHqKnyWsa0sc1tFtZ 36qIrEtig==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lblNX-003dpl-VG; Wed, 28 Apr 2021 14:41:28 +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 1lblNV-003dpc-Aw for linux-arm-kernel@desiato.infradead.org; Wed, 28 Apr 2021 14:41:25 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Type:MIME-Version:References: In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=D9x0b5WKf5AMl4pGTiwav/0e8GjWy3WsQ9s+xq2x5Nw=; b=rOihqlX+UewYpYtgYMbnAhI3K2 IIb9cPC3L2j88rzYJVoeyzlKIpbsQmg8TdWrUsDKqhe712SiS6oi5uVqlloF7Aify48i0IYd8xxad PhCoAebuINHEOOnMX3WFWRS2j8O/opVUew5JFL4Q9yXrOqx30NvtuBxW67YUsbY2V7j1PFqcBV3mw FExmirz3nhsiXuxQ6rHlTEsUzpOKwsA0N1KfMlCwj3oMpeCrwWLI565O+d7gvOohcQVRp0xX18QpN DL67oR9v4HATAtdFjQsbnId0Z7BaeOVzInbLhcsXPzsu+4zArYV+ORjfrazRrjViA0VEuvy2UtQdX j81nbWkw==; Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lblNR-00HUxu-6M for linux-arm-kernel@lists.infradead.org; Wed, 28 Apr 2021 14:41:23 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 5FF9161222; Wed, 28 Apr 2021 14:41:20 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94) (envelope-from ) id 1lblNO-009s9T-Al; Wed, 28 Apr 2021 15:41:18 +0100 Date: Wed, 28 Apr 2021 15:41:17 +0100 Message-ID: <87im46o4j6.wl-maz@kernel.org> From: Marc Zyngier To: Mark Rutland 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 In-Reply-To: <20210428111555.50880-1-mark.rutland@arm.com> References: <20210428111555.50880-1-mark.rutland@arm.com> 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 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: mark.rutland@arm.com, linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com, will@kernel.org, yuzenghui@huawei.com 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-20210428_074121_330462_8B87FF04 X-CRM114-Status: GOOD ( 39.05 ) 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 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? In the current code, the ORR makes it plain that only the I bit has changed at this stage. Is there some other state tracking that make this complexity irrelevant? 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