[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d312c572-f232-a4e9-2ecc-023050528a29@huawei.com>
Date: Sat, 7 Jan 2023 10:10:18 +0800
From: Miaohe Lin <linmiaohe@...wei.com>
To: Borislav Petkov <bp@...en8.de>
CC: "x86@...nel.org" <x86@...nel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>,
Dave Hansen <dave.hansen@...ux.intel.com>, <hpa@...or.com>
Subject: Re: [BUG Report] undefined reference to `convert_to_fxsr'
On 2023/1/6 7:14, Borislav Petkov wrote:
> On Tue, Jan 03, 2023 at 11:05:01AM +0800, Miaohe Lin wrote:
>> Yes, it still reproduces in my working server. It might be the problem of gcc
>> version.
>
> What is that compiler you're using? Where did you get the package from? Does it
> have some out-of-tree patches in it?
My compiler is gcc 7.3.0:
linux-rtfsc:/home/linmiaohe/linux # gcc --version
gcc (GCC) 7.3.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
linux-rtfsc:/home/linmiaohe/linux #
And I cloned the code from https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git. It's a
clean git repo without applying any out-of-tree patch to it.
>
>> I think it's because convert_to_fxsr() is only defined when CONFIG_X86_32 or
>> CONFIG_IA32_EMULATION is enabled.
>
> No, convert_to_fxsr() is part of arch/x86/kernel/fpu/regset.c which is built-in
> unconditionally.
>
> What happens is this here:
>
> bool fpu__restore_sig(void __user *buf, int ia32_frame)
>
> ...
>
> } else {
> success = __fpu_restore_sig(buf, buf_fx, ia32_fxstate);
> }
>
> That ia32_fxstate is false because
>
> ia32_frame &= (IS_ENABLED(CONFIG_X86_32) ||
> IS_ENABLED(CONFIG_IA32_EMULATION));
>
>
> neither of those config items are set...
>
> /*
> * Only FXSR enabled systems need the FX state quirk.
> * FRSTOR does not need it and can use the fast path.
> */
> if (ia32_frame && use_fxsr()) {
> buf_fx = buf + sizeof(struct fregs_state);
> size += sizeof(struct fregs_state);
> ia32_fxstate = true;
> ^^^^^^^^^^^^^^^^^^^
>
> ... so this doesn't happen.
>
> }
>
> Then, in __fpu_restore_sig() you have:
>
> if (likely(!ia32_fxstate)) {
> /* Restore the FPU registers directly from user memory. */
> return restore_fpregs_from_user(buf_fx, user_xfeatures, fx_only,
> state_size);
> }
>
> and since ia32_fxstate is false, we return here, the compiler sees that
> everything behind that code is dead code and eliminates it.
Many thanks for your explanation! It's really helpful. I checked generated asm code
and I found calling to convert_to_fxsr is not eliminated as expected:
objdump -S arch/x86/kernel/fpu/signal.o
3fc: e9 00 00 00 00 jmpq 401 <__fpu_restore_sig+0x401>
fpregs->xsave.header.xfeatures |= XFEATURE_MASK_FPSSE;
401: 48 83 8d 40 02 00 00 orq $0x3,0x240(%rbp)
408: 03
convert_to_fxsr(&fpregs->fxsave, &env);
^^^^^^^^^^^^^^^
409: 48 8d 74 24 10 lea 0x10(%rsp),%rsi
linux-rtfsc:/home/linmiaohe/linux # objdump -S arch/x86/kernel/fpu/signal.o | grep convert_to_fxsr
convert_to_fxsr(&fpregs->fxsave, &env);
linux-rtfsc:/home/linmiaohe/linux #
And if I compile the code with below patch, convert_to_fxsr is eliminated:
linux-rtfsc:/home/linmiaohe/linux # objdump -S arch/x86/kernel/fpu/signal.o | grep convert_to_fxsr
linux-rtfsc:/home/linmiaohe/linux #
diff --git a/arch/x86/include/asm/fpu/signal.h b/arch/x86/include/asm/fpu/signal.h
index 611fa41711af..77ea052a8967 100644
--- a/arch/x86/include/asm/fpu/signal.h
+++ b/arch/x86/include/asm/fpu/signal.h
@@ -20,8 +20,14 @@
extern void convert_from_fxsr(struct user_i387_ia32_struct *env,
struct task_struct *tsk);
+#if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
extern void convert_to_fxsr(struct fxregs_state *fxsave,
const struct user_i387_ia32_struct *env);
+#else
+static inline void convert_to_fxsr(struct fxregs_state *fxsave,
+ const struct user_i387_ia32_struct *env)
+{}
+#endif
unsigned long
fpu__alloc_mathframe(unsigned long sp, int ia32_frame,
--
2.27.0
>
> Your compiler doesn't, apparently.
>
> It does remove it from regset.c, though, as it sees it is an unused function,
> which leads to this undefined reference.
>
> So it looks like a funky compiler to me...
It seems another issue with old compiler. Sigh... :(
Thanks,
Miaohe Lin
Powered by blists - more mailing lists