From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752538AbbHEKcf (ORCPT ); Wed, 5 Aug 2015 06:32:35 -0400 Received: from mail-wi0-f173.google.com ([209.85.212.173]:36811 "EHLO mail-wi0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751226AbbHEKcc (ORCPT ); Wed, 5 Aug 2015 06:32:32 -0400 Date: Wed, 5 Aug 2015 12:32:27 +0200 From: Ingo Molnar To: Dave Hansen Cc: dave.hansen@linux.intel.com, linux-kernel@vger.kernel.org, bp@alien8.de, fenghua.yu@intel.com, hpa@zytor.com, x86@kernel.org, Thomas Gleixner , Peter Zijlstra , Linus Torvalds , Andy Lutomirski , Denys Vlasenko Subject: Re: [PATCH] x86, fpu: correct XSAVE xstate size calculation Message-ID: <20150805103227.GA3233@gmail.com> References: <20150728172143.6DDFECA7@viggo.jf.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150728172143.6DDFECA7@viggo.jf.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Dave Hansen wrote: > > From: Dave Hansen > > Note: our xsaves support is currently broken and disabled. This > patch does not fix it, but it is an incremental improvement. It > might be useful to someone backporting the entire set of XSAVES > patches at some point, but it should not be backported alone. > > There are currently two xsave buffer formats: standard and > compacted. The standard format is waht 'XSAVE' and 'XSAVEOPT' > produce while 'XSAVES' and 'XSAVEC' produce a compacted-formet > buffer. (The kernel never uses XSAVEC) > > But, the XSAVES buffer *ALSO* contains "system state components" > which are never saved by a plain XSAVE. So, XSAVES has two > things that might make its buffer differently-sized from an > XSAVE-produced one. > > The current code assumes that an XSAVES buffer's size is simply > the sum of the sizes of the (user) states which are supported. > This seems to work in most cases, but it is not consistent with > what the SDM says, and it breaks if we 'align' a component in the > buffer. The calculation is also unnecessary work since the CPU > *tells* us the size of the buffer directly. > > This patch just reads the size of the buffer right out of the > CPUID leaf instead of trying to derive it. So how will we know where to find which field, if we cannot even do a size calculation? I realize that the calculation and what CPUID gives us should match, but it's not really good for the kernel to not know the precise layout of a critical task context data structure ... So can we turn this into 'double check the CPUID size and print a warning on mismatch' kind of boot time sanity check? Preferably for all XSAVE* data formats we can run into. I'd be fine with applying such a patch ahead of enabling compaction again. Thanks, Ingo