linux-kernel - Re: [PATCH v3 06/10] x86/mce: Break up __mcheck_cpu_apply

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241029213920.GB1229628@yaz-khff2.amd.com>
Date: Tue, 29 Oct 2024 17:39:20 -0400
From: Yazen Ghannam <yazen.ghannam@....com>
To: Qiuxu Zhuo <qiuxu.zhuo@...el.com>
Cc: bp@...en8.de, tony.luck@...el.com, tglx@...utronix.de,
	dave.hansen@...ux.intel.com, mingo@...hat.com, hpa@...or.com,
	x86@...nel.org, linux-edac@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 06/10] x86/mce: Break up __mcheck_cpu_apply_quirks()

On Fri, Oct 25, 2024 at 10:45:58AM +0800, Qiuxu Zhuo wrote:
> From: Tony Luck <tony.luck@...el.com>
> 
> Split each vendor specific part into its own helper function.
> 
> Tested-by: Qiuxu Zhuo <qiuxu.zhuo@...el.com>
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@...el.com>
> ---
> 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