[PATCH v5 0/3] x86/mce: Fix timer list corruption and avoid redundant polling

[PATCH v5 0/3] x86/mce: Fix timer list corruption and avoid redundant polling

[Aaron Tomlin]

This series addresses two distinct issues within the x86 Machine Check
Architecture (MCA) timer subsystem: a race condition during runtime CPU
reconfiguration that can corrupt the kernel timer wheel, and redundant
periodic software polling of banks that never log corrected errors.

Patch 1 fixes a concurrency race between sysfs configuration updates
(mce_restart()) and asynchronous CMCI interrupts. When mce_restart() runs,
a concurrent CMCI interrupt can arm mce_timer on a remote CPU before the
restart IPI arrives. By removing the redundant timer_setup() call from
__mcheck_cpu_init_timer(), Patch 1 ensures that mce_timer descriptors are
not re-initialised while actively linked in the timer wheel, avoiding
potential linked-list corruption and kernel crashes.

Patch 2 resolves a lost-update concurrency race on the per-CPU
mce_poll_banks bitmap between timer softirqs (cmci_storm_end()) and CMCI
hardirqs (cmci_storm_begin()). By switching to atomic set_bit() and
clear_bit() operations, Patch 2 prevents interrupted non-atomic
read-modify-write sequences from clobbering bank bits, ensuring banks
entering storm mode are not permanently lost from polling telemetry.

Patch 3 implements Tony Luck's suggested approach by recognising that banks
without CMCI support on modern Intel platforms (such as the PCU bank) never
report corrected or UCNA errors (per Intel SDM Vol 3B 18.5). It clears
these non-CMCI banks from mce_poll_banks and ensures mce_timer is never
armed when mce_poll_banks is empty. Additionally, it integrates a
housekeeping check (HK_TYPE_TIMER) so that on legacy platforms or
polling-only configurations where mce_poll_banks is non-empty, routine
polling is restricted to housekeeping CPUs, sparing isolated nohz_full
cores from timer interrupts. This eliminates polling timer jitter across
all CPUs in steady state on modern hardware while preserving full polling
capabilities and isolation guarantees.

Thank you.

Changes since v4:

 - Added a patch to switch cmci_storm_begin() and cmci_storm_end() to use
   set_bit() and clear_bit(), preventing lost updates on mce_poll_banks
   when a timer softirq is interrupted by a CMCI hardirq (Marco Crivellari)

 - Switched to clear_bit() in cmci_claim_bank()

 - Link to v4: https://lore.kernel.org/lkml/20260903041320.179965-1-atomlin@atomlin.com/

Changes since v3:

 - Removed redundant code since field poll_only of struct storm_bank is no
   longer set

 - Link to v3: https://lore.kernel.org/lkml/20260903013933.172063-1-atomlin@atomlin.com/

Changes since v2:

 - Bounded bitmap_empty() in should_enable_timer() to
   this_cpu_read(mce_num_banks) to prevent initialised upper bits from
   keeping the timer active (Tony Luck)

 - Clarified that on polling fallback systems, restricting mce_timer to
   housekeeping CPUs leaves core-private banks on isolated cores exempt
   from polling, while preserving shared platform telemetry
   (e.g., Memory Controller ECC)

 - Link to v2: https://lore.kernel.org/lkml/20260902020234.149814-1-atomlin@atomlin.com/

Changes since v1:

 - Fixed a pre-existing race condition in mce_restart() by removing the
   redundant timer_setup() call in __mcheck_cpu_init_timer(), preventing
   active timer wheel linked-list corruption

 - Non-CMCI banks are cleared from mce_poll_banks in cmci_claim_bank(),
   and should_enable_timer() verifies bitmap_empty(mce_poll_banks) before
   checking HK_TYPE_TIMER

 - Link to v1: https://lore.kernel.org/lkml/20260901151138.132950-1-atomlin@atomlin.com/

Aaron Tomlin (3):
  x86/mce: Do not reinitialise mce_timer structure on CPU restart
  x86/mce/threshold: Use atomic bit operations on mce_poll_banks
  x86/mce: Avoid arming periodic polling timer when not required

 arch/x86/kernel/cpu/mce/core.c      |  8 +++++++-
 arch/x86/kernel/cpu/mce/intel.c     | 13 +++++++------
 arch/x86/kernel/cpu/mce/internal.h  |  2 --
 arch/x86/kernel/cpu/mce/threshold.c |  8 ++------
 4 files changed, 16 insertions(+), 15 deletions(-)

-- 
2.55.0

[PATCH v5 1/3] x86/mce: Do not reinitialise mce_timer structure on CPU restart

[Aaron Tomlin]

During early CPU initialisation, mcheck_cpu_init() invokes
__mcheck_cpu_setup_timer(), which initialises the per-CPU mce_timer
descriptor via timer_setup(t, mce_timer_fn, TIMER_PINNED).

However, __mcheck_cpu_init_timer() is also called at runtime during
sysfs reconfiguration (via mce_cpu_restart() and mce_enable_ce()),
redundantly re-initialising mce_timer with timer_setup().

This introduces a race condition against asynchronous CMCI interrupts.
When mce_restart() runs, it invokes mce_timer_delete_all() before
broadcasting an IPI to execute mce_cpu_restart() on all CPUs. If a
hardware CMCI interrupt fires on another CPU before the IPI is serviced,
the CMCI handler triggers storm tracking and arms mce_timer via
mce_timer_kick(true), enqueuing it in the active timer wheel.

When mce_cpu_restart() runs subsequently in IPI context,
__mcheck_cpu_init_timer() calls timer_setup(), which invokes
__init_timer() on the already-queued timer. This clears the timer's
internal list pointers (entry.next and entry.pprev) whilst it remains
linked in the active timer wheel bucket, causing linked list corruption,
softirq lockups, or kernel panics during timer expiration.

Fix this by removing the redundant timer_setup() call from
__mcheck_cpu_init_timer(). The timer is already initialised once during
CPU setup; runtime restart paths only need to arm it via
mce_start_timer().

Fixes: 26c3c283c5b0 ("x86: mce: Split timer init")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: stable@vger.kernel.org
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 arch/x86/kernel/cpu/mce/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index ab469605fc89..765e8103b0d2 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -2091,7 +2091,6 @@ static void __mcheck_cpu_init_timer(void)
 {
 	struct timer_list *t = this_cpu_ptr(&mce_timer);
 
-	timer_setup(t, mce_timer_fn, TIMER_PINNED);
 	mce_start_timer(t);
 }
 
-- 
2.55.0

[PATCH v5 2/3] x86/mce/threshold: Use atomic bit operations on mce_poll_banks

[Aaron Tomlin]

cmci_storm_begin() and cmci_storm_end() modify the per-CPU bitmap
mce_poll_banks using non-atomic __set_bit() and __clear_bit().

While mce_poll_banks is a per-CPU variable, cmci_storm_end() runs in timer
softirq context with local hardirqs enabled, whereas cmci_storm_begin()
can be invoked from CMCI hardirq context (via intel_threshold_interrupt()).

If a CMCI hardirq fires while a timer softirq is midway through a
non-atomic read-modify-write operation on mce_poll_banks, the hardirq's
bit update will be overwritten and lost when the softirq resumes.

If a bank entering storm mode loses its bit in mce_poll_banks, it will
neither generate interrupts (as its hardware threshold is bumped to
CMCI_STORM_THRESHOLD) nor be polled by mce_timer_fn(). Consequently, the
bank remains unpolled and its error telemetry is permanently lost until
the next reboot.

Fix this by switching to atomic set_bit() and clear_bit() operations in
cmci_storm_begin() and cmci_storm_end().

Fixes: 7eae17c4add5 ("x86/mce: Add per-bank CMCI storm mitigation")
Cc: stable@vger.kernel.org
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 arch/x86/kernel/cpu/mce/threshold.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mce/threshold.c b/arch/x86/kernel/cpu/mce/threshold.c
index 6c370d5af5bd..af8346f06318 100644
--- a/arch/x86/kernel/cpu/mce/threshold.c
+++ b/arch/x86/kernel/cpu/mce/threshold.c
@@ -86,7 +86,7 @@ void cmci_storm_begin(unsigned int bank)
 {
 	struct mca_storm_desc *storm = this_cpu_ptr(&storm_desc);
 
-	__set_bit(bank, this_cpu_ptr(mce_poll_banks));
+	set_bit(bank, this_cpu_ptr(mce_poll_banks));
 	storm->banks[bank].in_storm_mode = true;
 
 	/*
@@ -102,7 +102,7 @@ void cmci_storm_end(unsigned int bank)
 	struct mca_storm_desc *storm = this_cpu_ptr(&storm_desc);
 
 	if (!mce_flags.amd_threshold)
-		__clear_bit(bank, this_cpu_ptr(mce_poll_banks));
+		clear_bit(bank, this_cpu_ptr(mce_poll_banks));
 	storm->banks[bank].history = 0;
 	storm->banks[bank].in_storm_mode = false;
 
-- 
2.55.0

[PATCH v5 3/3] x86/mce: Avoid arming periodic polling timer when not required

[Aaron Tomlin]

On x86 platforms, the Machine Check Architecture (MCA) subsystem arms a
per-CPU, pinned standard timer (mce_timer) to periodically poll hardware
banks for "silent" corrected machine check errors. Because mce_timer is
pinned to the local CPU via TIMER_PINNED, the timer core cannot migrate
its expiration to a housekeeping CPU, causing periodic latency jitter on
isolated cores.

On Intel systems supporting Corrected Machine Check Interrupt (CMCI),
hardware generates an interrupt for banks with CMCI enabled, clearing
their respective bits in mce_poll_banks. However, for banks where
MCI_CTL2_CMCI_EN does not stick (such as the Power Control Unit bank),
Linux historically assumed software polling was required and left the
bank flagged in mce_poll_banks.

Per the Intel SDM (Vol 3B, Section 18.5 "Corrected Machine Check Error
Interrupt"), if bit 30 of IA32_MCi_CTL2 is zero, no CMCI is available
for that bank and no corrected or Uncorrected No Action Required (UCNA)
errors will be reported on that bank. Therefore, polling such banks is
redundant and wasteful.

Clear non-CMCI banks from mce_poll_banks in cmci_claim_bank() on
CMCI-capable CPUs, and amend should_enable_timer() to:
    1.  Check bitmap_empty(mce_poll_banks) bounded to the number of
        implemented banks (this_cpu_read(mce_num_banks)) so that
        mce_timer is never armed when no banks require polling.

    2.  Pin the MCE polling timer to a HK_TYPE_TIMER housekeeping CPU,
        sparing isolated nohz_full cores from periodic wakeups on
        systems requiring mce_poll_banks polling.

On systems requiring software polling such as legacy platforms lacking CMCI
or when booted with mce=no_cmci, mce_timer is restricted to
housekeeping CPUs. While this intentionally leaves core-private banks
(e.g. L1/L2 caches) on isolated cores exempt from periodic polling to
guarantee zero timer jitter, housekeeping CPUs continue to harvest
telemetry from their own banks as well as shared package-level
resources. Synchronous (#MC) exceptions on isolated cores remain
entirely unaffected.

Suggested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 arch/x86/kernel/cpu/mce/core.c      |  7 +++++++
 arch/x86/kernel/cpu/mce/intel.c     | 13 +++++++------
 arch/x86/kernel/cpu/mce/internal.h  |  2 --
 arch/x86/kernel/cpu/mce/threshold.c |  4 ----
 4 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 765e8103b0d2..b01ad1a9b455 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -25,6 +25,7 @@
 #include <linux/delay.h>
 #include <linux/ctype.h>
 #include <linux/sched.h>
+#include <linux/sched/isolation.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
 #include <linux/slab.h>
@@ -1759,6 +1760,12 @@ void (*mc_poll_banks)(void) = mc_poll_banks_default;
 
 static bool should_enable_timer(unsigned long iv)
 {
+	if (bitmap_empty(this_cpu_ptr(mce_poll_banks), this_cpu_read(mce_num_banks)))
+		return false;
+
+	if (!housekeeping_cpu(smp_processor_id(), HK_TYPE_TIMER))
+		return false;
+
 	return !mca_cfg.ignore_ce && iv;
 }
 
diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c
index 4655223ba560..d264ec8688bb 100644
--- a/arch/x86/kernel/cpu/mce/intel.c
+++ b/arch/x86/kernel/cpu/mce/intel.c
@@ -229,16 +229,17 @@ static u64 cmci_pick_threshold(u64 val, int *bios_zero_thresh)
  */
 static void cmci_claim_bank(int bank, u64 val, int bios_zero_thresh, int *bios_wrong_thresh)
 {
-	struct mca_storm_desc *storm = this_cpu_ptr(&storm_desc);
-
 	val |= MCI_CTL2_CMCI_EN;
 	wrmsrq(MSR_IA32_MCx_CTL2(bank), val);
 	rdmsrq(MSR_IA32_MCx_CTL2(bank), val);
 
-	/* If the enable bit did not stick, this bank should be polled. */
+	/*
+	 * If the enable bit did not stick, this bank does not support CMCI
+	 * and no corrected or UCNA errors will be reported on this bank
+	 * (SDM Vol 3B 18.5). No polling is needed.
+	 */
 	if (!(val & MCI_CTL2_CMCI_EN)) {
-		WARN_ON(!test_bit(bank, this_cpu_ptr(mce_poll_banks)));
-		storm->banks[bank].poll_only = true;
+		clear_bit(bank, this_cpu_ptr(mce_poll_banks));
 		return;
 	}
 
@@ -250,7 +251,7 @@ static void cmci_claim_bank(int bank, u64 val, int bios_zero_thresh, int *bios_w
 		mce_inherit_storm(bank);
 		cmci_storm_begin(bank);
 	} else {
-		__clear_bit(bank, this_cpu_ptr(mce_poll_banks));
+		clear_bit(bank, this_cpu_ptr(mce_poll_banks));
 	}
 
 	/*
diff --git a/arch/x86/kernel/cpu/mce/internal.h b/arch/x86/kernel/cpu/mce/internal.h
index a31cf984619c..32ae574d2ad5 100644
--- a/arch/x86/kernel/cpu/mce/internal.h
+++ b/arch/x86/kernel/cpu/mce/internal.h
@@ -84,13 +84,11 @@ static inline u32  mce_get_apei_thr_limit(void) { return 0; }
  *
  * timestamp:		Last time (in jiffies) that the bank was polled.
  * in_storm_mode:	Is this bank in storm mode?
- * poll_only:		Bank does not support CMCI, skip storm tracking.
  */
 struct storm_bank {
 	u64 history;
 	u64 timestamp;
 	bool in_storm_mode;
-	bool poll_only;
 };
 
 #define NUM_HISTORY_BITS (sizeof(u64) * BITS_PER_BYTE)
diff --git a/arch/x86/kernel/cpu/mce/threshold.c b/arch/x86/kernel/cpu/mce/threshold.c
index af8346f06318..af8936262613 100644
--- a/arch/x86/kernel/cpu/mce/threshold.c
+++ b/arch/x86/kernel/cpu/mce/threshold.c
@@ -118,10 +118,6 @@ void mce_track_storm(struct mce *mce)
 	unsigned int shift = 1;
 	u64 history = 0;
 
-	/* No tracking needed for banks that do not support CMCI */
-	if (storm->banks[mce->bank].poll_only)
-		return;
-
 	/*
 	 * When a bank is in storm mode it is polled once per second and
 	 * the history mask will record about the last minute of poll results.
-- 
2.55.0

Re: [PATCH v5 0/3] x86/mce: Fix timer list corruption and avoid redundant polling

[Aaron Tomlin]

On Thu, Sep 03, 2026 at 03:41:27PM -0400, Aaron Tomlin wrote:
> This series addresses two distinct issues within the x86 Machine Check
> Architecture (MCA) timer subsystem: a race condition during runtime CPU
> reconfiguration that can corrupt the kernel timer wheel, and redundant
> periodic software polling of banks that never log corrected errors.
> 
> Patch 1 fixes a concurrency race between sysfs configuration updates
> (mce_restart()) and asynchronous CMCI interrupts. When mce_restart() runs,
> a concurrent CMCI interrupt can arm mce_timer on a remote CPU before the
> restart IPI arrives. By removing the redundant timer_setup() call from
> __mcheck_cpu_init_timer(), Patch 1 ensures that mce_timer descriptors are
> not re-initialised while actively linked in the timer wheel, avoiding
> potential linked-list corruption and kernel crashes.
> 
> Patch 2 resolves a lost-update concurrency race on the per-CPU
> mce_poll_banks bitmap between timer softirqs (cmci_storm_end()) and CMCI
> hardirqs (cmci_storm_begin()). By switching to atomic set_bit() and
> clear_bit() operations, Patch 2 prevents interrupted non-atomic
> read-modify-write sequences from clobbering bank bits, ensuring banks
> entering storm mode are not permanently lost from polling telemetry.
> 
> Patch 3 implements Tony Luck's suggested approach by recognising that banks
> without CMCI support on modern Intel platforms (such as the PCU bank) never
> report corrected or UCNA errors (per Intel SDM Vol 3B 18.5). It clears
> these non-CMCI banks from mce_poll_banks and ensures mce_timer is never
> armed when mce_poll_banks is empty. Additionally, it integrates a
> housekeeping check (HK_TYPE_TIMER) so that on legacy platforms or
> polling-only configurations where mce_poll_banks is non-empty, routine
> polling is restricted to housekeeping CPUs, sparing isolated nohz_full
> cores from timer interrupts. This eliminates polling timer jitter across
> all CPUs in steady state on modern hardware while preserving full polling
> capabilities and isolation guarantees.
> 
> Thank you.
> 
> Changes since v4:
> 
>  - Added a patch to switch cmci_storm_begin() and cmci_storm_end() to use
>    set_bit() and clear_bit(), preventing lost updates on mce_poll_banks
>    when a timer softirq is interrupted by a CMCI hardirq (Marco Crivellari)
> 
>  - Switched to clear_bit() in cmci_claim_bank()
> 
>  - Link to v4: https://lore.kernel.org/lkml/20260903041320.179965-1-atomlin@atomlin.com/
> 
> Changes since v3:
> 
>  - Removed redundant code since field poll_only of struct storm_bank is no
>    longer set
> 
>  - Link to v3: https://lore.kernel.org/lkml/20260903013933.172063-1-atomlin@atomlin.com/
> 
> Changes since v2:
> 
>  - Bounded bitmap_empty() in should_enable_timer() to
>    this_cpu_read(mce_num_banks) to prevent initialised upper bits from
>    keeping the timer active (Tony Luck)
> 
>  - Clarified that on polling fallback systems, restricting mce_timer to
>    housekeeping CPUs leaves core-private banks on isolated cores exempt
>    from polling, while preserving shared platform telemetry
>    (e.g., Memory Controller ECC)
> 
>  - Link to v2: https://lore.kernel.org/lkml/20260902020234.149814-1-atomlin@atomlin.com/
> 
> Changes since v1:
> 
>  - Fixed a pre-existing race condition in mce_restart() by removing the
>    redundant timer_setup() call in __mcheck_cpu_init_timer(), preventing
>    active timer wheel linked-list corruption
> 
>  - Non-CMCI banks are cleared from mce_poll_banks in cmci_claim_bank(),
>    and should_enable_timer() verifies bitmap_empty(mce_poll_banks) before
>    checking HK_TYPE_TIMER
> 
>  - Link to v1: https://lore.kernel.org/lkml/20260901151138.132950-1-atomlin@atomlin.com/
> 
> Aaron Tomlin (3):
>   x86/mce: Do not reinitialise mce_timer structure on CPU restart
>   x86/mce/threshold: Use atomic bit operations on mce_poll_banks
>   x86/mce: Avoid arming periodic polling timer when not required
> 
>  arch/x86/kernel/cpu/mce/core.c      |  8 +++++++-
>  arch/x86/kernel/cpu/mce/intel.c     | 13 +++++++------
>  arch/x86/kernel/cpu/mce/internal.h  |  2 --
>  arch/x86/kernel/cpu/mce/threshold.c |  8 ++------
>  4 files changed, 16 insertions(+), 15 deletions(-)
> 
> -- 
> 2.55.0
> 

Hi Tony,

Sashiko [1] has correctly highlighted additional pre-existing race
conditions in this area. Should you prefer, I would be more than happy to
incorporate the fixes for these into the current series?

[1]: https://sashiko.dev/#/patchset/20260903194130.186096-1-atomlin%40atomlin.com

Kind regards,
-- 
Aaron Tomlin

RE: [PATCH v5 0/3] x86/mce: Fix timer list corruption and avoid redundant polling

[Luck, Tony]

Hi Aaron,

> Sashiko [1] has correctly highlighted additional pre-existing race
> conditions in this area. Should you prefer, I would be more than happy to
> incorporate the fixes for these into the current series?
>
> [1]: https://sashiko.dev/#/patchset/20260903194130.186096-1-atomlin%40atomlin.com

I'm not sure about the first Sashiko issue ... there is a wrong statement:
 
   Since the hardware threshold for a stormy bank is set to
   CMCI_STORM_THRESHOLD and no longer generates interrupts,

Setting the storm threshold doesn't disable interrupts. It just prevents
generation of a new interrupt from a bank until enough errors are logged
to meet the threshold. So the user won't see logs for a while. But should
another storm occur, then things will fix themselves without a reboot.

If you see an elegant solution to this race, then go ahead with a patch. But
I wouldn't stress if this one isn't fixed.

The second report regarding firmware first banks does look easy to solve.
Just change cmci_skip_banks() to clear the bit in mce_poll_banks?

-Tony

Re: [PATCH v5 0/3] x86/mce: Fix timer list corruption and avoid redundant polling

[Borislav Petkov]

On Thu, Sep 03, 2026 at 03:41:27PM -0400, Aaron Tomlin wrote:
> This series addresses two distinct issues within the x86 Machine Check
> Architecture (MCA) timer subsystem: a race condition during runtime CPU
> reconfiguration that can corrupt the kernel timer wheel, and redundant
> periodic software polling of banks that never log corrected errors.

N T Sep 01              Aaron Tomlin ( :2.6K|) [RFC PATCH] x86/mce: Avoid arming periodic polling timer on isolated CPUs
N T Sep 01              Aaron Tomlin ( :2.3K|) [PATCH v2 0/2] x86/mce: Fix timer list corruption and avoid redundant polling
N T Sep 02              Aaron Tomlin ( :3.0K|) [PATCH v3 0/2] x86/mce: Fix timer list corruption and avoid redundant polling
N T Sep 03              Aaron Tomlin ( :3.1K|) [PATCH v4 0/2] x86/mce: Fix timer list corruption and avoid redundant polling
N T Sep 03              Aaron Tomlin ( :4.0K|) [PATCH v5 0/3] x86/mce: Fix timer list corruption and avoid redundant polling

5 submissions on the same topic in 3 days.

Please stop spamming!

From: Documentation/process/submitting-patches.rst

Don't get discouraged - or impatient
------------------------------------

After you have submitted your change, be patient and wait.  Reviewers are
busy people and may not get to your patch right away.

Once upon a time, patches used to disappear into the void without comment,
but the development process works more smoothly than that now.  You should
receive comments within a week or so; if that does not happen, make sure
that you have sent your patches to the right place.  Wait for a minimum of
one week before resubmitting or pinging reviewers - possibly longer during
busy times like merge windows.

Pls read the documentation about how you should work with the community:

Documentation/process/

while you're waiting for review.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v5 0/3] x86/mce: Fix timer list corruption and avoid redundant polling

[Aaron Tomlin]

On Thu, Sep 03, 2026 at 03:17:45PM -0700, Borislav Petkov wrote:
> On Thu, Sep 03, 2026 at 03:41:27PM -0400, Aaron Tomlin wrote:
> > This series addresses two distinct issues within the x86 Machine Check
> > Architecture (MCA) timer subsystem: a race condition during runtime CPU
> > reconfiguration that can corrupt the kernel timer wheel, and redundant
> > periodic software polling of banks that never log corrected errors.
> 
> N T Sep 01              Aaron Tomlin ( :2.6K|) [RFC PATCH] x86/mce: Avoid arming periodic polling timer on isolated CPUs
> N T Sep 01              Aaron Tomlin ( :2.3K|) [PATCH v2 0/2] x86/mce: Fix timer list corruption and avoid redundant polling
> N T Sep 02              Aaron Tomlin ( :3.0K|) [PATCH v3 0/2] x86/mce: Fix timer list corruption and avoid redundant polling
> N T Sep 03              Aaron Tomlin ( :3.1K|) [PATCH v4 0/2] x86/mce: Fix timer list corruption and avoid redundant polling
> N T Sep 03              Aaron Tomlin ( :4.0K|) [PATCH v5 0/3] x86/mce: Fix timer list corruption and avoid redundant polling
> 
> 5 submissions on the same topic in 3 days.

Hi Boris,

Understood, and apologies for the noise on the list.

The rapid cadence was not borne out of impatience, but rather active,
iterative engagement with Tony Luck and Marco Crivellari to incorporate
their direct feedback, alongside addressing pre-existing issues flagged by
Sashiko. Each revision sought to turn around those specific corrections
promptly.

That being said, I appreciate the review backlog this creates and will
ensure subsequent work is given appropriate time to settle before posting
further revisions, unless instructed otherwise.


Kind regards,
-- 
Aaron Tomlin

Re: [PATCH v5 0/3] x86/mce: Fix timer list corruption and avoid redundant polling

[Borislav Petkov]

On Thu, Sep 03, 2026 at 08:30:40PM -0400, Aaron Tomlin wrote:
> The rapid cadence was not borne out of impatience, but rather active,
> iterative engagement with Tony Luck and Marco Crivellari to incorporate
> their direct feedback, alongside addressing pre-existing issues flagged by
> Sashiko. Each revision sought to turn around those specific corrections
> promptly.

How would you feel if I am doing a patchset and I send you a new version each
time I fix an issue there?

That's not how this is done.

You collect *all* feedback, you reply to the list with your feedback to
Sashiko review so that people can see whether that is going to be addressed or
it doesn't make sense after explaining why, you test your patchset thoroughly and
*then* you send again.

You don't have to wait a whole week but you have to give people a chance to
take a look at your pile. Note that you're not the only one sending patches.

How about you try to review some patches of other people instead of only
sending, while waiting for people to take a look at yours?

And perhaps test some -rc kernels and report issues?

That would be of immense help than simply only sending patches.

Balancing the generation of new code and reviewing of other code on the ML
would be the optimal thing to do, I would say. And very much appreciated by
maintainers - much more than only sending patches.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v5 0/3] x86/mce: Fix timer list corruption and avoid redundant polling

[Aaron Tomlin]

On Thu, Sep 03, 2026 at 05:45:28PM -0700, Borislav Petkov wrote:
> On Thu, Sep 03, 2026 at 08:30:40PM -0400, Aaron Tomlin wrote:
> > The rapid cadence was not borne out of impatience, but rather active,
> > iterative engagement with Tony Luck and Marco Crivellari to incorporate
> > their direct feedback, alongside addressing pre-existing issues flagged by
> > Sashiko. Each revision sought to turn around those specific corrections
> > promptly.
> 
> How would you feel if I am doing a patchset and I send you a new version each
> time I fix an issue there?
> 
> That's not how this is done.
> 
> You collect *all* feedback, you reply to the list with your feedback to
> Sashiko review so that people can see whether that is going to be addressed or
> it doesn't make sense after explaining why, you test your patchset thoroughly and
> *then* you send again.
> 
> You don't have to wait a whole week but you have to give people a chance to
> take a look at your pile. Note that you're not the only one sending patches.
> 
> How about you try to review some patches of other people instead of only
> sending, while waiting for people to take a look at yours?
> 
> And perhaps test some -rc kernels and report issues?
> 
> That would be of immense help than simply only sending patches.
> 
> Balancing the generation of new code and reviewing of other code on the ML
> would be the optimal thing to do, I would say. And very much appreciated by
> maintainers - much more than only sending patches.
> 
> Thx.

Hi Boris,

Fair point, and taken entirely.

Thank you for the guidance.

Kind regards,
-- 
Aaron Tomlin

Re: [PATCH v5 0/3] x86/mce: Fix timer list corruption and avoid redundant polling

[Aaron Tomlin]

On Thu, Sep 03, 2026 at 10:02:56PM +0000, Luck, Tony wrote:
> Hi Aaron,
> 
> > Sashiko [1] has correctly highlighted additional pre-existing race
> > conditions in this area. Should you prefer, I would be more than happy to
> > incorporate the fixes for these into the current series?
> >
> > [1]: https://sashiko.dev/#/patchset/20260903194130.186096-1-atomlin%40atomlin.com
> 
> I'm not sure about the first Sashiko issue ... there is a wrong statement:
>  
>    Since the hardware threshold for a stormy bank is set to
>    CMCI_STORM_THRESHOLD and no longer generates interrupts,
> 
> Setting the storm threshold doesn't disable interrupts. It just prevents
> generation of a new interrupt from a bank until enough errors are logged
> to meet the threshold. So the user won't see logs for a while. But should
> another storm occur, then things will fix themselves without a reboot.
> 
> If you see an elegant solution to this race, then go ahead with a patch. But
> I wouldn't stress if this one isn't fixed.
> 
> The second report regarding firmware first banks does look easy to solve.
> Just change cmci_skip_banks() to clear the bit in mce_poll_banks?
> 
> -Tony

Hi Tony,

Yes, you are entirely right regarding the first report. Sashiko's assertion
that error telemetry is permanently lost until reboot is incorrect.
In arch/x86/kernel/cpu/mce/intel.c, I see:

        #define CMCI_STORM_THRESHOLD    32749

Setting the hardware threshold to 32749 merely defers further interrupts
until that count is reached; once enough errors accumulate, hardware
triggers a CMCI interrupt, and cmci_storm_begin() restores active storm
polling.

However, the preemption window I believe is real. Because cmci_storm_end()
runs in timer softirq context (via mce_timer_fn()) with local interrupts
enabled:

        void cmci_storm_end(unsigned int bank)
        {
            ...

            /* If no banks left in storm mode, stop polling. */
            if (!--storm->stormy_bank_count)
                mce_timer_kick(false);
        }

If a CMCI hardirq preempts the CPU after stormy_bank_count is decremented
to zero, but before mce_timer_kick(false) is called, the hardirq's
invocation of cmci_storm_begin() will increment stormy_bank_count to 1 and
call mce_timer_kick(true). When the softirq resumes, its delayed
mce_timer_kick(false) will erroneously override storm mode, leaving the CPU
with stormy_bank_count == 1 while the timer reverts to the 5-minute
interval.

An elegant solution is to protect the storm transitions in both
cmci_storm_begin() and cmci_storm_end() using local_irq_save() and
local_irq_restore(). This serialises the counter updates and timer kicks
against local hardirq preemption:

--- a/arch/x86/kernel/cpu/mce/threshold.c
+++ b/arch/x86/kernel/cpu/mce/threshold.c
@@ -85,29 +85,37 @@ static void mce_handle_storm(unsigned int bank, bool on)
 void cmci_storm_begin(unsigned int bank)
 {
     struct mca_storm_desc *storm = this_cpu_ptr(&storm_desc);
+    unsigned long flags;

+    local_irq_save(flags);
     set_bit(bank, this_cpu_ptr(mce_poll_banks));
     storm->banks[bank].in_storm_mode = true;

     /*
      * If this is the first bank on this CPU to enter storm mode
      * start polling.
      */
     if (++storm->stormy_bank_count == 1)
         mce_timer_kick(true);
+    local_irq_restore(flags);
 }

 void cmci_storm_end(unsigned int bank)
 {
     struct mca_storm_desc *storm = this_cpu_ptr(&storm_desc);
+    unsigned long flags;

+    local_irq_save(flags);
     if (!mce_flags.amd_threshold)
         clear_bit(bank, this_cpu_ptr(mce_poll_banks));
     storm->banks[bank].history = 0;
     storm->banks[bank].in_storm_mode = false;

     /* If no banks left in storm mode, stop polling. */
     if (!--storm->stormy_bank_count)
         mce_timer_kick(false);
+    local_irq_restore(flags);
 }

Now, regarding the second report, Sashiko appears to be correct. During
early boot, acpi_hest_init() -> mce_disable_bank() broadcasts via
on_each_cpu() to clear Firmware First banks from mce_poll_banks, but CPUs
that are brought online late or physically hotplugged miss this broadcast.

When those CPUs come online, cmci_skip_bank() currently bails out early
without clearing mce_poll_banks:

        /* Skip banks in firmware first mode */
        if (test_bit(bank, mce_banks_ce_disabled))
            return true;

Because mce_poll_banks is statically initialised to ~0UL, the bit remains
set, defeating bitmap_empty() on hotplugged CPUs and causing mce_timer_fn()
to periodically poll and clear Firmware First status registers.

Clearing the bit in cmci_skip_bank() resolves this cleanly:

--- a/arch/x86/kernel/cpu/mce/intel.c
+++ b/arch/x86/kernel/cpu/mce/intel.c
@@ -181,8 +181,10 @@ static bool cmci_skip_bank(int bank, u64 *val)
     if (test_bit(bank, owned))
         return true;

     /* Skip banks in firmware first mode */
-    if (test_bit(bank, mce_banks_ce_disabled))
+    if (test_bit(bank, mce_banks_ce_disabled)) {
+        clear_bit(bank, this_cpu_ptr(mce_poll_banks));
         return true;
+    }

     rdmsrq(MSR_IA32_MCx_CTL2(bank), *val);

If you are happy with these two changes, I will fold the local_irq_save()
fix into Patch 2/3 and the cmci_skip_bank() fix into Patch 3/3 for v6.


Kind regards,
-- 
Aaron Tomlin

Re: [PATCH v5 0/3] x86/mce: Fix timer list corruption and avoid redundant polling

[Luck, Tony]

On Fri, Sep 04, 2026 at 09:54:18AM -0400, Aaron Tomlin wrote:
> An elegant solution is to protect the storm transitions in both
> cmci_storm_begin() and cmci_storm_end() using local_irq_save() and
> local_irq_restore(). This serialises the counter updates and timer kicks
> against local hardirq preemption:
> 
> --- a/arch/x86/kernel/cpu/mce/threshold.c
> +++ b/arch/x86/kernel/cpu/mce/threshold.c
> @@ -85,29 +85,37 @@ static void mce_handle_storm(unsigned int bank, bool on)
>  void cmci_storm_begin(unsigned int bank)
>  {
>      struct mca_storm_desc *storm = this_cpu_ptr(&storm_desc);
> +    unsigned long flags;
> 
> +    local_irq_save(flags);
>      set_bit(bank, this_cpu_ptr(mce_poll_banks));
>      storm->banks[bank].in_storm_mode = true;
> 
>      /*
>       * If this is the first bank on this CPU to enter storm mode
>       * start polling.
>       */
>      if (++storm->stormy_bank_count == 1)
>          mce_timer_kick(true);
> +    local_irq_restore(flags);
>  }
> 
>  void cmci_storm_end(unsigned int bank)
>  {
>      struct mca_storm_desc *storm = this_cpu_ptr(&storm_desc);
> +    unsigned long flags;
> 
> +    local_irq_save(flags);
>      if (!mce_flags.amd_threshold)
>          clear_bit(bank, this_cpu_ptr(mce_poll_banks));
>      storm->banks[bank].history = 0;
>      storm->banks[bank].in_storm_mode = false;
> 
>      /* If no banks left in storm mode, stop polling. */
>      if (!--storm->stormy_bank_count)
>          mce_timer_kick(false);
> +    local_irq_restore(flags);
>  }

I ran this past an internal AI, and it said there was still a race in
mce_track_storm(). This test:

	if (storm->banks[mce->bank].in_storm_mode) {

is made with interrupts enabled, so another CMCI immediately after
picking which of the if/else paths to take could change the value of
in_storm_mode which then leads to corruption of the storm state machine.

> Now, regarding the second report, Sashiko appears to be correct. During
> early boot, acpi_hest_init() -> mce_disable_bank() broadcasts via
> on_each_cpu() to clear Firmware First banks from mce_poll_banks, but CPUs
> that are brought online late or physically hotplugged miss this broadcast.
> 
> When those CPUs come online, cmci_skip_bank() currently bails out early
> without clearing mce_poll_banks:
> 
>         /* Skip banks in firmware first mode */
>         if (test_bit(bank, mce_banks_ce_disabled))
>             return true;
> 
> Because mce_poll_banks is statically initialised to ~0UL, the bit remains
> set, defeating bitmap_empty() on hotplugged CPUs and causing mce_timer_fn()
> to periodically poll and clear Firmware First status registers.
> 
> Clearing the bit in cmci_skip_bank() resolves this cleanly:
> 
> --- a/arch/x86/kernel/cpu/mce/intel.c
> +++ b/arch/x86/kernel/cpu/mce/intel.c
> @@ -181,8 +181,10 @@ static bool cmci_skip_bank(int bank, u64 *val)
>      if (test_bit(bank, owned))
>          return true;
> 
>      /* Skip banks in firmware first mode */
> -    if (test_bit(bank, mce_banks_ce_disabled))
> +    if (test_bit(bank, mce_banks_ce_disabled)) {
> +        clear_bit(bank, this_cpu_ptr(mce_poll_banks));
>          return true;
> +    }

This looks right.

>      rdmsrq(MSR_IA32_MCx_CTL2(bank), *val);
> 
> If you are happy with these two changes, I will fold the local_irq_save()
> fix into Patch 2/3 and the cmci_skip_bank() fix into Patch 3/3 for v6.
> 

I don't think these fixes should be folded into existing patches in this
series. They are distinct changes fixing specific long standing issues.
They deserve their own patches under the "one change per patch" doctrine.

Also we still have:

static void __mce_disable_bank(void *arg)
{
        int bank = *((int *)arg);
        __clear_bit(bank, this_cpu_ptr(mce_poll_banks));
        cmci_disable_bank(bank);
}

That should switch over to the atomic clear_bank() or there should
be a comment on why non-atomic is OK here and bad everywhere else.

-Tony