public inbox for gdb@sourceware.org
 help / color / mirror / Atom feed
* Adding MIPS registers (was Re: [PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL)
       [not found]     ` <alpine.DEB.1.10.1409031517450.27075@tp.orcam.me.uk>
@ 2014-09-09 16:46       ` James Hogan
  2014-09-09 17:57         ` Maciej W. Rozycki
  0 siblings, 1 reply; 5+ messages in thread
From: James Hogan @ 2014-09-09 16:46 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb

Hi Maciej,

On 03/09/14 22:21, Maciej W. Rozycki wrote:
> On Wed, 3 Sep 2014, James Hogan wrote:
>> This is my first GDB patch submitted upstream (although I have a pile of
>> RFC patches for FR=1, FRE=1, MSA support I'm still getting into shape),
>> so I don't have push access yet. Is it easy to arrange?
> 
>  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.

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).
* expose CP0 Config5 as a new register (contains the FRE bit, only
really relevant when FR=1)
* (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).

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

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.

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.

Thanks
James

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Adding MIPS registers (was Re: [PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL)
  2014-09-09 16:46       ` Adding MIPS registers (was Re: [PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL) James Hogan
@ 2014-09-09 17:57         ` Maciej W. Rozycki
  2014-09-09 20:39           ` James Hogan
  0 siblings, 1 reply; 5+ messages in thread
From: Maciej W. Rozycki @ 2014-09-09 17:57 UTC (permalink / raw)
  To: James Hogan; +Cc: gdb

James,

> >  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.

> 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, with the UFR 
feature, so the register set presented will have to change dynamically, 
according to that setting even for Linux targets.  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.

> * 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), 
what is this bit for and why was it added to CP0.Config5 rather than 
CP0.Status?  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.

> * (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?

> 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?

 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.

> 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.

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

  Maciej

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Adding MIPS registers (was Re: [PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL)
  2014-09-09 17:57         ` Maciej W. Rozycki
@ 2014-09-09 20:39           ` James Hogan
  2014-09-10  6:47             ` Paul Burton
  0 siblings, 1 reply; 5+ messages in thread
From: James Hogan @ 2014-09-09 20:39 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb, Matthew Fortune, Paul Burton

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Adding MIPS registers (was Re: [PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL)
  2014-09-09 20:39           ` James Hogan
@ 2014-09-10  6:47             ` Paul Burton
  2014-09-10  7:44               ` Matthew Fortune
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Burton @ 2014-09-10  6:47 UTC (permalink / raw)
  To: James Hogan; +Cc: Maciej W. Rozycki, gdb, Matthew Fortune

On Tue, Sep 09, 2014 at 09:39:22PM +0100, James Hogan wrote:
> > 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.

Correct, UFR (along with UFE) is currently not enabled by Linux, and I'm
not aware of any need to deal with the pain involved in enabling it. There
is a need to switch mode across a whole process, which will need to be met
in some other way (the prctls in my internal branch, and hopefully the
same once upstream).

Paul

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: Adding MIPS registers (was Re: [PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL)
  2014-09-10  6:47             ` Paul Burton
@ 2014-09-10  7:44               ` Matthew Fortune
  0 siblings, 0 replies; 5+ messages in thread
From: Matthew Fortune @ 2014-09-10  7:44 UTC (permalink / raw)
  To: Paul Burton, James Hogan; +Cc: Maciej W. Rozycki, gdb


> On Tue, Sep 09, 2014 at 09:39:22PM +0100, James Hogan wrote:
> > > 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.
> 
> Correct, UFR (along with UFE) is currently not enabled by Linux, and I'm
> not aware of any need to deal with the pain involved in enabling it. There
> is a need to switch mode across a whole process, which will need to be met
> in some other way (the prctls in my internal branch, and hopefully the
> same once upstream).

We are quite a long way off topic from the original post but all this is
important for the bulk of James' further patches...

The O32 FR0/FR1 ABI documentation includes basic details of the new FRE
hardware mode and the corresponding new ABI extension FP64A:

https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking#7._Defining_O32_FP64A

A formal PRA document with FRE described is due within a week or so.

It also shows how the program loader is expected to behave:

https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking#10.4._Program_loader

And how mode switching is to be performed:

https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking#10.5.2._Setting_the_FPU_mode

I would like to propose that GDB is not able to change the FR or FRE bits
directly. These bits have to be handled with extreme caution and there is
no plan to support users changing them at arbitrary points either on bare
metal or user-land. The intention is that a program is provided with a
stable environment to run in where every loaded function (and region of
a function) can be executed in the current mode. When adding new code into
a process then a new hardware mode may be required but the change of mode
is performed atomically across all threads in a process. For bare-metal
there is almost no reason anyone will need a mode switch so that can be
disregarded entirely.

I'll reply about register layouts separately although that topic is mostly
subjective.

Thanks,
Matthew

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-09-10  7:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1409608120-23991-1-git-send-email-james.hogan@imgtec.com>
     [not found] ` <alpine.DEB.1.10.1409031340130.27075@tp.orcam.me.uk>
     [not found]   ` <20140903125111.GF12084@jhogan-linux.le.imgtec.org>
     [not found]     ` <alpine.DEB.1.10.1409031517450.27075@tp.orcam.me.uk>
2014-09-09 16:46       ` Adding MIPS registers (was Re: [PATCH v2] Reset errno before PTRACE_PEEKUSER for MIPS DSP_CONTROL) James Hogan
2014-09-09 17:57         ` Maciej W. Rozycki
2014-09-09 20:39           ` James Hogan
2014-09-10  6:47             ` Paul Burton
2014-09-10  7:44               ` Matthew Fortune

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).