Re: [Qemu-devel] [PATCH uq/master 2/2] KVM: make XSAVE support more robust

[Gleb Natapov]

On Thu, Sep 05, 2013 at 03:06:22PM +0200, Paolo Bonzini wrote:
> QEMU moves state from CPUArchState to struct kvm_xsave and back when it
> invokes the KVM_*_XSAVE ioctls.  Because it doesn't treat the XSAVE
> region as an opaque blob, it might be impossible to set some state on
> the destination if migrating to an older version.
> 
> This patch blocks migration if it finds that unsupported bits are set
> in the XSTATE_BV header field.  To make this work robustly, QEMU should
> only report in env->xstate_bv those fields that will actually end up
> in the migration stream.
We usually handle host cpu differences in cpuid layer, not by trying to
validate migration data. i.e CPUID.0D should be configurable and
management should be able to query QEMU what is supported and prevent
migration attempt accordingly.

> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  target-i386/kvm.c     | 3 ++-
>  target-i386/machine.c | 4 ++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 749aa09..df08a4b 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1291,7 +1291,8 @@ static int kvm_get_xsave(X86CPU *cpu)
>              sizeof env->fpregs);
>      memcpy(env->xmm_regs, &xsave->region[XSAVE_XMM_SPACE],
>              sizeof env->xmm_regs);
> -    env->xstate_bv = *(uint64_t *)&xsave->region[XSAVE_XSTATE_BV];
> +    env->xstate_bv = *(uint64_t *)&xsave->region[XSAVE_XSTATE_BV] &
> +            XSTATE_SUPPORTED;
Don't we just drop state here that will not be restored on the
destination and destination will not be able to tell since we masked
unsupported bits?

>      memcpy(env->ymmh_regs, &xsave->region[XSAVE_YMMH_SPACE],
>              sizeof env->ymmh_regs);
>      return 0;
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index dc81cde..9e2cfcf 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -278,6 +278,10 @@ static int cpu_post_load(void *opaque, int version_id)
>      CPUX86State *env = &cpu->env;
>      int i;
>  
> +    if (env->xstate_bv & ~XSTATE_SUPPORTED) {
> +        return -EINVAL;
> +    }
> + 
>      /*
>       * Real mode guest segments register DPL should be zero.
>       * Older KVM version were setting it wrongly.
> -- 
> 1.8.3.1

--
                        Gleb.