All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] hw_breakpoint fixes
@ 2010-09-17  2:48 Frederic Weisbecker
  2010-09-17  2:48 ` [PATCH 1/2] x86: Fix instruction breakpoint encoding Frederic Weisbecker
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Frederic Weisbecker @ 2010-09-17  2:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Matt Helsley, Peter Zijlstra, Prasad

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: Fix instruction breakpoint encoding

Matt Helsley (1):
      hw breakpoints: Fix pid namespace bug


 arch/x86/include/asm/hw_breakpoint.h |    2 +-
 arch/x86/kernel/hw_breakpoint.c      |   40 ++++++++++++++++-----------------
 kernel/hw_breakpoint.c               |    3 +-
 3 files changed, 22 insertions(+), 23 deletions(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] x86: Fix instruction breakpoint encoding
  2010-09-17  2:48 [GIT PULL] hw_breakpoint fixes Frederic Weisbecker
@ 2010-09-17  2:48 ` Frederic Weisbecker
  2010-09-17  2:48 ` [PATCH 2/2] hw breakpoints: Fix pid namespace bug Frederic Weisbecker
  2010-09-17  6:59 ` [GIT PULL] hw_breakpoint fixes Ingo Molnar
  2 siblings, 0 replies; 4+ messages in thread
From: Frederic Weisbecker @ 2010-09-17  2:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Kelvie Wong, Prasad,
	Mahesh Salgaonkar, Will Deacon

Lengths and types of breakpoints are encoded in a half byte
into CPU registers. However when we extract these values
and store them, we add a high half byte part to them: 0x40 to the
length and 0x80 to the type.
When that gets reloaded to the CPU registers, the high part
is masked.

While making the instruction breakpoints available for perf,
I zapped that high part on instruction breakpoint encoding
and that broke the arch -> generic translation used by ptrace
instruction breakpoints. Writing dr7 to set an inst breakpoint
was then failing.

There is no apparent reason for these high parts so we could get
rid of them altogether. That's an invasive change though so let's
do that later and for now fix the problem by restoring that inst
breakpoint high part encoding in this sole patch.

Reported-by: Kelvie Wong <kelvie@ieee.org>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Prasad <prasad@linux.vnet.ibm.com>
Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/x86/include/asm/hw_breakpoint.h |    2 +-
 arch/x86/kernel/hw_breakpoint.c      |   40 ++++++++++++++++-----------------
 2 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/arch/x86/include/asm/hw_breakpoint.h b/arch/x86/include/asm/hw_breakpoint.h
index 528a11e..824ca07 100644
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -20,7 +20,7 @@ struct arch_hw_breakpoint {
 #include <linux/list.h>
 
 /* Available HW breakpoint length encodings */
-#define X86_BREAKPOINT_LEN_X		0x00
+#define X86_BREAKPOINT_LEN_X		0x40
 #define X86_BREAKPOINT_LEN_1		0x40
 #define X86_BREAKPOINT_LEN_2		0x44
 #define X86_BREAKPOINT_LEN_4		0x4c
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index a474ec3..ff15c9d 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -206,11 +206,27 @@ int arch_check_bp_in_kernelspace(struct perf_event *bp)
 int arch_bp_generic_fields(int x86_len, int x86_type,
 			   int *gen_len, int *gen_type)
 {
-	/* Len */
-	switch (x86_len) {
-	case X86_BREAKPOINT_LEN_X:
+	/* Type */
+	switch (x86_type) {
+	case X86_BREAKPOINT_EXECUTE:
+		if (x86_len != X86_BREAKPOINT_LEN_X)
+			return -EINVAL;
+
+		*gen_type = HW_BREAKPOINT_X;
 		*gen_len = sizeof(long);
+		return 0;
+	case X86_BREAKPOINT_WRITE:
+		*gen_type = HW_BREAKPOINT_W;
 		break;
+	case X86_BREAKPOINT_RW:
+		*gen_type = HW_BREAKPOINT_W | HW_BREAKPOINT_R;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Len */
+	switch (x86_len) {
 	case X86_BREAKPOINT_LEN_1:
 		*gen_len = HW_BREAKPOINT_LEN_1;
 		break;
@@ -229,21 +245,6 @@ int arch_bp_generic_fields(int x86_len, int x86_type,
 		return -EINVAL;
 	}
 
-	/* Type */
-	switch (x86_type) {
-	case X86_BREAKPOINT_EXECUTE:
-		*gen_type = HW_BREAKPOINT_X;
-		break;
-	case X86_BREAKPOINT_WRITE:
-		*gen_type = HW_BREAKPOINT_W;
-		break;
-	case X86_BREAKPOINT_RW:
-		*gen_type = HW_BREAKPOINT_W | HW_BREAKPOINT_R;
-		break;
-	default:
-		return -EINVAL;
-	}
-
 	return 0;
 }
 
@@ -316,9 +317,6 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp)
 	ret = -EINVAL;
 
 	switch (info->len) {
-	case X86_BREAKPOINT_LEN_X:
-		align = sizeof(long) -1;
-		break;
 	case X86_BREAKPOINT_LEN_1:
 		align = 0;
 		break;
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] hw breakpoints: Fix pid namespace bug
  2010-09-17  2:48 [GIT PULL] hw_breakpoint fixes Frederic Weisbecker
  2010-09-17  2:48 ` [PATCH 1/2] x86: Fix instruction breakpoint encoding Frederic Weisbecker
@ 2010-09-17  2:48 ` Frederic Weisbecker
  2010-09-17  6:59 ` [GIT PULL] hw_breakpoint fixes Ingo Molnar
  2 siblings, 0 replies; 4+ messages in thread
From: Frederic Weisbecker @ 2010-09-17  2:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Matt Helsley, Robin Green, Peter Zijlstra, Prasad,
	Arnaldo Carvalho de Melo, Steven Rostedt, Will Deacon,
	Mahesh Salgaonkar, 2.6.33-2.6.35, Ingo Molnar,
	Frederic Weisbecker

From: Matt Helsley <matthltc@us.ibm.com>

Hardware breakpoints can't be registered within pid namespaces
because tsk->pid is passed rather than the pid in the current
namespace.

(See https://bugzilla.kernel.org/show_bug.cgi?id=17281 )

This is a quick fix demonstrating the problem but is not the
best method of solving the problem since passing pids internally
is not the best way to avoid pid namespace bugs. Subsequent patches
will show a better solution.

Much thanks to Frederic Weisbecker <fweisbec@gmail.com> for doing
the bulk of the work finding this bug.

Reported-by: Robin Green <greenrd@greenrd.org>
Signed-off-by: Matt Helsley <matthltc@us.ibm.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Prasad <prasad@linux.vnet.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Cc: 2.6.33-2.6.35 <stable@kernel.org>
LKML-Reference: <f63454af09fb1915717251570423eb9ddd338340.1284407762.git.matthltc@us.ibm.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/hw_breakpoint.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c
index d71a987..c7c2aed 100644
--- a/kernel/hw_breakpoint.c
+++ b/kernel/hw_breakpoint.c
@@ -433,7 +433,8 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
 			    perf_overflow_handler_t triggered,
 			    struct task_struct *tsk)
 {
-	return perf_event_create_kernel_counter(attr, -1, tsk->pid, triggered);
+	return perf_event_create_kernel_counter(attr, -1, task_pid_vnr(tsk),
+						triggered);
 }
 EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
 
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [GIT PULL] hw_breakpoint fixes
  2010-09-17  2:48 [GIT PULL] hw_breakpoint fixes Frederic Weisbecker
  2010-09-17  2:48 ` [PATCH 1/2] x86: Fix instruction breakpoint encoding Frederic Weisbecker
  2010-09-17  2:48 ` [PATCH 2/2] hw breakpoints: Fix pid namespace bug Frederic Weisbecker
@ 2010-09-17  6:59 ` Ingo Molnar
  2 siblings, 0 replies; 4+ messages in thread
From: Ingo Molnar @ 2010-09-17  6:59 UTC (permalink / raw)
  To: Frederic Weisbecker; +Cc: LKML, Matt Helsley, Peter Zijlstra, Prasad


* 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: Fix instruction breakpoint encoding
> 
> Matt Helsley (1):
>       hw breakpoints: Fix pid namespace bug
> 
> 
>  arch/x86/include/asm/hw_breakpoint.h |    2 +-
>  arch/x86/kernel/hw_breakpoint.c      |   40 ++++++++++++++++-----------------
>  kernel/hw_breakpoint.c               |    3 +-
>  3 files changed, 22 insertions(+), 23 deletions(-)

Pulled, thanks a lot Frederic!

	Ingo

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2010-09-17  6:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-17  2:48 [GIT PULL] hw_breakpoint fixes Frederic Weisbecker
2010-09-17  2:48 ` [PATCH 1/2] x86: Fix instruction breakpoint encoding Frederic Weisbecker
2010-09-17  2:48 ` [PATCH 2/2] hw breakpoints: Fix pid namespace bug Frederic Weisbecker
2010-09-17  6:59 ` [GIT PULL] hw_breakpoint fixes Ingo Molnar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.