From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <Ian.Campbell@citrix.com>
Subject: Re: mini-os: x86: virtual-timer interrupt and
 get_time_values_from_xen
Date: Tue, 21 Oct 2014 11:38:49 +0100
Message-ID: <1413887929.23337.20.camel@citrix.com>
References: <CAG4opy-fLbB17OOCH5RRWMOrab=Fpbur_vu_z4BoFgo-wss0zQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta5.messagelabs.com ([195.245.231.135])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <Ian.Campbell@citrix.com>) id 1XgWq6-0007jk-Up
	for xen-devel@lists.xenproject.org; Tue, 21 Oct 2014 10:38:55 +0000
In-Reply-To: <CAG4opy-fLbB17OOCH5RRWMOrab=Fpbur_vu_z4BoFgo-wss0zQ@mail.gmail.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Thomas Leonard <talex5@gmail.com>, Samuel Thibault <samuel.thibault@ens-lyon.org>, keir@xen.org
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
List-Id: xen-devel@lists.xenproject.org

(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.