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

[Jan Kiszka]

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 <address@hidden>
>>> ---
>>>
>>>  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

signature.asc
Description: OpenPGP digital signature