Re: [Minios-devel] [UNIKRAFT PATCH v4 3/9] plat/linuxu: Add linuxu (x86_64) timer support

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello,


On 08/30/2018 03:52 PM, Florian Schmidt wrote:
> Hi Sharan,
>
>
> On 08/27/2018 02:38 PM, Costin Lupu wrote:
>
> > > Missing include of signal.h
> > Right, more than that the current patch should come *after* the patch
> > adding interrupts support in linuxu.
> I'll reverse the order and include linuxu/signal.h
>
> > > > +void ukplat_time_init(void)
> > > > +{
> > > > +    struct uk_sigevent sigev;
> > > > +    struct itimerspec its;
> > > > +    int rc;
> > > > +
> > > > +    ukplat_irq_register(TIMER_SIGNUM, timer_handler, NULL);
> > > > +
> > > > +    memset(&sigev, 0, sizeof(sigev));
> > > > +    sigev.sigev_notify = 0;
> > >
> > > A minor comment, prefer if we use SIGALRM inplace of TIMER_SIGNUM :)
> > > since we are working with signal numbers here.
> > TIMER_SIGNUM is a define over the preferred signal number (in our case
> > SIGALRM). I don't see what's wrong with this approach.
> I think this is fine and we can keep it like that?
>
> > > >      void ukplat_time_fini(void)
> > > >    {
> > >
> > > Do we want to unregister the timer interrupt while cleaning up the
> > > platform?
> > Yeah, why not?
> >
> > > > +    sys_timer_delete(timerid);
> > > >    }
> Also seems like the right thing to do to me. ANy reasons why that would 
> be a bad idea?
This is fine and I had it as a note for self to check if we do 
ukplat_irq_unregister. Sorry my mistake!!!

> Cheers,
> Florian
Thanks & Regards
Sharan

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