From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11386 invoked by alias); 27 Nov 2010 18:01:27 -0000 Received: (qmail 11378 invoked by uid 22791); 27 Nov 2010 18:01:26 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 27 Nov 2010 18:01:15 +0000 Received: (qmail 3230 invoked from network); 27 Nov 2010 18:01:14 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 27 Nov 2010 18:01:14 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [patch 2/3] Implement support for PowerPC BookE ranged watchpoints Date: Sat, 27 Nov 2010 18:01:00 -0000 User-Agent: KMail/1.13.5 (Linux/2.6.33-29-realtime; KDE/4.4.5; x86_64; ; ) Cc: Thiago Jung Bauermann , Jan Kratochvil , Joel Brobecker , Eli Zaretskii References: <1290549100.3164.47.camel@hactar> <1290806122.3009.37.camel@hactar> <201011271747.39053.pedro@codesourcery.com> In-Reply-To: <201011271747.39053.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201011271801.11710.pedro@codesourcery.com> 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/msg00470.txt.bz2 On Saturday 27 November 2010 17:47:38, Pedro Alves wrote: > On Friday 26 November 2010 21:15:22, Thiago Jung Bauermann wrote: > > > 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... > > Blame the hardware designers, not me. :-) > > > 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? > > Yes, that's what I think we should do. I can help you code this, > but I'm going computer-less real soon now until the 3rd December, > and I'll only be able to help/review changes then onwards. > > Here's what I think we should do in more detail: > > 1 - make ppc-linux-nat.c insert ranged watchpoints for len > 1. > > 2 - make ppc-linux-nat.c insert exact watchpoints when len == 1. BTW, optional, but something that the ppc watchpoint support is lacking. Step 2.5, ppc-linux-nat.c should be taught to keep a list of watchpoint requests, and merge watch requests for the same addresses. See for an example of what that would fix. > > with the above, watchpoints will behave as core gdb wants, > while we will still use only one register in the case that > is possible. > > 3 - add a new command to let the user select that gdb should > trust that primitive types are always accessed through their > address (== exact). > My opinion is that it should be off by default, on the > grounds that most users aren't and shouldn't be that > familiar with the debug support of the chip or target > they're using. Users want things to Just Work. > Every case that gdb doesn't behave the way it is > supposed to by default, and there's diverging behavior > in different target, is a case for frustration, bug > reports, and support requests. Another argument is that > simulators/emulators (think qemu, for example) may not > have this limitation, and can implement the RSP > watchpoint packets as gdb expects (always ranged). > > 4 - implementation wise, when the option added in #3 is > enabled, gdb (breakpoint.c) checks if the memory being watched > corresponds to a scalar type. (or, going a step > further, if wrapped in a structure or union or array, > that only contains a single scalar, recursively). If > true, then the "struct value" gdb used to record the > expression's current value (for comparison with the > new value whenever the low level watchpoint triggers) is > the same as usual, but, the watchpoint location that is associated > with the watchpoint is set to have length == 1, instead of > the whole width of the watched memory range. > This way, the target is requested to watch a single > byte at the scalar's address. Note how due to the > change done in #2, ppc-linux-nat.c will insert an > PPC_BREAKPOINT_MODE_EXACT in this case. > > This means that no changes to the target_insert_watchpoint > interface are required. This also means that no > changes to the remote protocol are required. If > the new option is on, and the user wants to watch > an "int i", whose address is 0xD3ADB33F, the target > will see a "Z2,D3ADB33F,1" request. That is, watch > writes to the 1 byte long range starting at D3ADB33F. > That is the same as saying trap only on writes to > D3ADB33F. > > This also means that no new breakpoint_ops type > is required either. A new flag in the breakpoint will do > (so that the "exact" property of watchpoints already > created is preserved if the user flips the switch). > > WDYT? Sounds reasonable? I'd prefer doing these changes > as first step. I haven't looked yet at what kind of masks > masked watchpoints support, but if they only support masks in the form > of (binary) 11110000, 1111111100, etc. (no alternating 1s and 0s), > then no target_insert_watchpoint or remote protocol change is > required either for those. -- Pedro Alves