Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1737999 > unrolled thread
| Started by | Ingo Molnar <mingo@kernel.org> |
|---|---|
| First post | 2017-09-23 15:10 +0200 |
| Last post | 2017-09-24 01:30 +0200 |
| Articles | 20 on this page of 68 — 8 participants |
Back to article view | Back to linux.kernel
[PATCH 00/33] x86 FPU fixes and cleanups for v4.14 Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[PATCH 19/33] x86/fpu: Decouple fpregs_activate()/fpregs_deactivate() from fpu->fpregs_active Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Decouple fpregs_activate()/fpregs_deactivate() from fpu->fpregs_active tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 15/33] x86/fpu: Simplify fpu->fpregs_active use Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Simplify fpu->fpregs_active use tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 26/33] x86/fpu: Reinitialize FPU registers if restoring FPU state fails Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Reinitialize FPU registers if restoring FPU state fails tip-bot for Eric Biggers <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 32/33] x86/fpu: Rename fpu__activate_curr() to fpu__initialize() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Rename fpu__activate_curr() to fpu__initialize() tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:50 +0200
[PATCH 31/33] x86/fpu: Simplify and speed up fpu__copy() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Simplify and speed up fpu__copy() tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:50 +0200
[PATCH 21/33] x86/fpu: Add FPU state copying quirk to handle XRSTOR failure on Intel Skylake CPUs Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Add FPU state copying quirk to handle XRSTOR failure on Intel Skylake CPUs tip-bot for Rik van Riel <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 13/33] x86/fpu: Remove 'kbuf' parameter from the copy_user_to_xstate() API Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Remove 'kbuf' parameter from the copy_user_to_xstate() API tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 33/33] x86/fpu: Rename fpu__activate_fpstate_read/write() to fpu__read/write() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[PATCH 02/33] x86/fpu: Split copy_xstate_to_user() into copy_xstate_to_kernel() & copy_xstate_to_user() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Split copy_xstate_to_user() into copy_xstate_to_kernel() & copy_xstate_to_user() tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:30 +0200
[PATCH 16/33] x86/fpu: Make the fpu state change in fpu__clear() scheduler-atomic Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Make the fpu state change in fpu__clear() scheduler-atomic tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 14/33] x86/fpu: Flip the parameter order in copy_*_to_xstate() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Flip the parameter order in copy_*_to_xstate() tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 11/33] x86/fpu: Split copy_user_to_xstate() into copy_kernel_to_xstate() & copy_user_to_xstate() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Split copy_user_to_xstate() into copy_kernel_to_xstate() & copy_user_to_xstate() tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 07/33] x86/fpu: Remove the 'start_pos' parameter from the __copy_xstate_to_*() functions Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Remove the 'start_pos' parameter from the __copy_xstate_to_*() functions tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 01/33] x86/fpu: Rename copyin_to_xsaves()/copyout_from_xsaves() to copy_user_to_xstate()/copy_xstate_to_user() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Rename copyin_to_xsaves()/copyout_from_xsaves() to copy_user_to_xstate()/copy_xstate_to_user() tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:30 +0200
[PATCH 29/33] x86/fpu: Rename fpu::fpstate_active to fpu::initialized Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Rename fpu::fpstate_active to fpu::initialized tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:50 +0200
[PATCH 17/33] x86/fpu: Split the state handling in fpu__drop() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Split the state handling in fpu__drop() tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 09/33] x86/fpu: Change 'size_total' parameter to unsigned and standardize the size checks in copy_xstate_to_*() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Change 'size_total' parameter to unsigned and standardize the size checks in copy_xstate_to_*() tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 30/33] x86/fpu: Fix stale comments about lazy FPU logic Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Fix stale comments about lazy FPU logic tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:50 +0200
[PATCH 08/33] x86/fpu: Clarify parameter names in the copy_xstate_to_*() methods Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
Re: [PATCH 08/33] x86/fpu: Clarify parameter names in the copy_xstate_to_*() methods Thomas Gleixner <tglx@linutronix.de> - 2017-09-25 22:00 +0200
Re: [PATCH 08/33] x86/fpu: Clarify parameter names in the copy_xstate_to_*() methods Thomas Gleixner <tglx@linutronix.de> - 2017-09-25 22:10 +0200
[tip:x86/fpu] x86/fpu: Clarify parameter names in the copy_xstate_to_*() methods tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 20/33] x86/fpu: Remove struct fpu::fpregs_active Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Remove struct fpu::fpregs_active tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 03/33] x86/fpu: Remove 'ubuf' parameter from the copy_xstate_to_kernel() APIs Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Remove 'ubuf' parameter from the copy_xstate_to_kernel() APIs tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:30 +0200
[PATCH 23/33] x86/fpu: Turn WARN_ON() in context switch into WARN_ON_FPU() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Turn WARN_ON() in context switch into WARN_ON_FPU() tip-bot for Andi Kleen <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 22/33] x86/fpu: Fix boolreturn.cocci warnings Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Fix boolreturn.cocci warnings tip-bot for kbuild test robot <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 06/33] x86/fpu: Clean up the parameter definitions of copy_xstate_to_*() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Clean up the parameter definitions of copy_xstate_to_*() tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 04/33] x86/fpu: Remove 'kbuf' parameter from the copy_xstate_to_user() APIs Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Remove 'kbuf' parameter from the copy_xstate_to_user() APIs tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 05/33] x86/fpu: Clean up parameter order in the copy_xstate_to_*() APIs Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Clean up parameter order in the copy_xstate_to_*() APIs tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 24/33] x86/fpu: Don't let userspace set bogus xcomp_bv Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Don't let userspace set bogus xcomp_bv tip-bot for Eric Biggers <tipbot@zytor.com> - 2017-09-26 10:50 +0200
[PATCH 18/33] x86/fpu: Change fpu->fpregs_active users to fpu->fpstate_active Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Change fpu->fpregs_active users to fpu->fpstate_active tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 27/33] x86/fpu: Simplify fpu__activate_fpstate_read() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Fix fpu__activate_fpstate_read() and update comments tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 12/33] x86/fpu: Remove 'ubuf' parameter from the copy_kernel_to_xstate() API Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Remove 'ubuf' parameter from the copy_kernel_to_xstate() API tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
[PATCH 25/33] x86/fpu: Tighten validation of user-supplied xstate_header Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[PATCH 28/33] x86/fpu: Remove fpu__current_fpstate_write_begin/end() Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
[tip:x86/fpu] x86/fpu: Remove fpu__current_fpstate_write_begin/end() tip-bot for Ingo Molnar <tipbot@zytor.com> - 2017-09-26 10:40 +0200
Re: [PATCH 00/33] x86 FPU fixes and cleanups for v4.14 Ingo Molnar <mingo@kernel.org> - 2017-09-23 15:10 +0200
Re: [PATCH 00/33] x86 FPU fixes and cleanups for v4.14 Juergen Gross <jgross@suse.com> - 2017-09-23 17:10 +0200
Re: [PATCH 00/33] x86 FPU fixes and cleanups for v4.14 Ingo Molnar <mingo@kernel.org> - 2017-09-24 01:30 +0200
Page 3 of 4 — ← Prev page 1 2 [3] 4 Next page →
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-09-23 15:10 +0200 |
| Subject | [PATCH 20/33] x86/fpu: Remove struct fpu::fpregs_active |
| Message-ID | <usSlZ-3wU-45@gated-at.bofh.it> |
| In reply to | #1737999 |
The previous changes paved the way for the removal of the
fpu::fpregs_active state flag - we now only have the
fpu::fpstate_active and fpu::last_cpu fields left.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/fpu/internal.h | 5 -----
arch/x86/include/asm/fpu/types.h | 23 -----------------------
arch/x86/include/asm/trace/fpu.h | 5 +----
arch/x86/kernel/fpu/core.c | 9 ---------
arch/x86/kernel/fpu/signal.c | 2 --
5 files changed, 1 insertion(+), 43 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index 42a601673c09..629e7abcd6c9 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -526,14 +526,12 @@ static inline int fpregs_state_valid(struct fpu *fpu, unsigned int cpu)
*/
static inline void fpregs_deactivate(struct fpu *fpu)
{
- fpu->fpregs_active = 0;
this_cpu_write(fpu_fpregs_owner_ctx, NULL);
trace_x86_fpu_regs_deactivated(fpu);
}
static inline void fpregs_activate(struct fpu *fpu)
{
- fpu->fpregs_active = 1;
this_cpu_write(fpu_fpregs_owner_ctx, fpu);
trace_x86_fpu_regs_activated(fpu);
}
@@ -552,8 +550,6 @@ static inline void fpregs_activate(struct fpu *fpu)
static inline void
switch_fpu_prepare(struct fpu *old_fpu, int cpu)
{
- WARN_ON_FPU(old_fpu->fpregs_active != old_fpu->fpstate_active);
-
if (old_fpu->fpstate_active) {
if (!copy_fpregs_to_fpstate(old_fpu))
old_fpu->last_cpu = -1;
@@ -561,7 +557,6 @@ switch_fpu_prepare(struct fpu *old_fpu, int cpu)
old_fpu->last_cpu = cpu;
/* But leave fpu_fpregs_owner_ctx! */
- old_fpu->fpregs_active = 0;
trace_x86_fpu_regs_deactivated(old_fpu);
} else
old_fpu->last_cpu = -1;
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index 3c80f5b9c09d..0c314a397cf5 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -299,29 +299,6 @@ struct fpu {
unsigned char fpstate_active;
/*
- * @fpregs_active:
- *
- * This flag determines whether a given context is actively
- * loaded into the FPU's registers and that those registers
- * represent the task's current FPU state.
- *
- * Note the interaction with fpstate_active:
- *
- * # task does not use the FPU:
- * fpstate_active == 0
- *
- * # task uses the FPU and regs are active:
- * fpstate_active == 1 && fpregs_active == 1
- *
- * # the regs are inactive but still match fpstate:
- * fpstate_active == 1 && fpregs_active == 0 && fpregs_owner == fpu
- *
- * The third state is what we use for the lazy restore optimization
- * on lazy-switching CPUs.
- */
- unsigned char fpregs_active;
-
- /*
* @state:
*
* In-memory copy of all FPU registers that we save/restore
diff --git a/arch/x86/include/asm/trace/fpu.h b/arch/x86/include/asm/trace/fpu.h
index 342e59789fcd..da565aae9fd2 100644
--- a/arch/x86/include/asm/trace/fpu.h
+++ b/arch/x86/include/asm/trace/fpu.h
@@ -12,7 +12,6 @@ DECLARE_EVENT_CLASS(x86_fpu,
TP_STRUCT__entry(
__field(struct fpu *, fpu)
- __field(bool, fpregs_active)
__field(bool, fpstate_active)
__field(u64, xfeatures)
__field(u64, xcomp_bv)
@@ -20,16 +19,14 @@ DECLARE_EVENT_CLASS(x86_fpu,
TP_fast_assign(
__entry->fpu = fpu;
- __entry->fpregs_active = fpu->fpregs_active;
__entry->fpstate_active = fpu->fpstate_active;
if (boot_cpu_has(X86_FEATURE_OSXSAVE)) {
__entry->xfeatures = fpu->state.xsave.header.xfeatures;
__entry->xcomp_bv = fpu->state.xsave.header.xcomp_bv;
}
),
- TP_printk("x86/fpu: %p fpregs_active: %d fpstate_active: %d xfeatures: %llx xcomp_bv: %llx",
+ TP_printk("x86/fpu: %p fpstate_active: %d xfeatures: %llx xcomp_bv: %llx",
__entry->fpu,
- __entry->fpregs_active,
__entry->fpstate_active,
__entry->xfeatures,
__entry->xcomp_bv
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 01a47e9edfb4..93103a909c47 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -147,8 +147,6 @@ void fpu__save(struct fpu *fpu)
WARN_ON_FPU(fpu != ¤t->thread.fpu);
preempt_disable();
- WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
-
trace_x86_fpu_before_save(fpu);
if (fpu->fpstate_active) {
if (!copy_fpregs_to_fpstate(fpu)) {
@@ -191,7 +189,6 @@ EXPORT_SYMBOL_GPL(fpstate_init);
int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)
{
- dst_fpu->fpregs_active = 0;
dst_fpu->last_cpu = -1;
if (!src_fpu->fpstate_active || !static_cpu_has(X86_FEATURE_FPU))
@@ -264,7 +261,6 @@ EXPORT_SYMBOL_GPL(fpu__activate_curr);
*/
void fpu__activate_fpstate_read(struct fpu *fpu)
{
- WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
/*
* If fpregs are active (in the current CPU), then
* copy them to the fpstate:
@@ -365,7 +361,6 @@ void fpu__current_fpstate_write_end(void)
{
struct fpu *fpu = ¤t->thread.fpu;
- WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
/*
* 'fpu' now has an updated copy of the state, but the
* registers may still be out of date. Update them with
@@ -419,8 +414,6 @@ void fpu__drop(struct fpu *fpu)
preempt_disable();
if (fpu == ¤t->thread.fpu) {
- WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
-
if (fpu->fpstate_active) {
/* Ignore delayed exceptions from user space */
asm volatile("1: fwait\n"
@@ -428,8 +421,6 @@ void fpu__drop(struct fpu *fpu)
_ASM_EXTABLE(1b, 2b));
fpregs_deactivate(fpu);
}
- } else {
- WARN_ON_FPU(fpu->fpregs_active);
}
fpu->fpstate_active = 0;
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index a88083ba7f8b..629106e51a29 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -171,8 +171,6 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
sizeof(struct user_i387_ia32_struct), NULL,
(struct _fpstate_32 __user *) buf) ? -1 : 1;
- WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
-
if (fpu->fpstate_active || using_compacted_format()) {
/* Save the live register state to the user directly. */
if (copy_fpregs_to_sigframe(buf_fx))
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Ingo Molnar <tipbot@zytor.com> |
|---|---|
| Date | 2017-09-26 10:40 +0200 |
| Subject | [tip:x86/fpu] x86/fpu: Remove struct fpu::fpregs_active |
| Message-ID | <utTzm-2jm-71@gated-at.bofh.it> |
| In reply to | #1738020 |
Commit-ID: 99dc26bda233ee722bbd370bddf20beece3ffb93
Gitweb: http://git.kernel.org/tip/99dc26bda233ee722bbd370bddf20beece3ffb93
Author: Ingo Molnar <mingo@kernel.org>
AuthorDate: Sat, 23 Sep 2017 15:00:03 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 24 Sep 2017 13:04:34 +0200
x86/fpu: Remove struct fpu::fpregs_active
The previous changes paved the way for the removal of the
fpu::fpregs_active state flag - we now only have the
fpu::fpstate_active and fpu::last_cpu fields left.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Link: http://lkml.kernel.org/r/20170923130016.21448-21-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/fpu/internal.h | 5 -----
arch/x86/include/asm/fpu/types.h | 23 -----------------------
arch/x86/include/asm/trace/fpu.h | 5 +----
arch/x86/kernel/fpu/core.c | 9 ---------
arch/x86/kernel/fpu/signal.c | 2 --
5 files changed, 1 insertion(+), 43 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index 42a6016..629e7abc 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -526,14 +526,12 @@ static inline int fpregs_state_valid(struct fpu *fpu, unsigned int cpu)
*/
static inline void fpregs_deactivate(struct fpu *fpu)
{
- fpu->fpregs_active = 0;
this_cpu_write(fpu_fpregs_owner_ctx, NULL);
trace_x86_fpu_regs_deactivated(fpu);
}
static inline void fpregs_activate(struct fpu *fpu)
{
- fpu->fpregs_active = 1;
this_cpu_write(fpu_fpregs_owner_ctx, fpu);
trace_x86_fpu_regs_activated(fpu);
}
@@ -552,8 +550,6 @@ static inline void fpregs_activate(struct fpu *fpu)
static inline void
switch_fpu_prepare(struct fpu *old_fpu, int cpu)
{
- WARN_ON_FPU(old_fpu->fpregs_active != old_fpu->fpstate_active);
-
if (old_fpu->fpstate_active) {
if (!copy_fpregs_to_fpstate(old_fpu))
old_fpu->last_cpu = -1;
@@ -561,7 +557,6 @@ switch_fpu_prepare(struct fpu *old_fpu, int cpu)
old_fpu->last_cpu = cpu;
/* But leave fpu_fpregs_owner_ctx! */
- old_fpu->fpregs_active = 0;
trace_x86_fpu_regs_deactivated(old_fpu);
} else
old_fpu->last_cpu = -1;
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index 3c80f5b..0c314a3 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -299,29 +299,6 @@ struct fpu {
unsigned char fpstate_active;
/*
- * @fpregs_active:
- *
- * This flag determines whether a given context is actively
- * loaded into the FPU's registers and that those registers
- * represent the task's current FPU state.
- *
- * Note the interaction with fpstate_active:
- *
- * # task does not use the FPU:
- * fpstate_active == 0
- *
- * # task uses the FPU and regs are active:
- * fpstate_active == 1 && fpregs_active == 1
- *
- * # the regs are inactive but still match fpstate:
- * fpstate_active == 1 && fpregs_active == 0 && fpregs_owner == fpu
- *
- * The third state is what we use for the lazy restore optimization
- * on lazy-switching CPUs.
- */
- unsigned char fpregs_active;
-
- /*
* @state:
*
* In-memory copy of all FPU registers that we save/restore
diff --git a/arch/x86/include/asm/trace/fpu.h b/arch/x86/include/asm/trace/fpu.h
index 342e597..da565aa 100644
--- a/arch/x86/include/asm/trace/fpu.h
+++ b/arch/x86/include/asm/trace/fpu.h
@@ -12,7 +12,6 @@ DECLARE_EVENT_CLASS(x86_fpu,
TP_STRUCT__entry(
__field(struct fpu *, fpu)
- __field(bool, fpregs_active)
__field(bool, fpstate_active)
__field(u64, xfeatures)
__field(u64, xcomp_bv)
@@ -20,16 +19,14 @@ DECLARE_EVENT_CLASS(x86_fpu,
TP_fast_assign(
__entry->fpu = fpu;
- __entry->fpregs_active = fpu->fpregs_active;
__entry->fpstate_active = fpu->fpstate_active;
if (boot_cpu_has(X86_FEATURE_OSXSAVE)) {
__entry->xfeatures = fpu->state.xsave.header.xfeatures;
__entry->xcomp_bv = fpu->state.xsave.header.xcomp_bv;
}
),
- TP_printk("x86/fpu: %p fpregs_active: %d fpstate_active: %d xfeatures: %llx xcomp_bv: %llx",
+ TP_printk("x86/fpu: %p fpstate_active: %d xfeatures: %llx xcomp_bv: %llx",
__entry->fpu,
- __entry->fpregs_active,
__entry->fpstate_active,
__entry->xfeatures,
__entry->xcomp_bv
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 01a47e9..93103a9 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -147,8 +147,6 @@ void fpu__save(struct fpu *fpu)
WARN_ON_FPU(fpu != ¤t->thread.fpu);
preempt_disable();
- WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
-
trace_x86_fpu_before_save(fpu);
if (fpu->fpstate_active) {
if (!copy_fpregs_to_fpstate(fpu)) {
@@ -191,7 +189,6 @@ EXPORT_SYMBOL_GPL(fpstate_init);
int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)
{
- dst_fpu->fpregs_active = 0;
dst_fpu->last_cpu = -1;
if (!src_fpu->fpstate_active || !static_cpu_has(X86_FEATURE_FPU))
@@ -264,7 +261,6 @@ EXPORT_SYMBOL_GPL(fpu__activate_curr);
*/
void fpu__activate_fpstate_read(struct fpu *fpu)
{
- WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
/*
* If fpregs are active (in the current CPU), then
* copy them to the fpstate:
@@ -365,7 +361,6 @@ void fpu__current_fpstate_write_end(void)
{
struct fpu *fpu = ¤t->thread.fpu;
- WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
/*
* 'fpu' now has an updated copy of the state, but the
* registers may still be out of date. Update them with
@@ -419,8 +414,6 @@ void fpu__drop(struct fpu *fpu)
preempt_disable();
if (fpu == ¤t->thread.fpu) {
- WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
-
if (fpu->fpstate_active) {
/* Ignore delayed exceptions from user space */
asm volatile("1: fwait\n"
@@ -428,8 +421,6 @@ void fpu__drop(struct fpu *fpu)
_ASM_EXTABLE(1b, 2b));
fpregs_deactivate(fpu);
}
- } else {
- WARN_ON_FPU(fpu->fpregs_active);
}
fpu->fpstate_active = 0;
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index a88083b..629106e 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -171,8 +171,6 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
sizeof(struct user_i387_ia32_struct), NULL,
(struct _fpstate_32 __user *) buf) ? -1 : 1;
- WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
-
if (fpu->fpstate_active || using_compacted_format()) {
/* Save the live register state to the user directly. */
if (copy_fpregs_to_sigframe(buf_fx))
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-09-23 15:10 +0200 |
| Subject | [PATCH 03/33] x86/fpu: Remove 'ubuf' parameter from the copy_xstate_to_kernel() APIs |
| Message-ID | <usSlZ-3wU-47@gated-at.bofh.it> |
| In reply to | #1737999 |
The 'ubuf' parameter is unused in the _kernel() side of the API, remove it.
This simplifies the code and makes it easier to think about.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/fpu/xstate.h | 2 +-
arch/x86/kernel/fpu/regset.c | 2 +-
arch/x86/kernel/fpu/xstate.c | 21 ++++++---------------
3 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 92dc8ca14124..c762574a245f 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -48,7 +48,7 @@ void fpu__xstate_clear_all_cpu_caps(void);
void *get_xsave_addr(struct xregs_state *xsave, int xstate);
const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf, struct xregs_state *xsave);
+int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave);
int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf, struct xregs_state *xsave);
int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave);
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index b6d12d66d04b..34e74adf9d5d 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -93,7 +93,7 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
if (using_compacted_format()) {
if (kbuf)
- ret = copy_xstate_to_kernel(pos, count, kbuf, ubuf, xsave);
+ ret = copy_xstate_to_kernel(pos, count, kbuf, xsave);
else
ret = copy_xstate_to_user(pos, count, kbuf, ubuf, xsave);
} else {
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 38561539cb99..71d3bda2b898 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -926,7 +926,7 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
*/
static inline int
__copy_xstate_to_kernel(unsigned int pos, unsigned int count,
- void *kbuf, void __user *ubuf,
+ void *kbuf,
const void *data, const int start_pos,
const int end_pos)
{
@@ -936,12 +936,7 @@ __copy_xstate_to_kernel(unsigned int pos, unsigned int count,
if (end_pos < 0 || pos < end_pos) {
unsigned int copy = (end_pos < 0 ? count : min(count, end_pos - pos));
- if (kbuf) {
- memcpy(kbuf + pos, data, copy);
- } else {
- if (__copy_to_user(ubuf + pos, data, copy))
- return -EFAULT;
- }
+ memcpy(kbuf + pos, data, copy);
}
return 0;
}
@@ -953,8 +948,7 @@ __copy_xstate_to_kernel(unsigned int pos, unsigned int count,
* It supports partial copy but pos always starts from zero. This is called
* from xstateregs_get() and there we check the CPU has XSAVES.
*/
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
- void __user *ubuf, struct xregs_state *xsave)
+int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave)
{
unsigned int offset, size;
int ret, i;
@@ -979,8 +973,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_kernel(offset, size, kbuf, ubuf, &header, 0, count);
-
+ ret = __copy_xstate_to_kernel(offset, size, kbuf, &header, 0, count);
if (ret)
return ret;
@@ -994,8 +987,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_kernel(offset, size, kbuf, ubuf, src, 0, count);
-
+ ret = __copy_xstate_to_kernel(offset, size, kbuf, src, 0, count);
if (ret)
return ret;
@@ -1011,8 +1003,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_kernel(offset, size, kbuf, ubuf, xstate_fx_sw_bytes, 0, count);
-
+ ret = __copy_xstate_to_kernel(offset, size, kbuf, xstate_fx_sw_bytes, 0, count);
if (ret)
return ret;
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Ingo Molnar <tipbot@zytor.com> |
|---|---|
| Date | 2017-09-26 10:30 +0200 |
| Subject | [tip:x86/fpu] x86/fpu: Remove 'ubuf' parameter from the copy_xstate_to_kernel() APIs |
| Message-ID | <utTpD-2dI-5@gated-at.bofh.it> |
| In reply to | #1738021 |
Commit-ID: 4d981cf2d96f29cdfa7d4972c8b377fe7baa9c4c
Gitweb: http://git.kernel.org/tip/4d981cf2d96f29cdfa7d4972c8b377fe7baa9c4c
Author: Ingo Molnar <mingo@kernel.org>
AuthorDate: Sat, 23 Sep 2017 14:59:46 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 24 Sep 2017 13:04:31 +0200
x86/fpu: Remove 'ubuf' parameter from the copy_xstate_to_kernel() APIs
The 'ubuf' parameter is unused in the _kernel() side of the API, remove it.
This simplifies the code and makes it easier to think about.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Link: http://lkml.kernel.org/r/20170923130016.21448-4-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/fpu/xstate.h | 2 +-
arch/x86/kernel/fpu/regset.c | 2 +-
arch/x86/kernel/fpu/xstate.c | 21 ++++++---------------
3 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 92dc8ca..c762574 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -48,7 +48,7 @@ void fpu__xstate_clear_all_cpu_caps(void);
void *get_xsave_addr(struct xregs_state *xsave, int xstate);
const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf, struct xregs_state *xsave);
+int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave);
int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf, struct xregs_state *xsave);
int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave);
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index b6d12d6..34e74ad 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -93,7 +93,7 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
if (using_compacted_format()) {
if (kbuf)
- ret = copy_xstate_to_kernel(pos, count, kbuf, ubuf, xsave);
+ ret = copy_xstate_to_kernel(pos, count, kbuf, xsave);
else
ret = copy_xstate_to_user(pos, count, kbuf, ubuf, xsave);
} else {
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 3856153..71d3bda 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -926,7 +926,7 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
*/
static inline int
__copy_xstate_to_kernel(unsigned int pos, unsigned int count,
- void *kbuf, void __user *ubuf,
+ void *kbuf,
const void *data, const int start_pos,
const int end_pos)
{
@@ -936,12 +936,7 @@ __copy_xstate_to_kernel(unsigned int pos, unsigned int count,
if (end_pos < 0 || pos < end_pos) {
unsigned int copy = (end_pos < 0 ? count : min(count, end_pos - pos));
- if (kbuf) {
- memcpy(kbuf + pos, data, copy);
- } else {
- if (__copy_to_user(ubuf + pos, data, copy))
- return -EFAULT;
- }
+ memcpy(kbuf + pos, data, copy);
}
return 0;
}
@@ -953,8 +948,7 @@ __copy_xstate_to_kernel(unsigned int pos, unsigned int count,
* It supports partial copy but pos always starts from zero. This is called
* from xstateregs_get() and there we check the CPU has XSAVES.
*/
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
- void __user *ubuf, struct xregs_state *xsave)
+int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave)
{
unsigned int offset, size;
int ret, i;
@@ -979,8 +973,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_kernel(offset, size, kbuf, ubuf, &header, 0, count);
-
+ ret = __copy_xstate_to_kernel(offset, size, kbuf, &header, 0, count);
if (ret)
return ret;
@@ -994,8 +987,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_kernel(offset, size, kbuf, ubuf, src, 0, count);
-
+ ret = __copy_xstate_to_kernel(offset, size, kbuf, src, 0, count);
if (ret)
return ret;
@@ -1011,8 +1003,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_kernel(offset, size, kbuf, ubuf, xstate_fx_sw_bytes, 0, count);
-
+ ret = __copy_xstate_to_kernel(offset, size, kbuf, xstate_fx_sw_bytes, 0, count);
if (ret)
return ret;
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-09-23 15:10 +0200 |
| Subject | [PATCH 23/33] x86/fpu: Turn WARN_ON() in context switch into WARN_ON_FPU() |
| Message-ID | <usSlZ-3wU-61@gated-at.bofh.it> |
| In reply to | #1737999 |
From: Andi Kleen <ak@linux.intel.com> copy_xregs_to_kernel checks if the alternatives have been already patched. This WARN_ON() is always executed in every context switch. All the other checks in fpu internal.h are WARN_ON_FPU(), but this one is plain WARN_ON(). I assume it was forgotten to switch it. So switch it to WARN_ON_FPU() too to avoid some unnecessary code in the context switch, and a potentially expensive cache line miss for the global variable. Signed-off-by: Andi Kleen <ak@linux.intel.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/20170329062605.4970-1-andi@firstfloor.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/asm/fpu/internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 629e7abcd6c9..2dca7c65319c 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -350,7 +350,7 @@ static inline void copy_xregs_to_kernel(struct xregs_state *xstate) u32 hmask = mask >> 32; int err; - WARN_ON(!alternatives_patched); + WARN_ON_FPU(!alternatives_patched); XSTATE_XSAVE(xstate, lmask, hmask, err); -- 2.11.0
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Andi Kleen <tipbot@zytor.com> |
|---|---|
| Date | 2017-09-26 10:40 +0200 |
| Subject | [tip:x86/fpu] x86/fpu: Turn WARN_ON() in context switch into WARN_ON_FPU() |
| Message-ID | <utTzn-2jm-87@gated-at.bofh.it> |
| In reply to | #1738022 |
Commit-ID: 03eaec81ac09814817e9f0307d572ffe8365f980 Gitweb: http://git.kernel.org/tip/03eaec81ac09814817e9f0307d572ffe8365f980 Author: Andi Kleen <ak@linux.intel.com> AuthorDate: Sat, 23 Sep 2017 15:00:06 +0200 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Sun, 24 Sep 2017 13:04:35 +0200 x86/fpu: Turn WARN_ON() in context switch into WARN_ON_FPU() copy_xregs_to_kernel checks if the alternatives have been already patched. This WARN_ON() is always executed in every context switch. All the other checks in fpu internal.h are WARN_ON_FPU(), but this one is plain WARN_ON(). I assume it was forgotten to switch it. So switch it to WARN_ON_FPU() too to avoid some unnecessary code in the context switch, and a potentially expensive cache line miss for the global variable. Signed-off-by: Andi Kleen <ak@linux.intel.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Andy Lutomirski <luto@amacapital.net> Cc: Borislav Petkov <bp@alien8.de> Cc: Dave Hansen <dave.hansen@linux.intel.com> Cc: Eric Biggers <ebiggers3@gmail.com> Cc: Fenghua Yu <fenghua.yu@intel.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Rik van Riel <riel@redhat.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Yu-cheng Yu <yu-cheng.yu@intel.com> Link: http://lkml.kernel.org/r/20170329062605.4970-1-andi@firstfloor.org Link: http://lkml.kernel.org/r/20170923130016.21448-24-mingo@kernel.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/asm/fpu/internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 629e7abc..2dca7c6 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -350,7 +350,7 @@ static inline void copy_xregs_to_kernel(struct xregs_state *xstate) u32 hmask = mask >> 32; int err; - WARN_ON(!alternatives_patched); + WARN_ON_FPU(!alternatives_patched); XSTATE_XSAVE(xstate, lmask, hmask, err);
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-09-23 15:10 +0200 |
| Subject | [PATCH 22/33] x86/fpu: Fix boolreturn.cocci warnings |
| Message-ID | <usSlZ-3wU-63@gated-at.bofh.it> |
| In reply to | #1737999 |
From: kbuild test robot <fengguang.wu@intel.com>
arch/x86/kernel/fpu/xstate.c:931:9-10: WARNING: return of 0/1 in function 'xfeatures_mxcsr_quirk' with return type bool
Return statements in functions returning bool should use true/false instead of 1/0.
Generated by: scripts/coccinelle/misc/boolreturn.cocci
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kbuild-all@01.org
Cc: tipbuild@zytor.com
Link: http://lkml.kernel.org/r/20170306004553.GA25764@lkp-wsm-ep1
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/fpu/xstate.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 41c52256bdce..fda1109cc355 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -929,12 +929,12 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
static inline bool xfeatures_mxcsr_quirk(u64 xfeatures)
{
if (!(xfeatures & (XFEATURE_MASK_SSE|XFEATURE_MASK_YMM)))
- return 0;
+ return false;
if (xfeatures & XFEATURE_MASK_FP)
- return 0;
+ return false;
- return 1;
+ return true;
}
/*
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for kbuild test robot <tipbot@zytor.com> |
|---|---|
| Date | 2017-09-26 10:40 +0200 |
| Subject | [tip:x86/fpu] x86/fpu: Fix boolreturn.cocci warnings |
| Message-ID | <utTzl-2jm-33@gated-at.bofh.it> |
| In reply to | #1738023 |
Commit-ID: 4f8cef59bad29344aca0e2e6b0ad18dadd078fd0
Gitweb: http://git.kernel.org/tip/4f8cef59bad29344aca0e2e6b0ad18dadd078fd0
Author: kbuild test robot <fengguang.wu@intel.com>
AuthorDate: Sat, 23 Sep 2017 15:00:05 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 24 Sep 2017 13:04:35 +0200
x86/fpu: Fix boolreturn.cocci warnings
arch/x86/kernel/fpu/xstate.c:931:9-10: WARNING: return of 0/1 in function 'xfeatures_mxcsr_quirk' with return type bool
Return statements in functions returning bool should use true/false instead of 1/0.
Generated by: scripts/coccinelle/misc/boolreturn.cocci
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kbuild-all@01.org
Cc: tipbuild@zytor.com
Link: http://lkml.kernel.org/r/20170306004553.GA25764@lkp-wsm-ep1
Link: http://lkml.kernel.org/r/20170923130016.21448-23-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/fpu/xstate.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 41c5225..fda1109 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -929,12 +929,12 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
static inline bool xfeatures_mxcsr_quirk(u64 xfeatures)
{
if (!(xfeatures & (XFEATURE_MASK_SSE|XFEATURE_MASK_YMM)))
- return 0;
+ return false;
if (xfeatures & XFEATURE_MASK_FP)
- return 0;
+ return false;
- return 1;
+ return true;
}
/*
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-09-23 15:10 +0200 |
| Subject | [PATCH 06/33] x86/fpu: Clean up the parameter definitions of copy_xstate_to_*() |
| Message-ID | <usSm0-3wU-67@gated-at.bofh.it> |
| In reply to | #1737999 |
Remove pointless 'const' of non-pointer input parameter.
Remove unnecessary parenthesis that shows uncertainty about arithmetic operator precedence.
Clarify copy_xstate_to_user() description.
No change in functionality.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/fpu/xstate.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 0a299468510f..9647e7256179 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -927,13 +927,13 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
static inline int
__copy_xstate_to_kernel(void *kbuf,
const void *data,
- unsigned int pos, unsigned int count, const int start_pos, const int end_pos)
+ unsigned int pos, unsigned int count, int start_pos, int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
if (end_pos < 0 || pos < end_pos) {
- unsigned int copy = (end_pos < 0 ? count : min(count, end_pos - pos));
+ unsigned int copy = end_pos < 0 ? count : min(count, end_pos - pos);
memcpy(kbuf + pos, data, copy);
}
@@ -1010,13 +1010,13 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int po
}
static inline int
-__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos, unsigned int count, const int start_pos, const int end_pos)
+__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos, unsigned int count, int start_pos, int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
if (end_pos < 0 || pos < end_pos) {
- unsigned int copy = (end_pos < 0 ? count : min(count, end_pos - pos));
+ unsigned int copy = end_pos < 0 ? count : min(count, end_pos - pos);
if (__copy_to_user(ubuf + pos, data, copy))
return -EFAULT;
@@ -1026,7 +1026,7 @@ __copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos, uns
/*
* Convert from kernel XSAVES compacted format to standard format and copy
- * to a ptrace buffer. It supports partial copy but pos always starts from
+ * to a user-space buffer. It supports partial copy but pos always starts from
* zero. This is called from xstateregs_get() and there we check the CPU
* has XSAVES.
*/
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Ingo Molnar <tipbot@zytor.com> |
|---|---|
| Date | 2017-09-26 10:40 +0200 |
| Subject | [tip:x86/fpu] x86/fpu: Clean up the parameter definitions of copy_xstate_to_*() |
| Message-ID | <utTzl-2jm-43@gated-at.bofh.it> |
| In reply to | #1738024 |
Commit-ID: becb2bb72ff906cc0d2bac3ee9574f694364823b
Gitweb: http://git.kernel.org/tip/becb2bb72ff906cc0d2bac3ee9574f694364823b
Author: Ingo Molnar <mingo@kernel.org>
AuthorDate: Sat, 23 Sep 2017 14:59:49 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 24 Sep 2017 13:04:31 +0200
x86/fpu: Clean up the parameter definitions of copy_xstate_to_*()
Remove pointless 'const' of non-pointer input parameter.
Remove unnecessary parenthesis that shows uncertainty about arithmetic operator precedence.
Clarify copy_xstate_to_user() description.
No change in functionality.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Link: http://lkml.kernel.org/r/20170923130016.21448-7-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/fpu/xstate.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 0a29946..9647e72 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -927,13 +927,13 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
static inline int
__copy_xstate_to_kernel(void *kbuf,
const void *data,
- unsigned int pos, unsigned int count, const int start_pos, const int end_pos)
+ unsigned int pos, unsigned int count, int start_pos, int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
if (end_pos < 0 || pos < end_pos) {
- unsigned int copy = (end_pos < 0 ? count : min(count, end_pos - pos));
+ unsigned int copy = end_pos < 0 ? count : min(count, end_pos - pos);
memcpy(kbuf + pos, data, copy);
}
@@ -1010,13 +1010,13 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int po
}
static inline int
-__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos, unsigned int count, const int start_pos, const int end_pos)
+__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos, unsigned int count, int start_pos, int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
if (end_pos < 0 || pos < end_pos) {
- unsigned int copy = (end_pos < 0 ? count : min(count, end_pos - pos));
+ unsigned int copy = end_pos < 0 ? count : min(count, end_pos - pos);
if (__copy_to_user(ubuf + pos, data, copy))
return -EFAULT;
@@ -1026,7 +1026,7 @@ __copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos, uns
/*
* Convert from kernel XSAVES compacted format to standard format and copy
- * to a ptrace buffer. It supports partial copy but pos always starts from
+ * to a user-space buffer. It supports partial copy but pos always starts from
* zero. This is called from xstateregs_get() and there we check the CPU
* has XSAVES.
*/
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-09-23 15:10 +0200 |
| Subject | [PATCH 04/33] x86/fpu: Remove 'kbuf' parameter from the copy_xstate_to_user() APIs |
| Message-ID | <usSlZ-3wU-59@gated-at.bofh.it> |
| In reply to | #1737999 |
The 'kbuf' parameter is unused in the _user() side of the API, remove it.
This simplifies the code and makes it easier to think about.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/fpu/xstate.h | 2 +-
arch/x86/kernel/fpu/regset.c | 2 +-
arch/x86/kernel/fpu/xstate.c | 25 +++++++------------------
3 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index c762574a245f..65bd68c30cd0 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -49,7 +49,7 @@ void *get_xsave_addr(struct xregs_state *xsave, int xstate);
const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave);
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf, struct xregs_state *xsave);
+int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave);
int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave);
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index 34e74adf9d5d..fd6dbdd8fde6 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -95,7 +95,7 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
if (kbuf)
ret = copy_xstate_to_kernel(pos, count, kbuf, xsave);
else
- ret = copy_xstate_to_user(pos, count, kbuf, ubuf, xsave);
+ ret = copy_xstate_to_user(pos, count, ubuf, xsave);
} else {
fpstate_sanitize_xstate(fpu);
/*
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 71d3bda2b898..2d8f3344875d 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1011,10 +1011,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
}
static inline int
-__copy_xstate_to_user(unsigned int pos, unsigned int count,
- void *kbuf, void __user *ubuf,
- const void *data, const int start_pos,
- const int end_pos)
+__copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, const void *data, const int start_pos, const int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
@@ -1022,12 +1019,8 @@ __copy_xstate_to_user(unsigned int pos, unsigned int count,
if (end_pos < 0 || pos < end_pos) {
unsigned int copy = (end_pos < 0 ? count : min(count, end_pos - pos));
- if (kbuf) {
- memcpy(kbuf + pos, data, copy);
- } else {
- if (__copy_to_user(ubuf + pos, data, copy))
- return -EFAULT;
- }
+ if (__copy_to_user(ubuf + pos, data, copy))
+ return -EFAULT;
}
return 0;
}
@@ -1038,8 +1031,7 @@ __copy_xstate_to_user(unsigned int pos, unsigned int count,
* zero. This is called from xstateregs_get() and there we check the CPU
* has XSAVES.
*/
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf,
- void __user *ubuf, struct xregs_state *xsave)
+int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave)
{
unsigned int offset, size;
int ret, i;
@@ -1064,8 +1056,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf,
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_user(offset, size, kbuf, ubuf, &header, 0, count);
-
+ ret = __copy_xstate_to_user(offset, size, ubuf, &header, 0, count);
if (ret)
return ret;
@@ -1079,8 +1070,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf,
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_user(offset, size, kbuf, ubuf, src, 0, count);
-
+ ret = __copy_xstate_to_user(offset, size, ubuf, src, 0, count);
if (ret)
return ret;
@@ -1096,8 +1086,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf,
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_user(offset, size, kbuf, ubuf, xstate_fx_sw_bytes, 0, count);
-
+ ret = __copy_xstate_to_user(offset, size, ubuf, xstate_fx_sw_bytes, 0, count);
if (ret)
return ret;
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Ingo Molnar <tipbot@zytor.com> |
|---|---|
| Date | 2017-09-26 10:40 +0200 |
| Subject | [tip:x86/fpu] x86/fpu: Remove 'kbuf' parameter from the copy_xstate_to_user() APIs |
| Message-ID | <utTzk-2jm-11@gated-at.bofh.it> |
| In reply to | #1738025 |
Commit-ID: a69c158fb3e7a91220f55029bf222a4e678d16e9
Gitweb: http://git.kernel.org/tip/a69c158fb3e7a91220f55029bf222a4e678d16e9
Author: Ingo Molnar <mingo@kernel.org>
AuthorDate: Sat, 23 Sep 2017 14:59:47 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 24 Sep 2017 13:04:31 +0200
x86/fpu: Remove 'kbuf' parameter from the copy_xstate_to_user() APIs
The 'kbuf' parameter is unused in the _user() side of the API, remove it.
This simplifies the code and makes it easier to think about.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Link: http://lkml.kernel.org/r/20170923130016.21448-5-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/fpu/xstate.h | 2 +-
arch/x86/kernel/fpu/regset.c | 2 +-
arch/x86/kernel/fpu/xstate.c | 25 +++++++------------------
3 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index c762574..65bd68c3 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -49,7 +49,7 @@ void *get_xsave_addr(struct xregs_state *xsave, int xstate);
const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave);
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf, struct xregs_state *xsave);
+int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave);
int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave);
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index 34e74ad..fd6dbdd 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -95,7 +95,7 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
if (kbuf)
ret = copy_xstate_to_kernel(pos, count, kbuf, xsave);
else
- ret = copy_xstate_to_user(pos, count, kbuf, ubuf, xsave);
+ ret = copy_xstate_to_user(pos, count, ubuf, xsave);
} else {
fpstate_sanitize_xstate(fpu);
/*
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 71d3bda..2d8f334 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1011,10 +1011,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
}
static inline int
-__copy_xstate_to_user(unsigned int pos, unsigned int count,
- void *kbuf, void __user *ubuf,
- const void *data, const int start_pos,
- const int end_pos)
+__copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, const void *data, const int start_pos, const int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
@@ -1022,12 +1019,8 @@ __copy_xstate_to_user(unsigned int pos, unsigned int count,
if (end_pos < 0 || pos < end_pos) {
unsigned int copy = (end_pos < 0 ? count : min(count, end_pos - pos));
- if (kbuf) {
- memcpy(kbuf + pos, data, copy);
- } else {
- if (__copy_to_user(ubuf + pos, data, copy))
- return -EFAULT;
- }
+ if (__copy_to_user(ubuf + pos, data, copy))
+ return -EFAULT;
}
return 0;
}
@@ -1038,8 +1031,7 @@ __copy_xstate_to_user(unsigned int pos, unsigned int count,
* zero. This is called from xstateregs_get() and there we check the CPU
* has XSAVES.
*/
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf,
- void __user *ubuf, struct xregs_state *xsave)
+int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave)
{
unsigned int offset, size;
int ret, i;
@@ -1064,8 +1056,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf,
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_user(offset, size, kbuf, ubuf, &header, 0, count);
-
+ ret = __copy_xstate_to_user(offset, size, ubuf, &header, 0, count);
if (ret)
return ret;
@@ -1079,8 +1070,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf,
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_user(offset, size, kbuf, ubuf, src, 0, count);
-
+ ret = __copy_xstate_to_user(offset, size, ubuf, src, 0, count);
if (ret)
return ret;
@@ -1096,8 +1086,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf,
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_user(offset, size, kbuf, ubuf, xstate_fx_sw_bytes, 0, count);
-
+ ret = __copy_xstate_to_user(offset, size, ubuf, xstate_fx_sw_bytes, 0, count);
if (ret)
return ret;
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-09-23 15:10 +0200 |
| Subject | [PATCH 05/33] x86/fpu: Clean up parameter order in the copy_xstate_to_*() APIs |
| Message-ID | <usSlZ-3wU-53@gated-at.bofh.it> |
| In reply to | #1737999 |
Parameter ordering is weird:
int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave);
int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave);
'pos' and 'count', which are attributes of the destination buffer, are listed before the destination
buffer itself ...
List them after the primary arguments instead.
This makes the code more similar to regular memcpy() variant APIs.
No change in functionality.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/fpu/xstate.h | 4 ++--
arch/x86/kernel/fpu/regset.c | 4 ++--
arch/x86/kernel/fpu/xstate.c | 25 ++++++++++++-------------
3 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 65bd68c30cd0..e4430b84939d 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -48,8 +48,8 @@ void fpu__xstate_clear_all_cpu_caps(void);
void *get_xsave_addr(struct xregs_state *xsave, int xstate);
const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave);
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave);
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int pos, unsigned int count);
+int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned int pos, unsigned int count);
int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave);
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index fd6dbdd8fde6..ec1404194b65 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -93,9 +93,9 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
if (using_compacted_format()) {
if (kbuf)
- ret = copy_xstate_to_kernel(pos, count, kbuf, xsave);
+ ret = copy_xstate_to_kernel(kbuf, xsave, pos, count);
else
- ret = copy_xstate_to_user(pos, count, ubuf, xsave);
+ ret = copy_xstate_to_user(ubuf, xsave, pos, count);
} else {
fpstate_sanitize_xstate(fpu);
/*
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 2d8f3344875d..0a299468510f 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -925,10 +925,9 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
* the source data pointer or increment pos, count, kbuf, and ubuf.
*/
static inline int
-__copy_xstate_to_kernel(unsigned int pos, unsigned int count,
- void *kbuf,
- const void *data, const int start_pos,
- const int end_pos)
+__copy_xstate_to_kernel(void *kbuf,
+ const void *data,
+ unsigned int pos, unsigned int count, const int start_pos, const int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
@@ -948,7 +947,7 @@ __copy_xstate_to_kernel(unsigned int pos, unsigned int count,
* It supports partial copy but pos always starts from zero. This is called
* from xstateregs_get() and there we check the CPU has XSAVES.
*/
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave)
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int pos, unsigned int count)
{
unsigned int offset, size;
int ret, i;
@@ -973,7 +972,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_kernel(offset, size, kbuf, &header, 0, count);
+ ret = __copy_xstate_to_kernel(kbuf, &header, offset, size, 0, count);
if (ret)
return ret;
@@ -987,7 +986,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_kernel(offset, size, kbuf, src, 0, count);
+ ret = __copy_xstate_to_kernel(kbuf, src, offset, size, 0, count);
if (ret)
return ret;
@@ -1003,7 +1002,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_kernel(offset, size, kbuf, xstate_fx_sw_bytes, 0, count);
+ ret = __copy_xstate_to_kernel(kbuf, xstate_fx_sw_bytes, offset, size, 0, count);
if (ret)
return ret;
@@ -1011,7 +1010,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
}
static inline int
-__copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, const void *data, const int start_pos, const int end_pos)
+__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos, unsigned int count, const int start_pos, const int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
@@ -1031,7 +1030,7 @@ __copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, c
* zero. This is called from xstateregs_get() and there we check the CPU
* has XSAVES.
*/
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave)
+int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned int pos, unsigned int count)
{
unsigned int offset, size;
int ret, i;
@@ -1056,7 +1055,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf,
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_user(offset, size, ubuf, &header, 0, count);
+ ret = __copy_xstate_to_user(ubuf, &header, offset, size, 0, count);
if (ret)
return ret;
@@ -1070,7 +1069,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf,
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_user(offset, size, ubuf, src, 0, count);
+ ret = __copy_xstate_to_user(ubuf, src, offset, size, 0, count);
if (ret)
return ret;
@@ -1086,7 +1085,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf,
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_user(offset, size, ubuf, xstate_fx_sw_bytes, 0, count);
+ ret = __copy_xstate_to_user(ubuf, xstate_fx_sw_bytes, offset, size, 0, count);
if (ret)
return ret;
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Ingo Molnar <tipbot@zytor.com> |
|---|---|
| Date | 2017-09-26 10:40 +0200 |
| Subject | [tip:x86/fpu] x86/fpu: Clean up parameter order in the copy_xstate_to_*() APIs |
| Message-ID | <utTzk-2jm-7@gated-at.bofh.it> |
| In reply to | #1738026 |
Commit-ID: d7eda6c99cc75f1c41d67abf988f37a10045a370
Gitweb: http://git.kernel.org/tip/d7eda6c99cc75f1c41d67abf988f37a10045a370
Author: Ingo Molnar <mingo@kernel.org>
AuthorDate: Sat, 23 Sep 2017 14:59:48 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 24 Sep 2017 13:04:31 +0200
x86/fpu: Clean up parameter order in the copy_xstate_to_*() APIs
Parameter ordering is weird:
int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave);
int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave);
'pos' and 'count', which are attributes of the destination buffer, are listed before the destination
buffer itself ...
List them after the primary arguments instead.
This makes the code more similar to regular memcpy() variant APIs.
No change in functionality.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Link: http://lkml.kernel.org/r/20170923130016.21448-6-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/fpu/xstate.h | 4 ++--
arch/x86/kernel/fpu/regset.c | 4 ++--
arch/x86/kernel/fpu/xstate.c | 25 ++++++++++++-------------
3 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 65bd68c3..e4430b8 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -48,8 +48,8 @@ void fpu__xstate_clear_all_cpu_caps(void);
void *get_xsave_addr(struct xregs_state *xsave, int xstate);
const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave);
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave);
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int pos, unsigned int count);
+int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned int pos, unsigned int count);
int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave);
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index fd6dbdd..ec14041 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -93,9 +93,9 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
if (using_compacted_format()) {
if (kbuf)
- ret = copy_xstate_to_kernel(pos, count, kbuf, xsave);
+ ret = copy_xstate_to_kernel(kbuf, xsave, pos, count);
else
- ret = copy_xstate_to_user(pos, count, ubuf, xsave);
+ ret = copy_xstate_to_user(ubuf, xsave, pos, count);
} else {
fpstate_sanitize_xstate(fpu);
/*
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 2d8f334..0a29946 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -925,10 +925,9 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
* the source data pointer or increment pos, count, kbuf, and ubuf.
*/
static inline int
-__copy_xstate_to_kernel(unsigned int pos, unsigned int count,
- void *kbuf,
- const void *data, const int start_pos,
- const int end_pos)
+__copy_xstate_to_kernel(void *kbuf,
+ const void *data,
+ unsigned int pos, unsigned int count, const int start_pos, const int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
@@ -948,7 +947,7 @@ __copy_xstate_to_kernel(unsigned int pos, unsigned int count,
* It supports partial copy but pos always starts from zero. This is called
* from xstateregs_get() and there we check the CPU has XSAVES.
*/
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave)
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int pos, unsigned int count)
{
unsigned int offset, size;
int ret, i;
@@ -973,7 +972,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_kernel(offset, size, kbuf, &header, 0, count);
+ ret = __copy_xstate_to_kernel(kbuf, &header, offset, size, 0, count);
if (ret)
return ret;
@@ -987,7 +986,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_kernel(offset, size, kbuf, src, 0, count);
+ ret = __copy_xstate_to_kernel(kbuf, src, offset, size, 0, count);
if (ret)
return ret;
@@ -1003,7 +1002,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_kernel(offset, size, kbuf, xstate_fx_sw_bytes, 0, count);
+ ret = __copy_xstate_to_kernel(kbuf, xstate_fx_sw_bytes, offset, size, 0, count);
if (ret)
return ret;
@@ -1011,7 +1010,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
}
static inline int
-__copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, const void *data, const int start_pos, const int end_pos)
+__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos, unsigned int count, const int start_pos, const int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
@@ -1031,7 +1030,7 @@ __copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, c
* zero. This is called from xstateregs_get() and there we check the CPU
* has XSAVES.
*/
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave)
+int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned int pos, unsigned int count)
{
unsigned int offset, size;
int ret, i;
@@ -1056,7 +1055,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf,
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_user(offset, size, ubuf, &header, 0, count);
+ ret = __copy_xstate_to_user(ubuf, &header, offset, size, 0, count);
if (ret)
return ret;
@@ -1070,7 +1069,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf,
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_user(offset, size, ubuf, src, 0, count);
+ ret = __copy_xstate_to_user(ubuf, src, offset, size, 0, count);
if (ret)
return ret;
@@ -1086,7 +1085,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf,
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_user(offset, size, ubuf, xstate_fx_sw_bytes, 0, count);
+ ret = __copy_xstate_to_user(ubuf, xstate_fx_sw_bytes, offset, size, 0, count);
if (ret)
return ret;
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-09-23 15:10 +0200 |
| Subject | [PATCH 24/33] x86/fpu: Don't let userspace set bogus xcomp_bv |
| Message-ID | <usSm0-3wU-65@gated-at.bofh.it> |
| In reply to | #1737999 |
From: Eric Biggers <ebiggers@google.com>
On x86, userspace can use the ptrace() or rt_sigreturn() system calls to
set a task's extended state (xstate) or "FPU" registers. ptrace() can
set them for another task using the PTRACE_SETREGSET request with
NT_X86_XSTATE, while rt_sigreturn() can set them for the current task.
In either case, registers can be set to any value, but the kernel
assumes that the XSAVE area itself remains valid in the sense that the
CPU can restore it.
However, in the case where the kernel is using the uncompacted xstate
format (which it does whenever the XSAVES instruction is unavailable),
it was possible for userspace to set the xcomp_bv field in the
xstate_header to an arbitrary value. However, all bits in that field
are reserved in the uncompacted case, so when switching to a task with
nonzero xcomp_bv, the XRSTOR instruction failed with a #GP fault. This
caused the WARN_ON_FPU(err) in copy_kernel_to_xregs() to be hit. In
addition, since the error is otherwise ignored, the FPU registers from
the task previously executing on the CPU were leaked.
Fix the bug by checking that the user-supplied value of xcomp_bv is 0 in
the uncompacted case, and returning an error otherwise.
The reason for validating xcomp_bv rather than simply overwriting it
with 0 is that we want userspace to see an error if it (incorrectly)
provides an XSAVE area in compacted format rather than in uncompacted
format.
Note that as before, in case of error we clear the task's FPU state.
This is perhaps non-ideal, especially for PTRACE_SETREGSET; it might be
better to return an error before changing anything. But it seems the
"clear on error" behavior is fine for now, and it's a little tricky to
do otherwise because it would mean we couldn't simply copy the full
userspace state into kernel memory in one __copy_from_user().
This bug was found by syzkaller, which hit the above-mentioned
WARN_ON_FPU():
WARNING: CPU: 1 PID: 0 at ./arch/x86/include/asm/fpu/internal.h:373 __switch_to+0x5b5/0x5d0
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.13.0 #453
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff9ba2bc8e42c0 task.stack: ffffa78cc036c000
RIP: 0010:__switch_to+0x5b5/0x5d0
RSP: 0000:ffffa78cc08bbb88 EFLAGS: 00010082
RAX: 00000000fffffffe RBX: ffff9ba2b8bf2180 RCX: 00000000c0000100
RDX: 00000000ffffffff RSI: 000000005cb10700 RDI: ffff9ba2b8bf36c0
RBP: ffffa78cc08bbbd0 R08: 00000000929fdf46 R09: 0000000000000001
R10: 0000000000000000 R11: 0000000000000000 R12: ffff9ba2bc8e42c0
R13: 0000000000000000 R14: ffff9ba2b8bf3680 R15: ffff9ba2bf5d7b40
FS: 00007f7e5cb10700(0000) GS:ffff9ba2bf400000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004005cc CR3: 0000000079fd5000 CR4: 00000000001406e0
Call Trace:
Code: 84 00 00 00 00 00 e9 11 fd ff ff 0f ff 66 0f 1f 84 00 00 00 00 00 e9 e7 fa ff ff 0f ff 66 0f 1f 84 00 00 00 00 00 e9 c2 fa ff ff <0f> ff 66 0f 1f 84 00 00 00 00 00 e9 d4 fc ff ff 66 66 2e 0f 1f
Here is a C reproducer. The expected behavior is that the program spin
forever with no output. However, on a buggy kernel running on a
processor with the "xsave" feature but without the "xsaves" feature
(e.g. Sandy Bridge through Broadwell for Intel), within a second or two
the program reports that the xmm registers were corrupted, i.e. were not
restored correctly. With CONFIG_X86_DEBUG_FPU=y it also hits the above
kernel warning.
#define _GNU_SOURCE
#include <stdbool.h>
#include <inttypes.h>
#include <linux/elf.h>
#include <stdio.h>
#include <sys/ptrace.h>
#include <sys/uio.h>
#include <sys/wait.h>
#include <unistd.h>
int main(void)
{
int pid = fork();
uint64_t xstate[512];
struct iovec iov = { .iov_base = xstate, .iov_len = sizeof(xstate) };
if (pid == 0) {
bool tracee = true;
for (int i = 0; i < sysconf(_SC_NPROCESSORS_ONLN) && tracee; i++)
tracee = (fork() != 0);
uint32_t xmm0[4] = { [0 ... 3] = tracee ? 0x00000000 : 0xDEADBEEF };
asm volatile(" movdqu %0, %%xmm0\n"
" mov %0, %%rbx\n"
"1: movdqu %%xmm0, %0\n"
" mov %0, %%rax\n"
" cmp %%rax, %%rbx\n"
" je 1b\n"
: "+m" (xmm0) : : "rax", "rbx", "xmm0");
printf("BUG: xmm registers corrupted! tracee=%d, xmm0=%08X%08X%08X%08X\n",
tracee, xmm0[0], xmm0[1], xmm0[2], xmm0[3]);
} else {
usleep(100000);
ptrace(PTRACE_ATTACH, pid, 0, 0);
wait(NULL);
ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov);
xstate[65] = -1;
ptrace(PTRACE_SETREGSET, pid, NT_X86_XSTATE, &iov);
ptrace(PTRACE_CONT, pid, 0, 0);
wait(NULL);
}
return 1;
}
Note: the program only tests for the bug using the ptrace() system call.
The bug can also be reproduced using the rt_sigreturn() system call, but
only when called from a 32-bit program, since for 64-bit programs the
kernel restores the FPU state from the signal frame by doing XRSTOR
directly from userspace memory (with proper error checking).
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Rik van Riel <riel@redhat.com>
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: <stable@vger.kernel.org> [v3.17+]
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Fixes: 0b29643a5843 ("x86/xsaves: Change compacted format xsave area header")
Link: http://lkml.kernel.org/r/20170922174156.16780-2-ebiggers3@gmail.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/fpu/regset.c | 4 ++++
arch/x86/kernel/fpu/signal.c | 9 +++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index 19a7385a912c..c764f7405322 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -141,6 +141,10 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
ret = copy_user_to_xstate(xsave, ubuf);
} else {
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
+
+ /* xcomp_bv must be 0 when using uncompacted format */
+ if (!ret && xsave->header.xcomp_bv)
+ ret = -EINVAL;
}
/*
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 629106e51a29..d34349934702 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -324,11 +324,16 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
*/
fpu__drop(fpu);
- if (using_compacted_format())
+ if (using_compacted_format()) {
err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
- else
+ } else {
err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
+ /* xcomp_bv must be 0 when using uncompacted format */
+ if (!err && fpu->state.xsave.header.xcomp_bv)
+ err = -EINVAL;
+ }
+
if (err || __copy_from_user(&env, buf, sizeof(env))) {
fpstate_init(&fpu->state);
trace_x86_fpu_init_state(fpu);
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Eric Biggers <tipbot@zytor.com> |
|---|---|
| Date | 2017-09-26 10:50 +0200 |
| Subject | [tip:x86/fpu] x86/fpu: Don't let userspace set bogus xcomp_bv |
| Message-ID | <utTIZ-2nL-7@gated-at.bofh.it> |
| In reply to | #1738027 |
Commit-ID: 814fb7bb7db5433757d76f4c4502c96fc53b0b5e
Gitweb: http://git.kernel.org/tip/814fb7bb7db5433757d76f4c4502c96fc53b0b5e
Author: Eric Biggers <ebiggers@google.com>
AuthorDate: Sat, 23 Sep 2017 15:00:07 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 25 Sep 2017 09:26:32 +0200
x86/fpu: Don't let userspace set bogus xcomp_bv
On x86, userspace can use the ptrace() or rt_sigreturn() system calls to
set a task's extended state (xstate) or "FPU" registers. ptrace() can
set them for another task using the PTRACE_SETREGSET request with
NT_X86_XSTATE, while rt_sigreturn() can set them for the current task.
In either case, registers can be set to any value, but the kernel
assumes that the XSAVE area itself remains valid in the sense that the
CPU can restore it.
However, in the case where the kernel is using the uncompacted xstate
format (which it does whenever the XSAVES instruction is unavailable),
it was possible for userspace to set the xcomp_bv field in the
xstate_header to an arbitrary value. However, all bits in that field
are reserved in the uncompacted case, so when switching to a task with
nonzero xcomp_bv, the XRSTOR instruction failed with a #GP fault. This
caused the WARN_ON_FPU(err) in copy_kernel_to_xregs() to be hit. In
addition, since the error is otherwise ignored, the FPU registers from
the task previously executing on the CPU were leaked.
Fix the bug by checking that the user-supplied value of xcomp_bv is 0 in
the uncompacted case, and returning an error otherwise.
The reason for validating xcomp_bv rather than simply overwriting it
with 0 is that we want userspace to see an error if it (incorrectly)
provides an XSAVE area in compacted format rather than in uncompacted
format.
Note that as before, in case of error we clear the task's FPU state.
This is perhaps non-ideal, especially for PTRACE_SETREGSET; it might be
better to return an error before changing anything. But it seems the
"clear on error" behavior is fine for now, and it's a little tricky to
do otherwise because it would mean we couldn't simply copy the full
userspace state into kernel memory in one __copy_from_user().
This bug was found by syzkaller, which hit the above-mentioned
WARN_ON_FPU():
WARNING: CPU: 1 PID: 0 at ./arch/x86/include/asm/fpu/internal.h:373 __switch_to+0x5b5/0x5d0
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.13.0 #453
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff9ba2bc8e42c0 task.stack: ffffa78cc036c000
RIP: 0010:__switch_to+0x5b5/0x5d0
RSP: 0000:ffffa78cc08bbb88 EFLAGS: 00010082
RAX: 00000000fffffffe RBX: ffff9ba2b8bf2180 RCX: 00000000c0000100
RDX: 00000000ffffffff RSI: 000000005cb10700 RDI: ffff9ba2b8bf36c0
RBP: ffffa78cc08bbbd0 R08: 00000000929fdf46 R09: 0000000000000001
R10: 0000000000000000 R11: 0000000000000000 R12: ffff9ba2bc8e42c0
R13: 0000000000000000 R14: ffff9ba2b8bf3680 R15: ffff9ba2bf5d7b40
FS: 00007f7e5cb10700(0000) GS:ffff9ba2bf400000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004005cc CR3: 0000000079fd5000 CR4: 00000000001406e0
Call Trace:
Code: 84 00 00 00 00 00 e9 11 fd ff ff 0f ff 66 0f 1f 84 00 00 00 00 00 e9 e7 fa ff ff 0f ff 66 0f 1f 84 00 00 00 00 00 e9 c2 fa ff ff <0f> ff 66 0f 1f 84 00 00 00 00 00 e9 d4 fc ff ff 66 66 2e 0f 1f
Here is a C reproducer. The expected behavior is that the program spin
forever with no output. However, on a buggy kernel running on a
processor with the "xsave" feature but without the "xsaves" feature
(e.g. Sandy Bridge through Broadwell for Intel), within a second or two
the program reports that the xmm registers were corrupted, i.e. were not
restored correctly. With CONFIG_X86_DEBUG_FPU=y it also hits the above
kernel warning.
#define _GNU_SOURCE
#include <stdbool.h>
#include <inttypes.h>
#include <linux/elf.h>
#include <stdio.h>
#include <sys/ptrace.h>
#include <sys/uio.h>
#include <sys/wait.h>
#include <unistd.h>
int main(void)
{
int pid = fork();
uint64_t xstate[512];
struct iovec iov = { .iov_base = xstate, .iov_len = sizeof(xstate) };
if (pid == 0) {
bool tracee = true;
for (int i = 0; i < sysconf(_SC_NPROCESSORS_ONLN) && tracee; i++)
tracee = (fork() != 0);
uint32_t xmm0[4] = { [0 ... 3] = tracee ? 0x00000000 : 0xDEADBEEF };
asm volatile(" movdqu %0, %%xmm0\n"
" mov %0, %%rbx\n"
"1: movdqu %%xmm0, %0\n"
" mov %0, %%rax\n"
" cmp %%rax, %%rbx\n"
" je 1b\n"
: "+m" (xmm0) : : "rax", "rbx", "xmm0");
printf("BUG: xmm registers corrupted! tracee=%d, xmm0=%08X%08X%08X%08X\n",
tracee, xmm0[0], xmm0[1], xmm0[2], xmm0[3]);
} else {
usleep(100000);
ptrace(PTRACE_ATTACH, pid, 0, 0);
wait(NULL);
ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov);
xstate[65] = -1;
ptrace(PTRACE_SETREGSET, pid, NT_X86_XSTATE, &iov);
ptrace(PTRACE_CONT, pid, 0, 0);
wait(NULL);
}
return 1;
}
Note: the program only tests for the bug using the ptrace() system call.
The bug can also be reproduced using the rt_sigreturn() system call, but
only when called from a 32-bit program, since for 64-bit programs the
kernel restores the FPU state from the signal frame by doing XRSTOR
directly from userspace memory (with proper error checking).
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Rik van Riel <riel@redhat.com>
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: <stable@vger.kernel.org> [v3.17+]
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Fixes: 0b29643a5843 ("x86/xsaves: Change compacted format xsave area header")
Link: http://lkml.kernel.org/r/20170922174156.16780-2-ebiggers3@gmail.com
Link: http://lkml.kernel.org/r/20170923130016.21448-25-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/fpu/regset.c | 4 ++++
arch/x86/kernel/fpu/signal.c | 9 +++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index 19a7385..c764f74 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -141,6 +141,10 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
ret = copy_user_to_xstate(xsave, ubuf);
} else {
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
+
+ /* xcomp_bv must be 0 when using uncompacted format */
+ if (!ret && xsave->header.xcomp_bv)
+ ret = -EINVAL;
}
/*
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 629106e..da68ea1 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -324,11 +324,16 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
*/
fpu__drop(fpu);
- if (using_compacted_format())
+ if (using_compacted_format()) {
err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
- else
+ } else {
err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
+ /* xcomp_bv must be 0 when using uncompacted format */
+ if (!err && state_size > offsetof(struct xregs_state, header) && fpu->state.xsave.header.xcomp_bv)
+ err = -EINVAL;
+ }
+
if (err || __copy_from_user(&env, buf, sizeof(env))) {
fpstate_init(&fpu->state);
trace_x86_fpu_init_state(fpu);
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-09-23 15:10 +0200 |
| Subject | [PATCH 18/33] x86/fpu: Change fpu->fpregs_active users to fpu->fpstate_active |
| Message-ID | <usSlZ-3wU-55@gated-at.bofh.it> |
| In reply to | #1737999 |
We want to simplify the FPU state machine by eliminating fpu->fpregs_active,
and we can do that because the two state flags (::fpregs_active and
::fpstate_active) are set essentially together.
The old lazy FPU switching code used to make a distinction - but there's
no lazy switching code anymore, we always switch in an 'eager' fashion.
Do this by first changing all substantial uses of fpu->fpregs_active
to fpu->fpstate_active and adding a few debug checks to double check
our assumption is correct.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/fpu/internal.h | 4 +++-
arch/x86/kernel/fpu/core.c | 16 ++++++++++------
arch/x86/kernel/fpu/signal.c | 4 +++-
arch/x86/mm/pkeys.c | 3 +--
4 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index b223c57dd5dc..7fa676f93ac1 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -556,7 +556,9 @@ static inline void fpregs_activate(struct fpu *fpu)
static inline void
switch_fpu_prepare(struct fpu *old_fpu, int cpu)
{
- if (old_fpu->fpregs_active) {
+ WARN_ON_FPU(old_fpu->fpregs_active != old_fpu->fpstate_active);
+
+ if (old_fpu->fpstate_active) {
if (!copy_fpregs_to_fpstate(old_fpu))
old_fpu->last_cpu = -1;
else
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 815dfba7781a..eab244622402 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -100,7 +100,7 @@ void __kernel_fpu_begin(void)
kernel_fpu_disable();
- if (fpu->fpregs_active) {
+ if (fpu->fpstate_active) {
/*
* Ignore return value -- we don't care if reg state
* is clobbered.
@@ -116,7 +116,7 @@ void __kernel_fpu_end(void)
{
struct fpu *fpu = ¤t->thread.fpu;
- if (fpu->fpregs_active)
+ if (fpu->fpstate_active)
copy_kernel_to_fpregs(&fpu->state);
kernel_fpu_enable();
@@ -147,8 +147,10 @@ void fpu__save(struct fpu *fpu)
WARN_ON_FPU(fpu != ¤t->thread.fpu);
preempt_disable();
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
+
trace_x86_fpu_before_save(fpu);
- if (fpu->fpregs_active) {
+ if (fpu->fpstate_active) {
if (!copy_fpregs_to_fpstate(fpu)) {
copy_kernel_to_fpregs(&fpu->state);
}
@@ -262,11 +264,12 @@ EXPORT_SYMBOL_GPL(fpu__activate_curr);
*/
void fpu__activate_fpstate_read(struct fpu *fpu)
{
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
/*
* If fpregs are active (in the current CPU), then
* copy them to the fpstate:
*/
- if (fpu->fpregs_active) {
+ if (fpu->fpstate_active) {
fpu__save(fpu);
} else {
if (!fpu->fpstate_active) {
@@ -362,12 +365,13 @@ void fpu__current_fpstate_write_end(void)
{
struct fpu *fpu = ¤t->thread.fpu;
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
/*
* 'fpu' now has an updated copy of the state, but the
* registers may still be out of date. Update them with
* an XRSTOR if they are active.
*/
- if (fpu->fpregs_active)
+ if (fpu->fpstate_active)
copy_kernel_to_fpregs(&fpu->state);
/*
@@ -417,7 +421,7 @@ void fpu__drop(struct fpu *fpu)
if (fpu == ¤t->thread.fpu) {
WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
- if (fpu->fpregs_active) {
+ if (fpu->fpstate_active) {
/* Ignore delayed exceptions from user space */
asm volatile("1: fwait\n"
"2:\n"
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 684025654d0c..a88083ba7f8b 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -171,7 +171,9 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
sizeof(struct user_i387_ia32_struct), NULL,
(struct _fpstate_32 __user *) buf) ? -1 : 1;
- if (fpu->fpregs_active || using_compacted_format()) {
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
+
+ if (fpu->fpstate_active || using_compacted_format()) {
/* Save the live register state to the user directly. */
if (copy_fpregs_to_sigframe(buf_fx))
return -1;
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index e2c23472233e..4d24269c071f 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -18,7 +18,6 @@
#include <asm/cpufeature.h> /* boot_cpu_has, ... */
#include <asm/mmu_context.h> /* vma_pkey() */
-#include <asm/fpu/internal.h> /* fpregs_active() */
int __execute_only_pkey(struct mm_struct *mm)
{
@@ -45,7 +44,7 @@ int __execute_only_pkey(struct mm_struct *mm)
*/
preempt_disable();
if (!need_to_set_mm_pkey &&
- current->thread.fpu.fpregs_active &&
+ current->thread.fpu.fpstate_active &&
!__pkru_allows_read(read_pkru(), execute_only_pkey)) {
preempt_enable();
return execute_only_pkey;
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Ingo Molnar <tipbot@zytor.com> |
|---|---|
| Date | 2017-09-26 10:40 +0200 |
| Subject | [tip:x86/fpu] x86/fpu: Change fpu->fpregs_active users to fpu->fpstate_active |
| Message-ID | <utTzl-2jm-41@gated-at.bofh.it> |
| In reply to | #1738028 |
Commit-ID: f1c8cd0176078c7bcafdc89cac447cab672a0b5e
Gitweb: http://git.kernel.org/tip/f1c8cd0176078c7bcafdc89cac447cab672a0b5e
Author: Ingo Molnar <mingo@kernel.org>
AuthorDate: Sat, 23 Sep 2017 15:00:01 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 24 Sep 2017 13:04:34 +0200
x86/fpu: Change fpu->fpregs_active users to fpu->fpstate_active
We want to simplify the FPU state machine by eliminating fpu->fpregs_active,
and we can do that because the two state flags (::fpregs_active and
::fpstate_active) are set essentially together.
The old lazy FPU switching code used to make a distinction - but there's
no lazy switching code anymore, we always switch in an 'eager' fashion.
Do this by first changing all substantial uses of fpu->fpregs_active
to fpu->fpstate_active and adding a few debug checks to double check
our assumption is correct.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Link: http://lkml.kernel.org/r/20170923130016.21448-19-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/fpu/internal.h | 4 +++-
arch/x86/kernel/fpu/core.c | 16 ++++++++++------
arch/x86/kernel/fpu/signal.c | 4 +++-
arch/x86/mm/pkeys.c | 3 +--
4 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index b223c57..7fa676f 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -556,7 +556,9 @@ static inline void fpregs_activate(struct fpu *fpu)
static inline void
switch_fpu_prepare(struct fpu *old_fpu, int cpu)
{
- if (old_fpu->fpregs_active) {
+ WARN_ON_FPU(old_fpu->fpregs_active != old_fpu->fpstate_active);
+
+ if (old_fpu->fpstate_active) {
if (!copy_fpregs_to_fpstate(old_fpu))
old_fpu->last_cpu = -1;
else
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 815dfba..eab2446 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -100,7 +100,7 @@ void __kernel_fpu_begin(void)
kernel_fpu_disable();
- if (fpu->fpregs_active) {
+ if (fpu->fpstate_active) {
/*
* Ignore return value -- we don't care if reg state
* is clobbered.
@@ -116,7 +116,7 @@ void __kernel_fpu_end(void)
{
struct fpu *fpu = ¤t->thread.fpu;
- if (fpu->fpregs_active)
+ if (fpu->fpstate_active)
copy_kernel_to_fpregs(&fpu->state);
kernel_fpu_enable();
@@ -147,8 +147,10 @@ void fpu__save(struct fpu *fpu)
WARN_ON_FPU(fpu != ¤t->thread.fpu);
preempt_disable();
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
+
trace_x86_fpu_before_save(fpu);
- if (fpu->fpregs_active) {
+ if (fpu->fpstate_active) {
if (!copy_fpregs_to_fpstate(fpu)) {
copy_kernel_to_fpregs(&fpu->state);
}
@@ -262,11 +264,12 @@ EXPORT_SYMBOL_GPL(fpu__activate_curr);
*/
void fpu__activate_fpstate_read(struct fpu *fpu)
{
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
/*
* If fpregs are active (in the current CPU), then
* copy them to the fpstate:
*/
- if (fpu->fpregs_active) {
+ if (fpu->fpstate_active) {
fpu__save(fpu);
} else {
if (!fpu->fpstate_active) {
@@ -362,12 +365,13 @@ void fpu__current_fpstate_write_end(void)
{
struct fpu *fpu = ¤t->thread.fpu;
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
/*
* 'fpu' now has an updated copy of the state, but the
* registers may still be out of date. Update them with
* an XRSTOR if they are active.
*/
- if (fpu->fpregs_active)
+ if (fpu->fpstate_active)
copy_kernel_to_fpregs(&fpu->state);
/*
@@ -417,7 +421,7 @@ void fpu__drop(struct fpu *fpu)
if (fpu == ¤t->thread.fpu) {
WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
- if (fpu->fpregs_active) {
+ if (fpu->fpstate_active) {
/* Ignore delayed exceptions from user space */
asm volatile("1: fwait\n"
"2:\n"
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 6840256..a88083b 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -171,7 +171,9 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
sizeof(struct user_i387_ia32_struct), NULL,
(struct _fpstate_32 __user *) buf) ? -1 : 1;
- if (fpu->fpregs_active || using_compacted_format()) {
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
+
+ if (fpu->fpstate_active || using_compacted_format()) {
/* Save the live register state to the user directly. */
if (copy_fpregs_to_sigframe(buf_fx))
return -1;
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index e2c2347..4d24269 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -18,7 +18,6 @@
#include <asm/cpufeature.h> /* boot_cpu_has, ... */
#include <asm/mmu_context.h> /* vma_pkey() */
-#include <asm/fpu/internal.h> /* fpregs_active() */
int __execute_only_pkey(struct mm_struct *mm)
{
@@ -45,7 +44,7 @@ int __execute_only_pkey(struct mm_struct *mm)
*/
preempt_disable();
if (!need_to_set_mm_pkey &&
- current->thread.fpu.fpregs_active &&
+ current->thread.fpu.fpstate_active &&
!__pkru_allows_read(read_pkru(), execute_only_pkey)) {
preempt_enable();
return execute_only_pkey;
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-09-23 15:10 +0200 |
| Subject | [PATCH 27/33] x86/fpu: Simplify fpu__activate_fpstate_read() |
| Message-ID | <usSm0-3wU-73@gated-at.bofh.it> |
| In reply to | #1737999 |
fpu__activate_fpstate_read() can only ever be called for a non-current,
non-executing (stopped) task - so make sure this is checked via a warning
and remove the current-task logic.
This also fixes an incorrect (but harmless) warning introduced by one of
the earlier patches.
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/fpu/core.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 93103a909c47..7bd4edb76c1d 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -256,26 +256,18 @@ EXPORT_SYMBOL_GPL(fpu__activate_curr);
*
* If the task has not used the FPU before then initialize its
* fpstate.
- *
- * If the task has used the FPU before then save it.
*/
void fpu__activate_fpstate_read(struct fpu *fpu)
{
- /*
- * If fpregs are active (in the current CPU), then
- * copy them to the fpstate:
- */
- if (fpu->fpstate_active) {
- fpu__save(fpu);
- } else {
- if (!fpu->fpstate_active) {
- fpstate_init(&fpu->state);
- trace_x86_fpu_init_state(fpu);
+ WARN_ON_FPU(fpu == ¤t->thread.fpu);
- trace_x86_fpu_activate_state(fpu);
- /* Safe to do for current and for stopped child tasks: */
- fpu->fpstate_active = 1;
- }
+ if (!fpu->fpstate_active) {
+ fpstate_init(&fpu->state);
+ trace_x86_fpu_init_state(fpu);
+
+ trace_x86_fpu_activate_state(fpu);
+ /* Safe to do for current and for stopped child tasks: */
+ fpu->fpstate_active = 1;
}
}
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Ingo Molnar <tipbot@zytor.com> |
|---|---|
| Date | 2017-09-26 10:40 +0200 |
| Subject | [tip:x86/fpu] x86/fpu: Fix fpu__activate_fpstate_read() and update comments |
| Message-ID | <utTzl-2jm-47@gated-at.bofh.it> |
| In reply to | #1738029 |
Commit-ID: 4618e90965f272fe522f2af2523a60d0d4bc78f3
Gitweb: http://git.kernel.org/tip/4618e90965f272fe522f2af2523a60d0d4bc78f3
Author: Ingo Molnar <mingo@kernel.org>
AuthorDate: Sat, 23 Sep 2017 15:00:10 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 26 Sep 2017 09:41:09 +0200
x86/fpu: Fix fpu__activate_fpstate_read() and update comments
fpu__activate_fpstate_read() can be called for the current task
when coredumping - or for stopped tasks when ptrace-ing.
Implement this properly in the code and update the comments.
This also fixes an incorrect (but harmless) warning introduced by
one of the earlier patches.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Link: http://lkml.kernel.org/r/20170923130016.21448-28-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/fpu/core.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 93103a9..afd3f2a 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -254,18 +254,21 @@ EXPORT_SYMBOL_GPL(fpu__activate_curr);
/*
* This function must be called before we read a task's fpstate.
*
- * If the task has not used the FPU before then initialize its
- * fpstate.
+ * There's two cases where this gets called:
+ *
+ * - for the current task (when coredumping), in which case we have
+ * to save the latest FPU registers into the fpstate,
+ *
+ * - or it's called for stopped tasks (ptrace), in which case the
+ * registers were already saved by the context-switch code when
+ * the task scheduled out - we only have to initialize the registers
+ * if they've never been initialized.
*
* If the task has used the FPU before then save it.
*/
void fpu__activate_fpstate_read(struct fpu *fpu)
{
- /*
- * If fpregs are active (in the current CPU), then
- * copy them to the fpstate:
- */
- if (fpu->fpstate_active) {
+ if (fpu == ¤t->thread.fpu) {
fpu__save(fpu);
} else {
if (!fpu->fpstate_active) {
[toc] | [prev] | [next] | [standalone]
Page 3 of 4 — ← Prev page 1 2 [3] 4 Next page →
Back to top | Article view | linux.kernel
csiph-web