| Message ID | 20190109114744.10936-4-bigeasy@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6] x86: load FPU registers on return to userland | expand |
On Wed, Jan 09, 2019 at 12:47:25PM +0100, Sebastian Andrzej Siewior wrote: > The preempt_disable() section was introduced in commit <---- newline here. > a10b6a16cdad8 ("x86/fpu: Make the fpu state change in fpu__clear() scheduler-atomic") <---- newline here. > and it was said to be temporary. > > fpu__initialize() initializes the FPU struct to its "init" value and > then sets ->initialized to 1. The last part is the important one. > The content of the `state' does not matter because it gets set via > copy_init_fpstate_to_fpregs(). > A preemption here has little meaning because the register will always be s/register/registers/ > set to the same content after copy_init_fpstate_to_fpregs(). A softirq > with a kernel_fpu_begin() could also force to save FPU's register after ditto. > fpu__initialize() without changing the outcome here. > > Remove the preempt_disable() section in fpu__clear(), preemption here > does not hurt. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > arch/x86/kernel/fpu/core.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c > index 1d3ae7988f7f2..1940319268aef 100644 > --- a/arch/x86/kernel/fpu/core.c > +++ b/arch/x86/kernel/fpu/core.c > @@ -366,11 +366,9 @@ void fpu__clear(struct fpu *fpu) > * Make sure fpstate is cleared and initialized. > */ > if (static_cpu_has(X86_FEATURE_FPU)) { > - preempt_disable(); > fpu__initialize(fpu); > user_fpu_begin(); > copy_init_fpstate_to_fpregs(); > - preempt_enable(); > } > } > > -- With the above addressed: Reviewed-by: Borislav Petkov <bp@suse.de>
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index 1d3ae7988f7f2..1940319268aef 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -366,11 +366,9 @@ void fpu__clear(struct fpu *fpu) * Make sure fpstate is cleared and initialized. */ if (static_cpu_has(X86_FEATURE_FPU)) { - preempt_disable(); fpu__initialize(fpu); user_fpu_begin(); copy_init_fpstate_to_fpregs(); - preempt_enable(); } }
The preempt_disable() section was introduced in commit a10b6a16cdad8 ("x86/fpu: Make the fpu state change in fpu__clear() scheduler-atomic") and it was said to be temporary. fpu__initialize() initializes the FPU struct to its "init" value and then sets ->initialized to 1. The last part is the important one. The content of the `state' does not matter because it gets set via copy_init_fpstate_to_fpregs(). A preemption here has little meaning because the register will always be set to the same content after copy_init_fpstate_to_fpregs(). A softirq with a kernel_fpu_begin() could also force to save FPU's register after fpu__initialize() without changing the outcome here. Remove the preempt_disable() section in fpu__clear(), preemption here does not hurt. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- arch/x86/kernel/fpu/core.c | 2 -- 1 file changed, 2 deletions(-)