Home » Mailing list archives » linux-kernel » 2008 » January » 18

Re: [PATCH] x86: make clflush a required feature on x86_64

Previous thread: RE: Bitops source problem by Pravin Nanaware on Friday, January 18, 2008 - 12:40 am. (1 message)

Next thread: [PATCH] x86: merge asm-x86/alternative.h by Kyle McMartin on Friday, January 18, 2008 - 1:03 am. (3 messages)
[view original message]
From: Kyle McMartin <kyle@...>
To: <hpa@...>
Cc: <tglx@...>, <mingo@...>, <linux-kernel@...>
Subject: [PATCH] x86: make clflush a required feature on x86_64
Date: Friday, January 18, 2008 - 12:59 am

Hopefully nobody will be stupid enough to implement a cpu without

it. Frankly, it seems safe enough given we already require SSE2.


This means the compiler can optimise away "if (!cpu_has_clflush)"

blocks.


Signed-off-by: Kyle McMartin <kyle@mcmartin.ca>

---

 include/asm-x86/cpufeature_64.h     |    3 +++

 include/asm-x86/required-features.h |    4 +++-

 2 files changed, 6 insertions(+), 1 deletions(-)


diff --git a/include/asm-x86/cpufeature_64.h b/include/asm-x86/cpufeature_64.h

index e18496b..ef11d27 100644

--- a/include/asm-x86/cpufeature_64.h

+++ b/include/asm-x86/cpufeature_64.h

@@ -18,6 +18,9 @@

 #undef  cpu_has_mp

 #define cpu_has_mp             1 /* XXX */


+#undef	cpu_has_clflush

+#define	cpu_has_clflush        1

+

 #undef  cpu_has_k6_mtrr

 #define cpu_has_k6_mtrr        0


diff --git a/include/asm-x86/required-features.h b/include/asm-x86/required-features.h

index 7400d3a..cc27664 100644

--- a/include/asm-x86/required-features.h

+++ b/include/asm-x86/required-features.h

@@ -45,6 +45,7 @@

 #define NEED_XMM	(1<<(X86_FEATURE_XMM & 31))

 #define NEED_XMM2	(1<<(X86_FEATURE_XMM2 & 31))

 #define NEED_LM		(1<<(X86_FEATURE_LM & 31))

+#define NEED_CLFLSH	(1<<(X86_FEATURE_CLFLSH & 31))

 #else

 #define NEED_PSE	0

 #define NEED_MSR	0

@@ -53,11 +54,12 @@

 #define NEED_XMM	0

 #define NEED_XMM2	0

 #define NEED_LM		0

+#define NEED_CLFLSH	0

 #endif


 #define REQUIRED_MASK0	(NEED_FPU|NEED_PSE|NEED_MSR|NEED_PAE|\

 			 NEED_CX8|NEED_PGE|NEED_FXSR|NEED_CMOV|\

-			 NEED_XMM|NEED_XMM2)

+			 NEED_XMM|NEED_XMM2|NEED_CLFLSH)

 #define SSE_MASK	(NEED_XMM|NEED_XMM2)


 #define REQUIRED_MASK1	(NEED_LM|NEED_3DNOW)

--

1.5.3.7


--
[view original message]
From: Andi Kleen <andi@...>
To: Kyle McMartin <kyle@...>
Cc: <hpa@...>, <tglx@...>, <mingo@...>, <linux-kernel@...>
Subject: Re: [PATCH] x86: make clflush a required feature on x86_64
Date: Friday, January 18, 2008 - 1:53 am

The original required CPUID bit set for x86-64 came out of very

detailed discussions and thinking from AMD. I think they had good

reasons to chose the current set. I would only recommend to depart

from it for very good reasons and frankly you haven't given any.

You're talking about removing two instructions.


One problem that we had in the past is that some simulators

only implement the absolutely minimum feature set and you

might have well broken one of these with this.


-Andi

--
[view original message]
From: Kyle McMartin <kyle@...>
To: Andi Kleen <andi@...>
Cc: Kyle McMartin <kyle@...>, <hpa@...>, <tglx@...>, <mingo@...>, <linux-kernel@...>
Subject: Re: [PATCH] x86: make clflush a required feature on x86_64
Date: Friday, January 18, 2008 - 2:01 am

Yeah, true. Please ignore the patch folks.


cheers,	Kyle

--
[view original message]
From: H. Peter Anvin <hpa@...>
To: Kyle McMartin <kyle@...>
Cc: Andi Kleen <andi@...>, <tglx@...>, <mingo@...>, <linux-kernel@...>
Subject: Re: [PATCH] x86: make clflush a required feature on x86_64
Date: Friday, January 18, 2008 - 2:27 am

Simulators can be fixed, even if it takes time; for something like

clflush this is easier since clflush can be implemented as a simple noop

for most of them.


I just verified that Bochs 2.3.0 lacks this CPUID bit whereas the

current version, 2.3.6, enables CLFLUSH iff SSE2 is enabled.  Qemu 0.9.0

has CLFLUSH.  Andi, do you happen to know of any specific simulators

which are problematic?  I would assume any recent version of SimNow is

up to date.


	-hpa

--
[view original message]
From: Pavel Machek <pavel@...>
To: H. Peter Anvin <hpa@...>
Cc: Kyle McMartin <kyle@...>, Andi Kleen <andi@...>, <tglx@...>, <mingo@...>, <linux-kernel@...>
Subject: Re: [PATCH] x86: make clflush a required feature on x86_64
Date: Friday, January 11, 2008 - 2:16 pm

simics? It was very useful long time ago...

							Pavel

--

(english) http://www.livejournal.com/~pavelmachek

(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

--
[view original message]
From: Andi Kleen <andi@...>
To: H. Peter Anvin <hpa@...>
Cc: Kyle McMartin <kyle@...>, Andi Kleen <andi@...>, <tglx@...>, <mingo@...>, <linux-kernel@...>
Subject: Re: [PATCH] x86: make clflush a required feature on x86_64
Date: Friday, January 18, 2008 - 2:54 am

> Simulators can be fixed, 


I don't know of any specific ones that lack CLFLUSH, although Bochs

definitely had similar problems in the past.


-Andi

--
[view original message]
From: H. Peter Anvin <hpa@...>
To: Andi Kleen <andi@...>
Cc: Kyle McMartin <kyle@...>, <tglx@...>, <mingo@...>, <linux-kernel@...>
Subject: Re: [PATCH] x86: make clflush a required feature on x86_64
Date: Friday, January 18, 2008 - 9:56 am

Well, simulators are generally expected to follow the architecture, not

vice versa.  I would tend to agree with the coupling that recent

versions of Bochs appeared to have made here -- I think we're unlikely

to see any processors with sse2 sans clflush, so keeping code branches

in which will never be executed seems like a bad idea in the long term.

  I'm much more worried about the possibility of embedded 64-bit CPUs

who try to skimp on SSE2 than CLFLUSH.


Now, that being said, this being encapsulated in the required set (and

unified, which means the branches will be executed on 32-bit hardware)

it seems the impact of leaving them in is small for now.  We can

re-evaluate that as appropriate.  Either way, we should *not* have

#ifdef CONFIG_X86_64 around usage sites, circumventing the master 


OK, so we're talking about outdated versions of Bochs, then?


	-hpa


--
[view original message]
From: Andi Kleen <andi@...>
To: H. Peter Anvin <hpa@...>
Cc: Andi Kleen <andi@...>, Kyle McMartin <kyle@...>, <tglx@...>, <mingo@...>, <linux-kernel@...>
Subject: Re: [PATCH] x86: make clflush a required feature on x86_64
Date: Friday, January 18, 2008 - 10:18 am

Here's another argument: Ingo just asked me to add a noclflush option

to the code. Guess what check that option will need? 


Besides compared to the cost of a flushing clflush the branches are absolutely

in the noise.


-Andi

--
[view original message]
From: H. Peter Anvin <hpa@...>
To: Kyle McMartin <kyle@...>
Cc: <tglx@...>, <mingo@...>, <linux-kernel@...>
Subject: Re: [PATCH] x86: make clflush a required feature on x86_64
Date: Friday, January 18, 2008 - 1:04 am

Unnecessary.  These overrides are only needed for the anticases (known

to be zero) or for some special hacks.


Stuff that have proper CPUID bits get these defined as constants via the

  REQUIRED_MASK macros.


	-hpa

--
[view original message]
From: Kyle McMartin <kyle@...>
To: H. Peter Anvin <hpa@...>
Cc: <tglx@...>, <mingo@...>, <linux-kernel@...>
Subject: [PATCH] x86_64: remove redundant cpu_has_ definitions
Date: Friday, January 18, 2008 - 1:25 am

PSE, PGE, XMM, XMM2, and FXSR are defined as required features, and

will be optimized to a constant at compile time. Remove their redundant

definitions.


Signed-off-by: Kyle McMartin <kyle@mcmartin.ca>


--- a/include/asm-x86/cpufeature.h

+++ b/include/asm-x86/cpufeature.h

@@ -195,21 +195,6 @@

 #undef  cpu_has_centaur_mcr

 #define cpu_has_centaur_mcr	0


-#undef  cpu_has_pse

-#define cpu_has_pse		1

-

-#undef  cpu_has_pge

-#define cpu_has_pge		1

-

-#undef  cpu_has_xmm

-#define cpu_has_xmm		1

-

-#undef  cpu_has_xmm2

-#define cpu_has_xmm2		1

-

-#undef  cpu_has_fxsr

-#define cpu_has_fxsr		1

-

 #endif /* CONFIG_X86_64 */


 #endif /* _ASM_X86_CPUFEATURE_H */

--
[view original message]
From: Ingo Molnar <mingo@...>
To: Kyle McMartin <kyle@...>
Cc: H. Peter Anvin <hpa@...>, <tglx@...>, <mingo@...>, <linux-kernel@...>
Subject: Re: [PATCH] x86_64: remove redundant cpu_has_ definitions
Date: Friday, January 18, 2008 - 4:07 am

thanks, applied.


	Ingo

--
Previous thread: RE: Bitops source problem by Pravin Nanaware on Friday, January 18, 2008 - 12:40 am. (1 message)

Next thread: [PATCH] x86: merge asm-x86/alternative.h by Kyle McMartin on Friday, January 18, 2008 - 1:03 am. (3 messages)