RE: [PATCH v3] MIPS: kernel: elf: Improve the overall ABI and FPU mode checks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux