Re: mini-os: x86: virtual-timer interrupt and get_time_values_from_xen

[Ian Campbell <Ian.Campbell@citrix.com>]

(adding some CCs, and not trimming quotes for their benefit)

On Fri, 2014-10-17 at 15:12 +0100, Thomas Leonard wrote:
> In Mini-OS on x86 we have:
> 
> static void timer_handler(evtchn_port_t ev, struct pt_regs *regs, void *ign)
> {
>     get_time_values_from_xen();
>     update_wallclock();
> }
> 
> static evtchn_port_t port;
> void init_time(void)
> {
>     printk("Initialising timer interface\n");
>     port = bind_virq(VIRQ_TIMER, &timer_handler, NULL);
>     unmask_evtchn(port);
> }
> 
> It seems that the timer values are only correct after the first timer
> interrupt occurs. For example, test.c can output this (if you remove
> xenbus_tester so it starts the timer test quickly enough):
> 
>     Periodic thread started.
>     T(s=8734 us=790808)
>     T(s=1413543796 us=487564)
>     T(s=1413543797 us=487867)
>     ...
> 
> I can fix this by getting it to read the values in init_time:
> 
> void init_time(void)
> {
>     printk("Initialising timer interface\n");
>     port = bind_virq(VIRQ_TIMER, &timer_handler, NULL);
>     unmask_evtchn(port);
> 
>     get_time_values_from_xen();
>     update_wallclock();
> }
> 
> But now I wonder whether the code in timer_handler is needed at all.
> It seems that all code that reads the timer already ensures these
> values are up-to-date by comparing the version number (presumably this
> doesn't work the first time because shadow isn't initialised). With
> the fix to init_time, is it safe to use an empty handler?

It seems like you might be right wrt get_time_values_from_xen(), since
the shadow struct is static to this file all users must be in there, and
on inspection it does seem like everything is always up to date.
(get_nsec_offset appears not, but it's only caller takes care of it).

However I think the update_wallclock is needed, since gettimeofday
doesn't force the update of shadow_ts (nb: different thing to shadow).
Essentially it uses the monotonic_clock to interpolate between timer
ticks.

Ian.