From: Tony Luck <tony.luck@intel.com>
To: Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>,
"Peter Zijlstra (Intel)" <peterz@infradead.org>,
Uros Bizjak <ubizjak@gmail.com>,
Rick Edgecombe <rick.p.edgecombe@intel.com>,
Arnd Bergmann <arnd@arndb.de>, Tony Luck <tony.luck@intel.com>,
Mateusz Guzik <mjguzik@gmail.com>,
Thomas Renninger <trenn@suse.de>,
Greg Kroah-Hartman <gregkh@suse.de>,
Andi Kleen <ak@linux.intel.com>,
linux-kernel@vger.kernel.org, patches@lists.linux.dev
Subject: [PATCH v2] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR_INTEL
Date: Thu, 16 May 2024 09:29:25 -0700 [thread overview]
Message-ID: <20240516162925.79245-1-tony.luck@intel.com> (raw)
Code in v.9 arch/x86/kernel/smpboot.c was changed by commit 4db64279bc2b
("x86/cpu: Switch to new Intel CPU model defines") from old code:
440 static const struct x86_cpu_id intel_cod_cpu[] = {
441 X86_MATCH_INTEL_FAM6_MODEL(HASWELL_X, 0), /* COD */
442 X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_X, 0), /* COD */
443 X86_MATCH_INTEL_FAM6_MODEL(ANY, 1), /* SNC */
444 {}
445 };
446
447 static bool match_llc(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
448 {
449 const struct x86_cpu_id *id = x86_match_cpu(intel_cod_cpu);
new code:
440 static const struct x86_cpu_id intel_cod_cpu[] = {
441 X86_MATCH_VFM(INTEL_HASWELL_X, 0), /* COD */
442 X86_MATCH_VFM(INTEL_BROADWELL_X, 0), /* COD */
443 X86_MATCH_VFM(INTEL_ANY, 1), /* SNC */
444 {}
445 };
446
447 static bool match_llc(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
448 {
449 const struct x86_cpu_id *id = x86_match_cpu(intel_cod_cpu);
On an Intel CPU with SNC enabled this code previously matched the rule
on line 443 to avoid printing messages about insane cache configuration.
The new code did not match any rules.
Expanding the macros for the intel_cod_cpu[] array shows that the old
is equivalent to:
static const struct x86_cpu_id intel_cod_cpu[] = {
[0] = { .vendor = 0, .family = 6, .model = 0x3F, .steppings = 0, .feature = 0, .driver_data = 0 },
[1] = { .vendor = 0, .family = 6, .model = 0x4F, .steppings = 0, .feature = 0, .driver_data = 0 },
[2] = { .vendor = 0, .family = 6, .model = 0x00, .steppings = 0, .feature = 0, .driver_data = 0 },
[3] = { .vendor = 0, .family = 0, .model = 0x00, .steppings = 0, .feature = 0, .driver_data = 0 }
}
while the new code expands to:
static const struct x86_cpu_id intel_cod_cpu[] = {
[0] = { .vendor = 0, .family = 6, .model = 0x3F, .steppings = 0, .feature = 0, .driver_data = 0 },
[1] = { .vendor = 0, .family = 6, .model = 0x4F, .steppings = 0, .feature = 0, .driver_data = 0 },
[2] = { .vendor = 0, .family = 0, .model = 0x00, .steppings = 0, .feature = 0, .driver_data = 0 },
[3] = { .vendor = 0, .family = 0, .model = 0x00, .steppings = 0, .feature = 0, .driver_data = 0 }
}
Looking at the code for x86_match_cpu():
36 const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match)
37 {
38 const struct x86_cpu_id *m;
39 struct cpuinfo_x86 *c = &boot_cpu_data;
40
41 for (m = match;
42 m->vendor | m->family | m->model | m->steppings | m->feature;
43 m++) {
...
56 }
57 return NULL;
58 }
59 EXPORT_SYMBOL(x86_match_cpu);
it is clear that there was no match because the ANY entry in the table
(array index 2) is now the loop termination condition (all of vendor,
family, model, steppings, and feature are zero).
So this code was working before because the "ANY" check was looking for
any Intel CPU in family 6. But fails now because the family is a wild
card. So the root cause is that x86_match_cpu() has never been able to
match on a rule with just X86_VENDOR_INTEL and all other fields set to
wildcards.
Fix by changing X86_VENDOR_INTEL to a non-zero value (4 was lowest
unused value, so I picked that).
Fixes: 644e9cbbe3fc ("Add driver auto probing for x86 features v4")
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
Changes since v1:
1) More detailed commit description.
2) Changed "Fixes" tag. Commit 4db64279bc2b merely revealed a twelve
year old gap in the implementation of x86_match_cpu().
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
next reply other threads:[~2024-05-16 16:29 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-16 16:29 Tony Luck [this message]
2024-05-16 21:27 ` [PATCH v2] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR_INTEL Luck, Tony
2024-05-17 14:43 ` Borislav Petkov
2024-05-17 17:35 ` H. Peter Anvin
2024-05-17 18:17 ` Luck, Tony
2024-05-17 20:42 ` Thomas Gleixner
2024-05-17 20:41 ` Thomas Gleixner
2024-05-17 20:54 ` Luck, Tony
2024-05-20 23:33 ` H. Peter Anvin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240516162925.79245-1-tony.luck@intel.com \
--to=tony.luck@intel.com \
--cc=ak@linux.intel.com \
--cc=arnd@arndb.de \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=gregkh@suse.de \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=mjguzik@gmail.com \
--cc=patches@lists.linux.dev \
--cc=peterz@infradead.org \
--cc=rick.p.edgecombe@intel.com \
--cc=tglx@linutronix.de \
--cc=trenn@suse.de \
--cc=ubizjak@gmail.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).