From: Radim Krcmar <rkrcmar@redhat.com>
To: Marcelo Tosatti <mtosatti@redhat.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
Paolo Bonzini <pbonzini@redhat.com>,
Richard Cochran <richardcochran@gmail.com>,
Miroslav Lichvar <mlichvar@redhat.com>
Subject: Re: [patch 1/3] KVM: x86: provide realtime host clock via vsyscall notifiers
Date: Fri, 13 Jan 2017 17:28:09 +0100 [thread overview]
Message-ID: <20170113162809.GE22440@potion> (raw)
In-Reply-To: <20170113153406.GA4796@amt.cnet>
2017-01-13 13:34-0200, Marcelo Tosatti:
> On Fri, Jan 13, 2017 at 04:18:04PM +0100, Radim Krcmar wrote:
>> 2017-01-13 10:01-0200, Marcelo Tosatti:
>> > Expose the realtime host clock and save the TSC value
>> > used for the clock calculation.
>> >
>> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>> >
>> > ---
>> > arch/x86/kvm/x86.c | 38 ++++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 38 insertions(+)
>> >
>> > Index: kvm-ptpdriver/arch/x86/kvm/x86.c
>> > ===================================================================
>> > --- kvm-ptpdriver.orig/arch/x86/kvm/x86.c 2017-01-13 08:59:03.015895353 -0200
>> > +++ kvm-ptpdriver/arch/x86/kvm/x86.c 2017-01-13 09:04:46.581415259 -0200
>> > @@ -1139,6 +1139,8 @@
>> >
>> > u64 boot_ns;
>> > u64 nsec_base;
>> > + u64 wall_time_sec;
>> > + u64 wall_time_snsec;
>>
>> The leading "s" in "snsec" looks like a copy-paste residue.
>
> Just copying the userspace vsyscall interface.
Oh, so the "s" means "sub-" for sub-nanosecond precision.
>> > };
>> >
>> > static struct pvclock_gtod_data pvclock_gtod_data;
>> > @@ -1162,6 +1164,9 @@
>> > vdata->boot_ns = boot_ns;
>> > vdata->nsec_base = tk->tkr_mono.xtime_nsec;
>> >
>> > + vdata->wall_time_sec = tk->xtime_sec;
>> > + vdata->wall_time_snsec = tk->tkr_mono.xtime_nsec;
>>
>> Using tk->tkr_mono offsets for real time seems wrong -- what happens if
>> the real time is half a second shifted from monotonic time?
>
> Both the userspace vsyscall interface and getnstimeofday
> use it for realtime clock.
>
> Monotonic clock adds the offset:
>
> vdata->monotonic_time_snsec = tk->tkr_mono.xtime_nsec
> +
> ((u64)tk->wall_to_monotonic.tv_nsec
> << tk->tkr_mono.shift);
I see, thanks. Makes me wonder why our monotonic time is correct then,
but that is problably thanks to boot_ns.
>> If it's ok, then vdata->nsec_base == vdata->wall_time_snsec, so we don't
>> need it.
>
> Just copying the userspace vsyscall interface.
>
> Do you actually want to change the "s" and unify wall_time_snsec with
> nsec_base?
The "s" isn't important, even though I don't think we do anything that
would justify it, but make use just 8 bytes for both.
Renaming nsec_base is ok, but I'm not sure what tk->tkr_mono.xtime_nsec
is anymore.
next prev parent reply other threads:[~2017-01-13 16:28 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-13 12:01 [patch 0/3] KVM virtual PTP driver Marcelo Tosatti
2017-01-13 12:01 ` [patch 1/3] KVM: x86: provide realtime host clock via vsyscall notifiers Marcelo Tosatti
2017-01-13 15:18 ` Radim Krcmar
2017-01-13 15:34 ` Marcelo Tosatti
2017-01-13 16:28 ` Radim Krcmar [this message]
2017-01-13 17:51 ` Marcelo Tosatti
2017-01-16 15:40 ` Radim Krcmar
2017-01-13 15:41 ` Konrad Rzeszutek Wilk
2017-01-13 15:46 ` Marcelo Tosatti
2017-01-13 12:01 ` [patch 2/3] KVM: x86: add KVM_HC_CLOCK_OFFSET hypercall Marcelo Tosatti
2017-01-13 15:31 ` Radim Krcmar
2017-01-13 15:43 ` Marcelo Tosatti
2017-01-13 17:07 ` Radim Krcmar
2017-01-13 17:57 ` Marcelo Tosatti
2017-01-13 12:01 ` [patch 3/3] PTP: add kvm PTP driver Marcelo Tosatti
2017-01-13 15:56 ` Radim Krcmar
2017-01-13 17:40 ` Marcelo Tosatti
2017-01-16 16:26 ` Radim Krcmar
2017-01-16 16:54 ` Radim Krcmar
2017-01-16 17:08 ` Marcelo Tosatti
2017-01-16 17:27 ` Radim Krcmar
2017-01-16 17:39 ` Marcelo Tosatti
2017-01-16 18:01 ` Radim Krcmar
2017-01-16 19:36 ` Marcelo Tosatti
2017-01-16 19:47 ` Marcelo Tosatti
2017-01-16 20:01 ` Marcelo Tosatti
2017-01-17 8:03 ` Miroslav Lichvar
2017-01-17 11:30 ` Marcelo Tosatti
2017-01-17 15:36 ` Radim Krcmar
2017-01-18 12:17 ` Marcelo Tosatti
2017-01-18 12:24 ` Marcelo Tosatti
2017-01-18 12:46 ` Paolo Bonzini
2017-01-18 13:36 ` Miroslav Lichvar
2017-01-18 14:02 ` Paolo Bonzini
2017-01-18 14:50 ` Marcelo Tosatti
2017-01-18 15:35 ` Radim Krcmar
2017-01-18 15:45 ` Paolo Bonzini
2017-01-18 15:57 ` Marcelo Tosatti
2017-01-18 14:24 ` Marcelo Tosatti
2017-01-18 15:54 ` Miroslav Lichvar
2017-01-18 16:07 ` Paolo Bonzini
2017-01-18 16:14 ` Radim Krcmar
2017-01-18 14:37 ` Marcelo Tosatti
2017-01-18 14:53 ` Marcelo Tosatti
2017-01-18 15:20 ` Radim Krcmar
2017-01-18 15:28 ` Marcelo Tosatti
2017-01-20 14:18 ` Radim Krcmar
2017-01-18 15:59 ` Radim Krcmar
2017-01-16 17:04 ` Marcelo Tosatti
2017-01-16 17:46 ` Radim Krcmar
2017-01-16 19:33 ` Marcelo Tosatti
2017-01-14 15:26 ` Richard Cochran
2017-01-16 15:48 ` Radim Krcmar
2017-01-13 18:45 [patch 0/3] KVM virtual PTP driver (v2) Marcelo Tosatti
2017-01-13 18:45 ` [patch 1/3] KVM: x86: provide realtime host clock via vsyscall notifiers Marcelo Tosatti
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170113162809.GE22440@potion \
--to=rkrcmar@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mlichvar@redhat.com \
--cc=mtosatti@redhat.com \
--cc=pbonzini@redhat.com \
--cc=richardcochran@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).