Re: [PATCH v3] MIPS: kernel: elf: Improve the overall ABI and FPU mode checks
From: Måns Rullgård <hidden>
Date: 2015-02-26 10:31:45
Markos Chandras [off-list ref] writes:
quoted hunk ↗ jump to hunk
On Thu, Feb 26, 2015 at 09:31:36AM +0000, Matthew Fortune wrote:quoted
Markos Chandras [off-list ref] writes:quoted
On Thu, Feb 26, 2015 at 09:14:41AM +0000, Måns Rullgård wrote:quoted
Markos Chandras [off-list ref] writes:quoted
On Tue, Feb 24, 2015 at 02:26:04PM +0000, Matthew Fortune wrote:quoted
Måns Rullgård [off-list ref] writes:quoted
Markos Chandras [off-list ref] writes:quoted
Hi, On Tue, Feb 24, 2015 at 01:17:33PM +0000, Måns Rullgård wrote:quoted
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.quoted
quoted
+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.quoted
quoted
quoted
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, MatthewI 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.quoted
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 entireOK, 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 problemdiff --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.
Seems to work. -- Måns Rullgård mans@mansr.com