Messages in this thread | | | Subject | Re: [PATCH v3 9/9] x86/xsaves: Re-enable XSAVES | From | "H. Peter Anvin" <> | Date | Tue, 01 Mar 2016 16:53:53 -0800 |
| |
On March 1, 2016 4:45:41 PM PST, Dave Hansen <dave.hansen@linux.intel.com> wrote: >On 03/01/2016 04:34 PM, Yu-cheng Yu wrote: >> On Tue, Mar 01, 2016 at 03:56:12PM -0800, Dave Hansen wrote: >>> On 02/29/2016 09:42 AM, Yu-cheng Yu wrote: >>>> - setup_clear_cpu_cap(X86_FEATURE_XSAVES); >>>> + if (!config_enabled(CONFIG_X86_64)) >>>> + setup_clear_cpu_cap(X86_FEATURE_XSAVES); >>>> } >>> >>> I think we need a much better explanation of this for posterity. >Why >>> are we not supporting this now, and what would someone have to do in >the >>> future in order to enable it? >>> >> If anyone is using this newer feature, then that user is most likely >using >> a 64-bit capable processor and a 64-bit kernel. The intention here is >to >> take out the complexity and any potential of error. If the user >removes >> the restriction and builds a private kernel, it should work but we >have >> not checked all possible combinations. I will put these in the >comments. > >A user can go download a 32-bit version of Ubuntu or Debian and install >it on a 64-bit processor today. It's a very easy mistake to make when >downloading the install CD. > >In any case, I don't have a _problem_ with leaving i386 in the dust >here. I just want us to be very explicit about what we are doing. > >>>> + /* >>>> + * Make it clear that XSAVES supervisor states are not yet >>>> + * implemented should anyone expect it to work by changing >>>> + * bits in XFEATURE_MASK_* macros and XCR0. >>>> + */ >>>> + WARN_ONCE((xfeatures_mask & XFEATURE_MASK_SUPERVISOR), >>>> + "x86/fpu: XSAVES supervisor states are not yet implemented.\n"); >>>> + >>>> cr4_set_bits(X86_CR4_OSXSAVE); >>>> xsetbv(XCR_XFEATURE_ENABLED_MASK, xfeatures_mask); >>>> } >>> >>> Let's also do a: >>> >>> xfeatures_mask &= ~XFEATURE_MASK_SUPERVISOR; >>> >>> Otherwise, we have a broken system at the moment. >>> >> Currently, if anyone sets any supervisor state in xfeatures_mask, the >> kernel prints out the warning then goes into a protection fault. >> That is a very strong indication to the user. Do we want to mute it? > >By "goes into a protection fault", do you mean that it doesn't boot? > >I'd just rather we put the kernel in a known-safe configuration >(masking >supervisor state out of xfeatures_mask) rather than rely on the general >protection fault continuing to be generated by whatever is generating >it.
Differences between i386 and x86-64 generally add problems, so unless this requires significant 32-bit-specific code we should not exclude i386 just because. -- Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.
| |