From patchwork Fri Aug 3 05:55:44 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Nikunj A. Dadhania" X-Patchwork-Id: 1268571 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id B1D63DFF71 for ; Fri, 3 Aug 2012 05:56:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751319Ab2HCF4i (ORCPT ); Fri, 3 Aug 2012 01:56:38 -0400 Received: from e28smtp02.in.ibm.com ([122.248.162.2]:43006 "EHLO e28smtp02.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751079Ab2HCF4g (ORCPT ); Fri, 3 Aug 2012 01:56:36 -0400 Received: from /spool/local by e28smtp02.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 3 Aug 2012 11:26:31 +0530 Received: from d28relay04.in.ibm.com (9.184.220.61) by e28smtp02.in.ibm.com (192.168.1.132) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 3 Aug 2012 11:26:26 +0530 Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay04.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q735uONY25362450 for ; Fri, 3 Aug 2012 11:26:25 +0530 Received: from d28av02.in.ibm.com (loopback [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q735uNEs015277 for ; Fri, 3 Aug 2012 15:56:24 +1000 Received: from abhimanyu.in.ibm.com.vnet.linux.ibm.com (abhimanyu.in.ibm.com [9.124.35.147]) by d28av02.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id q735uNCL015270 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 3 Aug 2012 15:56:23 +1000 From: Nikunj A Dadhania To: Marcelo Tosatti Cc: peterz@infradead.org, avi@redhat.com, raghukt@linux.vnet.ibm.com, alex.shi@intel.com, mingo@elte.hu, kvm@vger.kernel.org, hpa@zytor.com Subject: Re: [PATCH v3 4/8] KVM-HV: Add VCPU running/pre-empted state for guest In-Reply-To: <20120802195628.GA30673@amt.cnet> References: <20120731104312.16662.27889.stgit@abhimanyu.in.ibm.com> <20120731104838.16662.82021.stgit@abhimanyu.in.ibm.com> <20120802195628.GA30673@amt.cnet> User-Agent: Notmuch/0.10.2+70~gf0e0053 (http://notmuchmail.org) Emacs/24.0.95.1 (x86_64-unknown-linux-gnu) Date: Fri, 03 Aug 2012 11:25:44 +0530 Message-ID: <87a9yc7c4f.fsf@abhimanyu.in.ibm.com> MIME-Version: 1.0 x-cbid: 12080305-5816-0000-0000-000003D78300 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Thu, 2 Aug 2012 16:56:28 -0300, Marcelo Tosatti wrote: > > > > + case MSR_KVM_VCPU_STATE: > > + vcpu->arch.v_state.vs_page = gfn_to_page(vcpu->kvm, data >> PAGE_SHIFT); > > + vcpu->arch.v_state.vs_offset = data & ~(PAGE_MASK | KVM_MSR_ENABLED); > > Assign vs_offset after success. > > > + > > + if (is_error_page(vcpu->arch.v_state.vs_page)) { > > + kvm_release_page_clean(vcpu->arch.time_page); > > + vcpu->arch.v_state.vs_page = NULL; > > + pr_info("KVM: VCPU_STATE - Unable to pin the page\n"); > > Missing break or return; > > > + } > > + vcpu->arch.v_state.msr_val = data; > > + break; > > + > > case MSR_IA32_MCG_CTL: > > Please verify this code carefully again. > > Also leaking the page reference. > > > vcpu->arch.apf.msr_val = 0; > > vcpu->arch.st.msr_val = 0; > > + vcpu->arch.v_state.msr_val = 0; > > Add a newline and comment (or even better a new helper). > > > > kvmclock_reset(vcpu); > How about something like the below. I have tried to look at time_page for reference: --- 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 diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 580abcf..c82cc12 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1604,6 +1604,16 @@ static void kvm_clear_vcpu_state(struct kvm_vcpu *vcpu) kunmap_atomic(kaddr); } +static void kvm_vcpu_state_reset(struct kvm_vcpu *vcpu) +{ + vcpu->arch.v_state.msr_val = 0; + vcpu->arch.v_state.vs_offset = 0; + if (vcpu->arch.v_state.vs_page) { + kvm_release_page_dirty(vcpu->arch.v_state.vs_page); + vcpu->arch.v_state.vs_page = NULL; + } +} + int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data) { bool pr = false; @@ -1724,14 +1734,17 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data) break; case MSR_KVM_VCPU_STATE: + kvm_vcpu_state_reset(vcpu); + vcpu->arch.v_state.vs_page = gfn_to_page(vcpu->kvm, data >> PAGE_SHIFT); - vcpu->arch.v_state.vs_offset = data & ~(PAGE_MASK | KVM_MSR_ENABLED); if (is_error_page(vcpu->arch.v_state.vs_page)) { - kvm_release_page_clean(vcpu->arch.time_page); + kvm_release_page_clean(vcpu->arch.v_state.vs_page); vcpu->arch.v_state.vs_page = NULL; pr_info("KVM: VCPU_STATE - Unable to pin the page\n"); + break; } + vcpu->arch.v_state.vs_offset = data & ~(PAGE_MASK | KVM_MSR_ENABLED); vcpu->arch.v_state.msr_val = data; break; @@ -6053,6 +6066,7 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu) { kvmclock_reset(vcpu); + kvm_vcpu_state_reset(vcpu); free_cpumask_var(vcpu->arch.wbinvd_dirty_mask); fx_free(vcpu); @@ -6109,9 +6123,9 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu) kvm_make_request(KVM_REQ_EVENT, vcpu); vcpu->arch.apf.msr_val = 0; vcpu->arch.st.msr_val = 0; - vcpu->arch.v_state.msr_val = 0; kvmclock_reset(vcpu); + kvm_vcpu_state_reset(vcpu); kvm_clear_async_pf_completion_queue(vcpu); kvm_async_pf_hash_reset(vcpu);