Messages in this thread | | | Subject | Re: [PATCH v3 05/10] x86/fpu/xstate: Define new functions for clearing fpregs and xstates | From | Yu-cheng Yu <> | Date | Wed, 29 Apr 2020 10:02:46 -0700 |
| |
On Wed, 2020-04-29 at 18:39 +0200, Borislav Petkov wrote: > On Wed, Apr 29, 2020 at 09:06:44AM -0700, Yu-cheng Yu wrote: > > From: Fenghua Yu <fenghua.yu@intel.com> > > > > Currently, fpu__clear() clears all fpregs and xstates. Once XSAVES > > supervisor states are introduced, supervisor settings (e.g. CET xstates) > > must remain active for signals; It is necessary to have separate functions: > > > > - Create fpu__clear_user_states(): clear only user settings for signals; > > - Create fpu__clear_all(): clear both user and supervisor settings in > > flush_thread(). > > > > Also modify copy_init_fpstate_to_fpregs() to take a mask from above two > > functions. > > > > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> > > Co-developed-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > > Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com> > > Reviewed-by: Tony Luck <tony.luck@intel.com> > > > > v3: > > - Put common code into a static function fpu__clear(), with a parameter > > user_only. > > > > v2: > > - Fixed an issue where fpu__clear_user_states() drops supervisor xstates. > > - Revise commit log. > > Try applying that patch from this mail yourself and see whether the > patch changelog will remain in the commit message or it will get > discarded.
My mistake! I will fix it.
> > > @@ -318,18 +313,40 @@ static inline void copy_init_fpstate_to_fpregs(void) > > * Called by sys_execve(), by the signal handler code and by various > > * error paths. > > */ > > -void fpu__clear(struct fpu *fpu) > > +static void fpu__clear(struct fpu *fpu, int user_only) > > { > > - WARN_ON_FPU(fpu != ¤t->thread.fpu); /* Almost certainly an anomaly */ > > + WARN_ON_FPU(fpu != ¤t->thread.fpu); > > Why did you remove the side comment? > > Is it wrong? > > Why do you do such arbitrary changes which are not needed instead of > concentrating on only the changes the patch should do?
It has been some time since Thomas commented on this tail comment. https://lore.kernel.org/lkml/alpine.DEB.2.21.1908161703010.1923@nanos.tec.linutronix.de/
I think why not fixing it while at it.
Yu-cheng
| |