R2300 (not the hay baler)
Hello,
Does anyone still care about the R2300? I ask because I'm working on the FP context switching code & noticed that I'm pretty sure the fpu_save_single & fpu_restore_single macros used only from r2300_switch.S are broken. They store each 32 bit value at the start of the location of the 64 bit FP registers context in memory, which I believe:
1) Won't work for odd indexed FP registers with the FPU emulator, ptrace or other code which assumes that 32 bit FP data is held in the even-indexed 64 bit FP register context.
2) On big endian systems the 32 bit values will get saved to the most significant bits of the 64 bit context which I imagine will cause yet more problems.
It seems like the only changes to r2300_switch.S for a *long* time have been to keep it in sync with r4k_switch.S & the CPU is old enough that all I get when I google for it is information about some hay baler.
In short: does anyone care if I just submit a patch removing the R2300 code instead of blindly attempting to fix it up?
Thanks,
Paul
Quoted 10 lines · Paul Burton
Does anyone still care about the R2300? I ask because I'm working on the FP context switching code & noticed that I'm pretty sure the
fpu_save_single&fpu_restore_singlemacros used only fromr2300_switch.Sare broken. They store each 32 bit value at the start of the location of the 64 bit FP registers context in memory, which I believe:1) Won't work for odd indexed FP registers with the FPU emulator, ptrace or other code which assumes that 32 bit FP data is held in the even-indexed 64 bit FP register context.
Note that much of that code has changed for 3.13 and the new code may or may not have inherited this bug.
Quoted 10 lines
2) On big endian systems the 32 bit values will get saved to the most significant bits of the 64 bit context which I imagine will cause yet more problems.
It seems like the only changes to
r2300_switch.Sfor a *long* time have been to keep it in sync withr4k_switch.S& the CPU is old enough that all I get when I google for it is information about some hay baler.In short: does anyone care if I just submit a patch removing the R2300 code instead of blindly attempting to fix it up?
Linux/MIPS is a product of the post-R3000 era - but Maciej (on cc) is doing his best to keep it alive and going on DECstations, including R2000 and R3000 based ones. DECstations are little endian and all of them have a R2010 rsp R3010, that is have hardware floating point.
I myself have an R3000-based workstation, a clone of a MIPS RS3230 (I think) sitting on my floor and waiting to be reactivated. It's still running RISC/os.
Ralf
Quoted 9 lines · Ralf Baechle
Quoted 7 lines · Paul Burton
Does anyone still care about the R2300? I ask because I'm working on the FP context switching code & noticed that I'm pretty sure the
fpu_save_single&fpu_restore_singlemacros used only fromr2300_switch.Sare broken. They store each 32 bit value at the start of the location of the 64 bit FP registers context in memory, which I believe:
Quoted 7 lines
Quoted 3 lines
1) Won't work for odd indexed FP registers with the FPU emulator, ptrace or other code which assumes that 32 bit FP data is held in the even-indexed 64 bit FP register context.
Note that much of that code has changed for 3.13 and the new code may or may not have inherited this bug.
May I ask which code you mean? Is there some FP work not in mips-for- linux-next or on the mailing list?
Quoted 16 lines
Quoted 10 lines
2) On big endian systems the 32 bit values will get saved to the most significant bits of the 64 bit context which I imagine will cause yet more problems.
It seems like the only changes to
r2300_switch.Sfor a *long* time have been to keep it in sync withr4k_switch.S& the CPU is old enough that all I get when I google for it is information about some hay baler.In short: does anyone care if I just submit a patch removing the R2300 code instead of blindly attempting to fix it up?
Linux/MIPS is a product of the post-R3000 era - but Maciej (on cc) is doing his best to keep it alive and going on DECstations, including R2000 and R3000 based ones. DECstations are little endian and all of them have a R2010 rsp R3010, that is have hardware floating point.
Quoted 6 lines
I myself have an R3000-based workstation, a clone of a MIPS RS3230 (I think) sitting on my floor and waiting to be reactivated. It's still running RISC/os.
Ralf
Fair enough, I'll leave the r2300_switch.S & fpu_{save,restore}_single code alone (apart from changes which will leave the bug as-is).
Thanks,
Paul
Quoted 15 lines · Paul Burton
Does anyone still care about the R2300? I ask because I'm working on the FP context switching code & noticed that I'm pretty sure the
fpu_save_single&fpu_restore_singlemacros used only fromr2300_switch.Sare broken. They store each 32 bit value at the start of the location of the 64 bit FP registers context in memory, which I believe:1) Won't work for odd indexed FP registers with the FPU emulator, ptrace or other code which assumes that 32 bit FP data is held in the even-indexed 64 bit FP register context.
2) On big endian systems the 32 bit values will get saved to the most significant bits of the 64 bit context which I imagine will cause yet more problems.
Quoted 6 lines
It seems like the only changes to
r2300_switch.Sfor a *long* time have been to keep it in sync withr4k_switch.S& the CPU is old enough that all I get when I google for it is information about some hay baler.In short: does anyone care if I just submit a patch removing the R2300 code instead of blindly attempting to fix it up?
Well, it worked the last time I tried (a couple of weeks ago) with actual
hardware (an R3400 integrated CPU/FPU), though maybe I missed something. There hasn't been a lot of R2000/R3000-class hardware development recently so no surprise our code didn't need any changes to match hardware updates. At this point I see no reason to retire this code, there's nothing wrong with it. If there's an actual bug, then it should be fixed. A test case should be easy to make, and then we can start from there.
If you are concerned about register layout in ptrace packets, then please
see mips_read_fp_register_single and mips_read_fp_register_double in GDB sources and the comment above them; notice the register buffer offset of 4 applied in the big-endian case -- what r2300_switch.S does is exactly what the userland expects (of course it might be that r4k_switch.S is wrong in some cases; actually I remember a discussion with Ralf where we came to this very conclusion and rather than converting r4k_switch.S to use LWC1/SWC1 -- that would degrade performance a bit for FP context switches -- considered a helper to convert between the internal and the ptrace format).
Maciej
Quoted 24 lines · Maciej W. Rozycki
Quoted 15 lines · Paul Burton
Does anyone still care about the R2300? I ask because I'm working on the FP context switching code & noticed that I'm pretty sure the
fpu_save_single&fpu_restore_singlemacros used only fromr2300_switch.Sare broken. They store each 32 bit value at the start of the location of the 64 bit FP registers context in memory, which I believe:1) Won't work for odd indexed FP registers with the FPU emulator, ptrace or other code which assumes that 32 bit FP data is held in the even-indexed 64 bit FP register context.
2) On big endian systems the 32 bit values will get saved to the most significant bits of the 64 bit context which I imagine will cause yet more problems.
Quoted 6 lines
It seems like the only changes to
r2300_switch.Sfor a *long* time have been to keep it in sync withr4k_switch.S& the CPU is old enough that all I get when I google for it is information about some hay baler.In short: does anyone care if I just submit a patch removing the R2300 code instead of blindly attempting to fix it up?
Quoted 8 lines
Well, it worked the last time I tried (a couple of weeks ago) with actual
hardware (an R3400 integrated CPU/FPU), though maybe I missed something. There hasn't been a lot of R2000/R3000-class hardware development recently so no surprise our code didn't need any changes to match hardware updates. At this point I see no reason to retire this code, there's nothing wrong with it. If there's an actual bug, then it should be fixed. A test case should be easy to make, and then we can start from there.
Yup that's fine, I'm not trying to scrap working code I just didn't realise this code was still in use (which is why I asked about it). I saw that the r2300_switch.S code seems to differ from the r4k_switch.S code in its storage of 32 bit FP context & assumed that the older code was less used & thus likely the incorrect one. It seems that assumption was incorrect given the ABI expected by userland which you point out below.
This does differ from the context layout the FPU emulator expects, but I suppose that's not an issue.
Quoted 11 lines
If you are concerned about register layout in ptrace packets, then please
seemips_read_fp_register_singleandmips_read_fp_register_doublein GDB sources and the comment above them; notice the register buffer offset of 4 applied in the big-endian case -- whatr2300_switch.Sdoes is exactly what the userland expects (of course it might be thatr4k_switch.Sis wrong in some cases; actually I remember a discussion with Ralf where we came to this very conclusion and rather than convertingr4k_switch.Sto use LWC1/SWC1 -- that would degrade performance a bit for FP context switches -- considered a helper to convert between the internal and the ptrace format).
Quoted 2 lines
Maciej
Do you know what happened to that or have a link to that discussion? I don't see that conversion being done at the moment, which makes me suspect that the kernel might handle ptrace incorrectly (arguably more nicely, but still incorrectly) for mips32 tasks with FR=0 on an R4K class CPU. I'll have a look.
Thanks,
Paul
Quoted 17 lines · Paul Burton
Quoted 8 lines
Well, it worked the last time I tried (a couple of weeks ago) with actual
hardware (an R3400 integrated CPU/FPU), though maybe I missed something. There hasn't been a lot of R2000/R3000-class hardware development recently so no surprise our code didn't need any changes to match hardware updates. At this point I see no reason to retire this code, there's nothing wrong with it. If there's an actual bug, then it should be fixed. A test case should be easy to make, and then we can start from there.Yup that's fine, I'm not trying to scrap working code I just didn't realise this code was still in use (which is why I asked about it). I saw that the
r2300_switch.Scode seems to differ from ther4k_switch.Scode in its storage of 32 bit FP context & assumed that the older code was less used & thus likely the incorrect one. It seems that assumption was incorrect given the ABI expected by userland which you point out below.
Quoted 2 lines
This does differ from the context layout the FPU emulator expects, but I suppose that's not an issue.
I reckon the emulator had similar appropriate conditionals (in helper
macros or suchlike) to handle both cases, I hope they haven't gone by accident (or I haven't got MIPS FP emulators mixed up in my memory, there are so many of them...).
Quoted 11 lines
Quoted 10 lines
If you are concerned about register layout in ptrace packets, then please
seemips_read_fp_register_singleandmips_read_fp_register_doublein GDB sources and the comment above them; notice the register buffer offset of 4 applied in the big-endian case -- whatr2300_switch.Sdoes is exactly what the userland expects (of course it might be thatr4k_switch.Sis wrong in some cases; actually I remember a discussion with Ralf where we came to this very conclusion and rather than convertingr4k_switch.Sto use LWC1/SWC1 -- that would degrade performance a bit for FP context switches -- considered a helper to convert between the internal and the ptrace format).
Quoted 5 lines
Do you know what happened to that or have a link to that discussion? I don't see that conversion being done at the moment, which makes me suspect that the kernel might handle ptrace incorrectly (arguably more nicely, but still incorrectly) for mips32 tasks with FR=0 on an R4K class CPU. I'll have a look.
I think the discussion was off-list (Ralf, would you mind if I digged up
any clues from there?). The format has been set long ago, and is also odd enough to have 32 64-bit slots in the PTRACE_GETFPREGS/PTRACE_SETFPREGS structure even for o32 processes (that now should be unexpectedly helpful for FP64 o32 processes though), so there's little sense discussing its prettiness or ugliness at this point in the game.
Also I'm not sure what the core file format is for the FP context, it may
be worth double-checking too.
Please feel free to poke me directly if you have any further issues about
MIPS I ISA compatibility.
Maciej
Quoted 2 lines · Maciej W. Rozycki
I think the discussion was off-list (Ralf, would you mind if I digged up
any clues from there?).
Please go ahead.
Quoted 8 lines
The format has been set long ago, and is also odd
enough to have 32 64-bit slots in the PTRACE_GETFPREGS/PTRACE_SETFPREGS structure even for o32 processes (that now should be unexpectedly helpful for FP64 o32 processes though), so there's little sense discussing its prettiness or ugliness at this point in the game.Also I'm not sure what the core file format is for the FP context, it may
be worth double-checking too.
Is there some test suite for that kind of stuff?
Quoted 2 lines
Please feel free to poke me directly if you have any further issues about
MIPS I ISA compatibility.
Makes me wonder if there's a MIPS emulator that emulates a MIPS I system faithfully.
Ralf
Quoted 4 lines · Ralf Baechle
Quoted 2 lines
Also I'm not sure what the core file format is for the FP context, it may
be worth double-checking too.Is there some test suite for that kind of stuff?
GDB has some core file support coverage; I'm not sure if it's meticulous
enough to catch such issues though, and also if it checks OS-produced cores or ones made with its own generate-core-file' (aka gcore') command only. Besides if ptrace(2) and the core dumper are consistent with each other, then chances are that any bugs there cancel each other too.
Quoted 5 lines
Quoted 2 lines
Please feel free to poke me directly if you have any further issues about
MIPS I ISA compatibility.Makes me wonder if there's a MIPS emulator that emulates a MIPS I system faithfully.
The Algorithmics FPU emulator (the original version shipped with their
SDE kit, not the kernel version we got contributed) could have been strapped for true MIPS I support I believe. Also GNU sim might be able to emulate a whole MIPS I CPU, although I haven't checked. Other than that I haven't seen any (QEMU does not).
Maciej
Quoted 24 lines · Maciej W. Rozycki
Quoted 11 lines
Quoted 10 lines
If you are concerned about register layout in ptrace packets, then please
seemips_read_fp_register_singleandmips_read_fp_register_doublein GDB sources and the comment above them; notice the register buffer offset of 4 applied in the big-endian case -- whatr2300_switch.Sdoes is exactly what the userland expects (of course it might be thatr4k_switch.Sis wrong in some cases; actually I remember a discussion with Ralf where we came to this very conclusion and rather than convertingr4k_switch.Sto use LWC1/SWC1 -- that would degrade performance a bit for FP context switches -- considered a helper to convert between the internal and the ptrace format).Quoted 5 lines
Do you know what happened to that or have a link to that discussion? I don't see that conversion being done at the moment, which makes me suspect that the kernel might handle ptrace incorrectly (arguably more nicely, but still incorrectly) for mips32 tasks with FR=0 on an R4K class CPU. I'll have a look.
I think the discussion was off-list (Ralf, would you mind if I digged up
any clues from there?). The format has been set long ago, and is also odd enough to have 32 64-bit slots in the PTRACE_GETFPREGS/PTRACE_SETFPREGS structure even for o32 processes (that now should be unexpectedly helpful for FP64 o32 processes though), so there's little sense discussing its prettiness or ugliness at this point in the game.
Quoted 5 lines
Also I'm not sure what the core file format is for the FP context, it may
be worth double-checking too.Please feel free to poke me directly if you have any further issues about
MIPS I ISA compatibility.
Ok I finally had time to look at this. It seems that r2300_switch.S used to match the current behaviour of r4k_switch.S. Ralf made it that way by saving to the appropriate 32 bits of the even numbered 64 bit values of the FP context, taking endianness into account, in the following commit:
...and then you fixed up ptrace to always expect values stored in the format now used by r4k_switch.S (& at the time used by r2300_switch.S too):
Unfortunately later when Ralf replaced the FPU_SAVE_SINGLE macro with the fpu_save_single macro in this commit:
...he effectively reverted r2300_switch.S to its old behaviour, whilst ptrace continues to expect the r4k_switch.S-like behaviour. So as far as I can tell the original intended FP register layout was that currently used by r4k_switch.S. That makes r2300_switch.S the incorrect one - fixed 11 years ago & broken again 10 years ago.
What I'm less sure about right now is what gdb has come to expect in the meantime - but from your description it sounds like it expects the r2300_switch.S behaviour? In which case I suspect that although it seems the original intended ptrace ABI was broken long ago & the easiest fix may be for the kernel to just go with the unintended ABI on r4k-class cores too? I'll have a read through more gdb code & try to confirm.
Thanks,
Paul
Re: MASS MAILING: Re: R2300 (not the hay baler)
Quoted 37 lines · Paul Burton
Quoted 7 lines · Maciej W. Rozycki
I think the discussion was off-list (Ralf, would you mind if I digged up
any clues from there?). The format has been set long ago, and is also odd enough to have 32 64-bit slots in the PTRACE_GETFPREGS/PTRACE_SETFPREGS structure even for o32 processes (that now should be unexpectedly helpful for FP64 o32 processes though), so there's little sense discussing its prettiness or ugliness at this point in the game.Quoted 5 lines
Also I'm not sure what the core file format is for the FP context, it may
be worth double-checking too.Please feel free to poke me directly if you have any further issues about
MIPS I ISA compatibility.Ok I finally had time to look at this. It seems that
r2300_switch.Sused to match the current behaviour ofr4k_switch.S. Ralf made it that way by saving to the appropriate 32 bits of the even numbered 64 bit values of the FP context, taking endianness into account, in the following commit:...and then you fixed up ptrace to always expect values stored in the format now used by
r4k_switch.S(& at the time used byr2300_switch.Stoo):Unfortunately later when Ralf replaced the
FPU_SAVE_SINGLEmacro with thefpu_save_singlemacro in this commit:...he effectively reverted
r2300_switch.Sto its old behaviour, whilst ptrace continues to expect the r4k_switch.S-like behaviour. So as far as I can tell the original intended FP register layout was that currently used byr4k_switch.S. That makesr2300_switch.Sthe incorrect one - fixed 11 years ago & broken again 10 years ago.
Quoted 6 lines
What I'm less sure about right now is what gdb has come to expect in the meantime - but from your description it sounds like it expects the
r2300_switch.Sbehaviour? In which case I suspect that although it seems the original intended ptrace ABI was broken long ago & the easiest fix may be for the kernel to just go with the unintended ABI on r4k-class cores too? I'll have a read through more gdb code & try to confirm.
Maciej: are you sure this is working correctly with r2300_switch.S? gdb seems to be working as I'd expect with r4k_switch.S and I have no r2k/r3k hardware to test on.
Thanks,
Paul
Quoted 4 lines · Paul Burton
Date: Tue, 19 Nov 2013 11:07:22 +0000
From: Paul Burton <[email protected]>
To: "[email protected]" <[email protected]>
Subject: R2300 (not the hay baler)
:-) on the hay baler btw.
Of course this thread is only going to make google think that linux-mips.org is into R2300 hay balers ;-)
Ralf