From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751936AbdAXAx2 (ORCPT ); Mon, 23 Jan 2017 19:53:28 -0500 Received: from mga07.intel.com ([134.134.136.100]:23826 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750873AbdAXAx1 (ORCPT ); Mon, 23 Jan 2017 19:53:27 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,276,1477983600"; d="scan'208";a="51746171" Subject: Re: [tip:x86/urgent] x86/fpu: Set the xcomp_bv when we fake up a XSAVES area To: Kevin Hao , Yu-cheng Yu References: <1485075023-30161-1-git-send-email-haokexin@gmail.com> <20170123165529.GA4996@test-lenovo> <2be814b7-9fd6-7955-b4e3-6ecb4ef76052@linux.intel.com> <20170123205725.GA3920@test-lenovo> <90b0fbee-273b-31c2-6fe9-228f00c0a205@linux.intel.com> <20170123211640.GA3976@test-lenovo> <20170124001447.GD15017@pek-khao-d1> Cc: fenghua.yu@intel.com, dvlasenk@redhat.com, peterz@infradead.org, oleg@redhat.com, mingo@kernel.org, linux-kernel@vger.kernel.org, brgerst@gmail.com, luto@kernel.org, bp@alien8.de, jpoimboe@redhat.com, hpa@zytor.com, quentin.casasnovas@oracle.com, tglx@linutronix.de, torvalds@linux-foundation.org, riel@redhat.com, linux-tip-commits@vger.kernel.org From: Dave Hansen Message-ID: <0d717394-eaf0-4d29-1aae-218ffcc8f06b@linux.intel.com> Date: Mon, 23 Jan 2017 16:53:25 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <20170124001447.GD15017@pek-khao-d1> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >> The fix I am proposing is... >> >> state->xsave.header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT | >> xfeatures_mask; > > Actually I thought about this change before I made this patch, but I don't this > is the right fix. It is always error prone to init the xcomp_bv to all the > supported feature. In case like copyin_to_xsaves(), it is possible that the > features which should be set in xcomp_bv do not equal to all the supported > features. Please see the following codes in copyin_to_xsaves(): > /* > * The state that came in from userspace was user-state only. > * Mask all the user states out of 'xfeatures': > */ > xsave->header.xfeatures &= XFEATURE_MASK_SUPERVISOR; > > /* > * Add back in the features that came in from userspace: > */ > xsave->header.xfeatures |= xfeatures; Hi Kevin, I think you may be confusing 'xfeatures' with 'xcomp_bv'. xfeatures tells you what features are present in the buffer while xcomp_bv tells you what *format* the buffer is in. Userspace never dictates the *format* of the kernel buffer, only the contents of each state. So, it makes sense that the copyin code would not (and should not) modify xcomp_bv. We ensure that xcomp_bv has all supported states set all the time, or we're *supposed* to.