[PATCH] x86/fpu: Revert earlier patch of Disable AVX when eagerfpu is off

[PATCH] x86/fpu: Revert earlier patch of Disable AVX when eagerfpu is off

[Yu-cheng Yu]

AVX was mistakenly believed to be dependent on eagerfpu switch.
This turns out to be false. The earlier patch should be reverted.

Original patch:
    http://git.kernel.org/tip/394db20ca240741a08d472173db13d6f6a6e5a28

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Reported_by: Leonid Shatz <leonid.shatz@ravellosystems.com>
Cc: x86@kernel.org
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Ravi V. Shankar <ravi.v.shankar@intel.com>
Cc: Leonid Shatz <leonid.shatz@ravellosystems.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/fpu/xstate.h | 9 ++++-----
 arch/x86/kernel/fpu/init.c        | 6 ------
 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index af30fde..f23cd8c 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -20,16 +20,15 @@
 
 /* Supported features which support lazy state saving */
 #define XFEATURE_MASK_LAZY	(XFEATURE_MASK_FP | \
-				 XFEATURE_MASK_SSE)
-
-/* Supported features which require eager state saving */
-#define XFEATURE_MASK_EAGER	(XFEATURE_MASK_BNDREGS | \
-				 XFEATURE_MASK_BNDCSR | \
+				 XFEATURE_MASK_SSE | \
 				 XFEATURE_MASK_YMM | \
 				 XFEATURE_MASK_OPMASK | \
 				 XFEATURE_MASK_ZMM_Hi256 | \
 				 XFEATURE_MASK_Hi16_ZMM)
 
+/* Supported features which require eager state saving */
+#define XFEATURE_MASK_EAGER	(XFEATURE_MASK_BNDREGS | XFEATURE_MASK_BNDCSR)
+
 /* All currently supported features */
 #define XCNTXT_MASK	(XFEATURE_MASK_LAZY | XFEATURE_MASK_EAGER)
 
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index 6d9f0a7..f0ab368 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -300,12 +300,6 @@ u64 __init fpu__get_supported_xfeatures_mask(void)
 static void __init fpu__clear_eager_fpu_features(void)
 {
 	setup_clear_cpu_cap(X86_FEATURE_MPX);
-	setup_clear_cpu_cap(X86_FEATURE_AVX);
-	setup_clear_cpu_cap(X86_FEATURE_AVX2);
-	setup_clear_cpu_cap(X86_FEATURE_AVX512F);
-	setup_clear_cpu_cap(X86_FEATURE_AVX512PF);
-	setup_clear_cpu_cap(X86_FEATURE_AVX512ER);
-	setup_clear_cpu_cap(X86_FEATURE_AVX512CD);
 }
 
 /*
-- 
1.9.1

Re: [PATCH] x86/fpu: Revert earlier patch of Disable AVX when eagerfpu is off

[Ingo Molnar]

* Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:

> AVX was mistakenly believed to be dependent on eagerfpu switch.
> This turns out to be false. The earlier patch should be reverted.
> 
> Original patch:
>     http://git.kernel.org/tip/394db20ca240741a08d472173db13d6f6a6e5a28

So the original patch had a whole host of explanations of why that change is 
correct. This revert should explain where that argumentation was wrong.

Also note that eagerfpu=off is on the way out, on v4.6 we'll switch all CPUs to 
eagerfpu:

  58122bf1d856 x86/fpu: Default eagerfpu=on on all CPUs

and in the not so distant future, if everything goes fine with the eager mode, I'd 
like to drop the lazy FPU context switching code altogether - which will simplify 
a lot of code. At that point the 'eagerfpu' boot option will go away as well.

Thanks,

	Ingo

Re: [PATCH] x86/fpu: Revert earlier patch of Disable AVX when eagerfpu is off

[Yu-cheng Yu]

On Wed, Mar 09, 2016 at 01:46:26PM +0100, Ingo Molnar wrote:
> 
> * Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> 
> > AVX was mistakenly believed to be dependent on eagerfpu switch.
> > This turns out to be false. The earlier patch should be reverted.
> > 
> > Original patch:
> >     http://git.kernel.org/tip/394db20ca240741a08d472173db13d6f6a6e5a28
> 
> So the original patch had a whole host of explanations of why that change is 
> correct. This revert should explain where that argumentation was wrong.

I will send out another version that includes more details.

Thanks,
Yu-cheng

Re: [PATCH] x86/fpu: Revert earlier patch of Disable AVX when eagerfpu is off

[Ingo Molnar]

* Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:

> On Wed, Mar 09, 2016 at 01:46:26PM +0100, Ingo Molnar wrote:
> > 
> > * Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > 
> > > AVX was mistakenly believed to be dependent on eagerfpu switch.
> > > This turns out to be false. The earlier patch should be reverted.
> > > 
> > > Original patch:
> > >     http://git.kernel.org/tip/394db20ca240741a08d472173db13d6f6a6e5a28
> > 
> > So the original patch had a whole host of explanations of why that change is 
> > correct. This revert should explain where that argumentation was wrong.
> 
> I will send out another version that includes more details.

Great, thanks!

	Ingo

Re: [PATCH] x86/fpu: Revert earlier patch of Disable AVX when eagerfpu is off

[H. Peter Anvin]

On 03/09/2016 04:46 AM, Ingo Molnar wrote:
> 
> * Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> 
>> AVX was mistakenly believed to be dependent on eagerfpu switch.
>> This turns out to be false. The earlier patch should be reverted.
>>
>> Original patch:
>>     http://git.kernel.org/tip/394db20ca240741a08d472173db13d6f6a6e5a28
> 
> So the original patch had a whole host of explanations of why that change is 
> correct. This revert should explain where that argumentation was wrong.
> 
> Also note that eagerfpu=off is on the way out, on v4.6 we'll switch all CPUs to 
> eagerfpu:
> 
>   58122bf1d856 x86/fpu: Default eagerfpu=on on all CPUs
> 
> and in the not so distant future, if everything goes fine with the eager mode, I'd 
> like to drop the lazy FPU context switching code altogether - which will simplify 
> a lot of code. At that point the 'eagerfpu' boot option will go away as well.
> 

Last I heard the only use case which was seeing trouble with eagerfpu
was KVM during service of I/O events.  Might be worth checking into...

	-hpa