patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
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>, Andi Kleen <ak@linux.intel.com>,
	linux-kernel@vger.kernel.org, patches@lists.linux.dev
Subject: [PATCH v6 02/49] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR_INTEL
Date: Mon, 20 May 2024 15:45:33 -0700	[thread overview]
Message-ID: <20240520224620.9480-3-tony.luck@intel.com> (raw)
In-Reply-To: <20240520224620.9480-1-tony.luck@intel.com>

Code in v6.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 = 1 },
[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 = 1 },
[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 adding a new flags field to struct x86_cpu_id that has a bit set
to indicate that this entry in the array is valid. Update X86_MATCH*()
macros to set that bit. Chenge the end-marker check in x86_match_cpu()
to just check the flags field for this bit.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Suggested-by: Borislav Petkov <bp@alien8.de>
Fixes: 644e9cbbe3fc ("Add driver auto probing for x86 features v4")
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
---
 include/linux/mod_devicetable.h      | 4 ++++
 arch/x86/include/asm/cpu_device_id.h | 2 ++
 arch/x86/kernel/cpu/match.c          | 4 +---
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 7a9a07ea451b..ca3468ad06ff 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -690,6 +690,7 @@ struct x86_cpu_id {
 	__u16 model;
 	__u16 steppings;
 	__u16 feature;	/* bit index */
+	__u16 flags;
 	kernel_ulong_t driver_data;
 };
 
@@ -700,6 +701,9 @@ struct x86_cpu_id {
 #define X86_STEPPING_ANY 0
 #define X86_FEATURE_ANY 0	/* Same as FPU, you can't test for that */
 
+/* x86_cpu_id::flags */
+#define X86_CPU_ID_FLAG_ENTRY_VALID	BIT(0)
+
 /*
  * Generic table type for matching CPU features.
  * @feature:	the bit number of the feature (0 - 65535)
diff --git a/arch/x86/include/asm/cpu_device_id.h b/arch/x86/include/asm/cpu_device_id.h
index 970a232009c3..54a71c669ce9 100644
--- a/arch/x86/include/asm/cpu_device_id.h
+++ b/arch/x86/include/asm/cpu_device_id.h
@@ -79,6 +79,7 @@
 	.model		= _model,					\
 	.steppings	= _steppings,					\
 	.feature	= _feature,					\
+	.flags		= X86_CPU_ID_FLAG_ENTRY_VALID,			\
 	.driver_data	= (unsigned long) _data				\
 }
 
@@ -89,6 +90,7 @@
 	.model		= _model,					\
 	.steppings	= _steppings,					\
 	.feature	= _feature,					\
+	.flags		= X86_CPU_ID_FLAG_ENTRY_VALID,			\
 	.driver_data	= (unsigned long) _data				\
 }
 
diff --git a/arch/x86/kernel/cpu/match.c b/arch/x86/kernel/cpu/match.c
index 8651643bddae..8e7de733320a 100644
--- a/arch/x86/kernel/cpu/match.c
+++ b/arch/x86/kernel/cpu/match.c
@@ -38,9 +38,7 @@ const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match)
 	const struct x86_cpu_id *m;
 	struct cpuinfo_x86 *c = &boot_cpu_data;
 
-	for (m = match;
-	     m->vendor | m->family | m->model | m->steppings | m->feature;
-	     m++) {
+	for (m = match; m->flags & X86_CPU_ID_FLAG_ENTRY_VALID; m++) {
 		if (m->vendor != X86_VENDOR_ANY && c->x86_vendor != m->vendor)
 			continue;
 		if (m->family != X86_FAMILY_ANY && c->x86 != m->family)
-- 
2.45.0


  parent reply	other threads:[~2024-05-20 22:46 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-20 22:45 [PATCH v6 00/49] New Intel CPUID families Tony Luck
2024-05-20 22:45 ` [PATCH v6 01/49] crypto: x86/aes-xts - Switch to new Intel CPU model defines Tony Luck
2024-05-21 17:22   ` Borislav Petkov
2024-05-21 17:36     ` Eric Biggers
2024-05-22  3:32     ` Herbert Xu
2024-05-20 22:45 ` Tony Luck [this message]
2024-05-21  7:49   ` [PATCH v6 02/49] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR_INTEL Borislav Petkov
2024-05-21 15:48     ` Tony Luck
2024-05-21 17:18       ` Borislav Petkov
2024-05-20 22:45 ` [PATCH v6 03/49] tpm: Switch to new Intel CPU model defines Tony Luck
2024-05-20 22:45 ` [PATCH v6 04/49] platform/x86/intel/ifs: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 05/49] media: atomisp: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 06/49] ASoC: Intel: avs: es8336: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 07/49] platform/x86: intel_scu_wdt: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 08/49] KVM: x86/pmu: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 09/49] KVM: VMX: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 10/49] cpufreq: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 11/49] intel_idle: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 12/49] PCI: PM: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 13/49] powercap: intel_rapl: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 14/49] ASoC: Intel: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 15/49] thermal: intel: intel_tcc_cooling: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 16/49] x86/platform/intel-mid: " Tony Luck
2024-05-21 14:12   ` Andy Shevchenko
2024-05-21 16:10     ` [PATCH v6.1 " Tony Luck
2024-05-21 16:17       ` Andy Shevchenko
2024-05-20 22:45 ` [PATCH v6 17/49] platform/x86: intel_speed_select_if: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 18/49] platform/x86: intel-uncore-freq: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 19/49] platform/x86: intel_ips: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 20/49] platform/x86: intel_telemetry: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 21/49] platform/x86: intel: telemetry: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 22/49] platform/x86: intel_turbo_max_3: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 23/49] platform/x86: p2sb: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 24/49] platform/x86/intel: pmc: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 25/49] platform/x86/intel/pmc: " Tony Luck
2024-05-20 22:45 ` [PATCH v6 26/49] crypto: x86/poly1305 - " Tony Luck
2024-05-20 22:45 ` [PATCH v6 27/49] crypto: x86/twofish " Tony Luck
2024-05-20 22:45 ` [PATCH v6 28/49] x86/cpu/intel: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 29/49] x86/PCI: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 30/49] x86/virt/tdx: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 31/49] perf/x86/intel: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 32/49] x86/platform/atom: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 33/49] x86/cpu: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 34/49] x86/boot: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 35/49] EDAC/i10nm: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 36/49] EDAC, pnd2: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 37/49] EDAC/sb_edac: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 38/49] EDAC/skx: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 39/49] extcon: axp288: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 40/49] ACPI: LPSS: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 41/49] ACPI: x86: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 42/49] cpufreq: intel_pstate: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 43/49] perf/x86/rapl: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 44/49] platform/x86: ISST: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 45/49] powercap: intel_rapl: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 46/49] tools/power/turbostat: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 47/49] peci, hwmon: " Tony Luck
2024-05-20 22:46 ` [PATCH v6 48/49] x86/cpu/vfm: Delete X86_MATCH_INTEL_FAM6_MODEL[_STEPPING]() macros Tony Luck
2024-05-20 22:46 ` [PATCH v6 49/49] x86/cpu/vfm: Delete all the *_FAM6_ CPU #defines Tony Luck
2024-05-21  8:32 ` [PATCH v6 00/49] New Intel CPUID families Borislav Petkov
2024-05-21 15:21   ` Luck, Tony
2024-05-28 17:34 ` Tony Luck
2024-06-04 23:29 ` Sean Christopherson

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=20240520224620.9480-3-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=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).