From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10545 invoked by alias); 21 Sep 2011 13:57:39 -0000 Received: (qmail 10534 invoked by uid 22791); 21 Sep 2011 13:57:37 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate7.uk.ibm.com (HELO mtagate7.uk.ibm.com) (194.196.100.167) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 21 Sep 2011 13:57:19 +0000 Received: from d06nrmr1806.portsmouth.uk.ibm.com (d06nrmr1806.portsmouth.uk.ibm.com [9.149.39.193]) by mtagate7.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p8LDvH5p008442 for ; Wed, 21 Sep 2011 13:57:17 GMT Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by d06nrmr1806.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p8LDvHsP2437278 for ; Wed, 21 Sep 2011 14:57:17 +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 p8LKvBPQ019733 for ; Wed, 21 Sep 2011 14:57:12 -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 p8LKvAjb019685; Wed, 21 Sep 2011 14:57:10 -0600 Message-Id: <201109212057.p8LKvAjb019685@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Wed, 21 Sep 2011 15:57:15 +0200 Subject: Re: [rfc, gdbserver] Support hardware watchpoints on ARM To: pedro@codesourcery.com (Pedro Alves) Date: Wed, 21 Sep 2011 14:20:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, patches@linaro.org In-Reply-To: <201109211429.31335.pedro@codesourcery.com> from "Pedro Alves" at Sep 21, 2011 02:29:31 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-09/txt/msg00383.txt.bz2 Pedro Alves wrote: > I was just looking over the patch before lunch, and > meanwhile you've committed it. :-) It looks fine to me in any > case. :-) I just had a couple minor remarks. Oops, sorry. Thanks for looking over it! > On Monday 12 September 2011 18:23:00, Ulrich Weigand wrote: > > + if (hwbp_type == arm_hwbp_break) > > + { > > + /* For breakpoints, the length field encodes the mode. */ > > + switch (len) > > + { > > + case 2: /* 16-bit Thumb mode breakpoint */ > > + case 3: /* 32-bit Thumb mode breakpoint */ > > + mask = 0x3 << (addr & 2); > > + break; > > + case 4: /* 32-bit ARM mode breakpoint */ > > + mask = 0xf; > > + break; > > + default: > > + /* Unsupported. */ > > + return -1; > > + } > > + > > + addr &= ~3; > > Is this ~3 correct for 16-bit Thumb? Yes, it is. The address value must always have its two low bits clear. For Thumb, the selection of which of the two halfwords the breakpoint is to apply to is done via control bits (that's what the "mask" value is about). > > +static void > > +arm_prepare_to_resume (struct lwp_info *lwp) > > +{ > > + int pid = lwpid_of (lwp); > > + struct process_info *proc = find_process_pid (pid_of (lwp)); > > + struct arch_process_info *proc_info = proc->private->arch_private; > > + struct arch_lwp_info *lwp_info = lwp->arch_private; > > + int i; > > + > > + for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++) > > It's a bit unfortunate that arm_linux_get_hw_breakpoint_count > relies on the current_inferior global having been set to LWP by > the callers. We try to avoid that when we have an LWP handy. > Can we make arm_linux_get_hw_breakpoint_count take an LWP argument? 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 ... (In this particular routine, we actually don't really have to query the exact hardware count. We might just as well do the loop all the way through MAX_BPTS ...) > On Monday 12 September 2011 18:23:00, Ulrich Weigand wrote: > > + 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")); > > + > > + 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")); > > I think perror_with_name would be better, so we can see on the logs > the errno ptrace set on error. Agreed, I'll change this. Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com