Patchwork [4/4] PPC64-HWBKPT: Enable hw-breakpoints while handling intervening signals

login
register
mail settings
Submitter K.Prasad
Date May 25, 2010, 9:15 a.m.
Message ID <20100525091505.GE29003@in.ibm.com>
Download mbox | patch
Permalink /patch/53521/
State Superseded
Headers show

Comments

K.Prasad - May 25, 2010, 9:15 a.m.
A signal delivered between a hw_breakpoint_handler() and the
single_step_dabr_instruction() will not have the breakpoint active during
signal handling (since breakpoint will not be restored through single-stepping
due to absence of MSR_SE bit on the signal frame). Enable breakpoints before
signal delivery.

Restore hw-breakpoints if the user-context is altered in the signal handler.

Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/hw_breakpoint.h |    2 ++
 arch/powerpc/kernel/hw_breakpoint.c      |   16 ++++++++++++++++
 arch/powerpc/kernel/signal.c             |    3 +++
 arch/powerpc/kernel/signal_32.c          |    2 ++
 arch/powerpc/kernel/signal_64.c          |    2 ++
 5 files changed, 25 insertions(+)
Paul Mackerras - May 27, 2010, 6:32 a.m.
On Tue, May 25, 2010 at 02:45:05PM +0530, K.Prasad wrote:

> A signal delivered between a hw_breakpoint_handler() and the
> single_step_dabr_instruction() will not have the breakpoint active during
> signal handling (since breakpoint will not be restored through single-stepping
> due to absence of MSR_SE bit on the signal frame). Enable breakpoints before
> signal delivery.
> 
> Restore hw-breakpoints if the user-context is altered in the signal handler.

...

>  /*
> + * Restores the breakpoint on the debug registers.
> + * Invoke this function if it is known that the execution context is about to
> + * change to cause loss of MSR_SE settings.
> + */
> +void thread_change_pc(struct task_struct *tsk)
> +{
> +	struct arch_hw_breakpoint *info;
> +
> +	if (likely(!tsk->thread.last_hit_ubp))
> +		return;
> +
> +	info = counter_arch_bp(tsk->thread.last_hit_ubp);
> +	set_dabr(info->address | info->type | DABR_TRANSLATION);
> +}

I think this function needs to clear current->thread.last_hit_ubp.
You also need to pass the regs to it so it can clear MSR_SE from
regs->msr.

> Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal_64.c
> ===================================================================
> --- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal_64.c
> +++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal_64.c
> @@ -33,6 +33,7 @@
>  #include <asm/cacheflush.h>
>  #include <asm/syscalls.h>
>  #include <asm/vdso.h>
> +#include <asm/hw_breakpoint.h>
>  
>  #include "signal.h"
>  
> @@ -312,6 +313,7 @@ int sys_swapcontext(struct ucontext __us
>  		    || __copy_to_user(&old_ctx->uc_sigmask,
>  				      &current->blocked, sizeof(sigset_t)))
>  			return -EFAULT;
> +		thread_change_pc(current);

This is in the part of the code that is saving the old context, after
the old context has been saved.  We need to do it before saving the
old context so that the saved context doesn't have the MSR_SE bit set
in its MSR image.  And similarly in the 32-bit version.

In fact, we probably don't need to do anything here.  We should never
get into a system call (such as sys_swapcontext or sys_sigreturn) when
single-stepping a load or store that caused a DABR match.  If we do,
something very strange is happening.  So I would leave out the
swapcontext changes for this version.

Paul.

Patch

Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===================================================================
--- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/hw_breakpoint.h
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -65,9 +65,11 @@  static inline void hw_breakpoint_disable
 {
 	set_dabr(0);
 }
+extern void thread_change_pc(struct task_struct *tsk);
 
 #else	/* CONFIG_HAVE_HW_BREAKPOINT */
 static inline void hw_breakpoint_disable(void) { }
+static inline void thread_change_pc(struct task_struct *tsk) { }
 #endif	/* CONFIG_HAVE_HW_BREAKPOINT */
 #endif	/* __KERNEL__ */
 #endif	/* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===================================================================
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -176,6 +176,22 @@  int arch_validate_hwbkpt_settings(struct
 }
 
 /*
+ * Restores the breakpoint on the debug registers.
+ * Invoke this function if it is known that the execution context is about to
+ * change to cause loss of MSR_SE settings.
+ */
+void thread_change_pc(struct task_struct *tsk)
+{
+	struct arch_hw_breakpoint *info;
+
+	if (likely(!tsk->thread.last_hit_ubp))
+		return;
+
+	info = counter_arch_bp(tsk->thread.last_hit_ubp);
+	set_dabr(info->address | info->type | DABR_TRANSLATION);
+}
+
+/*
  * Handle debug exception notifications.
  */
 int __kprobes hw_breakpoint_handler(struct die_args *args)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
===================================================================
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
@@ -11,6 +11,7 @@ 
 
 #include <linux/tracehook.h>
 #include <linux/signal.h>
+#include <asm/hw_breakpoint.h>
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
 
@@ -149,6 +150,8 @@  static int do_signal_pending(sigset_t *o
 	if (current->thread.dabr)
 		set_dabr(current->thread.dabr);
 #endif
+	 /* Re-enable the breakpoints for the signal stack */
+	thread_change_pc(current);
 
 	if (is32) {
         	if (ka.sa.sa_flags & SA_SIGINFO)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal_64.c
===================================================================
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal_64.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal_64.c
@@ -33,6 +33,7 @@ 
 #include <asm/cacheflush.h>
 #include <asm/syscalls.h>
 #include <asm/vdso.h>
+#include <asm/hw_breakpoint.h>
 
 #include "signal.h"
 
@@ -312,6 +313,7 @@  int sys_swapcontext(struct ucontext __us
 		    || __copy_to_user(&old_ctx->uc_sigmask,
 				      &current->blocked, sizeof(sigset_t)))
 			return -EFAULT;
+		thread_change_pc(current);
 	}
 	if (new_ctx == NULL)
 		return 0;
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal_32.c
===================================================================
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal_32.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal_32.c
@@ -42,6 +42,7 @@ 
 #include <asm/syscalls.h>
 #include <asm/sigcontext.h>
 #include <asm/vdso.h>
+#include <asm/hw_breakpoint.h>
 #ifdef CONFIG_PPC64
 #include "ppc32.h"
 #include <asm/unistd.h>
@@ -996,6 +997,7 @@  long sys_swapcontext(struct ucontext __u
 		    || put_sigset_t(&old_ctx->uc_sigmask, &current->blocked)
 		    || __put_user(to_user_ptr(mctx), &old_ctx->uc_regs))
 			return -EFAULT;
+		thread_change_pc(current);
 	}
 	if (new_ctx == NULL)
 		return 0;