From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751128AbdBJBSd (ORCPT ); Thu, 9 Feb 2017 20:18:33 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47038 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751007AbdBJBSc (ORCPT ); Thu, 9 Feb 2017 20:18:32 -0500 Message-ID: <1486687889.2096.29.camel@redhat.com> Subject: Re: [PATCH v2] x86/fpu: copy MXCSR & MXCSR_FLAGS with SSE/YMM state From: Rik van Riel To: Borislav Petkov Cc: mingo@kernel.org, linux-kernel@vger.kernel.org, luto@kernel.org, dave.hansen@linux.intel.com, yu-cheng.yu@intel.com, hpa@zytor.com Date: Thu, 09 Feb 2017 19:51:29 -0500 In-Reply-To: <20170210000224.qgwwyozscwjegpqm@pd.tnic> References: <20170209184347.2ef977b9@annuminas.surriel.com> <20170210000224.qgwwyozscwjegpqm@pd.tnic> Organization: Red Hat, Inc Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 10 Feb 2017 00:51:32 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2017-02-10 at 01:02 +0100, Borislav Petkov wrote: > On Thu, Feb 09, 2017 at 06:43:47PM -0500, Rik van Riel wrote: > > On Skylake CPUs I noticed that XRSTOR is unable to deal with xsave > > areas > > created by copyout_from_xsaves if the xstate has only SSE/YMM > > state, but > > 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 area. > > > > 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/kernel/fpu/xstate.c | 44 > > ++++++++++++++++++++++++++++++++++++++++++++ > >  1 file changed, 44 insertions(+) > > ... > > > @@ -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 = sizeof(u64); // copy mxcsr & mxcsr_flags > >     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > We don't do // comments, do we? > > And side-line comments are always impairing the readability of the > code > unless it is a struct's members or asm or so ... Good point. OTOH, I don't really want to add an extra line to each of these blocks of code, either... Ingo, how would you like me to do these comments? Or should I have a magic #define with comment somewhere, like this? /* Copy both mxcsr & mxcsr_flags */ #define MXCSR_AND_FLAGS_SIZE sizeof(u64)