From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU Date: Thu, 20 Sep 2012 12:19:56 +0300 Message-ID: <505ADFBC.3080109@redhat.com> References: <1347437424-3006-1-git-send-email-xudong.hao@intel.com> <20120913162636.GA10191@amt.cnet> <20120913162929.GB10191@amt.cnet> <50520C67.8020609@redhat.com> <403610A45A2B5242BD291EDAE8B37D300FEC2B2A@SHSMSX102.ccr.corp.intel.com> <20120917133058.GA8919@amt.cnet> <403610A45A2B5242BD291EDAE8B37D300FEC34AA@SHSMSX102.ccr.corp.intel.com> <50599D22.5070308@redhat.com> <403610A45A2B5242BD291EDAE8B37D300FEC46D8@SHSMSX102.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , "kvm@vger.kernel.org" , "Zhang, Xiantao" To: "Hao, Xudong" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:37362 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751441Ab2ITJUA (ORCPT ); Thu, 20 Sep 2012 05:20:00 -0400 In-Reply-To: <403610A45A2B5242BD291EDAE8B37D300FEC46D8@SHSMSX102.ccr.corp.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On 09/20/2012 04:43 AM, Hao, Xudong wrote: >> -----Original Message----- >> From: Avi Kivity [mailto:avi@redhat.com] >> Sent: Wednesday, September 19, 2012 6:24 PM >> To: Hao, Xudong >> Cc: Marcelo Tosatti; kvm@vger.kernel.org; Zhang, Xiantao >> Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU >> > That may be: >> > >> > static bool lazy_fpu_allowed() >> > { >> > return !(vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY)); >> > } >> >> Shouldn't it depend on cr4.osxsave as well? >> > > It do need to check cr4.osxsave due to a separate function. > > static bool lazy_fpu_allowed(struct kvm_vcpu *vcpu) > { > return !kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) || > !(vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY)); > } Yes. > >> > >> >> On guest entry: >> >> if (!lazy_fpu_allowed(vcpu)) >> >> kvm_x86_ops->fpu_activate(vcpu); >> > >> >> But we already have that: >> >> if (vcpu->fpu_active) >> kvm_load_guest_fpu(vcpu); >> >> so why not manage fpu_active to be always set when needed? I don't want >> more checks in the entry path. >> > I means add fpu_active() in kvm_set_xcr(), not in guest entry. Then the fpu_active will be set always when guest initialize xstate. > > @@ -574,6 +574,9 @@ int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr) > kvm_inject_gp(vcpu, 0); > return 1; > } > + if (!lazy_fpu_allowed(vcpu)) > + kvm_x86_ops->fpu_activate(vcpu); > return 0; > And of course on cr4 update. So a function update_lazy_fpu() to be called from both places is needed. -- error compiling committee.c: too many arguments to function