From patchwork Thu Sep 13 21:41:02 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Will Deacon X-Patchwork-Id: 1454331 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (unknown [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id D10B8DF24C for ; Thu, 13 Sep 2012 21:55:23 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TCHAT-0005WV-QW; Thu, 13 Sep 2012 21:41:50 +0000 Received: from cam-admin0.cambridge.arm.com ([217.140.96.50]) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TCHAP-0005Vj-EB for linux-arm-kernel@lists.infradead.org; Thu, 13 Sep 2012 21:41:46 +0000 Received: from mudshark.cambridge.arm.com (mudshark.cambridge.arm.com [10.1.79.58]) by cam-admin0.cambridge.arm.com (8.12.6/8.12.6) with ESMTP id q8DLf3E7029615; Thu, 13 Sep 2012 22:41:03 +0100 (BST) Date: Thu, 13 Sep 2012 22:41:02 +0100 From: Will Deacon To: Valentin Pistol Subject: Re: ARM: hw_breakpoint: silent EPERM when setting ARM_DSCR_MDBGEN on ARM_DEBUG_ARCH_V7_ECP14 Message-ID: <20120913214102.GB11399@mudshark.cambridge.arm.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Note: CRM114 invocation failed X-Spam-Score: -7.4 (-------) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-7.4 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [217.140.96.50 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -0.5 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On Thu, Sep 13, 2012 at 07:42:09PM +0100, Valentin Pistol wrote: > Hi everyone, Hi Valentin, > This is my first list post, thanks for bearing with me. You might want to configure your mail client to wrap lines at 76-80 columns in order to make your mails more readable. > Summary: > Kernel is compiled with CONFIG_HAVE_HW_BREAKPOINT but after boot the DSCR register does not have monitor mode enabled which is required for debug register use. That means that hardware breakpoints are unusable on your platform, likely because it has been locked down in the hardware (there is a signal that can be tied off to prevent monitor mode debug). > I am interested in using the ptrace interface for hw breakpoints/watchpoints on a Cortex A9. I found out that watchpoints are not triggering and tracked down an issue to the hw_breakpoint.c. The flow is as follows: > arch_hw_breakpoint_init > on_each_cpu(reset_ctrl_regs, &cpumask, 1); > reset_ctrl_regs > enable_monitor_mode > > enable_monitor_mode performs ARM_DBG_WRITE(c2, 2, (dscr | ARM_DSCR_MDBGEN)); > Another read on dscr is performed to check if the bit was set and if not it returns -EPERM. In my case the write is not successful so it returns. > > Since the reset_ctrl_regs code just returns on failure setting the monitor mode, there is no reporting of this. > if (enable_monitor_mode) > return ret; I'll fix this, we should print something here. Thanks. > After boot, my ptrace test example with PTRACE_GETHBPREGS works fine but PTRACE_SETHBPREGS fails silently inside arch_install_hw_breakpoint due to the same enable_monitor_mode issue. That's probably because the monitor mode check is at *install* time, which is asynchronous wrt the ptrace system call (i.e. when the child is next scheduled). How about we move the check to validation time instead? We should never try to install an unvalidated breakpoint and trying to support hardware debuggers and software debuggers simultaneously isn't actually support by the architecture, requiring some discipline from the guy with the JTAG cable. Completely untested patch below, please let me know how you get on... Will --->8 diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c index ba386bd..f513b0e 100644 --- a/arch/arm/kernel/hw_breakpoint.c +++ b/arch/arm/kernel/hw_breakpoint.c @@ -322,14 +322,9 @@ int arch_install_hw_breakpoint(struct perf_event *bp) { struct arch_hw_breakpoint *info = counter_arch_bp(bp); struct perf_event **slot, **slots; - int i, max_slots, ctrl_base, val_base, ret = 0; + int i, max_slots, ctrl_base, val_base; u32 addr, ctrl; - /* Ensure that we are in monitor mode and halting mode is disabled. */ - ret = enable_monitor_mode(); - if (ret) - goto out; - addr = info->address; ctrl = encode_ctrl_reg(info->ctrl) | 0x1; @@ -356,10 +351,8 @@ int arch_install_hw_breakpoint(struct perf_event *bp) } } - if (WARN_ONCE(i == max_slots, "Can't find any breakpoint slot\n")) { - ret = -EBUSY; - goto out; - } + if (WARN_ONCE(i == max_slots, "Can't find any breakpoint slot\n")) + return -EBUSY; /* Override the breakpoint data with the step data. */ if (info->step_ctrl.enabled) { @@ -378,8 +371,7 @@ int arch_install_hw_breakpoint(struct perf_event *bp) /* Setup the control register. */ write_wb_reg(ctrl_base + i, ctrl); -out: - return ret; + return 0; } void arch_uninstall_hw_breakpoint(struct perf_event *bp) @@ -590,6 +582,10 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp) int ret = 0; u32 offset, alignment_mask = 0x3; + /* Ensure that we are in monitor mode and halting mode is disabled. */ + if (enable_monitor_mode()) + return -ENODEV; + /* Build the arch_hw_breakpoint. */ ret = arch_build_bp_info(bp); if (ret) @@ -935,8 +931,10 @@ static void reset_ctrl_regs(void *unused) isb(); reset_regs: - if (enable_monitor_mode()) + if (enable_monitor_mode()) { + pr_warning("CPU %d failed to enable monitor mode\n", cpu); return; + } /* We must also reset any reserved registers. */ raw_num_brps = get_num_brp_resources();