Re: KVM: x86: update masterclock when kvmclock_offset is calculated

[Paolo Bonzini <pbonzini@redhat.com>]

Il 20/08/2013 20:20, Marcelo Tosatti ha scritto:
> 
> The offset to add to the hosts monotonic time, kvmclock_offset, is
> calculated against the monotonic time at KVM_SET_CLOCK ioctl time.
> 
> Request a master clock update at this time, to reduce a potentially
> unbounded difference between the values of the masterclock and
> the clock value used to calculate kvmclock_offset.
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> Index: linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c
> ===================================================================
> --- linux-2.6-kvmclock-fixes.orig/arch/x86/kvm/x86.c
> +++ linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c
> @@ -3806,6 +3806,7 @@ long kvm_arch_vm_ioctl(struct file *filp
>  		delta = user_ns.clock - now_ns;
>  		local_irq_enable();
>  		kvm->arch.kvmclock_offset = delta;
> +		kvm_gen_update_masterclock(kvm);
>  		break;
>  	}
>  	case KVM_GET_CLOCK: {

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

While reviewing this patch, which BTW looks good, I noticed the handling
of KVM_REQ_MCLOCK_INPROGRESS, the dummy request that is never processed
and is only used to block guest entry.

It seems to me that this bit is not necessary.  After
KVM_REQ_CLOCK_UPDATE is issued, no guest entries will happen because
kvm_guest_time_update will try to take the pvclock_gtod_sync_lock,
currently taken by kvm_gen_update_masterclock.

Thus, you do not need the dummy request.  You can simply issue
KVM_REQ_CLOCK_UPDATE before calling pvclock_update_vm_gtod_copy (with
the side effect of exiting VCPUs).  VCPUs will stall in
kvm_guest_time_update until pvclock_gtod_sync_lock is released by
kvm_gen_update_masterclock.  What do you think?

On top of this, optionally the spinlock could become an rw_semaphore so
that clock updates for different VCPUs will not be serialized.  The
effect is probably not visible, though.

Paolo

> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>