KVM: x86: XSAVE/XRSTOR live migration support
PATCH 1/1 KVM: x86: XSAVE/XRSTOR live migration support
This patch enable save/restore of xsave state.
--- arch/x86/include/asm/kvm.h | 29 ++++++++++++++++ arch/x86/kvm/x86.c | 79 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/kvm.h | 6 +++ 3 files changed, 114 insertions(+), 0 deletions(-)
arch/x86/include/asm/kvm.h +29 -0
@@ -22,6 +22,7 @@#define __KVM_HAVE_XEN_HVM#define __KVM_HAVE_VCPU_EVENTS#define __KVM_HAVE_DEBUGREGS +#define __KVM_HAVE_XSAVE/* Architectural interrupt line count. */ #define KVM_NR_INTERRUPTS 256
@@ -299,4 +300,32 @@ struct kvm_debugregs { __u64 reserved[9]; }; +/* 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 ]; +}; +#endif /* _ASM_X86_KVM_H */
arch/x86/kvm/x86.c +79 -0
97 lines not on this page · raw message
include/linux/kvm.h +6 -0
@@ -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 #ifdef KVM_CAP_IRQ_ROUTING
@@ -714,6 +717,9 @@ struct kvm_clock_data {#define KVM_GET_DEBUGREGS _IOR(KVMIO, 0xa1, struct kvm_debugregs)#define KVM_SET_DEBUGREGS _IOW(KVMIO, 0xa2, struct kvm_debugregs)#define KVM_ENABLE_CAP _IOW(KVMIO, 0xa3, struct kvm_enable_cap) +/* Available with KVM_CAP_XSAVE */ +#define KVM_GET_XSAVE _IOR(KVMIO, 0xa4, struct kvm_xsave) +#define KVM_SET_XSAVE _IOW(KVMIO, 0xa5, struct kvm_xsave)#define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
Signature · 1 line
1.7.0.1
Quoted 8 lines · Sheng Yang
This patch enable save/restore of xsave state.
Signed-off-by Sheng Yang<[email protected]>--- arch/x86/include/asm/kvm.h | 29 ++++++++++++++++ arch/x86/kvm/x86.c | 79 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/kvm.h | 6 +++
Documentation/kvm/api.txt +++++++++++++
Quoted 19 lines
arch/x86/include/asm/kvm.h +17
-0+/* 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 {
arch/x86/include/asm/kvm.h struct kvm_debugregs { · line 329
Quoted 10 lines
arch/x86/include/asm/kvm.h +10
-0+__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.
arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, · line 2376
Quoted 12 lines
arch/x86/kvm/x86.c +7
-0@@ -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?
Quoted 5 lines
arch/x86/kvm/x86.c +5
-0+ + 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;
arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu,
Quoted 10 lines
arch/x86/kvm/x86.c +9
-0+ 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.
Quoted 9 lines
arch/x86/kvm/x86.c +4
-0+ + guest_xsave->xcr0 = vcpu->arch.xcr0; +} +long kvm_arch_vcpu_ioctl (struct file *filp,unsigned int ioctl,unsigned long arg) {
arch/x86/kvm/x86.c long kvm_arch_vcpu_ioctl(struct file *filp, · line 2625
Quoted 6 lines
arch/x86/kvm/x86.c +2
-0@@ -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).
include/linux/kvm.h struct kvm_enable_cap { · line 530
Quoted 12 lines
include/linux/kvm.h +3
-0@@ -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.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 6 lines · On Thursday 27 May 2010 18:02:31 Avi Kivity wrote:
Quoted 5 lines · Sheng Yang
This patch enable save/restore of xsave state.
Signed-off-by Sheng Yang<[email protected]>---
Quoted 6 lines
Quoted 4 lines
arch/x86/include/asm/kvm.h | 29 ++++++++++++++++ arch/x86/kvm/x86.c | 79++++++++++++++++++++++++++++++++++++++++++++
include/linux/kvm.h
| 6 +++
Documentation/kvm/api.txt+++++++++++++
Yes...
Quoted 24 lines
Quoted 24 lines
arch/x86/include/asm/kvm.h +23
-0+/* 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 ];
Quoted 8 lines
arch/x86/include/asm/kvm.h struct kvm_debugregs { · line 329
Quoted 4 lines
arch/x86/include/asm/kvm.h +4
-0+ } 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.
Would check it.
Quoted 4 lines
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.
Um, I've tried that, but failed mysteriously... Would check what's wrong.
Quoted 4 lines
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"?
Quoted 8 lines
Quoted 8 lines
(quoted diff) +0
-0@@ -2363,6 +2366,59 @@ static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, return 0; }
Quoted 10 lines
arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, · line 2375
Quoted 7 lines
arch/x86/kvm/x86.c +7
-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?
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?
Quoted 10 lines
Quoted 10 lines
arch/x86/kvm/x86.c +9
-0+ + 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,
Quoted 8 lines
arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, · line 2389
Quoted 5 lines
arch/x86/kvm/x86.c +5
-0+ 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.
Would try.
Quoted 17 lines
Quoted 17 lines
arch/x86/kvm/x86.c +4
-0+ + 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 ;
Quoted 6 lines
arch/x86/kvm/x86.c long kvm_arch_vcpu_ioctl(struct file *filp, · line 2624
Quoted 4 lines
arch/x86/kvm/x86.c +2
-0} +case KVM_GET_XSAVE : { +struct kvm_xsave xsave;Too big for stack (especially if we reserve room for growth).
Oops...
Quoted 11 lines
Quoted 11 lines
include/linux/kvm.h +0
-0@@ -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
Quoted 5 lines
include/linux/kvm.h struct kvm_enable_cap { · line 529
Quoted 3 lines
include/linux/kvm.h +3
-0+#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...
--
regards
Yang, Sheng
Quoted 8 lines · Sheng Yang
Quoted 5 lines
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?
Quoted 11 lines
Quoted 11 lines
arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, · line 2375
Quoted 8 lines
arch/x86/kvm/x86.c +7
-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?
Quoted 3 lines
Sound good. But seems we still need
KVM_CAP_XSAVEto 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.
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
include/linux/kvm.h +0
-0@@ -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
Quoted 8 lines
Quoted 6 lines
include/linux/kvm.h struct kvm_enable_cap { · line 530
Quoted 4 lines
include/linux/kvm.h +3
-0+#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.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 45 lines · On Thursday 27 May 2010 18:02:31 Avi Kivity wrote:
Quoted 9 lines · Sheng Yang
This patch enable save/restore of xsave state.
Signed-off-by Sheng Yang<[email protected]>---
arch/x86/include/asm/kvm.h | 29 ++++++++++++++++ arch/x86/kvm/x86.c | 79++++++++++++++++++++++++++++++++++++++++++++
include/linux/kvm.h
| 6 +++
Documentation/kvm/api.txt+++++++++++++Quoted 17 lines
arch/x86/include/asm/kvm.h +17
-0+/* 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 {arch/x86/include/asm/kvm.h struct kvm_debugregs { · line 329
Quoted 10 lines
arch/x86/include/asm/kvm.h +10
-0+__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.
Quoted 3 lines
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.
I think we can reserve one page here. But one big array make it harder to work with QEmu CPUState. Do we need lots of marcos in QEmu to parse the array? Also it's easier to transfer get/set_fpu to get/set_xsave interface using current structure I think.
--
regards
Yang, Sheng
Quoted 79 lines
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.Quoted 4 lines
arch/x86/kvm/x86.c +0
-0@@ -2363,6 +2366,59 @@ static int kvm_vcpu_ioctl_x86_set_debugregs(structkvm_vcpu *vcpu, return 0;arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_x86_set_debugregs(struct · line 2378
Quoted 10 lines
arch/x86/kvm/x86.c +7
-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?
Quoted 4 lines
arch/x86/kvm/x86.c +4
-0+ + guest_xsave->i387.cwd = xsave->i387.cwd; + guest_xsave->i387.swd = xsave->i387.swd; + guest_xsave->i387.twd = xsave->i387.twd;arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, · line 2389
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+ 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.
Quoted 12 lines
arch/x86/kvm/x86.c +4
-0+ + guest_xsave->xcr0 = vcpu->arch.xcr0; +} +long kvm_arch_vcpu_ioctl (struct file *filp,unsigned int ioctl,unsigned long arg) {arch/x86/kvm/x86.c long kvm_arch_vcpu_ioctl(struct file *filp, · line 2627
Quoted 8 lines
arch/x86/kvm/x86.c +2
-0@@ -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).
include/linux/kvm.h struct kvm_enable_cap { · line 531
Quoted 13 lines
include/linux/kvm.h +3
-0@@ -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.
Quoted 5 lines · Sheng Yang
Quoted 5 lines
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.
Quoted 9 lines
Quoted 4 lines
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.
I think we can reserve one page here. But one big array make it harder to work with QEmu CPUState. Do we need lots of marcos in QEmu to parse the array? Also it's easier to transfer get/set_fpu to get/set_xsave interface using current structure I think.
We'll need that code somewhere, so we aren't losing anything by putting it in userspace (in fact, qemu already has to have most of this code since it supports fxsave/fxrstor emulation).
What we gain is that if we make a bug, it is easier to fix it in userspace than in the kernel.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.