From: Dave Hansen <dave@sr71.net>
To: linux-kernel@vger.kernel.org
Cc: Dave Hansen <dave@sr71.net>,
dave.hansen@linux.intel.com, x86@kernel.org,
fenghua.yu@intel.com, bp@alien8.de, oleg@redhat.com,
luto@amacapital.net
Subject: [PATCH 2/2] x86, fpu: fix boot time GPF with eagerfpu=off
Date: Tue, 28 Apr 2015 10:00:10 -0700 [thread overview]
Message-ID: <20150428170010.D2827334@viggo.jf.intel.com> (raw)
In-Reply-To: <20150428170009.3965F03E@viggo.jf.intel.com>
From: Dave Hansen <dave.hansen@linux.intel.com>
I'm seeing some widespread failures to boot if I boot with
eagerfpu=off. This only happens on hardware that supports
'xsaves', which is not widely available, so it is not
seen widely in practice.
The failure comes from init hitting a GPF very early in its
execution. The GPF comes from a 'xrstors' operation that
the kernel performs which kills init.
The problem is essentially that "fpu_finit()" does not have
any knowledge about how to initialize an FPU buffer
(tsk->fpu->state) which will be restored with xsaves. This
does not hit the use_eager_fpu() xsaves code because it
always does a __save_fpu() to initialize the FPU state from
the the process doing the fork()'s *registers* instead of
the 'init_xstate_buf' via init_fpu().
The code calls fx_finit(&fpu->state->fxsave) on the FPU state
to initialize the 'fxsave' style FPU buffer and then soon
after does an 'xrstors'-style restore. That promptly GPF's.
As far as I can tell init_fpu() is only called when a task
has !used_math(). That never happens in the eagerfpu=on
case.
We never noticed this problem because this commit:
e7d820a5: x86, xsave: Support eager-only xsave features, add MPX support
effectively disabled lazy FPU mode on all hardware that
supports xsaves. I think that commit itself is wrong
and there is no fundamental reason we can not support
lazy FPU mode with the MPX registers being xsave'd. But,
that's another patch.
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andy Lutomirski <luto@amacapital.net>
---
b/arch/x86/kernel/i387.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff -puN arch/x86/kernel/i387.c~x86-fpu-fix-boot-time-gpf-with-eagerfpuoff arch/x86/kernel/i387.c
--- a/arch/x86/kernel/i387.c~x86-fpu-fix-boot-time-gpf-with-eagerfpuoff 2015-04-28 09:51:55.634326943 -0700
+++ b/arch/x86/kernel/i387.c 2015-04-28 09:53:23.065270248 -0700
@@ -19,6 +19,7 @@
#include <asm/i387.h>
#include <asm/fpu-internal.h>
#include <asm/user.h>
+#include <asm/xcr.h>
static DEFINE_PER_CPU(bool, in_kernel_fpu);
@@ -218,6 +219,41 @@ void fpu_init(void)
eager_fpu_init();
}
+/*
+ * This will take the 'xsave' portion of a 'thread_xstate'
+ * and get it in to a state where an 'xrstor' or 'xrstors'
+ * of the buffer will result in all of the xsave state
+ * components being set to their initialized state.
+ *
+ * We initialize the minimal set of fields in the header
+ * and rely on the CPU to initialize the registers
+ * themselves.
+ */
+static void xsave_fpu_buf_init(struct xsave_hdr_struct *xhdr)
+{
+ /*
+ * This will cause the xrstor[s] to initialize
+ * all the xsave state components.
+ */
+ xhdr->xstate_bv = 0;
+ /*
+ * xrstors will GPF if this bit is not set
+ */
+ if (boot_cpu_has(X86_FEATURE_XSAVEC))
+ xhdr->xcomp_bv = XCOMP_BV_COMPACTION_ENABLED;
+ else
+ xhdr->xcomp_bv = 0;
+ /*
+ * SDM Says we will GPF:
+ * "If bytes 63:16 of the XSAVE header are not all zero."
+ * If this reserved size ever changes, we will need to
+ * update this code to look at and conditionally
+ * initialize the new fields, just like ->xcomp_bv above.
+ */
+ BUILD_BUG_ON(sizeof(xhdr->reserved) != 48);
+ memset(xhdr->reserved, 0, sizeof(xhdr->reserved));
+}
+
void fpu_finit(struct fpu *fpu)
{
if (!cpu_has_fpu) {
@@ -225,6 +261,15 @@ void fpu_finit(struct fpu *fpu)
return;
}
+ /*
+ * The xsave code does not require the entire buffer
+ * to be zeroed, only a chunk of the header.
+ */
+ if (cpu_has_xsave) {
+ xsave_fpu_buf_init(&fpu->state->xsave.xsave_hdr);
+ return;
+ }
+
memset(fpu->state, 0, xstate_size);
if (cpu_has_fxsr) {
_
prev parent reply other threads:[~2015-04-28 16:59 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-28 17:00 [PATCH 1/2] x86, fpu: use kvm definition of compaction bit and make it generic Dave Hansen
2015-04-28 17:00 ` Dave Hansen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150428170010.D2827334@viggo.jf.intel.com \
--to=dave@sr71.net \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=fenghua.yu@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=oleg@redhat.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®