Commit 4f836347 authored by Ingo Molnar's avatar Ingo Molnar

x86/fpu: Rename fpu_save_init() to copy_fpregs_to_fpstate()

So fpu_save_init() is a historic name that got its name when the only
way the FPU state was FNSAVE, which cleared (well, destroyed) the FPU
state after saving it.

Nowadays the name is misleading, because ever since the introduction of
FXSAVE (and more modern FPU saving instructions) the 'we need to reload
the FPU state' part is only true if there's a pending FPU exception [*],
which is almost never the case.

So rename it to copy_fpregs_to_fpstate() to make it clear what's
happening. Also add a few comments about why we cannot keep registers
in certain cases.

Also clean up the control flow a bit, to make it more apparent when
we are dropping/keeping FP registers, and to optimize the common
case (of keeping fpregs) some more.

[*] Probably not true anymore, modern instructions always leave the FPU
    state intact, even if exceptions are pending: because pending FP
    exceptions are posted on the next FP instruction, not asynchronously.

    They were truly asynchronous back in the IRQ13 case, and we had to
    synchronize with them, but that code is not working anymore: we don't
    have IRQ13 mapped in the IDT anymore.

    But a cleanup patch is obviously not the place to change subtle behavior.
Reviewed-by: default avatarBorislav Petkov <bp@alien8.de>
Cc: Andy Lutomirski <luto@amacapital.net>
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: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 91066588
...@@ -265,9 +265,15 @@ static inline void fpu_fxsave(struct fpu *fpu) ...@@ -265,9 +265,15 @@ static inline void fpu_fxsave(struct fpu *fpu)
/* /*
* These must be called with preempt disabled. Returns * These must be called with preempt disabled. Returns
* 'true' if the FPU state is still intact. * 'true' if the FPU state is still intact and we can
* keep registers active.
*
* The legacy FNSAVE instruction cleared all FPU state
* unconditionally, so registers are essentially destroyed.
* Modern FPU state can be kept in registers, if there are
* no pending FP exceptions. (Note the FIXME below.)
*/ */
static inline int fpu_save_init(struct fpu *fpu) static inline int copy_fpregs_to_fpstate(struct fpu *fpu)
{ {
if (use_xsave()) { if (use_xsave()) {
xsave_state(&fpu->state->xsave); xsave_state(&fpu->state->xsave);
...@@ -276,13 +282,16 @@ static inline int fpu_save_init(struct fpu *fpu) ...@@ -276,13 +282,16 @@ static inline int fpu_save_init(struct fpu *fpu)
* xsave header may indicate the init state of the FP. * xsave header may indicate the init state of the FP.
*/ */
if (!(fpu->state->xsave.header.xfeatures & XSTATE_FP)) if (!(fpu->state->xsave.header.xfeatures & XSTATE_FP))
return 1; goto keep_fpregs;
} else if (use_fxsr()) { } else {
if (use_fxsr()) {
fpu_fxsave(fpu); fpu_fxsave(fpu);
} else { } else {
/* FNSAVE always clears FPU registers: */
asm volatile("fnsave %[fx]; fwait" asm volatile("fnsave %[fx]; fwait"
: [fx] "=m" (fpu->state->fsave)); : [fx] "=m" (fpu->state->fsave));
return 0; goto drop_fpregs;
}
} }
/* /*
...@@ -295,9 +304,14 @@ static inline int fpu_save_init(struct fpu *fpu) ...@@ -295,9 +304,14 @@ static inline int fpu_save_init(struct fpu *fpu)
*/ */
if (unlikely(fpu->state->fxsave.swd & X87_FSW_ES)) { if (unlikely(fpu->state->fxsave.swd & X87_FSW_ES)) {
asm volatile("fnclex"); asm volatile("fnclex");
return 0; goto drop_fpregs;
} }
keep_fpregs:
return 1; return 1;
drop_fpregs:
return 0;
} }
extern void fpu__save(struct fpu *fpu); extern void fpu__save(struct fpu *fpu);
...@@ -448,7 +462,7 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu) ...@@ -448,7 +462,7 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu)
(use_eager_fpu() || new_fpu->counter > 5); (use_eager_fpu() || new_fpu->counter > 5);
if (old_fpu->fpregs_active) { if (old_fpu->fpregs_active) {
if (!fpu_save_init(old_fpu)) if (!copy_fpregs_to_fpstate(old_fpu))
old_fpu->last_cpu = -1; old_fpu->last_cpu = -1;
else else
old_fpu->last_cpu = cpu; old_fpu->last_cpu = cpu;
......
...@@ -102,7 +102,7 @@ void __kernel_fpu_begin(void) ...@@ -102,7 +102,7 @@ void __kernel_fpu_begin(void)
kernel_fpu_disable(); kernel_fpu_disable();
if (fpu->fpregs_active) { if (fpu->fpregs_active) {
fpu_save_init(fpu); copy_fpregs_to_fpstate(fpu);
} else { } else {
this_cpu_write(fpu_fpregs_owner_ctx, NULL); this_cpu_write(fpu_fpregs_owner_ctx, NULL);
if (!use_eager_fpu()) if (!use_eager_fpu())
...@@ -196,7 +196,7 @@ void fpu__save(struct fpu *fpu) ...@@ -196,7 +196,7 @@ void fpu__save(struct fpu *fpu)
if (use_eager_fpu()) { if (use_eager_fpu()) {
__save_fpu(fpu); __save_fpu(fpu);
} else { } else {
fpu_save_init(fpu); copy_fpregs_to_fpstate(fpu);
fpregs_deactivate(fpu); fpregs_deactivate(fpu);
} }
} }
......
...@@ -395,7 +395,7 @@ dotraplinkage void do_bounds(struct pt_regs *regs, long error_code) ...@@ -395,7 +395,7 @@ dotraplinkage void do_bounds(struct pt_regs *regs, long error_code)
* It is not directly accessible, though, so we need to * It is not directly accessible, though, so we need to
* do an xsave and then pull it out of the xsave buffer. * do an xsave and then pull it out of the xsave buffer.
*/ */
fpu_save_init(&tsk->thread.fpu); copy_fpregs_to_fpstate(&tsk->thread.fpu);
xsave_buf = &(tsk->thread.fpu.state->xsave); xsave_buf = &(tsk->thread.fpu.state->xsave);
bndcsr = get_xsave_addr(xsave_buf, XSTATE_BNDCSR); bndcsr = get_xsave_addr(xsave_buf, XSTATE_BNDCSR);
if (!bndcsr) if (!bndcsr)
......
...@@ -7058,7 +7058,7 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) ...@@ -7058,7 +7058,7 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
return; return;
vcpu->guest_fpu_loaded = 0; vcpu->guest_fpu_loaded = 0;
fpu_save_init(&vcpu->arch.guest_fpu); copy_fpregs_to_fpstate(&vcpu->arch.guest_fpu);
__kernel_fpu_end(); __kernel_fpu_end();
++vcpu->stat.fpu_reload; ++vcpu->stat.fpu_reload;
kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu);
......
...@@ -357,7 +357,7 @@ static __user void *task_get_bounds_dir(struct task_struct *tsk) ...@@ -357,7 +357,7 @@ static __user void *task_get_bounds_dir(struct task_struct *tsk)
* The bounds directory pointer is stored in a register * The bounds directory pointer is stored in a register
* only accessible if we first do an xsave. * only accessible if we first do an xsave.
*/ */
fpu_save_init(&tsk->thread.fpu); copy_fpregs_to_fpstate(&tsk->thread.fpu);
bndcsr = get_xsave_addr(&tsk->thread.fpu.state->xsave, XSTATE_BNDCSR); bndcsr = get_xsave_addr(&tsk->thread.fpu.state->xsave, XSTATE_BNDCSR);
if (!bndcsr) if (!bndcsr)
return MPX_INVALID_BOUNDS_DIR; return MPX_INVALID_BOUNDS_DIR;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment