From patchwork Wed Sep 21 16:17:13 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulrich Weigand X-Patchwork-Id: 4228 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 6046623EFD for ; Wed, 21 Sep 2011 16:17:17 +0000 (UTC) Received: from mail-fx0-f52.google.com (mail-fx0-f52.google.com [209.85.161.52]) by fiordland.canonical.com (Postfix) with ESMTP id 48F2CA18AD6 for ; Wed, 21 Sep 2011 16:17:17 +0000 (UTC) Received: by fxe23 with SMTP id 23so2544617fxe.11 for ; Wed, 21 Sep 2011 09:17:17 -0700 (PDT) Received: by 10.223.55.136 with SMTP id u8mr1328245fag.46.1316621836971; Wed, 21 Sep 2011 09:17:16 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.152.18.198 with SMTP id y6cs131867lad; Wed, 21 Sep 2011 09:17:16 -0700 (PDT) Received: by 10.216.26.203 with SMTP id c53mr690030wea.45.1316621836161; Wed, 21 Sep 2011 09:17:16 -0700 (PDT) Received: from mtagate7.uk.ibm.com (mtagate7.uk.ibm.com. [194.196.100.167]) by mx.google.com with ESMTPS id z21si4704563wec.66.2011.09.21.09.17.15 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 21 Sep 2011 09:17:16 -0700 (PDT) Received-SPF: pass (google.com: domain of uweigand@de.ibm.com designates 194.196.100.167 as permitted sender) client-ip=194.196.100.167; Authentication-Results: mx.google.com; spf=pass (google.com: domain of uweigand@de.ibm.com designates 194.196.100.167 as permitted sender) smtp.mail=uweigand@de.ibm.com Received: from d06nrmr1307.portsmouth.uk.ibm.com (d06nrmr1307.portsmouth.uk.ibm.com [9.149.38.129]) by mtagate7.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p8LGHFhV000720 for ; Wed, 21 Sep 2011 16:17:15 GMT Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by d06nrmr1307.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p8LGHFnY2375760 for ; Wed, 21 Sep 2011 17:17:15 +0100 Received: from d06av02.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p8LNHAWW030240 for ; Wed, 21 Sep 2011 17:17:10 -0600 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with SMTP id p8LNH9C6030183; Wed, 21 Sep 2011 17:17:09 -0600 Message-Id: <201109212317.p8LNH9C6030183@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Wed, 21 Sep 2011 18:17:13 +0200 Subject: [commit] Re: [rfc, gdbserver] Support hardware watchpoints on ARM To: pedro@codesourcery.com (Pedro Alves) Date: Wed, 21 Sep 2011 18:17:13 +0200 (CEST) From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, patches@linaro.org In-Reply-To: <201109211528.59723.pedro@codesourcery.com> from "Pedro Alves" at Sep 21, 2011 03:28:59 PM X-Mailer: ELM [version 2.5 PL2] MIME-Version: 1.0 Pedro Alves wrote: > On Wednesday 21 September 2011 15:26:30, Ulrich Weigand wrote: > > Pedro Alves wrote: > > > On Wednesday 21 September 2011 14:57:15, Ulrich Weigand wrote: > > > > Well, since this is global system property that is actually only > > > > queried once and then returned from a cache, adding a LWP argument > > > > would appear to be somewhat misleading ... > > > > > > We can always just document what the argument means :-) In this > > > case, it'd serve as currently stopped LWP to run ptrace on in case > > > the cache is not set yet. > > > > That still seems odd to me :-) I'd rather make the caching explicit, > > e.g. by retrieving the information once in arm_arch_setup and then > > just always using it. > > Works for me. :-) OK, so here's the patch I committed to address the two issues. Re-tested with no regressions on arm-linux-gnueabi. Bye, Ulrich ChangeLog: * linux-arm-low.c (struct arm_linux_hwbp_cap): Remove. (arm_linux_hwbp_cap): New static variable. (arm_linux_get_hwbp_cap): Replace by ... (arm_linux_init_hwbp_cap): ... this new function. (arm_linux_get_hw_breakpoint_count): Use arm_linux_hwbp_cap. (arm_linux_get_hw_watchpoint_count): Likewise. (arm_linux_get_hw_watchpoint_max_length): Likewise. (arm_arch_setup): Call arm_linux_init_hwbp_cap. (arm_prepare_to_resume): Use perror_with_name instead of error. Index: gdb/gdbserver/linux-arm-low.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/linux-arm-low.c,v retrieving revision 1.28 diff -u -p -r1.28 linux-arm-low.c --- gdb/gdbserver/linux-arm-low.c 21 Sep 2011 12:39:50 -0000 1.28 +++ gdb/gdbserver/linux-arm-low.c 21 Sep 2011 14:44:52 -0000 @@ -55,13 +55,13 @@ void init_registers_arm_with_neon (void) #endif /* Information describing the hardware breakpoint capabilities. */ -struct arm_linux_hwbp_cap +static struct { unsigned char arch; unsigned char max_wp_length; unsigned char wp_count; unsigned char bp_count; -}; +} arm_linux_hwbp_cap; /* Enum describing the different types of ARM hardware break-/watch-points. */ typedef enum @@ -341,72 +341,49 @@ ps_get_thread_area (const struct ps_proc } -/* Get hold of the Hardware Breakpoint information for the target we are - attached to. Returns NULL if the kernel doesn't support Hardware - breakpoints at all, or a pointer to the information structure. */ -static const struct arm_linux_hwbp_cap * -arm_linux_get_hwbp_cap (void) -{ - /* The info structure we return. */ - static struct arm_linux_hwbp_cap info; - - /* Is INFO in a good state? -1 means that no attempt has been made to - initialize INFO; 0 means an attempt has been made, but it failed; 1 - means INFO is in an initialized state. */ - static int available = -1; - - if (available == -1) - { - int pid = lwpid_of (get_thread_lwp (current_inferior)); - unsigned int val; +/* Query Hardware Breakpoint information for the target we are attached to + (using PID as ptrace argument) and set up arm_linux_hwbp_cap. */ +static void +arm_linux_init_hwbp_cap (int pid) +{ + unsigned int val; - if (ptrace (PTRACE_GETHBPREGS, pid, 0, &val) < 0) - available = 0; - else - { - info.arch = (unsigned char)((val >> 24) & 0xff); - info.max_wp_length = (unsigned char)((val >> 16) & 0xff); - info.wp_count = (unsigned char)((val >> 8) & 0xff); - info.bp_count = (unsigned char)(val & 0xff); - available = (info.arch != 0); - } + if (ptrace (PTRACE_GETHBPREGS, pid, 0, &val) < 0) + return; - if (available) - { - if (info.wp_count > MAX_WPTS) - internal_error (__FILE__, __LINE__, - "Unsupported number of watchpoints"); - if (info.bp_count > MAX_BPTS) - internal_error (__FILE__, __LINE__, - "Unsupported number of breakpoints"); - } - } + arm_linux_hwbp_cap.arch = (unsigned char)((val >> 24) & 0xff); + if (arm_linux_hwbp_cap.arch == 0) + return; - return available == 1 ? &info : NULL; + arm_linux_hwbp_cap.max_wp_length = (unsigned char)((val >> 16) & 0xff); + arm_linux_hwbp_cap.wp_count = (unsigned char)((val >> 8) & 0xff); + arm_linux_hwbp_cap.bp_count = (unsigned char)(val & 0xff); + + if (arm_linux_hwbp_cap.wp_count > MAX_WPTS) + internal_error (__FILE__, __LINE__, "Unsupported number of watchpoints"); + if (arm_linux_hwbp_cap.bp_count > MAX_BPTS) + internal_error (__FILE__, __LINE__, "Unsupported number of breakpoints"); } /* How many hardware breakpoints are available? */ static int arm_linux_get_hw_breakpoint_count (void) { - const struct arm_linux_hwbp_cap *cap = arm_linux_get_hwbp_cap (); - return cap != NULL ? cap->bp_count : 0; + return arm_linux_hwbp_cap.bp_count; } /* How many hardware watchpoints are available? */ static int arm_linux_get_hw_watchpoint_count (void) { - const struct arm_linux_hwbp_cap *cap = arm_linux_get_hwbp_cap (); - return cap != NULL ? cap->wp_count : 0; + return arm_linux_hwbp_cap.wp_count; } /* Maximum length of area watched by hardware watchpoint. */ static int arm_linux_get_hw_watchpoint_max_length (void) { - const struct arm_linux_hwbp_cap *cap = arm_linux_get_hwbp_cap (); - return cap != NULL ? cap->max_wp_length : 0; + return arm_linux_hwbp_cap.max_wp_length; } /* Initialize an ARM hardware break-/watch-point control register value. @@ -735,12 +712,12 @@ arm_prepare_to_resume (struct lwp_info * if (arm_hwbp_control_is_enabled (proc_info->bpts[i].control)) if (ptrace (PTRACE_SETHBPREGS, pid, ((i << 1) + 1), &proc_info->bpts[i].address) < 0) - error (_("Unexpected error setting breakpoint address")); + perror_with_name ("Unexpected error setting breakpoint address"); if (arm_hwbp_control_is_initialized (proc_info->bpts[i].control)) if (ptrace (PTRACE_SETHBPREGS, pid, ((i << 1) + 2), &proc_info->bpts[i].control) < 0) - error (_("Unexpected error setting breakpoint")); + perror_with_name ("Unexpected error setting breakpoint"); lwp_info->bpts_changed[i] = 0; } @@ -753,12 +730,12 @@ arm_prepare_to_resume (struct lwp_info * if (arm_hwbp_control_is_enabled (proc_info->wpts[i].control)) if (ptrace (PTRACE_SETHBPREGS, pid, -((i << 1) + 1), &proc_info->wpts[i].address) < 0) - error (_("Unexpected error setting watchpoint address")); + perror_with_name ("Unexpected error setting watchpoint address"); if (arm_hwbp_control_is_initialized (proc_info->wpts[i].control)) if (ptrace (PTRACE_SETHBPREGS, pid, -((i << 1) + 2), &proc_info->wpts[i].control) < 0) - error (_("Unexpected error setting watchpoint")); + perror_with_name ("Unexpected error setting watchpoint"); lwp_info->wpts_changed[i] = 0; } @@ -790,6 +767,11 @@ arm_get_hwcap (unsigned long *valp) static void arm_arch_setup (void) { + int pid = lwpid_of (get_thread_lwp (current_inferior)); + + /* Query hardware watchpoint/breakpoint capabilities. */ + arm_linux_init_hwbp_cap (pid); + arm_hwcap = 0; if (arm_get_hwcap (&arm_hwcap) == 0) { @@ -805,7 +787,6 @@ arm_arch_setup (void) if (arm_hwcap & HWCAP_VFP) { - int pid; char *buf; /* NEON implies either no VFP, or VFPv3-D32. We only support @@ -819,7 +800,6 @@ arm_arch_setup (void) /* Now make sure that the kernel supports reading these registers. Support was added in 2.6.30. */ - pid = lwpid_of (get_thread_lwp (current_inferior)); errno = 0; buf = xmalloc (32 * 8 + 4); if (ptrace (PTRACE_GETVFPREGS, pid, 0, buf) < 0