From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030844AbbD1Q76 (ORCPT ); Tue, 28 Apr 2015 12:59:58 -0400 Received: from mga09.intel.com ([134.134.136.24]:39741 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030443AbbD1Q75 (ORCPT ); Tue, 28 Apr 2015 12:59:57 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,664,1422950400"; d="scan'208";a="486771639" Subject: [PATCH 2/2] x86, fpu: fix boot time GPF with eagerfpu=off To: linux-kernel@vger.kernel.org Cc: Dave Hansen , dave.hansen@linux.intel.com, x86@kernel.org, fenghua.yu@intel.com, bp@alien8.de, oleg@redhat.com, luto@amacapital.net From: Dave Hansen Date: Tue, 28 Apr 2015 10:00:10 -0700 References: <20150428170009.3965F03E@viggo.jf.intel.com> In-Reply-To: <20150428170009.3965F03E@viggo.jf.intel.com> Message-Id: <20150428170010.D2827334@viggo.jf.intel.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Dave Hansen 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 Cc: x86@kernel.org Cc: Fenghua Yu Cc: Borislav Petkov Cc: Oleg Nesterov Cc: Andy Lutomirski --- 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 #include #include +#include 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) { _