MIME-Version: 1.0
In-Reply-To: <CAO6TR8V5agz+5QbcWM2G6mOdQJcdTOwHXg+T_ufrTwXM8O-icA@mail.gmail.com>
References: <CAO6TR8W1ViJonskb0eObG+THObpabh909ob0Oz_8LAvNMuOPmA@mail.gmail.com>
	<alpine.DEB.2.11.1512101942470.3623@nanos>
	<CALCETrUaWxqyuyE2FqCs0ULPBdKMwCPtaoneyt4yBA8r5a5=yQ@mail.gmail.com>
	<alpine.DEB.2.11.1512102050290.3623@nanos>
	<CAO6TR8V5agz+5QbcWM2G6mOdQJcdTOwHXg+T_ufrTwXM8O-icA@mail.gmail.com>
Date: Fri, 11 Dec 2015 12:04:15 -0700
Message-ID: <CAO6TR8V9uJhAs54P_TVVzR453a7ByarrJdB5zRvUFxjasQsLwA@mail.gmail.com>
Subject: Re: [PATCH v2 1/1] Fix int1 recursion when no perf_bp_event is registeredy
From: Jeff Merkey <linux.mdb@gmail.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@amacapital.net>, LKML <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
        X86 ML <x86@kernel.org>, Peter Zijlstra <peterz@infradead.org>,
        Andy Lutomirski <luto@kernel.org>,
        Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
        Steven Rostedt <rostedt@goodmis.org>, Borislav Petkov <bp@alien8.de>,
        Jiri Olsa <jolsa@kernel.org>
Content-Type: text/plain; charset=UTF-8
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1253
Lines: 33

One thing I noticed in this section of code:

	/* Handle all the breakpoints that were triggered */
	for (i = 0; i < HBP_NUM; ++i) {
		if (likely(!(dr6 & (DR_TRAP0 << i))))
			continue;

                ... snip ...

		if (bp->hw.info.type == X86_BREAKPOINT_EXECUTE)
			args->regs->flags |= X86_EFLAGS_RF;

		rcu_read_unlock();
	}

Whoever wrote this loop did not seem to understand have observed the
code path in action on intel hardware.  There is NEVER a case I have
seen when the hardware sends multiple breakpoint statuses through dr6,
they are sent one at a time.  So the rolling check through all the
status bits is pointless since only one breakpoint will be reported by
the hardware at a time.  This is not to say that someone in the future
might may change it, but these interrupts are delivered one by one in
order and if there are duplicates (like a read/write breakpoint set at
the same address as an execute breakpoint.

Does anyone know why it was coded this way because its flat wrong.

Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/