| 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
| ||
|
Message-ID: <64314c7a-5212-4bf0-8181-8bbada0e81b5@suse.com>
Date: Thu, 11 Sep 2025 20:01:17 +0300
From: Nikolay Borisov <nik.borisov@...e.com>
To: Yazen Ghannam <yazen.ghannam@....com>, x86@...nel.org,
Tony Luck <tony.luck@...el.com>, "Rafael J. Wysocki" <rafael@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-edac@...r.kernel.org,
Smita.KoralahalliChannabasappa@....com, Qiuxu Zhuo <qiuxu.zhuo@...el.com>,
linux-acpi@...r.kernel.org
Subject: Re: [PATCH v6 15/15] x86/mce: Save and use APEI corrected threshold
limit
On 9/8/25 18:40, Yazen Ghannam wrote:
> The MCA threshold limit generally is not something that needs to change
> during runtime. It is common for a system administrator to decide on a
> policy for their managed systems.
>
> If MCA thresholding is OS-managed, then the threshold limit must be set
> at every boot. However, many systems allow the user to set a value in
> their BIOS. And this is reported through an APEI HEST entry even if
> thresholding is not in FW-First mode.
>
> Use this value, if available, to set the OS-managed threshold limit.
> Users can still override it through sysfs if desired for testing or
> debug.
>
> APEI is parsed after MCE is initialized. So reset the thresholding
> blocks later to pick up the threshold limit.
>
> Signed-off-by: Yazen Ghannam <yazen.ghannam@....com>
> ---
>
> Notes:
> Link:
> https://lore.kernel.org/r/20250825-wip-mca-updates-v5-20-865768a2eef8@amd.com
>
> v5->v6:
> * No change.
>
> v4->v5:
> * No change.
>
> v3->v4:
> * New in v4.
>
> arch/x86/include/asm/mce.h | 6 ++++++
> arch/x86/kernel/acpi/apei.c | 2 ++
> arch/x86/kernel/cpu/mce/amd.c | 18 ++++++++++++++++--
> arch/x86/kernel/cpu/mce/internal.h | 2 ++
> arch/x86/kernel/cpu/mce/threshold.c | 13 +++++++++++++
> 5 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
> index 7d6588195d56..1cfbfff0be3f 100644
> --- a/arch/x86/include/asm/mce.h
> +++ b/arch/x86/include/asm/mce.h
> @@ -308,6 +308,12 @@ DECLARE_PER_CPU(struct mce, injectm);
> /* Disable CMCI/polling for MCA bank claimed by firmware */
> extern void mce_disable_bank(int bank);
>
> +#ifdef CONFIG_X86_MCE_THRESHOLD
> +void mce_save_apei_thr_limit(u32 thr_limit);
> +#else
> +static inline void mce_save_apei_thr_limit(u32 thr_limit) { }
> +#endif /* CONFIG_X86_MCE_THRESHOLD */
> +
> /*
> * Exception handler
> */
> diff --git a/arch/x86/kernel/acpi/apei.c b/arch/x86/kernel/acpi/apei.c
> index 0916f00a992e..e21419e686eb 100644
> --- a/arch/x86/kernel/acpi/apei.c
> +++ b/arch/x86/kernel/acpi/apei.c
> @@ -19,6 +19,8 @@ int arch_apei_enable_cmcff(struct acpi_hest_header *hest_hdr, void *data)
> if (!cmc->enabled)
> return 0;
>
> + mce_save_apei_thr_limit(cmc->notify.error_threshold_value);
> +
> /*
> * We expect HEST to provide a list of MC banks that report errors
> * in firmware first mode. Otherwise, return non-zero value to
> diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
> index b895559e80ad..9b746080351f 100644
> --- a/arch/x86/kernel/cpu/mce/amd.c
> +++ b/arch/x86/kernel/cpu/mce/amd.c
> @@ -489,6 +489,18 @@ static void threshold_restart_bank(unsigned int bank, bool intr_en)
> }
> }
>
> +/* Try to use the threshold limit reported through APEI. */
> +static u16 get_thr_limit(void)
> +{
> + u32 thr_limit = mce_get_apei_thr_limit();
> +
> + /* Fallback to old default if APEI limit is not available. */
> + if (!thr_limit)
> + return THRESHOLD_MAX;
> +
> + return min(thr_limit, THRESHOLD_MAX);
> +}
> +
> static void mce_threshold_block_init(struct threshold_block *b, int offset)
> {
> struct thresh_restart tr = {
> @@ -497,7 +509,7 @@ static void mce_threshold_block_init(struct threshold_block *b, int offset)
> .lvt_off = offset,
> };
>
> - b->threshold_limit = THRESHOLD_MAX;
> + b->threshold_limit = get_thr_limit();
> threshold_restart_block(&tr);
> };
>
> @@ -1071,7 +1083,7 @@ static int allocate_threshold_blocks(unsigned int cpu, struct threshold_bank *tb
> b->address = address;
> b->interrupt_enable = 0;
> b->interrupt_capable = lvt_interrupt_supported(bank, high);
> - b->threshold_limit = THRESHOLD_MAX;
> + b->threshold_limit = get_thr_limit();
>
> if (b->interrupt_capable) {
> default_attrs[2] = &interrupt_enable.attr;
> @@ -1082,6 +1094,8 @@ static int allocate_threshold_blocks(unsigned int cpu, struct threshold_bank *tb
>
> list_add(&b->miscj, &tb->miscj);
>
> + mce_threshold_block_init(b, (high & MASK_LVTOFF_HI) >> 20);
Why is this necessary? Shouldn't this patch consist of mainly
s/THRESHOLD_MAX/get_thr_limit();
In allocate_threshold_block have already properly set threshold_limit.
So this change really ensures threshold_restart_block is being called
for the given block being initialized. Ignoring the changed threshold
limit logic, why is this extra call necessary now and wasn't before?
> +
> err = kobject_init_and_add(&b->kobj, &threshold_ktype, tb->kobj, get_name(cpu, bank, b));
> if (err)
> goto out_free;
> diff --git a/arch/x86/kernel/cpu/mce/internal.h b/arch/x86/kernel/cpu/mce/internal.h
> index 9920ee5fb34c..a31cf984619c 100644
> --- a/arch/x86/kernel/cpu/mce/internal.h
> +++ b/arch/x86/kernel/cpu/mce/internal.h
> @@ -67,6 +67,7 @@ void mce_track_storm(struct mce *mce);
> void mce_inherit_storm(unsigned int bank);
> bool mce_get_storm_mode(void);
> void mce_set_storm_mode(bool storm);
> +u32 mce_get_apei_thr_limit(void);
> #else
> static inline void cmci_storm_begin(unsigned int bank) {}
> static inline void cmci_storm_end(unsigned int bank) {}
> @@ -74,6 +75,7 @@ static inline void mce_track_storm(struct mce *mce) {}
> static inline void mce_inherit_storm(unsigned int bank) {}
> static inline bool mce_get_storm_mode(void) { return false; }
> static inline void mce_set_storm_mode(bool storm) {}
> +static inline u32 mce_get_apei_thr_limit(void) { return 0; }
> #endif
>
> /*
> diff --git a/arch/x86/kernel/cpu/mce/threshold.c b/arch/x86/kernel/cpu/mce/threshold.c
> index 45144598ec74..d00d5bf9959d 100644
> --- a/arch/x86/kernel/cpu/mce/threshold.c
> +++ b/arch/x86/kernel/cpu/mce/threshold.c
> @@ -13,6 +13,19 @@
>
> #include "internal.h"
>
> +static u32 mce_apei_thr_limit;
> +
> +void mce_save_apei_thr_limit(u32 thr_limit)
> +{
> + mce_apei_thr_limit = thr_limit;
> + pr_info("HEST: Corrected error threshold limit = %u\n", thr_limit);
> +}
> +
> +u32 mce_get_apei_thr_limit(void)
> +{
> + return mce_apei_thr_limit;
> +}
> +
> static void default_threshold_interrupt(void)
> {
> pr_err("Unexpected threshold interrupt at vector %x\n",
>
Powered by blists - more mailing lists