2017-05-03 10:43-0300, Marcelo Tosatti:
> In the masterclock enabled case, kvmclock_offset must be adjusted so
> that user_ns.clock = master_kernel_ns + kvmclock_offset (that is, the
> value set from KVM_SET_CLOCK is the one visible at system_timestamp).
IIUC, we want to achieve
user_ns.clock == get_kvmclock_ns(kvm)
and the important fix for kvm master clock is the move of
kvm_gen_update_masterclock() before we read the time.
The rest is just a minor optimization that also ignores time since
master_kernel_ns() and therefore pins user_ns.clock to a slightly
earlier time.
But all attention was given to the "minor optimization" -- have I missed
something about the direct use of ka->master_kernel_ns?
(If not, I'd rather have just the one-line move.)
---
Detailed reasoning.
kvm_gen_update_masterclock() shifts the master clock (changes its
percieved frequency, because it is reset from kernel boot clock again,
even though they are diverging), so we must not compute the
kvmclock_offset with an old master clock and apply it to a shifted one.
Using offset from ka->master_kernel_ns or get_kvmclock_ns()
("ka->master_kernel_ns + small delta") doesn't make much difference
then, because the user_ns is already an indeterminable point in the
past. (We just assume that user_ns.clock is now, whenever that is.)
---
And a possible improvement.
Using kvm_gen_update_masterclock() seems superfluous. There are three
major cases depending on state of the master clock:
enabled)
We can just match the kvmclock_offset so the following holds
user_ns.clock == get_kvmclock_ns(kvm)
No need to update the master clock.
disabled)
The kvmclock_offset is set from the kernel boot clock and that is
already correct.
disabled, but call to kvm_gen_update_masterclock() would enable it)
The master clock will be updates with the kernel boot clock when a
VCPU runs. This means that master clock and kernel boot clock will
begin diverging at a later point, but the initial offset is the same.
Userspace can only tell the difference by calling KVM_GET_CLOCK
afterwards and seeing the stable bit unset.
Guest is mostly unaffected -- the inaccuracy from calling
KVM_SET_CLOCK is much bigger than accumulation of a slightly
different frequency will in few jiffies.
Do you see a reason to use kvm_gen_update_masterclock()?
I think that the code could be just:
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 464da936c53d..f024216a858d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4177,7 +4177,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = 0;
now_ns = get_kvmclock_ns(kvm);
kvm->arch.kvmclock_offset += user_ns.clock - now_ns;
- kvm_gen_update_masterclock(kvm);
+ kvm_make_all_cpus_request(kvm, KVM_REQ_CLOCK_UPDATE);
break;
}
case KVM_GET_CLOCK: {
Thanks.
