Something like this?
diff --git a/drivers/kvmclock.cc b/drivers/kvmclock.cc
index 68389dfb..4af229d5 100644
--- a/drivers/kvmclock.cc
+++ b/drivers/kvmclock.cc
@@ -31,6 +31,8 @@ protected:
private:
static bool _new_kvmclock_msrs;
pvclock_wall_clock* _wall;
+ u64 _wall_phys;
+ msr _wall_time_msr;
static percpu<pvclock_vcpu_time_info> _sys;
pvclock _pvclock;
};
@@ -50,11 +52,11 @@ 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_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));
+ _wall_phys = mmu::virt_to_phys(_wall);
}
void kvmclock::init_on_cpu()
@@ -79,6 +81,7 @@ bool kvmclock::probe()
u64 kvmclock::wall_clock_boot()
{
+ processor::wrmsr(_wall_time_msr, _wall_phys);
return _pvclock.wall_clock_boot(_wall);
}
Cheers,
Rick
On Mon, 2018-10-22 at 16:13 +0300, Nadav Har'El wrote:
>
> 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);
> > }
> >
--
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.
