[PATCH] x86/cpu/intel: Replace PAT erratum model/family magic numbers

[PATCH] x86/cpu/intel: Replace PAT erratum model/family magic numbers

[Dave Hansen]

There's an erratum[1] that prevents the PAT from working correctly.
The kernel currently disables PAT support on those CPUs.  But it
does it with some magic numbers.

Replace the magic numbers with new "IFM" macros.

Make the check refer to the last affected CPU (INTEL_CORE_YONAH)
rather than the first fixed one. This makes it easier to find the
documentation of the erratum since Intel documents where it is
broken and not where it is fixed.

I don't think the Pentium Pro (or Pentium II) is actually affected.
But the old check included them, so it can't hurt to keep doing the
same.  I'm also not completely sure about the "Pentium M" CPUs
(models 0x9 and 0xd).  But, again, they were included in in the
old checks and were close Pentium III derivatives, so are likely
affected.

While we're at it, revise the comment referring to the erratum name
and making sure it is a quote of the language from the actual errata
doc.  That should make it easier to find in the future when the URL
inevitably changes.

Why bother with this in the first place? It actually gets rid of one
of the very few remaining direct references to c->x86{,_model}.

1. https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/pentium-dual-core-specification-update.pdf
   Document 316515 Version 010

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---
 arch/x86/include/asm/intel-family.h |  2 ++
 arch/x86/kernel/cpu/intel.c         | 18 ++++++++++--------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
index f81a851c46dca..27bdf3b55c6f0 100644
--- a/arch/x86/include/asm/intel-family.h
+++ b/arch/x86/include/asm/intel-family.h
@@ -47,6 +47,8 @@
 /* Wildcard match for FAM6 so X86_MATCH_VFM(ANY) works */
 #define INTEL_ANY			IFM(X86_FAMILY_ANY, X86_MODEL_ANY)
 
+#define INTEL_PENTIUM_PRO		IFM(6, 0x01)
+
 #define INTEL_FAM6_CORE_YONAH		0x0E
 #define INTEL_CORE_YONAH		IFM(6, 0x0E)
 
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index a9ea0dba6f0cf..b1515ab00e640 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -304,16 +304,18 @@ static void early_init_intel(struct cpuinfo_x86 *c)
 	}
 
 	/*
-	 * There is a known erratum on Pentium III and Core Solo
-	 * and Core Duo CPUs.
-	 * " Page with PAT set to WC while associated MTRR is UC
-	 *   may consolidate to UC "
-	 * Because of this erratum, it is better to stick with
-	 * setting WC in MTRR rather than using PAT on these CPUs.
+	 * PAT is broken on early family 6 CPUs, the last of which
+	 * is "Yonah" where the erratum is named "AN7":
 	 *
-	 * Enable PAT WC only on P4, Core 2 or later CPUs.
+	 * 	Page with PAT (Page Attribute Table) Set to USWC
+	 * 	(Uncacheable Speculative Write Combine) While
+	 * 	Associated MTRR (Memory Type Range Register) Is UC
+	 * 	(Uncacheable) May Consolidate to UC
+	 *
+	 * Disable PAT and fall back to MTRR on these CPUs.
 	 */
-	if (c->x86 == 6 && c->x86_model < 15)
+	if (c->x86_vfm >= INTEL_PENTIUM_PRO &&
+	    c->x86_vfm <= INTEL_CORE_YONAH)
 		clear_cpu_cap(c, X86_FEATURE_PAT);
 
 	/*
-- 
2.34.1

[tip: x86/cpu] x86/cpu/intel: Replace PAT erratum model/family magic numbers with symbolic IFM references

[tip-bot2 for Dave Hansen]

The following commit has been merged into the x86/cpu branch of tip:

Commit-ID:     fd82221a59fa5ce9dc7523e11c5e995104a28cb0
Gitweb:        https://git.kernel.org/tip/fd82221a59fa5ce9dc7523e11c5e995104a28cb0
Author:        Dave Hansen <dave.hansen@linux.intel.com>
AuthorDate:    Thu, 29 Aug 2024 15:00:42 -07:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 03 Sep 2024 11:18:58 +02:00

x86/cpu/intel: Replace PAT erratum model/family magic numbers with symbolic IFM references

There's an erratum that prevents the PAT from working correctly:

   https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/pentium-dual-core-specification-update.pdf
   # Document 316515 Version 010

The kernel currently disables PAT support on those CPUs, but it
does it with some magic numbers.

Replace the magic numbers with the new "IFM" macros.

Make the check refer to the last affected CPU (INTEL_CORE_YONAH)
rather than the first fixed one. This makes it easier to find the
documentation of the erratum since Intel documents where it is
broken and not where it is fixed.

I don't think the Pentium Pro (or Pentium II) is actually affected.
But the old check included them, so it can't hurt to keep doing the
same.  I'm also not completely sure about the "Pentium M" CPUs
(models 0x9 and 0xd).  But, again, they were included in in the
old checks and were close Pentium III derivatives, so are likely
affected.

While we're at it, revise the comment referring to the erratum name
and making sure it is a quote of the language from the actual errata
doc.  That should make it easier to find in the future when the URL
inevitably changes.

Why bother with this in the first place? It actually gets rid of one
of the very few remaining direct references to c->x86{,_model}.

No change in functionality intended.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Len Brown <len.brown@intel.com>
Link: https://lore.kernel.org/r/20240829220042.1007820-1-dave.hansen@linux.intel.com
---
 arch/x86/include/asm/intel-family.h |  2 ++
 arch/x86/kernel/cpu/intel.c         | 18 ++++++++++--------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
index f81a851..27bdf3b 100644
--- a/arch/x86/include/asm/intel-family.h
+++ b/arch/x86/include/asm/intel-family.h
@@ -47,6 +47,8 @@
 /* Wildcard match for FAM6 so X86_MATCH_VFM(ANY) works */
 #define INTEL_ANY			IFM(X86_FAMILY_ANY, X86_MODEL_ANY)
 
+#define INTEL_PENTIUM_PRO		IFM(6, 0x01)
+
 #define INTEL_FAM6_CORE_YONAH		0x0E
 #define INTEL_CORE_YONAH		IFM(6, 0x0E)
 
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 08b95a3..e7656cb 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -311,16 +311,18 @@ static void early_init_intel(struct cpuinfo_x86 *c)
 	}
 
 	/*
-	 * There is a known erratum on Pentium III and Core Solo
-	 * and Core Duo CPUs.
-	 * " Page with PAT set to WC while associated MTRR is UC
-	 *   may consolidate to UC "
-	 * Because of this erratum, it is better to stick with
-	 * setting WC in MTRR rather than using PAT on these CPUs.
+	 * PAT is broken on early family 6 CPUs, the last of which
+	 * is "Yonah" where the erratum is named "AN7":
 	 *
-	 * Enable PAT WC only on P4, Core 2 or later CPUs.
+	 * 	Page with PAT (Page Attribute Table) Set to USWC
+	 * 	(Uncacheable Speculative Write Combine) While
+	 * 	Associated MTRR (Memory Type Range Register) Is UC
+	 * 	(Uncacheable) May Consolidate to UC
+	 *
+	 * Disable PAT and fall back to MTRR on these CPUs.
 	 */
-	if (c->x86 == 6 && c->x86_model < 15)
+	if (c->x86_vfm >= INTEL_PENTIUM_PRO &&
+	    c->x86_vfm <= INTEL_CORE_YONAH)
 		clear_cpu_cap(c, X86_FEATURE_PAT);
 
 	/*

Re: [tip: x86/cpu] x86/cpu/intel: Replace PAT erratum model/family magic numbers with symbolic IFM references

[Dave Hansen]

On 9/3/24 02:32, tip-bot2 for Dave Hansen wrote:
> -	if (c->x86 == 6 && c->x86_model < 15)
> +	if (c->x86_vfm >= INTEL_PENTIUM_PRO &&
> +	    c->x86_vfm <= INTEL_CORE_YONAH)
>  		clear_cpu_cap(c, X86_FEATURE_PAT);

Andy Cooper did point out that there is a theoretical behavioral change
here with c->x86_model==0.  There is a reference to the existence of
such a beast on at least on random web page[1] on the Internet as "P6
A-step".

But the SDM neither confirms nor denies that such a model ever existed.
If the SDM can't be bothered to acknowledge its existence, Linux
probably shouldn't either.

Either way, we're talking about a 32-bit CPU that's almost 30 years old
and was probably pre-production anyway.

I'm fine with the patch as-is.

1. https://www.sandpile.org/x86/cpuid.htm

Re: [tip: x86/cpu] x86/cpu/intel: Replace PAT erratum model/family magic numbers with symbolic IFM references

[Andrew Cooper]

On 03/09/2024 7:46 pm, Dave Hansen wrote:
> On 9/3/24 02:32, tip-bot2 for Dave Hansen wrote:
>> -	if (c->x86 == 6 && c->x86_model < 15)
>> +	if (c->x86_vfm >= INTEL_PENTIUM_PRO &&
>> +	    c->x86_vfm <= INTEL_CORE_YONAH)
>>  		clear_cpu_cap(c, X86_FEATURE_PAT);
> Andy Cooper did point out that there is a theoretical behavioral change
> here with c->x86_model==0.  There is a reference to the existence of
> such a beast on at least on random web page[1] on the Internet as "P6
> A-step".
>
> But the SDM neither confirms nor denies that such a model ever existed.
> If the SDM can't be bothered to acknowledge its existence, Linux
> probably shouldn't either.
>
> Either way, we're talking about a 32-bit CPU that's almost 30 years old
> and was probably pre-production anyway.
>
> I'm fine with the patch as-is.
>
> 1. https://www.sandpile.org/x86/cpuid.htm

This same purveyor of top quality x86 history pointed out that PAT
didn't exist on the Pentium, PPro, or P2, so they are unlikely to be
affected by this erratum.

Other cross references if they're helpful:
 * Banias Y31
 * Dothan X14
 * Yonah AE7
 * Yonah Xeon AF7

Finally, this looks suspiciously like it's the bug described in footnote
1 of https://sandpile.org/x86/coherent.htm MTRR/PAT conflicts which
otherwise identified that the early PAT-capable chips did behave as
expected.

~Andrew