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

[Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>]

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