Re: [PATCH v2 1/2] x86/mce/amd: Fix inverted interrupt enablement during storm handling

[Jasjeet Rangi <jrangi@purestorage.com>]

On Fri, Aug 14, 2026 at 4:25 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Aug 12, 2026 at 04:15:13PM -0600, Jasjeet Rangi wrote:
> > mce_amd_handle_storm() currently does the opposite of what storm
> > handling needs: it enables threshold interrupts when a storm is detected
> > and disables them when the storm subsides.
> >
> > In addition, machine_check_poll() -> clear_bank() -> amd_clear_bank() ->
> > amd_reset_thr_limit() will unconditionally enable threshold interrupts,
> > which undoes storm mode behavior.
>
> Except that the Intel side doesn't touch the CMCI_EN bit in
> cmci_set_threshold(). And we should not diverge here. The thresholding
> interrupt should not be a problem because with increased polling frequency
> during a storm, we should not be really getting thresholding interrupts
> because the polling code will pick up all MCEs that get logged, first.
>
> And the second patch is not really making things better because, well, "on" is
> "in_storm_mode". Basically the same thing. So I'm going to queue the below:
>
> ---
> Author: Jasjeet Rangi <jrangi@purestorage.com>
> Date:   Wed Aug 12 16:15:13 2026 -0600
>
>     x86/MCE/AMD: Fix inverted interrupt enablement during storm handling
>
>     mce_amd_handle_storm() currently does the opposite of what storm
>     handling needs: it enables thresholding interrupts when a storm is
>     detected and disables them when the storm subsides.
>
>     Flip the "on" function argument before passing it to threshold_restart_bank()
>     as it should have been done.
>
>     To clarify: "on" to mce_handle_storm() means, the storm is on now when
>     "on" is true, and off when "on" is false.
>
>       [ bp: Simplify. ]
>
>     Fixes: 5c4663ed1eac ("x86/mce: Handle AMD threshold interrupt storms")
>     Signed-off-by: Jasjeet Rangi <jrangi@purestorage.com>
>     Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
>     Cc: stable@vger.kernel.org
>     Link: https://patch.msgid.link/20260812221514.598842-2-jrangi@purestorage.com
>
> diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
> index f916fb4c5d13..1cc20b855b7e 100644
> --- a/arch/x86/kernel/cpu/mce/amd.c
> +++ b/arch/x86/kernel/cpu/mce/amd.c
> @@ -865,7 +865,7 @@ static void amd_deferred_error_interrupt(void)
>
>  void mce_amd_handle_storm(unsigned int bank, bool on)
>  {
> -       threshold_restart_bank(bank, on);
> +       threshold_restart_bank(bank, !on);
>  }
>
>  static void amd_reset_thr_limit(unsigned int bank)
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

I'm ok with dropping patch 2. But I don't think we should drop the
amd_reset_thr_limit() hunk of patch 1. On AMD if storm conditions are
met, amd_reset_thr_limit() will get called in the same code path as
mce_amd_handle_storm().

```
void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
{
...
	for (i = 0; i < this_cpu_read(mce_num_banks); i++) {
...
		if (!mca_cfg.cmci_disabled)
			mce_track_storm(m); // <- mce_amd_handle_storm()
...
clear_it:
		clear_bank(m); // <- amd_reset_thr_limit()
	}
```

Inverting `on` in mce_amd_handle_storm() alone is not enough because
clear_bank() will immediately and unconditionally enable the interrupt
again. Also, the threshold is sysfs configurable. For example, if the
threshold is set to 1, even when storm handling is on there will be
effectively no polling.

On Intel the threshold is temporarily set to a very high value because
the goal is to effectively disable interrupts for CEs without disabling
interrupts for certain UEs signaled via CMCI. In older kernels the Intel
driver used to disable the interrupt.
From the current Intel code:
```
/*
 * High threshold to limit CMCI rate during storms. Max supported is
 * 0x7FFF. Use this slightly smaller value so it has a distinctive
 * signature when some asks "Why am I not seeing all corrected errors?"
 * A high threshold is used instead of just disabling CMCI for a
 * bank because both corrected and uncorrected errors may be logged
 * in the same bank and signalled with CMCI. The threshold only applies
 * to corrected errors, so keeping CMCI enabled means that uncorrected
 * errors will still be processed in a timely fashion.
 */
#define CMCI_STORM_THRESHOLD 32749
```
I do not see anything similar for AMD.

Thanks,
Jasjeet