Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1288243 > unrolled thread
| Started by | Jeff Merkey <linux.mdb@gmail.com> |
|---|---|
| First post | 2015-12-10 08:20 +0100 |
| Last post | 2015-12-14 00:20 +0100 |
| Articles | 3 on this page of 23 — 6 participants |
Back to article view | Back to linux.kernel
[PATCH 1/1] Fix int1 recursion when no perf_bp_event is registered Jeff Merkey <linux.mdb@gmail.com> - 2015-12-10 08:20 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registered Thomas Gleixner <tglx@linutronix.de> - 2015-12-10 20:00 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registered Andy Lutomirski <luto@amacapital.net> - 2015-12-10 20:10 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registered Jiri Olsa <jolsa@redhat.com> - 2015-12-10 20:30 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registered Borislav Petkov <bp@alien8.de> - 2015-12-10 20:30 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Thomas Gleixner <tglx@linutronix.de> - 2015-12-10 22:00 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Andy Lutomirski <luto@amacapital.net> - 2015-12-10 22:10 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Thomas Gleixner <tglx@linutronix.de> - 2015-12-10 22:20 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Jeff Merkey <linux.mdb@gmail.com> - 2015-12-10 22:30 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Jeff Merkey <linux.mdb@gmail.com> - 2015-12-10 22:20 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Ingo Molnar <mingo@kernel.org> - 2015-12-14 09:10 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Jeff Merkey <linux.mdb@gmail.com> - 2015-12-14 09:20 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Jeff Merkey <linux.mdb@gmail.com> - 2015-12-14 09:30 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Ingo Molnar <mingo@kernel.org> - 2015-12-14 10:30 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Jeff Merkey <linux.mdb@gmail.com> - 2015-12-14 19:00 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Andy Lutomirski <luto@amacapital.net> - 2015-12-14 19:00 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Jeff Merkey <linux.mdb@gmail.com> - 2015-12-14 19:20 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Jeff Merkey <linux.mdb@gmail.com> - 2015-12-14 19:20 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Jeff Merkey <linux.mdb@gmail.com> - 2015-12-10 22:20 +0100
Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy Jeff Merkey <linux.mdb@gmail.com> - 2015-12-10 23:30 +0100
Re: [PATCH v2 1/1] Fix int1 recursion when no perf_bp_event is registeredy Jeff Merkey <linux.mdb@gmail.com> - 2015-12-11 09:10 +0100
Re: [PATCH v2 1/1] Fix int1 recursion when no perf_bp_event is registeredy Jeff Merkey <linux.mdb@gmail.com> - 2015-12-11 20:10 +0100
Re: [PATCH v2 1/1] Fix int1 recursion when no perf_bp_event is registeredy Jeff Merkey <linux.mdb@gmail.com> - 2015-12-14 00:20 +0100
Page 2 of 2 — ← Prev page 1 [2]
| From | Jeff Merkey <linux.mdb@gmail.com> |
|---|---|
| Date | 2015-12-11 09:10 +0100 |
| Subject | Re: [PATCH v2 1/1] Fix int1 recursion when no perf_bp_event is registeredy |
| Message-ID | <qEqW6-1rK-3@gated-at.bofh.it> |
| In reply to | #1288922 |
I have completed testing of the following patch, added the fix and log
a message when the error occurs. I also reviewed the code paths that
touch this section of code. The lazy debug register behaviors pass
state back and forth with this subsystem through the thread.debugred6
virtualized register value to determine whether or not another
notify_die handler has been registered. The case that causes the
lockup is when NOTIFY_STOP is set which halts calls to other handlers
and also bypasses the checks at the bottom of the do_debug() function
in traps.c and the bp event handler is NULL. In this case, the int1
exception has no code path to set the resume flag absent this patch.
There are some complex behaviors in this code path that require
careful review. I used the following code to test this change to the
kernel compiled as a module. It's a pretty robust test since it sets
breakpoints at interrupt contexts and process context. Unloading the
module clears the breakpoints. I can send this test module as a
separate patch if requested to verify.
Test Module to test this patch:
#include <linux/module.h>
#include <linux/string.h>
#include <linux/time.h>
#include <linux/mm.h>
#include <linux/slab.h>
#include <linux/init.h>
#include <linux/blkdev.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
#include <linux/ip.h>
#include <linux/udp.h>
#include <linux/in.h>
#include <linux/inet.h>
#include <linux/version.h>
#include <linux/random.h>
#include <linux/ctype.h>
#include <linux/freezer.h>
#include <asm/atomic.h>
#include <net/sock.h>
#include <linux/delay.h>
#include <net/checksum.h>
#include <linux/fs.h>
#include <linux/pagemap.h>
#include <linux/highmem.h>
#include <linux/string.h>
#include <asm/uaccess.h>
#include <linux/namei.h>
#include <linux/statfs.h>
#include <linux/backing-dev.h>
#include "linux/kallsyms.h"
#define BREAK_EXECUTE 0
#define BREAK_WRITE 1
#define BREAK_IOPORT 2
#define BREAK_READWRITE 3
#define ONE_BYTE_FIELD 0
#define TWO_BYTE_FIELD 1
#define EIGHT_BYTE_FIELD 2
#define FOUR_BYTE_FIELD 3
/* DR7 Register */
#define L0_BIT 0x00000001
#define G0_BIT 0x00000002
#define L1_BIT 0x00000004
#define G1_BIT 0x00000008
#define L2_BIT 0x00000010
#define G2_BIT 0x00000020
#define L3_BIT 0x00000040
#define G3_BIT 0x00000080
#define LEXACT 0x00000100
#define GEXACT 0x00000200
#define GDETECT 0x00002000
#define DR7DEF 0x00000400
static int __init init_leaf(void)
{
unsigned long dr7 = 0;
printk("0: %lX\n", kallsyms_lookup_name("reschedule_interrupt"));
printk("1: %lX\n", kallsyms_lookup_name("apic_timer_interrupt"));
printk("2: %lX\n", kallsyms_lookup_name("schedule"));
printk("3: %lX\n", kallsyms_lookup_name("schedule"));
set_debugreg(kallsyms_lookup_name("reschedule_interrupt"), 0);
set_debugreg(kallsyms_lookup_name("apic_timer_interrupt"), 1);
set_debugreg(kallsyms_lookup_name("schedule"), 2);
set_debugreg(kallsyms_lookup_name("schedule"), 3);
dr7 &= 0xFFF0FFFF;
dr7 |= G0_BIT;
dr7 |= ((BREAK_EXECUTE << ((0 * 4) + 16)) | (ONE_BYTE_FIELD << ((0
* 4) + 18)));
dr7 &= 0xFF0FFFFF;
dr7 |= G1_BIT;
dr7 |= ((BREAK_EXECUTE << ((1 * 4) + 16)) | (ONE_BYTE_FIELD << ((1
* 4) + 18)));
dr7 &= 0xF0FFFFFF;
dr7 |= G2_BIT;
dr7 |= ((BREAK_EXECUTE << ((2 * 4) + 16)) | (ONE_BYTE_FIELD << ((2
* 4) + 18)));
dr7 &= 0x0FFFFFFF;
dr7 |= G3_BIT;
dr7 |= ((BREAK_READWRITE << ((3 * 4) + 16)) | (ONE_BYTE_FIELD <<
((3 * 4) + 18)));
set_debugreg(dr7, 7);
return 0;
}
static void __exit cleanup_leaf(void)
{
set_debugreg(0L, 7);
return;
}
module_init(init_leaf);
module_exit(cleanup_leaf);
MODULE_LICENSE("GPL");
The log output when the breakpoints are triggered on a 4 core system
is as expected. I did not put code in that will clear the breakpoints
automatically when this condition is detected. This is an item to
discuss.
[ 852.228301] hw_breakpoint_handler: 8816 callbacks suppressed
[ 852.228314] INFO: spurious INT1 exception dr6: 0x1 dr7: 0x300004AA
[ 852.228336] INFO: spurious INT1 exception dr6: 0x4 dr7: 0x300004AA
[ 852.228366] INFO: spurious INT1 exception dr6: 0x4 dr7: 0x300004AA
[ 852.228883] INFO: spurious INT1 exception dr6: 0x1 dr7: 0x300004AA
[ 852.228887] INFO: spurious INT1 exception dr6: 0x4 dr7: 0x300004AA
[ 852.228973] INFO: spurious INT1 exception dr6: 0x4 dr7: 0x300004AA
[ 852.229250] INFO: spurious INT1 exception dr6: 0x2 dr7: 0x300004AA
[ 852.229274] INFO: spurious INT1 exception dr6: 0x4 dr7: 0x300004AA
[ 852.229546] INFO: spurious INT1 exception dr6: 0x4 dr7: 0x300004AA
[ 852.230260] INFO: spurious INT1 exception dr6: 0x2 dr7: 0x300004AA
[ 857.285756] hw_breakpoint_handler: 3381 callbacks suppressed
[ 857.285764] INFO: spurious INT1 exception dr6: 0x1 dr7: 0x300004AA
[ 857.285780] INFO: spurious INT1 exception dr6: 0x4 dr7: 0x300004AA
Open Items:
Should we also disable these breakpoints by changing DR7 if they are
spurious or just throw up a lot of messages?
Jeff
Signed-off-by: Jeff V. Merkey <jeffmerkey@gmail.com>
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 50a3fad..1613a61 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -444,7 +444,7 @@ EXPORT_SYMBOL_GPL(hw_breakpoint_restore);
static int hw_breakpoint_handler(struct die_args *args)
{
int i, cpu, rc = NOTIFY_STOP;
- struct perf_event *bp;
+ struct perf_event *bp = NULL;
unsigned long dr7, dr6;
unsigned long *dr6_p;
@@ -475,6 +475,13 @@ static int hw_breakpoint_handler(struct die_args *args)
for (i = 0; i < HBP_NUM; ++i) {
if (likely(!(dr6 & (DR_TRAP0 << i))))
continue;
+ /*
+ * check if we got an execute breakpoint
+ * from the dr7 register. if we did, set
+ * the resume flag to avoid int1 recursion.
+ */
+ if ((dr7 & (3 << ((i * 4) + 16))) == 0)
+ args->regs->flags |= X86_EFLAGS_RF;
/*
* The counter may be concurrently released but that can only
@@ -503,7 +510,9 @@ static int hw_breakpoint_handler(struct die_args *args)
/*
* Set up resume flag to avoid breakpoint recursion when
- * returning back to origin.
+ * returning back to origin. Perform the check
+ * twice in case the event handler altered the
+ * system flags.
*/
if (bp->hw.info.type == X86_BREAKPOINT_EXECUTE)
args->regs->flags |= X86_EFLAGS_RF;
@@ -519,6 +528,18 @@ static int hw_breakpoint_handler(struct die_args *args)
(dr6 & (~DR_TRAP_BITS)))
rc = NOTIFY_DONE;
+ /*
+ * if we are about to signal to
+ * do_debug() to stop further processing
+ * and we have not ascertained the source
+ * of the breakpoint, log it as spurious.
+ */
+ if (rc == NOTIFY_STOP && !bp) {
+ printk_ratelimited(KERN_INFO
+ "INFO: spurious INT1 exception dr6: 0x%lX dr7: 0x%lX\n",
+ dr6, dr7);
+ }
+
set_debugreg(dr7, 7);
put_cpu();
--
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/
[toc] | [prev] | [next] | [standalone]
| From | Jeff Merkey <linux.mdb@gmail.com> |
|---|---|
| Date | 2015-12-11 20:10 +0100 |
| Subject | Re: [PATCH v2 1/1] Fix int1 recursion when no perf_bp_event is registeredy |
| Message-ID | <qEBeN-7o-13@gated-at.bofh.it> |
| In reply to | #1289288 |
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/
[toc] | [prev] | [next] | [standalone]
| From | Jeff Merkey <linux.mdb@gmail.com> |
|---|---|
| Date | 2015-12-14 00:20 +0100 |
| Subject | Re: [PATCH v2 1/1] Fix int1 recursion when no perf_bp_event is registeredy |
| Message-ID | <qFo5Q-6pM-31@gated-at.bofh.it> |
| In reply to | #1289288 |
Is there anything else I need to do on this patch, is it acceptable, do you want a v3 that disables the errant breakpoints? Problem with that approach is if in fact there is a "lazy" breakpoint we might end up clearing it being set if it fires off too soon. So this is the first time I have submitted a patch through this new process so what happens now? -- 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/
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web