Re: [PATCH 2/2] cyclic: return early from cyclic_run() if the list is empty

On 7/30/26 12:55, Rasmus Villemoes wrote:
> It is possible that schedule(), and hence cyclic_run(), gets called
> very early, perhaps even from assembly code. With commit
> 9c1b13b3fd2 ("cyclic: reduce get_timer_us() calls inside
> hlist_for_each_entry_safe()"), there is now an unconditional
> get_timer_us(0) done outside the loop, and depending on the platform,
> the timer infrastructure may not be set up yet. In at least one case,
> that has caused a divide-by-0 and hence a failure to boot.
> 
> Platforms should really ensure their timers are ready ASAP, and in the
> concrete case reported, that was indeed possible to fix that
> way. However, it doesn't hurt to also insert an early return here, and
> that could prevent other such hard-to-debug boot failures.
> 
> Reported-by: Emanuele Ghidoli <[email protected]>
> Link: https://marc.info/?l=u-boot&m=178481834846283&w=2
> Fixes: 9c1b13b3fd2 ("cyclic: reduce get_timer_us() calls inside hlist_for_each_entry_safe()")
> Signed-off-by: Rasmus Villemoes <[email protected]>

Reviewed-by: Stefan Roese <[email protected]>

Thanks,
Stefan

> ---
>   common/cyclic.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/common/cyclic.c b/common/cyclic.c
> index 2bc3c773f27..232143171ab 100644
> --- a/common/cyclic.c
> +++ b/common/cyclic.c
> @@ -63,6 +63,19 @@ static void cyclic_run(void)
>   	struct hlist_node *tmp;
>   	u64 now, after, cpu_time;
>   
> +	/*
> +	 * Nothing to do if the list is empty. Also, schedule() can be
> +	 * called before timer infrastructure is ready, in which case
> +	 * calling get_timer_us() before the (empty) loop could cause
> +	 * a divide-by-0 or otherwise crash the system. No clients
> +	 * should be registered before the timer infrastructure is up,
> +	 * so the check for the list being empty should be
> +	 * ok. Otherwise, we would need a new GD_FLG_TIMERS_READY
> +	 * flag.
> +	 */
> +	if (hlist_empty(&gd->cyclic_list))
> +		 return;
> +
>   	/* Prevent recursion */
>   	if (gd->flags & GD_FLG_CYCLIC_RUNNING)
>   		return;