Re: [Minios-devel] [UNIKRAFT PATCHv5 24/46] plat/kvm: Introduce a time_ops for different architectures

["Wei Chen (Arm Technology China)" <Wei.Chen@xxxxxxx>]

> -----Original Message-----
> From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Sent: 2018年9月7日 17:46
> To: Wei Chen (Arm Technology China) <Wei.Chen@xxxxxxx>; minios-
> devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
> Subject: Re: [Minios-devel] [UNIKRAFT PATCHv5 24/46] plat/kvm: Introduce a
> time_ops for different architectures
> 
> 
> 
> On 07.09.2018 07:57, Wei Chen (Arm Technology China) wrote:
> > Hi Simon,
> >
> >> -----Original Message-----
> >> From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> >> Sent: 2018年9月6日 23:53
> >> To: Wei Chen (Arm Technology China) <Wei.Chen@xxxxxxx>; minios-
> >> devel@xxxxxxxxxxxxxxxxxxxx
> >> Cc: Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
> >> Subject: Re: [Minios-devel] [UNIKRAFT PATCHv5 24/46] plat/kvm: Introduce a
> >> time_ops for different architectures
> >>
> >> Hi Wei,
> >>
> >> On 10.08.2018 09:08, Wei Chen wrote:
> >>> Different architectures have different timer implementation. In
> >>> order to avoid using too many #ifdef, we introduce this time_ops
> >>> for different architectures to implement arch_timer_ops.
> >>
> >> does the timer implementation this change during runtime or can you
> >> settle on one specific timer implementation during compile time?
> >
> > No, timer implementation is settled during compile time, it could not
> > be changed during runtime.
> >
> >> Why not providing a different C file that implements your ARM timer? We
> >> could avoid the indirection and save the timeops struct.
> >
> > I have implemented a different C file for ARM timer already. But I can't
> > use the same API names for ARM timer library. Because the tscclock_ prefix
> > is x86 specific. If I don't use the arch_timer_ops to standardize the
> > callback function, in ukplat_monotonic_clock we have to use:
> >
> 
> Now I see the problem. Yes, that move with tscclock wasn't clever.
> I think you could solve it by moving time.c and tscclock.c to the x86/
> subdirectory and create a new file in arm or arm64 with your
> implementation. You would then provide the UKPLAT API functions
> (ukplat_monotonic_clock() and so on) directly with your files.
> 
> 

Ok, This is also a solution to the problem, I will change to this in next
version.

> > __nsec ukplat_monotonic_clock(void)
> > {
> > #if defined(__x86_64__)
> > return tscclock_monotonic();
> > #else if defined(__ARM64__)
> > return generic_timer_monotonic ();
> > #endif
> > }
> >
> >>
> >>>
> >>> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> >>> ---
> >>>    include/uk/plat/time.h | 8 ++++++++
> >>>    plat/kvm/time.c        | 7 ++++---
> >>>    plat/kvm/tscclock.c    | 6 ++++++
> >>>    3 files changed, 18 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/include/uk/plat/time.h b/include/uk/plat/time.h
> >>> index 202e0f9..2aacfce 100644
> >>> --- a/include/uk/plat/time.h
> >>> +++ b/include/uk/plat/time.h
> >>> @@ -47,6 +47,14 @@ void ukplat_time_fini(void);
> >>>
> >>>    __nsec ukplat_monotonic_clock(void);
> >>>
> >>> +struct ukplat_time_ops {
> >>> + int (*init)(void);
> >>> + __u64 (*monotonic)(void);
> >>> + __u64 (*epochoffset)(void);
> >>> +};
> >>> +
> >>> +extern struct ukplat_time_ops arch_timer_ops;
> >>> +
> >>>    #ifdef __cplusplus
> >>>    }
> >>>    #endif
> >>> diff --git a/plat/kvm/time.c b/plat/kvm/time.c
> >>> index 1fb48bf..eb8bfc2 100644
> >>> --- a/plat/kvm/time.c
> >>> +++ b/plat/kvm/time.c
> >>> @@ -31,17 +31,18 @@
> >>>    #include <kvm/tscclock.h>
> >>>    #include <uk/assert.h>
> >>>
> >>> +struct ukplat_time_ops *time_ops = &arch_timer_ops;
> >>>
> >>>    /* return ns since time_init() */
> >>>    __nsec ukplat_monotonic_clock(void)
> >>>    {
> >>> - return tscclock_monotonic();
> >>> + return time_ops->monotonic();
> >>>    }
> >>>
> >>>    /* return wall time in nsecs */
> >>>    __nsec ukplat_clock_wall(void)
> >>>    {
> >>> - return tscclock_monotonic() + tscclock_epochoffset();
> >>> + return time_ops->monotonic() + time_ops->epochoffset();
> >>>    }
> >>>
> >>>    static int timer_handler(void *arg __unused)
> >>> @@ -59,7 +60,7 @@ void ukplat_time_init(void)
> >>>           if (rc < 0)
> >>>                   UK_CRASH("Failed to register timer interrupt 
> >>> handler\n");
> >>>
> >>> - rc = tscclock_init();
> >>> + rc = time_ops->init();
> >>>           if (rc < 0)
> >>>                   UK_CRASH("Failed to initialize TSCCLOCK\n");
> >>>    }
> >>> diff --git a/plat/kvm/tscclock.c b/plat/kvm/tscclock.c
> >>> index 8961659..990902b 100644
> >>> --- a/plat/kvm/tscclock.c
> >>> +++ b/plat/kvm/tscclock.c
> >>> @@ -358,3 +358,9 @@ void time_block_until(__snsec until)
> >>>                           break;
> >>>           }
> >>>    }
> >>> +
> >>> +struct ukplat_time_ops arch_timer_ops = {
> >>> + .init = tscclock_init,
> >>> + .monotonic = tscclock_monotonic,
> >>> + .epochoffset = tscclock_epochoffset,
> >>> +};
> >>>
> >>
> >> Thanks,
> >>
> >> Simon
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel