From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4608 invoked by alias); 26 Mar 2008 17:59:51 -0000 Received: (qmail 4597 invoked by uid 22791); 26 Mar 2008 17:59:50 -0000 X-Spam-Check-By: sourceware.org Received: from NaN.false.org (HELO nan.false.org) (208.75.86.248) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 26 Mar 2008 17:59:30 +0000 Received: from nan.false.org (localhost [127.0.0.1]) by nan.false.org (Postfix) with ESMTP id 25473983A1; Wed, 26 Mar 2008 17:59:28 +0000 (GMT) Received: from caradoc.them.org (22.svnf5.xdsl.nauticom.net [209.195.183.55]) by nan.false.org (Postfix) with ESMTP id A10F598140; Wed, 26 Mar 2008 17:59:27 +0000 (GMT) Received: from drow by caradoc.them.org with local (Exim 4.69) (envelope-from ) id 1JeZuI-0005rs-Qd; Wed, 26 Mar 2008 13:59:26 -0400 Date: Wed, 26 Mar 2008 17:59:00 -0000 From: Daniel Jacobowitz To: "Maciej W. Rozycki" Cc: gdb-patches@sourceware.org, Chris Dearman , "Maciej W. Rozycki" Subject: Re: MIPS: Handle the DSP registers Message-ID: <20080326175926.GA10127@caradoc.them.org> Mail-Followup-To: "Maciej W. Rozycki" , gdb-patches@sourceware.org, Chris Dearman , "Maciej W. Rozycki" References: <20071218004304.GA29420@caradoc.them.org> <20080321184329.GO25307@caradoc.them.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17 (2007-12-11) 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: 2008-03/txt/msg00405.txt.bz2 On Wed, Mar 26, 2008 at 04:58:28PM +0000, Maciej W. Rozycki wrote: > Well, I am actually unsure whether I really *need* it (which probably > means I do not), but it does not look like a good engineering practice to > have this initial meta-state. I suppose no description should be > available before a target has been connected to or a native debuggee > process started so that it is seen when some code somewhere tries to > access this effectively inexistent state. There will be no description unless the architecture supplies a default one. The initialization sequence does not look like it would if I'd been writing a debugger from scratch, I admit. It was very tangled up but this is the best compromise I could come up with. > > ...this. The register numbers don't have to be different. You've > > added a new internal register numbering for Linux; that isn't > > necessary. Where are Linux and bare metal different? Is it in an > > existing stub, and if so, what registers does that stub report? > > DSP registers were added pretty late in the game when CP0 registers had > already been defined for bare iron, so there is no way to have the same > numbers for Linux and bare iron (unless we want to have a hole in the > Linux register space). They were added at the end of the already defined > ranges, respectively which is I think reasonable. And these numbers have > to match ptrace() and the existing remote debug stubs (e.g. YAMON), don't > they? > > > GDB internal register numbers are used for two purposes. They are the > > default for the remote P/p and T packet numbering, and the g/G packet > > ordering. If a target description describes the registers, > > tdesc_remote_register_number is used instead. And they are passed to > > GDB functions and determine the register cache layout. This is > > internal; just be consistent. > > Obviously the numbers returned by tdesc_remote_register_number() are > purely internal, but the "legacy" ones are seen externally as mentioned > above; or am I getting confused here? You've got this backwards. The numbers _taken_ by tdesc_remote_register_number are internal. The numbers returned by it are the external interface with a remote stub. If the remote stub implements the new registers and p/P, or returns the new registers in T responses (both reasonably unlikely) then the numbers matter. If it only implements the new registers and g/G, the numbers do not matter but the ordering of non-zero-sized registers determines the layout of the g/G packet. Ptrace isn't affected by these numbers at all; translation is done by mips_linux_register_addr for instance. If there are no existing remote stubs (ignoring gdbserver, here, newer versions of gdbserver will use XML) that transmit the new registers, then there's no need to give them any particular numbers. If there are existing stubs you want to preserve, then we can add a translation method if you can tell us what the actual register layout you want is. > > The regcache.c/regcache.h bits are now unused; they were for your > > inf-ptrace.c change. > > The MDI change still relies on these -- I'll migrate them there, thanks > for spotting. > > Hmm, actually I have just spotted I have already committed a build-fix > change to remote-mips.c that I submitted a while ago that makes the file > rely on the function; still it is not built without the MDI change, so I > propose not to take them out and wait for MDI to get in. Does the MDI patch rely on passing "1" to the new function? If not, please just use regcache_invalidate. Speaking about good engineering design, a function which marks cached data as valid without putting in any data is pretty suspicious. > The kernel is wrong -- with MIPS64 processors additional HIx/LOx > registers are 64-bit as the traditional HI/LO ones (which, if you notice > instruction encoding, could as well be called HI0/LO0 now) -- all the four > provide the same semantics. The control register is 32-bit-wide in all > cases. Then an N32 GDB is not going to be able to read these registers unless we create a regset for them, FYI. ptrace takes and returns longs on n32, not long longs. I experimented with using long long, but it was a terrible mess. > It just looked wrong to me to depend on a preprocessor macro where we > strive to make GDB multi-platform. Your point is of course valid, but > after a little thought I believe my change is right anyway. Given a > 64-bit kernel and an (n)64 or n32 GDB binary you may want to debug an o32 > executable; I have not investigated how ptrace() is meant to work in such > a scenario and I do not insist on getting rid of the macro dependency with > this patch, but this is something to be kept in mind in my opinion. That exact requirement is why it's written the way it is. If GDB is 64-bit, then we can always read 64-bit registers from the target; the pseudo ABI registers will be the right size for o32. > These are the "ACcumulator eXtension" registers as defined by the > SmartMIPS ASE. One is added per each HI/LO pair; though to the best of my > knowledge nobody has manufactured a MIPS core with the DSP and SmartMIPS > ASEs both included at the same time, hence only a single ACX register (for > the traditional HI/LO pair) has ever been in implemented in existing > hardware. I see, thanks. > I have noticed I missed register format descriptions (what are they for > anyway? -- that don't seem to be documented in the obvious places) and They're for gdbserver; they define its register cache. If I wrote the missing XML files for all existing gdbserver targets, I could replace them with pregenerated C files using xsltproc (just like I did for the ones currently in features/*.c); I haven't gotten round to it. > fixed a couple of issues that I discovered in testing too. Here is a new > version. As native regression testing takes quite a while, I will do this > once we agree on a reasonable candidate for commission. Sounds fine. If you'll forgive me, I'll wait to look at the patch until we resolve the issues above. -- Daniel Jacobowitz CodeSourcery