[patch] x86, avx: don't use avx instructions with "noxsave" boot param

[patch] x86, avx: don't use avx instructions with "noxsave" boot param

[Suresh Siddha]

Check the kernel has indeed enabled xsave before using AVX instructions.

Fixes the kernel boot panic with "noxsave" boot parameter.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---
 arch/x86/include/asm/xor_avx.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/xor_avx.h b/arch/x86/include/asm/xor_avx.h
index 2510d35..77987cd 100644
--- a/arch/x86/include/asm/xor_avx.h
+++ b/arch/x86/include/asm/xor_avx.h
@@ -197,12 +197,12 @@ static struct xor_block_template xor_block_avx = {
 
 #define AVX_XOR_SPEED \
 do { \
-	if (cpu_has_avx) \
+	if (cpu_has_avx && cpu_has_osxsave) \
 		xor_speed(&xor_block_avx); \
 } while (0)
 
 #define AVX_SELECT(FASTEST) \
-	(cpu_has_avx ? &xor_block_avx : FASTEST)
+	((cpu_has_avx && cpu_has_osxsave) ? &xor_block_avx : FASTEST)
 
 #else
 

Re: [patch] x86, avx: don't use avx instructions with "noxsave" boot param

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1128 bytes --]

On Mon, 30 Jul 2012 15:46:15 -0700 Suresh Siddha <suresh.b.siddha@intel.com>
wrote:

> Check the kernel has indeed enabled xsave before using AVX instructions.
> 
> Fixes the kernel boot panic with "noxsave" boot parameter.
> 
> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>

Looks consistent with other usage - thanks.
Applied.

Should it go in '-stable' kernels too?

NeilBrown

> ---
>  arch/x86/include/asm/xor_avx.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/xor_avx.h b/arch/x86/include/asm/xor_avx.h
> index 2510d35..77987cd 100644
> --- a/arch/x86/include/asm/xor_avx.h
> +++ b/arch/x86/include/asm/xor_avx.h
> @@ -197,12 +197,12 @@ static struct xor_block_template xor_block_avx = {
>  
>  #define AVX_XOR_SPEED \
>  do { \
> -	if (cpu_has_avx) \
> +	if (cpu_has_avx && cpu_has_osxsave) \
>  		xor_speed(&xor_block_avx); \
>  } while (0)
>  
>  #define AVX_SELECT(FASTEST) \
> -	(cpu_has_avx ? &xor_block_avx : FASTEST)
> +	((cpu_has_avx && cpu_has_osxsave) ? &xor_block_avx : FASTEST)
>  
>  #else
>  
> 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [patch] x86, avx: don't use avx instructions with "noxsave" boot param

[H. Peter Anvin]

I'm wondering if we shouldn't just kill the affected CPUID bits if osxsave is off...

NeilBrown <neilb@suse.de> wrote:

>On Mon, 30 Jul 2012 15:46:15 -0700 Suresh Siddha
><suresh.b.siddha@intel.com>
>wrote:
>
>> Check the kernel has indeed enabled xsave before using AVX
>instructions.
>> 
>> Fixes the kernel boot panic with "noxsave" boot parameter.
>> 
>> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
>
>Looks consistent with other usage - thanks.
>Applied.
>
>Should it go in '-stable' kernels too?
>
>NeilBrown
>
>> ---
>>  arch/x86/include/asm/xor_avx.h |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/xor_avx.h
>b/arch/x86/include/asm/xor_avx.h
>> index 2510d35..77987cd 100644
>> --- a/arch/x86/include/asm/xor_avx.h
>> +++ b/arch/x86/include/asm/xor_avx.h
>> @@ -197,12 +197,12 @@ static struct xor_block_template xor_block_avx
>= {
>>  
>>  #define AVX_XOR_SPEED \
>>  do { \
>> -	if (cpu_has_avx) \
>> +	if (cpu_has_avx && cpu_has_osxsave) \
>>  		xor_speed(&xor_block_avx); \
>>  } while (0)
>>  
>>  #define AVX_SELECT(FASTEST) \
>> -	(cpu_has_avx ? &xor_block_avx : FASTEST)
>> +	((cpu_has_avx && cpu_has_osxsave) ? &xor_block_avx : FASTEST)
>>  
>>  #else
>>  
>> 

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [patch] x86, avx: don't use avx instructions with "noxsave" boot param

[Suresh Siddha]

On Mon, 2012-07-30 at 21:33 -0700, H. Peter Anvin wrote:
> I'm wondering if we shouldn't just kill the affected CPUID bits if osxsave is off...

Yes, I considered clearing AVX bit (and any future xsave feature bits)
as part of the parsing "noxsave" parameter in x86_xsave_setup().

But thought checking for osxsave was more explicit and there are not
many places using avx code. Also, this follows the SDM guidelines and
sets a nice precedent for someone (app's) to follow in future.

I am ok either way!

thanks,
suresh

Re: [patch] x86, avx: don't use avx instructions with "noxsave" boot param

[H. Peter Anvin]

On 07/31/2012 09:27 AM, Suresh Siddha wrote:
> On Mon, 2012-07-30 at 21:33 -0700, H. Peter Anvin wrote:
>> I'm wondering if we shouldn't just kill the affected CPUID bits if osxsave is off...
>
> Yes, I considered clearing AVX bit (and any future xsave feature bits)
> as part of the parsing "noxsave" parameter in x86_xsave_setup().
>
> But thought checking for osxsave was more explicit and there are not
> many places using avx code. Also, this follows the SDM guidelines and
> sets a nice precedent for someone (app's) to follow in future.
>
> I am ok either way!
>

My main reason for thinking that it would be good to clear the bits is 
that it plays a lot nicer with alternatives and with static_cpu_has().

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [patch] x86, avx: don't use avx instructions with "noxsave" boot param

[Suresh Siddha]

On Tue, 2012-07-31 at 09:43 -0700, H. Peter Anvin wrote:
> My main reason for thinking that it would be good to clear the bits is 
> that it plays a lot nicer with alternatives and with static_cpu_has().

Fair enough. Neil, can you please drop the previous patch and Peter,
please queue this for v3.6. Thanks.

---
From: Suresh Siddha <suresh.b.siddha@intel.com>
Subject: x86, avx: don't use avx instructions with "noxsave" boot param

Clear AVX, AVX2 features along with clearing XSAVE feature bits,
as part of the parsing "noxsave" parameter.

Fixes the kernel boot panic with "noxsave" boot parameter.

We could have checked cpu_has_osxsave along with cpu_has_avx etc, but Peter
mentioned clearing the feature bits will be better for uses like
static_cpu_has() etc.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: stable@kernel.org	# v3.5
---
 arch/x86/kernel/cpu/common.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 46d8786..a5fbc3c 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -144,6 +144,8 @@ static int __init x86_xsave_setup(char *s)
 {
 	setup_clear_cpu_cap(X86_FEATURE_XSAVE);
 	setup_clear_cpu_cap(X86_FEATURE_XSAVEOPT);
+	setup_clear_cpu_cap(X86_FEATURE_AVX);
+	setup_clear_cpu_cap(X86_FEATURE_AVX2);
 	return 1;
 }
 __setup("noxsave", x86_xsave_setup);

Re: [patch] x86, avx: don't use avx instructions with "noxsave" boot param

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 453 bytes --]

On Tue, 31 Jul 2012 10:29:14 -0700 Suresh Siddha <suresh.b.siddha@intel.com>
wrote:

> On Tue, 2012-07-31 at 09:43 -0700, H. Peter Anvin wrote:
> > My main reason for thinking that it would be good to clear the bits is 
> > that it plays a lot nicer with alternatives and with static_cpu_has().
> 
> Fair enough. Neil, can you please drop the previous patch and Peter,
> please queue this for v3.6. Thanks.
>

Dropped.  Thanks,

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[tip:x86/urgent] x86, avx: don't use avx instructions with " noxsave" boot param

[tip-bot for Suresh Siddha]

Commit-ID:  c6fd893da927c6cefb2ece22402765379921a834
Gitweb:     http://git.kernel.org/tip/c6fd893da927c6cefb2ece22402765379921a834
Author:     Suresh Siddha <suresh.b.siddha@intel.com>
AuthorDate: Tue, 31 Jul 2012 10:29:14 -0700
Committer:  H. Peter Anvin <hpa@zytor.com>
CommitDate: Wed, 8 Aug 2012 13:41:42 -0700

x86, avx: don't use avx instructions with "noxsave" boot param

Clear AVX, AVX2 features along with clearing XSAVE feature bits,
as part of the parsing "noxsave" parameter.

Fixes the kernel boot panic with "noxsave" boot parameter.

We could have checked cpu_has_osxsave along with cpu_has_avx etc, but Peter
mentioned clearing the feature bits will be better for uses like
static_cpu_has() etc.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Link: http://lkml.kernel.org/r/1343755754.2041.2.camel@sbsiddha-desk.sc.intel.com
Cc: <stable@vger.kernel.org>	# v3.5
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/cpu/common.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 46d8786..a5fbc3c 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -144,6 +144,8 @@ static int __init x86_xsave_setup(char *s)
 {
 	setup_clear_cpu_cap(X86_FEATURE_XSAVE);
 	setup_clear_cpu_cap(X86_FEATURE_XSAVEOPT);
+	setup_clear_cpu_cap(X86_FEATURE_AVX);
+	setup_clear_cpu_cap(X86_FEATURE_AVX2);
 	return 1;
 }
 __setup("noxsave", x86_xsave_setup);