[GIT PULL] hw-breakpoint fixes

[GIT PULL] hw-breakpoint fixes

[Frederic Weisbecker]

Ingo,

Please pull the perf/urgent branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
	perf/urgent

Thanks,
	Frederic
---

Frederic Weisbecker (1):
      x86: Ignore trap bits on single step exceptions

Jason Wessel (1):
      perf,hw_breakpoint: Initialize hardware api earlier


 arch/x86/kernel/hw_breakpoint.c |    4 ++++
 include/linux/hw_breakpoint.h   |    4 ++++
 kernel/hw_breakpoint.c          |    3 +--
 kernel/perf_event.c             |    6 ++++++
 4 files changed, 15 insertions(+), 2 deletions(-)

[PATCH 1/2] x86: Ignore trap bits on single step exceptions

[Frederic Weisbecker]

When a single step exception fires, the trap bits, used to
signal hardware breakpoints, are in a random state.

These trap bits might be set if another exception will follow,
like a breakpoint in the next instruction, or a watchpoint in the
previous one. Or there can be any junk there.

So if we handle these trap bits during the single step exception,
we are going to handle an exception twice, or we are going to
handle junk.

Just ignore them in this case.

This fixes https://bugzilla.kernel.org/show_bug.cgi?id=21332

Reported-by: Michael Stefaniuc <mstefani@redhat.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Maciej Rutecki <maciej.rutecki@gmail.com>
Cc: Alexandre Julliard <julliard@winehq.org>
Cc: Jason Wessel <jason.wessel@windriver.com>
Cc: All since 2.6.33.x <stable@kernel.org>
---
 arch/x86/kernel/hw_breakpoint.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index ff15c9d..42c5942 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -433,6 +433,10 @@ static int __kprobes hw_breakpoint_handler(struct die_args *args)
 	dr6_p = (unsigned long *)ERR_PTR(args->err);
 	dr6 = *dr6_p;
 
+	/* If it's a single step, TRAP bits are random */
+	if (dr6 & DR_STEP)
+		return NOTIFY_DONE;
+
 	/* Do an early return if no trap bits are set in DR6 */
 	if ((dr6 & DR_TRAP_BITS) == 0)
 		return NOTIFY_DONE;
-- 
1.6.2.3

[PATCH 2/2] perf,hw_breakpoint: Initialize hardware api earlier

[Frederic Weisbecker]

From: Jason Wessel <jason.wessel@windriver.com>

When using early debugging, the kernel does not initialize the
hw_breakpoint API early enough and causes the late initialization of
the kernel debugger to fail. The boot arguments are:

    earlyprintk=vga ekgdboc=kbd kgdbwait

Then simply type "go" at the kdb prompt and boot. The kernel will
later emit the message:

    kgdb: Could not allocate hwbreakpoints

And at that point the kernel debugger will cease to work correctly.

The solution is to initialize the hw_breakpoint at the same time that
all the other perf call backs are initialized instead of using a
core_initcall() initialization which happens well after the kernel
debugger can make use of hardware breakpoints.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Ingo Molnar <mingo@elte.hu>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <4CD3396D.1090308@windriver.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/linux/hw_breakpoint.h |    4 ++++
 kernel/hw_breakpoint.c        |    3 +--
 kernel/perf_event.c           |    6 ++++++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index a2d6ea4..d1e55fe 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -33,6 +33,8 @@ enum bp_type_idx {
 
 #ifdef CONFIG_HAVE_HW_BREAKPOINT
 
+extern int __init init_hw_breakpoint(void);
+
 static inline void hw_breakpoint_init(struct perf_event_attr *attr)
 {
 	memset(attr, 0, sizeof(*attr));
@@ -108,6 +110,8 @@ static inline struct arch_hw_breakpoint *counter_arch_bp(struct perf_event *bp)
 
 #else /* !CONFIG_HAVE_HW_BREAKPOINT */
 
+static inline int __init init_hw_breakpoint(void) { return 0; }
+
 static inline struct perf_event *
 register_user_hw_breakpoint(struct perf_event_attr *attr,
 			    perf_overflow_handler_t triggered,
diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c
index 2c9120f..e532582 100644
--- a/kernel/hw_breakpoint.c
+++ b/kernel/hw_breakpoint.c
@@ -620,7 +620,7 @@ static struct pmu perf_breakpoint = {
 	.read		= hw_breakpoint_pmu_read,
 };
 
-static int __init init_hw_breakpoint(void)
+int __init init_hw_breakpoint(void)
 {
 	unsigned int **task_bp_pinned;
 	int cpu, err_cpu;
@@ -655,6 +655,5 @@ static int __init init_hw_breakpoint(void)
 
 	return -ENOMEM;
 }
-core_initcall(init_hw_breakpoint);
 
 
diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index 517d827..05b7d8c 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -31,6 +31,7 @@
 #include <linux/kernel_stat.h>
 #include <linux/perf_event.h>
 #include <linux/ftrace_event.h>
+#include <linux/hw_breakpoint.h>
 
 #include <asm/irq_regs.h>
 
@@ -6295,6 +6296,8 @@ perf_cpu_notify(struct notifier_block *self, unsigned long action, void *hcpu)
 
 void __init perf_event_init(void)
 {
+	int ret;
+
 	perf_event_init_all_cpus();
 	init_srcu_struct(&pmus_srcu);
 	perf_pmu_register(&perf_swevent);
@@ -6302,4 +6305,7 @@ void __init perf_event_init(void)
 	perf_pmu_register(&perf_task_clock);
 	perf_tp_register();
 	perf_cpu_notifier(perf_cpu_notify);
+
+	ret = init_hw_breakpoint();
+	WARN(ret, "hw_breakpoint initialization failed with: %d", ret);
 }
-- 
1.6.2.3

Re: [PATCH 2/2] perf,hw_breakpoint: Initialize hardware api earlier

[Peter Zijlstra]

On Sat, 2010-11-13 at 22:37 +0100, Frederic Weisbecker wrote:
> From: Jason Wessel <jason.wessel@windriver.com>
> 
> When using early debugging, the kernel does not initialize the
> hw_breakpoint API early enough and causes the late initialization of
> the kernel debugger to fail. The boot arguments are:
> 
>     earlyprintk=vga ekgdboc=kbd kgdbwait
> 
> Then simply type "go" at the kdb prompt and boot. The kernel will
> later emit the message:
> 
>     kgdb: Could not allocate hwbreakpoints
> 
> And at that point the kernel debugger will cease to work correctly.
> 
> The solution is to initialize the hw_breakpoint at the same time that
> all the other perf call backs are initialized instead of using a
> core_initcall() initialization which happens well after the kernel
> debugger can make use of hardware breakpoints.

How early is it needed? I've got a patch converting the x86 (still need
to do the other hardware pmus) to early_initcall() instead of random
places in the arch bringup (notably before the perf core init).

Re: [PATCH 2/2] perf,hw_breakpoint: Initialize hardware api earlier

[Jason Wessel]

On 11/13/2010 03:46 PM, Peter Zijlstra wrote:
> On Sat, 2010-11-13 at 22:37 +0100, Frederic Weisbecker wrote:
>   
>> From: Jason Wessel <jason.wessel@windriver.com>
>>
>> When using early debugging, the kernel does not initialize the
>> hw_breakpoint API early enough and causes the late initialization of
>> the kernel debugger to fail. The boot arguments are:
>>
>>     earlyprintk=vga ekgdboc=kbd kgdbwait
>>
>> Then simply type "go" at the kdb prompt and boot. The kernel will
>> later emit the message:
>>
>>     kgdb: Could not allocate hwbreakpoints
>>
>> And at that point the kernel debugger will cease to work correctly.
>>
>> The solution is to initialize the hw_breakpoint at the same time that
>> all the other perf call backs are initialized instead of using a
>> core_initcall() initialization which happens well after the kernel
>> debugger can make use of hardware breakpoints.
>>     
>
> How early is it needed? 

The HW breakpoint callback needs to be registered at the same time the
rest of the perf callbacks are registered.  More specifically, the
infrastructure setup needs to be completed before the debugger calls
"register_wide_hw_breakpoint(&attr, NULL)" in arch/x86/kernel/kgdb.c.  
After the debugger calls the registration function, from that point on
perf owns the reservation system for all hw break point registers.

> I've got a patch converting the x86 (still need
> to do the other hardware pmus) to early_initcall() instead of random
> places in the arch bringup (notably before the perf core init).
>
>   

This sounds to me like it would be early enough.  I could certainly run
the simple test case in the patch to make sure it still works, if you
point me to your patch(es).  I imagine I should also test the hand off
procedure where the debugger uses the registers directly up until the
point that perf is capable of handling reservations for the hw
breakpoint slots.

Thanks,
Jason.

Re: [PATCH 2/2] perf,hw_breakpoint: Initialize hardware api earlier

[Peter Zijlstra]

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

On Sun, 2010-11-14 at 07:33 -0600, Jason Wessel wrote:
> 
> This sounds to me like it would be early enough.  I could certainly run
> the simple test case in the patch to make sure it still works, if you
> point me to your patch(es).  I imagine I should also test the hand off
> procedure where the debugger uses the registers directly up until the
> point that perf is capable of handling reservations for the hw
> breakpoint slots. 

I had to actually write them -- I had some hacky things in my sysfs RFC.
Find attached.

[-- Attachment #2: perf-i386-fix-kconfig.patch --]
[-- Type: text/x-patch, Size: 879 bytes --]

Subject: perf, x86: Fixup Kconfig deps
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Tue Nov 16 21:49:01 CET 2010

This leads to a Kconfig dep inversion, x86 selects PERF_EVENT (due to
a hw_breakpoint dep) but doesn't unconditionally provide
HAVE_PERF_EVENT.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <new-submission>
---
 arch/x86/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/arch/x86/Kconfig
===================================================================
--- linux-2.6.orig/arch/x86/Kconfig
+++ linux-2.6/arch/x86/Kconfig
@@ -21,7 +21,7 @@ config X86
 	select HAVE_UNSTABLE_SCHED_CLOCK
 	select HAVE_IDE
 	select HAVE_OPROFILE
-	select HAVE_PERF_EVENTS if (!M386 && !M486)
+	select HAVE_PERF_EVENTS
 	select HAVE_IRQ_WORK
 	select HAVE_IOREMAP_PROT
 	select HAVE_KPROBES

[-- Attachment #3: perf-fix-hw-init.patch --]
[-- Type: text/x-patch, Size: 6904 bytes --]

Subject: perf, arch: Use early_initcall() for all arch pmu implementations
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Tue Nov 16 22:08:28 CET 2010

Currently architectures use various random locations to init the PMU
driver, for some this happens before the perf core code is
initialized.

In order to avoid calling perf_pmu_register() before the core code is
up and running and able to deal with it, move all arch init to at
least early_initcall (some archs use a later init, which is fine).

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <new-submission>
---
 arch/alpha/include/asm/perf_event.h |    6 ------
 arch/alpha/kernel/irq_alpha.c       |    2 --
 arch/alpha/kernel/perf_event.c      |    9 ++++++---
 arch/sparc/include/asm/perf_event.h |    4 ----
 arch/sparc/kernel/nmi.c             |    2 --
 arch/sparc/kernel/perf_event.c      |    7 +++++--
 arch/x86/include/asm/perf_event.h   |    2 --
 arch/x86/kernel/cpu/common.c        |    1 -
 arch/x86/kernel/cpu/perf_event.c    |    9 ++++++---
 9 files changed, 17 insertions(+), 25 deletions(-)

Index: linux-2.6/arch/alpha/include/asm/perf_event.h
===================================================================
--- linux-2.6.orig/arch/alpha/include/asm/perf_event.h
+++ linux-2.6/arch/alpha/include/asm/perf_event.h
@@ -1,10 +1,4 @@
 #ifndef __ASM_ALPHA_PERF_EVENT_H
 #define __ASM_ALPHA_PERF_EVENT_H
 
-#ifdef CONFIG_PERF_EVENTS
-extern void init_hw_perf_events(void);
-#else
-static inline void init_hw_perf_events(void)    { }
-#endif
-
 #endif /* __ASM_ALPHA_PERF_EVENT_H */
Index: linux-2.6/arch/alpha/kernel/irq_alpha.c
===================================================================
--- linux-2.6.orig/arch/alpha/kernel/irq_alpha.c
+++ linux-2.6/arch/alpha/kernel/irq_alpha.c
@@ -112,8 +112,6 @@ init_IRQ(void)
 	wrent(entInt, 0);
 
 	alpha_mv.init_irq();
-
-	init_hw_perf_events();
 }
 
 /*
Index: linux-2.6/arch/alpha/kernel/perf_event.c
===================================================================
--- linux-2.6.orig/arch/alpha/kernel/perf_event.c
+++ linux-2.6/arch/alpha/kernel/perf_event.c
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/kdebug.h>
 #include <linux/mutex.h>
+#include <linux/init.h>
 
 #include <asm/hwrpb.h>
 #include <asm/atomic.h>
@@ -863,13 +864,13 @@ static void alpha_perf_event_irq_handler
 /*
  * Init call to initialise performance events at kernel startup.
  */
-void __init init_hw_perf_events(void)
+int __init init_hw_perf_events(void)
 {
 	pr_info("Performance events: ");
 
 	if (!supported_cpu()) {
 		pr_cont("No support for your CPU.\n");
-		return;
+		return 0;
 	}
 
 	pr_cont("Supported CPU type!\n");
@@ -882,5 +883,7 @@ void __init init_hw_perf_events(void)
 	alpha_pmu = &ev67_pmu;
 
 	perf_pmu_register(&pmu);
-}
 
+	return 0;
+}
+early_initcall(init_hw_perf_events);
Index: linux-2.6/arch/sparc/include/asm/perf_event.h
===================================================================
--- linux-2.6.orig/arch/sparc/include/asm/perf_event.h
+++ linux-2.6/arch/sparc/include/asm/perf_event.h
@@ -4,8 +4,6 @@
 #ifdef CONFIG_PERF_EVENTS
 #include <asm/ptrace.h>
 
-extern void init_hw_perf_events(void);
-
 #define perf_arch_fetch_caller_regs(regs, ip)		\
 do {							\
 	unsigned long _pstate, _asi, _pil, _i7, _fp;	\
@@ -26,8 +24,6 @@ do {							\
 	(regs)->u_regs[UREG_I6] = _fp;			\
 	(regs)->u_regs[UREG_I7] = _i7;			\
 } while (0)
-#else
-static inline void init_hw_perf_events(void)	{ }
 #endif
 
 #endif
Index: linux-2.6/arch/sparc/kernel/nmi.c
===================================================================
--- linux-2.6.orig/arch/sparc/kernel/nmi.c
+++ linux-2.6/arch/sparc/kernel/nmi.c
@@ -270,8 +270,6 @@ int __init nmi_init(void)
 			atomic_set(&nmi_active, -1);
 		}
 	}
-	if (!err)
-		init_hw_perf_events();
 
 	return err;
 }
Index: linux-2.6/arch/sparc/kernel/perf_event.c
===================================================================
--- linux-2.6.orig/arch/sparc/kernel/perf_event.c
+++ linux-2.6/arch/sparc/kernel/perf_event.c
@@ -1307,20 +1307,23 @@ static bool __init supported_pmu(void)
 	return false;
 }
 
-void __init init_hw_perf_events(void)
+int __init init_hw_perf_events(void)
 {
 	pr_info("Performance events: ");
 
 	if (!supported_pmu()) {
 		pr_cont("No support for PMU type '%s'\n", sparc_pmu_type);
-		return;
+		return 0;
 	}
 
 	pr_cont("Supported PMU type is '%s'\n", sparc_pmu_type);
 
 	perf_pmu_register(&pmu);
 	register_die_notifier(&perf_event_nmi_notifier);
+
+	return 0;
 }
+early_initcall(init_hw_perf_event);
 
 void perf_callchain_kernel(struct perf_callchain_entry *entry,
 			   struct pt_regs *regs)
Index: linux-2.6/arch/x86/include/asm/perf_event.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/perf_event.h
+++ linux-2.6/arch/x86/include/asm/perf_event.h
@@ -125,7 +125,6 @@ union cpuid10_edx {
 #define IBS_OP_MAX_CNT_EXT	0x007FFFFFULL	/* not a register bit mask */
 
 #ifdef CONFIG_PERF_EVENTS
-extern void init_hw_perf_events(void);
 extern void perf_events_lapic_init(void);
 
 #define PERF_EVENT_INDEX_OFFSET			0
@@ -156,7 +155,6 @@ extern unsigned long perf_misc_flags(str
 }
 
 #else
-static inline void init_hw_perf_events(void)		{ }
 static inline void perf_events_lapic_init(void)	{ }
 #endif
 
Index: linux-2.6/arch/x86/kernel/cpu/common.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/common.c
+++ linux-2.6/arch/x86/kernel/cpu/common.c
@@ -894,7 +894,6 @@ void __init identify_boot_cpu(void)
 #else
 	vgetcpu_set_mode();
 #endif
-	init_hw_perf_events();
 }
 
 void __cpuinit identify_secondary_cpu(struct cpuinfo_x86 *c)
Index: linux-2.6/arch/x86/kernel/cpu/perf_event.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/perf_event.c
+++ linux-2.6/arch/x86/kernel/cpu/perf_event.c
@@ -1348,7 +1348,7 @@ static void __init pmu_check_apic(void)
 	pr_info("no hardware sampling interrupt available.\n");
 }
 
-void __init init_hw_perf_events(void)
+int __init init_hw_perf_events(void)
 {
 	struct event_constraint *c;
 	int err;
@@ -1363,11 +1363,11 @@ void __init init_hw_perf_events(void)
 		err = amd_pmu_init();
 		break;
 	default:
-		return;
+		return 0;
 	}
 	if (err != 0) {
 		pr_cont("no PMU driver, software events only.\n");
-		return;
+		return 0;
 	}
 
 	pmu_check_apic();
@@ -1420,7 +1420,10 @@ void __init init_hw_perf_events(void)
 
 	perf_pmu_register(&pmu);
 	perf_cpu_notifier(x86_pmu_notifier);
+
+	return 0;
 }
+early_initcall(init_hw_perf_events);
 
 static inline void x86_pmu_read(struct perf_event *event)
 {

[-- Attachment #4: perf-fix-init.patch --]
[-- Type: text/x-patch, Size: 1403 bytes --]

Subject: perf: Move perf_event_init() into main.c
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Tue Nov 16 22:12:05 CET 2010

Currently we call perf_event_init() from sched_init(). In order to
make it more obvious move it to the cannnonical location.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <new-submission>
---
 init/main.c    |    2 ++
 kernel/sched.c |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/init/main.c
===================================================================
--- linux-2.6.orig/init/main.c
+++ linux-2.6/init/main.c
@@ -68,6 +68,7 @@
 #include <linux/sfi.h>
 #include <linux/shmem_fs.h>
 #include <linux/slab.h>
+#include <linux/perf_event.h>
 
 #include <asm/io.h>
 #include <asm/bugs.h>
@@ -604,6 +605,7 @@ asmlinkage void __init start_kernel(void
 				"enabled *very* early, fixing it\n");
 		local_irq_disable();
 	}
+	perf_event_init();
 	rcu_init();
 	radix_tree_init();
 	/* init some links before init_ISA_irqs() */
Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c
+++ linux-2.6/kernel/sched.c
@@ -7876,8 +7876,6 @@ void __init sched_init(void)
 		zalloc_cpumask_var(&cpu_isolated_map, GFP_NOWAIT);
 #endif /* SMP */
 
-	perf_event_init();
-
 	scheduler_running = 1;
 }
 

[-- Attachment #5: perf-fix-tp-bp-init.patch --]
[-- Type: text/x-patch, Size: 1825 bytes --]

Subject: perf: Use early_initcall() for tracepoint and breakpoint init
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Tue Nov 16 22:14:41 CET 2010

Just like other pmu implementations, use early_initcall().

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <new-submission>
---
 kernel/hw_breakpoint.c |    2 +-
 kernel/perf_event.c    |    9 +++------
 2 files changed, 4 insertions(+), 7 deletions(-)

Index: linux-2.6/kernel/hw_breakpoint.c
===================================================================
--- linux-2.6.orig/kernel/hw_breakpoint.c
+++ linux-2.6/kernel/hw_breakpoint.c
@@ -655,6 +655,6 @@ static int __init init_hw_breakpoint(voi
 
 	return -ENOMEM;
 }
-core_initcall(init_hw_breakpoint);
+early_initcall(init_hw_breakpoint);
 
 
Index: linux-2.6/kernel/perf_event.c
===================================================================
--- linux-2.6.orig/kernel/perf_event.c
+++ linux-2.6/kernel/perf_event.c
@@ -4831,10 +4831,12 @@ static struct pmu perf_tracepoint = {
 	.read		= perf_swevent_read,
 };
 
-static inline void perf_tp_register(void)
+static __init int perf_tp_init(void)
 {
 	perf_pmu_register(&perf_tracepoint);
+	return 0;
 }
+early_initcall(perf_tp_init);
 
 static int perf_event_set_filter(struct perf_event *event, void __user *arg)
 {
@@ -4861,10 +4863,6 @@ static void perf_event_free_filter(struc
 
 #else
 
-static inline void perf_tp_register(void)
-{
-}
-
 static int perf_event_set_filter(struct perf_event *event, void __user *arg)
 {
 	return -ENOENT;
@@ -6365,6 +6363,5 @@ void __init perf_event_init(void)
 	perf_pmu_register(&perf_swevent);
 	perf_pmu_register(&perf_cpu_clock);
 	perf_pmu_register(&perf_task_clock);
-	perf_tp_register();
 	perf_cpu_notifier(perf_cpu_notify);
 }

Re: [GIT PULL] hw-breakpoint fixes

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Ingo,
> 
> Please pull the perf/urgent branch that can be found at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> 	perf/urgent
> 
> Thanks,
> 	Frederic
> ---
> 
> Frederic Weisbecker (1):
>       x86: Ignore trap bits on single step exceptions
> 
> Jason Wessel (1):
>       perf,hw_breakpoint: Initialize hardware api earlier
> 
> 
>  arch/x86/kernel/hw_breakpoint.c |    4 ++++
>  include/linux/hw_breakpoint.h   |    4 ++++
>  kernel/hw_breakpoint.c          |    3 +--
>  kernel/perf_event.c             |    6 ++++++
>  4 files changed, 15 insertions(+), 2 deletions(-)

Pulled, thanks a lot Frederic!

	Ingo

Re: [PATCH 2/2] perf,hw_breakpoint: Initialize hardware api earlier

[Jason Wessel]

On 11/16/2010 03:24 PM, Peter Zijlstra wrote:
> On Sun, 2010-11-14 at 07:33 -0600, Jason Wessel wrote:
>> This sounds to me like it would be early enough.  I could certainly run
>> the simple test case in the patch to make sure it still works, if you
>> point me to your patch(es).  I imagine I should also test the hand off
>> procedure where the debugger uses the registers directly up until the
>> point that perf is capable of handling reservations for the hw
>> breakpoint slots.
>
> I had to actually write them -- I had some hacky things in my sysfs RFC.
> Find attached.

It is worth summarizing the out of band discussion for anyone following
this thread.

The patch set did not address the problem of initializing the
hw_breakpoint api early enough to allow the breakpoint register hand
off to work correctly.


> Index: linux-2.6/kernel/hw_breakpoint.c
> ===========================================================
> --- linux-2.6.orig/kernel/hw_breakpoint.c
> +++ linux-2.6/kernel/hw_breakpoint.c
> @@ -655,6 +655,6 @@ static int __init init_hw_breakpoint(voi
> 
>         return -ENOMEM;
>  }
> -core_initcall(init_hw_breakpoint);
> +early_initcall(init_hw_breakpoint);

An early_initcall is still well after smp_init() and not early enough
for the kgdb arch setup to succeed when transitioning from early
debugging to late debugging.

There is a patch pending in the tip tree which moves the
hw_breakpoint_api registration to the same time the other perf
callbacks register.  We will go with that for now.

In the long term picture, at some point we should have a hand off
function that tells an early user of the hw breakpoint registers to
stop using them.  Presently there is a small window of overlap where
perf and kgdb both might write to the hw break point registers up
until the late kgdb initialization completes.  Presently there are no
regression tests for this condition and it might be the case that no
one ever has to debug this area with a hardware breakpoint, so we'll
leave it as a TODO item for the time being.

Thanks,
Jason.

Re: [PATCH 1/2] x86: Ignore trap bits on single step exceptions

[Michael Stefaniuc]

Hello Frederic,

On 11/13/2010 10:37 PM, Frederic Weisbecker wrote:
> When a single step exception fires, the trap bits, used to
> signal hardware breakpoints, are in a random state.
>
> These trap bits might be set if another exception will follow,
> like a breakpoint in the next instruction, or a watchpoint in the
> previous one. Or there can be any junk there.
>
> So if we handle these trap bits during the single step exception,
> we are going to handle an exception twice, or we are going to
> handle junk.
>
> Just ignore them in this case.
>
> This fixes https://bugzilla.kernel.org/show_bug.cgi?id=21332
sorry for the delay in testing this.

I have cherry-picked this patch on top of v2.6.37-rc3-102-gea49b16 and 
the ntdll/exception tests pass now.

Many thanks
bye
	michael

>
> Reported-by: Michael Stefaniuc<mstefani@redhat.com>
> Signed-off-by: Frederic Weisbecker<fweisbec@gmail.com>
> Cc: Rafael J. Wysocki<rjw@sisk.pl>
> Cc: Maciej Rutecki<maciej.rutecki@gmail.com>
> Cc: Alexandre Julliard<julliard@winehq.org>
> Cc: Jason Wessel<jason.wessel@windriver.com>
> Cc: All since 2.6.33.x<stable@kernel.org>
> ---
>   arch/x86/kernel/hw_breakpoint.c |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
> index ff15c9d..42c5942 100644
> --- a/arch/x86/kernel/hw_breakpoint.c
> +++ b/arch/x86/kernel/hw_breakpoint.c
> @@ -433,6 +433,10 @@ static int __kprobes hw_breakpoint_handler(struct die_args *args)
>   	dr6_p = (unsigned long *)ERR_PTR(args->err);
>   	dr6 = *dr6_p;
>
> +	/* If it's a single step, TRAP bits are random */
> +	if (dr6&  DR_STEP)
> +		return NOTIFY_DONE;
> +
>   	/* Do an early return if no trap bits are set in DR6 */
>   	if ((dr6&  DR_TRAP_BITS) == 0)
>   		return NOTIFY_DONE;


-- 
Michael Stefaniuc                           Tel.: +49-711-96437-199
Consulting Communications Engineer          Fax.: +49-711-96437-111
--------------------------------------------------------------------
Reg. Adresse: Red Hat GmbH, Otto-Hahn-Strasse 20, 85609 Dornach
Handelsregister: Amtsgericht Muenchen HRB 153243
Geschäftsführer: Brendan Lane, Charlie Peters, Michael Cunningham,
                  Charles Cachera

Re: [PATCH 1/2] x86: Ignore trap bits on single step exceptions

[Frederic Weisbecker]

On Wed, Nov 24, 2010 at 09:32:02PM +0100, Michael Stefaniuc wrote:
> Hello Frederic,
> 
> On 11/13/2010 10:37 PM, Frederic Weisbecker wrote:
> >When a single step exception fires, the trap bits, used to
> >signal hardware breakpoints, are in a random state.
> >
> >These trap bits might be set if another exception will follow,
> >like a breakpoint in the next instruction, or a watchpoint in the
> >previous one. Or there can be any junk there.
> >
> >So if we handle these trap bits during the single step exception,
> >we are going to handle an exception twice, or we are going to
> >handle junk.
> >
> >Just ignore them in this case.
> >
> >This fixes https://bugzilla.kernel.org/show_bug.cgi?id=21332
> sorry for the delay in testing this.
> 
> I have cherry-picked this patch on top of v2.6.37-rc3-102-gea49b16
> and the ntdll/exception tests pass now.
> 
> Many thanks
> bye
> 	michael

Thanks for testing!