From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28218 invoked by alias); 27 Mar 2008 17:13:50 -0000 Received: (qmail 28190 invoked by uid 22791); 27 Mar 2008 17:13:45 -0000 X-Spam-Check-By: sourceware.org Received: from dmz.mips-uk.com (HELO dmz.mips-uk.com) (194.74.144.194) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 27 Mar 2008 17:13:16 +0000 Received: from internal-mx1 ([192.168.192.240] helo=ukservices1.mips.com) by dmz.mips-uk.com with esmtp (Exim 3.35 #1 (Debian)) id 1Jevf5-00012R-00; Thu, 27 Mar 2008 17:13:11 +0000 Received: from perivale.mips.com ([192.168.192.200]) by ukservices1.mips.com with esmtp (Exim 3.36 #1 (Debian)) id 1Jevex-0006Tw-00; Thu, 27 Mar 2008 17:13:03 +0000 Received: from macro (helo=localhost) by perivale.mips.com with local-esmtp (Exim 4.63) (envelope-from ) id 1Jevex-0002r5-Ix; Thu, 27 Mar 2008 17:13:03 +0000 Date: Thu, 27 Mar 2008 17:13:00 -0000 From: "Maciej W. Rozycki" To: Daniel Jacobowitz cc: gdb-patches@sourceware.org, Chris Dearman , "Maciej W. Rozycki" Subject: Re: MIPS: Handle the DSP registers In-Reply-To: <20080326175926.GA10127@caradoc.them.org> Message-ID: References: <20071218004304.GA29420@caradoc.them.org> <20080321184329.GO25307@caradoc.them.org> <20080326175926.GA10127@caradoc.them.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-MIPS-Technologies-UK-MailScanner: Found to be clean X-MIPS-Technologies-UK-MailScanner-From: macro@mips.com 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/msg00442.txt.bz2 On Wed, 26 Mar 2008, Daniel Jacobowitz wrote: > 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. Ah, I see, indeed -- I got confused at one point. So the numbers in XML files are externally visible and these from the enum containing MIPS_ZERO_REGNUM, etc. are internal. Which means you are right they can be assigned randomly (so your choice is certainly not worse than mine but it's densier, which is an advantage here); though some other bits which predate target descriptions in my patch set depend on these numbers for remote targets, so again -- I will put that dependency to where the dependee is (1). > Ptrace isn't affected by these numbers at all; translation is done > by mips_linux_register_addr for instance. Indeed -- I was too fast writing this. > 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. As far as I know it both YAMON and the SDE library debugging stub both provide access to cp0 registers. I may not have time to check whether they use the problematic packets, but with the code shuffle above (1) this becomes a non-issue for this lone change. > Does the MDI patch rely on passing "1" to the new function? If not, > please just use regcache_invalidate. Speaking about good engineering Well, it passes "-1", so it is reasonable and not exactly the same as regcache_invalidate(). > design, a function which marks cached data as valid without putting > in any data is pretty suspicious. Indeed -- another patch in the set adds: regcache_set_valid_p (regcache, cookednum, regcache_valid_p (regcache, rawnum)); at the botton of mips_pseudo_register_read() too, which is a place, I believe, the scenario you are referring to may actually happen; it used to for sure. Similarly there used to be another possibility for this to trigger in code we have to access registers through a frame. > 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. Hmm, I have had a peek at the kernel and it looks like PTRACE_POKEUSR and PTRACE_PEEKUSR requests are completely broken for n32; it's just that we have a way around it for GP/FP registers... > > 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. Fair enough, though still hackish in my opinion. ;-) > > 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. Well, certainly fixing ptrace() to work with DSP registers one way or another for n32 can wait until we have a relevant piece of silicon and I'll sort out the numbering issue as referred to above (1). Please let me know whether there is anything else you have had in mind and I'll send a new version of the patch shortly. Maciej