Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 2 Jul 2020 17:02:50 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Naresh Kamboju <naresh.kamboju@linaro.org>
Cc:     Andy Lutomirski <luto@amacapital.net>,
        Thomas Gleixner <tglx@linutronix.de>,
        Andy Lutomirski <luto@kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        X86 ML <x86@kernel.org>, cj.chengjian@huawei.com,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Borislav Petkov <bp@alien8.de>,
        Minchan Kim <minchan@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Michel Lespinasse <walken@google.com>,
        lkft-triage@lists.linaro.org,
        Dave Hansen <dave.hansen@linux.intel.com>
Subject: Re: Perf: WARNING: arch/x86/entry/common.c:624
 idtentry_exit_cond_rcu+0x92/0xc0
Message-ID: <20200702150250.GS4800@hirez.programming.kicks-ass.net>
References: <87imfwd5f6.fsf@nanos.tec.linutronix.de>
 <8DD3180E-0E69-4FD6-92C3-311AAB3F688F@amacapital.net>
 <87d064d13p.fsf@nanos.tec.linutronix.de>
 <CA+G9fYvVEMVaF=qzhCpJ8NMb1-VN4cEh6sw8P_eNFCLQYOjCzA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CA+G9fYvVEMVaF=qzhCpJ8NMb1-VN4cEh6sw8P_eNFCLQYOjCzA@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jul 02, 2020 at 07:57:54PM +0530, Naresh Kamboju wrote:
> I have reported this warning on linux-next and now it is happening on
> linux mainline tree.
> May I know , are we missing a fix patch on linus 's tree ?
> 
> - Naresh
> ---
> While running selftest x86 single_step_syscall_32 on
> i386 kernel linux 5.8.0-rc3 kernel warning noticed.
> 
> steps to reproduce:
> --------------------------
> perf test
> and
> cd /opt/kselftests/default-in-kernel/x86
> ./single_step_syscall_32
> 
> crash dump,
> [ 1324.774385] kselftest: Running tests in x86
> [ 1324.830187] ------------[ cut here ]------------
> [ 1324.834820] IRQs not disabled as expected
> [ 1324.838838] WARNING: CPU: 2 PID: 5365 at
> /usr/src/kernel/arch/x86/entry/common.c:645
> idtentry_exit_cond_rcu+0x92/0xc0

How's this?

DEFINE_IDTENTRY_DEBUG() is DEFINE_IDTENTRY_RAW on x86_64
                           DEFINE_IDTENTRY on i386

calling exc_debug_*() from DEFINE_IDTENTRY() does a double layer of
idtentry_{enter,exit}*() functions.

Also, handle_debug() is still a trainwreck, we should never get to
cond_local_irq_enable() when !user.

Completely untested...

---
 arch/x86/kernel/traps.c | 49 +++++++++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index b0195771c932..47d6b46e1564 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -806,8 +806,17 @@ static void handle_debug(struct pt_regs *regs, unsigned long dr6, bool user)
 	 * If DR6 is zero, no point in trying to handle it. The kernel is
 	 * not using INT1.
 	 */
-	if (!user && !dr6)
-		return;
+	if (!user) {
+		/*
+		 * Catch SYSENTER with TF set and clear DR_STEP. If this hit a
+		 * watchpoint at the same time then that will still be handled.
+		 */
+		if ((dr6 & DR_STEP) && is_sysenter_singlestep(regs))
+			dr6 &= ~DR_STEP;
+
+		if (!dr6)
+			return;
+	}
 
 	/*
 	 * If dr6 has no reason to give us about the origin of this trap,
@@ -859,25 +868,29 @@ static void handle_debug(struct pt_regs *regs, unsigned long dr6, bool user)
 	cond_local_irq_disable(regs);
 }
 
+#ifdef CONFIG_X86_64
 static __always_inline void exc_debug_kernel(struct pt_regs *regs,
 					     unsigned long dr6)
 {
 	bool irq_state = idtentry_enter_nmi(regs);
 	instrumentation_begin();
 
-	/*
-	 * Catch SYSENTER with TF set and clear DR_STEP. If this hit a
-	 * watchpoint at the same time then that will still be handled.
-	 */
-	if ((dr6 & DR_STEP) && is_sysenter_singlestep(regs))
-		dr6 &= ~DR_STEP;
-
 	handle_debug(regs, dr6, false);
 
 	instrumentation_end();
 	idtentry_exit_nmi(regs, irq_state);
 }
 
+/* IST stack entry */
+DEFINE_IDTENTRY_DEBUG(exc_debug)
+{
+	unsigned long dr6, dr7;
+
+	debug_enter(&dr6, &dr7);
+	exc_debug_kernel(regs, dr6);
+	debug_exit(dr7);
+}
+
 static __always_inline void exc_debug_user(struct pt_regs *regs,
 					   unsigned long dr6)
 {
@@ -890,17 +903,6 @@ static __always_inline void exc_debug_user(struct pt_regs *regs,
 	idtentry_exit_user(regs);
 }
 
-#ifdef CONFIG_X86_64
-/* IST stack entry */
-DEFINE_IDTENTRY_DEBUG(exc_debug)
-{
-	unsigned long dr6, dr7;
-
-	debug_enter(&dr6, &dr7);
-	exc_debug_kernel(regs, dr6);
-	debug_exit(dr7);
-}
-
 /* User entry, runs on regular task stack */
 DEFINE_IDTENTRY_DEBUG_USER(exc_debug)
 {
@@ -917,12 +919,7 @@ DEFINE_IDTENTRY_DEBUG(exc_debug)
 	unsigned long dr6, dr7;
 
 	debug_enter(&dr6, &dr7);
-
-	if (user_mode(regs))
-		exc_debug_user(regs, dr6);
-	else
-		exc_debug_kernel(regs, dr6);
-
+	handle_debug(regs, dr6, user_mode(regs));
 	debug_exit(dr7);
 }
 #endif