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:55:25 +0200 Message-ID: <5A08ED7D.2020003@ORACLE.COM> References: <1509891090-8985-1-git-send-email-liran.alon@oracle.com> <5A08EAEA.9060701@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]:17208 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751243AbdKMAze (ORCPT ); Sun, 12 Nov 2017 19:55:34 -0500 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 13/11/17 02:52, Wanpeng Li wrote: > 2017-11-13 8:44 GMT+08:00 Liran Alon : >> >> >> 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 :) > > But the codes which you modify just works for kvm guest I think. No, it's the code that runs in KVM hypervisor for any guest that knows how to work with KVM pv-clock PV interface. Linux guest is just one of the possible guests which can use this interface. kvmclock_init() you mentioned is part of the linux-guest. But there are other guests which use KVM pv-clock PV interface. -Liran > > Regards, > Wanpeng Li > >> >> -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 >>>> >>