From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] KVM: x86: XSAVE/XRSTOR live migration support Date: Thu, 27 May 2010 13:02:31 +0300 Message-ID: <4BFE4337.4010502@redhat.com> References: <1274953721-5068-1-git-send-email-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 To: Sheng Yang Return-path: Received: from mx1.redhat.com ([209.132.183.28]:44175 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755408Ab0E0KCf (ORCPT ); Thu, 27 May 2010 06:02:35 -0400 In-Reply-To: <1274953721-5068-1-git-send-email-sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On 05/27/2010 12:48 PM, Sheng Yang wrote: > This patch enable save/restore of xsave state. > > Signed-off-by: Sheng Yang > --- > arch/x86/include/asm/kvm.h | 29 ++++++++++++++++ > arch/x86/kvm/x86.c | 79 ++++++++++++++++++++++++++++++++++++++++++++ > include/linux/kvm.h | 6 +++ > Documentation/kvm/api.txt +++++++++++++ > > +/* for KVM_CAP_XSAVE */ > +struct kvm_xsave { > + struct { > + __u16 cwd; > + __u16 swd; > + __u16 twd; > + __u16 fop; > + __u64 rip; > + __u64 rdp; > + __u32 mxcsr; > + __u32 mxcsr_mask; > + __u32 st_space[32]; > + __u32 xmm_space[64]; > + __u32 padding[12]; > + __u32 sw_reserved[12]; > + } i387; > + struct { > + __u64 xstate_bv; > + __u64 reserved1[2]; > + __u64 reserved2[5]; > + } xsave_hdr; > + struct { > + __u32 ymmh_space[64]; > + } ymmh; > + __u64 xcr0; > + __u32 padding[256]; > +}; > Need to reserve way more space here for future xsave growth. I think at least 4K. LRB wa 32x512bit = 1K (though it probably isn't a candidate for vmx). Would be good to get an opinion from your processor architects. I don't think we need to detail the contents of the structures since they're described by the SDM; so we can have just a large array that is 1:1 with the xsave as saved by the fpu. 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. > @@ -2363,6 +2366,59 @@ static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, > return 0; > } > > +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? > + > + guest_xsave->i387.cwd = xsave->i387.cwd; > + guest_xsave->i387.swd = xsave->i387.swd; > + guest_xsave->i387.twd = xsave->i387.twd; > + guest_xsave->i387.fop = xsave->i387.fop; > + guest_xsave->i387.rip = xsave->i387.rip; > + guest_xsave->i387.rdp = xsave->i387.rdp; > + memcpy(guest_xsave->i387.st_space, xsave->i387.st_space, 128); > + memcpy(guest_xsave->i387.xmm_space, xsave->i387.xmm_space, > + sizeof guest_xsave->i387.xmm_space); > + > + guest_xsave->xsave_hdr.xstate_bv = xsave->xsave_hdr.xstate_bv; > + memcpy(guest_xsave->ymmh.ymmh_space, xsave->ymmh.ymmh_space, > + sizeof xsave->ymmh.ymmh_space); > And we can do a big memcpy here. But we need to limit it to what the host actually allocated. > + > + guest_xsave->xcr0 = vcpu->arch.xcr0; > +} > + > > long kvm_arch_vcpu_ioctl(struct file *filp, > unsigned int ioctl, unsigned long arg) > { > @@ -2564,6 +2620,29 @@ long kvm_arch_vcpu_ioctl(struct file *filp, > r = kvm_vcpu_ioctl_x86_set_debugregs(vcpu,&dbgregs); > break; > } > + case KVM_GET_XSAVE: { > + struct kvm_xsave xsave; > Too big for stack (especially if we reserve room for growth). > 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. -- error compiling committee.c: too many arguments to function