From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753168AbdBJOWX (ORCPT ); Fri, 10 Feb 2017 09:22:23 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35816 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753011AbdBJOWV (ORCPT ); Fri, 10 Feb 2017 09:22:21 -0500 Date: Fri, 10 Feb 2017 08:54:45 -0500 From: Rik van Riel To: Ingo Molnar Cc: Borislav Petkov , linux-kernel@vger.kernel.org, luto@kernel.org, dave.hansen@linux.intel.com, yu-cheng.yu@intel.com, hpa@zytor.com Subject: [PATCH v3] x86/fpu: copy MXCSR & MXCSR_FLAGS with SSE/YMM state Message-ID: <20170210085445.0f1cc708@annuminas.surriel.com> In-Reply-To: <20170210080054.GA30386@gmail.com> References: <20170209184347.2ef977b9@annuminas.surriel.com> <20170210000224.qgwwyozscwjegpqm@pd.tnic> <1486687889.2096.29.camel@redhat.com> <20170210080054.GA30386@gmail.com> Organization: Red Hat MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Fri, 10 Feb 2017 13:54:50 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 10 Feb 2017 09:00:54 +0100 Ingo Molnar wrote: > > /* Copy both mxcsr & mxcsr_flags */ > > #define MXCSR_AND_FLAGS_SIZE sizeof(u64) > > Yeah, that define would make it pretty clear what's going on. Please make it a bit > more vebose: > > /* Copy both mxcsr & mxcsr_flags with a single u64 memcpy: */ > #define MXCSR_AND_FLAGS_SIZE sizeof(u64) OK, here it is :) ---8<--- Subject: x86/fpu: copy MXCSR & MXCSR_FLAGS with SSE/YMM state On Skylake CPUs I noticed that XRSTOR is unable to deal with states created by copyout_from_xsaves if the xstate has only SSE/YMM state, and no FP state. That is, xfeatures had XFEATURE_MASK_SSE set, but not XFEATURE_MASK_FP. The reason is that part of the SSE/YMM state lives in the MXCSR and MXCSR_FLAGS fields of the FP state. Ensure that whenever we copy SSE or YMM state around, the MXCSR and MXCSR_FLAGS fields are also copied around. Signed-off-by: Rik van Riel --- arch/x86/include/asm/fpu/types.h | 3 +++ arch/x86/kernel/fpu/xstate.c | 44 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h index d15cbfe0e8c4..ea65ab22e349 100644 --- a/arch/x86/include/asm/fpu/types.h +++ b/arch/x86/include/asm/fpu/types.h @@ -68,6 +68,9 @@ struct fxregs_state { /* Default value for fxregs_state.mxcsr: */ #define MXCSR_DEFAULT 0x1f80 +/* Copy both mxcsr & mxcsr_flags with a single u64 memcpy: */ +#define MXCSR_AND_FLAGS_SIZE sizeof(u64) + /* * Software based FPU emulation state. This is arbitrary really, * it matches the x87 format to make it easier to understand: diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 772a069f8fbf..992a7f8f4988 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -920,6 +920,23 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, #endif /* ! CONFIG_ARCH_HAS_PKEYS */ /* + * Weird legacy quirk: SSE and YMM states store information in the + * MXCSR and MXCSR_FLAGS fields of the FP area. That means if the FP + * area is marked is unused in the xfeatures header, we need to copy + * MXCSR and MXCSR_FLAGS if either SSE or YMM are in use. + */ +static inline bool xfeatures_need_mxcsr_copy(u64 xfeatures) +{ + if (!(xfeatures & (XFEATURE_MASK_SSE|XFEATURE_MASK_YMM))) + return 0; + + if (xfeatures & XFEATURE_MASK_FP) + return 0; + + return 1; +} + +/* * This is similar to user_regset_copyout(), but will not add offset to * the source data pointer or increment pos, count, kbuf, and ubuf. */ @@ -987,6 +1004,13 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int of } + if (xfeatures_need_mxcsr_copy(header.xfeatures)) { + offset = offsetof(struct fxregs_state, mxcsr); + size = MXCSR_AND_FLAGS_SIZE; + __copy_xstate_to_kernel(kbuf, &xsave->i387.mxcsr, offset, + size, size_total); + } + /* * Fill xsave->i387.sw_reserved value for ptrace frame: */ @@ -1069,6 +1093,13 @@ int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned i } + if (xfeatures_need_mxcsr_copy(header.xfeatures)) { + offset = offsetof(struct fxregs_state, mxcsr); + size = MXCSR_AND_FLAGS_SIZE; + __copy_xstate_to_user(ubuf, &xsave->i387.mxcsr, offset, + size, size_total); + } + /* * Fill xsave->i387.sw_reserved value for ptrace frame: */ @@ -1121,6 +1152,12 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf) } } + if (xfeatures_need_mxcsr_copy(xfeatures)) { + offset = offsetof(struct fxregs_state, mxcsr); + size = MXCSR_AND_FLAGS_SIZE; + memcpy(&xsave->i387.mxcsr, kbuf + offset, size); + } + /* * The state that came in from userspace was user-state only. * Mask all the user states out of 'xfeatures': @@ -1176,6 +1213,13 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf) } } + if (xfeatures_need_mxcsr_copy(xfeatures)) { + offset = offsetof(struct fxregs_state, mxcsr); + size = MXCSR_AND_FLAGS_SIZE; + if (__copy_from_user(&xsave->i387.mxcsr, ubuf + offset, size)) + return -EFAULT; + } + /* * The state that came in from userspace was user-state only. * Mask all the user states out of 'xfeatures':