[RFC PATCH 00/15] Prepare for new Intel family models

[RFC PATCH 00/15] Prepare for new Intel family models

[Sohil Mehta]

---Tl;Dr---
Audit all the Intel family model checks to get ready for the upcoming family 18
and 19 models.
Patches 1-5: Fixes in arch/x86 and drivers
Patches 6-15: Cleanups in arch/x86 to convert x86_model checks to VFM ones.

This series does not include cleanups in drivers/

---Background---
Mainstream Intel processors have been using family number 6 for a couple of
decades. The last deviation from this were the Netburst architecture based
Pentium 4s that had a family number of 15. Intel has recently started to
introduce extended family numbers greater than 15 [1]. Though newer CPUs can
have any family number, the currently planned CPUs lie in families 18 and 19.

Some kernel code assumes that the family number would always remain 6.  There
are checks that apply to recent family 6 models such as Lunar Lake and
Clearwater Forest but don't automatically extend to family 19 models such as
Diamond Rapids. This series aims to fix and cleanup all of such Intel specific
checks (mainly in arch/x86) to avoid such issues in the future. It also
converts almost all of the x86_model checks in arch/x86 to the new VFM ones.

OTOH, x86_model usage in drivers/ is a huge mess. Some drivers might need to be
completely rewritten to make them future proof. I have attempted a couple of
fixes in cpufreq and hwmon but I am uncertain of their efficacy.  A more
thorough clean up of drivers is needed to replace all x86_model usage with
the new VFM checks.

---Assumptions and Trade-offs---
Newer CPUs will have model numbers only in family 6 or after family 15.  No new
processors would be added between family 6 and 15. There are a couple of
related quirks that have been described below.

As a convention, Intel family numbers are referenced using decimals (family 15,
19, etc.) even though the AMD specific code might prefer hexadecimals in
similar situations.

It would be preferable to have simpler and more maintainable checks that might
in a rare situation change the behavior on really old platforms. If someone
pops up with an issue, the code would be fixed later.
For example, the check,
	c->x86_vfm >= INTEL_PENTIUM_PRO
is preferred over,
	c->x86 == 6 || c->x86 > 15
if the likelihood of adversely affecting family 15 is low.

Also, the CPU defines in intel-family.h have been added as and when needed to
make reviewing and applying patches out of order easier. Please let me know if
it would be preferable to add all the defines in a single patch at the
beginning.

---Noteworthy quirks---
Pentium II Overdrive - A unique family number:
  Wikipedia says[2], In Intel's "Family/Model/Stepping" scheme, the Pentium II
  OverDrive CPU identifies itself as family 6, model 3, though this is 
  misleading, as it is not based on the family 6/model 3 Klamath core. As 
  mentioned in the Pentium II Processor update documentation from Intel, 
  "although this processor has a CPUID of 163xh, it uses a Pentium II processor 
  CPUID 065xh processor core."

  A dump of the microcode file 06-03-02 shows:
    001/001: sig 0x00001632, pf_mask 0x00, 1998-06-10, rev 0x0002, size 2048
  An archived CPUID dump [3] also says:
    CPUID 00000001: 00001632-00000000-00000000-0183FBFF

  That would translate to a family number of 22 (0x16). This aberration is 
  not explicitly handled anywhere in the kernel so the platform might already 
  be broken. This series might make it worse for the platform if the latest 
  kernel works on it by chance.

Xeon Phi coprocessors:
  The first couple of Knights line of products (Knights Ferry and Knights
  Corner) have a family number of 11. But they were released purely as
  coprocessors and would not directly boot mainline Linux. The later
  versions Knights Landing and Knights Mill could act as a host processor
  but they used family number 6. So, the changes in this series should not
  affect any of these platforms.

---Links---
[1]: commit d1fb034b75a8 ("x86/cpu: Add two Intel CPU model numbers")
[2]: https://en.wikipedia.org/wiki/Pentium_II#Pentium_II_OverDrive
[3]: https://github.com/InstLatx64/InstLatx64/blob/master/GenuineIntel/GenuineIntel0000632_P2OD_CPUID.txt

The series has only been lightly tested on a couple of family 6 systems.
Planning to test it on more systems in the future. Any testing on legacy
platforms would be greatly helpful.

Many thanks to Tony and Dave for guiding me through this historical maze.

Sohil Mehta (15):
  x86/apic: Fix 32-bit APIC initialization for extended Intel families
  x86/apic: Fix smp init delay for extended Intel families
  x86/cpu/intel: Fix init_intel() checks for extended family numbers
  cpufreq: Fix the efficient idle check for Intel extended families
  hwmon: Fix Intel family checks to include extended family numbers
  x86/microcode: Update the Intel processor flag scan check
  x86/mtrr: Modify a x86_model check to an Intel VFM check
  x86/cpu/intel: Replace early family 6 checks with VFM ones
  x86/cpu/intel: Replace family 15 checks with VFM ones
  x86/cpu/intel: Replace family 5 model checks with VFM ones
  x86/pat: Replace Intel model checks with VFM ones
  x86/acpi/cstate: Improve Intel family model checks
  x86/cpu/intel: Bound the non-architectural constant_tsc model checks
  perf/x86: Simplify p6_pmu_init()
  perf/x86/p4: Replace Pentium 4 model checks with VFM ones

 arch/x86/events/intel/p4.c            |  7 ++--
 arch/x86/events/intel/p6.c            | 28 ++++---------
 arch/x86/include/asm/intel-family.h   | 16 ++++++++
 arch/x86/kernel/acpi/cstate.c         |  8 ++--
 arch/x86/kernel/apic/apic.c           |  4 +-
 arch/x86/kernel/cpu/intel.c           | 58 ++++++++++++---------------
 arch/x86/kernel/cpu/microcode/intel.c |  3 +-
 arch/x86/kernel/cpu/mtrr/generic.c    |  4 +-
 arch/x86/kernel/smpboot.c             |  8 ++--
 arch/x86/mm/pat/memtype.c             |  7 ++--
 drivers/cpufreq/cpufreq_ondemand.c    | 13 +++---
 drivers/hwmon/coretemp.c              | 26 ++++++++----
 12 files changed, 96 insertions(+), 86 deletions(-)


base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
-- 
2.43.0

[RFC PATCH 01/15] x86/apic: Fix 32-bit APIC initialization for extended Intel families

[Sohil Mehta]

detect_init_APIC() limits the APIC detection to families 6 and 15.
Extend the check to family numbers beyond 15. Also, convert it to a VFM
check to make it simpler.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/kernel/apic/apic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index c5fb28e6451a..13dac8f78213 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2014,8 +2014,8 @@ static bool __init detect_init_APIC(void)
 	case X86_VENDOR_HYGON:
 		break;
 	case X86_VENDOR_INTEL:
-		if (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15 ||
-		    (boot_cpu_data.x86 == 5 && boot_cpu_has(X86_FEATURE_APIC)))
+		if ((boot_cpu_data.x86 == 5 && boot_cpu_has(X86_FEATURE_APIC)) ||
+		    boot_cpu_data.x86_vfm >= INTEL_PENTIUM_PRO)
 			break;
 		goto no_apic;
 	default:
-- 
2.43.0

[RFC PATCH 02/15] x86/apic: Fix smp init delay for extended Intel families

[Sohil Mehta]

The MP specification version 1.4 references the i486 and early Pentium
processors in family 5. However, all processors starting with family 6
likely do not need the 10 msec INIT delay. The omission of the Pentium
4s (family 15) seems like an oversight in the original check.

With some risk, choose a simpler check and extend the quirk to all
recent and upcoming Intel processors.

While at it, fix the command line parameter comment to match with the
actual name.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/kernel/smpboot.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index b5a8f0891135..6c98e9178963 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -668,7 +668,7 @@ static void impress_friends(void)
  * But that slows boot and resume on modern processors, which include
  * many cores and don't require that delay.
  *
- * Cmdline "init_cpu_udelay=" is available to over-ride this delay.
+ * Cmdline "cpu_init_udelay=" is available to override this delay.
  * Modern processor families are quirked to remove the delay entirely.
  */
 #define UDELAY_10MS_DEFAULT 10000
@@ -690,9 +690,9 @@ static void __init smp_quirk_init_udelay(void)
 		return;
 
 	/* if modern processor, use no delay */
-	if (((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) && (boot_cpu_data.x86 == 6)) ||
-	    ((boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) && (boot_cpu_data.x86 >= 0x18)) ||
-	    ((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) && (boot_cpu_data.x86 >= 0xF))) {
+	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL && boot_cpu_data.x86_vfm >= INTEL_PENTIUM_PRO) ||
+	    (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON && boot_cpu_data.x86 >= 0x18) ||
+	    (boot_cpu_data.x86_vendor == X86_VENDOR_AMD && boot_cpu_data.x86 >= 0xF)) {
 		init_udelay = 0;
 		return;
 	}
-- 
2.43.0

[RFC PATCH 03/15] x86/cpu/intel: Fix init_intel() checks for extended family numbers

[Sohil Mehta]

X86_FEATURE_REP_GOOD is only set for family 6 processors.  Extend the
check to family numbers beyond 15.

It is uncertain whether the Pentium 4s (family 15) should set the
feature flag as well. Commit 185f3b9da24c ("x86: make intel.c have
64-bit support code") that originally set X86_FEATURE_REP_GOOD also set
the x86_cache_alignment preference for family 15 processors. The
omission of the family 15 seems intentional.

Also, the 32-bit user copy alignment preference is only set for family 6
and 15 processors. Extend the preference to family numbers beyond 15.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/kernel/cpu/intel.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 8ded9f859a3a..f44b2e618fb3 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -449,23 +449,16 @@ static void intel_workarounds(struct cpuinfo_x86 *c)
 	    (c->x86_stepping < 0x6 || c->x86_stepping == 0xb))
 		set_cpu_bug(c, X86_BUG_11AP);
 
-
 #ifdef CONFIG_X86_INTEL_USERCOPY
 	/*
 	 * Set up the preferred alignment for movsl bulk memory moves
+	 * Family 4 - 486: untested
+	 * Family 5 - Pentium: untested
+	 * Family 6 - PII/PIII only like movsl with 8-byte alignment
+	 * Family 15 - P4 is OK down to 8-byte alignment
 	 */
-	switch (c->x86) {
-	case 4:		/* 486: untested */
-		break;
-	case 5:		/* Old Pentia: untested */
-		break;
-	case 6:		/* PII/PIII only like movsl with 8-byte alignment */
-		movsl_mask.mask = 7;
-		break;
-	case 15:	/* P4 is OK down to 8-byte alignment */
+	if (c->x86_vfm >= INTEL_PENTIUM_PRO)
 		movsl_mask.mask = 7;
-		break;
-	}
 #endif
 
 	intel_smp_check(c);
@@ -563,7 +556,7 @@ static void init_intel(struct cpuinfo_x86 *c)
 #ifdef CONFIG_X86_64
 	if (c->x86 == 15)
 		c->x86_cache_alignment = c->x86_clflush_size * 2;
-	if (c->x86 == 6)
+	if (c->x86 == 6 || c->x86 > 15)
 		set_cpu_cap(c, X86_FEATURE_REP_GOOD);
 #else
 	/*
-- 
2.43.0

[RFC PATCH 04/15] cpufreq: Fix the efficient idle check for Intel extended families

[Sohil Mehta]

should_io_be_busy() only considers certain family 6 CPUs as having
efficient idling. However, Arjan (the original author) says that choice
was due to the lack of testing done on the old systems. He suggests to
consider all Intel processors as having efficient idle.

Extend the check to all processors starting with family 6.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 drivers/cpufreq/cpufreq_ondemand.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index a7c38b8b3e78..dfafb161f1c4 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -15,6 +15,10 @@
 #include <linux/tick.h>
 #include <linux/sched/cpufreq.h>
 
+#ifdef CONFIG_X86
+#include <asm/cpu_device_id.h>
+#endif
+
 #include "cpufreq_ondemand.h"
 
 /* On-demand governor macros */
@@ -32,8 +36,7 @@ static unsigned int default_powersave_bias;
 /*
  * Not all CPUs want IO time to be accounted as busy; this depends on how
  * efficient idling at a higher frequency/voltage is.
- * Pavel Machek says this is not so for various generations of AMD and old
- * Intel systems.
+ * Pavel Machek says this is not so for various generations of AMD.
  * Mike Chan (android.com) claims this is also not true for ARM.
  * Because of this, whitelist specific known (series) of CPUs by default, and
  * leave all others up to the user.
@@ -42,11 +45,11 @@ static int should_io_be_busy(void)
 {
 #if defined(CONFIG_X86)
 	/*
-	 * For Intel, Core 2 (model 15) and later have an efficient idle.
+	 * Starting with Family 6 consider all Intel CPUs to have an
+	 * efficient idle.
 	 */
 	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
-			boot_cpu_data.x86 == 6 &&
-			boot_cpu_data.x86_model >= 15)
+	    boot_cpu_data.x86_vfm >= INTEL_PENTIUM_PRO)
 		return 1;
 #endif
 	return 0;
-- 
2.43.0

[RFC PATCH 05/15] hwmon: Fix Intel family checks to include extended family numbers

[Sohil Mehta]

The current Intel family-model checks in the coretemp driver seem to
implicitly assume family 6. Extend the checks to include the extended
family numbers beyond 15 as well.

Also, add explicit checks for family 6 in places where it is assumed
implicitly.

x86_model checks seem inconsistent and scattered throughout the driver.
Consolidating and converting them to VFM ones would be a useful addition
in future.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 drivers/hwmon/coretemp.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index 1b9203b20d70..1aa67a2b5f18 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -185,6 +185,13 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev)
 			return tjmax_table[i].tjmax;
 	}
 
+	/*
+	 * Return without adjustment if the Family isn't 6.
+	 * The rest of the function assumes Family 6.
+	 */
+	if (c->x86 != 6)
+		return tjmax;
+
 	for (i = 0; i < ARRAY_SIZE(tjmax_model_table); i++) {
 		const struct tjmax_model *tm = &tjmax_model_table[i];
 		if (c->x86_model == tm->model &&
@@ -260,14 +267,17 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev)
 
 static bool cpu_has_tjmax(struct cpuinfo_x86 *c)
 {
+	u8 family = c->x86;
 	u8 model = c->x86_model;
 
-	return model > 0xe &&
-	       model != 0x1c &&
-	       model != 0x26 &&
-	       model != 0x27 &&
-	       model != 0x35 &&
-	       model != 0x36;
+	return family > 15 ||
+	       (family == 6 &&
+		model > 0xe &&
+		model != 0x1c &&
+		model != 0x26 &&
+		model != 0x27 &&
+		model != 0x35 &&
+		model != 0x36);
 }
 
 static int get_tjmax(struct temp_data *tdata, struct device *dev)
@@ -460,7 +470,7 @@ static int chk_ucode_version(unsigned int cpu)
 	 * Readings might stop update when processor visited too deep sleep,
 	 * fixed for stepping D0 (6EC).
 	 */
-	if (c->x86_model == 0xe && c->x86_stepping < 0xc && c->microcode < 0x39) {
+	if (c->x86 == 6 && c->x86_model == 0xe && c->x86_stepping < 0xc && c->microcode < 0x39) {
 		pr_err("Errata AE18 not fixed, update BIOS or microcode of the CPU!\n");
 		return -ENODEV;
 	}
@@ -580,7 +590,7 @@ static int create_core_data(struct platform_device *pdev, unsigned int cpu,
 	 * MSR_IA32_TEMPERATURE_TARGET register. Atoms don't have the register
 	 * at all.
 	 */
-	if (c->x86_model > 0xe && c->x86_model != 0x1c)
+	if (c->x86 > 15 || (c->x86 == 6 && c->x86_model > 0xe && c->x86_model != 0x1c))
 		if (get_ttarget(tdata, &pdev->dev) >= 0)
 			tdata->attr_size++;
 
-- 
2.43.0

[RFC PATCH 06/15] x86/microcode: Update the Intel processor flag scan check

[Sohil Mehta]

The check whether to read IA32_PLATFORM_ID MSR is misleading. It doesn't
seem to consider family while comparing the model number. This works
because init_intel_microcode() bails out if the processor family is less
than 6. It is better to update the current check to specifically include
family 6.

Ideally, a VFM check would make it more readable. But, there isn't a
macro to derive VFM from sig.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/kernel/cpu/microcode/intel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index f3d534807d91..734819a12d5f 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -74,7 +74,8 @@ void intel_collect_cpu_info(struct cpu_signature *sig)
 	sig->pf = 0;
 	sig->rev = intel_get_microcode_revision();
 
-	if (x86_model(sig->sig) >= 5 || x86_family(sig->sig) > 6) {
+	/* TODO: Simplify this using a VFM check? */
+	if ((x86_family(sig->sig) == 6 && x86_model(sig->sig) >= 5) || x86_family(sig->sig) > 6) {
 		unsigned int val[2];
 
 		/* get processor flags from MSR 0x17 */
-- 
2.43.0

[RFC PATCH 07/15] x86/mtrr: Modify a x86_model check to an Intel VFM check

[Sohil Mehta]

Get rid of one of the last few Intel x86_model checks in arch/x86.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/kernel/cpu/mtrr/generic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index 7b29ebda024f..36f5dbd2f482 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -12,6 +12,7 @@
 #include <asm/processor-flags.h>
 #include <asm/cacheinfo.h>
 #include <asm/cpufeature.h>
+#include <asm/cpu_device_id.h>
 #include <asm/hypervisor.h>
 #include <asm/mshyperv.h>
 #include <asm/tlbflush.h>
@@ -1025,8 +1026,7 @@ int generic_validate_add_page(unsigned long base, unsigned long size,
 	 * For Intel PPro stepping <= 7
 	 * must be 4 MiB aligned and not touch 0x70000000 -> 0x7003FFFF
 	 */
-	if (mtrr_if == &generic_mtrr_ops && boot_cpu_data.x86 == 6 &&
-	    boot_cpu_data.x86_model == 1 &&
+	if (mtrr_if == &generic_mtrr_ops && boot_cpu_data.x86_vfm == INTEL_PENTIUM_PRO &&
 	    boot_cpu_data.x86_stepping <= 7) {
 		if (base & ((1 << (22 - PAGE_SHIFT)) - 1)) {
 			pr_warn("mtrr: base(0x%lx000) is not 4 MiB aligned\n", base);
-- 
2.43.0

[RFC PATCH 08/15] x86/cpu/intel: Replace early family 6 checks with VFM ones

[Sohil Mehta]

Introduce names for some old pentium models and replace with VFM based
checks.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/include/asm/intel-family.h |  3 +++
 arch/x86/kernel/cpu/intel.c         | 13 ++++++-------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
index 6d7b04ffc5fd..41278093bcd4 100644
--- a/arch/x86/include/asm/intel-family.h
+++ b/arch/x86/include/asm/intel-family.h
@@ -46,6 +46,9 @@
 #define INTEL_ANY			IFM(X86_FAMILY_ANY, X86_MODEL_ANY)
 
 #define INTEL_PENTIUM_PRO		IFM(6, 0x01)
+#define INTEL_PENTIUM_II_KLAMATH	IFM(6, 0x03)
+#define INTEL_PENTIUM_III_TUALATIN	IFM(6, 0x0B)
+#define INTEL_PENTIUM_M_DOTHAN		IFM(6, 0x0D)
 
 #define INTEL_CORE_YONAH		IFM(6, 0x0E)
 
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index f44b2e618fb3..cde8f45ccd57 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -195,7 +195,7 @@ void intel_unlock_cpuid_leafs(struct cpuinfo_x86 *c)
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
 		return;
 
-	if (c->x86 < 6 || (c->x86 == 6 && c->x86_model < 0xd))
+	if (c->x86_vfm < INTEL_PENTIUM_M_DOTHAN)
 		return;
 
 	/*
@@ -301,7 +301,7 @@ static void early_init_intel(struct cpuinfo_x86 *c)
 	 * If fast string is not enabled in IA32_MISC_ENABLE for any reason,
 	 * clear the fast string and enhanced fast string CPU capabilities.
 	 */
-	if (c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xd)) {
+	if (c->x86_vfm >= INTEL_PENTIUM_M_DOTHAN) {
 		rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
 		if (!(misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING)) {
 			pr_info("Disabled fast string operations\n");
@@ -350,9 +350,7 @@ static void bsp_init_intel(struct cpuinfo_x86 *c)
 int ppro_with_ram_bug(void)
 {
 	/* Uses data from early_cpu_detect now */
-	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
-	    boot_cpu_data.x86 == 6 &&
-	    boot_cpu_data.x86_model == 1 &&
+	if (boot_cpu_data.x86_vfm == INTEL_PENTIUM_PRO &&
 	    boot_cpu_data.x86_stepping < 8) {
 		pr_info("Pentium Pro with Errata#50 detected. Taking evasive action.\n");
 		return 1;
@@ -413,7 +411,8 @@ static void intel_workarounds(struct cpuinfo_x86 *c)
 	 * SEP CPUID bug: Pentium Pro reports SEP but doesn't have it until
 	 * model 3 mask 3
 	 */
-	if ((c->x86<<8 | c->x86_model<<4 | c->x86_stepping) < 0x633)
+	if ((c->x86_vfm == INTEL_PENTIUM_II_KLAMATH && c->x86_stepping < 3) ||
+	    c->x86_vfm < INTEL_PENTIUM_II_KLAMATH)
 		clear_cpu_cap(c, X86_FEATURE_SEP);
 
 	/*
@@ -620,7 +619,7 @@ static unsigned int intel_size_cache(struct cpuinfo_x86 *c, unsigned int size)
 	 * to determine which, so we use a boottime override
 	 * for the 512kb model, and assume 256 otherwise.
 	 */
-	if ((c->x86 == 6) && (c->x86_model == 11) && (size == 0))
+	if (c->x86_vfm == INTEL_PENTIUM_III_TUALATIN && size == 0)
 		size = 256;
 
 	/*
-- 
2.43.0

[RFC PATCH 09/15] x86/cpu/intel: Replace family 15 checks with VFM ones

[Sohil Mehta]

Introduce names for some old pentium 4 models and replace with VFM based
checks.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/include/asm/intel-family.h | 4 ++++
 arch/x86/kernel/cpu/intel.c         | 6 +++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
index 41278093bcd4..79cfd42c849f 100644
--- a/arch/x86/include/asm/intel-family.h
+++ b/arch/x86/include/asm/intel-family.h
@@ -182,6 +182,10 @@
 /* Family 5 */
 #define INTEL_QUARK_X1000		IFM(5, 0x09) /* Quark X1000 SoC */
 
+/* Family 15 */
+#define INTEL_P4_WILLAMETTE		IFM(15, 0x01) /* Also Xeon Foster */
+#define INTEL_P4_PRESCOTT		IFM(15, 0x03)
+
 /* Family 19 */
 #define INTEL_PANTHERCOVE_X		IFM(19, 0x01) /* Diamond Rapids */
 
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index cde8f45ccd57..26221f38db70 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -256,8 +256,8 @@ static void early_init_intel(struct cpuinfo_x86 *c)
 #endif
 
 	/* CPUID workaround for 0F33/0F34 CPU */
-	if (c->x86 == 0xF && c->x86_model == 0x3
-	    && (c->x86_stepping == 0x3 || c->x86_stepping == 0x4))
+	if (c->x86_vfm == INTEL_P4_PRESCOTT &&
+	    (c->x86_stepping == 0x3 || c->x86_stepping == 0x4))
 		c->x86_phys_bits = 36;
 
 	/*
@@ -430,7 +430,7 @@ static void intel_workarounds(struct cpuinfo_x86 *c)
 	 * P4 Xeon erratum 037 workaround.
 	 * Hardware prefetcher may cause stale data to be loaded into the cache.
 	 */
-	if ((c->x86 == 15) && (c->x86_model == 1) && (c->x86_stepping == 1)) {
+	if (c->x86_vfm == INTEL_P4_WILLAMETTE && c->x86_stepping == 1) {
 		if (msr_set_bit(MSR_IA32_MISC_ENABLE,
 				MSR_IA32_MISC_ENABLE_PREFETCH_DISABLE_BIT) > 0) {
 			pr_info("CPU: C0 stepping P4 Xeon detected.\n");
-- 
2.43.0

[RFC PATCH 10/15] x86/cpu/intel: Replace family 5 model checks with VFM ones

[Sohil Mehta]

Introduce names for some family 5 models and convert VFM based checks.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/include/asm/intel-family.h |  3 +++
 arch/x86/kernel/cpu/intel.c         | 11 +++++------
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
index 79cfd42c849f..025d091be98e 100644
--- a/arch/x86/include/asm/intel-family.h
+++ b/arch/x86/include/asm/intel-family.h
@@ -180,6 +180,9 @@
 #define INTEL_XEON_PHI_KNM		IFM(6, 0x85) /* Knights Mill */
 
 /* Family 5 */
+#define INTEL_FAM5_START		IFM(5, 0x00) /* Notational marker, also P5 A-step */
+#define INTEL_PENTIUM_75		IFM(5, 0x02) /* P54C - Need a better name */
+#define INTEL_PENTIUM_MMX		IFM(5, 0x04) /* P55C */
 #define INTEL_QUARK_X1000		IFM(5, 0x09) /* Quark X1000 SoC */
 
 /* Family 15 */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 26221f38db70..26962a602e86 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -367,9 +367,8 @@ static void intel_smp_check(struct cpuinfo_x86 *c)
 	/*
 	 * Mask B, Pentium, but not Pentium MMX
 	 */
-	if (c->x86 == 5 &&
-	    c->x86_stepping >= 1 && c->x86_stepping <= 4 &&
-	    c->x86_model <= 3) {
+	if (c->x86_vfm >= INTEL_FAM5_START && c->x86_vfm < INTEL_PENTIUM_MMX &&
+	    c->x86_stepping >= 1 && c->x86_stepping <= 4) {
 		/*
 		 * Remember we have B step Pentia with bugs
 		 */
@@ -396,7 +395,7 @@ static void intel_workarounds(struct cpuinfo_x86 *c)
 	 * The Quark is also family 5, but does not have the same bug.
 	 */
 	clear_cpu_bug(c, X86_BUG_F00F);
-	if (c->x86 == 5 && c->x86_model < 9) {
+	if (c->x86_vfm >= INTEL_FAM5_START && c->x86_vfm < INTEL_QUARK_X1000) {
 		static int f00f_workaround_enabled;
 
 		set_cpu_bug(c, X86_BUG_F00F);
@@ -444,7 +443,7 @@ static void intel_workarounds(struct cpuinfo_x86 *c)
 	 * integrated APIC (see 11AP erratum in "Pentium Processor
 	 * Specification Update").
 	 */
-	if (boot_cpu_has(X86_FEATURE_APIC) && (c->x86<<8 | c->x86_model<<4) == 0x520 &&
+	if (boot_cpu_has(X86_FEATURE_APIC) && c->x86_vfm == INTEL_PENTIUM_75 &&
 	    (c->x86_stepping < 0x6 || c->x86_stepping == 0xb))
 		set_cpu_bug(c, X86_BUG_11AP);
 
@@ -626,7 +625,7 @@ static unsigned int intel_size_cache(struct cpuinfo_x86 *c, unsigned int size)
 	 * Intel Quark SoC X1000 contains a 4-way set associative
 	 * 16K cache with a 16 byte cache line and 256 lines per tag
 	 */
-	if ((c->x86 == 5) && (c->x86_model == 9))
+	if (c->x86_vfm == INTEL_QUARK_X1000)
 		size = 16;
 	return size;
 }
-- 
2.43.0

[RFC PATCH 11/15] x86/pat: Replace Intel model checks with VFM ones

[Sohil Mehta]

Introduce markers and names for some family 6 and family 15  models and
replace with VFM based checks.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/include/asm/intel-family.h | 2 ++
 arch/x86/mm/pat/memtype.c           | 7 ++++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
index 025d091be98e..76a184361930 100644
--- a/arch/x86/include/asm/intel-family.h
+++ b/arch/x86/include/asm/intel-family.h
@@ -186,8 +186,10 @@
 #define INTEL_QUARK_X1000		IFM(5, 0x09) /* Quark X1000 SoC */
 
 /* Family 15 */
+#define INTEL_FAM15_START		IFM(15, 0x00) /* Notational marker */
 #define INTEL_P4_WILLAMETTE		IFM(15, 0x01) /* Also Xeon Foster */
 #define INTEL_P4_PRESCOTT		IFM(15, 0x03)
+#define INTEL_P4_CEDARMILL		IFM(15, 0x06) /* Also Xeon Dempsey */
 
 /* Family 19 */
 #define INTEL_PANTHERCOVE_X		IFM(19, 0x01) /* Diamond Rapids */
diff --git a/arch/x86/mm/pat/memtype.c b/arch/x86/mm/pat/memtype.c
index feb8cc6a12bf..e6d7dc608b77 100644
--- a/arch/x86/mm/pat/memtype.c
+++ b/arch/x86/mm/pat/memtype.c
@@ -43,6 +43,7 @@
 #include <linux/fs.h>
 #include <linux/rbtree.h>
 
+#include <asm/cpu_device_id.h>
 #include <asm/cacheflush.h>
 #include <asm/cacheinfo.h>
 #include <asm/processor.h>
@@ -290,9 +291,9 @@ void __init pat_bp_init(void)
 		return;
 	}
 
-	if ((c->x86_vendor == X86_VENDOR_INTEL) &&
-	    (((c->x86 == 0x6) && (c->x86_model <= 0xd)) ||
-	     ((c->x86 == 0xf) && (c->x86_model <= 0x6)))) {
+	if (c->x86_vendor == X86_VENDOR_INTEL &&
+	    ((c->x86_vfm >= INTEL_PENTIUM_PRO && c->x86_vfm <= INTEL_PENTIUM_M_DOTHAN) ||
+	    (c->x86_vfm > INTEL_FAM15_START && c->x86_vfm <= INTEL_P4_CEDARMILL))) {
 		/*
 		 * PAT support with the lower four entries. Intel Pentium 2,
 		 * 3, M, and 4 are affected by PAT errata, which makes the
-- 
2.43.0

[RFC PATCH 12/15] x86/acpi/cstate: Improve Intel family model checks

[Sohil Mehta]

Update the Intel family checks to consistently use Family 15 instead of
Family 0xF. Also, get rid of one of last usages of x86_model by using
the new VFM checks.

Update the incorrect comment since the check has changed since the
initial commit ee1ca48fae7e ("ACPI: Disable ARB_DISABLE on platforms
where it is not needed").

commit 3e2ada5867b7 ("ACPI: fix Compaq Evo N800c (Pentium 4m) boot hang
regression") removed the P4s and commit 03a05ed11529 ("ACPI: Use the
ARB_DISABLE for the CPU which model id is less than 0x0f.") got rid of
CORE_YONAH.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/include/asm/intel-family.h | 3 +++
 arch/x86/kernel/acpi/cstate.c       | 8 ++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
index 76a184361930..73e458440fcb 100644
--- a/arch/x86/include/asm/intel-family.h
+++ b/arch/x86/include/asm/intel-family.h
@@ -179,6 +179,9 @@
 #define INTEL_XEON_PHI_KNL		IFM(6, 0x57) /* Knights Landing */
 #define INTEL_XEON_PHI_KNM		IFM(6, 0x85) /* Knights Mill */
 
+/* Notational marker denoting the last Family 6 model */
+#define INTEL_FAM6_LAST		        IFM(6, 0xFF)
+
 /* Family 5 */
 #define INTEL_FAM5_START		IFM(5, 0x00) /* Notational marker, also P5 A-step */
 #define INTEL_PENTIUM_75		IFM(5, 0x02) /* P54C - Need a better name */
diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c
index f3ffd0a3a012..6d87d1ebe89b 100644
--- a/arch/x86/kernel/acpi/cstate.c
+++ b/arch/x86/kernel/acpi/cstate.c
@@ -13,6 +13,7 @@
 #include <linux/sched.h>
 
 #include <acpi/processor.h>
+#include <asm/cpu_device_id.h>
 #include <asm/mwait.h>
 #include <asm/special_insns.h>
 
@@ -46,12 +47,11 @@ void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
 	/*
 	 * On all recent Intel platforms, ARB_DISABLE is a nop.
 	 * So, set bm_control to zero to indicate that ARB_DISABLE
-	 * is not required while entering C3 type state on
-	 * P4, Core and beyond CPUs
+	 * is not required while entering C3 type state.
 	 */
 	if (c->x86_vendor == X86_VENDOR_INTEL &&
-	    (c->x86 > 0xf || (c->x86 == 6 && c->x86_model >= 0x0f)))
-			flags->bm_control = 0;
+	    (c->x86 > 15 || (c->x86_vfm >= INTEL_CORE2_MEROM && c->x86_vfm <= INTEL_FAM6_LAST)))
+		flags->bm_control = 0;
 
 	if (c->x86_vendor == X86_VENDOR_CENTAUR) {
 		if (c->x86 > 6 || (c->x86 == 6 && c->x86_model == 0x0f &&
-- 
2.43.0

[RFC PATCH 13/15] x86/cpu/intel: Bound the non-architectural constant_tsc model checks

[Sohil Mehta]

Constant TSC has been architectural on Intel CPUs for a while. Supported
CPUs use the architectural Invariant TSC bit in CPUID.80000007. A family
model check is not required for such CPUs.

Prevent unnecessary confusion but restricting the checks and moving it
closer to the architectural check.

Invariant TSC was likely introduced around the Nehalam timeframe on the
Xeon side and Saltwell timeframe on the Atom side. (Needs confirmation)
Due to interspersed model numbers use an Ivybridge model number check to
be safe.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/kernel/cpu/intel.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 26962a602e86..d37ef3a72234 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -210,10 +210,6 @@ static void early_init_intel(struct cpuinfo_x86 *c)
 {
 	u64 misc_enable;
 
-	if ((c->x86 == 0xf && c->x86_model >= 0x03) ||
-		(c->x86 == 0x6 && c->x86_model >= 0x0e))
-		set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
-
 	if (c->x86 >= 6 && !cpu_has(c, X86_FEATURE_IA64))
 		c->microcode = intel_get_microcode_revision();
 
@@ -272,6 +268,11 @@ static void early_init_intel(struct cpuinfo_x86 *c)
 		set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
 	}
 
+	/* Some older CPUs have invariant TSC but may not report it architecturally via 8000_0007 */
+	if ((c->x86_vfm >= INTEL_P4_PRESCOTT && c->x86_vfm <= INTEL_P4_WILLAMETTE) ||
+	    (c->x86_vfm >= INTEL_CORE_YONAH && c->x86_vfm <= INTEL_IVYBRIDGE))
+		set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
+
 	/* Penwell and Cloverview have the TSC which doesn't sleep on S3 */
 	switch (c->x86_vfm) {
 	case INTEL_ATOM_SALTWELL_MID:
-- 
2.43.0

[RFC PATCH 14/15] perf/x86: Simplify p6_pmu_init()

[Sohil Mehta]

A switch case is unnecessary when only a single case matters. Also, the
gaps in the case numbers are due to no CPU with those model numbers
being released.

Avoid the switch case and combine the cases into simpler VFM checks.
Also, this gets rid of one last few Intel x86_model comparisons. No
functional change intended.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/events/intel/p6.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/arch/x86/events/intel/p6.c b/arch/x86/events/intel/p6.c
index a6cffb4f4ef5..37e3beb6d633 100644
--- a/arch/x86/events/intel/p6.c
+++ b/arch/x86/events/intel/p6.c
@@ -2,6 +2,8 @@
 #include <linux/perf_event.h>
 #include <linux/types.h>
 
+#include <asm/cpu_device_id.h>
+
 #include "../perf_event.h"
 
 /*
@@ -244,35 +246,19 @@ static __init void p6_pmu_rdpmc_quirk(void)
 	}
 }
 
+/* Only called for Family 6 CPUs without X86_FEATURE_ARCH_PERFMON */
 __init int p6_pmu_init(void)
 {
 	x86_pmu = p6_pmu;
 
-	switch (boot_cpu_data.x86_model) {
-	case  1: /* Pentium Pro */
-		x86_add_quirk(p6_pmu_rdpmc_quirk);
-		break;
-
-	case  3: /* Pentium II - Klamath */
-	case  5: /* Pentium II - Deschutes */
-	case  6: /* Pentium II - Mendocino */
-		break;
-
-	case  7: /* Pentium III - Katmai */
-	case  8: /* Pentium III - Coppermine */
-	case 10: /* Pentium III Xeon */
-	case 11: /* Pentium III - Tualatin */
-		break;
-
-	case  9: /* Pentium M - Banias */
-	case 13: /* Pentium M - Dothan */
-		break;
-
-	default:
+	if (boot_cpu_data.x86_vfm >= INTEL_CORE_YONAH) {
 		pr_cont("unsupported p6 CPU model %d ", boot_cpu_data.x86_model);
 		return -ENODEV;
 	}
 
+	if (boot_cpu_data.x86_vfm == INTEL_PENTIUM_PRO)
+		x86_add_quirk(p6_pmu_rdpmc_quirk);
+
 	memcpy(hw_cache_event_ids, p6_hw_cache_event_ids,
 		sizeof(hw_cache_event_ids));
 
-- 
2.43.0

[RFC PATCH 15/15] perf/x86/p4: Replace Pentium 4 model checks with VFM ones

[Sohil Mehta]

Introduce names for some old pentium 4 models and replace with VFM based
checks.

Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
 arch/x86/events/intel/p4.c          | 7 ++++---
 arch/x86/include/asm/intel-family.h | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/p4.c b/arch/x86/events/intel/p4.c
index 844bc4fc4724..fb726c6fc6e7 100644
--- a/arch/x86/events/intel/p4.c
+++ b/arch/x86/events/intel/p4.c
@@ -10,6 +10,7 @@
 #include <linux/perf_event.h>
 
 #include <asm/perf_event_p4.h>
+#include <asm/cpu_device_id.h>
 #include <asm/hardirq.h>
 #include <asm/apic.h>
 
@@ -732,9 +733,9 @@ static bool p4_event_match_cpu_model(unsigned int event_idx)
 {
 	/* INSTR_COMPLETED event only exist for model 3, 4, 6 (Prescott) */
 	if (event_idx == P4_EVENT_INSTR_COMPLETED) {
-		if (boot_cpu_data.x86_model != 3 &&
-			boot_cpu_data.x86_model != 4 &&
-			boot_cpu_data.x86_model != 6)
+		if (boot_cpu_data.x86_vfm != INTEL_P4_PRESCOTT &&
+		    boot_cpu_data.x86_vfm != INTEL_P4_PRESCOTT_2M &&
+		    boot_cpu_data.x86_vfm != INTEL_P4_CEDARMILL)
 			return false;
 	}
 
diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
index 73e458440fcb..77840c0b0df3 100644
--- a/arch/x86/include/asm/intel-family.h
+++ b/arch/x86/include/asm/intel-family.h
@@ -192,6 +192,7 @@
 #define INTEL_FAM15_START		IFM(15, 0x00) /* Notational marker */
 #define INTEL_P4_WILLAMETTE		IFM(15, 0x01) /* Also Xeon Foster */
 #define INTEL_P4_PRESCOTT		IFM(15, 0x03)
+#define INTEL_P4_PRESCOTT_2M		IFM(15, 0x04)
 #define INTEL_P4_CEDARMILL		IFM(15, 0x06) /* Also Xeon Dempsey */
 
 /* Family 19 */
-- 
2.43.0

Re: [RFC PATCH 01/15] x86/apic: Fix 32-bit APIC initialization for extended Intel families

[Dave Hansen]

On 12/20/24 13:36, Sohil Mehta wrote:
> detect_init_APIC() limits the APIC detection to families 6 and 15.
> Extend the check to family numbers beyond 15. Also, convert it to a VFM
> check to make it simpler.

This changelog doesn't _quite_ fit the code. I'd rather it was a bit
more precise. It does not _just_ expand to "numbers beyond 15".

Also, It doesn't _really_ help to have the changelog tell us the
function name. That's why we have diff -p.  How about:

	APIC detection is currently limited to a few specific families
	and will not match the upcoming families >=18.

	Extend the check to include all families 6 or greater. Also
	convert it to a VFM check to make it simpler.

>  arch/x86/kernel/apic/apic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index c5fb28e6451a..13dac8f78213 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -2014,8 +2014,8 @@ static bool __init detect_init_APIC(void)
>  	case X86_VENDOR_HYGON:
>  		break;
>  	case X86_VENDOR_INTEL:
> -		if (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15 ||
> -		    (boot_cpu_data.x86 == 5 && boot_cpu_has(X86_FEATURE_APIC)))
> +		if ((boot_cpu_data.x86 == 5 && boot_cpu_has(X86_FEATURE_APIC)) ||
> +		    boot_cpu_data.x86_vfm >= INTEL_PENTIUM_PRO)
>  			break;
>  		goto no_apic;
>  	default:

With that changelog tweak:

Acked-by: Dave Hansen <dave.hansen@linux.intel.com>

Re: [RFC PATCH 02/15] x86/apic: Fix smp init delay for extended Intel families

[Dave Hansen]

On 12/20/24 13:36, Sohil Mehta wrote:
> The MP specification version 1.4 references the i486 and early Pentium
> processors in family 5. 

Can you please elaborate on how this reference is relevant to the patch
at hand?

> However, all processors starting with family 6 likely do not need the
> 10 msec INIT delay. The omission of the Pentium 4s (family 15) seems
> like an oversight in the original check.
> 
> With some risk, choose a simpler check and extend the quirk to all
> recent and upcoming Intel processors.

I'm struggling to follow this a bit.

I think these are the facts that matter:

 * init_udelay=0 means "no quirk"
 * Modern CPUs don't have the quirk
 * The current check says "only family 6 is modern"
 * Family 15 is _probably_ modern and just forgotten about

And this is what you're doing in the end:

Consider everything PPro and later to be modern, including all of
families 6, 15 and the new 18/19 CPUs.

Right?

Re: [RFC PATCH 03/15] x86/cpu/intel: Fix init_intel() checks for extended family numbers

[Dave Hansen]

On 12/20/24 13:36, Sohil Mehta wrote:
> X86_FEATURE_REP_GOOD is only set for family 6 processors.  Extend the
> check to family numbers beyond 15.

Could you explain why, please?

> It is uncertain whether the Pentium 4s (family 15) should set the
> feature flag as well. Commit 185f3b9da24c ("x86: make intel.c have
> 64-bit support code") that originally set X86_FEATURE_REP_GOOD also set
> the x86_cache_alignment preference for family 15 processors. The
> omission of the family 15 seems intentional.
> 
> Also, the 32-bit user copy alignment preference is only set for family 6
> and 15 processors. Extend the preference to family numbers beyond 15.

Can you please provide some more context so it's clear which hunk this
refers to?  Alternatively, can you break this out into a separate patch?

Re: [RFC PATCH 00/15] Prepare for new Intel family models

[Andrew Cooper]

> ---Noteworthy quirks---
> Pentium II Overdrive - A unique family number:
>   Wikipedia says[2], In Intel's "Family/Model/Stepping" scheme, the Pentium II
>   OverDrive CPU identifies itself as family 6, model 3, though this is 
>   misleading, as it is not based on the family 6/model 3 Klamath core. As 
>   mentioned in the Pentium II Processor update documentation from Intel, 
>   "although this processor has a CPUID of 163xh, it uses a Pentium II processor 
>   CPUID 065xh processor core."
>
>   A dump of the microcode file 06-03-02 shows:
>     001/001: sig 0x00001632, pf_mask 0x00, 1998-06-10, rev 0x0002, size 2048
>   An archived CPUID dump [3] also says:
>     CPUID 00000001: 00001632-00000000-00000000-0183FBFF
>
>   That would translate to a family number of 22 (0x16). This aberration is 
>   not explicitly handled anywhere in the kernel so the platform might already 
>   be broken. This series might make it worse for the platform if the latest 
>   kernel works on it by chance.

Are you sure?  Bits 13:12 are the type field, and the 0x1 you've got is
for an OverDrive processor.

x86_family() will consider this to be family 6 as far as I can see.

~Andrew

Re: [RFC PATCH 06/15] x86/microcode: Update the Intel processor flag scan check

[Borislav Petkov]

On Fri, Dec 20, 2024 at 09:37:01PM +0000, Sohil Mehta wrote:
> The check whether to read IA32_PLATFORM_ID MSR is misleading. It doesn't
> seem to consider family while comparing the model number. This works
> because init_intel_microcode() bails out if the processor family is less
> than 6. It is better to update the current check to specifically include
> family 6.
> 
> Ideally, a VFM check would make it more readable. But, there isn't a
> macro to derive VFM from sig.
> 
> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
> ---
>  arch/x86/kernel/cpu/microcode/intel.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index f3d534807d91..734819a12d5f 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -74,7 +74,8 @@ void intel_collect_cpu_info(struct cpu_signature *sig)
>  	sig->pf = 0;
>  	sig->rev = intel_get_microcode_revision();
>  
> -	if (x86_model(sig->sig) >= 5 || x86_family(sig->sig) > 6) {
> +	/* TODO: Simplify this using a VFM check? */

No TODOs. Take your time and do it right from the get-go please.

-- 
Regards/Gruss,
    Boris.

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

Re: [RFC PATCH 08/15] x86/cpu/intel: Replace early family 6 checks with VFM ones

[David Laight]

On Fri, 20 Dec 2024 21:37:03 +0000
Sohil Mehta <sohil.mehta@intel.com> wrote:

> Introduce names for some old pentium models and replace with VFM based
> checks.
> 
> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
> ---
>  arch/x86/include/asm/intel-family.h |  3 +++
>  arch/x86/kernel/cpu/intel.c         | 13 ++++++-------
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
> index 6d7b04ffc5fd..41278093bcd4 100644
> --- a/arch/x86/include/asm/intel-family.h
> +++ b/arch/x86/include/asm/intel-family.h
> @@ -46,6 +46,9 @@
>  #define INTEL_ANY			IFM(X86_FAMILY_ANY, X86_MODEL_ANY)
>  
>  #define INTEL_PENTIUM_PRO		IFM(6, 0x01)
> +#define INTEL_PENTIUM_II_KLAMATH	IFM(6, 0x03)
> +#define INTEL_PENTIUM_III_TUALATIN	IFM(6, 0x0B)
> +#define INTEL_PENTIUM_M_DOTHAN		IFM(6, 0x0D)
>  
>  #define INTEL_CORE_YONAH		IFM(6, 0x0E)

I think I'd add all the extra defines in one patch.

Isn't the order 'funny' as well.
I think it ends up being neither numeric or date order.
Increasing family order is probably best, but the headings for each
'family' probably need to be a bit more descriptive (esp 15 - all P4
netburst).

	David

Re: [RFC PATCH 06/15] x86/microcode: Update the Intel processor flag scan check

[Dave Hansen]

On 12/20/24 13:37, Sohil Mehta wrote:
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -74,7 +74,8 @@ void intel_collect_cpu_info(struct cpu_signature *sig)
>  	sig->pf = 0;
>  	sig->rev = intel_get_microcode_revision();
>  
> -	if (x86_model(sig->sig) >= 5 || x86_family(sig->sig) > 6) {
> +	/* TODO: Simplify this using a VFM check? */
> +	if ((x86_family(sig->sig) == 6 && x86_model(sig->sig) >= 5) || x86_family(sig->sig) > 6) {
>  		unsigned int val[2];
>  
>  		/* get processor flags from MSR 0x17 */

I suspect this code is kinda bogus in the first place. sig->sig is just
cpuid_eax(1) and:

	void cpu_detect(struct cpuinfo_x86 *c)
	{
		...
                cpuid(0x00000001, &tfms, &misc, &junk, &cap0);
                c->x86          = x86_family(tfms);

So I'm not quite sure why this code feels the need to redo CPUID and
re-parse it. Other bits of the microcode update may need 'sig' in its
unparsed form to conveniently compare with parts of the microcode image,
but that's no reason to re-parse it too.

I _think_ this code can just use good old cpu_data() and the existing
VFM mechanism.

Re: [RFC PATCH 08/15] x86/cpu/intel: Replace early family 6 checks with VFM ones

[Dave Hansen]

On 12/21/24 02:35, David Laight wrote:
>>  #define INTEL_PENTIUM_PRO		IFM(6, 0x01)
>> +#define INTEL_PENTIUM_II_KLAMATH	IFM(6, 0x03)
>> +#define INTEL_PENTIUM_III_TUALATIN	IFM(6, 0x0B)
>> +#define INTEL_PENTIUM_M_DOTHAN		IFM(6, 0x0D)
>>  
>>  #define INTEL_CORE_YONAH		IFM(6, 0x0E)
> 
> I think I'd add all the extra defines in one patch.

I actually prefer them being with their first user. That way, if patches
get dropped or moved, nothing can be in the wrong order or unnecessary.

> Isn't the order 'funny' as well.
> I think it ends up being neither numeric or date order.
> Increasing family order is probably best, but the headings for each
> 'family' probably need to be a bit more descriptive (esp 15 - all P4
> netburst).

<shrug>

They're already out of order because Quark is a weirdo and we didn't
want to look at it all the time so it got stuck at the bottom. It's
_fine_ when it's 99% family 6 plus 2 other weirdos.

But it should probably get sorted at some point because it's going to be
hard to find Quark and P4 if the order is

	fam 6,1
	fam 5
	fam 15
	fam 18
	fam 19
	fam 20
	...

But I think all that has to be done is to move Quark up to the top.

Re: [RFC PATCH 08/15] x86/cpu/intel: Replace early family 6 checks with VFM ones

[David Laight]

On Sat, 21 Dec 2024 07:57:10 -0800
Dave Hansen <dave.hansen@intel.com> wrote:

> On 12/21/24 02:35, David Laight wrote:
> >>  #define INTEL_PENTIUM_PRO		IFM(6, 0x01)
> >> +#define INTEL_PENTIUM_II_KLAMATH	IFM(6, 0x03)
> >> +#define INTEL_PENTIUM_III_TUALATIN	IFM(6, 0x0B)
> >> +#define INTEL_PENTIUM_M_DOTHAN		IFM(6, 0x0D)
> >>  
> >>  #define INTEL_CORE_YONAH		IFM(6, 0x0E)  
> > 
> > I think I'd add all the extra defines in one patch.  
> 
> I actually prefer them being with their first user. That way, if patches
> get dropped or moved, nothing can be in the wrong order or unnecessary.

Does 'unnecessary' matter.
It isn't as though there are hundreds of them.

	David

Re: [RFC PATCH 05/15] hwmon: Fix Intel family checks to include extended family numbers

[Guenter Roeck]

On 12/20/24 13:37, Sohil Mehta wrote:
> The current Intel family-model checks in the coretemp driver seem to
> implicitly assume family 6. Extend the checks to include the extended
> family numbers beyond 15 as well.
> 
> Also, add explicit checks for family 6 in places where it is assumed
> implicitly.
> 
> x86_model checks seem inconsistent and scattered throughout the driver.
> Consolidating and converting them to VFM ones would be a useful addition
> in future.
> 

That seems to be irrelevant for the patch description.

> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

The patch is independent, but since it is submitted as part of a series
and there is no comment suggesting otherwise, I assume that it is expected
to be pushed together with that series, and I won't take it into the hwmon
branch.

Guenter

Re: [RFC PATCH 08/15] x86/cpu/intel: Replace early family 6 checks with VFM ones

[Dave Hansen]

On 12/21/24 08:48, David Laight wrote:
> On Sat, 21 Dec 2024 07:57:10 -0800
> Dave Hansen <dave.hansen@intel.com> wrote:
> 
>> On 12/21/24 02:35, David Laight wrote:
>>>>  #define INTEL_PENTIUM_PRO		IFM(6, 0x01)
>>>> +#define INTEL_PENTIUM_II_KLAMATH	IFM(6, 0x03)
>>>> +#define INTEL_PENTIUM_III_TUALATIN	IFM(6, 0x0B)
>>>> +#define INTEL_PENTIUM_M_DOTHAN		IFM(6, 0x0D)
>>>>  
>>>>  #define INTEL_CORE_YONAH		IFM(6, 0x0E)  
>>> I think I'd add all the extra defines in one patch.  
>> I actually prefer them being with their first user. That way, if patches
>> get dropped or moved, nothing can be in the wrong order or unnecessary.
> Does 'unnecessary' matter.
> It isn't as though there are hundreds of them.

It matters to me.

I'll let Sohil make the decision if he want to follow your preferences
or mine.

Re: [RFC PATCH 05/15] hwmon: Fix Intel family checks to include extended family numbers

[Sohil Mehta]

On 12/21/2024 9:27 AM, Guenter Roeck wrote:


>> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
> 
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> 

Thank you for the Ack.

> The patch is independent, but since it is submitted as part of a series
> and there is no comment suggesting otherwise, I assume that it is expected
> to be pushed together with that series, and I won't take it into the hwmon
> branch.
> 

This first RFC version was mainly intended to be a conversation starter.
I am not very familiar with hwmon and my testing has been fairly
limited. I was hoping we can get at least one tested-by on this patch
before merging it.

I have tried to keep all the patches independent to make it easier to
merge whenever they seem ready. Please feel free to merge the patch if
it seems so.

Sohil

Re: [RFC PATCH 00/15] Prepare for new Intel family models

[Sohil Mehta]

On 12/20/2024 4:29 PM, Andrew Cooper wrote:
>>
>>   A dump of the microcode file 06-03-02 shows:
>>     001/001: sig 0x00001632, pf_mask 0x00, 1998-06-10, rev 0x0002, size 2048
>>   An archived CPUID dump [3] also says:
>>     CPUID 00000001: 00001632-00000000-00000000-0183FBFF
>>
>>   That would translate to a family number of 22 (0x16). This aberration is 
>>   not explicitly handled anywhere in the kernel so the platform might already 
>>   be broken. This series might make it worse for the platform if the latest 
>>   kernel works on it by chance.
> 
> Are you sure?  Bits 13:12 are the type field, and the 0x1 you've got is
> for an OverDrive processor.
> 

You are right. This is indeed the processor type field. I should have
spent more time reading the specification than Wikipedia :)


> x86_family() will consider this to be family 6 as far as I can see.
> 

On the bright side, we don't have to worry about breaking this CPU.

Sohil

Re: [RFC PATCH 06/15] x86/microcode: Update the Intel processor flag scan check

[Sohil Mehta]

On 12/21/2024 1:11 AM, Borislav Petkov wrote:
>>  
>> -	if (x86_model(sig->sig) >= 5 || x86_family(sig->sig) > 6) {
>> +	/* TODO: Simplify this using a VFM check? */
> 
> No TODOs. Take your time and do it right from the get-go please.
> 

Sorry, this sneaked through. I was supposed to modify the comment before
sending it out. For this RFC, I was looking for suggestions since there
didn't seem a straight forward way to convert sig to VFM.

But as Dave mentioned in the other thread, maybe we don't need to redo
CPUID in the first place and this code can reuse cpu_data(). I'll
explore that path first.

Sohil

Re: [RFC PATCH 08/15] x86/cpu/intel: Replace early family 6 checks with VFM ones

[Sohil Mehta]

On 12/21/2024 7:57 AM, Dave Hansen wrote:
>> Isn't the order 'funny' as well.
>> I think it ends up being neither numeric or date order.
>> Increasing family order is probably best, but the headings for each
>> 'family' probably need to be a bit more descriptive (esp 15 - all P4
>> netburst).
> 
> <shrug>
> 
> They're already out of order because Quark is a weirdo and we didn't
> want to look at it all the time so it got stuck at the bottom. It's
> _fine_ when it's 99% family 6 plus 2 other weirdos.
> 
> But it should probably get sorted at some point because it's going to be
> hard to find Quark and P4 if the order is
> 
> 	fam 6,1
> 	fam 5
> 	fam 15
> 	fam 18
> 	fam 19
> 	fam 20
> 	...
> 
> But I think all that has to be done is to move Quark up to the top.
> 

Patch 10/15 adds a few Family 5 defines. I'll move Family 5 to the top
along with that.

Re: [RFC PATCH 01/15] x86/apic: Fix 32-bit APIC initialization for extended Intel families

[Sohil Mehta]

On 12/20/2024 3:13 PM, Dave Hansen wrote:
> On 12/20/24 13:36, Sohil Mehta wrote:
>> detect_init_APIC() limits the APIC detection to families 6 and 15.
>> Extend the check to family numbers beyond 15. Also, convert it to a VFM
>> check to make it simpler.
> 
> This changelog doesn't _quite_ fit the code. I'd rather it was a bit
> more precise. It does not _just_ expand to "numbers beyond 15".
> 
> Also, It doesn't _really_ help to have the changelog tell us the
> function name. That's why we have diff -p.  How about:
> 
> 	APIC detection is currently limited to a few specific families
> 	and will not match the upcoming families >=18.
> 
> 	Extend the check to include all families 6 or greater. Also
> 	convert it to a VFM check to make it simpler.
> 
>>  arch/x86/kernel/apic/apic.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)

...

> With that changelog tweak:
> 
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>

Thanks for the Ack! The updated changelog does make it more precise.
I'll use that.

Re: [RFC PATCH 02/15] x86/apic: Fix smp init delay for extended Intel families

[Sohil Mehta]

On 12/20/2024 3:20 PM, Dave Hansen wrote:
> On 12/20/24 13:36, Sohil Mehta wrote:
>> The MP specification version 1.4 references the i486 and early Pentium
>> processors in family 5. 
> 
> Can you please elaborate on how this reference is relevant to the patch
> at hand?
> 

The comment above UDELAY_10MS_DEFAULT which references the MP
specification seems to be the basis of all the cpu_init_delay quirks.

I wanted to use that reference to justify that family 15 doesn't need
the 10 msec delay since it is not explicitly mentioned there. I realize
now that it's moot since the Pentium 4 wasn't released until 3 years
after it's publication.

I referred the latest MP initialization specification but that doesn't
say explicitly when the delay is needed vs not needed. However it does
say that later Family 6 models and Family 15 models have similar
initialization protocols.

"The selection of the BSP and APs is handled through a special system
bus cycle, without using BIPI and FIPI message arbitration (see Section
9.4.3, “MP Initialization Protocol Algorithm for MP Systems”). These
processor generations have CPUID signatures of family=0FH with
(model=0H, stepping>=0AH) or (model >0, all steppings); or family=06H,
extended_model=0, model>=0EH."

>> However, all processors starting with family 6 likely do not need the
>> 10 msec INIT delay. The omission of the Pentium 4s (family 15) seems
>> like an oversight in the original check.
>>
>> With some risk, choose a simpler check and extend the quirk to all
>> recent and upcoming Intel processors.
> 
> I'm struggling to follow this a bit.
> 

Because my interpretation of the code and the related comments is
opposite to yours. The usage of "quirk" in this context seems to be
inverted due to how this check has evolved over time.

> I think these are the facts that matter:
> 

The code says this:

/* if modern processor, use no delay */
	init_udelay = 0;
/* else, use legacy delay */
	init_udelay = UDELAY_10MS_DEFAULT;


The legacy/default delay is 10 msec and then the quirk applies to the
modern processors to remove the delay. This is the comment above
UDELAY_10MS_DEFAULT:

 * Modern processor families are quirked to remove the delay entirely.

>  * init_udelay=0 means "no quirk"
>  * Modern CPUs don't have the quirk
>  * The current check says "only family 6 is modern"
>  * Family 15 is _probably_ modern and just forgotten about
> 
> And this is what you're doing in the end:
> 
> Consider everything PPro and later to be modern, including all of
> families 6, 15 and the new 18/19 CPUs.
> 

That's right. We consider these CPUs to be modern and do not apply the
10 msec delay but the usage of "quirk" is confusing here.

I'll clarify the changelog without using "quirk" to avoid confusion.

Am I interpreting this wrong?

Re: [RFC PATCH 03/15] x86/cpu/intel: Fix init_intel() checks for extended family numbers

[Sohil Mehta]

On 12/20/2024 3:27 PM, Dave Hansen wrote:
> On 12/20/24 13:36, Sohil Mehta wrote:
>> X86_FEATURE_REP_GOOD is only set for family 6 processors.  Extend the
>> check to family numbers beyond 15.
> 
> Could you explain why, please?
> 

To answer this I was trying to understand where Fast string
(X86_FEATURE_REP_GOOD) is used. It looks like copy_page() in
lib/copy_page_64.S is the only place it really matters. clear_page() in
include/asm/page_64.h would likely use Enhanced fast strings (ERMS) if
available.

Would it be correct to say that copy_page() and potentially clear_page()
would be slower on Family 18/19 CPUs without the fix?

>> It is uncertain whether the Pentium 4s (family 15) should set the
>> feature flag as well. Commit 185f3b9da24c ("x86: make intel.c have
>> 64-bit support code") that originally set X86_FEATURE_REP_GOOD also set
>> the x86_cache_alignment preference for family 15 processors. The
>> omission of the family 15 seems intentional.
>>

Analyzing more, it seems the below check in early_init_intel() is not
really effective.

/*
 * If fast string is not enabled in IA32_MISC_ENABLE for any reason,
 * clear the fast string and enhanced fast string CPU capabilities.
 */
if (c->x86_vfm >= INTEL_PENTIUM_M_DOTHAN) {
	rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
	if (!(misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING)) {
		pr_info("Disabled fast string operations\n");
		setup_clear_cpu_cap(X86_FEATURE_REP_GOOD);
		setup_clear_cpu_cap(X86_FEATURE_ERMS);
	}
}


It gets overridden later in intel_init() with the below code:

if (c->x86 == 6)
	set_cpu_cap(c, X86_FEATURE_REP_GOOD);

Shouldn't the order of this be the other way around?

>> Also, the 32-bit user copy alignment preference is only set for family 6
>> and 15 processors. Extend the preference to family numbers beyond 15.
> 
> Can you please provide some more context so it's clear which hunk this
> refers to?  Alternatively, can you break this out into a separate patch?
> 

This is referring to the below chunk. Separating it seems like a better
idea to avoid ambiguity.

> -
>  #ifdef CONFIG_X86_INTEL_USERCOPY
>  	/*
>  	 * Set up the preferred alignment for movsl bulk memory moves
> +	 * Family 4 - 486: untested
> +	 * Family 5 - Pentium: untested
> +	 * Family 6 - PII/PIII only like movsl with 8-byte alignment
> +	 * Family 15 - P4 is OK down to 8-byte alignment
>  	 */
> -	switch (c->x86) {
> -	case 4:		/* 486: untested */
> -		break;
> -	case 5:		/* Old Pentia: untested */
> -		break;
> -	case 6:		/* PII/PIII only like movsl with 8-byte alignment */
> -		movsl_mask.mask = 7;
> -		break;
> -	case 15:	/* P4 is OK down to 8-byte alignment */
> +	if (c->x86_vfm >= INTEL_PENTIUM_PRO)
>  		movsl_mask.mask = 7;
> -		break;
> -	}
>  #endif