Re: [PATCH v3 05/10] x86/fpu/xstate: Define new functions for clearing fpregs and xstates

[Borislav Petkov]

On Wed, Apr 29, 2020 at 09:06:44AM -0700, Yu-cheng Yu wrote:
> From: Fenghua Yu <fenghua.yu@xxxxxxxxx>
> 
> 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@xxxxxxxxx>
> Co-developed-by: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx>
> Reviewed-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx>
> 
> 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.

> @@ -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 != &current->thread.fpu); /* Almost certainly an anomaly */
> +	WARN_ON_FPU(fpu != &current->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?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette