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
|
Home