From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23720 invoked by alias); 26 Nov 2010 21:15:40 -0000 Received: (qmail 23707 invoked by uid 22791); 26 Nov 2010 21:15:38 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from e24smtp03.br.ibm.com (HELO e24smtp03.br.ibm.com) (32.104.18.24) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 26 Nov 2010 21:15:30 +0000 Received: from d24relay01.br.ibm.com (d24relay01.br.ibm.com [9.8.31.16]) by e24smtp03.br.ibm.com (8.14.4/8.13.1) with ESMTP id oAQL6jKi000509 for ; Fri, 26 Nov 2010 19:06:45 -0200 Received: from d24av05.br.ibm.com (d24av05.br.ibm.com [9.18.232.44]) by d24relay01.br.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id oAQLE7a52646026 for ; Fri, 26 Nov 2010 18:14:07 -0300 Received: from d24av05.br.ibm.com (loopback [127.0.0.1]) by d24av05.br.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id oAQLFPOg005473 for ; Fri, 26 Nov 2010 19:15:25 -0200 Received: from [9.18.199.242] ([9.18.199.242]) by d24av05.br.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id oAQLFObt005469; Fri, 26 Nov 2010 19:15:24 -0200 Subject: Re: [patch 2/3] Implement support for PowerPC BookE ranged watchpoints From: Thiago Jung Bauermann To: Pedro Alves Cc: gdb-patches@sourceware.org, Jan Kratochvil , Joel Brobecker , Eli Zaretskii In-Reply-To: <201011251731.58135.pedro@codesourcery.com> References: <1290549100.3164.47.camel@hactar> <201011232316.56539.pedro@codesourcery.com> <1290632755.3164.94.camel@hactar> <201011251731.58135.pedro@codesourcery.com> Content-Type: text/plain; charset="UTF-8" Date: Fri, 26 Nov 2010 21:15:00 -0000 Message-ID: <1290806122.3009.37.camel@hactar> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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: 2010-11/txt/msg00461.txt.bz2 On Thu, 2010-11-25 at 17:31 +0000, Pedro Alves wrote: > On Wednesday 24 November 2010 21:05:55, Thiago Jung Bauermann wrote: > > On Tue, 2010-11-23 at 23:16 +0000, Pedro Alves wrote: > > > On Tuesday 23 November 2010 21:51:40, Thiago Jung Bauermann wrote: > That patch helped in the array case (maybe it was even a regression > that was fixed?), but even the structure case, gdb always behaved > the "ranged" way, AFAIK. E.g., if you "(gdb) watch" an int, assuming > 32-bit, gdb is supposed to be watching the range of 4 bytes starting > from the int's address. And watching "int i;" is supposed to be > the same as watching "struct {int i;} i"; Is there a difference between "int i" and "struct {int i;} i" besides having to access the integer using "i.i" instead of "i"? But I see your point, and agree. > > But also, the target needs to > > know that it's being asked to watch a region which is expected to have > > accesses in the middle of it. This is because of the difference of how > > hardware watchpoints work in server and embedded PowerPC processors. In > > the former, a hardware watchpoint triggers when any byte within an 8 > > byte window starting at the given address is accessed. In the latter, > > only accesses at the given address trigger the watchpoint. > > Hmm.. Oh? Without knowing a think about BookE, that is not what I'd > infer from: > > static int > ppc_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) > { > /* Handle sub-8-byte quantities. */ > if (len <= 0) > return 0; > > /* The new BookE ptrace interface tells if there are alignment restrictions > for watchpoints in the processors. In that case, we use that information > to determine the hardcoded watchable region for watchpoints. */ > if (have_ptrace_booke_interface ()) > { > if (booke_debug_info.data_bp_alignment > && (addr + len > (addr & ~(booke_debug_info.data_bp_alignment - 1)) > + booke_debug_info.data_bp_alignment)) data_bp_alignment tells GDB that the processor only accepts watchpoints at certain address intervals. This code assumes that if that is the case, then the watchpoint triggers with any access within the interval. This covers server processors. For BookE processors, data_bp_alignment should be 0 (since they can put a watchpoint at any address) and the function will return 1 regardless of len. Perhaps I should check for data_bp_alignment > 1 since 1 could also express that a watchpoint can be put anywere. > return 0; > } > /* addr+len must fall in the 8 byte watchable region for DABR-based > processors (i.e., server processors). Without the new BookE ptrace > interface, DAC-based processors (i.e., embedded processors) will use > addresses aligned to 4-bytes due to the way the read/write flags are > passed in the old ptrace interface. */ > else if (((ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE) > && (addr + len) > (addr & ~3) + 4) > || (addr + len) > (addr & ~7) + 8) > return 0; This code just preserves GDB behaviour when the new BookE ptrace interface is not available. I agree it's buggy but I didn't think it was a problem to address in this patch series (also, not sure how to fix it now that you mention that GDB expects watchpoints to be ranged). > > So for an embedded powerpc processor, it's not enough to know that it > > should watch a 4 byte region at a given address. If it represents an > > integer and thus it can expect accesses only at that address then a > > regular watchpoint is enough. But if it's an array of four chars, then > > it needs two watchpoint registers to set up a hardware watchpoint... > > It all looks to me that the "ranged" notion is backwards. > The default watchpoint that GDB expects to handle _is_ ranged. > E.g., that's how it works on x86, and DABR based PPC. > > That's why this: > > +* GDB now supports ranged watchpoints, which stop the inferior when it > + accesses any address within a specified memory range. > > Is just confusing, because this is not a new GDB feature. So I should be more specific and say: "GDB now supports hardware accelerated watchpoints which cover a memory region on embedded PowerPC processors running the Linux kernel." WDYT? > BTW, it's not only useful to watch the whole range of a > structure or arrays. It's often useful to watch what random > code is clobbering an int or pointer. The bad write may well be > to the middle of the integer or pointer. I bet that > users end up surprised if PPC doesn't catch that with > gdb's "watch" command. Good point. > > So that's way I created a target_insert_ranged_watchpoint. The other > > option would be to add a flag to target_insert_watchpoint... > > It appears to me that if there should be a new kind of way to > insert watchpoints, it should be to allow setting watchpoints > that only work if the accesses are aligned, that is to support > the behaviour you describe with PPC_BREAKPOINT_MODE_EXACT > in embedded processors. So in your opinion, the watch command should always use two watchpoint registers to set up a ranged watchpoint in BookE ppc? I'm a bit reluctant to use all the watchpoint registers to set up one watchpoint... Then comes the question of how to support the PPC_BREAKPOINT_MODE_EXACT behaviour... A new flag or command to let the user "opt in" to a less capable watchpoint, but one which uses less hardware resources? -- []'s Thiago Jung Bauermann IBM Linux Technology Center