From: Dave Hansen <dave@sr71.net>
To: dave@sr71.net
Cc: dave.hansen@linux.intel.com, mingo@kernel.org,
linux-kernel@vger.kernel.org, bp@alien8.de, fenghua.yu@intel.com,
hpa@zytor.com, x86@kernel.org
Subject: [PATCH] x86, fpu: correct XSAVE xstate size calculation
Date: Tue, 28 Jul 2015 10:21:43 -0700 [thread overview]
Message-ID: <20150728172143.6DDFECA7@viggo.jf.intel.com> (raw)
From: Dave Hansen <dave.hansen@linux.intel.com>
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.
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
---
b/arch/x86/kernel/fpu/xstate.c | 36 ++++++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 10 deletions(-)
diff -puN arch/x86/kernel/fpu/xstate.c~fix-xstate_size-calculation arch/x86/kernel/fpu/xstate.c
--- a/arch/x86/kernel/fpu/xstate.c~fix-xstate_size-calculation 2015-07-24 09:50:36.418385438 -0700
+++ b/arch/x86/kernel/fpu/xstate.c 2015-07-27 11:02:13.305376883 -0700
@@ -292,24 +292,40 @@ static void __init setup_init_fpu_buf(vo
/*
* Calculate total size of enabled xstates in XCR0/xfeatures_mask.
+ *
+ * Note the SDM's wording here. "sub-function 0" only enumerates
+ * the size of the *user* states. If we use it to size a buffer
+ * that we use 'XSAVES' on, we could potentially overflow the
+ * buffer because 'XSAVES' saves system states too.
+ *
+ * Note that we do not currently set any bits on IA32_XSS so
+ * 'XCR0 | IA32_XSS == XCR0' for now.
*/
static void __init init_xstate_size(void)
{
unsigned int eax, ebx, ecx, edx;
- int i;
if (!cpu_has_xsaves) {
+ /*
+ * - CPUID function 0DH, sub-function 0:
+ * EBX enumerates the size (in bytes) required by
+ * the XSAVE instruction for an XSAVE area
+ * containing all the *user* state components
+ * corresponding to bits currently set in XCR0.
+ */
cpuid_count(XSTATE_CPUID, 0, &eax, &ebx, &ecx, &edx);
xstate_size = ebx;
- return;
- }
-
- xstate_size = FXSAVE_SIZE + XSAVE_HDR_SIZE;
- for (i = 2; i < 64; i++) {
- if (test_bit(i, (unsigned long *)&xfeatures_mask)) {
- cpuid_count(XSTATE_CPUID, i, &eax, &ebx, &ecx, &edx);
- xstate_size += eax;
- }
+ } else {
+ /*
+ * - CPUID function 0DH, sub-function 1:
+ * EBX enumerates the size (in bytes) required by
+ * the XSAVES instruction for an XSAVE area
+ * containing all the state components
+ * corresponding to bits currently set in
+ * XCR0 | IA32_XSS.
+ */
+ cpuid_count(XSTATE_CPUID, 1, &eax, &ebx, &ecx, &edx);
+ xstate_size = ebx;
}
}
_
next reply other threads:[~2015-07-28 17:21 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-28 17:21 Dave Hansen [this message]
2015-08-05 10:32 ` [PATCH] x86, fpu: correct XSAVE xstate size calculation Ingo Molnar
2015-08-05 14:34 ` Dave Hansen
2015-08-06 7:15 ` Ingo Molnar
[not found] ` <CA+55aFxzOj-Ee=DN-_3CMeDeYVsmvmmgoxd3hp4MpRSp+og7AQ@mail.gmail.com>
2015-08-06 8:27 ` Ingo Molnar
2015-08-06 8:29 ` Ingo Molnar
2015-08-06 14:56 ` Dave Hansen
2015-08-06 16:03 ` Dave Hansen
2015-08-08 9:15 ` Ingo Molnar
2015-08-06 17:19 ` Dave Hansen
2015-08-08 9:06 ` Ingo Molnar
2015-08-10 21:14 ` Dave Hansen
2015-08-22 13:21 ` Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150728172143.6DDFECA7@viggo.jf.intel.com \
--to=dave@sr71.net \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=fenghua.yu@intel.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.