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

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

Hi, 

At 03:32 +0100 on 28 Oct (1288236759), Haitao Shan wrote:
> >> 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()!
> I once did the same as you said. But doing this in hvm_load_cpu_ctxt
> will depends on two:
> 1. hvm_load_cpu_ctxt can not be executed before xsave restore routine
> is executed. Otherwise, xsave_area contains no useful data at the time
> of copying.

OK; then you should copy the other way in in the xsave load routine as
well.  Xsave load will always happen after the CPU load since save
records are always written in increasing order of type.

That way, if the save file has no xsave record, the new domain's xsave
state is initalized from the fpu record, and if it does then the fpu
state is initialized from the xsave record.  I think that's the
behaviour you want.

In any case this is *definitely* wrong where it is because the memcpy
arguments are the wrong way round. :)

> 2. It seems to break restore when HVM guest (no touching eXtended
> States at all) saved on a Xsave-capable host is later restored on a
> Xsave-incapable host.

That not a safe thing to do anyway -- once you've told the guest (via
CPUID) that XSAVE is available you can't migrate it to a host where it's
not supported.

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