From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752486AbdAXIKO (ORCPT ); Tue, 24 Jan 2017 03:10:14 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:33604 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751945AbdAXIKL (ORCPT ); Tue, 24 Jan 2017 03:10:11 -0500 Date: Tue, 24 Jan 2017 09:09:57 +0100 From: Ingo Molnar To: Yu-cheng Yu Cc: x86@kernel.org, "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , linux-kernel@vger.kernel.org, Andy Lutomirski , Borislav Petkov , Dave Hansen , Fenghua Yu , Joakim Tjernlund , "Ravi V. Shankar" , haokexin@gmail.com Subject: Re: [PATCH] x86/fpu/xstate: Fix xcomp_bv in XSAVES header Message-ID: <20170124080957.GB11694@gmail.com> References: <1485212084-4418-1-git-send-email-yu-cheng.yu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1485212084-4418-1-git-send-email-yu-cheng.yu@intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Yu-cheng Yu wrote: > The compacted-format XSAVES area is determined at boot time and > never changed after. The field xsave.header.xcomp_bv indicates > which components are in the fixed XSAVES format. > > In fpstate_init() we did not set xcomp_bv to reflect the XSAVES > format since at the time there is no valid data. > > However, after we do copy_init_fpstate_to_fpregs() in fpu__clear(), > as in commit: b22cbe404a9cc3c7949e380fa1861e31934c8978, and when > __fpu_restore_sig() does fpu__restore() for a COMPAT-mode app, > a #GP occurs. This can be easily triggered by doing valgrind on > a COMPAT-mode "Hello World," as reported by Joakim Tjernlund and > others: > > https://bugzilla.kernel.org/show_bug.cgi?id=190061 > > Fix it by setting xcomp_bv correctly. > > Signed-off-by: Yu-cheng Yu > Reported-by: Joakim Tjernlund > --- > arch/x86/kernel/fpu/core.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c > index c289e2f..e540dc1 100644 > --- a/arch/x86/kernel/fpu/core.c > +++ b/arch/x86/kernel/fpu/core.c > @@ -9,6 +9,7 @@ > #include > #include > #include > +#include > #include > > #include > @@ -235,7 +236,8 @@ void fpstate_init(union fpregs_state *state) > * it will #GP. Make sure it is replaced after the memset(). > */ > if (static_cpu_has(X86_FEATURE_XSAVES)) > - state->xsave.header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT; > + state->xsave.header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT | > + xfeatures_mask; Ok, I have applied this - but it would be cleaner to go one step further and add a fpstate_init_xstate() method that does this in xstate.c and hides the details from arch/x86/kernel/fpu/core.c. Similar to how the FX-state initialization is done today: > if (static_cpu_has(X86_FEATURE_FXSR)) > fpstate_init_fxstate(&state->fxsave); Thanks, Ingo