Re: [PATCH v2 8/8] x86/fpu/xstate: Restore supervisor xstates for __fpu__restore_sig()

[Yu-cheng Yu]

On Fri, 2020-03-06 at 21:50 +0100, Borislav Petkov wrote:
> On Wed, Mar 04, 2020 at 10:18:46AM -0800, Yu-cheng Yu wrote:
> > There is another way to keep this patch...
> > 
> > if (xfeatures_mask_supervisor()) {
> > 	fpu->state.xsave.xfeatures &= xfeatures_mask_supervisor();
> 
> Is the subsequent XSAVE in copy_user_to_fpregs_zeroing() going to
> restore the user bits in XSTATE_BV you just cleared?
> 
> Sorry, it looks like it would but the SDM text is abysmal.

I checked and this won't work.



Earlier you wrote:

  53973 / (3*60 + 35) =~ 251 XSAVES invocations per second!

I would argue that the kernel does much more than that for context
switches.

These are from:
  perf record -a make -j32 bzImage

# Samples: 11M of event 'cycles'
# Event count (approx.): 7610600069602
#
# Overhead  Symbol
     2.19%  [.] ht_lookup_with_hash
     1.74%  [.] _int_malloc
     1.46%  [.] _cpp_lex_token
     1.46%  [.] ggc_internal_alloc
     1.10%  [.] cpp_get_token_with_location
     1.10%  [.] malloc
     1.05%  [.] _int_free
     0.71%  [.] elf_read
     0.70%  [.] ggc_internal_cleared_alloc
     0.69%  [.] htab_find_slot
     0.69%  [.] c_lex_with_flags
     0.61%  [.] get_combined_adhoc_loc
     0.57%  [.] linemap_position_for_column
[...]
     0.00%  [.] 0x0000000000bad020
     0.00%  [.] 0x0000000000b4952b
     0.00%  [k] __fpu__restore_sig

Here, __fpu__restore_sig() actually takes very little percentage.
Consider this and later maintenance, I think copy_xregs_to_kernel() is at
least not worse than saving each state separately.

Yu-cheng