mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Fix NXP STM clockevent bugs causing lockups
@ 2026-11-05  8:18 Enric Balletbo i Serra
  2026-11-05  8:18 ` [PATCH v3 1/3] clocksource/drivers/nxp-stm: Fix max_delta overflow on 64-bit platforms Enric Balletbo i Serra
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Enric Balletbo i Serra @ 2026-11-05  8:18 UTC (permalink / raw)
  To: Daniel Lezcano, Thomas Gleixner, Maxime Coquelin, Alexandre Torgue
  Cc: linux-kernel, linux-arm-kernel, linux-rt-devel,
	Ghennadi Procopciuc, NXP S32 Linux Team, stable, linux-stm32,
	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 an exact match between the
counter and the comparator registers. If the counter passes the
programmed value before the comparator takes it into account, the match
is missed and the hardware must count through all 2^32 ticks (~32.2s at
133 MHz) before firing again, causing soft lockups.

Two independent bugs made that possible:

  - The overshoot check in set_next_event() only detected a counter
    wraparound, not the counter advancing past the programmed value
    during the read/write sequence.

  - The channel-enable write at the end of set_next_event() lands after
    the overshoot check. If the counter passes the programmed value
    before the enable write takes effect, the check has already passed
    and the match is missed. No min_delta can close that window.

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 replaces the broken overshoot check with a margin based
comparison following the pattern of hpet_clkevt_set_next_event(), and
derives the minimum programming delta from that margin.

Patch 3 keeps the compare channel armed for the whole ONESHOT operation
and manages it from the state callbacks, like the HPET driver does, so
that the comparator write is the last hardware access of the programming
path and is covered by the overshoot check.

With this in place, any delay of the programming sequence - however
long the accesses to the timer module are stalled - degrades to a
-ETIME retry by the clockevents core instead of a 32-bit wraparound
stall.

Measurement procedure for the overshoot constants
--------------------------------------------------

Both constants (STM_MIN_CYCLES and STM_MIN_PROG_DELTA) are sized from
measurements on S32G, collected with temporary instrumentation on
compare channel 1 (which the driver never touches):

  - A write-to-match sweep programs the comparator at CNT + n for
    increasing n and measures how far the counter advances between the
    ordering read and the comparator write taking effect. Targets
    within ~32 ticks are never reachable (the write needs at least
    that long to land) and the window grows beyond ~256 ticks under
    bus stress. No misses were seen at larger targets, so the landing
    window is bounded on this platform. STM_MIN_CYCLES = 512 keeps a
    factor of two on the observed tail.

  - Tracking the read/write/read round trip of set_next_event() under
    the lockup reproducer (stress-ng matrix/memthrash plus 1ns
    clock_nanosleep hammers) shows a worst case of 574 counter ticks
    (~4.3us at 133 MHz), against ~146 ticks (~1.1us) without load.

The minimum programming delta must absorb the worst case round trip and
still leave STM_MIN_CYCLES to the event when it reaches the check,
otherwise a minimum delta program fails deterministically and spins in
-ETIME retries (CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST is not selected
on arm64). 574 + 512 exceeds the HPET-derived 1.5 * STM_MIN_CYCLES =
768, so that factor does not cover this platform; use
STM_MIN_PROG_DELTA = 3 * STM_MIN_CYCLES = 1536 ticks (~11.5us), which
covers the measured worst case with headroom.

Reproducer
----------

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
   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 sha2_ce sha256_arm64
   sha1_arm64_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 : 000000000c1c684 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 v3:
- Code review and commit message editing assisted by Claude:claude-opus-4-6.
- Rework the overshoot check following review feedback on v2: replace
  the signed <= 0 comparison with a margin based check in the style of
  hpet_clkevt_set_next_event(), and derive min_delta from the margin
  (3 * STM_MIN_CYCLES) instead of increasing it to a fixed 20us.
- Add a new patch which keeps the compare channel armed during the
  whole ONESHOT operation instead of enabling it on every event: the
  trailing channel-enable write was a posted write landing after the
  overshoot check, which no min_delta could compensate for.
- Size the margin constants from measurements on S32G instead of the
  estimated timings.

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 overshoot detection in set_next_event
      clocksource/drivers/nxp-stm: Arm the compare channel from the state callbacks

 drivers/clocksource/timer-nxp-stm.c | 101 +++++++++++++++++++++++-------------
 1 file changed, 65 insertions(+), 36 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261023-nxp-stm-fixes-ab0163517e6f

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/3] clocksource/drivers/nxp-stm: Fix max_delta overflow on 64-bit platforms
  2026-11-05  8:18 [PATCH v3 0/3] Fix NXP STM clockevent bugs causing lockups Enric Balletbo i Serra
@ 2026-11-05  8:18 ` Enric Balletbo i Serra
  2026-11-05  8:18 ` [PATCH v3 2/3] clocksource/drivers/nxp-stm: Fix overshoot detection in set_next_event Enric Balletbo i Serra
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Enric Balletbo i Serra @ 2026-11-05  8:18 UTC (permalink / raw)
  To: Daniel Lezcano, Thomas Gleixner, Maxime Coquelin, Alexandre Torgue
  Cc: linux-kernel, linux-arm-kernel, linux-rt-devel,
	Ghennadi Procopciuc, NXP S32 Linux Team, stable, linux-stm32,
	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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 2/3] clocksource/drivers/nxp-stm: Fix overshoot detection in set_next_event
  2026-11-05  8:18 [PATCH v3 0/3] Fix NXP STM clockevent bugs causing lockups Enric Balletbo i Serra
  2026-11-05  8:18 ` [PATCH v3 1/3] clocksource/drivers/nxp-stm: Fix max_delta overflow on 64-bit platforms Enric Balletbo i Serra
@ 2026-11-05  8:18 ` Enric Balletbo i Serra
  2026-11-05  8:18 ` [PATCH v3 3/3] clocksource/drivers/nxp-stm: Arm the compare channel from the state callbacks Enric Balletbo i Serra
  2026-11-06  8:42 ` [PATCH v3 0/3] Fix NXP STM clockevent bugs causing lockups Khristine Andreea Barbulescu
  3 siblings, 0 replies; 5+ messages in thread
From: Enric Balletbo i Serra @ 2026-11-05  8:18 UTC (permalink / raw)
  To: Daniel Lezcano, Thomas Gleixner, Maxime Coquelin, Alexandre Torgue
  Cc: linux-kernel, linux-arm-kernel, linux-rt-devel,
	Ghennadi Procopciuc, NXP S32 Linux Team, stable, linux-stm32,
	Enric Balletbo i Serra

The existing overshoot check effectively compares two counter readings
and only triggers on a counter wraparound between reads; the normal
case where the counter advances past the programmed value during the
read/write sequence is not detected.

Replace it with the approach of hpet_clkevt_set_next_event(): read the
counter after the comparator write and return -ETIME when less than
STM_MIN_CYCLES remain until the match. Derive the minimum programming
delta from that margin. See the cover letter for the measurement
procedure and constant derivation.

Fixes: cec32ac75827 ("clocksource/drivers/nxp-timer: Add the System Timer Module for the s32gx platforms")
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
 drivers/clocksource/timer-nxp-stm.c | 53 ++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 18 deletions(-)

diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
index ee249fed1b91..81527745b7e2 100644
--- a/drivers/clocksource/timer-nxp-stm.c
+++ b/drivers/clocksource/timer-nxp-stm.c
@@ -20,7 +20,6 @@
 #include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/sched_clock.h>
-#include <linux/units.h>
 
 #define STM_CR(__base)		(__base)
 
@@ -52,6 +51,17 @@
 
 #define STM_ENABLE_MASK	(STM_CR_FRZ | STM_CR_TEN)
 
+/*
+ * The comparator fires only on an exact match with the counter.
+ * STM_MIN_CYCLES is the post-write overshoot margin used by
+ * nxp_stm_clockevent_set_next_event(), following the same approach
+ * as hpet_clkevt_set_next_event(). STM_MIN_PROG_DELTA is the
+ * minimum programming delta derived from it. Both values are
+ * empirically sized from measurements on S32G.
+ */
+#define STM_MIN_CYCLES		512
+#define STM_MIN_PROG_DELTA	(STM_MIN_CYCLES * 3)
+
 struct stm_timer {
 	void __iomem *base;
 	unsigned long rate;
@@ -248,6 +258,7 @@ static int nxp_stm_clockevent_set_next_event(unsigned long delta, struct clock_e
 {
 	struct stm_timer *stm_timer = ced_to_stm(ced);
 	u32 val;
+	s32 res;
 
 	nxp_stm_clockevent_disable(stm_timer);
 
@@ -258,16 +269,26 @@ 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.
+	 * The comparator generates an interrupt only on an exact match
+	 * with the counter. If the counter reaches the programmed value
+	 * before the write to the comparator takes effect, the match is
+	 * missed and the next interrupt only fires after a full 32-bit
+	 * counter wraparound (~32.2s at 133 MHz).
+	 *
+	 * The counter read below targets the same module as the
+	 * comparator write, so it can only complete once the write has
+	 * reached the device, and the signed difference between the
+	 * programmed value and the counter reflects the hardware state at
+	 * that point. If less than STM_MIN_CYCLES remain to the match, the
+	 * write might not have made it into the comparator in time;
+	 * return -ETIME so the clockevents core retries with a larger
+	 * delta.
+	 *
+	 * This is the same approach as hpet_clkevt_set_next_event().
 	 */
-	if (val > nxp_stm_clockevent_read_counter(stm_timer) + delta)
+	res = (s32)(val - nxp_stm_clockevent_read_counter(stm_timer));
+
+	if (res < STM_MIN_CYCLES)
 		return -ETIME;
 
 	nxp_stm_clockevent_enable(stm_timer);
@@ -337,16 +358,12 @@ static int nxp_stm_clockevent_starting_cpu(unsigned int cpu)
 		return ret;
 
 	/*
-	 * 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.
-	 *
-	 * minimum ticks = (rate / MICRO) * 2
+	 * The minimum programming delta absorbs the measured worst case
+	 * read/write/read round trip and the overshoot margin, see the
+	 * comment on STM_MIN_CYCLES.
 	 */
 	clockevents_config_and_register(&stm_timer->ced, stm_timer->rate,
-					(stm_timer->rate / MICRO) * 2, 0x7FFFFFFFU);
+					STM_MIN_PROG_DELTA, 0x7FFFFFFFU);
 
 	return 0;
 }

-- 
2.56.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 3/3] clocksource/drivers/nxp-stm: Arm the compare channel from the state callbacks
  2026-11-05  8:18 [PATCH v3 0/3] Fix NXP STM clockevent bugs causing lockups Enric Balletbo i Serra
  2026-11-05  8:18 ` [PATCH v3 1/3] clocksource/drivers/nxp-stm: Fix max_delta overflow on 64-bit platforms Enric Balletbo i Serra
  2026-11-05  8:18 ` [PATCH v3 2/3] clocksource/drivers/nxp-stm: Fix overshoot detection in set_next_event Enric Balletbo i Serra
@ 2026-11-05  8:18 ` Enric Balletbo i Serra
  2026-11-06  8:42 ` [PATCH v3 0/3] Fix NXP STM clockevent bugs causing lockups Khristine Andreea Barbulescu
  3 siblings, 0 replies; 5+ messages in thread
From: Enric Balletbo i Serra @ 2026-11-05  8:18 UTC (permalink / raw)
  To: Daniel Lezcano, Thomas Gleixner, Maxime Coquelin, Alexandre Torgue
  Cc: linux-kernel, linux-arm-kernel, linux-rt-devel,
	Ghennadi Procopciuc, NXP S32 Linux Team, stable, linux-stm32,
	Enric Balletbo i Serra

set_next_event() disables the compare channel, programs the comparator
and enables the channel again after the overshoot check. The trailing
channel-enable write is a posted write landing after the check: if the
counter passes the programmed value before the enable takes effect, the
match is missed. No min_delta can close that window.

Keep the channel armed for the whole ONESHOT operation instead, and
manage it from the state callbacks, like the HPET driver does:

  - set_state_oneshot() and set_state_periodic() enable the channel;
    a stale pending interrupt flag is cleared before enabling.
    set_state_oneshot_stopped() and set_state_shutdown() disable it.

  - set_next_event() now only writes the comparator, covered by the
    overshoot check.

  - The interrupt handler reprograms the comparator only in periodic
    mode; in ONESHOT mode the stale value stays armed until the core
    programs a new event or stops the channel.

Fixes: cec32ac75827 ("clocksource/drivers/nxp-timer: Add the System Timer Module for the s32gx platforms")
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
 drivers/clocksource/timer-nxp-stm.c | 52 +++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/drivers/clocksource/timer-nxp-stm.c b/drivers/clocksource/timer-nxp-stm.c
index 81527745b7e2..8ad99863bdff 100644
--- a/drivers/clocksource/timer-nxp-stm.c
+++ b/drivers/clocksource/timer-nxp-stm.c
@@ -254,14 +254,27 @@ static int nxp_stm_clockevent_shutdown(struct clock_event_device *ced)
 	return 0;
 }
 
+static int nxp_stm_clockevent_set_oneshot(struct clock_event_device *ced)
+{
+	struct stm_timer *stm_timer = ced_to_stm(ced);
+
+	/*
+	 * Clear a stale interrupt flag so that enabling the channel
+	 * cannot fire an event from a previous match.
+	 */
+	writel(STM_CIR_CIF, STM_CIR0(stm_timer->base));
+
+	nxp_stm_clockevent_enable(stm_timer);
+
+	return 0;
+}
+
 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;
 	s32 res;
 
-	nxp_stm_clockevent_disable(stm_timer);
-
 	stm_timer->delta = delta;
 
 	val = nxp_stm_clockevent_read_counter(stm_timer) + delta;
@@ -288,18 +301,16 @@ static int nxp_stm_clockevent_set_next_event(unsigned long delta, struct clock_e
 	 */
 	res = (s32)(val - nxp_stm_clockevent_read_counter(stm_timer));
 
-	if (res < STM_MIN_CYCLES)
-		return -ETIME;
-
-	nxp_stm_clockevent_enable(stm_timer);
-
-	return 0;
+	return res < STM_MIN_CYCLES ? -ETIME : 0;
 }
 
 static int nxp_stm_clockevent_set_periodic(struct clock_event_device *ced)
 {
 	struct stm_timer *stm_timer = ced_to_stm(ced);
 
+	writel(STM_CIR_CIF, STM_CIR0(stm_timer->base));
+	nxp_stm_clockevent_enable(stm_timer);
+
 	return nxp_stm_clockevent_set_next_event(stm_timer->rate, ced);
 }
 
@@ -327,6 +338,8 @@ static int nxp_stm_clockevent_per_cpu_init(struct device *dev, struct stm_timer
 	stm_timer->ced.name = name;
 	stm_timer->ced.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
 	stm_timer->ced.set_state_shutdown = nxp_stm_clockevent_shutdown;
+	stm_timer->ced.set_state_oneshot = nxp_stm_clockevent_set_oneshot;
+	stm_timer->ced.set_state_oneshot_stopped = nxp_stm_clockevent_shutdown;
 	stm_timer->ced.set_state_periodic = nxp_stm_clockevent_set_periodic;
 	stm_timer->ced.set_next_event = nxp_stm_clockevent_set_next_event;
 	stm_timer->ced.suspend = nxp_stm_clockevent_suspend;
@@ -384,19 +397,18 @@ static irqreturn_t nxp_stm_module_interrupt(int irq, void *dev_id)
 	writel(STM_CIR_CIF, STM_CIR0(stm_timer->base));
 
 	/*
-	 * Update STM_CMP value using the counter value
+	 * The STM hardware has no periodic mode: the comparator stays
+	 * armed after a match, so periodic mode is emulated by
+	 * reprogramming the comparator here. In ONESHOT mode the stale
+	 * comparator value stays armed instead; the clockevents core
+	 * either programs a new event or stops the channel long before
+	 * the counter can wrap around to the stale value (~32.2s at
+	 * 133 MHz).
 	 */
-	val = nxp_stm_clockevent_read_counter(stm_timer) + stm_timer->delta;
-
-	writel(val, STM_CMP0(stm_timer->base));
-
-	/*
-	 * stm hardware doesn't support oneshot, it will generate an
-	 * interrupt and start the counter again so software needs to
-	 * disable the timer to stop the counter loop in ONESHOT mode.
-	 */
-	if (likely(clockevent_state_oneshot(ced)))
-		nxp_stm_clockevent_disable(stm_timer);
+	if (clockevent_state_periodic(ced)) {
+		val = nxp_stm_clockevent_read_counter(stm_timer) + stm_timer->delta;
+		writel(val, STM_CMP0(stm_timer->base));
+	}
 
 	ced->event_handler(ced);
 

-- 
2.56.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 0/3] Fix NXP STM clockevent bugs causing lockups
  2026-11-05  8:18 [PATCH v3 0/3] Fix NXP STM clockevent bugs causing lockups Enric Balletbo i Serra
                   ` (2 preceding siblings ...)
  2026-11-05  8:18 ` [PATCH v3 3/3] clocksource/drivers/nxp-stm: Arm the compare channel from the state callbacks Enric Balletbo i Serra
@ 2026-11-06  8:42 ` Khristine Andreea Barbulescu
  3 siblings, 0 replies; 5+ messages in thread
From: Khristine Andreea Barbulescu @ 2026-11-06  8:42 UTC (permalink / raw)
  To: Enric Balletbo i Serra, Daniel Lezcano, Thomas Gleixner,
	Maxime Coquelin, Alexandre Torgue
  Cc: linux-kernel, linux-arm-kernel, linux-rt-devel,
	Ghennadi Procopciuc, NXP S32 Linux Team, stable, linux-stm32

On 11/5/2026 10:18 AM, Enric Balletbo i Serra wrote:
> 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 an exact match between the
> counter and the comparator registers. If the counter passes the
> programmed value before the comparator takes it into account, the match
> is missed and the hardware must count through all 2^32 ticks (~32.2s at
> 133 MHz) before firing again, causing soft lockups.
> 
> Two independent bugs made that possible:
> 
>   - The overshoot check in set_next_event() only detected a counter
>     wraparound, not the counter advancing past the programmed value
>     during the read/write sequence.
> 
>   - The channel-enable write at the end of set_next_event() lands after
>     the overshoot check. If the counter passes the programmed value
>     before the enable write takes effect, the check has already passed
>     and the match is missed. No min_delta can close that window.
> 
> 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 replaces the broken overshoot check with a margin based
> comparison following the pattern of hpet_clkevt_set_next_event(), and
> derives the minimum programming delta from that margin.
> 
> Patch 3 keeps the compare channel armed for the whole ONESHOT operation
> and manages it from the state callbacks, like the HPET driver does, so
> that the comparator write is the last hardware access of the programming
> path and is covered by the overshoot check.
> 
> With this in place, any delay of the programming sequence - however
> long the accesses to the timer module are stalled - degrades to a
> -ETIME retry by the clockevents core instead of a 32-bit wraparound
> stall.
> 
> Measurement procedure for the overshoot constants
> --------------------------------------------------
> 
> Both constants (STM_MIN_CYCLES and STM_MIN_PROG_DELTA) are sized from
> measurements on S32G, collected with temporary instrumentation on
> compare channel 1 (which the driver never touches):
> 
>   - A write-to-match sweep programs the comparator at CNT + n for
>     increasing n and measures how far the counter advances between the
>     ordering read and the comparator write taking effect. Targets
>     within ~32 ticks are never reachable (the write needs at least
>     that long to land) and the window grows beyond ~256 ticks under
>     bus stress. No misses were seen at larger targets, so the landing
>     window is bounded on this platform. STM_MIN_CYCLES = 512 keeps a
>     factor of two on the observed tail.
> 
>   - Tracking the read/write/read round trip of set_next_event() under
>     the lockup reproducer (stress-ng matrix/memthrash plus 1ns
>     clock_nanosleep hammers) shows a worst case of 574 counter ticks
>     (~4.3us at 133 MHz), against ~146 ticks (~1.1us) without load.
> 
> The minimum programming delta must absorb the worst case round trip and
> still leave STM_MIN_CYCLES to the event when it reaches the check,
> otherwise a minimum delta program fails deterministically and spins in
> -ETIME retries (CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST is not selected
> on arm64). 574 + 512 exceeds the HPET-derived 1.5 * STM_MIN_CYCLES =
> 768, so that factor does not cover this platform; use
> STM_MIN_PROG_DELTA = 3 * STM_MIN_CYCLES = 1536 ticks (~11.5us), which
> covers the measured worst case with headroom.
> 
> Reproducer
> ----------
> 
> 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
>    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 sha2_ce sha256_arm64
>    sha1_arm64_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 : 000000000c1c684 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 v3:
> - Code review and commit message editing assisted by Claude:claude-opus-4-6.
> - Rework the overshoot check following review feedback on v2: replace
>   the signed <= 0 comparison with a margin based check in the style of
>   hpet_clkevt_set_next_event(), and derive min_delta from the margin
>   (3 * STM_MIN_CYCLES) instead of increasing it to a fixed 20us.
> - Add a new patch which keeps the compare channel armed during the
>   whole ONESHOT operation instead of enabling it on every event: the
>   trailing channel-enable write was a posted write landing after the
>   overshoot check, which no min_delta could compensate for.
> - Size the margin constants from measurements on S32G instead of the
>   estimated timings.
> 
> 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 overshoot detection in set_next_event
>       clocksource/drivers/nxp-stm: Arm the compare channel from the state callbacks
> 
>  drivers/clocksource/timer-nxp-stm.c | 101 +++++++++++++++++++++++-------------
>  1 file changed, 65 insertions(+), 36 deletions(-)
> ---
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> change-id: 20261023-nxp-stm-fixes-ab0163517e6f
> 
> Best regards,

Hi Enric,

Tested v3 on S32G399A-RDB3:
- trigger_lockup reproducer from the cover letter with stress-ng
  (matrix/memcpy/vm) for 30 min: no soft lockup or RCU stall
- stress-ng timer/timerfd stress: all stressors passed
- all 8 per-CPU STM clockevents register with the expected
  min_delta/max_delta, each IRQ fires only on its own CPU

Looks good to me.

Tested-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>

Regards,
Khristine

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-06  8:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-05  8:18 [PATCH v3 0/3] Fix NXP STM clockevent bugs causing lockups Enric Balletbo i Serra
2026-11-05  8:18 ` [PATCH v3 1/3] clocksource/drivers/nxp-stm: Fix max_delta overflow on 64-bit platforms Enric Balletbo i Serra
2026-11-05  8:18 ` [PATCH v3 2/3] clocksource/drivers/nxp-stm: Fix overshoot detection in set_next_event Enric Balletbo i Serra
2026-11-05  8:18 ` [PATCH v3 3/3] clocksource/drivers/nxp-stm: Arm the compare channel from the state callbacks Enric Balletbo i Serra
2026-11-06  8:42 ` [PATCH v3 0/3] Fix NXP STM clockevent bugs causing lockups Khristine Andreea Barbulescu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®