Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933038AbbELO5U (ORCPT ); Tue, 12 May 2015 10:57:20 -0400 Received: from foss.arm.com ([217.140.101.70]:32939 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932488AbbELO5S (ORCPT ); Tue, 12 May 2015 10:57:18 -0400 Date: Tue, 12 May 2015 15:57:15 +0100 From: Will Deacon To: Vaneet Narang Cc: Maninder Singh , "linux@arm.linux.org.uk" , "linux-kernel@vger.kernel.org" , Amit Arora , AJEET YADAV , AKHILESH KUMAR , "linux-arm-kernel@lists.infradead.org" Subject: Re: Fwd: Re: Re: [EDT] [PATCH 1/1] Fix: hw watchpoint continually triggers callback Message-ID: <20150512145714.GC2062@arm.com> References: <1609501084.68951431436373294.JavaMail.weblogic@ep2mlwas08a> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1609501084.68951431436373294.JavaMail.weblogic@ep2mlwas08a> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4130 Lines: 94 On Tue, May 12, 2015 at 02:12:54PM +0100, Vaneet Narang wrote: > On Tue, May 12, 2015 at 12:48:13PM +0100, Maninder Singh wrote: > >> On ARM, when a watchpoint is registered using register_wide_hw_breakpoint, > >> the callback handler endlessly runs until the watchpoint is unregistered. > >> The reason for this issue is debug interrupts gets raised before > >> executing the instruction, and after interrupt handling ARM tries to > >> execute the same instruction again , which results in interrupt getting > >> raised again. > >> > >> This patch fixes this issue by using KPROBES (getting the instruction > >> executed and incrementing PC to next instruction). > >> > >> Signed-off-by: Vaneet Narang > >> Signed-off-by: Maninder Singh > >> Reviewed-by: Amit Arora > >> Reviewed-by: Ajeet Yadav > >> --- > >> arch/arm/kernel/hw_breakpoint.c | 18 ++++++++++++++++++ > >> 1 files changed, 18 insertions(+), 0 deletions(-) > >> > >> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c > >> index dc7d0a9..ec72f86 100644 > >> --- a/arch/arm/kernel/hw_breakpoint.c > >> +++ b/arch/arm/kernel/hw_breakpoint.c > >> @@ -37,6 +37,9 @@ > >> #include > >> #include > >> #include > >> +#ifdef CONFIG_KPROBES > >> +#include > >> +#endif > >> > >> /* Breakpoint currently in use for each BRP. */ > >> static DEFINE_PER_CPU(struct perf_event *, bp_on_reg[ARM_MAX_BRP]); > >> @@ -757,6 +760,21 @@ static void watchpoint_handler(unsigned long addr, unsigned int fsr, > >> */ > >> if (!wp->overflow_handler) > >> enable_single_step(wp, instruction_pointer(regs)); > >> +#ifdef CONFIG_KPROBES > >> + else { > >> + struct kprobe kp; > >> + unsigned long flags; > >> + > >> + arch_uninstall_hw_breakpoint(wp); > >> + kp.addr = (kprobe_opcode_t *)instruction_pointer(regs); > >> + if (!arch_prepare_kprobe(&kp)) { > >> + local_irq_save(flags); > >> + kp.ainsn.insn_singlestep(&kp, regs); > >> + local_irq_restore(flags); > >> + } > >> + arch_install_hw_breakpoint(wp); > >> + } > >> +#endif > > >I don't think this is the right thing to do at all; the kernel already > >handles step exceptions using mismatched breakpoints when there is no > >overflow handler specified (e.g. using perf mem events). If you register a > >handler (e.g. gdb via ptrace) then you have to handle the step yourself. > > This fix is given for kernel developers who wants to use perf interface by > registering callback using register_wide_hw_breakpoint API. On every > callback trigger they have to unregister watchpoints otherwise callback > gets called in a loop and now issue is "when to register watch point back > ?". If you want to solve this, I think we need a better way to expose software single-step/emulation to the overflow handler. If we try to do this in the hw_breakpoint code itself, we run into problems: - What if another thread hits the same instruction whilst we are trying to step it? - What if there are two breakpoints or a breakpoint + watchpoint triggered by the same instruction? - What if the debugger didn't want to execute the instruction at all? > With this issue in place, it makes perf interface unusable. We didn't > faced this issue with x86. This is a good point. If perf/hw_breakpoint are supposed to hide the internal details of the debug architecture and make everything look and smell like x86, I'd like to see that documented somewhere. I don't think we'd generally be able to achieve that whilst solving the caveats I mention above, so we'd probably just end up removing this feature altogether, which would be a shame (and I don't think possible as it stands, since hw_breakpoint doesn't know about its caller). Will -- 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/