I believe the REQUIRED_MASK aproach was taken so that it was
easier to consult in assembly (arch/x86/kernel/verify_cpu.S).
DISABLED_MASK does not have the same restriction, but I
implemented it the same way for consistency.
--
We have a REQUIRED_MASK... which does two things:
1. Keeps a list of cpuid bits to check in very early boot and
refuse to boot if those are not present.
2. Consulted during cpu_has() checks, which allows us to
optimize out things at compile-time. In other words, if we
*KNOW* we will not boot with the feature off, then we can
safely assume that it will be present forever.
But, we don't have a similar mechanism for CPU features which
may be present but that we know we will not use. We simply
use our existing mechanisms to repeatedly check the status of
the bit at runtime (well, the alternatives patching helps here
but it does not provide compile-time optimization).
Adding a feature to disabled-features.h allows the bit to be
checked via cpu_has(), and all of the benefits of an #ifdef.
Before, we would have done this in a header:
#ifdef CONFIG_X86_INTEL_MPX
#define cpu_has_mpx cpu_has(X86_FEATURE_MPX
#else
#define cpu_has_mpx 0
#endif
and this in the code:
if (cpu_has_mpx)
do_some_mpx_thing();
Now, just add your feature to DISABLED_MASK and you can do this
everywhere, and get the same benefits you would have from
#ifdefs:
if (cpu_has(X86_FEATURE_MPX))
do_some_mpx_thing();
---
b/arch/x86/boot/mkcpustr.c | 1
b/arch/x86/include/asm/cpufeature.h | 18 +++++++++++++++
b/arch/x86/include/asm/disabled-features.h | 33 +++++++++++++++++++++++++++++
3 files changed, 52 insertions(+)
diff -puN arch/x86/boot/mkcpustr.c~x86-disabled_features arch/x86/boot/mkcpustr.c
--- a/arch/x86/boot/mkcpustr.c~x86-disabled_features 2014-06-20 09:12:25.511247513 -0700
+++ b/arch/x86/boot/mkcpustr.c 2014-06-20 09:12:25.517247783 -0700
@@ -16,6 +16,7 @@
#include <stdio.h>
#include "../include/asm/required-features.h"
+#include "../include/asm/disabled-features.h"
#include "../include/asm/cpufeature.h"
#include "../kernel/cpu/capflags.c"
diff -puN arch/x86/include/asm/cpufeature.h~x86-disabled_features arch/x86/include/asm/cpufeature.h
--- a/arch/x86/include/asm/cpufeature.h~x86-disabled_features 2014-06-20 09:12:25.513247603 -0700
+++ b/arch/x86/include/asm/cpufeature.h 2014-06-20 09:15:38.967987107 -0700
@@ -8,6 +8,10 @@
#include <asm/required-features.h>
#endif
+#ifndef _ASM_X86_DISABLED_FEATURES_H
+#include <asm/disabled-features.h>
+#endif
+
#define NCAPINTS 10 /* N 32-bit words worth of info */
#define NBUGINTS 1 /* N 32-bit bug flags */
@@ -260,12 +264,26 @@ extern const char * const x86_power_flag
(((bit)>>5)==8 && (1UL<<((bit)&31) & REQUIRED_MASK8)) || \
(((bit)>>5)==9 && (1UL<<((bit)&31) & REQUIRED_MASK9)) )
+#define DISABLED_MASK_BIT_SET(bit) \
+ ( (((bit)>>5)==0 && (1UL<<((bit)&31) & DISABLED_MASK0)) || \
+ (((bit)>>5)==1 && (1UL<<((bit)&31) & DISABLED_MASK1)) || \
+ (((bit)>>5)==2 && (1UL<<((bit)&31) & DISABLED_MASK2)) || \
+ (((bit)>>5)==3 && (1UL<<((bit)&31) & DISABLED_MASK3)) || \
+ (((bit)>>5)==4 && (1UL<<((bit)&31) & DISABLED_MASK4)) || \
+ (((bit)>>5)==5 && (1UL<<((bit)&31) & DISABLED_MASK5)) || \
+ (((bit)>>5)==6 && (1UL<<((bit)&31) & DISABLED_MASK6)) || \
+ (((bit)>>5)==7 && (1UL<<((bit)&31) & DISABLED_MASK7)) || \
+ (((bit)>>5)==8 && (1UL<<((bit)&31) & DISABLED_MASK8)) || \
+ (((bit)>>5)==9 && (1UL<<((bit)&31) & DISABLED_MASK9)) )
+
#define cpu_has(c, bit) \
(__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \
+ __builtin_constant_p(bit) && DISABLED_MASK_BIT_SET(bit) ? 0 : \
test_cpu_cap(c, bit))
#define this_cpu_has(bit) \
(__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \
+ __builtin_constant_p(bit) && DISABLED_MASK_BIT_SET(bit) ? 0 : \
x86_this_cpu_test_bit(bit, (unsigned long *)&cpu_info.x86_capability))
#define boot_cpu_has(bit) cpu_has(&boot_cpu_data, bit)
diff -puN /dev/null arch/x86/include/asm/disabled-features.h
--- /dev/null 2014-04-10 11:28:14.066815724 -0700
+++ b/arch/x86/include/asm/disabled-features.h 2014-06-20 09:15:38.968987152 -0700
@@ -0,0 +1,33 @@
+#ifndef _ASM_X86_DISABLED_FEATURES_H
+#define _ASM_X86_DISABLED_FEATURES_H
+
+/* These features, although they might be available in a CPU
+ * will not be used because the compile options to support
+ * them are not present.
+ *
+ * This code allows them to be checked and disabled at
+ * compile time withut an explicit #ifdef. Simply call
+ * cpu_has() or this_cpu_has().
+ * */
+
+#ifdef CONFIG_X86_INTEL_MPX
+# define HAVE_MPX (1<<(X86_FEATURE_MPX & 31))
+#else
+# define HAVE_MPX 0
+#endif
+
+/*
+ * Make sure to add features to the correct mask
+ */
+#define DISABLED_MASK0 0
+#define DISABLED_MASK1 0
+#define DISABLED_MASK2 0
+#define DISABLED_MASK3 0
+#define DISABLED_MASK4 0
+#define DISABLED_MASK5 0
+#define DISABLED_MASK6 0
+#define DISABLED_MASK7 0
+#define DISABLED_MASK8 0
+#define DISABLED_MASK9 (HAVE_MPX)
+
+#endif /* _ASM_X86_DISABLED_FEATURES_H */
_
There are a few other features than MPX that we can make
assumptions about at compile-time based on compile options.
Add them to disabled-features.h
---
b/arch/x86/include/asm/cpufeature.h | 18 ------------------
b/arch/x86/include/asm/disabled-features.h | 16 ++++++++++++++--
2 files changed, 14 insertions(+), 20 deletions(-)
diff -puN arch/x86/include/asm/cpufeature.h~x86-disabled_features-addmore arch/x86/include/asm/cpufeature.h
--- a/arch/x86/include/asm/cpufeature.h~x86-disabled_features-addmore 2014-06-20 09:16:08.420317511 -0700
+++ b/arch/x86/include/asm/cpufeature.h 2014-06-20 09:16:08.425317736 -0700
@@ -357,32 +357,14 @@ extern const char * const x86_power_flag
#define cpu_has_eager_fpu boot_cpu_has(X86_FEATURE_EAGER_FPU)
#define cpu_has_topoext boot_cpu_has(X86_FEATURE_TOPOEXT)
-#ifdef CONFIG_X86_INTEL_MPX
-#define cpu_has_mpx boot_cpu_has(X86_FEATURE_MPX)
-#else
-#define cpu_has_mpx 0
-#endif /* CONFIG_X86_INTEL_MPX */
-
#ifdef CONFIG_X86_64
-#undef cpu_has_vme
-#define cpu_has_vme 0
-
#undef cpu_has_pae
#define cpu_has_pae ___BUG___
#undef cpu_has_mp
#define cpu_has_mp 1
-#undef cpu_has_k6_mtrr
-#define cpu_has_k6_mtrr 0
-
-#undef cpu_has_cyrix_arr
-#define cpu_has_cyrix_arr 0
-
-#undef cpu_has_centaur_mcr
-#define cpu_has_centaur_mcr 0
-
#endif /* CONFIG_X86_64 */
#if __GNUC__ >= 4
diff -puN arch/x86/include/asm/disabled-features.h~x86-disabled_features-addmore arch/x86/include/asm/disabled-features.h
--- a/arch/x86/include/asm/disabled-features.h~x86-disabled_features-addmore 2014-06-20 09:16:08.422317600 -0700
+++ b/arch/x86/include/asm/disabled-features.h 2014-06-20 09:16:08.426317782 -0700
@@ -16,13 +16,25 @@
# define HAVE_MPX 0
#endif
+#ifdef CONFIG_X86_64
+# define HAVE_VME (1<<(X86_FEATURE_VME & 31))
+# define HAVE_K6_MTRR (1<<(X86_FEATURE_K6_MTRR & 31))
+# define HAVE_CYRIX_ARR (1<<(X86_FEATURE_CYRIX_ARR & 31))
+# define HAVE_CENTAUR_MCR (1<<(X86_FEATURE_CENTAUR_MCR & 31))
+#else
+# define HAVE_VME 0
+# define HAVE_K6_MTRR 0
+# define HAVE_CYRIX_ARR 0
+# define HAVE_CENTAUR_MCR 0
+#endif /* CONFIG_X86_64 */
+
/*
* Make sure to add features to the correct mask
*/
-#define DISABLED_MASK0 0
+#define DISABLED_MASK0 (HAVE_VME|HAVE_K6_MTRR)
#define DISABLED_MASK1 0
#define DISABLED_MASK2 0
-#define DISABLED_MASK3 0
+#define DISABLED_MASK3 (HAVE_CYRIX_ARR|HAVE_CENTAUR_MCR)
#define DISABLED_MASK4 0
#define DISABLED_MASK5 0
#define DISABLED_MASK6 0
_
Today, we assume that all 64-bit cpus have X86_FEATURE_MP. It
should be in the REQUIRED_MASK so that we do not need the #undef
trick for it.
---
b/arch/x86/include/asm/cpufeature.h | 3 ---
b/arch/x86/include/asm/required-features.h | 4 +++-
2 files changed, 3 insertions(+), 4 deletions(-)
diff -puN arch/x86/include/asm/cpufeature.h~x86-make-mp-required_feature arch/x86/include/asm/cpufeature.h
--- a/arch/x86/include/asm/cpufeature.h~x86-make-mp-required_feature 2014-06-20 09:16:10.180396969 -0700
+++ b/arch/x86/include/asm/cpufeature.h 2014-06-20 09:16:10.185397194 -0700
@@ -362,9 +362,6 @@ extern const char * const x86_power_flag
#undef cpu_has_pae
#define cpu_has_pae ___BUG___
-#undef cpu_has_mp
-#define cpu_has_mp 1
-
#endif /* CONFIG_X86_64 */
#if __GNUC__ >= 4
diff -puN arch/x86/include/asm/required-features.h~x86-make-mp-required_feature arch/x86/include/asm/required-features.h
--- a/arch/x86/include/asm/required-features.h~x86-make-mp-required_feature 2014-06-20 09:16:10.182397059 -0700
+++ b/arch/x86/include/asm/required-features.h 2014-06-20 09:16:10.186397240 -0700
@@ -67,6 +67,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_MP (1<<(X86_FEATURE_MP & 31))
#else
#define NEED_PSE 0
#define NEED_MSR 0
@@ -75,6 +76,7 @@
#define NEED_XMM 0
#define NEED_XMM2 0
#define NEED_LM 0
+#define NEED_MP 0
#endif
#define REQUIRED_MASK0 (NEED_FPU|NEED_PSE|NEED_MSR|NEED_PAE|\
@@ -82,7 +84,7 @@
NEED_XMM|NEED_XMM2)
#define SSE_MASK (NEED_XMM|NEED_XMM2)
-#define REQUIRED_MASK1 (NEED_LM|NEED_3DNOW)
+#define REQUIRED_MASK1 (NEED_LM|NEED_3DNOW|NEED_MP)
#define REQUIRED_MASK2 0
#define REQUIRED_MASK3 (NEED_NOPL)
_
On 06/20/2014 09:17 AM, Dave Hansen wrote:
> diff -puN /dev/null arch/x86/include/asm/disabled-features.h
> --- /dev/null 2014-04-10 11:28:14.066815724 -0700
> +++ b/arch/x86/include/asm/disabled-features.h 2014-06-20 09:15:38.968987152 -0700
> @@ -0,0 +1,33 @@
> +#ifndef _ASM_X86_DISABLED_FEATURES_H
> +#define _ASM_X86_DISABLED_FEATURES_H
> +
> +/* These features, although they might be available in a CPU
> + * will not be used because the compile options to support
> + * them are not present.
> + *
> + * This code allows them to be checked and disabled at
> + * compile time withut an explicit #ifdef. Simply call
> + * cpu_has() or this_cpu_has().
> + * */
> +
> +#ifdef CONFIG_X86_INTEL_MPX
> +# define HAVE_MPX (1<<(X86_FEATURE_MPX & 31))
> +#else
> +# define HAVE_MPX 0
> +#endif
> +
Is this an inverted test?
-hpa
On 06/20/2014 09:17 AM, Dave Hansen wrote:
> Today, we assume that all 64-bit cpus have X86_FEATURE_MP. It
> should be in the REQUIRED_MASK so that we do not need the #undef
> trick for it.
I don't think we enforce that the MP bit is set in CPUID, though.
Non-AMD processors will typically not set this bit at all, so the
feature validation code would have to be modified to know to not require
this bit.
-hpa
>
> -#define REQUIRED_MASK1 (NEED_LM|NEED_3DNOW)
> +#define REQUIRED_MASK1 (NEED_LM|NEED_3DNOW|NEED_MP)
>
On 06/20/2014 09:23 AM, H. Peter Anvin wrote:
> On 06/20/2014 09:17 AM, Dave Hansen wrote:
>> > Today, we assume that all 64-bit cpus have X86_FEATURE_MP. It
>> > should be in the REQUIRED_MASK so that we do not need the #undef
>> > trick for it.
> I don't think we enforce that the MP bit is set in CPUID, though.
> Non-AMD processors will typically not set this bit at all, so the
> feature validation code would have to be modified to know to not require
> this bit.
Ahh, OK. I'll drop this.
On 06/20/2014 09:30 AM, Dave Hansen wrote:
> On 06/20/2014 09:23 AM, H. Peter Anvin wrote:
>> On 06/20/2014 09:17 AM, Dave Hansen wrote:
>>>> Today, we assume that all 64-bit cpus have X86_FEATURE_MP. It
>>>> should be in the REQUIRED_MASK so that we do not need the #undef
>>>> trick for it.
>> I don't think we enforce that the MP bit is set in CPUID, though.
>> Non-AMD processors will typically not set this bit at all, so the
>> feature validation code would have to be modified to know to not require
>> this bit.
>
> Ahh, OK. I'll drop this.
>
We probably should just the cpu_has_mp macro entirely. All it is used
for is printing a warning in amd_k7_smp_check().
Andi, Borislav -- as far as I can tell, we have *never* enforced this on
the 64-bit kernel, although we have enforced it on 64-bit processors
running the 32-bit kernel. We should either enforce it on both or just
drop it. What is your opinion?
-hpa
On 06/20/2014 09:20 AM, H. Peter Anvin wrote:
>> > +#ifdef CONFIG_X86_INTEL_MPX
>> > +# define HAVE_MPX (1<<(X86_FEATURE_MPX & 31))
>> > +#else
>> > +# define HAVE_MPX 0
>> > +#endif
>> > +
> Is this an inverted test?
Yes, that was inverted. I've renamed it to make it a bit easier to read:
#ifdef CONFIG_X86_INTEL_MPX
# define DISABLE_MPX 0
#else
# define DISABLE_MPX (1<<(X86_FEATURE_MPX & 31))
#endif
On Fri, Jun 20, 2014 at 09:37:57AM -0700, H. Peter Anvin wrote:
> We probably should just the cpu_has_mp macro entirely. All it is used
> for is printing a warning in amd_k7_smp_check().
>
> Andi, Borislav -- as far as I can tell, we have *never* enforced this on
> the 64-bit kernel, although we have enforced it on 64-bit processors
> running the 32-bit kernel. We should either enforce it on both or just
> drop it. What is your opinion?
Well, my AMD CPUID guide says CPUID Fn8000_0001_EDX[19] is reserved,
i.e. that X86_FEATURE_MP bit will be probably cleared.
And so it is, on my F15h AMD it is not set:
eax in: 0x80000001, eax = 00600f20 ebx = 10000000 ecx = 01ebbfff edx = 2fd3fbff
^
----------|
So I don't think we should enforce it on 64-bit.
I guess we can leave the check in amd_k7_smp_check() though but remove that
nasty cpu_has_mp macro and do static_cpu_has() instead.
My 2¢ only.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On 06/20/2014 10:43 AM, Borislav Petkov wrote:
> On Fri, Jun 20, 2014 at 09:37:57AM -0700, H. Peter Anvin wrote:
>> We probably should just the cpu_has_mp macro entirely. All it is used
>> for is printing a warning in amd_k7_smp_check().
>>
>> Andi, Borislav -- as far as I can tell, we have *never* enforced this on
>> the 64-bit kernel, although we have enforced it on 64-bit processors
>> running the 32-bit kernel. We should either enforce it on both or just
>> drop it. What is your opinion?
>
> Well, my AMD CPUID guide says CPUID Fn8000_0001_EDX[19] is reserved,
> i.e. that X86_FEATURE_MP bit will be probably cleared.
>
> And so it is, on my F15h AMD it is not set:
>
> eax in: 0x80000001, eax = 00600f20 ebx = 10000000 ecx = 01ebbfff edx = 2fd3fbff
> ^
> ----------|
>
> So I don't think we should enforce it on 64-bit.
>
> I guess we can leave the check in amd_k7_smp_check() though but remove that
> nasty cpu_has_mp macro and do static_cpu_has() instead.
>
> My 2¢ only.
>
This is run before static_cpu_has().
The point, though, was that we "enforce" (taint) on 32 bits but not on
64 bits, which is clearly wrong.
My inclination is to completely kill amd_k7_smp_check() entirely, since
noone seems to know when it actually matters and it is clearly historic.
-hpa
Looking at the AMD init code, there is a whole bunch of other 32/64-bit
differences that are clearly bogus. I also see that amd_k7_smp_check()
doesn't even *exist* on 64 bits, and that init_amd_k7() which calls
amd_k7_smp_check() only is ever called for family == 6, despite having
tests for family 7 and above in it.
Urgh.
Borislav, any way that you would be willing to take this on, or get
someone from AMD to clean up this mess?
-hpa
On Fri, Jun 20, 2014 at 10:47:22AM -0700, H. Peter Anvin wrote:
> This is run before static_cpu_has().
static_cpu_has_safe() then - I didn't do it for no reason :-)
> The point, though, was that we "enforce" (taint) on 32 bits but not on
> 64 bits, which is clearly wrong.
Yeah, K7 is 32-bit only.
> My inclination is to completely kill amd_k7_smp_check() entirely,
> since noone seems to know when it actually matters and it is clearly
> historic.
I think DaveJ should know something about it - he gave that impression
last time when we were discussing 8c90487cdc64 ("Rename TAINT_UNSAFE_SMP
to TAINT_CPU_OUT_OF_SPEC").
CCed.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On Fri, Jun 20, 2014 at 10:50:25AM -0700, H. Peter Anvin wrote:
> Looking at the AMD init code, there is a whole bunch of other 32/64-bit
> differences that are clearly bogus. I also see that amd_k7_smp_check()
> doesn't even *exist* on 64 bits, and that init_amd_k7() which calls
> amd_k7_smp_check() only is ever called for family == 6, despite having
> tests for family 7 and above in it.
No, K7 is family 6. The tests are for c->x86_model, or am I looking at
the wrong place?
OTOH, init_amd() could probably use a cleanup of moving the per-family
code into init_amd_<fam>() functions and extending the switch-case.
I'll take a look.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On Fri, Jun 20, 2014 at 08:05:23PM +0200, Borislav Petkov wrote:
> On Fri, Jun 20, 2014 at 10:47:22AM -0700, H. Peter Anvin wrote:
> > This is run before static_cpu_has().
>
> static_cpu_has_safe() then - I didn't do it for no reason :-)
>
> > The point, though, was that we "enforce" (taint) on 32 bits but not on
> > 64 bits, which is clearly wrong.
>
> Yeah, K7 is 32-bit only.
>
> > My inclination is to completely kill amd_k7_smp_check() entirely,
> > since noone seems to know when it actually matters and it is clearly
> > historic.
>
> I think DaveJ should know something about it - he gave that impression
> last time when we were discussing 8c90487cdc64 ("Rename TAINT_UNSAFE_SMP
> to TAINT_CPU_OUT_OF_SPEC").
AMD sold two separate SKUs: the Athlon XP and the Athlon MP.
Only the latter was supposedly "certified" for use in multi-processor
boards. People found out however that sometimes the XP's 'worked'
if you modded them (see http://www.hardwaresecrets.com/article/How-to-Transform-an-Athlon-XP-into-an-Athlon-MP/24)
There was belief that AMD had reason beyond "more price mark-up for MP's"
and that those fuses had been blown for good reason (failing validation
in some conditions for eg).
I doubt anyone is actually even running such a system any more on
a modern kernel, and any weird crashes would be written off more by
"you're running 10+ year old hardware, it's probably broken" than
"it was never meant to do that".
Dave
On Fri, Jun 20, 2014 at 02:16:00PM -0400, Dave Jones wrote:
> AMD sold two separate SKUs: the Athlon XP and the Athlon MP.
> Only the latter was supposedly "certified" for use in multi-processor
> boards. People found out however that sometimes the XP's 'worked'
> if you modded them (see http://www.hardwaresecrets.com/article/How-to-Transform-an-Athlon-XP-into-an-Athlon-MP/24)
Haha, that was a fun read. Closing fuses with a pencil - this reminds me
of:
https://www.youtube.com/watch?v=6QihBIewyrY
> I doubt anyone is actually even running such a system any more on
> a modern kernel, and any weird crashes would be written off more by
> "you're running 10+ year old hardware, it's probably broken" than
> "it was never meant to do that".
Yeah, we can kill amd_k7_smp_check() but I don't see why - it doesn't
hurt anyone right now. I don't care all that much either way, though -
whatever.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On 06/20/2014 11:05 AM, Borislav Petkov wrote:
> On Fri, Jun 20, 2014 at 10:47:22AM -0700, H. Peter Anvin wrote:
>> This is run before static_cpu_has().
>
> static_cpu_has_safe() then - I didn't do it for no reason :-)
No, it has to be cpu_has() -- the dynamic, CPU-specific version.
-hpa
On 06/20/2014 11:15 AM, Borislav Petkov wrote:
> On Fri, Jun 20, 2014 at 10:50:25AM -0700, H. Peter Anvin wrote:
>> Looking at the AMD init code, there is a whole bunch of other 32/64-bit
>> differences that are clearly bogus. I also see that amd_k7_smp_check()
>> doesn't even *exist* on 64 bits, and that init_amd_k7() which calls
>> amd_k7_smp_check() only is ever called for family == 6, despite having
>> tests for family 7 and above in it.
>
> No, K7 is family 6. The tests are for c->x86_model, or am I looking at
> the wrong place?
>
> OTOH, init_amd() could probably use a cleanup of moving the per-family
> code into init_amd_<fam>() functions and extending the switch-case.
>
> I'll take a look.
>
Ah, yes, you're right.
This code is clearly not applicable to any 64-bit CPU, so cpu_has_mp is
simply a noop on 64 bits... so no need for Dave H. to worry about it at
all; we should get rid of it and replace it with cpu_has() in the AMD code.
I actually have Linus' old dual-processor K7 sitting in my garage, but
$DEITY knows if it actually runs.
-hpa
On Fri, Jun 20, 2014 at 11:54:14AM -0700, H. Peter Anvin wrote:
> No, it has to be cpu_has() -- the dynamic, CPU-specific version.
Ok, sry, but I have to ask: why cpu_has? Why not boot_cpu_has and thus
static_cpu_has_safe?
At
if (this_cpu->c_init)
this_cpu->c_init(c)
time in identify_cpu() when we execute init_amd(), we have run
generic_identify(c) which would have done get_cpu_cap() already and
boot_cpu_data.x86_capability[1] would have cpuid_edx(0x80000001) where
X86_FEATURE_MP bit would be set too.
Oh, and this happens after the first clearing
/* Clear/Set all flags overriden by options, after probe */
but this won't clear it. And we're before the building of the common
feature set at the end of identify_cpu(). But even that doesn't matter
in this case because this bit will be set in CPUID on every core and
thus remain set in boot_cpu_data.
Bah, so what am I missing?
Thanks.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On 06/20/2014 01:00 PM, Borislav Petkov wrote:
> On Fri, Jun 20, 2014 at 11:54:14AM -0700, H. Peter Anvin wrote:
>> No, it has to be cpu_has() -- the dynamic, CPU-specific version.
>
> Ok, sry, but I have to ask: why cpu_has? Why not boot_cpu_has and thus
> static_cpu_has_safe?
>
Because the whole point is testing each CPU and warn if it is unsuitable.
static-anything is just plain useless, because we test this once for
each CPU and patching a branch that we are never going to cross again is
just wasted effort.
-hpa
On Fri, Jun 20, 2014 at 01:22:55PM -0700, H. Peter Anvin wrote:
> Because the whole point is testing each CPU and warn if it is
> unsuitable.
>
> static-anything is just plain useless, because we test this once for
> each CPU and patching a branch that we are never going to cross again
> is just wasted effort.
Ah, I see. :-)
Thanks.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On Fri, Jun 20, 2014 at 11:57:21AM -0700, H. Peter Anvin wrote:
> I actually have Linus' old dual-processor K7 sitting in my garage, but
> $DEITY knows if it actually runs.
Bah, bring it to the scrap yard so that you can free up some space.
People have that computing power now in their pockets. :-)
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On 06/20/2014 09:17 AM, Dave Hansen wrote:
> +/* These features, although they might be available in a CPU
> + * will not be used because the compile options to support
> + * them are not present.
> + *
> + * This code allows them to be checked and disabled at
> + * compile time withut an explicit #ifdef. Simply call
> + * cpu_has() or this_cpu_has().
> + * */
> +
> +#ifdef CONFIG_X86_INTEL_MPX
> +# define HAVE_MPX (1<<(X86_FEATURE_MPX & 31))
> +#else
> +# define HAVE_MPX 0
> +#endif
Urg. arch/x86/kvm/vmx.c has:
if (boot_cpu_has(X86_FEATURE_MPX))
rdmsrl(MSR_IA32_BNDCFGS, vmx->host_state.msr_host_...
And in that case we really _do_ want to test the actual CPU bit, despite
how we've configured MPX in the host kernel. We can call test_cpu_cap()
directly, I guess. But, all these interfaces are getting confusing. :(
> We probably should just the cpu_has_mp macro entirely. All it is used
> for is printing a warning in amd_k7_smp_check().
>
> Andi, Borislav -- as far as I can tell, we have *never* enforced this on
> the 64-bit kernel, although we have enforced it on 64-bit processors
> running the 32-bit kernel. We should either enforce it on both or just
> drop it. What is your opinion?
Drop it everywhere.
-Andi