Re: Adding MIPS registers (was Re: [PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL)

[James Hogan <james dot hogan at imgtec dot com>]

Hi Maciej,

On Tue, Sep 09, 2014 at 06:56:51PM +0100, Maciej W. Rozycki wrote:
> > >  BTW, I've had some FR=1 stuff ongoing too, though regrettably stalled 
> > > recently, see:
> > > 
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=7518
> > > http://sourceware.org/ml/gdb-patches/2012-06/msg00201.html
> > > 
> > > -- based on an old patch from MIPS Technologies that wasn't quite there, 
> > > but still was a good starting point.  Bare iron only of course as there 
> > > was no FR=1 Linux ABI up until recently.  It will be interesting to see 
> > > how the two pieces compare.  Or actually it may make sense if I push my 
> > > piece first, there was just some concern about regcache that remained
> > > unresolved, so I'll see if I can reintegrate that change against current
> > > trunk, retest and repost.
> > 
> > Yes, that's what I've based my work on, but with some changes since it
> > needs to support FRE=1 too.
> 
>  Hmm, I think this stuff would be best made incrementally anyway.  We have 
> a bug to fix here and a new feature.  I'd rather kept changes to address 
> them separate.

Yes, it's all separated from your patch at the moment, but it does
change the fp_register_size to an fp_mode enum for FRE=1.

> > I've added a number of new MIPS registers, but I'm keen to get guidance
> > about gdbserver ABI issues. Perhaps you or somebody else could comment.
> > 
> > The main register changes are:
> > * the MIPS32 fp registers are sometimes extended to 64-bits, at least
> > when FR=1 (at the moment I've made gdbserver extend to 64-bits whenever
> > FIR.F64=1, so that the register format cannot change during use).
> 
>  One issue here is FR can now be changed at the run time,

Yes, the cooked registers still change appropriately depending on the FR
bit thanks to your code ("info float" shows different widths before and
after a mode change etc.), but I had trouble getting gdb to reload the
XML from gdbserver on a change, and it felt wrong for the register
layout to change under GDB's feet. It would presumably only be able to
notice the change by ensuring that both register formats have the status
register at the same location. Not to say it couldn't be made to work of
course.

> with the UFR 
> feature, so the register set presented will have to change dynamically, 
> according to that setting even for Linux targets.

I have a feeling that UFR is currently unlikely to be enabled in Linux,
due to subtleties with mode switches in multi-threaded processes
requiring kernel support. Matthew or Paul (on CC) can probably confirm
that.

> I have addressed this 
> to some extent for bare iron, but with Linux that means `gdbserver' 
> changes as well (as you noted).  I think raw registers should reflect 
> precisely the state of hardware and all the mapping based on FR should be 
> done for cooked registers only.

Do you mean having the size of the raw registers depend on FIR.F64, or
FR?

> > * expose CP0 Config5 as a new register (contains the FRE bit, only
> > really relevant when FR=1)
> 
>  I smell ABI problems here (think core files and maybe signal frames), 

It is planned for signal frames to extend sc_used_math with a couple
more bits, USED_FR and USED_HYBRID (i.e. FRE), although I can't think of
a reasonable use case for mode changing inside a signal handler (dlopen
in a signal handler?).

If core files have the Linux register sets in the notes sections and the
Config5 is added to the appropriate register sets (PRFPREG/NT_MIPS_MSA),
then it should be doable I think. Core files are on the list of things
to revisit though since I know for a fact that a gdb generated core file
doesn't do the right thing yet. Linux also unfortunately had some
breakage of core file formats between v3.13 and v3.17-rc1.

> what is this bit for

It is a sort of hybrid FR=1/FR=0 compatibility mode to allow MSA code
(which implies FR=1) to be combined with fp32 code. Even/odd singles are
in the lower/upper half of the even double, but because FR=1 the odd
double is also accessible. There's also a UFRE bit that can be enabled
(but not in Linux).

I believe in practise it causes all use of (at least) odd singles to
trap so they can be fixed up (compatibility is the main issue rather
than performance).

Matthew: Is that accurate?

> and why was it added to CP0.Config5 rather than 
> CP0.Status?

I'm not keen on it being in Config5 either. It seems they're running out
of bits in the status register. FWIW, the Config5 I added to the Linux
ptrace regsets only expose that one FRE bit at the moment.

> Is this stuff publicly documented anywhere? -- I don't see it 
> in documentation I have handy, not even for rev. 6 of the architecture, 
> let alone 5.

I'll check for documentation tomorrow. It's a pretty new bit I think,
but I thought it was in r6.

> > * (separately) expose the rest of the MSA vector registers as new
> > registers (raw register excludes the low 64-bits which alias the double
> > FP registers, pseudo register concatenates raw fp/vector register together).
> 
>  Hmm, offhand I think it would be cleaner done the other way round -- 
> expose raw 128-bit or 256-bit MSA registers as applicable and let the 
> cooked layer map parts to legacy FP or MSA registers as appropriate.  For 
> non-MSA cores keep exposing FP registers as usually.  Do you have a 
> specific reason why you did it like you did?

I toyed around with several approaches, starting with that one, but now
I've got my head around it it shouldn't be too hard to change. I was
originally concerned about compatibility with the remote protocol when
XML target description isn't available, but it sounds like that is
actually less of an issue than I thought, so I'll probably try this
again.

Note that it still needs a separate set of cooked vector register
numbers so that printing $f0/$w0 display in the appropriate way.

> > Currently I've done this by adding whole new target descriptions:
> > mips-fpu64-dsp-linux (mips-dsp-linux with 64-bit fp and config5)
> > mips-fpu64-linux (mips-linux with 64-bit fp and config5)
> > mips-msa-linux (64-bit fp registers, config5, vector registers)
> > mips64-msa-linux
> > 
> > and carefully appending Config5 to the following target descriptions (is
> > that a valid approach, adding registers to the end?):
> > mips64-dsp-linux
> > mips64-linux
> 
>  That doesn't matter I believe, the description has to be interpreted as a 
> whole by GDB anyway.  Registers that don't have a special interpretation 
> in GDB will simply be passed through and available by their names in the 
> usual contexts.  That is at least how this stuff is meant to be working 
> I'm told.
> 
>  Shouldn't e.g. mips-linux have a variant with CP0.Config5 too?

It could, but the FRE bit is only intended for use with FR=1, so it
would probably be pointless unless Config5 later gets some other
important bits that are needed when FR=0.

>  Note that inevitably you'll be able to change FR from GDB now too, just 
> by poking at it, e.g.:
> 
> (gdb) set $sr ^= 1 << 26
> 
> The ptrace(2) code in the kernel needs to recognise CP0.Config5.UFR being 
> set and allow r/w access to CP0.Status.FR with PTRACE_POKEUSER or 
> PTRACE_SETREGS, just as with the CTC1 moves to CP1.UFR and CP1.UNFR, and 
> in response to that you need to switch the target description to the new 
> register layout instantaneously.  Same with e.g. single-stepping over 
> these instructions.
> 
> > With a GDB & target that supports XML I believe all should be well.
> > However with a MIPS32 remote which supports FR=1 (i.e. when FIR.F64=1),
> > AFAICT GDB without XML target description support has no (simple) way to
> > tell whether the remote is actually providing 32-bit or 64-bit fp registers.
> 
>  IMO you don't have to be concerned with stone-age versions of GDB, our 
> intent is to support old remote stubs with current GDB (stubs are not 
> always easy to upgrade and sometimes not really possible, if built into 
> firmware for example), but not the other way round.  If you've got modern 
> `gdbserver', then just use GDB you've built along it.

Good to know, that's what I've been assuming (I was more concerned with
current GDB and old stub vs new stub...)

> > So, for MIPS do we care about GDB being built without XML support (at
> > least for gdbserver)? I.e. creation of whole new target descriptions
> > that a non-xml gdb won't recognise.
> 
>  No, as far as I'm concerned.

... Good :-)

>  BTW, while making such changes you need to verify the correct operation 
> of native GDB too.

Yes, I've been doing native/remote side by side (but didn't mention
native since I didn't have the same compatibility concerns).

Thanks a lot,

James