[PATCH] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR_INTEL

[PATCH] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR_INTEL

[Tony Luck]

There's a wildcard match in arch/x86/kernel/smpboot.c that wants
to hit on any CPU made by Intel. The match used to work because
the check was actually looking for any Intel CPU in family 6.

With the change to ease support for families other than 6,
this wildcard match failed because every entry in the struct x86_cpu_id
was zero. This is used as the end-marker in arrays of x86_cpu_id
structures, so can never match.

Failure to match meant the logic to detect Intel Sub-NUMA cluster
mode didn't operate and there were boot messages about insane
cache configuration.

Fix by changing X86_VENDOR_INTEL to a non-zero value (4 was lowest
unused value, so I picked that).

Fixes: 4db64279bc2b ("x86/cpu: Switch to new Intel CPU model defines")
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/include/asm/processor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index cb4f6c513c48..271c4c95bc37 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -175,10 +175,10 @@ struct cpuinfo_x86 {
 	unsigned		initialized : 1;
 } __randomize_layout;
 
-#define X86_VENDOR_INTEL	0
 #define X86_VENDOR_CYRIX	1
 #define X86_VENDOR_AMD		2
 #define X86_VENDOR_UMC		3
+#define X86_VENDOR_INTEL	4
 #define X86_VENDOR_CENTAUR	5
 #define X86_VENDOR_TRANSMETA	7
 #define X86_VENDOR_NSC		8
-- 
2.44.0

Re: [PATCH] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR_INTEL

[Borislav Petkov]

On Wed, May 15, 2024 at 02:43:57PM -0700, Tony Luck wrote:
> There's a wildcard match in arch/x86/kernel/smpboot.c that wants
> to hit on any CPU made by Intel. The match used to work because

intel_cod_cpu[] or which one?

Please be more specific.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

RE: [PATCH] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR_INTEL

[Luck, Tony]

>> There's a wildcard match in arch/x86/kernel/smpboot.c that wants
>> to hit on any CPU made by Intel. The match used to work because
>
> intel_cod_cpu[] or which one?
>
> Please be more specific.

intel_cod_cpu[] is the only one in arch/x86/kernel/smpboot.c

-Tony

Re: [PATCH] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR_INTEL

[Borislav Petkov]

On Wed, May 15, 2024 at 10:07:56PM +0000, Luck, Tony wrote:
> >> There's a wildcard match in arch/x86/kernel/smpboot.c that wants
> >> to hit on any CPU made by Intel. The match used to work because
> >
> > intel_cod_cpu[] or which one?
> >
> > Please be more specific.
> 
> intel_cod_cpu[] is the only one in arch/x86/kernel/smpboot.c

Sorry Tony, commit messages are not a guessing game but a precise
explanation as to what the problem is. Please try again.

Also, this change looks like "oh, lemme change the Intel vendor number
so that my conditional works".

But the Intel vendor has been 0 for what, 30 years?

Are you sure no other code in the tree relies on that? Audited?

Also, those x86_match_cpu functions with for-loop termination
conditionals checking whether stuff is not 0 - i.e., that:

        for (m = match;
             m->vendor | m->family | m->model | m->steppings | m->feature;
	     ^^^^^^^^^^^
             m++) {

are technically wrong when a vendor 0 is valid.

So this has all worked by chance and you've hit the one case where it
didn't.

Because it's not like we don't have a "invalid vendor" define:

#define X86_VENDOR_UNKNOWN      0xff

So, *actually*, all those terminators at the end of those arrays should
have those invalid values.

And there are a gazillion of those in the tree... :-\

Lovely.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette