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

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

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.

> __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