Re: [PATCH v3 06/10] x86/mce: Break up __mcheck_cpu_apply_quirks()

From: Yazen Ghannam
Date: Tue Oct 29 2024 - 17:39:41 EST


On Fri, Oct 25, 2024 at 10:45:58AM +0800, Qiuxu Zhuo wrote:
> From: Tony Luck <tony.luck@xxxxxxxxx>
>
> Split each vendor specific part into its own helper function.
>
> Tested-by: Qiuxu Zhuo <qiuxu.zhuo@xxxxxxxxx>
> Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx>
> Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@xxxxxxxxx>
> ---
> Changes in v3:
> - Newly added.
>
> arch/x86/kernel/cpu/mce/core.c | 194 ++++++++++++++++++---------------
> 1 file changed, 106 insertions(+), 88 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
> index 57c05015f984..bb8b1000fa0a 100644
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -1880,101 +1880,119 @@ static void __mcheck_cpu_check_banks(void)
> }
> }
>
> +static void apply_quirks_amd(struct cpuinfo_x86 *c)
> +{
> + struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array);
> + struct mca_config *cfg = &mca_cfg;
> +
> + /* 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);
> + }

Newline here please.

> + if (c->x86 < 0x11 && cfg->bootlog < 0) {
> + /*
> + * Lots of broken BIOS around that don't clear them
> + * by default and leave crap in there. Don't log:
> + */
> + cfg->bootlog = 0;
> + }

And here.

> + /*
> + * Various K7s with broken bank 0 around. Always disable
> + * by default.
> + */
> + if (c->x86 == 6 && this_cpu_read(mce_num_banks) > 0)
> + 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.
> + */
> + if (c->x86 == 0x15 && c->x86_model <= 0xf)
> + mce_flags.overflow_recov = 1;
> +
> + if (c->x86 >= 0x17 && c->x86 <= 0x1A)
> + mce_flags.zen_ifu_quirk = 1;
> +}
> +
> +static void apply_quirks_intel(struct cpuinfo_x86 *c)
> +{
> + struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array);
> + struct mca_config *cfg = &mca_cfg;

Is there a benefit to this pointer? We use mca_cfg.FIELD in most other
places.

Thanks,
Yazen