From 8c9e607376968865456b33d9a2efdee2c7e1030d Mon Sep 17 00:00:00 2001 From: Yu-cheng Yu Date: Thu, 12 Dec 2019 13:08:53 -0800 Subject: x86/fpu/xstate: Fix small issues In response to earlier comments, fix small issues before introducing XSAVES supervisor states: - Fix comments of xfeature_is_supervisor(). - Replace ((u64)1 << 63) with XCOMP_BV_COMPACTED_FORMAT. No functional changes. Signed-off-by: Yu-cheng Yu Signed-off-by: Borislav Petkov Reviewed-by: Dave Hansen Reviewed-by: Tony Luck Acked-by: Sebastian Andrzej Siewior Cc: Andy Lutomirski Cc: Dave Hansen Cc: Fenghua Yu Cc: "H. Peter Anvin" Cc: Ingo Molnar Cc: Peter Zijlstra Cc: "Ravi V. Shankar" Cc: Rik van Riel Cc: Thomas Gleixner Cc: x86-ml Link: https://lkml.kernel.org/r/20191212210855.19260-2-yu-cheng.yu@intel.com --- arch/x86/kernel/fpu/xstate.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'arch') diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index fa31470bbf24..4588fc57203e 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -110,12 +110,9 @@ EXPORT_SYMBOL_GPL(cpu_has_xfeatures); static int xfeature_is_supervisor(int xfeature_nr) { /* - * We currently do not support supervisor states, but if - * we did, we could find out like this. - * - * SDM says: If state component 'i' is a user state component, - * ECX[0] return 0; if state component i is a supervisor - * state component, ECX[0] returns 1. + * Extended State Enumeration Sub-leaves (EAX = 0DH, ECX = n, n > 1) + * returns ECX[0] set to (1) for a supervisor state, and cleared (0) + * for a user state. */ u32 eax, ebx, ecx, edx; @@ -419,7 +416,8 @@ static void __init setup_init_fpu_buf(void) print_xstate_features(); if (boot_cpu_has(X86_FEATURE_XSAVES)) - init_fpstate.xsave.header.xcomp_bv = (u64)1 << 63 | xfeatures_mask; + init_fpstate.xsave.header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT | + xfeatures_mask; /* * Init all the features state with header.xfeatures being 0x0 -- cgit From 158e2ee61f22b878d61de92bea5aad3d2df1c146 Mon Sep 17 00:00:00 2001 From: Yu-cheng Yu Date: Thu, 12 Dec 2019 13:08:54 -0800 Subject: x86/fpu/xstate: Make xfeature_is_supervisor()/xfeature_is_user() return bool Have both xfeature_is_supervisor()/xfeature_is_user() return bool because they are used only in boolean context. Suggested-by: Borislav Petkov Signed-off-by: Yu-cheng Yu Signed-off-by: Borislav Petkov Acked-by: Sebastian Andrzej Siewior Cc: Andy Lutomirski Cc: Dave Hansen Cc: Fenghua Yu Cc: "H. Peter Anvin" Cc: Ingo Molnar Cc: Peter Zijlstra Cc: "Ravi V. Shankar" Cc: Rik van Riel Cc: Thomas Gleixner Cc: Tony Luck Cc: x86-ml Link: https://lkml.kernel.org/r/20191212210855.19260-3-yu-cheng.yu@intel.com --- arch/x86/kernel/fpu/xstate.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'arch') diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 4588fc57203e..a1806598aaa4 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -107,7 +107,7 @@ int cpu_has_xfeatures(u64 xfeatures_needed, const char **feature_name) } EXPORT_SYMBOL_GPL(cpu_has_xfeatures); -static int xfeature_is_supervisor(int xfeature_nr) +static bool xfeature_is_supervisor(int xfeature_nr) { /* * Extended State Enumeration Sub-leaves (EAX = 0DH, ECX = n, n > 1) @@ -117,10 +117,10 @@ static int xfeature_is_supervisor(int xfeature_nr) u32 eax, ebx, ecx, edx; cpuid_count(XSTATE_CPUID, xfeature_nr, &eax, &ebx, &ecx, &edx); - return !!(ecx & 1); + return ecx & 1; } -static int xfeature_is_user(int xfeature_nr) +static bool xfeature_is_user(int xfeature_nr) { return !xfeature_is_supervisor(xfeature_nr); } -- cgit From bbc55341b9c67645d1a5471506370caf7dd4a203 Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Fri, 20 Dec 2019 20:59:06 +0100 Subject: x86/fpu: Deactivate FPU state after failure during state load In __fpu__restore_sig(), fpu_fpregs_owner_ctx needs to be reset if the FPU state was not fully restored. Otherwise the following may happen (on the same CPU): Task A Task B fpu_fpregs_owner_ctx *active* A.fpu __fpu__restore_sig() ctx switch load B.fpu *active* B.fpu fpregs_lock() copy_user_to_fpregs_zeroing() copy_kernel_to_xregs() *modify* copy_user_to_xregs() *fails* fpregs_unlock() ctx switch skip loading B.fpu, *active* B.fpu In the success case, fpu_fpregs_owner_ctx is set to the current task. In the failure case, the FPU state might have been modified by loading the init state. In this case, fpu_fpregs_owner_ctx needs to be reset in order to ensure that the FPU state of the following task is loaded from saved state (and not skipped because it was the previous state). Reset fpu_fpregs_owner_ctx after a failure during restore occurred, to ensure that the FPU state for the next task is always loaded. The problem was debugged-by Yu-cheng Yu . [ bp: Massage commit message. ] Fixes: 5f409e20b7945 ("x86/fpu: Defer FPU state load until return to userspace") Reported-by: Yu-cheng Yu Signed-off-by: Sebastian Andrzej Siewior Signed-off-by: Borislav Petkov Cc: Andy Lutomirski Cc: Dave Hansen Cc: Fenghua Yu Cc: "H. Peter Anvin" Cc: Ingo Molnar Cc: Jann Horn Cc: Peter Zijlstra Cc: "Ravi V. Shankar" Cc: Rik van Riel Cc: Thomas Gleixner Cc: Tony Luck Cc: x86-ml Link: https://lkml.kernel.org/r/20191220195906.plk6kpmsrikvbcfn@linutronix.de --- arch/x86/kernel/fpu/signal.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'arch') diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c index 0071b794ed19..400a05e1c1c5 100644 --- a/arch/x86/kernel/fpu/signal.c +++ b/arch/x86/kernel/fpu/signal.c @@ -352,6 +352,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size) fpregs_unlock(); return 0; } + fpregs_deactivate(fpu); fpregs_unlock(); } @@ -403,6 +404,8 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size) } if (!ret) fpregs_mark_activate(); + else + fpregs_deactivate(fpu); fpregs_unlock(); err_out: -- cgit