From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liran Alon Subject: Re: [PATCH] KVM: x86: pvclock: Handle first-time write to pvclock-page contains random junk Date: Mon, 13 Nov 2017 02:44:26 +0200 Message-ID: <5A08EAEA.9060701@ORACLE.COM> References: <1509891090-8985-1-git-send-email-liran.alon@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Paolo Bonzini , Radim Krcmar , kvm , idan.brown@ORACLE.COM, Konrad Rzeszutek Wilk To: Wanpeng Li Return-path: Received: from userp1040.oracle.com ([156.151.31.81]:51327 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751020AbdKMAof (ORCPT ); Sun, 12 Nov 2017 19:44:35 -0500 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 13/11/17 02:40, Wanpeng Li wrote: > 2017-11-05 22:11 GMT+08:00 Liran Alon : >> When guest passes KVM it's pvclock-page GPA via WRMSR to >> MSR_KVM_SYSTEM_TIME / MSR_KVM_SYSTEM_TIME_NEW, KVM don't initialize >> pvclock-page to some start-values. It just requests a clock-update which > > KVM does, please refer to memset(hv_clock, 0, size) in kvmclock_init(). kvmclock_init() is the code that runs in KVM-guest. I was talking here about the code that handles the WRMSR in KVM hypervisor code. The issue happens when the guest doesn't init pvclock-page as done in kvmclock_init(). Not all guests behave nicely :) -Liran > > Regards, > Wanpeng Li > >> will happen before entering to guest. >> >> The clock-update logic will call kvm_setup_pvclock_page() to update the >> pvclock-page with info. However, kvm_setup_pvclock_page() *wrongly* >> assumes that the version-field is initialized to an even number. This is >> wrong because at first-time write, field could be any-value. >> >> Fix simply makes sure that if first-time version-field is odd, increment >> it once more to make it even and only then start standard logic. >> This follows same logic as done in other pvclock shared-pages (See >> kvm_write_wall_clock() and record_steal_time()). >> >> Signed-off-by: Liran Alon >> Reviewed-by: Nikita Leshenko >> Reviewed-by: Konrad Rzeszutek Wilk >> Signed-off-by: Konrad Rzeszutek Wilk >> --- >> arch/x86/kvm/x86.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 03869eb7fcd6..181106080e41 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -1830,6 +1830,9 @@ static void kvm_setup_pvclock_page(struct kvm_vcpu *v) >> */ >> BUILD_BUG_ON(offsetof(struct pvclock_vcpu_time_info, version) != 0); >> >> + if (guest_hv_clock.version & 1) >> + ++guest_hv_clock.version; /* first time write, random junk */ >> + >> vcpu->hv_clock.version = guest_hv_clock.version + 1; >> kvm_write_guest_cached(v->kvm, &vcpu->pv_time, >> &vcpu->hv_clock, >> -- >> 1.9.1 >>