On Wed, Sep 25, 2019 at 08:10:20AM -0700, Yu-cheng Yu wrote: > Enable XSAVES supervisor states by setting MSR_IA32_XSS bits according to > CPUID enumeration results. Also revise comments at various places. > > Signed-off-by: Yu-cheng Yu <[email protected]> > Signed-off-by: Fenghua Yu <[email protected]>
Same issue as before. > Reviewed-by: Dave Hansen <[email protected]> > Reviewed-by: Tony Luck <[email protected]> > --- > arch/x86/kernel/fpu/xstate.c | 30 ++++++++++++++++++++---------- > 1 file changed, 20 insertions(+), 10 deletions(-) > > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c > index b7f2808dd3f4..a183c319d808 100644 > --- a/arch/x86/kernel/fpu/xstate.c > +++ b/arch/x86/kernel/fpu/xstate.c > @@ -233,13 +233,14 @@ void fpu__init_cpu_xstate(void) > * states can be set here. > */ > xsetbv(XCR_XFEATURE_ENABLED_MASK, xfeatures_mask_user()); > + > + /* > + * MSR_IA32_XSS sets supervisor states managed by XSAVES. > + */ > + if (boot_cpu_has(X86_FEATURE_XSAVES)) > + wrmsrl(MSR_IA32_XSS, xfeatures_mask_supervisor()); > } > > -/* > - * Note that in the future we will likely need a pair of > - * functions here: one for user xstates and the other for > - * system xstates. For now, they are the same. > - */ > static int xfeature_enabled(enum xfeature xfeature) > { > return !!(xfeatures_mask_all & (1UL << xfeature)); > @@ -623,9 +624,6 @@ static void do_extra_xstate_size_checks(void) > * the size of the *user* states. If we use it to size a buffer > * that we use 'XSAVES' on, we could potentially overflow the > * buffer because 'XSAVES' saves system states too. > - * > - * Note that we do not currently set any bits on IA32_XSS so > - * 'XCR0 | IA32_XSS == XCR0' for now. > */ > static unsigned int __init get_xsaves_size(void) > { > @@ -748,7 +746,11 @@ void __init fpu__init_system_xstate(void) > cpuid_count(XSTATE_CPUID, 0, &eax, &ebx, &ecx, &edx); > xfeatures_mask_all = eax + ((u64)edx << 32); > > - /* Place supervisor features in xfeatures_mask_all here */ > + /* > + * Find supervisor xstates supported by the processor. > + */ > + cpuid_count(XSTATE_CPUID, 1, &eax, &ebx, &ecx, &edx); > + xfeatures_mask_all |= ecx + ((u64)edx << 32); > > if ((xfeatures_mask_user() & XFEATURE_MASK_FPSSE) != > XFEATURE_MASK_FPSSE) { > /* > @@ -788,9 +790,10 @@ void __init fpu__init_system_xstate(void) > setup_xstate_comp(); > print_xstate_offset_size(); > > - pr_info("x86/fpu: Enabled xstate features 0x%llx: user 0x%llx, context > size is %d bytes, using '%s' format.\n", > + pr_info("x86/fpu: Enabled xstate features 0x%llx: user 0x%llx and > supervisor %llx, context size is %d bytes, using '%s' format.\n", > xfeatures_mask_all, > xfeatures_mask_user(), > + xfeatures_mask_supervisor(), So if you're dumping both user and supervisor, then you don't need to dump _all anymore. Do it this way: pr_info("x86/fpu: Enabled xstate features: (U: 0x%llx, S: 0x%llx), context size: %d bytes, using '%s' format.\n", xfeatures_mask_user(), xfeatures_mask_supervisor(), -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
