[02/24] Add MIPS32 FPU64 GDB target descriptions

  Add a couple of new target descriptions for MIPS32 targets with 64-bit
    floating point registers, with and without DSP.

    These are identical to their 32-bit FPU counterparts except that the FP
    registers are 64-bits long to allow for debugging of F64=1 MIPS32 targets
    such as MIPS32r2 compatible cores, and they include the Config5 CP0
    register which has an FRE bit.

    F64 targets have an FR bit in the status register to specify the effective
    register size, and the FRE bit is used with FR=1 to enable a compatibility
    mode which is similar to FR=0 but with usable odd doubles which don't
    alias with any other FP registers.

    gdb/ChangeLog:

    	* features/Makefile (WHICH): Add mips-fpu64-linux and
    	mips-fpu64-dsp-linux and Sort microblaze out of the way
        of mips/mips64
    	(mips-fpu64-expedite): Set.
    	(mips-fpu64-dsp-expedite): Set.
    	* features/mips-fpu64-dsp-linux.xml: New file.
    	* features/mips-fpu64-linux.xml: New file.
    	* features/mips-fpu64.xml: New file.
    	* features/mips-fpu64-dsp-linux.c: New generated file.
    	* features/mips-fpu64-linux.c: New generated file.
    	* regformats/mips-fpu64-dsp-linux.dat: New generated file.
    	* regformats/mips-fpu64-linux.dat: New generated file.

    gdb/gdbserver/ChangeLog:

    	* Makefile.in (clean): Delete mips-fpu64-linux.c and
    	mips-fpu64-dsp-linux.c.
    	(mips-fpu64-linux.c): New rule.
    	(mips-fpu64-dsp-linux.c): New rule.
    	* configure.srv (srv_regobj): Add mips-fpu64-linux.o and
    	mips-fpu64-dsp-linux.o.
    	(srv_xmlfiles): Add mips-fpu64-linux.xml,
    	mips-fpu64-dsp-linux.xml, mips-cp0-fpu64.xml and mips-fpu64.xml.
---
 gdb/features/Makefile                   |   6 +-
 gdb/features/mips-fpu64-dsp-linux.c     | 111 ++++++++++++++++++++++++++++++++
 gdb/features/mips-fpu64-dsp-linux.xml   |  20 ++++++
 gdb/features/mips-fpu64-linux.c         | 102 +++++++++++++++++++++++++++++
 gdb/features/mips-fpu64-linux.xml       |  19 ++++++
 gdb/features/mips-fpu64.xml             |  45 +++++++++++++
 gdb/gdbserver/Makefile.in               |   7 +-
 gdb/gdbserver/configure.srv             |   5 ++
 gdb/regformats/mips-fpu64-dsp-linux.dat |  85 ++++++++++++++++++++++++
 gdb/regformats/mips-fpu64-linux.dat     |  78 ++++++++++++++++++++++
 10 files changed, 475 insertions(+), 3 deletions(-)
 create mode 100644 gdb/features/mips-fpu64-dsp-linux.c
 create mode 100644 gdb/features/mips-fpu64-dsp-linux.xml
 create mode 100644 gdb/features/mips-fpu64-linux.c
 create mode 100644 gdb/features/mips-fpu64-linux.xml
 create mode 100644 gdb/features/mips-fpu64.xml
 create mode 100644 gdb/regformats/mips-fpu64-dsp-linux.dat
 create mode 100644 gdb/regformats/mips-fpu64-linux.dat
  

On Mon, 27 Jun 2016, Bhushan Attarde wrote:

>     Add a couple of new target descriptions for MIPS32 targets with 64-bit
>     floating point registers, with and without DSP.
> 
>     These are identical to their 32-bit FPU counterparts except that the FP
>     registers are 64-bits long to allow for debugging of F64=1 MIPS32 targets
>     such as MIPS32r2 compatible cores, and they include the Config5 CP0
>     register which has an FRE bit.

 Hmm, has Linux kernel support for CP0.Config5 accesses gone upstream 
already?  Can you give me an upstream commit ID and/or reference to the 
discussion where it has been approved if so?

 More importantly, what do we need CP0.Config5 access for in the first 
place?  It looks to me like this bit is irrelevant to GDB as it does not 
affect the native (raw) register format.  So the only use would be to let 
the user running a debugging session switch between the FRE and NFRE modes 
without the need to poke at CP1C.FRE or CP1C.NFRE registers with a CTC1 
instruction, which by itself makes sense to me, but needs a further 
consideration.

 Additionally exposing CP0.Config5 may have security implications, 
especially as parts of the register have not been defined yet in the 
architectures and we'd have to force architecture maintainers somehow to 
ask us every time they intend to add a bit to this register to check if 
this has security implications and has to be avoided and/or explicitly 
handled in software.

 So overall it looks to me like we should avoid it.  What we ought to do 
instead is -- since we need to extend structures anyway -- I think adding 
all the remaining CP1C registers.  This would cover the earlier CP1C.UFR 
and CP1C.UNFR registers as well as any future additions, which would be 
further qualified with CP1.FIR bits to remove unimplemented registers from 
the view.  Given the unusual semantics of these registers I'd be tempted 
to map both pairs to the read register only and pretend it to be r/w with 
the LSB directly mapping to CP0.Status.FR and CP0.Config5.FRE as with 
CFC1.  Access would be r/o if user access to the selected bit was not 
permitted.

 This would provide the necessary semantics and avoid any security 
implications as GDB would only be able to poke/peek at the same bits the 
debuggee itself could.

 This could be sorted out the same way in core files.

 Finally, do we need to have these bits recorded in a signal frame?

>     F64 targets have an FR bit in the status register to specify the effective
>     register size, and the FRE bit is used with FR=1 to enable a compatibility
>     mode which is similar to FR=0 but with usable odd doubles which don't
>     alias with any other FP registers.

 This seems to contradict to how I read the architecture specification, 
which IIUC states that CP0.Config5.FRE=1 merely causes all single FP 
operations to trap.  All 32 64-bit registers are usable for doubles with 
CP0.Status.FR=1 regardless of the state of CP0.Config5.FRE.  Have I missed 
anything here?  Can you please elaborate and perhaps rephrase the sentence 
so that the implications for GDB are more clearly seen?

 Further comments as to the change itself follow.

>     gdb/ChangeLog:
> 
>     	* features/Makefile (WHICH): Add mips-fpu64-linux and
>     	mips-fpu64-dsp-linux and Sort microblaze out of the way

 Mid-sentence capitalisation issue here.

>         of mips/mips64

 Indentation and missing full stop.

>     	(mips-fpu64-expedite): Set.
>     	(mips-fpu64-dsp-expedite): Set.
>     	* features/mips-fpu64-dsp-linux.xml: New file.
>     	* features/mips-fpu64-linux.xml: New file.
>     	* features/mips-fpu64.xml: New file.
>     	* features/mips-fpu64-dsp-linux.c: New generated file.
>     	* features/mips-fpu64-linux.c: New generated file.
>     	* regformats/mips-fpu64-dsp-linux.dat: New generated file.
>     	* regformats/mips-fpu64-linux.dat: New generated file.
> 
>     gdb/gdbserver/ChangeLog:
> 
>     	* Makefile.in (clean): Delete mips-fpu64-linux.c and
>     	mips-fpu64-dsp-linux.c.

 I think "Delete" is confusing here: are you deleting from the rule or 
deleting files?  Please rephrase to make it unambiguous.  Maybe: "Add a 
deletion of..."?

> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
> index 10173cf..b6baaf66 100644
> --- a/gdb/features/Makefile
> +++ b/gdb/features/Makefile
> @@ -57,8 +57,8 @@ WHICH = aarch64 \
>  	i386/x32 i386/x32-linux \
>  	i386/x32-avx i386/x32-avx-linux \
>  	i386/x32-avx512 i386/x32-avx512-linux \
> -	mips-linux mips-dsp-linux \
>  	microblaze-with-stack-protect \
> +	mips-linux mips-dsp-linux mips-fpu64-linux mips-fpu64-dsp-linux \
>  	mips64-linux mips64-dsp-linux \
>  	nios2-linux \
>  	rs6000/powerpc-32 \
> @@ -100,11 +100,13 @@ i386/x32-avx-expedite = rbp,rsp,rip
>  i386/x32-avx-linux-expedite = rbp,rsp,rip
>  i386/x32-avx512-expedite = rbp,rsp,rip
>  i386/x32-avx512-linux-expedite = rbp,rsp,rip
> +microblaze-expedite = r1,rpc
>  mips-expedite = r29,pc
>  mips-dsp-expedite = r29,pc
> +mips-fpu64-expedite = r29,pc
> +mips-fpu64-dsp-expedite = r29,pc
>  mips64-expedite = r29,pc
>  mips64-dsp-expedite = r29,pc
> -microblaze-expedite = r1,rpc
>  nios2-linux-expedite = sp,pc
>  powerpc-expedite = r1,pc
>  rs6000/powerpc-cell32l-expedite = r1,pc,r0,orig_r3,r4

 Please split Microblaze changes off to a separate preparatory patch.  
They can probably go in right away as obviously correct, but as a newcomer 
please allow 48 hours for people to chime in before committing.

 Speaking of which -- I don't see you listed in gdb/MAINTAINERS, so can 
you please clarify your status WRT the FSF copyright assignment?  Are you 
covered by a company assignment?  If your paperwork has been cleared, then  
I can commit your change on your behalf.

> diff --git a/gdb/features/mips-fpu64-dsp-linux.xml b/gdb/features/mips-fpu64-dsp-linux.xml
> new file mode 100644
> index 0000000..f0de5ce
> --- /dev/null
> +++ b/gdb/features/mips-fpu64-dsp-linux.xml
> @@ -0,0 +1,20 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2012-2013 Free Software Foundation, Inc.

 You need to list the current year in copyright notices for newly 
submitted files, i.e.:

<!-- Copyright (C) 2016 Free Software Foundation, Inc.

Likewise throughout.

> diff --git a/gdb/features/mips-fpu64.xml b/gdb/features/mips-fpu64.xml
> new file mode 100644
> index 0000000..88dc9d9
> --- /dev/null
> +++ b/gdb/features/mips-fpu64.xml
> @@ -0,0 +1,45 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2007-2013 Free Software Foundation, Inc.
> +
> +     Copying and distribution of this file, with or without modification,
> +     are permitted in any medium without royalty provided the copyright
> +     notice and this notice are preserved.  -->
> +
> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> +<feature name="org.gnu.gdb.mips.fpu">
> +  <reg name="f0" bitsize="64" type="ieee_double"/>
> +  <reg name="f1" bitsize="64" type="ieee_double"/>
> +  <reg name="f2" bitsize="64" type="ieee_double"/>
> +  <reg name="f3" bitsize="64" type="ieee_double"/>
> +  <reg name="f4" bitsize="64" type="ieee_double"/>
> +  <reg name="f5" bitsize="64" type="ieee_double"/>
> +  <reg name="f6" bitsize="64" type="ieee_double"/>
> +  <reg name="f7" bitsize="64" type="ieee_double"/>
> +  <reg name="f8" bitsize="64" type="ieee_double"/>
> +  <reg name="f9" bitsize="64" type="ieee_double"/>
> +  <reg name="f10" bitsize="64" type="ieee_double"/>
> +  <reg name="f11" bitsize="64" type="ieee_double"/>
> +  <reg name="f12" bitsize="64" type="ieee_double"/>
> +  <reg name="f13" bitsize="64" type="ieee_double"/>
> +  <reg name="f14" bitsize="64" type="ieee_double"/>
> +  <reg name="f15" bitsize="64" type="ieee_double"/>
> +  <reg name="f16" bitsize="64" type="ieee_double"/>
> +  <reg name="f17" bitsize="64" type="ieee_double"/>
> +  <reg name="f18" bitsize="64" type="ieee_double"/>
> +  <reg name="f19" bitsize="64" type="ieee_double"/>
> +  <reg name="f20" bitsize="64" type="ieee_double"/>
> +  <reg name="f21" bitsize="64" type="ieee_double"/>
> +  <reg name="f22" bitsize="64" type="ieee_double"/>
> +  <reg name="f23" bitsize="64" type="ieee_double"/>
> +  <reg name="f24" bitsize="64" type="ieee_double"/>
> +  <reg name="f25" bitsize="64" type="ieee_double"/>
> +  <reg name="f26" bitsize="64" type="ieee_double"/>
> +  <reg name="f27" bitsize="64" type="ieee_double"/>
> +  <reg name="f28" bitsize="64" type="ieee_double"/>
> +  <reg name="f29" bitsize="64" type="ieee_double"/>
> +  <reg name="f30" bitsize="64" type="ieee_double"/>
> +  <reg name="f31" bitsize="64" type="ieee_double"/>
> +
> +  <reg name="fcsr" bitsize="32" group="float"/>
> +  <reg name="fir" bitsize="32" group="float"/>
> +</feature>

 This is AFAICT the same as mips64-fpu.xml, except for the sizes of `fcsr' 
and `fir', wrongly set to 64 bits in the latter file.  I think we ought to 
avoid the unnecessary duplication, and just use a single template for the 
FPU, which is the same in the 64-bit FP case regardless of the machine 
word width of the CPU.

 Can you therefore please just reuse mips64-fpu.xml here, which I think 
should be renamed at the same time to mips-fpu64.xml, as with your 
proposal (GIT will detect a rename and show it properly with diffs and 
other stats)?  For historical reasons we need to keep $fcsr and $fir at 64 
bits in XML descriptions, as explained in commit 78b86327b530 ("mips-tdep: 
Make FCRs always 32-bit"), made specifically to address the inconsistency 
discovered in the context of your submission; none of any further FCRs 
added will have any historical implications though, so they ought to be 
32-bit as in hardware.

 You'll have to adjust the callers of `mips_collect_register_32bit' and 
`mips_supply_register_32bit' in `gdbserver' accordingly so that 
`use_64bit' is set according to the FPU rather than the CPU model then.  
I don't think you'll need to adjust anything in mips-linux-tdep.c, however 
please double-check.

> diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
> index 1e874e3..9913d6a 100644
> --- a/gdb/gdbserver/Makefile.in
> +++ b/gdb/gdbserver/Makefile.in
> @@ -357,7 +357,8 @@ clean:
>  	rm -f reg-tilegx.c reg-tilegx32.c
>  	rm -f arm-with-iwmmxt.c
>  	rm -f arm-with-vfpv2.c arm-with-vfpv3.c arm-with-neon.c
> -	rm -f mips-linux.c mips-dsp-linux.c
> +	rm -f mips-linux.c mips-dsp-linux.c mips-fpu64-linux.c
> +	rm -f mips-fpu64-dsp-linux.c
>  	rm -f mips64-linux.c mips64-dsp-linux.c
>  	rm -f nios2-linux.c
>  	rm -f powerpc-32.c powerpc-32l.c powerpc-64l.c powerpc-e500l.c

 This will then have to reflect the removal of mips64-fpu.xml of course.

> index a54b9e7..422fd90 100644
> --- a/gdb/gdbserver/configure.srv
> +++ b/gdb/gdbserver/configure.srv
> @@ -187,15 +187,20 @@ case "${target}" in
>  			;;
>    mips*-*-linux*)	srv_regobj="mips-linux.o"
>  			srv_regobj="${srv_regobj} mips-dsp-linux.o"
> +			srv_regobj="${srv_regobj} mips-fpu64-linux.o"
> +			srv_regobj="${srv_regobj} mips-fpu64-dsp-linux.o"
>  			srv_regobj="${srv_regobj} mips64-linux.o"
>  			srv_regobj="${srv_regobj} mips64-dsp-linux.o"
>  			srv_tgtobj="$srv_linux_obj linux-mips-low.o"
>  			srv_tgtobj="${srv_tgtobj} mips-linux-watch.o"
>  			srv_xmlfiles="mips-linux.xml"
>  			srv_xmlfiles="${srv_xmlfiles} mips-dsp-linux.xml"
> +			srv_xmlfiles="${srv_xmlfiles} mips-fpu64-linux.xml"
> +			srv_xmlfiles="${srv_xmlfiles} mips-fpu64-dsp-linux.xml"
>  			srv_xmlfiles="${srv_xmlfiles} mips-cpu.xml"
>  			srv_xmlfiles="${srv_xmlfiles} mips-cp0.xml"
>  			srv_xmlfiles="${srv_xmlfiles} mips-fpu.xml"
> +			srv_xmlfiles="${srv_xmlfiles} mips-fpu64.xml"
>  			srv_xmlfiles="${srv_xmlfiles} mips-dsp.xml"
>  			srv_xmlfiles="${srv_xmlfiles} mips64-linux.xml"
>  			srv_xmlfiles="${srv_xmlfiles} mips64-dsp-linux.xml"

 Likewise.

  Maciej
  

Hi Maciej,

On Wed, Oct 12, 2016 at 01:42:17PM +0100, Maciej W. Rozycki wrote:
> On Mon, 27 Jun 2016, Bhushan Attarde wrote:
> 
> >     Add a couple of new target descriptions for MIPS32 targets with 64-bit
> >     floating point registers, with and without DSP.
> > 
> >     These are identical to their 32-bit FPU counterparts except that the FP
> >     registers are 64-bits long to allow for debugging of F64=1 MIPS32 targets
> >     such as MIPS32r2 compatible cores, and they include the Config5 CP0
> >     register which has an FRE bit.
> 
>  Hmm, has Linux kernel support for CP0.Config5 accesses gone upstream 
> already?  Can you give me an upstream commit ID and/or reference to the 
> discussion where it has been approved if so?

I don't think it did go upstream yet.

> 
>  More importantly, what do we need CP0.Config5 access for in the first 
> place?  It looks to me like this bit is irrelevant to GDB as it does not 
> affect the native (raw) register format.  So the only use would be to let 
> the user running a debugging session switch between the FRE and NFRE modes 
> without the need to poke at CP1C.FRE or CP1C.NFRE registers with a CTC1 
> instruction, which by itself makes sense to me, but needs a further 
> consideration.

It allows the FRE bit to be read (I seem to remember this was the only
bit actually exposed through ptrace by the patch).

FRE simply causes certain instructions (all single precision FP
arithmetic instructions and FP word loads/stores) to trap to the kernel
so that it can emulate a variation/subset of FR=0, so the debugger would
use it to decide how to decode the single precision FP registers based
on the double precision FP registers (iirc).

> 
>  Additionally exposing CP0.Config5 may have security implications, 
> especially as parts of the register have not been defined yet in the 
> architectures and we'd have to force architecture maintainers somehow to 
> ask us every time they intend to add a bit to this register to check if 
> this has security implications and has to be avoided and/or explicitly 
> handled in software.

yes, as above it explicity only shows certain bits. I'm fine with the
api changing if necessary though since it isn't upstream.

cheers
James

> 
>  So overall it looks to me like we should avoid it.  What we ought to do 
> instead is -- since we need to extend structures anyway -- I think adding 
> all the remaining CP1C registers.  This would cover the earlier CP1C.UFR 
> and CP1C.UNFR registers as well as any future additions, which would be 
> further qualified with CP1.FIR bits to remove unimplemented registers from 
> the view.  Given the unusual semantics of these registers I'd be tempted 
> to map both pairs to the read register only and pretend it to be r/w with 
> the LSB directly mapping to CP0.Status.FR and CP0.Config5.FRE as with 
> CFC1.  Access would be r/o if user access to the selected bit was not 
> permitted.
> 
>  This would provide the necessary semantics and avoid any security 
> implications as GDB would only be able to poke/peek at the same bits the 
> debuggee itself could.
> 
>  This could be sorted out the same way in core files.
> 
>  Finally, do we need to have these bits recorded in a signal frame?
> 
> >     F64 targets have an FR bit in the status register to specify the effective
> >     register size, and the FRE bit is used with FR=1 to enable a compatibility
> >     mode which is similar to FR=0 but with usable odd doubles which don't
> >     alias with any other FP registers.
> 
>  This seems to contradict to how I read the architecture specification, 
> which IIUC states that CP0.Config5.FRE=1 merely causes all single FP 
> operations to trap.  All 32 64-bit registers are usable for doubles with 
> CP0.Status.FR=1 regardless of the state of CP0.Config5.FRE.  Have I missed 
> anything here?  Can you please elaborate and perhaps rephrase the sentence 
> so that the implications for GDB are more clearly seen?
> 
>  Further comments as to the change itself follow.
> 
> >     gdb/ChangeLog:
> > 
> >     	* features/Makefile (WHICH): Add mips-fpu64-linux and
> >     	mips-fpu64-dsp-linux and Sort microblaze out of the way
> 
>  Mid-sentence capitalisation issue here.
> 
> >         of mips/mips64
> 
>  Indentation and missing full stop.
> 
> >     	(mips-fpu64-expedite): Set.
> >     	(mips-fpu64-dsp-expedite): Set.
> >     	* features/mips-fpu64-dsp-linux.xml: New file.
> >     	* features/mips-fpu64-linux.xml: New file.
> >     	* features/mips-fpu64.xml: New file.
> >     	* features/mips-fpu64-dsp-linux.c: New generated file.
> >     	* features/mips-fpu64-linux.c: New generated file.
> >     	* regformats/mips-fpu64-dsp-linux.dat: New generated file.
> >     	* regformats/mips-fpu64-linux.dat: New generated file.
> > 
> >     gdb/gdbserver/ChangeLog:
> > 
> >     	* Makefile.in (clean): Delete mips-fpu64-linux.c and
> >     	mips-fpu64-dsp-linux.c.
> 
>  I think "Delete" is confusing here: are you deleting from the rule or 
> deleting files?  Please rephrase to make it unambiguous.  Maybe: "Add a 
> deletion of..."?
> 
> > diff --git a/gdb/features/Makefile b/gdb/features/Makefile
> > index 10173cf..b6baaf66 100644
> > --- a/gdb/features/Makefile
> > +++ b/gdb/features/Makefile
> > @@ -57,8 +57,8 @@ WHICH = aarch64 \
> >  	i386/x32 i386/x32-linux \
> >  	i386/x32-avx i386/x32-avx-linux \
> >  	i386/x32-avx512 i386/x32-avx512-linux \
> > -	mips-linux mips-dsp-linux \
> >  	microblaze-with-stack-protect \
> > +	mips-linux mips-dsp-linux mips-fpu64-linux mips-fpu64-dsp-linux \
> >  	mips64-linux mips64-dsp-linux \
> >  	nios2-linux \
> >  	rs6000/powerpc-32 \
> > @@ -100,11 +100,13 @@ i386/x32-avx-expedite = rbp,rsp,rip
> >  i386/x32-avx-linux-expedite = rbp,rsp,rip
> >  i386/x32-avx512-expedite = rbp,rsp,rip
> >  i386/x32-avx512-linux-expedite = rbp,rsp,rip
> > +microblaze-expedite = r1,rpc
> >  mips-expedite = r29,pc
> >  mips-dsp-expedite = r29,pc
> > +mips-fpu64-expedite = r29,pc
> > +mips-fpu64-dsp-expedite = r29,pc
> >  mips64-expedite = r29,pc
> >  mips64-dsp-expedite = r29,pc
> > -microblaze-expedite = r1,rpc
> >  nios2-linux-expedite = sp,pc
> >  powerpc-expedite = r1,pc
> >  rs6000/powerpc-cell32l-expedite = r1,pc,r0,orig_r3,r4
> 
>  Please split Microblaze changes off to a separate preparatory patch.  
> They can probably go in right away as obviously correct, but as a newcomer 
> please allow 48 hours for people to chime in before committing.
> 
>  Speaking of which -- I don't see you listed in gdb/MAINTAINERS, so can 
> you please clarify your status WRT the FSF copyright assignment?  Are you 
> covered by a company assignment?  If your paperwork has been cleared, then  
> I can commit your change on your behalf.
> 
> > diff --git a/gdb/features/mips-fpu64-dsp-linux.xml b/gdb/features/mips-fpu64-dsp-linux.xml
> > new file mode 100644
> > index 0000000..f0de5ce
> > --- /dev/null
> > +++ b/gdb/features/mips-fpu64-dsp-linux.xml
> > @@ -0,0 +1,20 @@
> > +<?xml version="1.0"?>
> > +<!-- Copyright (C) 2012-2013 Free Software Foundation, Inc.
> 
>  You need to list the current year in copyright notices for newly 
> submitted files, i.e.:
> 
> <!-- Copyright (C) 2016 Free Software Foundation, Inc.
> 
> Likewise throughout.
> 
> > diff --git a/gdb/features/mips-fpu64.xml b/gdb/features/mips-fpu64.xml
> > new file mode 100644
> > index 0000000..88dc9d9
> > --- /dev/null
> > +++ b/gdb/features/mips-fpu64.xml
> > @@ -0,0 +1,45 @@
> > +<?xml version="1.0"?>
> > +<!-- Copyright (C) 2007-2013 Free Software Foundation, Inc.
> > +
> > +     Copying and distribution of this file, with or without modification,
> > +     are permitted in any medium without royalty provided the copyright
> > +     notice and this notice are preserved.  -->
> > +
> > +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> > +<feature name="org.gnu.gdb.mips.fpu">
> > +  <reg name="f0" bitsize="64" type="ieee_double"/>
> > +  <reg name="f1" bitsize="64" type="ieee_double"/>
> > +  <reg name="f2" bitsize="64" type="ieee_double"/>
> > +  <reg name="f3" bitsize="64" type="ieee_double"/>
> > +  <reg name="f4" bitsize="64" type="ieee_double"/>
> > +  <reg name="f5" bitsize="64" type="ieee_double"/>
> > +  <reg name="f6" bitsize="64" type="ieee_double"/>
> > +  <reg name="f7" bitsize="64" type="ieee_double"/>
> > +  <reg name="f8" bitsize="64" type="ieee_double"/>
> > +  <reg name="f9" bitsize="64" type="ieee_double"/>
> > +  <reg name="f10" bitsize="64" type="ieee_double"/>
> > +  <reg name="f11" bitsize="64" type="ieee_double"/>
> > +  <reg name="f12" bitsize="64" type="ieee_double"/>
> > +  <reg name="f13" bitsize="64" type="ieee_double"/>
> > +  <reg name="f14" bitsize="64" type="ieee_double"/>
> > +  <reg name="f15" bitsize="64" type="ieee_double"/>
> > +  <reg name="f16" bitsize="64" type="ieee_double"/>
> > +  <reg name="f17" bitsize="64" type="ieee_double"/>
> > +  <reg name="f18" bitsize="64" type="ieee_double"/>
> > +  <reg name="f19" bitsize="64" type="ieee_double"/>
> > +  <reg name="f20" bitsize="64" type="ieee_double"/>
> > +  <reg name="f21" bitsize="64" type="ieee_double"/>
> > +  <reg name="f22" bitsize="64" type="ieee_double"/>
> > +  <reg name="f23" bitsize="64" type="ieee_double"/>
> > +  <reg name="f24" bitsize="64" type="ieee_double"/>
> > +  <reg name="f25" bitsize="64" type="ieee_double"/>
> > +  <reg name="f26" bitsize="64" type="ieee_double"/>
> > +  <reg name="f27" bitsize="64" type="ieee_double"/>
> > +  <reg name="f28" bitsize="64" type="ieee_double"/>
> > +  <reg name="f29" bitsize="64" type="ieee_double"/>
> > +  <reg name="f30" bitsize="64" type="ieee_double"/>
> > +  <reg name="f31" bitsize="64" type="ieee_double"/>
> > +
> > +  <reg name="fcsr" bitsize="32" group="float"/>
> > +  <reg name="fir" bitsize="32" group="float"/>
> > +</feature>
> 
>  This is AFAICT the same as mips64-fpu.xml, except for the sizes of `fcsr' 
> and `fir', wrongly set to 64 bits in the latter file.  I think we ought to 
> avoid the unnecessary duplication, and just use a single template for the 
> FPU, which is the same in the 64-bit FP case regardless of the machine 
> word width of the CPU.
> 
>  Can you therefore please just reuse mips64-fpu.xml here, which I think 
> should be renamed at the same time to mips-fpu64.xml, as with your 
> proposal (GIT will detect a rename and show it properly with diffs and 
> other stats)?  For historical reasons we need to keep $fcsr and $fir at 64 
> bits in XML descriptions, as explained in commit 78b86327b530 ("mips-tdep: 
> Make FCRs always 32-bit"), made specifically to address the inconsistency 
> discovered in the context of your submission; none of any further FCRs 
> added will have any historical implications though, so they ought to be 
> 32-bit as in hardware.
> 
>  You'll have to adjust the callers of `mips_collect_register_32bit' and 
> `mips_supply_register_32bit' in `gdbserver' accordingly so that 
> `use_64bit' is set according to the FPU rather than the CPU model then.  
> I don't think you'll need to adjust anything in mips-linux-tdep.c, however 
> please double-check.
> 
> > diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
> > index 1e874e3..9913d6a 100644
> > --- a/gdb/gdbserver/Makefile.in
> > +++ b/gdb/gdbserver/Makefile.in
> > @@ -357,7 +357,8 @@ clean:
> >  	rm -f reg-tilegx.c reg-tilegx32.c
> >  	rm -f arm-with-iwmmxt.c
> >  	rm -f arm-with-vfpv2.c arm-with-vfpv3.c arm-with-neon.c
> > -	rm -f mips-linux.c mips-dsp-linux.c
> > +	rm -f mips-linux.c mips-dsp-linux.c mips-fpu64-linux.c
> > +	rm -f mips-fpu64-dsp-linux.c
> >  	rm -f mips64-linux.c mips64-dsp-linux.c
> >  	rm -f nios2-linux.c
> >  	rm -f powerpc-32.c powerpc-32l.c powerpc-64l.c powerpc-e500l.c
> 
>  This will then have to reflect the removal of mips64-fpu.xml of course.
> 
> > index a54b9e7..422fd90 100644
> > --- a/gdb/gdbserver/configure.srv
> > +++ b/gdb/gdbserver/configure.srv
> > @@ -187,15 +187,20 @@ case "${target}" in
> >  			;;
> >    mips*-*-linux*)	srv_regobj="mips-linux.o"
> >  			srv_regobj="${srv_regobj} mips-dsp-linux.o"
> > +			srv_regobj="${srv_regobj} mips-fpu64-linux.o"
> > +			srv_regobj="${srv_regobj} mips-fpu64-dsp-linux.o"
> >  			srv_regobj="${srv_regobj} mips64-linux.o"
> >  			srv_regobj="${srv_regobj} mips64-dsp-linux.o"
> >  			srv_tgtobj="$srv_linux_obj linux-mips-low.o"
> >  			srv_tgtobj="${srv_tgtobj} mips-linux-watch.o"
> >  			srv_xmlfiles="mips-linux.xml"
> >  			srv_xmlfiles="${srv_xmlfiles} mips-dsp-linux.xml"
> > +			srv_xmlfiles="${srv_xmlfiles} mips-fpu64-linux.xml"
> > +			srv_xmlfiles="${srv_xmlfiles} mips-fpu64-dsp-linux.xml"
> >  			srv_xmlfiles="${srv_xmlfiles} mips-cpu.xml"
> >  			srv_xmlfiles="${srv_xmlfiles} mips-cp0.xml"
> >  			srv_xmlfiles="${srv_xmlfiles} mips-fpu.xml"
> > +			srv_xmlfiles="${srv_xmlfiles} mips-fpu64.xml"
> >  			srv_xmlfiles="${srv_xmlfiles} mips-dsp.xml"
> >  			srv_xmlfiles="${srv_xmlfiles} mips64-linux.xml"
> >  			srv_xmlfiles="${srv_xmlfiles} mips64-dsp-linux.xml"
> 
>  Likewise.
> 
>   Maciej
  

Hi James,

 Thanks for your input!

 Cc-ing linux-mips for the discussion about a ptrace(2) kernel API update; 
anyone interested in previous talk about this change please have a look 
at: <https://sourceware.org/ml/gdb-patches/2016-06/msg00441.html> and 
<https://sourceware.org/ml/gdb-patches/2016-10/msg00311.html> for the 
earlier messages.

> >  Hmm, has Linux kernel support for CP0.Config5 accesses gone upstream 
> > already?  Can you give me an upstream commit ID and/or reference to the 
> > discussion where it has been approved if so?
> 
> I don't think it did go upstream yet.

 Good!

> >  More importantly, what do we need CP0.Config5 access for in the first 
> > place?  It looks to me like this bit is irrelevant to GDB as it does not 
> > affect the native (raw) register format.  So the only use would be to let 
> > the user running a debugging session switch between the FRE and NFRE modes 
> > without the need to poke at CP1C.FRE or CP1C.NFRE registers with a CTC1 
> > instruction, which by itself makes sense to me, but needs a further 
> > consideration.
> 
> It allows the FRE bit to be read (I seem to remember this was the only
> bit actually exposed through ptrace by the patch).

 Then I think it makes sense even more not to create this artificial API 
and use the CP1C.FRE/CP1C.NFRE registers instead which do correspond to 
what hardware presents to user software.  Also with CP1C.UFR/CP1C.UNFR vs 
CP0.Status; while we want to retain the latter register in the view for 
historical reasons, it has always been read-only and I think it ought to 
remain such, with any writes to CP0.Status.FR executed via the former CP1C 
registers only.

> FRE simply causes certain instructions (all single precision FP
> arithmetic instructions and FP word loads/stores) to trap to the kernel
> so that it can emulate a variation/subset of FR=0, so the debugger would
> use it to decide how to decode the single precision FP registers based
> on the double precision FP registers (iirc).

 I don't think there is any value in it for GDB, I think all 64-bit FP 
registers ought to remain being presented as doubles and pairs of singles 
regardless of the mode selected (and also possibly fixed-point longs and 
pairs of fixed-point words).  We don't know what's emulated and what's not 
after all, and then the contents of FPRs are not interpreted by GDB itself 
anyhow except in user-supplied expressions or assignment requests, which 
for users' convenience I think should retain the maximum flexibility 
possible.

 So as I say it looks to me like the only, though obviously valid and 
wholeheartedly supported, use for CP1C.FRE/CP1C.NFRE would be for user's 
control of the execution environment.

> >  Additionally exposing CP0.Config5 may have security implications, 
> > especially as parts of the register have not been defined yet in the 
> > architectures and we'd have to force architecture maintainers somehow to 
> > ask us every time they intend to add a bit to this register to check if 
> > this has security implications and has to be avoided and/or explicitly 
> > handled in software.
> 
> yes, as above it explicity only shows certain bits. I'm fine with the
> api changing if necessary though since it isn't upstream.

 It sounds like a plan to me then -- any further questions or comments 
about the kernel API part, anyone?

  Maciej
  

On Wed, Oct 12, 2016 at 05:29:53PM +0100, Maciej W. Rozycki wrote:
> Hi James,
> 
>  Thanks for your input!
> 
>  Cc-ing linux-mips for the discussion about a ptrace(2) kernel API update; 
> anyone interested in previous talk about this change please have a look 
> at: <https://sourceware.org/ml/gdb-patches/2016-06/msg00441.html> and 
> <https://sourceware.org/ml/gdb-patches/2016-10/msg00311.html> for the 
> earlier messages.
> 
> > >  Hmm, has Linux kernel support for CP0.Config5 accesses gone upstream 
> > > already?  Can you give me an upstream commit ID and/or reference to the 
> > > discussion where it has been approved if so?
> > 
> > I don't think it did go upstream yet.
> 
>  Good!
> 
> > >  More importantly, what do we need CP0.Config5 access for in the first 
> > > place?  It looks to me like this bit is irrelevant to GDB as it does not 
> > > affect the native (raw) register format.  So the only use would be to let 
> > > the user running a debugging session switch between the FRE and NFRE modes 
> > > without the need to poke at CP1C.FRE or CP1C.NFRE registers with a CTC1 
> > > instruction, which by itself makes sense to me, but needs a further 
> > > consideration.
> > 
> > It allows the FRE bit to be read (I seem to remember this was the only
> > bit actually exposed through ptrace by the patch).
> 
>  Then I think it makes sense even more not to create this artificial API 
> and use the CP1C.FRE/CP1C.NFRE registers instead which do correspond to 
> what hardware presents to user software.

well, barely. Linux at least doesn't enable Config5.UFE or Config5.UFR,
since FP mode changes must be done for all threads in the process, so
userland can't actually directly access those FCRs either.

> Also with CP1C.UFR/CP1C.UNFR vs 
> CP0.Status; while we want to retain the latter register in the view for 
> historical reasons, it has always been read-only and I think it ought to 
> remain such, with any writes to CP0.Status.FR executed via the former CP1C 
> registers only.
> 
> > FRE simply causes certain instructions (all single precision FP
> > arithmetic instructions and FP word loads/stores) to trap to the kernel
> > so that it can emulate a variation/subset of FR=0, so the debugger would
> > use it to decide how to decode the single precision FP registers based
> > on the double precision FP registers (iirc).
> 
>  I don't think there is any value in it for GDB, I think all 64-bit FP 
> registers ought to remain being presented as doubles and pairs of singles 
> regardless of the mode selected (and also possibly fixed-point longs and 
> pairs of fixed-point words).  We don't know what's emulated and what's not 
> after all, and then the contents of FPRs are not interpreted by GDB itself 
> anyhow except in user-supplied expressions or assignment requests, which 
> for users' convenience I think should retain the maximum flexibility 
> possible.

Well it technically depends on
test_tsk_thread_flag(target, TIF_HYBRID_FPREGS)

So it allows gdb to detect Linux's Hybrid FPU mode, and present the fp
regs (e.g. f0 or f1) more like below to the user depending on the fp
mode, which is surely much more intuitive from an assembly debugging
point of view.

Its also worth noting that "When software changes the value of this bit
[Status.FR], the contents of the floating-point registers are
UNPREDICTABLE". I.e. there is no architectural unifying register layout
between FR=0 / FR=1, only convention. Linux will I presume intentionally
save in old mode and restore in new mode on an fp mode change according
to its own convention (such that the valid mode changes don't clobber
register state).

(disclaimer: I haven't looked at this gdb patchset in detail as to
whether any of below has changed since I worked on it).

(1) Even singles and doubles always overlap one another, as do odd
singles and doubles when FR=1 (and FRE=0):
	/* (little endian) */
	union __gdb_builtin_mips_fp64 {
	  float  f32;
	  double f64;
	  int32  i32;
	  int64  i64;
	};

(2) Odd singles when FR=0 (there are no odd doubles):
	union __gdb_builtin_mips_fp32 {
	  float f32;
	  int32_t i32;
	};

(3) Odd singles and doubles when FR=1, FRE=1 don't overlap at all:
	struct __gdb_builtin_mips_fp96 {
		union {
			double f64;
			int64  i64;
		};
		union {
			float  f32;
			int32  i32;
		};
	};

i.e.

FR=0:
 (1) even
       double:	FEDCBA9876543210
       single:	        76543210
 (2) odd
       single:	FEDCBA98

FR=1, FRE=0:
 (1) even
       double:	FEDCBA9876543210
       single:	        76543210
 (1) odd
       double:	                0123456789ABCDEF
       single:	                        89ABCDEF

FR=1, FRE=1:
 (1) even
       double:	FEDCBA9876543210
       single:	        76543210   (Hybrid FPR emulation)
 (3) odd
       double:	                0123456789ABCDEF
       single:	FEDCBA98           (Hybrid FPR emulation)
)

Cheers
James

> 
>  So as I say it looks to me like the only, though obviously valid and 
> wholeheartedly supported, use for CP1C.FRE/CP1C.NFRE would be for user's 
> control of the execution environment.
> 
> > >  Additionally exposing CP0.Config5 may have security implications, 
> > > especially as parts of the register have not been defined yet in the 
> > > architectures and we'd have to force architecture maintainers somehow to 
> > > ask us every time they intend to add a bit to this register to check if 
> > > this has security implications and has to be avoided and/or explicitly 
> > > handled in software.
> > 
> > yes, as above it explicity only shows certain bits. I'm fine with the
> > api changing if necessary though since it isn't upstream.
> 
>  It sounds like a plan to me then -- any further questions or comments 
> about the kernel API part, anyone?
> 
>   Maciej
  

On Wed, 12 Oct 2016, James Hogan wrote:

> >  Then I think it makes sense even more not to create this artificial API 
> > and use the CP1C.FRE/CP1C.NFRE registers instead which do correspond to 
> > what hardware presents to user software.
> 
> well, barely. Linux at least doesn't enable Config5.UFE or Config5.UFR,
> since FP mode changes must be done for all threads in the process, so
> userland can't actually directly access those FCRs either.

 Hmm, I didn't know that -- what was the reason for this design decision?  
Offhand the limitation does not appear necessary to me, each thread has 
its own distinct register set, so it does not appear to me that its mode 
of operation has to be the same across them all.  The current setting 
would still of course be inherited from the parent by any new threads 
created with clone(2).

 Anyway in that case the presented CP1C registers will have to be 
read-only.

> >  I don't think there is any value in it for GDB, I think all 64-bit FP 
> > registers ought to remain being presented as doubles and pairs of singles 
> > regardless of the mode selected (and also possibly fixed-point longs and 
> > pairs of fixed-point words).  We don't know what's emulated and what's not 
> > after all, and then the contents of FPRs are not interpreted by GDB itself 
> > anyhow except in user-supplied expressions or assignment requests, which 
> > for users' convenience I think should retain the maximum flexibility 
> > possible.
> 
> Well it technically depends on
> test_tsk_thread_flag(target, TIF_HYBRID_FPREGS)

 Sure, but the hardware representation is CP0.Config5.FRE/CP1C.FRE.

> So it allows gdb to detect Linux's Hybrid FPU mode, and present the fp
> regs (e.g. f0 or f1) more like below to the user depending on the fp
> mode, which is surely much more intuitive from an assembly debugging
> point of view.
> 
> Its also worth noting that "When software changes the value of this bit
> [Status.FR], the contents of the floating-point registers are
> UNPREDICTABLE". I.e. there is no architectural unifying register layout
> between FR=0 / FR=1, only convention. Linux will I presume intentionally
> save in old mode and restore in new mode on an fp mode change according
> to its own convention (such that the valid mode changes don't clobber
> register state).

 Well the contents might be unpredictable, but there sure will be some and 
GDB is supposed to present it.

> (disclaimer: I haven't looked at this gdb patchset in detail as to
> whether any of below has changed since I worked on it).
> 
> (1) Even singles and doubles always overlap one another, as do odd
> singles and doubles when FR=1 (and FRE=0):
> 	/* (little endian) */
> 	union __gdb_builtin_mips_fp64 {
> 	  float  f32;
> 	  double f64;
> 	  int32  i32;
> 	  int64  i64;
> 	};
> 
> (2) Odd singles when FR=0 (there are no odd doubles):
> 	union __gdb_builtin_mips_fp32 {
> 	  float f32;
> 	  int32_t i32;
> 	};
> 
> (3) Odd singles and doubles when FR=1, FRE=1 don't overlap at all:
> 	struct __gdb_builtin_mips_fp96 {
> 		union {
> 			double f64;
> 			int64  i64;
> 		};
> 		union {
> 			float  f32;
> 			int32  i32;
> 		};
> 	};
> 
> i.e.
> 
> FR=0:
>  (1) even
>        double:	FEDCBA9876543210
>        single:	        76543210
>  (2) odd
>        single:	FEDCBA98
> 
> FR=1, FRE=0:
>  (1) even
>        double:	FEDCBA9876543210
>        single:	        76543210
>  (1) odd
>        double:	                0123456789ABCDEF
>        single:	                        89ABCDEF
> 
> FR=1, FRE=1:
>  (1) even
>        double:	FEDCBA9876543210
>        single:	        76543210   (Hybrid FPR emulation)
>  (3) odd
>        double:	                0123456789ABCDEF
>        single:	FEDCBA98           (Hybrid FPR emulation)
> )

 I haven't got to this part so far and either way will have to think about 
it yet.  For one as I noted we do want to present vector (paired-single) 
data with FR=1, FRE=0 in addition to what you quoted above.  This was all 
implemented in an old MIPS UK patch originally written by Nigel Stephens 
and included with SDE, which I've never got to upstreaming; have you by 
any chance based your work on that?

 As to FR=1, FRE=1 your quoted representation of singles is a software 
convention only, so I'm not sure offhand how we might represent it in GDB 
to keep it reasonable; the 96-bit cooked FP register structure does not 
appeal to me at all TBH, but maybe it's the best we can do after all.

  Maciej
  

Maciej Rozycki <Maciej.Rozycki@imgtec.com> writes:
> On Wed, 12 Oct 2016, James Hogan wrote:
> 
> > >  Then I think it makes sense even more not to create this artificial
> > > API and use the CP1C.FRE/CP1C.NFRE registers instead which do
> > > correspond to what hardware presents to user software.
> >
> > well, barely. Linux at least doesn't enable Config5.UFE or
> > Config5.UFR, since FP mode changes must be done for all threads in the
> > process, so userland can't actually directly access those FCRs either.
> 
>  Hmm, I didn't know that -- what was the reason for this design
> decision?
> Offhand the limitation does not appear necessary to me, each thread has
> its own distinct register set, so it does not appear to me that its mode
> of operation has to be the same across them all.  The current setting
> would still of course be inherited from the parent by any new threads
> created with clone(2).
> 
>  Anyway in that case the presented CP1C registers will have to be read-
> only.

There is no need to support the CP1C.FRE/CP1C.NFRE CP1C.FR/CP1C.NFR
registers as they did not form part of the FR compatibility solution in the
end. They were added to the architecture as part of an earlier plan that
would have involved user-mode code switching mode on a per function basis.

They must not be enabled in Linux as use of them will lead to complete
chaos :-).

> > >  I don't think there is any value in it for GDB, I think all 64-bit
> > > FP registers ought to remain being presented as doubles and pairs of
> > > singles regardless of the mode selected (and also possibly
> > > fixed-point longs and pairs of fixed-point words).  We don't know
> > > what's emulated and what's not after all, and then the contents of
> > > FPRs are not interpreted by GDB itself anyhow except in
> > > user-supplied expressions or assignment requests, which for users'
> > > convenience I think should retain the maximum flexibility possible.
> >
> > Well it technically depends on
> > test_tsk_thread_flag(target, TIF_HYBRID_FPREGS)
> 
>  Sure, but the hardware representation is CP0.Config5.FRE/CP1C.FRE.

The FRE compatibility solution does require GDB to both know about and
modify the user-view of registers as the raw register data cannot be
interpreted by a user unassisted. My memory is a little rusty but I think
this already happens for FR=0 vs FR=1 in that GDB is provided with 32
64-bit registers and must present them as either:

FR=0
====
16 doubles by concatenating the low 32-bits of 2 consecutive registers
to form a double.
32* singles by showing the low 32-bits of each register (*odd registers
not being singles in mips V and below in FR=0.)

FR=1
====
32 doubles
32 singles
(32 128-bit)

FRE=1
=====
32 doubles
32 singles which are stored only in even numbered 64-bit registers with
the low 32-bit being an even numbered single and the high 32-bit being
an odd numbered single.
(32 128-bit)

GDB cannot show the FRE=1 state correctly without knowing which mode the
process is running in. I think this matches with comments from James
below.

> > So it allows gdb to detect Linux's Hybrid FPU mode, and present the fp
> > regs (e.g. f0 or f1) more like below to the user depending on the fp
> > mode, which is surely much more intuitive from an assembly debugging
> > point of view.
> >
> > Its also worth noting that "When software changes the value of this
> > bit [Status.FR], the contents of the floating-point registers are
> > UNPREDICTABLE". I.e. there is no architectural unifying register
> > layout between FR=0 / FR=1, only convention. Linux will I presume
> > intentionally save in old mode and restore in new mode on an fp mode
> > change according to its own convention (such that the valid mode
> > changes don't clobber register state).
> 
>  Well the contents might be unpredictable, but there sure will be some
> and GDB is supposed to present it.

The scheme we have guarantees that no FPU mode switch ever leaves the
register state in an unknown state which is another reason why users
cannot change mode directly. The kernel always performs the mode switch
and this happens with the FPU state in soft-context which is then
restored after the mode switch occurs.

> > (disclaimer: I haven't looked at this gdb patchset in detail as to
> > whether any of below has changed since I worked on it).
> >
> > (1) Even singles and doubles always overlap one another, as do odd
> > singles and doubles when FR=1 (and FRE=0):
> > 	/* (little endian) */
> > 	union __gdb_builtin_mips_fp64 {
> > 	  float  f32;
> > 	  double f64;
> > 	  int32  i32;
> > 	  int64  i64;
> > 	};
> >
> > (2) Odd singles when FR=0 (there are no odd doubles):
> > 	union __gdb_builtin_mips_fp32 {
> > 	  float f32;
> > 	  int32_t i32;
> > 	};
> >
> > (3) Odd singles and doubles when FR=1, FRE=1 don't overlap at all:
> > 	struct __gdb_builtin_mips_fp96 {
> > 		union {
> > 			double f64;
> > 			int64  i64;
> > 		};
> > 		union {
> > 			float  f32;
> > 			int32  i32;
> > 		};
> > 	};
> >
> > i.e.
> >
> > FR=0:
> >  (1) even
> >        double:	FEDCBA9876543210
> >        single:	        76543210
> >  (2) odd
> >        single:	FEDCBA98
> >
> > FR=1, FRE=0:
> >  (1) even
> >        double:	FEDCBA9876543210
> >        single:	        76543210
> >  (1) odd
> >        double:	                0123456789ABCDEF
> >        single:	                        89ABCDEF
> >
> > FR=1, FRE=1:
> >  (1) even
> >        double:	FEDCBA9876543210
> >        single:	        76543210   (Hybrid FPR emulation)
> >  (3) odd
> >        double:	                0123456789ABCDEF
> >        single:	FEDCBA98           (Hybrid FPR emulation)
> > )
> 
>  I haven't got to this part so far and either way will have to think
> about it yet.  For one as I noted we do want to present vector (paired-
> single) data with FR=1, FRE=0 in addition to what you quoted above.
> This was all implemented in an old MIPS UK patch originally written by
> Nigel Stephens and included with SDE, which I've never got to
> upstreaming; have you by any chance based your work on that?
> 
>  As to FR=1, FRE=1 your quoted representation of singles is a software
> convention only, so I'm not sure offhand how we might represent it in
> GDB to keep it reasonable; the 96-bit cooked FP register structure does
> not appeal to me at all TBH, but maybe it's the best we can do after
> all.

The whole FR compatibility scheme has some extremely intricate design
especially when it comes to FRE mode and I believe all tools have to play
along in order to get the end result to be seamless for users. If we
can do any simplification of GDB or the kernel interface then I'm open
to ideas.

A reference to the spec in case anyone doesn't know where it is:

https://dmz-portal.ba.imgtec.org/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking

Note that the spec does not include a definition of the ptrace extension
nor core dump extension (possibly not even designed yet).

While I remember the GDB patchset does need at least checking if not
extra work to cope with the way double precision type data is described
in dwarf for the various compile modes.

Matthew
  

On Thu, 13 Oct 2016, Matthew Fortune wrote:

> >  Hmm, I didn't know that -- what was the reason for this design
> > decision?
> > Offhand the limitation does not appear necessary to me, each thread has
> > its own distinct register set, so it does not appear to me that its mode
> > of operation has to be the same across them all.  The current setting
> > would still of course be inherited from the parent by any new threads
> > created with clone(2).
> > 
> >  Anyway in that case the presented CP1C registers will have to be read-
> > only.
> 
> There is no need to support the CP1C.FRE/CP1C.NFRE CP1C.FR/CP1C.NFR
> registers as they did not form part of the FR compatibility solution in the
> end. They were added to the architecture as part of an earlier plan that
> would have involved user-mode code switching mode on a per function basis.

 I wonder in that case whether we shouldn't simply have a virtual $fre 
register represented.  It could be 1-bit, but I suspect this may not play 
well with the RSP, so we can make it wider at the raw level and only 
truncate it at the cooked level, just as we do with some other registers 
including in particular my recent $fcsr/$fir change.

 If we had such a dedicated virtual $fre, and we decided sometime to let 
the user actually write to it and switch the mode process-wide, then we 
could simply invoke the right prctl(2) call in response to the user's 
ptrace(2) request.

 NB the x86-64 target has such support with PTRACE_ARCH_PRCTL already and 
PTRACE_POKEUSR writes to $fs/$gs also invoke prctl(2).  So perhaps we just 
ought to go ahead and do the same.

> They must not be enabled in Linux as use of them will lead to complete
> chaos :-).

 Hmm, that's a bit too vague for me I am afraid to accept as an argument 
in a technical discussion, but James's note about mode switches needed 
process-wide at DSO load time has convinced me, so unless you want 
something to add beyond that, feel free not to expand here.

> > > Well it technically depends on
> > > test_tsk_thread_flag(target, TIF_HYBRID_FPREGS)
> > 
> >  Sure, but the hardware representation is CP0.Config5.FRE/CP1C.FRE.
> 
> The FRE compatibility solution does require GDB to both know about and
> modify the user-view of registers as the raw register data cannot be
> interpreted by a user unassisted. My memory is a little rusty but I think
> this already happens for FR=0 vs FR=1 in that GDB is provided with 32
> 64-bit registers and must present them as either:

 The change between FR=0 and FR=1 is different as there the raw register 
format changes (and consequently the cooked as well).  Whereas for FRE=0 
vs FRE=1 only the cooked format changes.  Strictly speaking we could 
ignore cooked format changes and only pass through the raw register 
contents, however I agree in that providing the user with the available 
numeric formats the contents of an FPR or an FPR pair can be interpreted 
as is appropriate.

> FR=0
> ====
> 16 doubles by concatenating the low 32-bits of 2 consecutive registers
> to form a double.
> 32* singles by showing the low 32-bits of each register (*odd registers
> not being singles in mips V and below in FR=0.)
> 
> FR=1
> ====
> 32 doubles
> 32 singles
> (32 128-bit)
> 
> FRE=1
> =====
> 32 doubles
> 32 singles which are stored only in even numbered 64-bit registers with
> the low 32-bit being an even numbered single and the high 32-bit being
> an odd numbered single.
> (32 128-bit)
> 
> GDB cannot show the FRE=1 state correctly without knowing which mode the
> process is running in. I think this matches with comments from James
> below.

 Fair enough although arguably the native and FRE=1 states could be 
presented both at a time for the user to pick whichever he founds more 
convenient.  For example we present FR=0 singles for both even and odd FPR 
indices even on hardware which does not support single arithmetics on FPRs 
at odd indices, i.e. all legacy ISAs, up to MIPS IV inclusive.

> >  Well the contents might be unpredictable, but there sure will be some
> > and GDB is supposed to present it.
> 
> The scheme we have guarantees that no FPU mode switch ever leaves the
> register state in an unknown state which is another reason why users
> cannot change mode directly. The kernel always performs the mode switch
> and this happens with the FPU state in soft-context which is then
> restored after the mode switch occurs.

 Noted, though GDB does not actually care -- it serves as a pass-through 
really, or at least that's the intent.

> >  I haven't got to this part so far and either way will have to think
> > about it yet.  For one as I noted we do want to present vector (paired-
> > single) data with FR=1, FRE=0 in addition to what you quoted above.
> > This was all implemented in an old MIPS UK patch originally written by
> > Nigel Stephens and included with SDE, which I've never got to
> > upstreaming; have you by any chance based your work on that?
> > 
> >  As to FR=1, FRE=1 your quoted representation of singles is a software
> > convention only, so I'm not sure offhand how we might represent it in
> > GDB to keep it reasonable; the 96-bit cooked FP register structure does
> > not appeal to me at all TBH, but maybe it's the best we can do after
> > all.
> 
> The whole FR compatibility scheme has some extremely intricate design
> especially when it comes to FRE mode and I believe all tools have to play
> along in order to get the end result to be seamless for users. If we
> can do any simplification of GDB or the kernel interface then I'm open
> to ideas.

 I'll give it some thought yet and encourage everyone interested to do so 
as well.

> A reference to the spec in case anyone doesn't know where it is:
> 
> https://dmz-portal.ba.imgtec.org/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking
> 
> Note that the spec does not include a definition of the ptrace extension
> nor core dump extension (possibly not even designed yet).

 Thanks for the clarification.

> While I remember the GDB patchset does need at least checking if not
> extra work to cope with the way double precision type data is described
> in dwarf for the various compile modes.

 Noted, though with the cooked register view as outlined above it looks 
to me like register references are supposed to match with no extra effort.

 I'll see if it's worthwhile to push the original MIPS UK work first 
though.  It's been in use for over 10 years now, with our own SDE and then 
CodeSourcery toolchains, although regrettably serving non-XML-described 
targets only:

(gdb) show mips abi
The MIPS ABI is set automatically (currently "n64").
(gdb) show endian
The target endianness is set automatically (currently big endian)
(gdb) ptype $f0
type = union __gdb_builtin_type_mips_double_float_reg {
    double d;
    int64_t l;
    float s __attribute__ ((vector_size(2)));
    int32_t w __attribute__ ((vector_size(2)));
}
(gdb) info registers $f0
f0:  0xffffffffffffffff flt: -nan              dbl: -nan
(gdb) print $f0
$2 = {d = -nan(0xfffffffffffff), l = -1, s = {-nan(0x7fffff), -nan(0x7fffff)},
  w = {-1, -1}}
(gdb) set $f0.s[1] = -1
(gdb) info registers $f0
f0:  0xffffffffbf800000 flt: -1                dbl: -nan
(gdb) print $f0
$5 = {d = -nan(0xfffffbf800000), l = -1082130432, s = {-nan(0x7fffff), -1},
  w = {-1, -1082130432}}
(gdb) 

 Questions or comments?

  Maciej
  

On Fri, 21 Oct 2016, Maciej W. Rozycki wrote:

>  If we had such a dedicated virtual $fre, and we decided sometime to let 
> the user actually write to it and switch the mode process-wide, then we 
> could simply invoke the right prctl(2) call in response to the user's 
> ptrace(2) request.

 Or we could call it $fp_mode and map it directly to prctl(PR_GET_FP_MODE) 
and prctl(PR_SET_FP_MODE, ...).

  Maciej