lkml.org 
[lkml]   [2017]   [Jan]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/2] x86/fpu: copy MXCSR & MXCSR_FLAGS with SSE/YMM state
    On Wed, Jan 25, 2017 at 08:57:59PM -0500, riel@redhat.com wrote:
    > From: Rik van Riel <riel@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 <riel@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


    \
     
     \ /
      Last update: 2017-01-30 18:39    [W:4.295 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site