Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761501AbbEERxm (ORCPT ); Tue, 5 May 2015 13:53:42 -0400 Received: from mail-wi0-f174.google.com ([209.85.212.174]:33941 "EHLO mail-wi0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759779AbbEERxg (ORCPT ); Tue, 5 May 2015 13:53:36 -0400 From: Ingo Molnar To: linux-kernel@vger.kernel.org Cc: Andy Lutomirski , Borislav Petkov , Dave Hansen , Fenghua Yu , "H. Peter Anvin" , Linus Torvalds , Oleg Nesterov , Thomas Gleixner Subject: [PATCH 132/208] x86/fpu: Rename fpu_save_init() to copy_fpregs_to_fpstate() Date: Tue, 5 May 2015 19:50:24 +0200 Message-Id: <1430848300-27877-54-git-send-email-mingo@kernel.org> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1430848300-27877-1-git-send-email-mingo@kernel.org> References: <1430848300-27877-1-git-send-email-mingo@kernel.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6431 Lines: 183 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: Borislav Petkov Cc: Andy Lutomirski Cc: Dave Hansen Cc: Fenghua Yu Cc: H. Peter Anvin Cc: Linus Torvalds Cc: Oleg Nesterov Cc: Thomas Gleixner Signed-off-by: Ingo Molnar --- arch/x86/include/asm/fpu/internal.h | 34 ++++++++++++++++++++++++---------- arch/x86/kernel/fpu/core.c | 4 ++-- arch/x86/kernel/traps.c | 2 +- arch/x86/kvm/x86.c | 2 +- arch/x86/mm/mpx.c | 2 +- 5 files changed, 29 insertions(+), 15 deletions(-) diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 89c6ec80c1ac..11055f51e67a 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -265,9 +265,15 @@ static inline void fpu_fxsave(struct fpu *fpu) /* * 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()) { xsave_state(&fpu->state->xsave); @@ -276,13 +282,16 @@ static inline int fpu_save_init(struct fpu *fpu) * xsave header may indicate the init state of the FP. */ if (!(fpu->state->xsave.header.xfeatures & XSTATE_FP)) - return 1; - } else if (use_fxsr()) { - fpu_fxsave(fpu); + goto keep_fpregs; } else { - asm volatile("fnsave %[fx]; fwait" - : [fx] "=m" (fpu->state->fsave)); - return 0; + if (use_fxsr()) { + fpu_fxsave(fpu); + } else { + /* FNSAVE always clears FPU registers: */ + asm volatile("fnsave %[fx]; fwait" + : [fx] "=m" (fpu->state->fsave)); + goto drop_fpregs; + } } /* @@ -295,9 +304,14 @@ static inline int fpu_save_init(struct fpu *fpu) */ if (unlikely(fpu->state->fxsave.swd & X87_FSW_ES)) { asm volatile("fnclex"); - return 0; + goto drop_fpregs; } + +keep_fpregs: return 1; + +drop_fpregs: + return 0; } extern void fpu__save(struct fpu *fpu); @@ -448,7 +462,7 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu) (use_eager_fpu() || new_fpu->counter > 5); if (old_fpu->fpregs_active) { - if (!fpu_save_init(old_fpu)) + if (!copy_fpregs_to_fpstate(old_fpu)) old_fpu->last_cpu = -1; else old_fpu->last_cpu = cpu; diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index 34a4e1032424..538f2541b7f7 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -102,7 +102,7 @@ void __kernel_fpu_begin(void) kernel_fpu_disable(); if (fpu->fpregs_active) { - fpu_save_init(fpu); + copy_fpregs_to_fpstate(fpu); } else { this_cpu_write(fpu_fpregs_owner_ctx, NULL); if (!use_eager_fpu()) @@ -196,7 +196,7 @@ void fpu__save(struct fpu *fpu) if (use_eager_fpu()) { __save_fpu(fpu); } else { - fpu_save_init(fpu); + copy_fpregs_to_fpstate(fpu); fpregs_deactivate(fpu); } } diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index a65586edbb57..f028f1da3480 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -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 * 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); bndcsr = get_xsave_addr(xsave_buf, XSTATE_BNDCSR); if (!bndcsr) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 0b58b9397098..d90bf4afa2b0 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -7058,7 +7058,7 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) return; vcpu->guest_fpu_loaded = 0; - fpu_save_init(&vcpu->arch.guest_fpu); + copy_fpregs_to_fpstate(&vcpu->arch.guest_fpu); __kernel_fpu_end(); ++vcpu->stat.fpu_reload; kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c index 5563be313fd6..3287215be60a 100644 --- a/arch/x86/mm/mpx.c +++ b/arch/x86/mm/mpx.c @@ -357,7 +357,7 @@ static __user void *task_get_bounds_dir(struct task_struct *tsk) * The bounds directory pointer is stored in a register * 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); if (!bndcsr) return MPX_INVALID_BOUNDS_DIR; -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/