Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [PATCH v10 13/28] x86/fpu/xstate: Use feature disable (XFD) to protect dynamic user state | Date | Fri, 01 Oct 2021 22:20:16 +0200 |
| |
Chang,
On Fri, Oct 01 2021 at 17:02, Thomas Gleixner wrote: > On Wed, Aug 25 2021 at 08:53, Chang S. Bae wrote: >> +/** >> + * xfd_switch - Switches the MSR IA32_XFD context if needed. >> + * @prev: The previous task's struct fpu pointer >> + * @next: The next task's struct fpu pointer >> + */ >> +static inline void xfd_switch(struct fpu *prev, struct fpu *next) >> +{ >> + u64 prev_xfd_mask, next_xfd_mask; >> + >> + if (!cpu_feature_enabled(X86_FEATURE_XFD) || !xfeatures_mask_user_dynamic) >> + return; > > This is context switch, so this wants to be a static key which is turned > on during init when the CPU supports XFD and user dynamic features are > available. > >> + >> + prev_xfd_mask = prev->state_mask & xfeatures_mask_user_dynamic; >> + next_xfd_mask = next->state_mask & xfeatures_mask_user_dynamic; >> + >> + if (unlikely(prev_xfd_mask != next_xfd_mask)) >> + wrmsrl_safe(MSR_IA32_XFD, xfeatures_mask_user_dynamic ^ next_xfd_mask); >> +} >> + >> /* >> * Delay loading of the complete FPU state until the return to userland. >> * PKRU is handled separately. >> */ >> -static inline void switch_fpu_finish(struct fpu *new_fpu) >> +static inline void switch_fpu_finish(struct fpu *old_fpu, struct fpu *new_fpu) >> { >> - if (cpu_feature_enabled(X86_FEATURE_FPU)) >> + if (cpu_feature_enabled(X86_FEATURE_FPU)) { >> set_thread_flag(TIF_NEED_FPU_LOAD); >> + xfd_switch(old_fpu, new_fpu); > > Why has this to be done on context switch? Zero explanation provided. > > Why can't this be done in exit_to_user() where the FPU state restore is > handled?
DEFINE_PER_CPU(xfd_state);
update_xfd(fpu) { if (__this_cpu_read(xfd_state) != fpu->xfd_state) { wrmsrl(XFD, fpu->xfd_state); __this_cpu_write(xfd_state, fpu->xfd_state); } }
fpregs_restore_userregs() { if (!fpregs_state_valid(fpu, cpu)) { if (static_branch_unlikely(xfd_switching_enabled)) update_xfd(fpu); ... } }
Hmm?
Thanks,
tglx
| |