[PATCH 0/3] Fix NXP STM clockevent bugs causing lockups

[PATCH 0/3] Fix NXP STM clockevent bugs causing lockups

[Enric Balletbo i Serra via B4 Relay]

This series fixes three bugs in the NXP STM clocksource driver that can
cause soft lockups and missed timer events, particularly visible on
PREEMPT_RT kernels running on S32G platforms.

The NXP STM generates an interrupt only on exact match between the
counter and comparator registers. If the counter passes the programmed
value, the match is missed and the hardware must count through all 2^32
ticks (~32.2s at 133 MHz) before firing, causing soft lockups.

Patch 1 fixes max_delta: on 64-bit platforms, ULONG_MAX exceeds the
hardware's 32-bit compare register width, making it possible to program
an unreachable compare value.

Patch 2 fixes the overshoot detection in set_next_event: the existing
check only detected counter wraps, not the normal case where the counter
advances past the programmed value during the read-modify-write window.
Replace it with a signed comparison following the pattern used by
timer-imx-gpt.

Patch 3 increases min_delta from 2us to 20us: under bus contention on
PREEMPT_RT, 2us is not enough margin and the counter frequently
overshoots the programmed value, triggering repeated -ETIME retries.

The lockup can be reproduced on an unpatched S32G board with:

  # ./trigger_lockup &
  # stress-ng --matrix 0 --memthrash 0 & dmesg -w | grep -i "soft lockup"

Where trigger_lockup.c is:

  #define _GNU_SOURCE
  #include <stdio.h>
  #include <stdlib.h>
  #include <time.h>
  #include <pthread.h>
  #include <sched.h>
  #include <unistd.h>

  void *timer_hammer(void *arg) {
      long cpu = (long)arg;
      cpu_set_t cpuset;
      CPU_ZERO(&cpuset);
      CPU_SET(cpu, &cpuset);
      pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuset);

      /* Request 1 ns to force the kernel to clamp to min_delta (2 us) */
      struct timespec req = { .tv_sec = 0, .tv_nsec = 1 };

      while (1) {
          clock_nanosleep(CLOCK_MONOTONIC, 0, &req, NULL);
      }
      return NULL;
  }

  int main(void) {
      long ncpus = sysconf(_SC_NPROCESSORS_ONLN);
      pthread_t threads[ncpus];

      printf("[+] Hammering hrtimers with minimal deltas across %ld cores...\n", ncpus);
      printf("[+] Check dmesg for: 'BUG: soft lockup - CPU#... stuck for 32s!'\n");

      for (long i = 0; i < ncpus; i++) {
          pthread_create(&threads[i], NULL, timer_hammer, (void *)i);
      }

      for (long i = 0; i < ncpus; i++) {
          pthread_join(threads[i], NULL);
      }
      return 0;
  }

The soft lockup trace looks like:

  watchdog: BUG: soft lockup - CPU#1 stuck for 32s! [swapper/1:0]
  Modules linked in: nft_ct nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib
   nft_masq nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4
   nf_tables veth bridge stp llc rfkill binfmt_misc nls_ascii nls_cp437 vfat
   fat ina2xx rtc_pcf85063 regmap_spi micrel phy_package aquantia crc_itu_t
   ci_hdrc_imx dwmac_s32 crct10dif_ce ci_hdrc ghash_ce stmmac_platform
   sha2_ce sha256_arm64 sha1_ce udc_core stmmac ulpi phy_generic pcs_xpcs
   usbmisc_imx phylink ptp pps_core i2c_imx fsl_edma virt_dma flexcan
   can_dev s32g_wdt watchdog sch_fq_codel drm fuse drm_panel_orientation_quirks
   nfnetlink dmi_sysfs ipv6
  CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Tainted: G               X   ------  ---  6.12.0-274.el10iv.aarch64 #1 PREEMPT_RT
  Tainted: [X]=AUX
  Hardware name: Unknown Unknown Product/Unknown Product, BIOS 2022.04-15.el10_4 04/01/2022
  pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : default_idle_call+0x3c/0x170
  lr : default_idle_call+0x30/0x170
  sp : ffff80008011bdd0
  x29: ffff80008011bdd0 x28: 0000000000000000 x27: 0000000000000000
  x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
  x23: 00000000000000c0 x22: ffff0008803ac840 x21: ffff0008803ac840
  x20: 0000000000000000 x19: 0000000000000000 x18: 0000000000000000
  x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
  x14: 003369266b813144 x13: 0000000000000000 x12: 00000000000002eb
  x11: 00000000000000c0 x10: 00000000000010a0 x9 : ffff800080350ff4
  x8 : ffff0008803ad940 x7 : 0000000000000000 x6 : 0000000000000000
  x5 : 0000000000000004 x4 : 0000000000c1c684 x3 : 0000000000000001
  x2 : ffff0008803ac840 x1 : ffff800081712628 x0 : 0000000000000001
  Call trace:
   default_idle_call+0x3c/0x170 (P)
   cpuidle_idle_call+0x1a8/0x248
   do_idle+0xa4/0x118
   cpu_startup_entry+0x3c/0x50
   secondary_start_kernel+0x124/0x168
   __secondary_switched+0xc0/0xc8
  hrtimer: interrupt took 7246 ns

Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
Enric Balletbo i Serra (3):
      clocksource/drivers/nxp-stm: Fix max_delta overflow on 64-bit platforms
      clocksource/drivers/nxp-stm: Fix missed comparator match in set_next_event
      clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT

 drivers/clocksource/timer-nxp-stm.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)
---
base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
change-id: 20261023-nxp-stm-fixes-ab0163517e6f

Best regards,
-- 
Enric Balletbo i Serra <eballetb@redhat.com>

[PATCH 1/3] clocksource/drivers/nxp-stm: Fix max_delta overflow on 64-bit platforms

[Enric Balletbo i Serra via B4 Relay]

From: Enric Balletbo i Serra <eballetb@redhat.com>

The NXP STM timer has a 32-bit counter and 32-bit compare registers.
Using ULONG_MAX as the max_delta parameter for
clockevents_config_and_register() is incorrect on 64-bit platforms where
unsigned long is 64 bits, as this exceeds the hardware's capability.

Cap the max_delta to 0x7FFFFFFFU to match the 32-bit compare channel
width, avoiding potential timer event misses or unexpected delays caused
by programming an unreachable compare value.

Fixes: cec32ac75827 ("clocksource/drivers/nxp-timer: Add the System Timer Module for the s32gx platforms")

Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
 drivers/clocksource/timer-nxp-stm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
index 6fe098a4a33f..ee249fed1b91 100644
--- a/drivers/clocksource/timer-nxp-stm.c
+++ b/drivers/clocksource/timer-nxp-stm.c
@@ -346,7 +346,7 @@ static int nxp_stm_clockevent_starting_cpu(unsigned int cpu)
 	 * minimum ticks = (rate / MICRO) * 2
 	 */
 	clockevents_config_and_register(&stm_timer->ced, stm_timer->rate,
-					(stm_timer->rate / MICRO) * 2, ULONG_MAX);
+					(stm_timer->rate / MICRO) * 2, 0x7FFFFFFFU);
 
 	return 0;
 }

-- 
2.52.0

[PATCH 2/3] clocksource/drivers/nxp-stm: Fix missed comparator match in set_next_event

[Enric Balletbo i Serra via B4 Relay]

From: Enric Balletbo i Serra <eballetb@redhat.com>

The NXP STM generates an interrupt only on exact match between STM_CNT
and STM_CMP0. If the counter passes the comparator value before the
overshoot check runs, the match is missed and the hardware must count
through all 2^32 ticks (~32.2s at 133 MHz) before firing, causing soft
lockups.

The existing overshoot check:
    if (val > nxp_stm_clockevent_read_counter(stm_timer) + delta)
simplifies to checking if counter_before > counter_after, which only
detects a 32-bit counter wrap and misses the normal overshoot case where
the counter advances past val.

Use a signed comparison on the difference between the target and current
counter values instead, following the established pattern in
timer-imx-gpt. If the counter has reached or passed val, the signed
difference goes to zero or negative and we return -ETIME so the
clockevents layer retries immediately.

Fixes: cec32ac75827 ("clocksource/drivers/nxp-timer: Add the System Timer Module for the s32gx platforms")

Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
 drivers/clocksource/timer-nxp-stm.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
index ee249fed1b91..ed7662a4e2bc 100644
--- a/drivers/clocksource/timer-nxp-stm.c
+++ b/drivers/clocksource/timer-nxp-stm.c
@@ -247,7 +247,7 @@ static int nxp_stm_clockevent_shutdown(struct clock_event_device *ced)
 static int nxp_stm_clockevent_set_next_event(unsigned long delta, struct clock_event_device *ced)
 {
 	struct stm_timer *stm_timer = ced_to_stm(ced);
-	u32 val;
+	u32 val, now;
 
 	nxp_stm_clockevent_disable(stm_timer);
 
@@ -258,16 +258,12 @@ static int nxp_stm_clockevent_set_next_event(unsigned long delta, struct clock_e
 	writel(val, STM_CMP0(stm_timer->base));
 
 	/*
-	 * The counter is shared across the channels and can not be
-	 * stopped while we are setting the next event. If the delta
-	 * is very small it is possible the counter increases above
-	 * the computed 'val'. The min_delta value specified when
-	 * registering the clockevent will prevent that. The second
-	 * case is if the counter wraps while we compute the 'val' and
-	 * before writing the comparator register. We read the counter,
-	 * check if we are back in time and abort the timer with -ETIME.
+	 * If the counter reached or passed 'val' during or before the
+	 * write to the comparator, the hardware match will be missed,
+	 * resulting in a full 32-bit (32.2s) wraparound lockup.
 	 */
-	if (val > nxp_stm_clockevent_read_counter(stm_timer) + delta)
+	now = nxp_stm_clockevent_read_counter(stm_timer);
+	if ((int)(val - now) <= 0)
 		return -ETIME;
 
 	nxp_stm_clockevent_enable(stm_timer);

-- 
2.52.0

[PATCH 3/3] clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT

[Enric Balletbo i Serra via B4 Relay]

From: Enric Balletbo i Serra <eballetb@redhat.com>

Under bus contention on PREEMPT_RT kernels, the 2us min_delta is too
small and the counter can advance past the programmed comparator value
before the overshoot check runs. While the -ETIME fallback handles this
case, repeated retries add latency. Increase min_delta to 20us to reduce
collision probability.

Cc: stable@vger.kernel.org
Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
 drivers/clocksource/timer-nxp-stm.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
index ed7662a4e2bc..a0761d7f8b01 100644
--- a/drivers/clocksource/timer-nxp-stm.c
+++ b/drivers/clocksource/timer-nxp-stm.c
@@ -335,14 +335,14 @@ static int nxp_stm_clockevent_starting_cpu(unsigned int cpu)
 	/*
 	 * The timings measurement show reading the counter register
 	 * and writing to the comparator register takes as a maximum
-	 * value 1100 ns at 133MHz rate frequency. The timer must be
-	 * set above this value and to be secure we set the minimum
-	 * value equal to 2000ns, so 2us.
+	 * value 1100 ns at 133MHz rate frequency. However, under bus
+	 * contention on PREEMPT_RT, 2us is too small and causes missed
+	 * matches resulting in 32s wraparounds. Increase to 20us.
 	 *
-	 * minimum ticks = (rate / MICRO) * 2
+	 * minimum ticks = (rate / MICRO) * 20
 	 */
 	clockevents_config_and_register(&stm_timer->ced, stm_timer->rate,
-					(stm_timer->rate / MICRO) * 2, 0x7FFFFFFFU);
+					(stm_timer->rate / MICRO) * 20, 0x7FFFFFFFU);
 
 	return 0;
 }

-- 
2.52.0

Re: [PATCH 2/3] clocksource/drivers/nxp-stm: Fix missed comparator match in set_next_event

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clocksource: nxp-stm: overshoot race in nxp_stm_clockevent_set_next_event()
--

commit 45f273b782a28cec6ef30880d738a9cb690edce9
Author: Enric Balletbo i Serra <eballetb@redhat.com>

clocksource/drivers/nxp-stm: Fix missed comparator match in set_next_event

This patch fixes the overshoot check in nxp_stm_clockevent_set_next_event()
by using a signed comparison between the target comparator value and the
current counter value to avoid missed matches and 32-bit wraparound stalls.

> diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
> index ee249fed1b918..ed7662a4e2bc4 100644
> --- a/drivers/clocksource/timer-nxp-stm.c
> +++ b/drivers/clocksource/timer-nxp-stm.c
> @@ -247,7 +247,7 @@ static int nxp_stm_clockevent_shutdown(struct clock_event_device *ced)
>  static int nxp_stm_clockevent_set_next_event(unsigned long delta, struct clock_event_device *ced)
>  {
>  	struct stm_timer *stm_timer = ced_to_stm(ced);
> -	u32 val;
> +	u32 val, now;
>  
>  	nxp_stm_clockevent_disable(stm_timer);
>  
> @@ -258,16 +258,12 @@ static int nxp_stm_clockevent_set_next_event(unsigned long delta, struct clock_e
>  	writel(val, STM_CMP0(stm_timer->base));
>  
>  	/*
> -	 * The counter is shared across the channels and can not be
> -	 * stopped while we are setting the next event. If the delta
> -	 * is very small it is possible the counter increases above
> -	 * the computed 'val'. The min_delta value specified when
> -	 * registering the clockevent will prevent that. The second
> -	 * case is if the counter wraps while we compute the 'val' and
> -	 * before writing the comparator register. We read the counter,
> -	 * check if we are back in time and abort the timer with -ETIME.
> +	 * If the counter reached or passed 'val' during or before the
> +	 * write to the comparator, the hardware match will be missed,
> +	 * resulting in a full 32-bit (32.2s) wraparound lockup.
>  	 */
> -	if (val > nxp_stm_clockevent_read_counter(stm_timer) + delta)
> +	now = nxp_stm_clockevent_read_counter(stm_timer);
> +	if ((int)(val - now) <= 0)
>  		return -ETIME;
>  
>  	nxp_stm_clockevent_enable(stm_timer);

[Severity: High]
Can the counter still advance past val before nxp_stm_clockevent_enable()
arms the channel comparator in drivers/clocksource/timer-nxp-stm.c?

At the beginning of nxp_stm_clockevent_set_next_event(), channel 0 is
disabled before programming STM_CMP0:

	nxp_stm_clockevent_disable(stm_timer);
	...
	val = nxp_stm_clockevent_read_counter(stm_timer) + delta;
	writel(val, STM_CMP0(stm_timer->base));

Unlike timer-imx-gpt where the compare interrupt remains enabled while
writing TCMP, writing STM_CMP0 in the NXP STM does not arm the comparator
until nxp_stm_clockevent_enable() writes STM_CCR_CEN to STM_CCR0.

Because now is read from STM_CNT before nxp_stm_clockevent_enable() is
called, if now is sampled just below val (so (int)(val - now) <= 0 is false)
and STM_CNT reaches or passes val before the subsequent write to STM_CCR0
takes effect in hardware, the match occurs while channel 0 is still disabled
and is missed.

Should nxp_stm_clockevent_enable(stm_timer) be called before reading now for
the overshoot check, with nxp_stm_clockevent_disable(stm_timer) called on
the -ETIME path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-nxp-stm-fixes-v1-0-20a64a4ea454@redhat.com?part=2

Re: [PATCH 3/3] clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] clocksource/drivers/nxp-stm: overshoot check runs before enabling comparator
--

commit 1dea5edcdc7544abe47a47e00f3d79d5b290fb1a
Author: Enric Balletbo i Serra <eballetb@redhat.com>

clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT

This commit increases the clockevent min_delta from 2us to 20us in
nxp_stm_clockevent_starting_cpu() to avoid counter overshoot under bus
contention on PREEMPT_RT kernels.

>     clocksource/drivers/nxp-stm: Increase min_delta to 20us for PREEMPT_RT
>     
>     Under bus contention on PREEMPT_RT kernels, the 2us min_delta is too
>     small and the counter can advance past the programmed comparator value
>     before the overshoot check runs. While the -ETIME fallback handles this
>     case, repeated retries add latency. Increase min_delta to 20us to reduce
>     collision probability.
>     
>     Cc: stable@vger.kernel.org
>     Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>

> diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
> index ed7662a4e2bc4..a0761d7f8b013 100644
> --- a/drivers/clocksource/timer-nxp-stm.c
> +++ b/drivers/clocksource/timer-nxp-stm.c
> @@ -335,14 +335,14 @@ static int nxp_stm_clockevent_starting_cpu(unsigned int cpu)
>  	/*
>  	 * The timings measurement show reading the counter register
>  	 * and writing to the comparator register takes as a maximum
> -	 * value 1100 ns at 133MHz rate frequency. The timer must be
> -	 * set above this value and to be secure we set the minimum
> -	 * value equal to 2000ns, so 2us.
> +	 * value 1100 ns at 133MHz rate frequency. However, under bus
> +	 * contention on PREEMPT_RT, 2us is too small and causes missed
> +	 * matches resulting in 32s wraparounds. Increase to 20us.
>  	 *
> -	 * minimum ticks = (rate / MICRO) * 2
> +	 * minimum ticks = (rate / MICRO) * 20
>  	 */
>  	clockevents_config_and_register(&stm_timer->ced, stm_timer->rate,
> -					(stm_timer->rate / MICRO) * 2, 0x7FFFFFFFU);
> +					(stm_timer->rate / MICRO) * 20, 0x7FFFFFFFU);

[Severity: Medium]
The commit message states that the -ETIME fallback handles counter
overshoot and that increasing min_delta to 20us only reduces retry
latency, whereas the comment added in nxp_stm_clockevent_starting_cpu()
states that 2us causes missed matches resulting in 32s wraparounds.

Looking at nxp_stm_clockevent_set_next_event() in
drivers/clocksource/timer-nxp-stm.c:

static int nxp_stm_clockevent_set_next_event(unsigned long delta, struct clock_event_device *ced)
{
	struct stm_timer *stm_timer = ced_to_stm(ced);
	u32 val, now;

	nxp_stm_clockevent_disable(stm_timer);

	stm_timer->delta = delta;

	val = nxp_stm_clockevent_read_counter(stm_timer) + delta;

	writel(val, STM_CMP0(stm_timer->base));
	...
	now = nxp_stm_clockevent_read_counter(stm_timer);
	if ((int)(val - now) <= 0)
		return -ETIME;

	nxp_stm_clockevent_enable(stm_timer);

	return 0;
}

nxp_stm_clockevent_disable() clears STM_CCR_CEN in STM_CCR0, so no
hardware comparator match can occur until nxp_stm_clockevent_enable()
writes STM_CCR_CEN back to STM_CCR0.

Because now is read and checked against val before calling
nxp_stm_clockevent_enable(), if now is sampled just before val, can the
counter reach and pass val while the STM_CCR0 enable write is still in
flight under bus contention, missing the hardware match while
nxp_stm_clockevent_set_next_event() returns 0 and causing the 32.2s
wraparound?

Should nxp_stm_clockevent_set_next_event() enable the comparator channel
before reading now for the overshoot check (and disable it again if
returning -ETIME) so that the -ETIME check covers the channel-enable
write latency as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-nxp-stm-fixes-v1-0-20a64a4ea454@redhat.com?part=3