From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752379AbcEKRUv (ORCPT ); Wed, 11 May 2016 13:20:51 -0400 Received: from mx2.suse.de ([195.135.220.15]:49982 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751633AbcEKRUu (ORCPT ); Wed, 11 May 2016 13:20:50 -0400 Date: Wed, 11 May 2016 19:20:45 +0200 From: Borislav Petkov To: Yu-cheng Yu Cc: linux-kernel@vger.kernel.org, x86@kernel.org, "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , Dave Hansen , Andy Lutomirski , Sai Praneeth Prakhya , "Ravi V. Shankar" , Fenghua Yu Subject: Re: [PATCH v6 01/13] x86/xsaves: Define and use fpu_user_xstate_size Message-ID: <20160511172045.GH2180@pd.tnic> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: 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 On Tue, May 10, 2016 at 04:29:53PM -0700, Yu-cheng Yu wrote: > The XSAVE area of kernel can be in standard or compacted format; "The kernel xstate area... " and can we call it the xstate area as there are a bunch of XSAVE* insns touching it. The file which deals with it is even called that: arch/x86/kernel/fpu/xstate.c > it is always in standard format for user mode. When XSAVES is > enabled, the kernel uses the compacted format and it is necessary > to use a separate fpu_user_xstate_size for signal/ptrace frames. > > Based on an earlier patch from Fenghua Yu > > Signed-off-by: Fenghua Yu > [yu-cheng.yu@intel.com: rebase to current, rename to fpu_user_xstate_size] > Signed-off-by: Yu-cheng Yu > Reviewed-by: Dave Hansen Maybe I wasn't as clear as I hoped to be. Let me be more specific: So you either need to do: --- From: Fenghua ... Signed-off-by: Fenghua Signed-off-by: You ... --- or --- Based on an earlier patch from Fenghua Yu . Signed-off-by: You --- with the second variant making you the author implicitly because you're the sender. Makes more sense this way? > --- > arch/x86/include/asm/fpu/xstate.h | 1 - > arch/x86/include/asm/processor.h | 1 + > arch/x86/kernel/fpu/init.c | 5 ++- > arch/x86/kernel/fpu/signal.c | 27 ++++++++++---- > arch/x86/kernel/fpu/xstate.c | 76 ++++++++++++++++++++++++--------------- > 5 files changed, 73 insertions(+), 37 deletions(-) ... > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c > index b48ef35..dfac87d 100644 > --- a/arch/x86/kernel/fpu/xstate.c > +++ b/arch/x86/kernel/fpu/xstate.c > @@ -44,6 +44,13 @@ static unsigned int xstate_sizes[XFEATURE_MAX] = { [ 0 ... XFEATURE_MAX - 1] = > static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask)*8]; > > /* > + * The XSAVE area of kernel can be in standard or compacted format; "The kernel xstate area... " > + * it is always in standard format for user mode. This is the user > + * mode standard format size used for signal and ptrace frames. > + */ But yes, that's very nice commenting. > +unsigned int fpu_user_xstate_size; > + > +/* > * Clear all of the X86_FEATURE_* bits that are unavailable > * when the CPU has no XSAVE support. > */ ... > @@ -591,7 +598,15 @@ static bool is_supported_xstate_size(unsigned int test_xstate_size) > static int init_xstate_size(void) > { > /* Recompute the context size for enabled features: */ > - unsigned int possible_xstate_size = calculate_xstate_size(); > + unsigned int possible_xstate_size; > + unsigned int xsave_size; > + > + xsave_size = get_xsave_size(); > + > + if (cpu_has_xsaves) So next time you submit patches against -tip, please merge them with tip/master and see if they still build, at least. During last review I told you to use boot_cpu_has(X86_FEATURE_XSAVES) because those cpu_has_XXX things are going away and are gone in tip already. Please be more careful when incorporating review comments - otherwise it is waste of both yours and reviewer's time. Thanks. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --