Re: [Minios-devel] [UNIKRAFT PATCH v5 1/7] plat/common: Calculate shift factors for coversion between ns and tick

["Justin He (Arm Technology China)" <Justin.He@xxxxxxx>]

> -----Original Message-----
> From: Minios-devel <minios-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf
> Of Jia He
> Sent: 2019年9月25日 17:14
> To: minios-devel@xxxxxxxxxxxxxxxxxxxx; Santiago.Pagani@xxxxxxxxx
> Cc: Felipe Huici <felipe.huici@xxxxxxxxx>; Justin He (Arm Technology China)
> <Justin.He@xxxxxxx>; Wei Chen (Arm Technology China)
> <Wei.Chen@xxxxxxx>; Jianyong Wu (Arm Technology China)
> <Jianyong.Wu@xxxxxxx>; Kaly Xin (Arm Technology China)
> <Kaly.Xin@xxxxxxx>; Julien Grall <Julien.Grall@xxxxxxx>;
> Sharan.Santhanam@xxxxxxxxx; Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Subject: [Minios-devel] [UNIKRAFT PATCH v5 1/7] plat/common: Calculate
> shift factors for coversion between ns and tick
>
> We had shift factor for coverting counter ticks to ns, but it's not
> enough. Sometime, we need to covert ns to ticks. For example, we'll
> transfer sleep(ns) to counter ticks. If we don't have the shift factor,
> the conversion accuracy will be lower.
>
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> Signed-off-by: Jianyong Wu <jianyong.wu@xxxxxxx>
> Signed-off-by: Jia He <justin.he@xxxxxxx>
> ---
>  plat/common/arm/time.c | 118 ++++++++++++++++++++++++++++++++-------
> --
>  1 file changed, 93 insertions(+), 25 deletions(-)
>
> diff --git a/plat/common/arm/time.c b/plat/common/arm/time.c
> index 1b30903..0215932 100644
> --- a/plat/common/arm/time.c
> +++ b/plat/common/arm/time.c
> @@ -47,21 +47,89 @@
>  static uint64_t boot_ticks;
>  static uint32_t counter_freq;
>
> -/*
> - * Shift factor for counter scaling multiplier; referred to as S in the
> - * following comments.
> - */
> -static uint8_t counter_shift;
>
> -/* Multiplier for converting counter ticks to nsecs. (0.S) fixed point. */
> +/* Shift factor for converting ticks to ns */
> +static uint8_t counter_shift_to_ns;
> +
> +/* Shift factor for converting ns to ticks */
> +static uint8_t counter_shift_to_tick;
> +
> +/* Multiplier for converting counter ticks to nsecs */
>  static uint32_t ns_per_tick;
>
> +/* Multiplier for converting nsecs to counter ticks */
> +static uint32_t tick_per_ns;
> +
> +/*
> + * The maximum time range in seconds which can be converted by
> multiplier
> + * and shift factors. This will guarantee the converted value not to exceed
> + * 64-bit unsigned integer. Increase the time range will reduce the accuracy
> + * of conversion, because we will get smaller multiplier and shift factors.
> + * In this case, we selected 3600s as the time range.
> + */
> +#define __MAX_CONVERT_SECS   (3600UL)
> +#define __MAX_CONVERT_NS     (3600UL*NSEC_PER_SEC)
> +static uint64_t max_convert_ticks;
> +
>  /* How many nanoseconds per second */
>  #define NSEC_PER_SEC ukarch_time_sec_to_nsec(1)
>
>  static inline uint64_t ticks_to_ns(uint64_t ticks)
>  {
> -     return (ns_per_tick * ticks) >> counter_shift;
> +     UK_ASSERT(ticks <= max_convert_ticks);

I observed that if someone called this early in booting stage, it
will cause a pointless assert here. E.g. before max_convert_ticks
is initialized.
Tend to initialized max_convert_ticks) as u64(-1) instead of 0.

--
Cheers,
Justin (Jia He)


> +
> +     return (ns_per_tick * ticks) >> counter_shift_to_ns;
> +}
> +
> +static inline uint64_t ns_to_ticks(uint64_t ns)
> +{
> +     UK_ASSERT(ns <= __MAX_CONVERT_NS);
> +
> +     return (tick_per_ns * ns) >> counter_shift_to_tick;
> +}
> +
> +/*
> + * Calculate multiplier/shift factors for scaled math.
> + */
> +static void calculate_mult_shift(uint32_t *mult, uint8_t *shift,
> +             uint64_t from, uint64_t to)
> +{
> +     uint64_t tmp;
> +     uint32_t sft, sftacc = 32;
> +
> +     /*
> +      * Calculate the shift factor which is limiting the conversion
> +      * range:
> +      */
> +     tmp = ((uint64_t)__MAX_CONVERT_SECS * from) >> 32;
> +     while (tmp) {
> +             tmp >>= 1;
> +             sftacc--;
> +     }
> +
> +
> +     /*
> +      * Calculate shift factor (S) and scaling multiplier (M).
> +      *
> +      * (S) needs to be the largest shift factor (<= max_shift) where
> +      * the result of the M calculation below fits into uint32_t
> +      * without truncation.
> +      *
> +      * multiplier = (target << shift) / source
> +      */
> +     for (sft = 32; sft > 0; sft--) {
> +             tmp = (uint64_t) to << sft;
> +
> +             /* Ensuring we round to nearest when calculating the
> +              * multiplier
> +              */
> +             tmp += from / 2;
> +             tmp /= from;
> +             if ((tmp >> sftacc) == 0)
> +                     break;
> +     }
> +     *mult = tmp;
> +     *shift = sft;
>  }
>
>  /*
> @@ -145,29 +213,29 @@ static __u64  generic_timer_epochoffset(void)
>
>  static int generic_timer_init(void)
>  {
> +     counter_freq = get_counter_frequency();
> +
>       /*
> -      * Calculate counter shift factor and scaling multiplier.
> -      *
> -      * counter_shift (S) needs to be the largest (<=32) shift factor where
> -      * the result of the ns_per_tick calculation below fits into uint32_t
> -      * without truncation. Note that we disallow an S of zero to ensure
> -      * the loop always terminates.
> -      *
> -      * (0.S) ns_per_tick = NSEC_PER_SEC (S.S) / counter_freq (S.0)
> +      * Calculate the shift factor and scaling multiplier for
> +      * converting ticks to ns.
>        */
> -     uint64_t tmp;
> +     calculate_mult_shift(&ns_per_tick, &counter_shift_to_ns,
> +                             counter_freq, NSEC_PER_SEC);
>
> -     counter_freq = get_counter_frequency();
> -     counter_shift = 32;
> -     do {
> -             tmp = (NSEC_PER_SEC << counter_shift) / counter_freq;
> -             if ((tmp & 0xFFFFFFFF00000000L) == 0L)
> -                     ns_per_tick = (uint32_t)tmp;
> -             else
> -                     counter_shift--;
> -     } while (counter_shift > 0 && ns_per_tick == 0L);
> +     /* We disallow zero ns_per_tick */
>       UK_BUGON(!ns_per_tick);
>
> +     /*
> +      * Calculate the shift factor and scaling multiplier for
> +      * converting ns to ticks.
> +      */
> +     calculate_mult_shift(&tick_per_ns, &counter_shift_to_tick,
> +                             NSEC_PER_SEC, counter_freq);
> +
> +     /* We disallow zero ns_per_tick */
> +     UK_BUGON(!tick_per_ns);
> +
> +     max_convert_ticks = __MAX_CONVERT_SECS*counter_freq;
>       /*
>        * Monotonic time begins at boot_ticks (first read of counter
>        * before calibration).
> --
> 2.17.1
>
>
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel