Re: [Xen-devel] [Patch 4/4] Refining Xsave/Xrestore support

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

Hi,

Thanks for this - good to see XSAVE save/restore working.  I've no
comments on the tools part of this patch; it looks plausible but I
haven't reviewed it closely.

On the Xen HVM side:

> diff -r 9bf6b4030d70 xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c  Wed Oct 27 21:55:45 2010 +0800
> +++ b/xen/arch/x86/hvm/hvm.c  Wed Oct 27 22:17:24 2010 +0800
> @@ -575,8 +575,13 @@ static int hvm_save_cpu_ctxt(struct doma
>          vc = &v->arch.guest_context;
>  
>          if ( v->fpu_initialised )
> -            memcpy(ctxt.fpu_regs, &vc->fpu_ctxt, sizeof(ctxt.fpu_regs));
> -        else 
> +            if ( cpu_has_xsave )
> +                /* to restore guest img saved on xsave-incapable host */
> +                memcpy(v->arch.xsave_area, ctxt.fpu_regs,
> +                       sizeof(ctxt.fpu_regs));
> +            else
> +                memcpy(&vc->fpu_ctxt, ctxt.fpu_regs, sizeof(ctxt.fpu_regs));

I think this hunk belongs in hvm_LOAD_cpu_ctxt()! 

> +        else
>              memset(ctxt.fpu_regs, 0, sizeof(ctxt.fpu_regs));
>  
>          ctxt.rax = vc->user_regs.eax;

[...]

> +    ctxt = (struct hvm_hw_cpu_xsave *)&h->data[h->cur];
> +    h->cur += desc->length;
> +
> +    _xfeature_mask = ctxt->xfeature_mask;
> +    if ( (_xfeature_mask & xfeature_mask) != xfeature_mask )
> +        return -EINVAL;

This allows XSAVE records to be loaded on machines with fewer features.
Is that safe?

> +    v->arch.xcr0 = ctxt->xcr0;
> +    v->arch.xcr0_accum = ctxt->xcr0_accum;
> +    memcpy(v->arch.xsave_area, &ctxt->save_area, xsave_cntxt_size);
> +
> +    return 0;
> +}

Also, have you tested this on CPUs that don't support XSAVE?  The PV
hypercall looks like it will return -EFAULT after trying to
copy_from_user into a null pointer on the Xen side, but something more
explicit would be better.

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel