Re: [PATCH 4/4] qemu-kvm: Enable XSAVE live migration support

Jan Kiszka
2010-06-17T08:44:57Z

[-- Attachment #1: Type: text/plain, Size: 3572 bytes --]

Sheng Yang wrote:
> On Thursday 17 June 2010 15:41:43 Jan Kiszka wrote:
>> Sheng Yang wrote:
>>> Based on upstream xsave related fields.
>>>
>>> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
>>> ---
>>>
>>> qemu-kvm-x86.c | 95
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++- qemu-kvm.c
>>> | 24 ++++++++++++++
>>> qemu-kvm.h | 28 ++++++++++++++++
>>> 3 files changed, 146 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
>>> index 3c33e64..dcef8b5 100644
>>> --- a/qemu-kvm-x86.c
>>> +++ b/qemu-kvm-x86.c
>>> @@ -772,10 +772,26 @@ static void get_seg(SegmentCache *lhs, const struct
>>> kvm_segment *rhs)
>>>
>>> | (rhs->avl * DESC_AVL_MASK);
>>>
>>> }
>>>
>>> +#ifdef KVM_CAP_XSAVE
>>> +#define XSAVE_CWD_RIP 2
>>> +#define XSAVE_CWD_RDP 4
>>> +#define XSAVE_MXCSR 6
>>> +#define XSAVE_ST_SPACE 8
>>> +#define XSAVE_XMM_SPACE 40
>>> +#define XSAVE_XSTATE_BV 128
>>> +#define XSAVE_YMMH_SPACE 144
>>> +#endif
>>> +
>>>
>>> void kvm_arch_load_regs(CPUState *env, int level)
>>> {
>>>
>>> struct kvm_regs regs;
>>> struct kvm_fpu fpu;
>>>
>>> +#ifdef KVM_CAP_XSAVE
>>> + struct kvm_xsave* xsave;
>>> +#endif
>>> +#ifdef KVM_CAP_XCRS
>>> + struct kvm_xcrs xcrs;
>>> +#endif
>>>
>>> struct kvm_sregs sregs;
>>> struct kvm_msr_entry msrs[100];
>>> int rc, n, i;
>>>
>>> @@ -806,16 +822,53 @@ void kvm_arch_load_regs(CPUState *env, int level)
>>>
>>> kvm_set_regs(env, &regs);
>>>
>>> +#ifdef KVM_CAP_XSAVE
>>> + if (kvm_check_extension(kvm_state, KVM_CAP_XSAVE)) {
>>> + uint16_t cwd, swd, twd, fop;
>>> +
>>> + xsave = qemu_memalign(4096, sizeof(struct kvm_xsave));
>>> + memset(xsave, 0, sizeof(struct kvm_xsave));
>>> + cwd = swd = twd = fop = 0;
>>> + swd = env->fpus & ~(7 << 11);
>>> + swd |= (env->fpstt & 7) << 11;
>>> + cwd = env->fpuc;
>>> + for (i = 0; i < 8; ++i)
>>> + twd |= (!env->fptags[i]) << i;
>>> + xsave->region[0] = (uint32_t)(swd << 16) + cwd;
>>> + xsave->region[1] = (uint32_t)(fop << 16) + twd;
>>> + memcpy(&xsave->region[XSAVE_ST_SPACE], env->fpregs,
>>> + sizeof env->fpregs);
>>> + memcpy(&xsave->region[XSAVE_XMM_SPACE], env->xmm_regs,
>>> + sizeof env->xmm_regs);
>>> + xsave->region[XSAVE_MXCSR] = env->mxcsr;
>>> + *(uint64_t *)&xsave->region[XSAVE_XSTATE_BV] = env->xstate_bv;
>>> + memcpy(&xsave->region[XSAVE_YMMH_SPACE], env->ymmh_regs,
>>> + sizeof env->ymmh_regs);
>>> + kvm_set_xsave(env, xsave);
>>> +#ifdef KVM_CAP_XCRS
>>> + if (kvm_check_extension(kvm_state, KVM_CAP_XCRS)) {
>>> + xcrs.nr_xcrs = 1;
>>> + xcrs.flags = 0;
>>> + xcrs.xcrs[0].xcr = 0;
>>> + xcrs.xcrs[0].value = env->xcr0;
>>> + kvm_set_xcrs(env, &xcrs);
>>> + }
>>> +#endif /* KVM_CAP_XCRS */
>>> + } else {
>>> +#endif /* KVM_CAP_XSAVE */
>> Why not reusing kvm_put/get_xsave as defined for upstream? There should
>> be enough examples for that pattern. The result will be a tiny qemu-kvm
>> patch.
>
> Still lots of codes in kvm_arch_load/save_regs() duplicate with ones in kvm.c,
> e.g. kvm_get/put_sregs, kvm_get/put_msrs. So would like to wait for merging.

That we still have some legacy here is no good reason to increase it.
Just check how debugregs were introduced.

Jan

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]