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

2016-03-09 Thread H. Peter Anvin
On 03/09/2016 04:46 AM, Ingo Molnar wrote:
> 
> * Yu-cheng Yu  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




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

2016-03-09 Thread Ingo Molnar

* Yu-cheng Yu  wrote:

> On Wed, Mar 09, 2016 at 01:46:26PM +0100, Ingo Molnar wrote:
> > 
> > * Yu-cheng Yu  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

2016-03-09 Thread Yu-cheng Yu
On Wed, Mar 09, 2016 at 01:46:26PM +0100, Ingo Molnar wrote:
> 
> * Yu-cheng Yu  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

2016-03-09 Thread Ingo Molnar

* Yu-cheng Yu  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


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

2016-01-22 Thread 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 
Reported_by: Leonid Shatz 
Cc: x...@kernel.org
Cc: H. Peter Anvin 
Cc: Thomas Gleixner 
Cc: Dave Hansen 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Sai Praneeth Prakhya 
Cc: Ravi V. Shankar 
Cc: Leonid Shatz 
Cc: Fenghua Yu 
---
 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