[PATCH 3/5] lib: sbi: Fix sbi_timer for Asymmetric Multi-Processing

[Anup Patel]

On Sun, Apr 13, 2025 at 8:36 PM Xiang W <wxjstz at 126.com> wrote:
>
> The original get_time_val is assigned to get_ticks by detecting
> Zicntr during cold boot. However, if the multi-core system is not
> symmetrical, some cores do not have the Zicntr extension, and an
> error will occur when executing sbi_timer_value.
>
> Signed-off-by: Xiang W <wxjstz at 126.com>

Do we have real-world asymmetric SoC where only a subset
of CPUs have Zictnr. ?

If not then lets not add this complexity until such SoC shows up.

Regards,
Anup

> ---
>  lib/sbi/sbi_timer.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/lib/sbi/sbi_timer.c b/lib/sbi/sbi_timer.c
> index 14e5ee46..5266c612 100644
> --- a/lib/sbi/sbi_timer.c
> +++ b/lib/sbi/sbi_timer.c
> @@ -19,7 +19,6 @@
>  #include <sbi/sbi_timer.h>
>
>  static unsigned long time_delta_off;
> -static u64 (*get_time_val)(void);
>  static const struct sbi_timer_device *timer_dev = NULL;
>
>  #if __riscv_xlen == 32
> @@ -52,16 +51,19 @@ static void nop_delay_fn(void *opaque)
>  void sbi_timer_delay_loop(ulong units, u64 unit_freq,
>                           void (*delay_fn)(void *), void *opaque)
>  {
> +       struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>         u64 start_val, delta;
>
>         /* Do nothing if we don't have timer device */
> -       if (!timer_dev || !get_time_val) {
> +       if (!timer_dev ||
> +           (!sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR) &&
> +            !timer_dev->timer_value)) {
>                 sbi_printf("%s: called without timer device\n", __func__);
>                 return;
>         }
>
>         /* Save starting timer value */
> -       start_val = get_time_val();
> +       start_val = sbi_timer_value();
>
>         /* Compute desired timer value delta */
>         delta = ((u64)timer_dev->timer_freq * (u64)units);
> @@ -72,7 +74,7 @@ void sbi_timer_delay_loop(ulong units, u64 unit_freq,
>                 delay_fn = nop_delay_fn;
>
>         /* Busy loop until desired timer value delta reached */
> -       while ((get_time_val() - start_val) < delta)
> +       while ((sbi_timer_value() - start_val) < delta)
>                 delay_fn(opaque);
>  }
>
> @@ -91,8 +93,11 @@ bool sbi_timer_waitms_until(bool (*predicate)(void *), void *arg,
>
>  u64 sbi_timer_value(void)
>  {
> -       if (get_time_val)
> -               return get_time_val();
> +       struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> +       if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR))
> +               return get_ticks();
> +       if (timer_dev && timer_dev->timer_value)
> +               return timer_dev->timer_value();
>         return 0;
>  }
>
> @@ -181,8 +186,6 @@ void sbi_timer_set_device(const struct sbi_timer_device *dev)
>                 return;
>
>         timer_dev = dev;
> -       if (!get_time_val && timer_dev->timer_value)
> -               get_time_val = timer_dev->timer_value;
>  }
>
>  int sbi_timer_init(struct sbi_scratch *scratch, bool cold_boot)
> @@ -196,9 +199,6 @@ int sbi_timer_init(struct sbi_scratch *scratch, bool cold_boot)
>                 if (!time_delta_off)
>                         return SBI_ENOMEM;
>
> -               if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR))
> -                       get_time_val = get_ticks;
> -
>                 ret = sbi_platform_timer_init(plat);
>                 if (ret)
>                         return ret;
> --
> 2.47.2
>