Re: [Patch 4/4] PPC64-HWBKPT: Enable hw-breakpoints while handling intervening signals
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
> #include
> #include
> +#include
>
> #include "signal.h"
>
> @@ -312,6 +313,7 @@ int sys_swapcontext(struct ucontext __us
> || __copy_to_user(&old_ctx->uc_sigmask,
> ¤t->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.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 4/4] PPC64-HWBKPT: Enable hw-breakpoints while handling intervening signals
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
---
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(+)
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
#include
+#include
#include
#include
@@ -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
#include
#include
+#include
#include "signal.h"
@@ -312,6 +313,7 @@ int sys_swapcontext(struct ucontext __us
|| __copy_to_user(&old_ctx->uc_sigmask,
¤t->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
#include
#include
+#include
#ifdef CONFIG_PPC64
#include "ppc32.h"
#include
@@ -996,6 +997,7 @@ long sys_swapcontext(struct ucontext __u
|| put_sigset_t(&old_ctx->uc_sigmask, ¤t->blocked)
|| __put_user(to_user_ptr(mctx), &old_ctx->uc_regs))
return -EFAULT;
+ thread_change_pc(current);
}
if (new_ctx == NULL)
return 0;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 4/4] PPC64-HWBKPT: Enable hw-breakpoints while handling intervening signals
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 and clear them during sigreturn() syscall.
Limitation: Nested hw-breakpoint exceptions (where second exception is raised
inside signal context) will cause a 'double-hit' i.e. the first breakpoint
exception will be taken twice.
Restore hw-breakpoints if the user-context is altered in the signal handler
(causing loss of MSR_SE). Side-effect: 'Double-hit' of breakpoint if the
instruction pointer is unaltered in the new context.
Signed-off-by: K.Prasad
---
arch/powerpc/include/asm/hw_breakpoint.h |3 +++
arch/powerpc/kernel/hw_breakpoint.c | 28
arch/powerpc/kernel/signal.c |8
arch/powerpc/kernel/signal_32.c | 10 ++
arch/powerpc/kernel/signal_64.c |7 +++
5 files changed, 56 insertions(+)
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
@@ -43,6 +43,9 @@ static inline void hw_breakpoint_disable
{
set_dabr(0);
}
+extern void sighandler_install_bp(struct task_struct *tsk);
+extern void sigreturn_uninstall_bp(struct task_struct *tsk);
+extern void thread_change_pc(struct task_struct *tsk, unsigned long msr);
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
#endif /* __KERNEL__ */
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
@@ -175,6 +175,34 @@ int arch_validate_hwbkpt_settings(struct
return 0;
}
+void sighandler_install_bp(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);
+}
+
+void sigreturn_uninstall_bp(struct task_struct *tsk)
+{
+ if (unlikely(tsk->thread.last_hit_ubp))
+ set_dabr(0);
+}
+
+void thread_change_pc(struct task_struct *tsk, unsigned long new_msr)
+{
+ /*
+* Do not bother to restore breakpoints if single-stepping is not
+* cleared. single_step_dabr_instruction() will handle it if MSR_SE
+* is set.
+*/
+ if (!(new_msr & MSR_SE))
+ sighandler_install_bp(tsk);
+}
+
/*
* Handle debug exception notifications.
*/
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
#include
+#include
#include
#include
@@ -149,6 +150,13 @@ static int do_signal_pending(sigset_t *o
if (current->thread.dabr)
set_dabr(current->thread.dabr);
#endif
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ /*
+* Re-enable the breakpoints (if it was previously cleared in
+* hw_breakpoint_handler()) for the signal stack.
+*/
+ sighandler_install_bp(current);
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
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
#include
#include
+#include
#include "signal.h"
@@ -312,6 +313,9 @@ int sys_swapcontext(struct ucontext __us
|| __copy_to_user(&old_ctx->uc_sigmask,
¤t->blocked, sizeof(sigset_t)))
return -EFAULT;
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ thread_change_pc(current, new_msr);
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
}
if (new_ctx == NULL)
return 0;
@@ -364,6 +368,9 @@ int sys_rt_sigreturn(unsigned long r3, u
if (__copy_from_user(&set, &uc->uc_sigmask, sizeof(set)))
goto badframe;
restore_sigmask(&set);
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ sigreturn_uninstall_bp(current);
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
if (restore_sigcontext(regs, NULL, 1, &uc->uc_mcontext))
goto badframe;
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal_32.c
=
Re: [Patch 4/4] PPC64-HWBKPT: Enable hw-breakpoints while handling intervening signals
On Mon, May 24, 2010 at 04:04:19PM +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 and clear them during sigreturn() syscall. > > Limitation: Nested hw-breakpoint exceptions (where second exception is raised > inside signal context) will cause a 'double-hit' i.e. the first breakpoint > exception will be taken twice. I don't think this will actually cause a problem. In the case of a perf_event breakpoint, the semantics are trigger-after-execute, so the first hit won't cause a trigger, and perf_event won't double-count it. In the case of ptrace-style breakpoints, we don't single-step (it's up to the ptracer to do the single-stepping if needed) so the problem doesn't arise. In fact I don't think we even need to do anything on sigreturn. Yes, we are changing the NIP but we are changing it to a previous value as a result of an explicit action by the program, which is a bit different to what signal delivery and ptrace do. On signal delivery I was imagining that we would clear the MSR_SE bit before saving the MSR value in the signal frame, and reinstall the DABR value at the same time, and then essentially forget that we had already hit the breakpoint once and just wait for it to hit again. Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
