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

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

[Jasjeet Rangi]

Hi all,

This set fixes a bug in AMD MCE storm handling and renames a variable
that could have led to this bug. The variable rename patch is split from
the fix to keep the fix minimal and to allow easier backporting.

Patch 1: Fix inverted interrupt enablement during storm handling on AMD

Patch 2: Rename MCE storm handler parameters for storm mode

Thanks,
Jasjeet

Jasjeet Rangi (2):
  x86/mce/amd: Fix inverted interrupt enablement during storm handling
  x86/mce: Rename MCE storm handler parameters for storm mode

 arch/x86/kernel/cpu/mce/amd.c       | 9 ++++++---
 arch/x86/kernel/cpu/mce/intel.c     | 4 ++--
 arch/x86/kernel/cpu/mce/internal.h  | 8 ++++----
 arch/x86/kernel/cpu/mce/threshold.c | 6 +++---
 4 files changed, 15 insertions(+), 12 deletions(-)

-- 
2.50.1

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

[Jasjeet Rangi]

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.

Fix this by disabling interrupts when storm mode is entered and enabling
interrupts when storm mode is cleared. Also make amd_reset_thr_limit()
enable interrupts when the bank is not in storm mode and disable
interrupts when the bank is in storm mode.

Fixes: 5c4663ed1eac ("x86/mce: Handle AMD threshold interrupt storms")
Cc: stable@vger.kernel.org
Signed-off-by: Jasjeet Rangi <jrangi@purestorage.com>
---
 arch/x86/kernel/cpu/mce/amd.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
index 36e0df4d1342..04accdee6aab 100644
--- a/arch/x86/kernel/cpu/mce/amd.c
+++ b/arch/x86/kernel/cpu/mce/amd.c
@@ -864,12 +864,15 @@ 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)
 {
-	threshold_restart_bank(bank, true);
+	struct mca_storm_desc *storm = this_cpu_ptr(&storm_desc);
+	bool intr_en = !storm->banks[bank].in_storm_mode;
+
+	threshold_restart_bank(bank, intr_en);
 }
 
 /*
-- 
2.50.1

[PATCH v2 2/2] x86/mce: Rename MCE storm handler parameters for storm mode

[Jasjeet Rangi]

The function parameter "on" doesn't make it clear what exactly is being
turned on. This parameter is set to true when storm mode is activated.
On AMD, storm mode disables the bank's threshold interrupt. A value of
"on" actually means "interrupts off."

Rename mce_handle_storm(), mce_intel_handle_storm(), and
mce_amd_handle_storm() parameter "on" to "in_storm_mode" to disambiguate
its meaning.

Signed-off-by: Jasjeet Rangi <jrangi@purestorage.com>
---
 arch/x86/kernel/cpu/mce/amd.c       | 4 ++--
 arch/x86/kernel/cpu/mce/intel.c     | 4 ++--
 arch/x86/kernel/cpu/mce/internal.h  | 8 ++++----
 arch/x86/kernel/cpu/mce/threshold.c | 6 +++---
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
index 04accdee6aab..46880af61fd0 100644
--- a/arch/x86/kernel/cpu/mce/amd.c
+++ b/arch/x86/kernel/cpu/mce/amd.c
@@ -862,9 +862,9 @@ static void amd_deferred_error_interrupt(void)
 	machine_check_poll(MCP_TIMESTAMP, &this_cpu_ptr(&mce_amd_data)->dfr_intr_banks);
 }
 
-void mce_amd_handle_storm(unsigned int bank, bool on)
+void mce_amd_handle_storm(unsigned int bank, bool in_storm_mode)
 {
-	threshold_restart_bank(bank, !on);
+	threshold_restart_bank(bank, !in_storm_mode);
 }
 
 static void amd_reset_thr_limit(unsigned int bank)
diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c
index 4655223ba560..3e3cbdc4fedd 100644
--- a/arch/x86/kernel/cpu/mce/intel.c
+++ b/arch/x86/kernel/cpu/mce/intel.c
@@ -147,9 +147,9 @@ static void cmci_set_threshold(int bank, int thresh)
 	raw_spin_unlock_irqrestore(&cmci_discover_lock, flags);
 }
 
-void mce_intel_handle_storm(int bank, bool on)
+void mce_intel_handle_storm(int bank, bool in_storm_mode)
 {
-	if (on)
+	if (in_storm_mode)
 		cmci_set_threshold(bank, CMCI_STORM_THRESHOLD);
 	else
 		cmci_set_threshold(bank, cmci_threshold[bank]);
diff --git a/arch/x86/kernel/cpu/mce/internal.h b/arch/x86/kernel/cpu/mce/internal.h
index a31cf984619c..3f709faa7f1c 100644
--- a/arch/x86/kernel/cpu/mce/internal.h
+++ b/arch/x86/kernel/cpu/mce/internal.h
@@ -41,7 +41,7 @@ struct dentry *mce_get_debugfs_dir(void);
 extern mce_banks_t mce_banks_ce_disabled;
 
 #ifdef CONFIG_X86_MCE_INTEL
-void mce_intel_handle_storm(int bank, bool on);
+void mce_intel_handle_storm(int bank, bool in_storm_mode);
 void cmci_disable_bank(int bank);
 void intel_init_cmci(void);
 void intel_init_lmce(void);
@@ -49,7 +49,7 @@ void intel_clear_lmce(void);
 bool intel_filter_mce(struct mce *m);
 bool intel_mce_usable_address(struct mce *m);
 #else
-static inline void mce_intel_handle_storm(int bank, bool on) { }
+static inline void mce_intel_handle_storm(int bank, bool in_storm_mode) { }
 static inline void cmci_disable_bank(int bank) { }
 static inline void intel_init_cmci(void) { }
 static inline void intel_init_lmce(void) { }
@@ -269,7 +269,7 @@ void mce_prep_record_per_cpu(unsigned int cpu, struct mce *m);
 #ifdef CONFIG_X86_MCE_AMD
 void mce_threshold_create_device(unsigned int cpu);
 void mce_threshold_remove_device(unsigned int cpu);
-void mce_amd_handle_storm(unsigned int bank, bool on);
+void mce_amd_handle_storm(unsigned int bank, bool in_storm_mode);
 extern bool amd_filter_mce(struct mce *m);
 bool amd_mce_usable_address(struct mce *m);
 void amd_clear_bank(struct mce *m);
@@ -302,7 +302,7 @@ void smca_bsp_init(void);
 #else
 static inline void mce_threshold_create_device(unsigned int cpu)	{ }
 static inline void mce_threshold_remove_device(unsigned int cpu)	{ }
-static inline void mce_amd_handle_storm(unsigned int bank, bool on)	{ }
+static inline void mce_amd_handle_storm(unsigned int bank, bool in_storm_mode)	{ }
 static inline bool amd_filter_mce(struct mce *m) { return false; }
 static inline bool amd_mce_usable_address(struct mce *m) { return false; }
 static inline void amd_clear_bank(struct mce *m) { }
diff --git a/arch/x86/kernel/cpu/mce/threshold.c b/arch/x86/kernel/cpu/mce/threshold.c
index 6c370d5af5bd..ca19b5b7ffce 100644
--- a/arch/x86/kernel/cpu/mce/threshold.c
+++ b/arch/x86/kernel/cpu/mce/threshold.c
@@ -70,14 +70,14 @@ void mce_set_storm_mode(bool storm)
 	__this_cpu_write(storm_desc.poll_mode, storm);
 }
 
-static void mce_handle_storm(unsigned int bank, bool on)
+static void mce_handle_storm(unsigned int bank, bool in_storm_mode)
 {
 	switch (boot_cpu_data.x86_vendor) {
 	case X86_VENDOR_INTEL:
-		mce_intel_handle_storm(bank, on);
+		mce_intel_handle_storm(bank, in_storm_mode);
 		break;
 	case X86_VENDOR_AMD:
-		mce_amd_handle_storm(bank, on);
+		mce_amd_handle_storm(bank, in_storm_mode);
 		break;
 	}
 }
-- 
2.50.1

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

[Borislav Petkov]

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

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

[Jasjeet Rangi]

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

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

[Borislav Petkov]

On Mon, Aug 17, 2026 at 12:51:08PM -0600, Jasjeet Rangi wrote:
> Inverting `on` in mce_amd_handle_storm() alone is not enough because
> clear_bank() will immediately and unconditionally enable the interrupt
> again.

And?

What would happen then?

Did you read my previous reply?

"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."

Or do you have somehing in mind here which I cannot guess?

> 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.

Yeah, we don't protect users from themselves, you know :)

> 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.

We do clear MCi_MISC[ErrCnt] to 0 so that's the "highest' value we can do
before we raise an interrupt there :)

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Jasjeet Rangi]

I'm ok with the queued patch because it still fixes an important bug
(disabling interrupts on storm end is not correct). The rest of this
reply is talking about the concern that storms are not handled
properly in the AMD driver. I can make that a new patch thread if we
agree to make the behavior more Intel like.

On Tue, Aug 18, 2026 at 12:03 PM Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Aug 17, 2026 at 12:51:08PM -0600, Jasjeet Rangi wrote:
> > Inverting `on` in mce_amd_handle_storm() alone is not enough because
> > clear_bank() will immediately and unconditionally enable the interrupt
> > again.
>
> And?
>
> What would happen then?
>
> Did you read my previous reply?

On AMD a machine_check_poll() call will not necessarily set ErrCnt to 0.
In fact, it will not reset ErrCnt at all unless the overflow bit is
set. And the overflow bit gets set when it's time to generate an
interrupt. (Yazen and Smita please correct me if I'm wrong)

The threshold is not always high on AMD either. Since eeb3f76d73ba
(x86/mce: Save and use APEI corrected threshold limit)
it gets the value from the BIOS. The default on an AMD system I was
using was only 10. So ErrCnt will get reset to 4085 on that system
when the overflow bit is set. That is enough to not meaningfully
reduce the interrupt rate during an interrupt storm.

Unlike Intel, the AMD driver doesn't set the threshold to an extremely
large value when storm mode is turned on. It keeps it set to the same
configured value (default from BIOS).

So if interrupts are left enabled during storm mode like they would be
even with the queued patch, the AMD driver doesn't really reduce the
interrupt rate during a storm like the Intel driver does.

If we don't want to diverge from Intel, then the correct thing to do is
to update the AMD driver to do what the Intel driver does. I.e. don't
disable interrupts, but set the threshold_limit to a large value like
THRESHOLD_MAX during storm on, and restore it to what it was before upon
storm off.

> Yeah, we don't protect users from themselves, you know :)

I don't think having threshold=1 on AMD is too crazy. On the Intel
driver for example it defaults to 1 if not overridden by the BIOS.
```
/* Linux non-storm CMCI threshold (may be overridden by BIOS) */
#define CMCI_THRESHOLD		1
```

> We do clear MCi_MISC[ErrCnt] to 0 so that's the "highest' value we can do
> before we raise an interrupt there :)
And on AMD it does not get set to 0 after an interrupt. It gets set to
THRESHOLD_MAX - threshold_limit. So if your limit is 10, the ErrCnt will
get reset to 4085. This is the behavior since eeb3f76d73ba
(x86/mce: Save and use APEI corrected threshold limit). Prior to that
ErrCnt would get reset to 0 by default on AMD.

Thanks,
Jasjeet

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

[Yazen Ghannam]

On Wed, Aug 19, 2026 at 12:46:00AM -0600, Jasjeet Rangi wrote:
> I'm ok with the queued patch because it still fixes an important bug
> (disabling interrupts on storm end is not correct). The rest of this
> reply is talking about the concern that storms are not handled
> properly in the AMD driver. I can make that a new patch thread if we
> agree to make the behavior more Intel like.
> 
> On Tue, Aug 18, 2026 at 12:03 PM Borislav Petkov <bp@alien8.de> wrote:
> > On Mon, Aug 17, 2026 at 12:51:08PM -0600, Jasjeet Rangi wrote:
> > > Inverting `on` in mce_amd_handle_storm() alone is not enough because
> > > clear_bank() will immediately and unconditionally enable the interrupt
> > > again.
> >
> > And?
> >
> > What would happen then?
> >
> > Did you read my previous reply?
> 
> On AMD a machine_check_poll() call will not necessarily set ErrCnt to 0.
> In fact, it will not reset ErrCnt at all unless the overflow bit is
> set. And the overflow bit gets set when it's time to generate an
> interrupt. (Yazen and Smita please correct me if I'm wrong)

Right, that's in threshold_restart_block().

> 
> The threshold is not always high on AMD either. Since eeb3f76d73ba
> (x86/mce: Save and use APEI corrected threshold limit)
> it gets the value from the BIOS. The default on an AMD system I was
> using was only 10. So ErrCnt will get reset to 4085 on that system
> when the overflow bit is set. That is enough to not meaningfully
> reduce the interrupt rate during an interrupt storm.
> 
> Unlike Intel, the AMD driver doesn't set the threshold to an extremely
> large value when storm mode is turned on. It keeps it set to the same
> configured value (default from BIOS).
> 
> So if interrupts are left enabled during storm mode like they would be
> even with the queued patch, the AMD driver doesn't really reduce the
> interrupt rate during a storm like the Intel driver does.
> 
> If we don't want to diverge from Intel, then the correct thing to do is
> to update the AMD driver to do what the Intel driver does. I.e. don't
> disable interrupts, but set the threshold_limit to a large value like
> THRESHOLD_MAX during storm on, and restore it to what it was before upon
> storm off.

IMO, disabling the interrupt is the best way to manage the storm. The
reason for Intel to set a high limit is to keep the interrupt enabled.
The reason to keep the interrupt enabled is because the same interrupt
line is used for reporting "uncorrectable,no action" errors. This isn't
necessary on AMD because there's a separate interrupt line for those
errors: Deferred error interrupt.

> 
> > Yeah, we don't protect users from themselves, you know :)
> 
> I don't think having threshold=1 on AMD is too crazy. On the Intel
> driver for example it defaults to 1 if not overridden by the BIOS.
> ```
> /* Linux non-storm CMCI threshold (may be overridden by BIOS) */
> #define CMCI_THRESHOLD		1
> ```
> 
> > We do clear MCi_MISC[ErrCnt] to 0 so that's the "highest' value we can do
> > before we raise an interrupt there :)
> And on AMD it does not get set to 0 after an interrupt. It gets set to
> THRESHOLD_MAX - threshold_limit. So if your limit is 10, the ErrCnt will
> get reset to 4085. This is the behavior since eeb3f76d73ba
> (x86/mce: Save and use APEI corrected threshold limit). Prior to that
> ErrCnt would get reset to 0 by default on AMD.
> 

It's fair to set a low threshold limit. Some users want to see corrected
errors without needing to poll. And they'd like to see them ASAP.

The threshold limit isn't much of a contributor to interrupt storms. A
stuck bit/failing device will likely trigger a storm whether the limit
is '1' or '4095'.

Thanks,
Yazen