[PATCH v2 5/5] lib: utils/timer: Simplify MTIMER synchronization

Atish Patra atishp at atishpatra.org
Wed Aug 11 14:07:30 PDT 2021


On Sat, Aug 7, 2021 at 6:34 AM Anup Patel <anup.patel at wdc.com> wrote:
>
> We simplify MTIMER synchronization as follows:
>
> 1) Detect MTIMER devices with unique (or non-shared) MTIME
>    register at boot-time
> 2) Select first MTIMER device with no associated HART as our
>    reference MTIMER device
> 3) Only synchronize MTIMER devices with unique (or non-shared)
>    MTIME register using reference MTIMER device
> 4) Directly update the MTIME register at time of synchronization
>    because MTIME is a read/write register.
>
> Signed-off-by: Anup Patel <anup.patel at wdc.com>
> ---
>  include/sbi_utils/timer/aclint_mtimer.h |  7 ++-
>  lib/utils/timer/aclint_mtimer.c         | 67 +++++++++++++++----------
>  lib/utils/timer/fdt_timer_mtimer.c      | 39 ++++++++++++--
>  3 files changed, 80 insertions(+), 33 deletions(-)
>
> diff --git a/include/sbi_utils/timer/aclint_mtimer.h b/include/sbi_utils/timer/aclint_mtimer.h
> index fdc46cd..a9fe445 100644
> --- a/include/sbi_utils/timer/aclint_mtimer.h
> +++ b/include/sbi_utils/timer/aclint_mtimer.h
> @@ -31,14 +31,19 @@ struct aclint_mtimer_data {
>         u32 first_hartid;
>         u32 hart_count;
>         bool has_64bit_mmio;
> +       bool has_shared_mtime;
>         /* Private details (initialized and used by ACLINT MTIMER library) */
>         struct aclint_mtimer_data *time_delta_reference;
>         unsigned long time_delta_computed;
> -       u64 time_delta;
>         u64 (*time_rd)(volatile u64 *addr);
>         void (*time_wr)(bool timecmp, u64 value, volatile u64 *addr);
>  };
>
> +void aclint_mtimer_sync(struct aclint_mtimer_data *mt);
> +
> +void aclint_mtimer_set_reference(struct aclint_mtimer_data *mt,
> +                                struct aclint_mtimer_data *ref);
> +
>  int aclint_mtimer_warm_init(void);
>
>  int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
> diff --git a/lib/utils/timer/aclint_mtimer.c b/lib/utils/timer/aclint_mtimer.c
> index 0e4e846..d612b12 100644
> --- a/lib/utils/timer/aclint_mtimer.c
> +++ b/lib/utils/timer/aclint_mtimer.c
> @@ -57,7 +57,7 @@ static u64 mtimer_value(void)
>         u64 *time_val = (void *)mt->mtime_addr;
>
>         /* Read MTIMER Time Value */
> -       return mt->time_rd(time_val) + mt->time_delta;
> +       return mt->time_rd(time_val);
>  }
>
>  static void mtimer_event_stop(void)
> @@ -77,7 +77,7 @@ static void mtimer_event_start(u64 next_event)
>         u64 *time_cmp = (void *)mt->mtimecmp_addr;
>
>         /* Program MTIMER Time Compare */
> -       mt->time_wr(true, next_event - mt->time_delta,
> +       mt->time_wr(true, next_event,
>                     &time_cmp[target_hart - mt->first_hartid]);
>  }
>
> @@ -88,36 +88,51 @@ static struct sbi_timer_device mtimer = {
>         .timer_event_stop = mtimer_event_stop
>  };
>
> +void aclint_mtimer_sync(struct aclint_mtimer_data *mt)
> +{
> +       u64 v1, v2, mv, delta;
> +       u64 *mt_time_val, *ref_time_val;
> +       struct aclint_mtimer_data *reference;
> +
> +       /* Sync-up non-shared MTIME if reference is available */
> +       if (mt->has_shared_mtime || !mt->time_delta_reference)
> +               return;
> +
> +       reference = mt->time_delta_reference;
> +       mt_time_val = (void *)mt->mtime_addr;
> +       ref_time_val = (void *)reference->mtime_addr;
> +       if (!atomic_raw_xchg_ulong(&mt->time_delta_computed, 1)) {
> +               v1 = mt->time_rd(mt_time_val);
> +               mv = reference->time_rd(ref_time_val);
> +               v2 = mt->time_rd(mt_time_val);
> +               delta = mv - ((v1 / 2) + (v2 / 2));
> +               mt->time_wr(false, mt->time_rd(mt_time_val) + delta,
> +                           mt_time_val);
> +       }
> +
> +}
> +
> +void aclint_mtimer_set_reference(struct aclint_mtimer_data *mt,
> +                                struct aclint_mtimer_data *ref)
> +{
> +       if (!mt || !ref || mt == ref)
> +               return;
> +
> +       mt->time_delta_reference = ref;
> +       mt->time_delta_computed = 0;
> +}
> +
>  int aclint_mtimer_warm_init(void)
>  {
> -       u64 v1, v2, mv;
> +       u64 *mt_time_cmp;
>         u32 target_hart = current_hartid();
> -       struct aclint_mtimer_data *reference;
> -       u64 *mt_time_val, *mt_time_cmp, *ref_time_val;
>         struct aclint_mtimer_data *mt = mtimer_hartid2data[target_hart];
>
>         if (!mt)
>                 return SBI_ENODEV;
>
> -       /*
> -        * Compute delta if reference available
> -        *
> -        * We deliberately compute time_delta in warm init so that time_delta
> -        * is computed on a HART which is going to use given MTIMER. We use
> -        * atomic flag timer_delta_computed to ensure that only one HART does
> -        * time_delta computation.
> -        */
> -       if (mt->time_delta_reference) {
> -               reference = mt->time_delta_reference;
> -               mt_time_val = (void *)mt->mtime_addr;
> -               ref_time_val = (void *)reference->mtime_addr;
> -               if (!atomic_raw_xchg_ulong(&mt->time_delta_computed, 1)) {
> -                       v1 = mt->time_rd(mt_time_val);
> -                       mv = reference->time_rd(ref_time_val);
> -                       v2 = mt->time_rd(mt_time_val);
> -                       mt->time_delta = mv - ((v1 / 2) + (v2 / 2));
> -               }
> -       }
> +       /* Sync-up MTIME register */
> +       aclint_mtimer_sync(mt);
>
>         /* Clear Time Compare */
>         mt_time_cmp = (void *)mt->mtimecmp_addr;
> @@ -173,9 +188,7 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
>                 return SBI_EINVAL;
>
>         /* Initialize private data */
> -       mt->time_delta_reference = reference;
> -       mt->time_delta_computed = 0;
> -       mt->time_delta = 0;
> +       aclint_mtimer_set_reference(mt, reference);
>         mt->time_rd = mtimer_time_rd32;
>         mt->time_wr = mtimer_time_wr32;
>
> diff --git a/lib/utils/timer/fdt_timer_mtimer.c b/lib/utils/timer/fdt_timer_mtimer.c
> index 15a36ed..26b5a2a 100644
> --- a/lib/utils/timer/fdt_timer_mtimer.c
> +++ b/lib/utils/timer/fdt_timer_mtimer.c
> @@ -17,19 +17,18 @@
>
>  static unsigned long mtimer_count = 0;
>  static struct aclint_mtimer_data mtimer[MTIMER_MAX_NR];
> +static struct aclint_mtimer_data *mt_reference = NULL;
>
>  static int timer_mtimer_cold_init(void *fdt, int nodeoff,
>                                   const struct fdt_match *match)
>  {
> -       int rc;
> +       int i, rc;
>         unsigned long offset, addr[2], size[2];
> -       struct aclint_mtimer_data *mt, *mtmaster = NULL;
> +       struct aclint_mtimer_data *mt;
>
>         if (MTIMER_MAX_NR <= mtimer_count)
>                 return SBI_ENOSPC;
>         mt = &mtimer[mtimer_count];
> -       if (0 < mtimer_count)
> -               mtmaster = &mtimer[0];
>
>         rc = fdt_parse_aclint_node(fdt, nodeoff, true,
>                                    &addr[0], &size[0], &addr[1], &size[1],
> @@ -37,6 +36,7 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
>         if (rc)
>                 return rc;
>         mt->has_64bit_mmio = true;
> +       mt->has_shared_mtime = false;
>
>         if (match->data) { /* SiFive CLINT */
>                 /* Set CLINT addresses */
> @@ -63,10 +63,39 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
>                         mt->has_64bit_mmio = false;
>         }
>
> -       rc = aclint_mtimer_cold_init(mt, mtmaster);
> +       /* Check if MTIMER device has shared MTIME address */
> +       mt->has_shared_mtime = false;
> +       for (i = 0; i < mtimer_count; i++) {
> +               if (mtimer[i].mtime_addr == mt->mtime_addr) {
> +                       mt->has_shared_mtime = true;
> +                       break;
> +               }
> +       }
> +
> +       /* Initialize the MTIMER device */
> +       rc = aclint_mtimer_cold_init(mt, mt_reference);
>         if (rc)
>                 return rc;
>
> +       /*
> +        * Select first MTIMER device with no associated HARTs as
> +        * our reference MTIMER device
> +        */
> +       if (!mt->hart_count && !mt_reference) {
> +               mt_reference = mt;
> +               /*
> +                * Set reference for already propbed MTIMER devices
> +                * with non-shared MTIME
> +                */
> +               for (i = 0; i < mtimer_count; i++)
> +                       if (!mtimer[i].has_shared_mtime)
> +                               aclint_mtimer_set_reference(&mtimer[i], mt);
> +       }
> +

Is there a reason this is done after cold_init ?

> +       /* Explicitly sync-up MTIMER devices not associated with any HARTs */
> +       if (!mt->hart_count)
> +               aclint_mtimer_sync(mt);
> +
>         mtimer_count++;
>         return 0;
>  }
> --
> 2.25.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi


--
Regards,
Atish



More information about the opensbi mailing list