Make CoredumpAction use Task.getRegisterBanks()

Make CoredumpAction use Task.getRegisterBanks()

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]

Hi,

Phil and me were discussing cleaning up some of the
Memory/RegisterBuffers between ptrace and core proc Tasks on irc and
noticed that CoredumpAction was accessing the RegisterBanks directly
through the Isa. To make splitting the issue of getting the
RegisterBanks (Task specific) and doing the Register name mapping to
bank number and offset (Isa specific) easier we wanted to make all code
go through Task.getRegisterBanks(). This patch does that for
CoredumpAction:

2007-07-30  Mark Wielaard  <mwielaard@redhat.com>

    * CoredumpAction.java (): Use Task.getRegisterBanks() not
    Isa.getRegisterBankBuffers().

Tested on x86 and x86_64 without regressions.

Cheers,

Mark

diff -u -r1.16 CoredumpAction.java
--- frysk-core/frysk/util/CoredumpAction.java   16 Jul 2007 22:11:35 -0000     1.16
+++ frysk-core/frysk/util/CoredumpAction.java   30 Jul 2007 16:40:07 -0000
@@ -235,11 +235,8 @@
   {
     // New PRSTATUS Note Entry.
     ElfPrFPRegSet fpRegSet = new ElfPrFPRegSet();
-    Isa register = null;
-
-    register = task.getIsa();
 
-    ByteBuffer registerMaps[] = register.getRegisterBankBuffers(task.getProc().getPid());
+    ByteBuffer registerMaps[] = task.getRegisterBanks();
     if (registerMaps[1].capacity() <= 0)
       abandonCoreDump(new RuntimeException("FP Register bank is <=0"));
     byte[] regBuffer = new byte[(int) registerMaps[1].capacity()];


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Make CoredumpAction use Task.getRegisterBanks()

[Phil Muldoon]

Mark Wielaard wrote:
> Hi,
>
> Phil and me were discussing cleaning up some of the
> Memory/RegisterBuffers between ptrace and core proc Tasks on irc and
> noticed that CoredumpAction was accessing the RegisterBanks directly
> through the Isa. To make splitting the issue of getting the
> RegisterBanks (Task specific) and doing the Register name mapping to
> bank number and offset (Isa specific) easier we wanted to make all code
> go through Task.getRegisterBanks(). This patch does that for
> CoredumpAction:
>   

Looks good. The only function here is to attain the raw memory behind 
the byte buffers for floating point registers, as that is dumped whole 
into the FP Register Note.

CoredumpAction has been gutted locally and being refactored into a 
Corefile{arch}factory. I'll take your patch and reconstitute it there

Many Thanks

Phil

Re: Make CoredumpAction use Task.getRegisterBanks()

[Mark Wielaard]

Hi Phil,

On Mon, 2007-07-30 at 12:44 -0500, Phil Muldoon wrote:
> Mark Wielaard wrote:
> > Phil and me were discussing cleaning up some of the
> > Memory/RegisterBuffers between ptrace and core proc Tasks on irc and
> > noticed that CoredumpAction was accessing the RegisterBanks directly
> > through the Isa. To make splitting the issue of getting the
> > RegisterBanks (Task specific) and doing the Register name mapping to
> > bank number and offset (Isa specific) easier we wanted to make all code
> > go through Task.getRegisterBanks(). This patch does that for
> > CoredumpAction:
> >   
> Looks good. The only function here is to attain the raw memory behind 
> the byte buffers for floating point registers, as that is dumped whole 
> into the FP Register Note.
> 
> CoredumpAction has been gutted locally and being refactored into a 
> Corefile{arch}factory. I'll take your patch and reconstitute it there

Thanks. Please let me know if I should hold off on the rest of the
register banks cleanup till you finished this. Or if you want me to take
a look at what you have now to get a better understanding how to clean
up these buffers.

For ptrace/proc based Tasks there are basically 4 register banks (not
all of them used on all architectures) the Isa provides a mapping from
the actual register names to the bank and offset inside it. The four
banks are the regular registers (gotten through PTRACE_GETREGS), the
general floating point registers (gotten through PTRACE_GETFPREGS), the
extended floating point registers such like sse xmm registers (gotten
through PTRACE_getXFPREGS) and the other registers (gotten through the
start of USR memory space) that cover things like debug control
registers found by the Isa through the mapping in sys/user.h (some of
the other register banks are actually also contained in this one).

Does this map somewhat to how core files handle the register banks, so
you can just dump/read these banks from them?

Cheers,

Mark

Re: Make CoredumpAction use Task.getRegisterBanks()

[Phil Muldoon]

Mark Wielaard wrote:
>
> For ptrace/proc based Tasks there are basically 4 register banks (not
> all of them used on all architectures) the Isa provides a mapping from
> the actual register names to the bank and offset inside it. The four
> banks are the regular registers (gotten through PTRACE_GETREGS), the
> general floating point registers (gotten through PTRACE_GETFPREGS), the
> extended floating point registers such like sse xmm registers (gotten
> through PTRACE_getXFPREGS) and the other registers (gotten through the
> start of USR memory space) that cover things like debug control
> registers found by the Isa through the mapping in sys/user.h (some of
> the other register banks are actually also contained in this one).
>
> Does this map somewhat to how core files handle the register banks, so
> you can just dump/read these banks from them?
>   

Pretty much other than General Purpose registers which needs to be 
accessed independently via getRegisterByName(). The the rest are just 
dumped  wholesale into the notes, and in that instance accessed via 
task.getRegisterBanks().

I think, going forward access to the raw memory  for each register bank, 
and also a getRegisterByName() function (which is the functionality we 
have now) is needed, and probably will be down the line.

Regards

Phil

Re: Make CoredumpAction use Task.getRegisterBanks()

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 2185 bytes --]

On Tue, 2007-07-31 at 19:26 -0500, Phil Muldoon wrote:
> Mark Wielaard wrote:
> >
> > For ptrace/proc based Tasks there are basically 4 register banks (not
> > all of them used on all architectures) the Isa provides a mapping from
> > the actual register names to the bank and offset inside it. The four
> > banks are the regular registers (gotten through PTRACE_GETREGS), the
> > general floating point registers (gotten through PTRACE_GETFPREGS), the
> > extended floating point registers such like sse xmm registers (gotten
> > through PTRACE_getXFPREGS) and the other registers (gotten through the
> > start of USR memory space) that cover things like debug control
> > registers found by the Isa through the mapping in sys/user.h (some of
> > the other register banks are actually also contained in this one).
> >
> > Does this map somewhat to how core files handle the register banks, so
> > you can just dump/read these banks from them?
> >   
> 
> Pretty much other than General Purpose registers which needs to be 
> accessed independently via getRegisterByName(). The the rest are just 
> dumped  wholesale into the notes, and in that instance accessed via 
> task.getRegisterBanks().

Thanks. So this seems slightly different from how we do things with
ptrace/proc where we can have multiple different register banks (for
floating point, extended floating point and control/debug registers). We
might be able to always just use the USR address space to get at all of
those together in one go (I don't actually know why we don't do that, or
why ptrace provides different ways to access the same register sets).

> I think, going forward access to the raw memory  for each register bank, 
> and also a getRegisterByName() function (which is the functionality we 
> have now) is needed, and probably will be down the line.

If we can match up the raw memory for each register bank between
ptrace/proc and core files from Task that would be ideal. Then the Isa
can just do the getRegisterByName() mapping. I'll watch your rewrite of
the core file stuff and see if this makes things easier and clearer (I
guess it will).

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Make CoredumpAction use Task.getRegisterBanks()

[Phil Muldoon]

Mark Wielaard wrote:
>
> Thanks. So this seems slightly different from how we do things with
> ptrace/proc where we can have multiple different register banks (for
> floating point, extended floating point and control/debug registers). We
> might be able to always just use the USR address space to get at all of
> those together in one go (I don't actually know why we don't do that, or
> why ptrace provides different ways to access the same register sets).
>
>   

Apologies for the delay in replying.

For the FP and fpxregs (which I am writing code for right now), I just 
take the entire register buffer and dump it wholesale into the 
appropriate note in the core file. For GP registers there are some 
cross-isa issues (mainly in PPC) that require the code to pick them 
individually, sort them into order, them populate them as part of 
another note, the elf_prstatus note. This is an important distinction as 
GP registers do not have their own separate note area in core files, but 
are part of the thread elf_prstatus note. That is why both raw memory 
banks, and getRegisterByName are needed.

> If we can match up the raw memory for each register bank between
> ptrace/proc and core files from Task that would be ideal. Then the Isa
> can just do the getRegisterByName() mapping. I'll watch your rewrite of
> the core file stuff and see if this makes things easier and clearer (I
> guess it will).
>   

The two requirements needed for core files are:

1) Access to the raw memory behind the register via a ByteBuffer
2) Access to a logical method of getting registers by name, like 
getRegisterByName()

Regards

Phil

Re: Make CoredumpAction use Task.getRegisterBanks()

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 2113 bytes --]

Hi Phil,

I tried to look up the "core file spec" but it seems no such thing exits
you just need to grep through binutils, gdb and kernel sources to figure
it all out it seems. Actually various searches all seem to finally point
to your frysk core file support (You are the expert now!) :)

On Fri, 2007-08-03 at 09:18 -0500, Phil Muldoon wrote:
> For the FP and fpxregs (which I am writing code for right now), I just 
> take the entire register buffer and dump it wholesale into the 
> appropriate note in the core file.

ACK. I saw bug #4890. It looks like whether or not the fp and fpx
register banks exist in the first place is platform specific. But I
guess this is another one of these "what does the kernel dump and how
does gdb read it back in" types of questions...

> > If we can match up the raw memory for each register bank between
> > ptrace/proc and core files from Task that would be ideal. Then the Isa
> > can just do the getRegisterByName() mapping. I'll watch your rewrite of
> > the core file stuff and see if this makes things easier and clearer (I
> > guess it will).
> >   
> The two requirements needed for core files are:
> 
> 1) Access to the raw memory behind the register via a ByteBuffer
> 2) Access to a logical method of getting registers by name, like 
> getRegisterByName()

And it can then provide those back for the core file as Task consumer as
right?

The question I am trying to answer is whether or not we can move
getRegisterBankBuffers() fully into the (ptrace) Task and let the Isa
use a call on the Task to provide the getRegisterByName() mapping
functionality from bank/numer to register name. Currently Task (or any
other Task consumer can) call back into the Isa to get at the actual
register banks, which is messy (and will produce the wrong results for
core file backed Tasks). So if we can match up the results of
Task.getRegisterBanks()/sendrecRegisterBanks() between ptrace/core Task
implementations then we could clean the Isa interface up and make the
Isa/Task user always get the correct result.

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Make CoredumpAction use Task.getRegisterBanks()

[Phil Muldoon]

Mark Wielaard wrote:
>> I think, going forward access to the raw memory  for each register bank, 
>> and also a getRegisterByName() function (which is the functionality we 
>> have now) is needed, and probably will be down the line.
>>     
>
> If we can match up the raw memory for each register bank between
> ptrace/proc and core files from Task that would be ideal. Then the Isa
> can just do the getRegisterByName() mapping. I'll watch your rewrite of
> the core file stuff and see if this makes things easier and clearer (I
> guess it will).
>
>   
Now that all the register stuff is in, lets open this up again. In a 
corefile there are 3 sets of registers written for x86:

General Purpose (GP) Registers
Floating Point Registers (FP)
eXtended Floating Point (XFP) Registers

The first, GP Registers are written as part of a Elf prStatus note. One 
per thread. Because these registers are part of "another" struct, and do 
not have a note all to themselves, they have to be read one by one, 
sorted, then written into the structure along with other prStatus data 
(tid, and so on).

Both FP and XFP both have their own note types and can be written 
wholesale as a bytebuffer to the corefile. No interpretation needed.

Whether raw register memory access belongs in the ISA, the Task is not 
really important from a corefile view, though I would like to see it 
consistent.

Regards

Phil