From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH] KVM: x86: XSAVE/XRSTOR live migration support
Date: Thu, 27 May 2010 14:34:19 +0300
Message-ID: <4BFE58BB.6090404@redhat.com>
References: <1274953721-5068-1-git-send-email-sheng@linux.intel.com> <4BFE4337.4010502@redhat.com> <201005271833.11409.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 <mtosatti@redhat.com>, kvm@vger.kernel.org
To: Sheng Yang <sheng@linux.intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:60632 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754767Ab0E0LeW (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 27 May 2010 07:34:22 -0400
In-Reply-To: <201005271833.11409.sheng@linux.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 05/27/2010 01:33 PM, Sheng Yang wrote:
>
>> If we do that then xcr0 needs to be in a separate structure, say
>> kvm_xcr, with a flags field and reserved space of its own for future xcr
>> growth.
>>      
> I meant to put it into sregs, but found it's already full... How about "extended
> sregs"?
>    

Isn't this what xcr means?  xtended control register?

>>> +static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu,
>>> +					struct kvm_xsave *guest_xsave)
>>> +{
>>> +	struct xsave_struct *xsave =&vcpu->arch.guest_fpu.state->xsave;
>>> +
>>> +	if (!cpu_has_xsave)
>>> +		return;
>>>        
>> Hm, it would be nice to make it backward compatible and return the
>> legacy fpu instead.  I think the layouts are compatible?
>>      
> Sound good.  But seems we still need KVM_CAP_XSAVE to use this interface, and
> other processors would still go FPU interface. Seems didn't improve much?
>    

I would like the new interface to be used in all cases, this way we can 
deprecate the old one in a few years.

>>> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
>>> index 23ea022..5006761 100644
>>> --- a/include/linux/kvm.h
>>> +++ b/include/linux/kvm.h
>>> @@ -524,6 +524,9 @@ struct kvm_enable_cap {
>>>
>>>    #define KVM_CAP_PPC_OSI 52
>>>    #define KVM_CAP_PPC_UNSET_IRQ 53
>>>    #define KVM_CAP_ENABLE_CAP 54
>>>
>>> +#ifdef __KVM_HAVE_XSAVE
>>> +#define KVM_CAP_XSAVE 55
>>> +#endif
>>>        
>> Might make sense to have a separate KVM_CAP_XCR, just for consistency.
>>      
> Maybe EXTENDED_SREGS? But still every future field in the struct need a CAP...
>    

Might do

struct kvm_xcr {
     __u32 xcr;
     __u32 reserved;
     __u64 value;
};

struct kvm_xcrs {
     __u32 nr_xcrs;
     __u32 flags;
     struct kvm_xcr xcrs[KVM_MAX_XCRS];
     ... reserved;
};

which would allow new xcrs to be added easily.

You'll need to change kvm_set_xcr0() to kvm_set_xcr() for this to work 
though.

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