Re: [Minios-devel] [UNIKRAFT PATCH 7/7] plat/common: Implement generic_timer_cpu_block

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Wei,

On 13/12/2018 09:19, Wei Chen wrote:
> This function will be used when system enter sleep and need wakeup
> in a specific time. For ns_to_ticks precision, we limited the max
> sleep time to 3600 seconds.
>
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> Signed-off-by: Jianyong Wu <jianyong.wu@xxxxxxx>
> ---
>   plat/common/arm/time.c | 78 +++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 73 insertions(+), 5 deletions(-)
>
> diff --git a/plat/common/arm/time.c b/plat/common/arm/time.c
> index ae539a0..46f5f93 100644
> --- a/plat/common/arm/time.c
> +++ b/plat/common/arm/time.c
> @@ -76,6 +76,11 @@ static uint32_t tick_per_ns;
>    */
>   #define __MAX_CONVERT_SECS    3600UL
>   
> +/*
> + * Minimum delta to sleep using generic timer.
> + */
> +static uint32_t counter_mini_delta;
> +
>   /* How many nanoseconds per second */
>   #define NSEC_PER_SEC ukarch_time_sec_to_nsec(1)
>   
> @@ -219,6 +224,67 @@ static uint64_t generic_timer_epochoffset(void)
>         return 0;
>   }
>   
> +/*
> + * Returns early if any interrupts are serviced, or if the requested delay is
> + * too short. Must be called with interrupts disabled, will enable interrupts
> + * "atomically" during idle loop.
> + *
> + * This function must be called only from the scheduler. It will screw
> + * your system if you do otherwise. And, there is no reason you
> + * actually want to use it anywhere else. THIS IS NOT A YIELD or any
> + * kind of mutex_lock. It will simply halt the cpu, not allowing any
> + * other thread to execute.
> + */
> +static void generic_timer_cpu_block(uint64_t until_ns)
> +{
> +       uint64_t now_ns, delta_ns;
> +       uint64_t now_ticks, delta_ticks;
> +
> +       UK_ASSERT(ukplat_lcpu_irqs_disabled());
> +
> +       /* Record current ticks */
> +       now_ticks = generic_timer_get_ticks();
> +       now_ns = ticks_to_ns(now_ticks - boot_ticks);
> +
> +       /*
> +        * Compute delta in counter ticks. Return if it is less than minimum
> +        * safe amount of ticks. Essentially this will cause us to spin until
> +        * the timeout.
> +        */
> +       delta_ns = until_ns - now_ns;
> +       delta_ticks = ns_to_ticks(delta_ns);
> +       if (delta_ticks < counter_mini_delta) {
> +               /*
> +                * Since we are "spinning", quickly enable interrupts in
> +                * the hopes that we might get new work and can do something
> +                * else than spin.
> +                */
> +               ukplat_lcpu_enable_irq();
> +               nop();
Why do you need a nop() here? Should not just be sufficient to do enable -> 
disable?

[...]

>   static int generic_timer_init(int fdt_timer)
>   {
>         /* Get counter frequency from DTB or register */
> @@ -244,6 +310,12 @@ static int generic_timer_init(int fdt_timer)
>         /* We disallow zero ns_per_tick */
>         UK_BUGON(!tick_per_ns);
>   
> +	/*
> +        * Set minimal counter delta, programming seems to have an overhead
> +        * of 3-4us, but play it safe here.
Overhead on which platform? Bear in mind that the Arm Arm does not tell you how 
long it will take to write to the timer. So you at least need to explain how you 
found out that.
> +        */
> +       counter_mini_delta = ns_to_ticks(4000);
> +
>         return 0;
>   }
>   
> @@ -264,11 +336,7 @@ unsigned long sched_have_pending_events;
>   void time_block_until(__snsec until)
>   {
>         while ((__snsec) ukplat_monotonic_clock() < until) {
> -               /*
> -                * TODO:
> -                * As we haven't support interrupt on Arm, so we just
> -                * use busy polling for now.
> -                */
> +               generic_timer_cpu_block(until);
>                 if (__uk_test_and_clear_bit(0, &sched_have_pending_events))
>                         break;
>         }
Cheers,

--
Julien Grall

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel