From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753161AbdKMPA5 (ORCPT ); Mon, 13 Nov 2017 10:00:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36008 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752336AbdKMPA4 (ORCPT ); Mon, 13 Nov 2017 10:00:56 -0500 Subject: Re: [PATCH] KVM: X86: Avoid to handle first-time write when updating the pv stuffs each time To: Wanpeng Li , linux-kernel@vger.kernel.org, kvm@vger.kernel.org Cc: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Wanpeng Li , Liran Alon References: <1510555676-14977-1-git-send-email-wanpeng.li@hotmail.com> From: Paolo Bonzini Message-ID: <377129fc-27b9-8d70-bcd5-718eecd8b801@redhat.com> Date: Mon, 13 Nov 2017 16:00:52 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <1510555676-14977-1-git-send-email-wanpeng.li@hotmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 13 Nov 2017 15:00:56 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 13/11/2017 07:47, Wanpeng Li wrote: > From: Wanpeng Li > > There is a logic to handle first-time write when updating the > pvclock/wall clock/steal time shared memory pages each time, > actually we should do this logic during pv stuffs setup if we > suspect the version-field can't be guranteed to be initialized > to an even number by the guest. This patch fixes it by handling > the first-time write of pvclock/steal time during setup since > the update is frequent, and keeping the wall clock since it is > rare updating. The cost is almost zero, and the code is smaller without this patch, so I think it's a little better to leave things as is. Paolo > Cc: Paolo Bonzini > Cc: Radim Krčmář > Cc: Liran Alon > Signed-off-by: Wanpeng Li > --- > arch/x86/kvm/x86.c | 29 ++++++++++++++++++++++------- > 1 file changed, 22 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 4552427..19311e0 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -1833,9 +1833,6 @@ 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, > @@ -2126,9 +2123,6 @@ static void record_steal_time(struct kvm_vcpu *vcpu) > > vcpu->arch.st.steal.preempted = 0; > > - if (vcpu->arch.st.steal.version & 1) > - vcpu->arch.st.steal.version += 1; /* first time write, random junk */ > - > vcpu->arch.st.steal.version += 1; > > kvm_write_guest_cached(vcpu->kvm, &vcpu->arch.st.stime, > @@ -2256,8 +2250,19 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > &vcpu->arch.pv_time, data & ~1ULL, > sizeof(struct pvclock_vcpu_time_info))) > vcpu->arch.pv_time_enabled = false; > - else > + else { > + struct pvclock_vcpu_time_info guest_hv_clock; > + > vcpu->arch.pv_time_enabled = true; > + if (unlikely(kvm_read_guest_cached(vcpu->kvm, &vcpu->arch.pv_time, > + &guest_hv_clock, sizeof(guest_hv_clock)))) > + break; > + if (guest_hv_clock.version & 1) > + ++guest_hv_clock.version; /* first time write, random junk */ > + kvm_write_guest_cached(vcpu->kvm, &vcpu->arch.pv_time, > + &vcpu->arch.hv_clock, > + sizeof(vcpu->arch.hv_clock.version)); > + } > > break; > } > @@ -2283,6 +2288,16 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > if (!(data & KVM_MSR_ENABLED)) > break; > > + if (unlikely(kvm_read_guest_cached(vcpu->kvm, &vcpu->arch.st.stime, > + &vcpu->arch.st.steal, sizeof(struct kvm_steal_time)))) > + break; > + > + if (vcpu->arch.st.steal.version & 1) > + vcpu->arch.st.steal.version += 1; /* first time write, random junk */ > + > + kvm_write_guest_cached(vcpu->kvm, &vcpu->arch.st.stime, > + &vcpu->arch.st.steal, sizeof(struct kvm_steal_time)); > + > kvm_make_request(KVM_REQ_STEAL_UPDATE, vcpu); > > break; >