On 19/11/13 12:27, Maciej W. Rozycki wrote:
On Tue, 19 Nov 2013, Paul Burton wrote:
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?
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.
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
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
