Re: [PATCH 23/27] x86/fpu: Defer FPU state load until return to userspace

Borislav Petkov
2019-04-12T14:36:26Z

On Wed, Apr 03, 2019 at 06:41:52PM +0200, Sebastian Andrzej Siewior wrote:
> @@ -226,10 +236,9 @@ static void fpu__initialize(struct fpu *fpu)
> {
> WARN_ON_FPU(fpu != &current->thread.fpu);
>
> + set_thread_flag(TIF_NEED_FPU_LOAD);
> fpstate_init(&fpu->state);
> trace_x86_fpu_init_state(fpu);
> -
> - trace_x86_fpu_activate_state(fpu);

That is called nowhere after this patch.

Shouldn't it be called below, before fpregs_activate() because
fpregs_activate() does trace_x86_fpu_regs_activated()?

> /*
> @@ -308,6 +317,8 @@ void fpu__drop(struct fpu *fpu)
> */
> static inline void copy_init_fpstate_to_fpregs(void)
> {
> + fpregs_lock();
> +
> if (use_xsave())
> copy_kernel_to_xregs(&init_fpstate.xsave, -1);
> else if (static_cpu_has(X86_FEATURE_FXSR))
> @@ -317,6 +328,9 @@ static inline void copy_init_fpstate_to_fpregs(void)
>
> if (boot_cpu_has(X86_FEATURE_OSPKE))
> copy_init_pkru_to_fpregs();
> +
> + fpregs_mark_activate();
> + fpregs_unlock();
> }
>
> /*
> @@ -339,6 +353,45 @@ void fpu__clear(struct fpu *fpu)
> copy_init_fpstate_to_fpregs();
> }
>
> +/*
> + * Load FPU context before returning to userspace.
> + */
> +void switch_fpu_return(void)
> +{
> + if (!static_cpu_has(X86_FEATURE_FPU))
> + return;
> +
> + __fpregs_load_activate();
> +}
> +EXPORT_SYMBOL_GPL(switch_fpu_return);
> +
> +#ifdef CONFIG_X86_DEBUG_FPU
> +/*
> + * If current FPU state according to its tracking (loaded FPU ctx on this CPU)
> + * is not valid then we must have TIF_NEED_FPU_LOAD set so the context is loaded on
> + * return to userland.
> + */
> +void fpregs_assert_state_consistent(void)
> +{
> + struct fpu *fpu = &current->thread.fpu;
> +
> + if (test_thread_flag(TIF_NEED_FPU_LOAD))
> + return;
> + WARN_ON_FPU(!fpregs_state_valid(fpu, smp_processor_id()));
> +}
> +EXPORT_SYMBOL_GPL(fpregs_assert_state_consistent);
> +#endif
> +
> +void fpregs_mark_activate(void)
> +{
> + struct fpu *fpu = &current->thread.fpu;
> +

<--- here?

> + fpregs_activate(fpu);
> + fpu->last_cpu = smp_processor_id();
> + clear_thread_flag(TIF_NEED_FPU_LOAD);
> +}
> +EXPORT_SYMBOL_GPL(fpregs_mark_activate);

--
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.