On Mon, Oct 22, 2018 at 1:53 AM Rick Payne <ri...@rossfell.co.uk> wrote:

>
> We need to write the wall clock MSR every time we use it, to ensure we
> get the updated value. This allows the guest OSv to track the time of
> the host correctly.
>

After this patch, the host NTP keeps the time accurate on the guest as well?

Wow, I don't know how we missed this... Glauber even wrote in Linux's
virtual/kvm/msr.txt:

           MSR_KVM_WALL_CLOCK_NEW:
     ....
     The hypervisor is only guaranteed to update this data at the moment of
MSR write.
     Users that want to reliably query this information more than once have
     to write more than once to this MSR.

So it seems you're indeed right!

I seems that Linux as a guest also does what you did in this patch - use
the MSR on every invocation.
This is sad because it will slow down every call of
osv::clock::wall::now(), although it won't
slow down osv::clock::uptime::now() which is much more important (e.g., for
the scheduler).

*Glauber*, do you know if we have any other option?

It's surprising for me that KVM cannot write to the boot wallclock address
when it is changed (by Linux),
instead relying on polling from the guest side.

Another thing we could do is to reread the wall clock periodically (e.g.,
once each second)
but this will be ugly and cause probably not very desirable clock skips.



> Signed-off-by: Rick Payne <ri...@rossfell.co.uk>
> ---
>  drivers/kvmclock.cc | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/kvmclock.cc b/drivers/kvmclock.cc
> index 68389dfb..2e182954 100644
> --- a/drivers/kvmclock.cc
> +++ b/drivers/kvmclock.cc
> @@ -50,11 +50,8 @@ static u8 get_pvclock_flags()
>  kvmclock::kvmclock()
>      : _pvclock(get_pvclock_flags())
>  {
> -    auto wall_time_msr = (_new_kvmclock_msrs) ?
> -                         msr::KVM_WALL_CLOCK_NEW :
> msr::KVM_WALL_CLOCK;
>      _wall = new pvclock_wall_clock;
>      memset(_wall, 0, sizeof(*_wall));
> -    processor::wrmsr(wall_time_msr, mmu::virt_to_phys(_wall));
>  }
>
>  void kvmclock::init_on_cpu()
> @@ -79,6 +76,9 @@ bool kvmclock::probe()
>
>  u64 kvmclock::wall_clock_boot()
>  {
> +    auto wall_time_msr = (_new_kvmclock_msrs) ?
> +                         msr::KVM_WALL_CLOCK_NEW :
> msr::KVM_WALL_CLOCK;
> +    processor::wrmsr(wall_time_msr, mmu::virt_to_phys(_wall));
>

Perhaps we can shave off a few more nanoseconds if we saved wall_time_msr
and mmu::virt_to_phys(_wall) and didn't need to recalculate them every time?

     return _pvclock.wall_clock_boot(_wall);
>  }
>
> --
> 2.17.1
>
>
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to