Re: MIPS: Handle the DSP registers

["Maciej W. Rozycki" <macro@mips.com>]

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