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

Yu-cheng Yu
2020-03-10T20:36:21Z

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