x86: make clflush a required feature on x86_64
PATCH 1/1 x86: make clflush a required feature on x86_64
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.
--- include/asm-x86/cpufeature_64.h | 3 +++ include/asm-x86/required-features.h | 4 +++- 2 files changed, 6 insertions(+), 1 deletions(-)
include/asm-x86/cpufeature_64.h +3 -0
@@ -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
include/asm-x86/required-features.h +3 -1
@@ -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)
Signature · 1 line
1.5.3.7
Quoted 17 lines · Kyle McMartin
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 <[email protected]>--- include/asm-x86/cpufeature_64.h | 3 +++ include/asm-x86/required-features.h | 4 +++- 2 files changed, 6 insertions(+), 1 deletions(-)include/asm-x86/cpufeature_64.h +0
-0
include/asm-x86/cpufeature_64.h line 25
Quoted 8 lines
include/asm-x86/cpufeature_64.h +3
-0@@ -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
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
PATCH 1/1 x86_64: remove redundant cpu_has_ definitions
Quoted 3 lines · H. Peter Anvin
Unnecessary. These overrides are only needed for the anticases (known to be zero) or for some special hacks.
Cool, guess that means a bunch of them can go...
Quoted 3 lines
Stuff that have proper CPUID bits get these defined as constants via the
REQUIRED_MASKmacros.
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.
include/asm-x86/cpufeature.h +0 -15
@@ -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 */
Ingo Molnar
AppliedRe: [PATCH] x86_64: remove redundant cpu_has_ definitions
Quoted 11 lines · Kyle McMartin
include/asm-x86/cpufeature.h +0
-5@@ -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
include/asm-x86/cpufeature.h line 212
Quoted 10 lines
include/asm-x86/cpufeature.h +0
-10- -#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 -
thanks, applied.
Ingo
Quoted 5 lines · Kyle McMartin
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.
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
Quoted 3 lines · Andi Kleen
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.
Yeah, true. Please ignore the patch folks.
cheers, Kyle
Quoted 8 lines · Kyle McMartin
Quoted 3 lines · Andi Kleen
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.
Yeah, true. Please ignore the patch folks.
cheers, Kyle
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
Quoted 1 line
Simulators can be fixed,
They could, but why? I don't know of a good reason to require CLFLUSH.
Quoted 5 lines
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.
I don't know of any specific ones that lack CLFLUSH, although Bochs definitely had similar problems in the past.
-Andi
Quoted 3 lines · Andi Kleen
Quoted 1 line
Simulators can be fixed,
They could, but why? I don't know of a good reason to require CLFLUSH.
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 switch. It either goes in the required masks or it doesn't.
Quoted 4 lines
Quoted 1 line
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.
I don't know of any specific ones that lack CLFLUSH, although Bochs definitely had similar problems in the past.
OK, so we're talking about outdated versions of Bochs, then?
-hpa
Quoted 10 lines · H. Peter Anvin
Quoted 3 lines · Andi Kleen
Quoted 1 line
Simulators can be fixed,
They could, but why? I don't know of a good reason to require CLFLUSH.
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.
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
Quoted 17 lines · H. Peter Anvin
Quoted 11 lines · Kyle McMartin
Quoted 5 lines · Andi Kleen
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.Yeah, true. Please ignore the patch folks.
cheers, Kyle
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.
Quoted 6 lines
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.
simics? It was very useful long time ago...
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html