On Wed, Jan 25, 2017 at 08:57:59PM -0500, r...@redhat.com wrote:
> From: Rik van Riel <r...@redhat.com>
>
> 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 <r...@redhat.com>
> ---
> arch/x86/kernel/fpu/xstate.c | 39 ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index c1508d56ecfb..10b10917af81 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -1004,6 +1004,23 @@ int copyout_from_xsaves(unsigned int pos, unsigned int
> count, void *kbuf,
> }
>
> /*
> + * Restoring SSE/YMM state requires that MXCSR & MXCSR_MASK are saved.
> + * Those fields are part of the legacy FP state, and only get saved
> + * above if XFEATURES_MASK_FP is set.
> + *
> + * Copy out those fields if we have SSE/YMM but no FP register data.
> + */
> + if ((header.xfeatures & (XFEATURE_MASK_SSE|XFEATURE_MASK_YMM)) &&
> + !(header.xfeatures & XFEATURE_MASK_FP)) {
> + size = sizeof(u64);
> + ret = xstate_copyout(offset, size, kbuf, ubuf,
> + &xsave->i387.mxcsr, 0, count);
> +
> + if (ret)
> + return ret;
> + }
> +
> + /*
> * Fill xsave->i387.sw_reserved value for ptrace frame:
> */
> offset = offsetof(struct fxregs_state, sw_reserved);
> @@ -1030,6 +1047,7 @@ int copyin_to_xsaves(const void *kbuf, const void
> __user *ubuf,
> int i;
> u64 xfeatures;
> u64 allowed_features;
> + void *dst;
>
> offset = offsetof(struct xregs_state, header);
> size = sizeof(xfeatures);
> @@ -1053,7 +1071,7 @@ int copyin_to_xsaves(const void *kbuf, const void
> __user *ubuf,
> u64 mask = ((u64)1 << i);
>
> if (xfeatures & mask) {
> - void *dst = __raw_xsave_addr(xsave, 1 << i);
> + dst = __raw_xsave_addr(xsave, 1 << i);
>
> offset = xstate_offsets[i];
> size = xstate_sizes[i];
> @@ -1068,6 +1086,25 @@ int copyin_to_xsaves(const void *kbuf, const void
> __user *ubuf,
> }
>
> /*
> + * SSE/YMM state depends on the MXCSR & MXCSR_MASK fields from the FP
> + * state. If we restored only SSE/YMM state but not FP state, copy
> + * those fields to ensure the SSE/YMM state restore works.
> + */
In xstateregs_set(), we enforced the starting pos must be from (0), which in
XSAVE time, was probably for this reason. The real mistake here, I think, is
allowing skipping of xstate[0] and xstate[1]. Both should have been there
even for XSAVES compacted-format. Would it be a simpler fix just making sure
xstate[0] and xstate[1] are copied?
Yu-cheng
