Messages in this thread Patch in this message | | | Date | Fri, 10 Feb 2017 08:54:45 -0500 | From | Rik van Riel <> | Subject | [PATCH v3] x86/fpu: copy MXCSR & MXCSR_FLAGS with SSE/YMM state |
| |
On Fri, 10 Feb 2017 09:00:54 +0100 Ingo Molnar <mingo@kernel.org> 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 <riel@redhat.com> --- 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':
| |