diff mbox series

[v2,10/16] x86/mce: Separate global and per-CPU quirks

Message ID 20250213-wip-mca-updates-v2-10-3636547fe05f@amd.com (mailing list archive)
State New
Headers show
Series AMD MCA interrupts rework | expand

Commit Message

Yazen Ghannam Feb. 13, 2025, 4:45 p.m. UTC 
Many quirks are global configuration settings and a handful apply to
each CPU.

Move the per-CPU quirks to vendor init to execute them on each online
CPU. Set the global quirks during BSP-only init so they're only executed
once and early.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
---

Notes:
    v1->v2:
    * New in v2.

 arch/x86/kernel/cpu/mce/amd.c   | 23 +++++++++++++++++++++++
 arch/x86/kernel/cpu/mce/core.c  | 36 ++----------------------------------
 arch/x86/kernel/cpu/mce/intel.c | 15 +++++++++++++++
 3 files changed, 40 insertions(+), 34 deletions(-)

Comments

Zhuo, Qiuxu Feb. 18, 2025, 6:03 a.m. UTC | #1 
> From: Yazen Ghannam <yazen.ghannam@amd.com>
> [...]
> --- a/arch/x86/kernel/cpu/mce/intel.c
> +++ b/arch/x86/kernel/cpu/mce/intel.c
> @@ -468,8 +468,23 @@ static void intel_imc_init(struct cpuinfo_x86 *c)
>  	}
>  }
> 
> +static void intel_apply_quirks(struct cpuinfo_x86 *c) {
> +	/*
> +	 * SDM documents that on family 6 bank 0 should not be written
> +	 * because it aliases to another special BIOS controlled
> +	 * register.
> +	 * But it's not aliased anymore on model 0x1a+
> +	 * Don't ignore bank 0 completely because there could be a
> +	 * valid event later, merely don't write CTL0.

Is it better to add the following description here? So that it's clear
we don't apply the quirks for older CPUs.

    Older CPUs (prior to family 6) can't reach this point and already return early 
    due to the check of __mcheck_cpu_ancient_init().

> +	 */
> +	if (c->x86_vfm < INTEL_NEHALEM_EP &&
> this_cpu_read(mce_num_banks))
> +		this_cpu_ptr(mce_banks_array)[0].init = false; }
> +
[...]

LGTM. Thanks.

    Reviewed-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
Yazen Ghannam Feb. 19, 2025, 4:06 p.m. UTC | #2 
On Tue, Feb 18, 2025 at 06:03:42AM +0000, Zhuo, Qiuxu wrote:
> > From: Yazen Ghannam <yazen.ghannam@amd.com>
> > [...]
> > --- a/arch/x86/kernel/cpu/mce/intel.c
> > +++ b/arch/x86/kernel/cpu/mce/intel.c
> > @@ -468,8 +468,23 @@ static void intel_imc_init(struct cpuinfo_x86 *c)
> >  	}
> >  }
> > 
> > +static void intel_apply_quirks(struct cpuinfo_x86 *c) {
> > +	/*
> > +	 * SDM documents that on family 6 bank 0 should not be written
> > +	 * because it aliases to another special BIOS controlled
> > +	 * register.
> > +	 * But it's not aliased anymore on model 0x1a+
> > +	 * Don't ignore bank 0 completely because there could be a
> > +	 * valid event later, merely don't write CTL0.
> 
> Is it better to add the following description here? So that it's clear
> we don't apply the quirks for older CPUs.
> 
>     Older CPUs (prior to family 6) can't reach this point and already return early 
>     due to the check of __mcheck_cpu_ancient_init().
> 

I don't know. As you said, the older CPUs don't enter this code, so why
refer to them at all?

> > +	 */
> > +	if (c->x86_vfm < INTEL_NEHALEM_EP &&
> > this_cpu_read(mce_num_banks))
> > +		this_cpu_ptr(mce_banks_array)[0].init = false; }
> > +
> [...]
> 
> LGTM. Thanks.
> 
>     Reviewed-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> 

Thanks,
Yazen
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
index bf2b1dc5aaa9..c6510415159f 100644
--- a/arch/x86/kernel/cpu/mce/amd.c
+++ b/arch/x86/kernel/cpu/mce/amd.c
@@ -649,6 +649,28 @@  static void disable_err_thresholding(struct cpuinfo_x86 *c, unsigned int bank)
 		wrmsrl(MSR_K7_HWCR, hwcr);
 }
 
+static void amd_apply_quirks(struct cpuinfo_x86 *c)
+{
+	struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array);
+
+	/* This should be disabled by the BIOS, but isn't always */
+	if (c->x86 == 15 && this_cpu_read(mce_num_banks) > 4) {
+		/*
+		 * disable GART TBL walk error reporting, which
+		 * trips off incorrectly with the IOMMU & 3ware
+		 * & Cerberus:
+		 */
+		clear_bit(10, (unsigned long *)&mce_banks[4].ctl);
+	}
+
+	/*
+	 * Various K7s with broken bank 0 around. Always disable
+	 * by default.
+	 */
+	if (c->x86 == 6 && this_cpu_read(mce_num_banks))
+		mce_banks[0].ctl = 0;
+}
+
 /* cpu init entry point, called from mce.c with preempt off */
 void mce_amd_feature_init(struct cpuinfo_x86 *c)
 {
@@ -656,6 +678,7 @@  void mce_amd_feature_init(struct cpuinfo_x86 *c)
 	u32 low = 0, high = 0, address = 0;
 	int offset = -1;
 
+	amd_apply_quirks(c);
 	mce_flags.amd_threshold	 = 1;
 
 	for (bank = 0; bank < this_cpu_read(mce_num_banks); ++bank) {
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 38db802acde4..1ea52f6259a4 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1879,18 +1879,6 @@  static void __mcheck_cpu_init_prepare_banks(void)
 
 static void apply_quirks_amd(struct cpuinfo_x86 *c)
 {
-	struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array);
-
-	/* This should be disabled by the BIOS, but isn't always */
-	if (c->x86 == 15 && this_cpu_read(mce_num_banks) > 4) {
-		/*
-		 * disable GART TBL walk error reporting, which
-		 * trips off incorrectly with the IOMMU & 3ware
-		 * & Cerberus:
-		 */
-		clear_bit(10, (unsigned long *)&mce_banks[4].ctl);
-	}
-
 	if (c->x86 < 0x11 && mca_cfg.bootlog < 0) {
 		/*
 		 * Lots of broken BIOS around that don't clear them
@@ -1899,13 +1887,6 @@  static void apply_quirks_amd(struct cpuinfo_x86 *c)
 		mca_cfg.bootlog = 0;
 	}
 
-	/*
-	 * Various K7s with broken bank 0 around. Always disable
-	 * by default.
-	 */
-	if (c->x86 == 6 && this_cpu_read(mce_num_banks))
-		mce_banks[0].ctl = 0;
-
 	/*
 	 * overflow_recov is supported for F15h Models 00h-0fh
 	 * even though we don't have a CPUID bit for it.
@@ -1919,23 +1900,10 @@  static void apply_quirks_amd(struct cpuinfo_x86 *c)
 
 static void apply_quirks_intel(struct cpuinfo_x86 *c)
 {
-	struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array);
-
 	/* Older CPUs (prior to family 6) don't need quirks. */
 	if (c->x86_vfm < INTEL_PENTIUM_PRO)
 		return;
 
-	/*
-	 * SDM documents that on family 6 bank 0 should not be written
-	 * because it aliases to another special BIOS controlled
-	 * register.
-	 * But it's not aliased anymore on model 0x1a+
-	 * Don't ignore bank 0 completely because there could be a
-	 * valid event later, merely don't write CTL0.
-	 */
-	if (c->x86_vfm < INTEL_NEHALEM_EP && this_cpu_read(mce_num_banks))
-		mce_banks[0].init = false;
-
 	/*
 	 * All newer Intel systems support MCE broadcasting. Enable
 	 * synchronization with a one second timeout.
@@ -2255,6 +2223,8 @@  void cpu_mca_init(struct cpuinfo_x86 *c)
 
 	if (cap & MCG_SER_P)
 		mca_cfg.ser = 1;
+
+	__mcheck_cpu_apply_quirks(c);
 }
 
 /*
@@ -2274,8 +2244,6 @@  void mcheck_cpu_init(struct cpuinfo_x86 *c)
 
 	__mcheck_cpu_cap_init();
 
-	__mcheck_cpu_apply_quirks(c);
-
 	if (!mce_gen_pool_init()) {
 		mca_cfg.disabled = 1;
 		pr_emerg("Couldn't allocate MCE records pool!\n");
diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c
index f863df0ff42c..1a7aaee14991 100644
--- a/arch/x86/kernel/cpu/mce/intel.c
+++ b/arch/x86/kernel/cpu/mce/intel.c
@@ -468,8 +468,23 @@  static void intel_imc_init(struct cpuinfo_x86 *c)
 	}
 }
 
+static void intel_apply_quirks(struct cpuinfo_x86 *c)
+{
+	/*
+	 * SDM documents that on family 6 bank 0 should not be written
+	 * because it aliases to another special BIOS controlled
+	 * register.
+	 * But it's not aliased anymore on model 0x1a+
+	 * Don't ignore bank 0 completely because there could be a
+	 * valid event later, merely don't write CTL0.
+	 */
+	if (c->x86_vfm < INTEL_NEHALEM_EP && this_cpu_read(mce_num_banks))
+		this_cpu_ptr(mce_banks_array)[0].init = false;
+}
+
 void mce_intel_feature_init(struct cpuinfo_x86 *c)
 {
+	intel_apply_quirks(c);
 	intel_init_cmci();
 	intel_init_lmce();
 	intel_imc_init(c);