From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH v4] KVM: VMX: Enable XSAVE/XRSTORE for guest Date: Tue, 25 May 2010 10:14:39 +0300 Message-ID: <4BFB78DF.3040100@redhat.com> References: <1274695425-14185-1-git-send-email-sheng@linux.intel.com> <4BFA80CC.9070800@redhat.com> <201005251428.04300.sheng@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org, Dexuan Cui To: Sheng Yang Return-path: Received: from mx1.redhat.com ([209.132.183.28]:44846 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751888Ab0EYHOm (ORCPT ); Tue, 25 May 2010 03:14:42 -0400 In-Reply-To: <201005251428.04300.sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On 05/25/2010 09:28 AM, Sheng Yang wrote: >> >>> @@ -3354,6 +3356,29 @@ static int handle_wbinvd(struct kvm_vcpu *vcpu) >>> >>> return 1; >>> >>> } >>> >>> +static int handle_xsetbv(struct kvm_vcpu *vcpu) >>> +{ >>> + u64 new_bv = kvm_read_edx_eax(vcpu); >>> + >>> + if (kvm_register_read(vcpu, VCPU_REGS_RCX) != 0) >>> + goto err; >>> + if (vmx_get_cpl(vcpu) != 0) >>> + goto err; >>> + if (!(new_bv& XSTATE_FP)) >>> + goto err; >>> + if ((new_bv& XSTATE_YMM)&& !(new_bv& XSTATE_SSE)) >>> + goto err; >>> + if (new_bv& ~XCNTXT_MASK) >>> + goto err; >>> >> Ok. This means we must update kvm immediately when XCNTXT_MASK changes. >> >> (Otherwise we would use KVM_XCNTXT_MASK which is always smaller than >> than XCNTXT_MASK). >> > I guess use host_xcr0 here is better? > Yes - it might be smaller than XCNTXT_MASK >> may be simplified if we move xcr0 reload back to guest entry (... :) >> but make it lazy: >> >> save_host_state: nothing >> set cr4.osxsave: nothing >> clear cr4.osxsave: nothing >> guest entry: if (gcr4.osxsave&& !guest_xcr0_loaded) { >> guest_xcr0_loaded = true, load gxcr0 } >> load_host_state: if (guest_xcr0_loaded) { guest_xcr0_loaded = false; >> load host xcr0 } >> fpu switching: if (guest_xcr0_loaded) { guest_xcr0_loaded = false; >> load host xcr0 }, do fpu stuff >> >> So we delay xcr0 reload as late as possible for both entry and exit. >> > I think I got it. But why we need do it at "load_host_state()"? I guess just put > code before fpu testing in kvm_put_guest_fpu() is fine? > Right, load_host_state() is bad because it is vmx specific. kvm_put_guest_fpu() (or perhaps kvm_arch_vcpu_put()) is better. -- error compiling committee.c: too many arguments to function