On Thu, Feb 26, 2015 at 09:31:36AM +0000, Matthew Fortune wrote:
> Markos Chandras <Markos.Chandras@xxxxxxxxxx> writes:
> > On Thu, Feb 26, 2015 at 09:14:41AM +0000, Måns Rullgård wrote:
> > > Markos Chandras <markos.chandras@xxxxxxxxxx> writes:
> > >
> > > > On Tue, Feb 24, 2015 at 02:26:04PM +0000, Matthew Fortune wrote:
> > > >> Måns Rullgård <mans@xxxxxxxxx> writes:
> > > >> > Markos Chandras <markos.chandras@xxxxxxxxxx> writes:
> > > >> >
> > > >> > > Hi,
> > > >> > >
> > > >> > > On Tue, Feb 24, 2015 at 01:17:33PM +0000, Måns Rullgård wrote:
> > > >> > >> This patch (well, the variant that made it into 4.0-rc1)
> > > >> > >> breaks MIPS_ABI_FP_DOUBLE (the gcc default) apps on MIPS32.
> > > >> > >>
> > > >> > >
> > > >> > > Thanks for the report.
> > > >> > >
> > > >> > >> > +void mips_set_personality_fp(struct arch_elf_state *state)
> > {
> > > >> > >> > + /*
> > > >> > >> > + * This function is only ever called for O32 ELFs so we
> > should
> > > >> > >> > + * not be worried about N32/N64 binaries.
> > > >> > >> > + */
> > > >> > >> >
> > > >> > >> > - case MIPS_ABI_FP_XX:
> > > >> > >> > - case MIPS_ABI_FP_ANY:
> > > >> > >> > - if (!config_enabled(CONFIG_MIPS_O32_FP64_SUPPORT))
> > > >> > >> > - set_thread_flag(TIF_32BIT_FPREGS);
> > > >> > >> > - else
> > > >> > >> > - clear_thread_flag(TIF_32BIT_FPREGS);
> > > >> > >> > + if (!config_enabled(CONFIG_MIPS_O32_FP64_SUPPORT))
> > > >> > >> > + return;
> > > >> > >>
> > > >> > >> The problem is here. In a 32-bit configuration,
> > > >> > >> MIPS_O32_FP64_SUPPORT is always disabled, so the FP mode
> > > >> > >> doesn't get set. Simply deleting those two lines makes things
> > > >> > >> work again, but that's probably not the right fix.
> > > >>
> > > >> I don't recall the final decision on default on/off for this option
> > > >> but IIRC it is going to be off for everything except R6 in the
> > > >> first kernel version and then turned on by default(/option removed)
> > > >> when the code is proven for the following kernel version.
> > > >>
> > > >> > >>
> > > >> > > I had the impression that the loader would have set the FP mode
> > > >> > > earlier on. But that only may happen with the latest version
> > > >> > > of the tools.
> > > >> > >
> > > >> > > Perhaps instead of dropping these two lines we need a similar
> > > >> > > check on the arch_elf_pt_proc so we don't mess with the default
> > FPI abi?
> > > >> > >
> > > >> > > Having said that, dropping these two lines should be fine, it
> > > >> > > just means you do a little bit of extra work when loading your
> > > >> > > ELF files to check for ABI compatibility which shouldn't matter
> > in your case.
> > > >> >
> > > >> > There's another early return like this in arch_check_elf() which
> > > >> > should probably go as well, or everything will end up with the
> > default mode.
> > > >>
> > > >> Ironically I discussed these changes with Markos in an attempt to
> > > >> make all the new changes benign when:
> > > >>
> > > >> !config_enabled(CONFIG_MIPS_O32_FP64_SUPPORT)
> > > >>
> > > >> Clearly this has backfired. I will have to re-read the version of
> > > >> the code in 4.0-rc1 to see what is the root cause. The intention
> > > >> was that without the config option then the kernel would blindly
> > > >> continue to assume that all O32 binaries would run in the original
> > > >> TIF_32BIT_FPREGS mode. As I recall, the callers to
> > > >> mips_set_personality_fp were setting this mode which is why the
> > simple early return was added.
> > > >>
> > > >> Thanks,
> > > >> Matthew
> > > >
> > > > I think I can see what is going on. The problem is that
> > > > mips_set_personality_fp() (as already mentioned) is not executed for
> > > > !CONFIG_MIPS_O32_FP64_SUPPORT. The reason this is a problem (i think
> > > > this could only happen in 64-bit)
> > >
> > > It's definitely causing problems on my 74Kf system.
> > >
> > > > is that SET_PERSONALITY2 clears all the thread flags related to
> > > > 32-bit and FPU. The 32-bit flags will be set again by the
> > > > SET_PERSONALITY32_O32 but the FPU flags are not since the entire
>
> OK, so this sounds like what differs from what I remember seeing. I thought
> the various SET_PERSONALITY macros were setting up the default FPU flags
> (default as in to match the default O32 FP ABI) and then this would be
> updated in mips_set_personality_fp(). Iirc then mips_set_personality_fp
> is only called for the O32 case so the FPU related flags must be correctly
> set for n32/n64 in the macros. Why not just update the O32 macros to
> set the mode?
Yes that should be fine too.
Mans, could you try the following patch please since it seems you already have
a suitable environment ready to reproduce this problem
diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index 535f196ffe02..694925a26924 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -294,6 +294,9 @@ do { \
if (personality(current->personality) != PER_LINUX) \
set_personality(PER_LINUX); \
\
+ clear_thread_flag(TIF_HYBRID_FPREGS); \
+ set_thread_flag(TIF_32BIT_FPREGS); \
+ \
mips_set_personality_fp(state); \
\
current->thread.abi = &mips_abi; \
@@ -319,6 +322,8 @@ do { \
do { \
set_thread_flag(TIF_32BIT_REGS); \
set_thread_flag(TIF_32BIT_ADDR); \
+ clear_thread_flag(TIF_HYBRID_FPREGS); \
+ set_thread_flag(TIF_32BIT_FPREGS); \
\
mips_set_personality_fp(state); \
\
If that works, I will submit a format patch asap.
--
markos
