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

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

[Enric Balletbo i Serra]

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>
---
Changes in v2:
- Update the code comment in nxp_stm_clockevent_starting_cpu() and the
  commit message: clarify that 20us prevents repeated -ETIME retry
  latency, removing the misleading reference to 32s wraparounds.
- Link to v1: https://lore.kernel.org/r/20261026-nxp-stm-fixes-v1-0-20a64a4ea454@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 | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261023-nxp-stm-fixes-ab0163517e6f

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

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

[Enric Balletbo i Serra]

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

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

[Enric Balletbo i Serra]

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

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

[Enric Balletbo i Serra]

Under heavy bus contention on PREEMPT_RT kernels, a 2us min_delta is too
tight and causes ~32-second timer soft lockups.

Increase min_delta from 2us to 20us (~2,660 cycles at 133 MHz). This
provides sufficient margin under severe bus contention to ensure the
channel-enable write completes well before the counter reaches the match
value, resolving the 32-second soft lockup.

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 | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
index ed7662a4e2bc..a1a0e23d3342 100644
--- a/drivers/clocksource/timer-nxp-stm.c
+++ b/drivers/clocksource/timer-nxp-stm.c
@@ -335,14 +335,18 @@ 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 heavy
+	 * bus contention on PREEMPT_RT, 2us is too tight and can allow
+	 * the counter to pass the match value before the channel-enable
+	 * write takes effect, leading to 32s wraparounds.
 	 *
-	 * minimum ticks = (rate / MICRO) * 2
+	 * Increase to 20us to guarantee the comparator channel is fully
+	 * enabled before the match value is reached.
+	 *
+	 * 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.56.0

Re: [PATCH v2 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 precedes channel enable
--

commit 1283cc4078852e277605e923b00287b73e75d175
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 prevent 32-second timer wraparound
lockups under heavy bus contention on PREEMPT_RT kernels.

> diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
> index ed7662a4e2bc4..a1a0e23d33426 100644
> --- a/drivers/clocksource/timer-nxp-stm.c
> +++ b/drivers/clocksource/timer-nxp-stm.c
> @@ -335,14 +335,18 @@ 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 heavy
> +	 * bus contention on PREEMPT_RT, 2us is too tight and can allow
> +	 * the counter to pass the match value before the channel-enable
> +	 * write takes effect, leading to 32s wraparounds.
>  	 *
> -	 * minimum ticks = (rate / MICRO) * 2
> +	 * Increase to 20us to guarantee the comparator channel is fully
> +	 * enabled before the match value is reached.
> +	 *
> +	 * 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]
As reported by Sashiko on v1 [1], does increasing min_delta to 20us in
nxp_stm_clockevent_starting_cpu() actually guarantee that the comparator
channel is enabled before the match value is reached?

Looking at 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, and the
hardware only generates a match event when STM_CNT equals STM_CMP0 while
STM_CCR0[CEN] is enabled. Because nxp_stm_clockevent_enable() is called
after reading now and checking (int)(val - now) <= 0, the latency of writing
STM_CCR_CEN to STM_CCR0 is not covered by the -ETIME overshoot check.

For any programmed delta (20us or larger), if bus contention, an NMI, or
hypervisor/firmware delay between the two counter reads leaves val - now as
a small positive number of ticks, the overshoot check passes while the
channel is still disabled, and STM_CNT can pass val before the subsequent
nxp_stm_clockevent_enable() write takes effect in hardware.

Should nxp_stm_clockevent_set_next_event() enable the comparator channel
before reading now for the overshoot check (and call
nxp_stm_clockevent_disable() if (int)(val - now) <= 0 before returning
-ETIME) so the counter read is ordered after the STM_CCR0 write on the bus?

[1] https://lore.kernel.org/r/20261026-nxp-stm-fixes-v1-3-20a64a4ea454@redhat.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-nxp-stm-fixes-v2-0-8ca2706f30ef@redhat.com?part=3

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

[Enric Balletbo i Serra]

Hi,

On Mon, Nov 2, 2026 at 10:25 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] clocksource/drivers/nxp-stm: overshoot check precedes channel enable
> --
>
> commit 1283cc4078852e277605e923b00287b73e75d175
> 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 prevent 32-second timer wraparound
> lockups under heavy bus contention on PREEMPT_RT kernels.
>
> > diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
> > index ed7662a4e2bc4..a1a0e23d33426 100644
> > --- a/drivers/clocksource/timer-nxp-stm.c
> > +++ b/drivers/clocksource/timer-nxp-stm.c
> > @@ -335,14 +335,18 @@ 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 heavy
> > +      * bus contention on PREEMPT_RT, 2us is too tight and can allow
> > +      * the counter to pass the match value before the channel-enable
> > +      * write takes effect, leading to 32s wraparounds.
> >        *
> > -      * minimum ticks = (rate / MICRO) * 2
> > +      * Increase to 20us to guarantee the comparator channel is fully
> > +      * enabled before the match value is reached.
> > +      *
> > +      * 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]
> As reported by Sashiko on v1 [1], does increasing min_delta to 20us in
> nxp_stm_clockevent_starting_cpu() actually guarantee that the comparator
> channel is enabled before the match value is reached?
>
> Looking at 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, and the
> hardware only generates a match event when STM_CNT equals STM_CMP0 while
> STM_CCR0[CEN] is enabled. Because nxp_stm_clockevent_enable() is called
> after reading now and checking (int)(val - now) <= 0, the latency of writing
> STM_CCR_CEN to STM_CCR0 is not covered by the -ETIME overshoot check.
>
> For any programmed delta (20us or larger), if bus contention, an NMI, or
> hypervisor/firmware delay between the two counter reads leaves val - now as
> a small positive number of ticks, the overshoot check passes while the
> channel is still disabled, and STM_CNT can pass val before the subsequent
> nxp_stm_clockevent_enable() write takes effect in hardware.
>
> Should nxp_stm_clockevent_set_next_event() enable the comparator channel
> before reading now for the overshoot check (and call
> nxp_stm_clockevent_disable() if (int)(val - now) <= 0 before returning
> -ETIME) so the counter read is ordered after the STM_CCR0 write on the bus?
>

I answered this in the first version of the PATCH 2/3, this describes
the same race you pointed in the PATCH 2/3 and I think the race is
possible but I believe it cannot occur in practice with the current
min_delta:

set_next_event is called with local interrupts disabled, so
nothing can preempt or extend that window. The gap between reading
now and the writel in nxp_stm_clockevent_enable() is a
comparison, a branch, and a single MMIO write. With min_delta = 2us, the
check is still close to the write.

Reordering to enable-before-check introduces a different problem: if
the comparator match fires between nxp_stm_clockevent_enable() and
reading now, the interrupt handler delivers the event via
ced->event_handler(ced) and then we also return -ETIME, causing, if
I'm not mistaken
the clockevents layer to program another event, causing a double delivery.

Thanks,
   Enric

> [1] https://lore.kernel.org/r/20261026-nxp-stm-fixes-v1-3-20a64a4ea454@redhat.com
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261102-nxp-stm-fixes-v2-0-8ca2706f30ef@redhat.com?part=3
>