From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU
Date: Wed, 19 Sep 2012 13:23:30 +0300
Message-ID: <50599D22.5070308@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>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Cc: Marcelo Tosatti <mtosatti@redhat.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"Zhang, Xiantao" <xiantao.zhang@intel.com>
To: "Hao, Xudong" <xudong.hao@intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:44270 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753078Ab2ISKXd (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 19 Sep 2012 06:23:33 -0400
In-Reply-To: <403610A45A2B5242BD291EDAE8B37D300FEC34AA@SHSMSX102.ccr.corp.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 09/18/2012 04:08 AM, Hao, Xudong wrote:
>> 
>> The objective of the change is to disable lazy fpu loading (that is,
>> host fpu loaded in guest and vice-versa), 

Not vice versa.  We allow the guest fpu loaded in the host, but save it
on heavyweight exit or task switch.

when some bit except the
>> initial tree bits set in guest XCR0 (initial tree being XSTATE_FP|XSTATE_SSE|
>> XSTATE_YMM). Yes?
>> 
> 
> Yes, it's just the object.
> 
>> If i get that right, then the suggestion seems to be:
>> 
>> static bool lazy_fpu_allowed()
>> {
>> 	return (vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY));
>> }
>> 
> 
> 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?

> 
>> 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.

-- 
error compiling committee.c: too many arguments to function