xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 7116 bytes --]

I was testing xl/xm checkpoint with the latest c/s in the repo, 23300.
neither xl nor xm seem to work. The error code is 95 (EOPNOTSUPP).

Migration works but not checkpointing. While doing a xc_domain_resume,
the "modify_returncode" phase (for suspend_cancel) fails. Tracing through
the control flow, I found that the hypercall for set_vcpucontext
(in do_xen_hypercall() from xc_private.c) fails with this error code.

I have tested this with a 64-bit 2.6.39 and 32-bit 2.6.18 pv domU.
Any help would be great.

shriram

On Fri, May 6, 2011 at 5:01 PM, AP Xen <apxeng@gmail.com> wrote:

> On Tue, May 3, 2011 at 4:07 AM, George Dunlap
> <George.Dunlap@eu.citrix.com> wrote:
> > Have you tried it with other operating systems and found it to work?
> > I.e., is it something specific to Windows 7, or is it a general HVM /
> > Windows problem?
>
> I tried this with CentOS 5.6 and saw the same behavior.
>
> root@ubuntu:~# xl -vvv save -c centos /etc/xen/centoschk
> Saving to /etc/xen/centoschk new xl format (info 0x0/0x0/255)
> libxl: debug: libxl_dom.c:384:libxl__domain_suspend_common_callback
> issuing PVHVM suspend request via XenBus control node
> libxl: debug: libxl_dom.c:389:libxl__domain_suspend_common_callback
> wait for the guest to acknowledge suspend request
> libxl: debug: libxl_dom.c:434:libxl__domain_suspend_common_callback
> guest acknowledged suspend request
> libxl: debug: libxl_dom.c:438:libxl__domain_suspend_common_callback
> wait for the guest to suspend
> libxl: debug: libxl_dom.c:450:libxl__domain_suspend_common_callback
> guest has suspended
> xc: debug: outbuf_write: 4194304 > 90092@16687124
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: debug: outbuf_write: 4194304 > 4169716@12607500
> xc: detail: type fail: page 0 mfn 000f2000
> xc: detail: type fail: page 1 mfn 000f2001
> xc: detail: type fail: page 2 mfn 000f2002
> xc: detail: delta 9371ms, dom0 25%, target 0%, sent 920Mb/s, dirtied
> 0Mb/s 0 pages
> xc: detail: Total pages sent= 263168 (0.25x)
> xc: detail: (of which 0 were fixups)
> xc: detail: All memory is saved
> xc: detail: Save exit rc=0
> libxl: debug: libxl_dom.c:534:libxl__domain_save_device_model Saving
> device model state to /var/lib/xen/qemu-save.7
> libxl: debug: libxl_dom.c:546:libxl__domain_save_device_model Qemu
> state is 7204 bytes
>
>
> root@ubuntu:~# xl list
> Name                                        ID   Mem VCPUs      State
> Time(s)
> Domain-0                                    0  2914     4     r-----
>  6576.4
> centos                                        7  1019     2     ---ss-
> 575.4
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

[-- Attachment #1.2: Type: text/html, Size: 8408 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Ian Campbell]

On Tue, 2011-05-10 at 00:06 +0100, Shriram Rajagopalan wrote:
> I was testing xl/xm checkpoint with the latest c/s in the repo, 23300.
> neither xl nor xm seem to work. The error code is 95 (EOPNOTSUPP).
> 
> Migration works but not checkpointing. While doing a
> xc_domain_resume, 
> the "modify_returncode" phase (for suspend_cancel) fails. Tracing
> through 
> the control flow, I found that the hypercall for set_vcpucontext 
> (in do_xen_hypercall() from xc_private.c) fails with this error code. 
> 
> I have tested this with a 64-bit 2.6.39 and 32-bit 2.6.18 pv domU. 
> Any help would be great.

Are we still talking about HVM guests?

The most plausible looking EOPNOTSUPP from that code is in
xen/arch/x86/domain.c:arch_set_info_guest() but that is on a PV only
path.

There are only a small number of uses of EOPNOTSUPP in the hypervisor
and the rest are all in xen/arch/x86/hvm/hvm.c or
xen/arch/x86/hvm/nestedhvm.c and all are in nestedhvm related functions.
I guess you aren't using nested HVM though?!

Ian.

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 2422 bytes --]

On Tue, May 10, 2011 at 3:41 AM, Ian Campbell <Ian.Campbell@citrix.com>wrote:

> On Tue, 2011-05-10 at 00:06 +0100, Shriram Rajagopalan wrote:
> > I was testing xl/xm checkpoint with the latest c/s in the repo, 23300.
> > neither xl nor xm seem to work. The error code is 95 (EOPNOTSUPP).
> >
> > Migration works but not checkpointing. While doing a
> > xc_domain_resume,
> > the "modify_returncode" phase (for suspend_cancel) fails. Tracing
> > through
> > the control flow, I found that the hypercall for set_vcpucontext
> > (in do_xen_hypercall() from xc_private.c) fails with this error code.
> >
> > I have tested this with a 64-bit 2.6.39 and 32-bit 2.6.18 pv domU.
> > Any help would be great.
>
> Are we still talking about HVM guests?
>
> No! its all PV. There is a 2.6.39-rc1 debian guest and a 2.6.18 standard
xenlinux kernel based debian guest.

> The most plausible looking EOPNOTSUPP from that code is in
> xen/arch/x86/domain.c:arch_set_info_guest() but that is on a PV only
> path.
>
> And that rings with the pv guests I am using. It makes perfect sense,
looking
at that function and especially at the code that returns EOPNOTSUPP (the
only
place in the entire file).
   else
    {
        bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);

#ifdef CONFIG_X86_64
        fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
#endif

        for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
            fail |= v->arch.pv_vcpu.gdt_frames[i] != c(gdt_frames[i]);
        fail |= v->arch.pv_vcpu.gdt_ents != c(gdt_ents);

        fail |= v->arch.pv_vcpu.ldt_base != c(ldt_base);
        fail |= v->arch.pv_vcpu.ldt_ents != c(ldt_ents);

        if ( fail )
           return -EOPNOTSUPP;
    }

This change was introduced by c/s
changeset:   23142:f5e8d152a565
user:        Jan Beulich <jbeulich@novell.com>
date:        Tue Apr 05 13:01:25 2011 +0100
x86: split struct vcpu

I think I am missing something really obvious in this piece of code. The
xc_domain_resume code tries to modify the return value of shutdown hypercall
(i.e eax register is set to 1) and this code doesnt seem to check those
registers.


 There are only a small number of uses of EOPNOTSUPP in the hypervisor
> and the rest are all in xen/arch/x86/hvm/hvm.c or
> xen/arch/x86/hvm/nestedhvm.c and all are in nestedhvm related functions.
> I guess you aren't using nested HVM though?!
>
> Nope

> Ian.
>
>
> shriram

[-- Attachment #1.2: Type: text/html, Size: 3537 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Jan Beulich]

>>> On 10.05.11 at 16:52, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
> On Tue, May 10, 2011 at 3:41 AM, Ian Campbell <Ian.Campbell@citrix.com>wrote:
>> The most plausible looking EOPNOTSUPP from that code is in
>> xen/arch/x86/domain.c:arch_set_info_guest() but that is on a PV only
>> path.
>>
>> And that rings with the pv guests I am using. It makes perfect sense,
> looking
> at that function and especially at the code that returns EOPNOTSUPP (the
> only
> place in the entire file).
>    else
>     {
>         bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
> 
> #ifdef CONFIG_X86_64
>         fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
> #endif
> 
>         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
>             fail |= v->arch.pv_vcpu.gdt_frames[i] != c(gdt_frames[i]);
>         fail |= v->arch.pv_vcpu.gdt_ents != c(gdt_ents);
> 
>         fail |= v->arch.pv_vcpu.ldt_base != c(ldt_base);
>         fail |= v->arch.pv_vcpu.ldt_ents != c(ldt_ents);
> 
>         if ( fail )
>            return -EOPNOTSUPP;
>     }
> 
> This change was introduced by c/s
> changeset:   23142:f5e8d152a565
> user:        Jan Beulich <jbeulich@novell.com>
> date:        Tue Apr 05 13:01:25 2011 +0100
> x86: split struct vcpu
> 
> I think I am missing something really obvious in this piece of code. The
> xc_domain_resume code tries to modify the return value of shutdown hypercall
> (i.e eax register is set to 1) and this code doesnt seem to check those
> registers.

Correct - the code here checks only for values where the logic
needed to support changing the on an already initialized vCPU isn't
implemented. Previously, actual vCPU state and what was tracked
in struct vcpu could get out of sync in this case, potentially
confusing things further down (including possible security issues).

You'll want to figure out which part(s) actually differ, and why.
Only then we'll be able to tell whether mentioned c/s introduced
false positives.

Jan

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 2867 bytes --]

On Tue, May 10, 2011 at 10:02 AM, Jan Beulich <JBeulich@novell.com> wrote:

> >>> On 10.05.11 at 16:52, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
> > On Tue, May 10, 2011 at 3:41 AM, Ian Campbell <Ian.Campbell@citrix.com
> >wrote:
> >> The most plausible looking EOPNOTSUPP from that code is in
> >> xen/arch/x86/domain.c:arch_set_info_guest() but that is on a PV only
> >> path.
> >>
> >> And that rings with the pv guests I am using. It makes perfect sense,
> > looking
> > at that function and especially at the code that returns EOPNOTSUPP (the
> > only
> > place in the entire file).
> >    else
> >     {
> >         bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
> >
> > #ifdef CONFIG_X86_64
> >         fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
> > #endif
> >
> >         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
> >             fail |= v->arch.pv_vcpu.gdt_frames[i] != c(gdt_frames[i]);
> >         fail |= v->arch.pv_vcpu.gdt_ents != c(gdt_ents);
> >
> >         fail |= v->arch.pv_vcpu.ldt_base != c(ldt_base);
> >         fail |= v->arch.pv_vcpu.ldt_ents != c(ldt_ents);
> >
> >         if ( fail )
> >            return -EOPNOTSUPP;
> >     }
> >
> > This change was introduced by c/s
> > changeset:   23142:f5e8d152a565
> > user:        Jan Beulich <jbeulich@novell.com>
> > date:        Tue Apr 05 13:01:25 2011 +0100
> > x86: split struct vcpu
> >
> > I think I am missing something really obvious in this piece of code. The
> > xc_domain_resume code tries to modify the return value of shutdown
> hypercall
> > (i.e eax register is set to 1) and this code doesnt seem to check those
> > registers.
>
> Correct - the code here checks only for values where the logic
> needed to support changing the on an already initialized vCPU isn't
> implemented. Previously, actual vCPU state and what was tracked
> in struct vcpu could get out of sync in this case, potentially
> confusing things further down (including possible security issues).
>
> You'll want to figure out which part(s) actually differ, and why.
> Only then we'll be able to tell whether mentioned c/s introduced
> false positives.
>
> Bit confused. If I understand correctly, this piece of code checks new
values
of certain registers against old ones, for an already initialized VCPU. And
AFAIT,
it is checking the gdts, ldts & control registers. The xc_domain_resume code
just
changes one general purpose register eax. basically,

 get_vcpucontext()
 set_field(eax, 1) //to indicate SUSPEND_CANCEL
 set_vcpucontext()

I dont understand what you mean by "which parts actually differ & why".


And just a trivial question:
 is the hypervisor binary always compiled to a 32-bit elf? somehow, the
symbols file xen-syms-* is getting compiled to 64 bit ELF binary while
the xen binary is getting compiled to 32-bit binary.

shriram

> Jan
>
>

[-- Attachment #1.2: Type: text/html, Size: 3935 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Ian Campbell]

On Tue, 2011-05-10 at 16:50 +0100, Shriram Rajagopalan wrote:
> On Tue, May 10, 2011 at 10:02 AM, Jan Beulich <JBeulich@novell.com>
> wrote:
>         >>> On 10.05.11 at 16:52, Shriram Rajagopalan
>         <rshriram@cs.ubc.ca> wrote:
>         > On Tue, May 10, 2011 at 3:41 AM, Ian Campbell
>         <Ian.Campbell@citrix.com>wrote:
>         
>         
>         >> The most plausible looking EOPNOTSUPP from that code is in
>         >> xen/arch/x86/domain.c:arch_set_info_guest() but that is on
>         a PV only
>         >> path.
>         >>
>         >> And that rings with the pv guests I am using. It makes
>         perfect sense,
>         > looking
>         > at that function and especially at the code that returns
>         EOPNOTSUPP (the
>         > only
>         > place in the entire file).
>         >    else
>         >     {
>         >         bool_t fail = v->arch.pv_vcpu.ctrlreg[3] !=
>         c(ctrlreg[3]);
>         >
>         > #ifdef CONFIG_X86_64
>         >         fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
>         > #endif
>         >
>         >         for ( i = 0; i <
>         ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
>         >             fail |= v->arch.pv_vcpu.gdt_frames[i] !=
>         c(gdt_frames[i]);
>         >         fail |= v->arch.pv_vcpu.gdt_ents != c(gdt_ents);
>         >
>         >         fail |= v->arch.pv_vcpu.ldt_base != c(ldt_base);
>         >         fail |= v->arch.pv_vcpu.ldt_ents != c(ldt_ents);
>         >
>         >         if ( fail )
>         >            return -EOPNOTSUPP;
>         >     }
>         >
>         > This change was introduced by c/s
>         > changeset:   23142:f5e8d152a565
>         > user:        Jan Beulich <jbeulich@novell.com>
>         > date:        Tue Apr 05 13:01:25 2011 +0100
>         > x86: split struct vcpu
>         >
>         > I think I am missing something really obvious in this piece
>         of code. The
>         > xc_domain_resume code tries to modify the return value of
>         shutdown hypercall
>         > (i.e eax register is set to 1) and this code doesnt seem to
>         check those
>         > registers.
>         
>         
>         Correct - the code here checks only for values where the logic
>         needed to support changing the on an already initialized vCPU
>         isn't
>         implemented. Previously, actual vCPU state and what was
>         tracked
>         in struct vcpu could get out of sync in this case, potentially
>         confusing things further down (including possible security
>         issues).
>         
>         You'll want to figure out which part(s) actually differ, and
>         why.
>         Only then we'll be able to tell whether mentioned c/s
>         introduced
>         false positives.
>         
> Bit confused. If I understand correctly, this piece of code checks new
> values 
> of certain registers against old ones, for an already initialized
> VCPU. And AFAIT,
> it is checking the gdts, ldts & control registers. The
> xc_domain_resume code just
> changes one general purpose register eax. basically,
> 
>  get_vcpucontext()
>  set_field(eax, 1) //to indicate SUSPEND_CANCEL
>  set_vcpucontext()
> 
> I dont understand what you mean by "which parts actually differ &
> why".

Either get_vcpucontext is returning something "odd" or set_vcpucontext
is being overzealous in what it checks (the "false positives" Jan refers
too)

> And just a trivial question:
> is the hypervisor binary always compiled to a 32-bit elf? somehow, the
> symbols file xen-syms-* is getting compiled to 64 bit ELF binary while
> the xen binary is getting compiled to 32-bit binary. 

The hypervisor binary (32 or 64 bit) is always wrapped up as a 32 bit
ELF, see the use of xen/arch/x86/boot/mkelf32.c. Something to do with
keeping a bootloader happy, I think...

Ian.

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Jan Beulich]

>>> On 10.05.11 at 17:50, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
> On Tue, May 10, 2011 at 10:02 AM, Jan Beulich <JBeulich@novell.com> wrote:
> 
>> >>> On 10.05.11 at 16:52, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
>> > On Tue, May 10, 2011 at 3:41 AM, Ian Campbell <Ian.Campbell@citrix.com 
>> >wrote:
>> >> The most plausible looking EOPNOTSUPP from that code is in
>> >> xen/arch/x86/domain.c:arch_set_info_guest() but that is on a PV only
>> >> path.
>> >>
>> >> And that rings with the pv guests I am using. It makes perfect sense,
>> > looking
>> > at that function and especially at the code that returns EOPNOTSUPP (the
>> > only
>> > place in the entire file).
>> >    else
>> >     {
>> >         bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
>> >
>> > #ifdef CONFIG_X86_64
>> >         fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
>> > #endif
>> >
>> >         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
>> >             fail |= v->arch.pv_vcpu.gdt_frames[i] != c(gdt_frames[i]);
>> >         fail |= v->arch.pv_vcpu.gdt_ents != c(gdt_ents);
>> >
>> >         fail |= v->arch.pv_vcpu.ldt_base != c(ldt_base);
>> >         fail |= v->arch.pv_vcpu.ldt_ents != c(ldt_ents);
>> >
>> >         if ( fail )
>> >            return -EOPNOTSUPP;
>> >     }
>> >
>> > This change was introduced by c/s
>> > changeset:   23142:f5e8d152a565
>> > user:        Jan Beulich <jbeulich@novell.com>
>> > date:        Tue Apr 05 13:01:25 2011 +0100
>> > x86: split struct vcpu
>> >
>> > I think I am missing something really obvious in this piece of code. The
>> > xc_domain_resume code tries to modify the return value of shutdown
>> hypercall
>> > (i.e eax register is set to 1) and this code doesnt seem to check those
>> > registers.
>>
>> Correct - the code here checks only for values where the logic
>> needed to support changing the on an already initialized vCPU isn't
>> implemented. Previously, actual vCPU state and what was tracked
>> in struct vcpu could get out of sync in this case, potentially
>> confusing things further down (including possible security issues).
>>
>> You'll want to figure out which part(s) actually differ, and why.
>> Only then we'll be able to tell whether mentioned c/s introduced
>> false positives.
>>
>> Bit confused. If I understand correctly, this piece of code checks new
> values
> of certain registers against old ones, for an already initialized VCPU. And
> AFAIT,
> it is checking the gdts, ldts & control registers. The xc_domain_resume code
> just
> changes one general purpose register eax. basically,
> 
>  get_vcpucontext()
>  set_field(eax, 1) //to indicate SUSPEND_CANCEL
>  set_vcpucontext()
> 
> I dont understand what you mean by "which parts actually differ & why".

Quite obviously there are differences in one or more of the now
checked fields, and we need to find out where they are (and
why). This is regardless of the tools apparently only modifying
eax.

> And just a trivial question:
>  is the hypervisor binary always compiled to a 32-bit elf? somehow, the
> symbols file xen-syms-* is getting compiled to 64 bit ELF binary while
> the xen binary is getting compiled to 32-bit binary.

Yes, that's because it wants to boot from 32-bit GrUB (and the
multiboot protocol also is 32-bit only afaik).

Jan

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 8530 bytes --]

On Tue, May 10, 2011 at 11:03 AM, Jan Beulich <JBeulich@novell.com> wrote:

> >>> On 10.05.11 at 17:50, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
> > On Tue, May 10, 2011 at 10:02 AM, Jan Beulich <JBeulich@novell.com>
> wrote:
> >
> >> >>> On 10.05.11 at 16:52, Shriram Rajagopalan <rshriram@cs.ubc.ca>
> wrote:
> >> > On Tue, May 10, 2011 at 3:41 AM, Ian Campbell <
> Ian.Campbell@citrix.com
> >> >wrote:
> >> >> The most plausible looking EOPNOTSUPP from that code is in
> >> >> xen/arch/x86/domain.c:arch_set_info_guest() but that is on a PV only
> >> >> path.
> >> >>
> >> >> And that rings with the pv guests I am using. It makes perfect sense,
> >> > looking
> >> > at that function and especially at the code that returns EOPNOTSUPP
> (the
> >> > only
> >> > place in the entire file).
> >> >    else
> >> >     {
> >> >         bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
> >> >
> >> > #ifdef CONFIG_X86_64
> >> >         fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
> >> > #endif
> >> >
> >> >         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
> >> >             fail |= v->arch.pv_vcpu.gdt_frames[i] != c(gdt_frames[i]);
> >> >         fail |= v->arch.pv_vcpu.gdt_ents != c(gdt_ents);
> >> >
> >> >         fail |= v->arch.pv_vcpu.ldt_base != c(ldt_base);
> >> >         fail |= v->arch.pv_vcpu.ldt_ents != c(ldt_ents);
> >> >
> >> >         if ( fail )
> >> >            return -EOPNOTSUPP;
> >> >     }
> >> >
> >> > This change was introduced by c/s
> >> > changeset:   23142:f5e8d152a565
> >> > user:        Jan Beulich <jbeulich@novell.com>
> >> > date:        Tue Apr 05 13:01:25 2011 +0100
> >> > x86: split struct vcpu
> >> >
> >> > I think I am missing something really obvious in this piece of code.
> The
> >> > xc_domain_resume code tries to modify the return value of shutdown
> >> hypercall
> >> > (i.e eax register is set to 1) and this code doesnt seem to check
> those
> >> > registers.
> >>
> >> Correct - the code here checks only for values where the logic
> >> needed to support changing the on an already initialized vCPU isn't
> >> implemented. Previously, actual vCPU state and what was tracked
> >> in struct vcpu could get out of sync in this case, potentially
> >> confusing things further down (including possible security issues).
> >>
> >> You'll want to figure out which part(s) actually differ, and why.
> >> Only then we'll be able to tell whether mentioned c/s introduced
> >> false positives.
> >>
> >> Bit confused. If I understand correctly, this piece of code checks new
> > values
> > of certain registers against old ones, for an already initialized VCPU.
> And
> > AFAIT,
> > it is checking the gdts, ldts & control registers. The xc_domain_resume
> code
> > just
> > changes one general purpose register eax. basically,
> >
> >  get_vcpucontext()
> >  set_field(eax, 1) //to indicate SUSPEND_CANCEL
> >  set_vcpucontext()
> >
> > I dont understand what you mean by "which parts actually differ & why".
>
> Quite obviously there are differences in one or more of the now
> checked fields, and we need to find out where they are (and
> why). This is regardless of the tools apparently only modifying
> eax.
>
> > And just a trivial question:
> >  is the hypervisor binary always compiled to a 32-bit elf? somehow, the
> > symbols file xen-syms-* is getting compiled to 64 bit ELF binary while
> > the xen binary is getting compiled to 32-bit binary.
>
> Yes, that's because it wants to boot from 32-bit GrUB (and the
> multiboot protocol also is 32-bit only afaik).
>
> Jan
>
> I tried out a simple program that just gets and sets the VCPU 0's context
(no change
whatsoever to anything). There is no intermediate code involved (except for
the hypercall
bounce buffer stuff). If all is well, then this should work. But it doesnt!!
even for a PV guest.
 I get the same Operation Not supported error when I try to "set" the vcpu
context with the
same struct obtained via the get_vcpucontext hypercall!

Setup: 32bit 2.6.18 32-bit domU, 2.6.32.39 64-bit dom0, 64-bit xen-unstable
c/s 23300,

I suspend the domain via usual xenstore suspend.
The code for get/set vcpu context was taken verbatim from
tools/libxc/xc_resume.c:modify_returncode
 (just commented out the SET_FIELD line, that changes eax)

static int pv_guest_width(xc_interface *xch, uint32_t domid)
{
    DECLARE_DOMCTL;
    domctl.domain = domid;
    domctl.cmd = XEN_DOMCTL_get_address_size;
    if ( xc_domctl(xch, &domctl) != 0 )
    {
        perror("Could not get guest address size");
        return -1;
    }
    return domctl.u.address_size.size / 8;
}

static int get_set_vcpu_ctx(xc_interface *xch, unsigned int domid)
{
    vcpu_guest_context_any_t ctxt;
    xc_dominfo_t info;
    xen_capabilities_info_t caps;
    struct domain_info_context _dinfo = {};
    struct domain_info_context *dinfo = &_dinfo;
    int rc;

    if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 )
    {
        perror("Could not get domain info");
        return -1;
    }

    /* Probe PV guest address width. */
    dinfo->guest_width = pv_guest_width(xch, domid);
    if ( dinfo->guest_width < 0 )
          return -1;

    if ( (rc = xc_vcpu_getcontext(xch, domid, 0, &ctxt)) != 0 ) {
        perror("getcontext failed");
        return rc;
    }
    //    SET_FIELD(&ctxt, user_regs.eax, 1);

    if ( (rc = xc_vcpu_setcontext(xch, domid, 0, &ctxt)) != 0 ) {
        perror("setcontext failed");
        return rc;
    }

    fprintf(stderr, "get/set vcpu context succeeded\n");
    return 0;
}


and I get - setcontext: operation not supported!

now for the weirdness:
 Since the the setcontext failed I thought I should be able
to run the above sample code again and again with no side effect
(please correct my assumption if I am wrong).

But when I run the above code for the second time, I get a XEN panic!

(XEN) Xen BUG at domctl.c:1724
(XEN) ----[ Xen-4.2-unstable  x86_64  debug=y  Not tainted ]----
(XEN) CPU:    2
(XEN) RIP:    e008:[<ffff82c48014dd57>] arch_get_info_guest+0x5f7/0x7b0
(XEN) RFLAGS: 0000000000010202   CONTEXT: hypervisor
(XEN) rax: 0000000000000001   rbx: ffff8300228c4000   rcx: ffff8300228c4040
(XEN) rdx: 0000000000000000   rsi: 0000000000000000   rdi: ffff830450652210
(XEN) rbp: ffff83082a357da8   rsp: ffff83082a357d68   r8:  0000000000000002
(XEN) r9:  0000000000000002   r10: 0000000000000040   r11: 0000000000000000
(XEN) r12: ffff830450652010   r13: 0000000000000001   r14: ffff830829db9000
(XEN) r15: ffff830450652010   cr0: 0000000080050033   cr4: 00000000000026f0
(XEN) cr3: 000000047beef000   cr2: 0000000000d44048
(XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
(XEN) Xen stack trace from rsp=ffff83082a357d68:
(XEN)    ffff830829db9000 ffff8300228c4000 ffff83082a357d98 fffffffffffffff4
(XEN)    0000000000d40004 ffff8300228c4000 ffff830829db9000 ffff830450652010
(XEN)    ffff83082a357ef8 ffff82c48010351f ffff83082a357e48 ffff82c48016af84
(XEN)    0000000000000000 0000000000000070 ffff83082a357e28 000000000047beea
(XEN)    0000000000000000 ffff83082a30b000 ffff830450652010 ffff830450652010
(XEN)    ffff83082a357e48 0000000080164c7d aaaaaaaaaaaaaaaa ffff83082a30b000
(XEN)    ffff83082a357ef8 ffff82c480113d73 000000070000000d 0000000000000001
(XEN)    0000000000000000 0000000000d42004 0000000000000000 00007fef43c4a791
(XEN)    0000000000000001 0000000000000000 00007fff27dc7db0 00007fef43a1bd58
(XEN)    0000000000000024 0000000000000001 00007fff27dc9710 0000000000000001
(XEN)    0000000000d3f050 00007fef43c51325 0000000000000011 00007fff27dc7dd0
(XEN)    ffff83082a357ed8 ffff8300bf656000 0000000000000003 00007fff27dc7c60
(XEN)    00007fff27dc7c60 0000000000000000 00007cf7d5ca80c7 ffff82c48020e1e8
(XEN)    ffffffff8100948a 0000000000000024 0000000000000000 00007fff27dc7c60
(XEN)    00007fff27dc7c60 0000000000000003 ffff8807a0f2fe68 ffffffff8148d700
(XEN)    0000000000000282 0000000000000024 0000000000d3f050 0000000000d40004
(XEN)    0000000000000024 ffffffff8100948a 0000000100000000 00007fff27dc7ce0
(XEN)    0000000000d40004 0000010000000000 ffffffff8100948a 000000000000e033
(XEN)    0000000000000282 ffff8807a0f2fe20 000000000000e02b 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000002
(XEN) Xen call trace:
(XEN)    [<ffff82c48014dd57>] arch_get_info_guest+0x5f7/0x7b0
(XEN)    [<ffff82c48010351f>] do_domctl+0x10ad/0x195e
(XEN)    [<ffff82c48020e1e8>] syscall_enter+0xc8/0x122

I would appreciate any pointers on how to go about this.

shriram

[-- Attachment #1.2: Type: text/html, Size: 10464 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Jan Beulich]

>>> On 11.05.11 at 04:30, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
>> I tried out a simple program that just gets and sets the VCPU 0's context
> (no change
> whatsoever to anything). There is no intermediate code involved (except for
> the hypercall
> bounce buffer stuff). If all is well, then this should work. But it doesnt!!
> even for a PV guest.
>  I get the same Operation Not supported error when I try to "set" the vcpu
> context with the
> same struct obtained via the get_vcpucontext hypercall!
>...
> and I get - setcontext: operation not supported!

Again, you'll want to add debugging code to the hypervisor to check
what really is inconsistent.

> now for the weirdness:
>  Since the the setcontext failed I thought I should be able
> to run the above sample code again and again with no side effect
> (please correct my assumption if I am wrong).
> 
> But when I run the above code for the second time, I get a XEN panic!
> 
> (XEN) Xen BUG at domctl.c:1724
> (XEN) ----[ Xen-4.2-unstable  x86_64  debug=y  Not tainted ]----
> (XEN) CPU:    2
> (XEN) RIP:    e008:[<ffff82c48014dd57>] arch_get_info_guest+0x5f7/0x7b0
> (XEN) RFLAGS: 0000000000010202   CONTEXT: hypervisor
> (XEN) rax: 0000000000000001   rbx: ffff8300228c4000   rcx: ffff8300228c4040
> (XEN) rdx: 0000000000000000   rsi: 0000000000000000   rdi: ffff830450652210
> (XEN) rbp: ffff83082a357da8   rsp: ffff83082a357d68   r8:  0000000000000002
> (XEN) r9:  0000000000000002   r10: 0000000000000040   r11: 0000000000000000
> (XEN) r12: ffff830450652010   r13: 0000000000000001   r14: ffff830829db9000
> (XEN) r15: ffff830450652010   cr0: 0000000080050033   cr4: 00000000000026f0
> (XEN) cr3: 000000047beef000   cr2: 0000000000d44048
> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
> (XEN) Xen stack trace from rsp=ffff83082a357d68:
> (XEN)    ffff830829db9000 ffff8300228c4000 ffff83082a357d98 fffffffffffffff4
> (XEN)    0000000000d40004 ffff8300228c4000 ffff830829db9000 ffff830450652010
> (XEN)    ffff83082a357ef8 ffff82c48010351f ffff83082a357e48 ffff82c48016af84
> (XEN)    0000000000000000 0000000000000070 ffff83082a357e28 000000000047beea
> (XEN)    0000000000000000 ffff83082a30b000 ffff830450652010 ffff830450652010
> (XEN)    ffff83082a357e48 0000000080164c7d aaaaaaaaaaaaaaaa ffff83082a30b000
> (XEN)    ffff83082a357ef8 ffff82c480113d73 000000070000000d 0000000000000001
> (XEN)    0000000000000000 0000000000d42004 0000000000000000 00007fef43c4a791
> (XEN)    0000000000000001 0000000000000000 00007fff27dc7db0 00007fef43a1bd58
> (XEN)    0000000000000024 0000000000000001 00007fff27dc9710 0000000000000001
> (XEN)    0000000000d3f050 00007fef43c51325 0000000000000011 00007fff27dc7dd0
> (XEN)    ffff83082a357ed8 ffff8300bf656000 0000000000000003 00007fff27dc7c60
> (XEN)    00007fff27dc7c60 0000000000000000 00007cf7d5ca80c7 ffff82c48020e1e8
> (XEN)    ffffffff8100948a 0000000000000024 0000000000000000 00007fff27dc7c60
> (XEN)    00007fff27dc7c60 0000000000000003 ffff8807a0f2fe68 ffffffff8148d700
> (XEN)    0000000000000282 0000000000000024 0000000000d3f050 0000000000d40004
> (XEN)    0000000000000024 ffffffff8100948a 0000000100000000 00007fff27dc7ce0
> (XEN)    0000000000d40004 0000010000000000 ffffffff8100948a 000000000000e033
> (XEN)    0000000000000282 ffff8807a0f2fe20 000000000000e02b 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000002
> (XEN) Xen call trace:
> (XEN)    [<ffff82c48014dd57>] arch_get_info_guest+0x5f7/0x7b0
> (XEN)    [<ffff82c48010351f>] do_domctl+0x10ad/0x195e
> (XEN)    [<ffff82c48020e1e8>] syscall_enter+0xc8/0x122
> 
> I would appreciate any pointers on how to go about this.

This now indeed looks like an inconsistency between
arch_get_info_guest() and the newly introduced error path in
arch_set_info_guest() - the code to put v->arch.user_eflags into
the necessary state now simply doesn't run anymore. It simply
needs to be pulled up in that function (and a few other adjustments
seem also necessary):

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -856,6 +856,15 @@ int arch_set_info_guest(
         goto out;
     }
 
+    init_int80_direct_trap(v);
+
+    /* IOPL privileges are virtualised. */
+    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
+    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
+
+    /* Ensure real hardware interrupts are enabled. */
+    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
+
     if ( !v->is_initialised )
     {
         v->arch.pv_vcpu.ldt_base = c(ldt_base);
@@ -866,7 +875,11 @@ int arch_set_info_guest(
         bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
 
 #ifdef CONFIG_X86_64
-        fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
+        if ( !compat )
+        {
+            fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
+            fail |= !v->arch.pv_vcpu.ctrlreg[1] && !(flags & VGCF_in_kernel);
+        }
 #endif
 
         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
@@ -907,15 +920,6 @@ int arch_set_info_guest(
     v->arch.pv_vcpu.ctrlreg[0] &= X86_CR0_TS;
     v->arch.pv_vcpu.ctrlreg[0] |= read_cr0() & ~X86_CR0_TS;
 
-    init_int80_direct_trap(v);
-
-    /* IOPL privileges are virtualised. */
-    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
-    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
-
-    /* Ensure real hardware interrupts are enabled. */
-    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
-
     cr4 = v->arch.pv_vcpu.ctrlreg[4];
     v->arch.pv_vcpu.ctrlreg[4] = cr4 ? pv_guest_cr4_fixup(v, cr4) :
         real_cr4_to_pv_guest_cr4(mmu_cr4_features);

Can you give this a try?

The question is whether there are other inconsistencies lurking, and
hence whether it wouldn't be better to mark a vCPU on which setting
the context failed, not allowing it to resume or have its context
obtained anymore. That appears quite drastic though - Keir, what's
your opinion here?

Jan

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 7449 bytes --]

On Wed, May 11, 2011 at 2:47 AM, Jan Beulich <JBeulich@novell.com> wrote:

> >>> On 11.05.11 at 04:30, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
> >> I tried out a simple program that just gets and sets the VCPU 0's
> context
> > (no change
> > whatsoever to anything). There is no intermediate code involved (except
> for
> > the hypercall
> > bounce buffer stuff). If all is well, then this should work. But it
> doesnt!!
> > even for a PV guest.
> >  I get the same Operation Not supported error when I try to "set" the
> vcpu
> > context with the
> > same struct obtained via the get_vcpucontext hypercall!
> >...
> > and I get - setcontext: operation not supported!
>
> Again, you'll want to add debugging code to the hypervisor to check
> what really is inconsistent.
>
> > now for the weirdness:
> >  Since the the setcontext failed I thought I should be able
> > to run the above sample code again and again with no side effect
> > (please correct my assumption if I am wrong).
> >
> > But when I run the above code for the second time, I get a XEN panic!
> >
> > (XEN) Xen BUG at domctl.c:1724
> > (XEN) ----[ Xen-4.2-unstable  x86_64  debug=y  Not tainted ]----
> > (XEN) CPU:    2
> > (XEN) RIP:    e008:[<ffff82c48014dd57>] arch_get_info_guest+0x5f7/0x7b0
> > (XEN) RFLAGS: 0000000000010202   CONTEXT: hypervisor
> > (XEN) rax: 0000000000000001   rbx: ffff8300228c4000   rcx:
> ffff8300228c4040
> > (XEN) rdx: 0000000000000000   rsi: 0000000000000000   rdi:
> ffff830450652210
> > (XEN) rbp: ffff83082a357da8   rsp: ffff83082a357d68   r8:
>  0000000000000002
> > (XEN) r9:  0000000000000002   r10: 0000000000000040   r11:
> 0000000000000000
> > (XEN) r12: ffff830450652010   r13: 0000000000000001   r14:
> ffff830829db9000
> > (XEN) r15: ffff830450652010   cr0: 0000000080050033   cr4:
> 00000000000026f0
> > (XEN) cr3: 000000047beef000   cr2: 0000000000d44048
> > (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
> > (XEN) Xen stack trace from rsp=ffff83082a357d68:
> > (XEN)    ffff830829db9000 ffff8300228c4000 ffff83082a357d98
> fffffffffffffff4
> > (XEN)    0000000000d40004 ffff8300228c4000 ffff830829db9000
> ffff830450652010
> > (XEN)    ffff83082a357ef8 ffff82c48010351f ffff83082a357e48
> ffff82c48016af84
> > (XEN)    0000000000000000 0000000000000070 ffff83082a357e28
> 000000000047beea
> > (XEN)    0000000000000000 ffff83082a30b000 ffff830450652010
> ffff830450652010
> > (XEN)    ffff83082a357e48 0000000080164c7d aaaaaaaaaaaaaaaa
> ffff83082a30b000
> > (XEN)    ffff83082a357ef8 ffff82c480113d73 000000070000000d
> 0000000000000001
> > (XEN)    0000000000000000 0000000000d42004 0000000000000000
> 00007fef43c4a791
> > (XEN)    0000000000000001 0000000000000000 00007fff27dc7db0
> 00007fef43a1bd58
> > (XEN)    0000000000000024 0000000000000001 00007fff27dc9710
> 0000000000000001
> > (XEN)    0000000000d3f050 00007fef43c51325 0000000000000011
> 00007fff27dc7dd0
> > (XEN)    ffff83082a357ed8 ffff8300bf656000 0000000000000003
> 00007fff27dc7c60
> > (XEN)    00007fff27dc7c60 0000000000000000 00007cf7d5ca80c7
> ffff82c48020e1e8
> > (XEN)    ffffffff8100948a 0000000000000024 0000000000000000
> 00007fff27dc7c60
> > (XEN)    00007fff27dc7c60 0000000000000003 ffff8807a0f2fe68
> ffffffff8148d700
> > (XEN)    0000000000000282 0000000000000024 0000000000d3f050
> 0000000000d40004
> > (XEN)    0000000000000024 ffffffff8100948a 0000000100000000
> 00007fff27dc7ce0
> > (XEN)    0000000000d40004 0000010000000000 ffffffff8100948a
> 000000000000e033
> > (XEN)    0000000000000282 ffff8807a0f2fe20 000000000000e02b
> 0000000000000000
> > (XEN)    0000000000000000 0000000000000000 0000000000000000
> 0000000000000002
> > (XEN) Xen call trace:
> > (XEN)    [<ffff82c48014dd57>] arch_get_info_guest+0x5f7/0x7b0
> > (XEN)    [<ffff82c48010351f>] do_domctl+0x10ad/0x195e
> > (XEN)    [<ffff82c48020e1e8>] syscall_enter+0xc8/0x122
> >
> > I would appreciate any pointers on how to go about this.
>
> This now indeed looks like an inconsistency between
> arch_get_info_guest() and the newly introduced error path in
> arch_set_info_guest() - the code to put v->arch.user_eflags into
> the necessary state now simply doesn't run anymore. It simply
> needs to be pulled up in that function (and a few other adjustments
> seem also necessary):
>
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -856,6 +856,15 @@ int arch_set_info_guest(
>         goto out;
>     }
>
> +    init_int80_direct_trap(v);
> +
> +    /* IOPL privileges are virtualised. */
> +    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
> +    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
> +
> +    /* Ensure real hardware interrupts are enabled. */
> +    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
> +
>     if ( !v->is_initialised )
>     {
>          v->arch.pv_vcpu.ldt_base = c(ldt_base);
> @@ -866,7 +875,11 @@ int arch_set_info_guest(
>          bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
>
>  #ifdef CONFIG_X86_64
> -        fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
> +        if ( !compat )
> +        {
> +            fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
> +            fail |= !v->arch.pv_vcpu.ctrlreg[1] && !(flags &
> VGCF_in_kernel);
> +        }
>  #endif
>
>         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
> @@ -907,15 +920,6 @@ int arch_set_info_guest(
>     v->arch.pv_vcpu.ctrlreg[0] &= X86_CR0_TS;
>     v->arch.pv_vcpu.ctrlreg[0] |= read_cr0() & ~X86_CR0_TS;
>
> -    init_int80_direct_trap(v);
> -
> -    /* IOPL privileges are virtualised. */
> -    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
> -    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
> -
> -    /* Ensure real hardware interrupts are enabled. */
> -    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
> -
>     cr4 = v->arch.pv_vcpu.ctrlreg[4];
>     v->arch.pv_vcpu.ctrlreg[4] = cr4 ? pv_guest_cr4_fixup(v, cr4) :
>         real_cr4_to_pv_guest_cr4(mmu_cr4_features);
>
> Can you give this a try?

Ok. This patch solves the Xen panic issue but not the EOPNOTSUPP
error. That is, I can use my sample program to "try" to get/set the same
vcpu
context. As usual, only get context succeeded and set context failed with
same EOPNOTSUPP error, for 2.6.18 32-bit domU and 2.6.39 64 bit dom0

And as you said, I added more debugging.

(XEN) domain.c:893:d0 incoming cr3 42b33e000, cur cr3 827ba5000, fail = 1
(XEN) domain.c:901:d0 incoming cr1 42ba6c000, cur cr1 00000000, !(flags &
VGCF_in_kernel)=0,fail=1

corresponding code:
bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
gdprintk(XENLOG_WARNING,
            "incoming cr3 %08lx, cur cr3 %08lx, fail = %d\n",
             c(ctrlreg[3]), v->arch.pv_vcpu.ctrlreg[3], fail);

#ifdef CONFIG_X86_64
if ( !compat )
{
      fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
      gdprintk(XENLOG_WARNING,
                "incoming cr1 %08lx, cur cr1 %08lx, !(flags &
VGCF_in_kernel)=%d,fail=%d\n",
                 c(ctrlreg[1]), v->arch.pv_vcpu.ctrlreg[1], !(flags &
VGCF_in_kernel),fail);
      fail |= !v->arch.pv_vcpu.ctrlreg[1] && !(flags & VGCF_in_kernel);
...

shriram

The question is whether there are other inconsistencies lurking, and
> hence whether it wouldn't be better to mark a vCPU on which setting
> the context failed, not allowing it to resume or have its context
> obtained anymore. That appears quite drastic though - Keir, what's
> your opinion here?
>
> Jan
>
>

[-- Attachment #1.2: Type: text/html, Size: 8896 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 9817 bytes --]

On Wed, May 11, 2011 at 1:37 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca>wrote:

> On Wed, May 11, 2011 at 2:47 AM, Jan Beulich <JBeulich@novell.com> wrote:
>
>> >>> On 11.05.11 at 04:30, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
>> >> I tried out a simple program that just gets and sets the VCPU 0's
>> context
>> > (no change
>> > whatsoever to anything). There is no intermediate code involved (except
>> for
>> > the hypercall
>> > bounce buffer stuff). If all is well, then this should work. But it
>> doesnt!!
>> > even for a PV guest.
>> >  I get the same Operation Not supported error when I try to "set" the
>> vcpu
>> > context with the
>> > same struct obtained via the get_vcpucontext hypercall!
>> >...
>> > and I get - setcontext: operation not supported!
>>
>> Again, you'll want to add debugging code to the hypervisor to check
>> what really is inconsistent.
>>
>> > now for the weirdness:
>> >  Since the the setcontext failed I thought I should be able
>> > to run the above sample code again and again with no side effect
>> > (please correct my assumption if I am wrong).
>> >
>> > But when I run the above code for the second time, I get a XEN panic!
>> >
>> > (XEN) Xen BUG at domctl.c:1724
>> > (XEN) ----[ Xen-4.2-unstable  x86_64  debug=y  Not tainted ]----
>> > (XEN) CPU:    2
>> > (XEN) RIP:    e008:[<ffff82c48014dd57>] arch_get_info_guest+0x5f7/0x7b0
>> > (XEN) RFLAGS: 0000000000010202   CONTEXT: hypervisor
>> > (XEN) rax: 0000000000000001   rbx: ffff8300228c4000   rcx:
>> ffff8300228c4040
>> > (XEN) rdx: 0000000000000000   rsi: 0000000000000000   rdi:
>> ffff830450652210
>> > (XEN) rbp: ffff83082a357da8   rsp: ffff83082a357d68   r8:
>>  0000000000000002
>> > (XEN) r9:  0000000000000002   r10: 0000000000000040   r11:
>> 0000000000000000
>> > (XEN) r12: ffff830450652010   r13: 0000000000000001   r14:
>> ffff830829db9000
>> > (XEN) r15: ffff830450652010   cr0: 0000000080050033   cr4:
>> 00000000000026f0
>> > (XEN) cr3: 000000047beef000   cr2: 0000000000d44048
>> > (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
>> > (XEN) Xen stack trace from rsp=ffff83082a357d68:
>> > (XEN)    ffff830829db9000 ffff8300228c4000 ffff83082a357d98
>> fffffffffffffff4
>> > (XEN)    0000000000d40004 ffff8300228c4000 ffff830829db9000
>> ffff830450652010
>> > (XEN)    ffff83082a357ef8 ffff82c48010351f ffff83082a357e48
>> ffff82c48016af84
>> > (XEN)    0000000000000000 0000000000000070 ffff83082a357e28
>> 000000000047beea
>> > (XEN)    0000000000000000 ffff83082a30b000 ffff830450652010
>> ffff830450652010
>> > (XEN)    ffff83082a357e48 0000000080164c7d aaaaaaaaaaaaaaaa
>> ffff83082a30b000
>> > (XEN)    ffff83082a357ef8 ffff82c480113d73 000000070000000d
>> 0000000000000001
>> > (XEN)    0000000000000000 0000000000d42004 0000000000000000
>> 00007fef43c4a791
>> > (XEN)    0000000000000001 0000000000000000 00007fff27dc7db0
>> 00007fef43a1bd58
>> > (XEN)    0000000000000024 0000000000000001 00007fff27dc9710
>> 0000000000000001
>> > (XEN)    0000000000d3f050 00007fef43c51325 0000000000000011
>> 00007fff27dc7dd0
>> > (XEN)    ffff83082a357ed8 ffff8300bf656000 0000000000000003
>> 00007fff27dc7c60
>> > (XEN)    00007fff27dc7c60 0000000000000000 00007cf7d5ca80c7
>> ffff82c48020e1e8
>> > (XEN)    ffffffff8100948a 0000000000000024 0000000000000000
>> 00007fff27dc7c60
>> > (XEN)    00007fff27dc7c60 0000000000000003 ffff8807a0f2fe68
>> ffffffff8148d700
>> > (XEN)    0000000000000282 0000000000000024 0000000000d3f050
>> 0000000000d40004
>> > (XEN)    0000000000000024 ffffffff8100948a 0000000100000000
>> 00007fff27dc7ce0
>> > (XEN)    0000000000d40004 0000010000000000 ffffffff8100948a
>> 000000000000e033
>> > (XEN)    0000000000000282 ffff8807a0f2fe20 000000000000e02b
>> 0000000000000000
>> > (XEN)    0000000000000000 0000000000000000 0000000000000000
>> 0000000000000002
>> > (XEN) Xen call trace:
>> > (XEN)    [<ffff82c48014dd57>] arch_get_info_guest+0x5f7/0x7b0
>> > (XEN)    [<ffff82c48010351f>] do_domctl+0x10ad/0x195e
>> > (XEN)    [<ffff82c48020e1e8>] syscall_enter+0xc8/0x122
>> >
>> > I would appreciate any pointers on how to go about this.
>>
>> This now indeed looks like an inconsistency between
>> arch_get_info_guest() and the newly introduced error path in
>> arch_set_info_guest() - the code to put v->arch.user_eflags into
>> the necessary state now simply doesn't run anymore. It simply
>> needs to be pulled up in that function (and a few other adjustments
>> seem also necessary):
>>
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -856,6 +856,15 @@ int arch_set_info_guest(
>>         goto out;
>>     }
>>
>> +    init_int80_direct_trap(v);
>> +
>> +    /* IOPL privileges are virtualised. */
>> +    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
>> +    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
>> +
>> +    /* Ensure real hardware interrupts are enabled. */
>> +    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
>> +
>>     if ( !v->is_initialised )
>>     {
>>          v->arch.pv_vcpu.ldt_base = c(ldt_base);
>> @@ -866,7 +875,11 @@ int arch_set_info_guest(
>>          bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
>>
>>  #ifdef CONFIG_X86_64
>> -        fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
>> +        if ( !compat )
>> +        {
>> +            fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
>> +            fail |= !v->arch.pv_vcpu.ctrlreg[1] && !(flags &
>> VGCF_in_kernel);
>> +        }
>>  #endif
>>
>>         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
>> @@ -907,15 +920,6 @@ int arch_set_info_guest(
>>     v->arch.pv_vcpu.ctrlreg[0] &= X86_CR0_TS;
>>     v->arch.pv_vcpu.ctrlreg[0] |= read_cr0() & ~X86_CR0_TS;
>>
>> -    init_int80_direct_trap(v);
>> -
>> -    /* IOPL privileges are virtualised. */
>> -    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
>> -    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
>> -
>> -    /* Ensure real hardware interrupts are enabled. */
>> -    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
>> -
>>     cr4 = v->arch.pv_vcpu.ctrlreg[4];
>>     v->arch.pv_vcpu.ctrlreg[4] = cr4 ? pv_guest_cr4_fixup(v, cr4) :
>>         real_cr4_to_pv_guest_cr4(mmu_cr4_features);
>>
>> Can you give this a try?
>
> Ok. This patch solves the Xen panic issue but not the EOPNOTSUPP
> error. That is, I can use my sample program to "try" to get/set the same
> vcpu
> context. As usual, only get context succeeded and set context failed with
> same EOPNOTSUPP error, for 2.6.18 32-bit domU and 2.6.39 64 bit dom0
>
> And as you said, I added more debugging.
>
> (XEN) domain.c:893:d0 incoming cr3 42b33e000, cur cr3 827ba5000, fail = 1
> (XEN) domain.c:901:d0 incoming cr1 42ba6c000, cur cr1 00000000, !(flags &
> VGCF_in_kernel)=0,fail=1
>
> Looking at arch_get_info_guest in domctl.c , I see that cr3 is first copied
verbatim from the vcpu and
then modified in the if-else block
if ( !is_pv_32on64_domain(v->domain) )
        {
            c.nat->ctrlreg[3] = xen_pfn_to_cr3(
                pagetable_get_pfn(v->arch.guest_table));
#ifdef __x86_64__
            c.nat->ctrlreg[1] =
                pagetable_is_null(v->arch.guest_table_user) ? 0
                :
xen_pfn_to_cr3(pagetable_get_pfn(v->arch.guest_table_user));
#endif
....
   } else {
            l4_pgentry_t *l4e =
__va(pagetable_get_paddr(v->arch.guest_table));
            c.cmp->ctrlreg[3] = compat_pfn_to_cr3(l4e_get_pfn(*l4e));
}

This seems to account for the difference in the values that libxc supplies
(obtained from get context)
and the one validated against by arch_set_info_guest
 arch_set_context validates cr3 and cr1 against the wrong values (the
vcpu.cr[1/3]) while it should
 be validated against the value that results from the operation done in the
if-else loop in arch_get_info_guest

I have verified this too, with both a 32bit domU and 64bit domU.

64-bit PV domU (2.6.39..)
--------------------------------------
get_vcpu_context(): (debug output from arch_get_info_guest)
(XEN) domctl.c:1707:d0  copying cr1 00000000
(XEN) domctl.c:1707:d0  copying cr3 827bd5000
(XEN) domctl.c:1743:d0 not pv_32on64, outgoing cr3 42b85b000, cur cr3
827bd5000
(XEN) domctl.c:1746:d0 not pv_32on64, outgoing cr1 42b85c000, cur cr1
00000000

set_vcpu_context(): (debug output from arch_set_info_guest)
(XEN) domain.c:893:d0 incoming cr3 42b85b000, cur cr3 827bd5000, fail = 1
(XEN) domain.c:901:d0 incoming cr1 42b85c000, cur cr1 00000000, !(flags &
VGCF_in_kernel)=0,fail=1

32-bit PV domU (2.6.18)
----------------------------------
get_vcpu_context()
(XEN) domctl.c:1707:d0 copying cr1 00000000
(XEN) domctl.c:1707:d0 copying cr3 2960e008
(XEN) domctl.c:1758:d0 is pv_32on64, outgoing cr3 4f0ac004, cur cr3 2960e008

set_vcpu_context()
(XEN) domain.c:893:d0 incoming cr3 4f0ac004, cur cr3 2960e008, fail = 1


shriram

> corresponding code:
>
> bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
> gdprintk(XENLOG_WARNING,
>             "incoming cr3 %08lx, cur cr3 %08lx, fail = %d\n",
>              c(ctrlreg[3]), v->arch.pv_vcpu.ctrlreg[3], fail);
>
> #ifdef CONFIG_X86_64
>
> if ( !compat )
> {
>       fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
>        gdprintk(XENLOG_WARNING,
>                 "incoming cr1 %08lx, cur cr1 %08lx, !(flags &
> VGCF_in_kernel)=%d,fail=%d\n",
>                  c(ctrlreg[1]), v->arch.pv_vcpu.ctrlreg[1], !(flags &
> VGCF_in_kernel),fail);
>
>       fail |= !v->arch.pv_vcpu.ctrlreg[1] && !(flags & VGCF_in_kernel);
> ...
>
> shriram
>
> The question is whether there are other inconsistencies lurking, and
>> hence whether it wouldn't be better to mark a vCPU on which setting
>> the context failed, not allowing it to resume or have its context
>> obtained anymore. That appears quite drastic though - Keir, what's
>> your opinion here?
>>
>> Jan
>>
>>
>

[-- Attachment #1.2: Type: text/html, Size: 11864 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Jan Beulich]

>>> On 11.05.11 at 21:50, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
> Looking at arch_get_info_guest in domctl.c , I see that cr3 is first copied
> verbatim from the vcpu and
> then modified in the if-else block
> if ( !is_pv_32on64_domain(v->domain) )
>         {
>             c.nat->ctrlreg[3] = xen_pfn_to_cr3(
>                 pagetable_get_pfn(v->arch.guest_table));
> #ifdef __x86_64__
>             c.nat->ctrlreg[1] =
>                 pagetable_is_null(v->arch.guest_table_user) ? 0
>                 :
> xen_pfn_to_cr3(pagetable_get_pfn(v->arch.guest_table_user));
> #endif
> ....
>    } else {
>             l4_pgentry_t *l4e =
> __va(pagetable_get_paddr(v->arch.guest_table));
>             c.cmp->ctrlreg[3] = compat_pfn_to_cr3(l4e_get_pfn(*l4e));
> }
> 
> This seems to account for the difference in the values that libxc supplies
> (obtained from get context)
> and the one validated against by arch_set_info_guest
>  arch_set_context validates cr3 and cr1 against the wrong values (the
> vcpu.cr[1/3]) while it should
>  be validated against the value that results from the operation done in the
> if-else loop in arch_get_info_guest

The problem really is that ->ctrlreg[3] (and also ->ctrlreg[1]) aren't
really being kept up-to-date while the domain is running (they get
written only from arch_set_info_guest()), and as such we could
(should?) actually delete these fields, to not confuse people (like
me in this case).

Here's another shot at it:

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -856,6 +856,15 @@ int arch_set_info_guest(
         goto out;
     }
 
+    init_int80_direct_trap(v);
+
+    /* IOPL privileges are virtualised. */
+    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
+    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
+
+    /* Ensure real hardware interrupts are enabled. */
+    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
+
     if ( !v->is_initialised )
     {
         v->arch.pv_vcpu.ldt_base = c(ldt_base);
@@ -863,10 +872,25 @@ int arch_set_info_guest(
     }
     else
     {
-        bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
+        unsigned long pfn = pagetable_get_pfn(v->arch.guest_table);
+        bool_t fail;
 
+        if ( !compat )
+        {
+            fail = xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[3];
 #ifdef CONFIG_X86_64
-        fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
+            if ( pagetable_is_null(v->arch.guest_table_user) )
+                fail |= c.nat->ctrlreg[1] || !(flags & VGCF_in_kernel);
+            else
+            {
+                pfn = pagetable_get_pfn(v->arch.guest_table_user);
+                fail |= xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[1];
+            }
+#endif
+        }
+#ifdef CONFIG_COMPAT
+        else
+            fail = compat_pfn_to_cr3(pfn) != c.cmp->ctrlreg[3];
 #endif
 
         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
@@ -907,15 +931,6 @@ int arch_set_info_guest(
     v->arch.pv_vcpu.ctrlreg[0] &= X86_CR0_TS;
     v->arch.pv_vcpu.ctrlreg[0] |= read_cr0() & ~X86_CR0_TS;
 
-    init_int80_direct_trap(v);
-
-    /* IOPL privileges are virtualised. */
-    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
-    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
-
-    /* Ensure real hardware interrupts are enabled. */
-    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
-
     cr4 = v->arch.pv_vcpu.ctrlreg[4];
     v->arch.pv_vcpu.ctrlreg[4] = cr4 ? pv_guest_cr4_fixup(v, cr4) :
         real_cr4_to_pv_guest_cr4(mmu_cr4_features);

Jan

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 3920 bytes --]

On Fri, May 13, 2011 at 6:00 AM, Jan Beulich <JBeulich@novell.com> wrote:

> >>> On 11.05.11 at 21:50, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
> > Looking at arch_get_info_guest in domctl.c , I see that cr3 is first
> copied
> > verbatim from the vcpu and
> > then modified in the if-else block
> > if ( !is_pv_32on64_domain(v->domain) )
> >         {
> >             c.nat->ctrlreg[3] = xen_pfn_to_cr3(
> >                 pagetable_get_pfn(v->arch.guest_table));
> > #ifdef __x86_64__
> >             c.nat->ctrlreg[1] =
> >                 pagetable_is_null(v->arch.guest_table_user) ? 0
> >                 :
> > xen_pfn_to_cr3(pagetable_get_pfn(v->arch.guest_table_user));
> > #endif
> > ....
> >    } else {
> >             l4_pgentry_t *l4e =
> > __va(pagetable_get_paddr(v->arch.guest_table));
> >             c.cmp->ctrlreg[3] = compat_pfn_to_cr3(l4e_get_pfn(*l4e));
> > }
> >
> > This seems to account for the difference in the values that libxc
> supplies
> > (obtained from get context)
> > and the one validated against by arch_set_info_guest
> >  arch_set_context validates cr3 and cr1 against the wrong values (the
> > vcpu.cr[1/3]) while it should
> >  be validated against the value that results from the operation done in
> the
> > if-else loop in arch_get_info_guest
>
> The problem really is that ->ctrlreg[3] (and also ->ctrlreg[1]) aren't
> really being kept up-to-date while the domain is running (they get
> written only from arch_set_info_guest()), and as such we could
> (should?) actually delete these fields, to not confuse people (like
> me in this case).
>
> Here's another shot at it:
>
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -856,6 +856,15 @@ int arch_set_info_guest(
>         goto out;
>     }
>
> +    init_int80_direct_trap(v);
> +
> +    /* IOPL privileges are virtualised. */
> +    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
> +    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
> +
> +    /* Ensure real hardware interrupts are enabled. */
> +    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
> +
>     if ( !v->is_initialised )
>     {
>         v->arch.pv_vcpu.ldt_base = c(ldt_base);
> @@ -863,10 +872,25 @@ int arch_set_info_guest(
>     }
>     else
>     {
> -        bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
> +        unsigned long pfn = pagetable_get_pfn(v->arch.guest_table);
> +        bool_t fail;
>
> +        if ( !compat )
> +        {
> +            fail = xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[3];
>  #ifdef CONFIG_X86_64
> -        fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
> +            if ( pagetable_is_null(v->arch.guest_table_user) )
> +                fail |= c.nat->ctrlreg[1] || !(flags & VGCF_in_kernel);
> +            else
> +            {
> +                pfn = pagetable_get_pfn(v->arch.guest_table_user);
> +                fail |= xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[1];
> +            }
> +#endif
> +        }
> +#ifdef CONFIG_COMPAT
> +        else
> +            fail = compat_pfn_to_cr3(pfn) != c.cmp->ctrlreg[3];
>  #endif
>
>         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
> @@ -907,15 +931,6 @@ int arch_set_info_guest(
>      v->arch.pv_vcpu.ctrlreg[0] &= X86_CR0_TS;
>     v->arch.pv_vcpu.ctrlreg[0] |= read_cr0() & ~X86_CR0_TS;
>
> -    init_int80_direct_trap(v);
> -
> -    /* IOPL privileges are virtualised. */
> -    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
> -    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
> -
> -    /* Ensure real hardware interrupts are enabled. */
> -    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
> -
>     cr4 = v->arch.pv_vcpu.ctrlreg[4];
>     v->arch.pv_vcpu.ctrlreg[4] = cr4 ? pv_guest_cr4_fixup(v, cr4) :
>         real_cr4_to_pv_guest_cr4(mmu_cr4_features);
>
> Jan
>
> This one works only for 64-bit domUs. 32bit domU (on 64bit dom0) fails with

usual EOPNOTSUPP.

shriram

[-- Attachment #1.2: Type: text/html, Size: 4972 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Jan Beulich]

>>> On 15.05.11 at 00:15, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
> This one works only for 64-bit domUs. 32bit domU (on 64bit dom0) fails with
> usual EOPNOTSUPP.

Next (hopefully final) try below.

Jan

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -856,6 +856,15 @@ int arch_set_info_guest(
         goto out;
     }
 
+    init_int80_direct_trap(v);
+
+    /* IOPL privileges are virtualised. */
+    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
+    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
+
+    /* Ensure real hardware interrupts are enabled. */
+    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
+
     if ( !v->is_initialised )
     {
         v->arch.pv_vcpu.ldt_base = c(ldt_base);
@@ -863,11 +872,27 @@ int arch_set_info_guest(
     }
     else
     {
-        bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
+        unsigned long pfn = pagetable_get_pfn(v->arch.guest_table);
+        bool_t fail;
 
+        if ( !compat )
+        {
+            fail = xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[3];
 #ifdef CONFIG_X86_64
-        fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
+            if ( pagetable_is_null(v->arch.guest_table_user) )
+                fail |= c.nat->ctrlreg[1] || !(flags & VGCF_in_kernel);
+            else
+            {
+                pfn = pagetable_get_pfn(v->arch.guest_table_user);
+                fail |= xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[1];
+            }
+        } else {
+            l4_pgentry_t *l4tab = __va(pfn_to_paddr(pfn));
+
+            pfn = l4e_get_pfn(*l4tab);
+            fail = compat_pfn_to_cr3(pfn) != c.cmp->ctrlreg[3];
 #endif
+        }
 
         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
             fail |= v->arch.pv_vcpu.gdt_frames[i] != c(gdt_frames[i]);
@@ -907,15 +932,6 @@ int arch_set_info_guest(
     v->arch.pv_vcpu.ctrlreg[0] &= X86_CR0_TS;
     v->arch.pv_vcpu.ctrlreg[0] |= read_cr0() & ~X86_CR0_TS;
 
-    init_int80_direct_trap(v);
-
-    /* IOPL privileges are virtualised. */
-    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
-    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
-
-    /* Ensure real hardware interrupts are enabled. */
-    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
-
     cr4 = v->arch.pv_vcpu.ctrlreg[4];
     v->arch.pv_vcpu.ctrlreg[4] = cr4 ? pv_guest_cr4_fixup(v, cr4) :
         real_cr4_to_pv_guest_cr4(mmu_cr4_features);

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 2792 bytes --]

On Mon, May 16, 2011 at 8:02 AM, Jan Beulich <JBeulich@novell.com> wrote:

> >>> On 15.05.11 at 00:15, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
> > This one works only for 64-bit domUs. 32bit domU (on 64bit dom0) fails
> with
> > usual EOPNOTSUPP.
>
> Next (hopefully final) try below.
>
> Jan
>
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -856,6 +856,15 @@ int arch_set_info_guest(
>         goto out;
>     }
>
> +    init_int80_direct_trap(v);
> +
> +    /* IOPL privileges are virtualised. */
> +    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
> +    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
> +
> +    /* Ensure real hardware interrupts are enabled. */
> +    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
> +
>     if ( !v->is_initialised )
>     {
>         v->arch.pv_vcpu.ldt_base = c(ldt_base);
> @@ -863,11 +872,27 @@ int arch_set_info_guest(
>      }
>     else
>     {
> -        bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
> +        unsigned long pfn = pagetable_get_pfn(v->arch.guest_table);
> +        bool_t fail;
>
> +        if ( !compat )
> +        {
> +            fail = xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[3];
>  #ifdef CONFIG_X86_64
> -        fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
> +            if ( pagetable_is_null(v->arch.guest_table_user) )
> +                fail |= c.nat->ctrlreg[1] || !(flags & VGCF_in_kernel);
> +            else
> +            {
> +                pfn = pagetable_get_pfn(v->arch.guest_table_user);
> +                fail |= xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[1];
> +            }
> +        } else {
> +            l4_pgentry_t *l4tab = __va(pfn_to_paddr(pfn));
> +
> +            pfn = l4e_get_pfn(*l4tab);
> +            fail = compat_pfn_to_cr3(pfn) != c.cmp->ctrlreg[3];
>  #endif
> +        }
>
>         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
>              fail |= v->arch.pv_vcpu.gdt_frames[i] != c(gdt_frames[i]);
> @@ -907,15 +932,6 @@ int arch_set_info_guest(
>      v->arch.pv_vcpu.ctrlreg[0] &= X86_CR0_TS;
>     v->arch.pv_vcpu.ctrlreg[0] |= read_cr0() & ~X86_CR0_TS;
>
> -    init_int80_direct_trap(v);
> -
> -    /* IOPL privileges are virtualised. */
> -    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
> -    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
> -
> -    /* Ensure real hardware interrupts are enabled. */
> -    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
> -
>     cr4 = v->arch.pv_vcpu.ctrlreg[4];
>     v->arch.pv_vcpu.ctrlreg[4] = cr4 ? pv_guest_cr4_fixup(v, cr4) :
>         real_cr4_to_pv_guest_cr4(mmu_cr4_features);
>
>
> ok. this one works :). I ve tested with remus too :P. Can you please please
spin out a patch to be pushed into the repo ? thanks for the help.

shriram

[-- Attachment #1.2: Type: text/html, Size: 3579 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[AP Xen]

Is is possible for this (23348:3e8e1800d472) to be included in Xen 4.1.1?

Thanks,
AP

On Mon, May 16, 2011 at 6:48 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
> On Mon, May 16, 2011 at 8:02 AM, Jan Beulich <JBeulich@novell.com> wrote:
>>
>> >>> On 15.05.11 at 00:15, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
>> > This one works only for 64-bit domUs. 32bit domU (on 64bit dom0) fails
>> > with
>> > usual EOPNOTSUPP.
>>
>> Next (hopefully final) try below.
>>
>> Jan
>>
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -856,6 +856,15 @@ int arch_set_info_guest(
>>         goto out;
>>     }
>>
>> +    init_int80_direct_trap(v);
>> +
>> +    /* IOPL privileges are virtualised. */
>> +    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
>> +    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
>> +
>> +    /* Ensure real hardware interrupts are enabled. */
>> +    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
>> +
>>     if ( !v->is_initialised )
>>     {
>>         v->arch.pv_vcpu.ldt_base = c(ldt_base);
>> @@ -863,11 +872,27 @@ int arch_set_info_guest(
>>     }
>>     else
>>     {
>> -        bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
>> +        unsigned long pfn = pagetable_get_pfn(v->arch.guest_table);
>> +        bool_t fail;
>>
>> +        if ( !compat )
>> +        {
>> +            fail = xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[3];
>>  #ifdef CONFIG_X86_64
>> -        fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
>> +            if ( pagetable_is_null(v->arch.guest_table_user) )
>> +                fail |= c.nat->ctrlreg[1] || !(flags & VGCF_in_kernel);
>> +            else
>> +            {
>> +                pfn = pagetable_get_pfn(v->arch.guest_table_user);
>> +                fail |= xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[1];
>> +            }
>> +        } else {
>> +            l4_pgentry_t *l4tab = __va(pfn_to_paddr(pfn));
>> +
>> +            pfn = l4e_get_pfn(*l4tab);
>> +            fail = compat_pfn_to_cr3(pfn) != c.cmp->ctrlreg[3];
>>  #endif
>> +        }
>>
>>         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
>>             fail |= v->arch.pv_vcpu.gdt_frames[i] != c(gdt_frames[i]);
>> @@ -907,15 +932,6 @@ int arch_set_info_guest(
>>     v->arch.pv_vcpu.ctrlreg[0] &= X86_CR0_TS;
>>     v->arch.pv_vcpu.ctrlreg[0] |= read_cr0() & ~X86_CR0_TS;
>>
>> -    init_int80_direct_trap(v);
>> -
>> -    /* IOPL privileges are virtualised. */
>> -    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
>> -    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
>> -
>> -    /* Ensure real hardware interrupts are enabled. */
>> -    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
>> -
>>     cr4 = v->arch.pv_vcpu.ctrlreg[4];
>>     v->arch.pv_vcpu.ctrlreg[4] = cr4 ? pv_guest_cr4_fixup(v, cr4) :
>>         real_cr4_to_pv_guest_cr4(mmu_cr4_features);
>>
>>
> ok. this one works :). I ve tested with remus too :P. Can you please please
> spin out a patch to be pushed into the repo ? thanks for the help.
>
> shriram
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Keir Fraser]

It depends on c/s 23142, and I think it isn't even needed in the absence of
that other patch.

 -- Keir

On 24/05/2011 20:24, "AP Xen" <apxeng@gmail.com> wrote:

> Is is possible for this (23348:3e8e1800d472) to be included in Xen 4.1.1?
> 
> Thanks,
> AP
> 
> On Mon, May 16, 2011 at 6:48 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca>
> wrote:
>> On Mon, May 16, 2011 at 8:02 AM, Jan Beulich <JBeulich@novell.com> wrote:
>>> 
>>>>>> On 15.05.11 at 00:15, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
>>>> This one works only for 64-bit domUs. 32bit domU (on 64bit dom0) fails
>>>> with
>>>> usual EOPNOTSUPP.
>>> 
>>> Next (hopefully final) try below.
>>> 
>>> Jan
>>> 
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -856,6 +856,15 @@ int arch_set_info_guest(
>>>         goto out;
>>>     }
>>> 
>>> +    init_int80_direct_trap(v);
>>> +
>>> +    /* IOPL privileges are virtualised. */
>>> +    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
>>> +    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
>>> +
>>> +    /* Ensure real hardware interrupts are enabled. */
>>> +    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
>>> +
>>>     if ( !v->is_initialised )
>>>     {
>>>         v->arch.pv_vcpu.ldt_base = c(ldt_base);
>>> @@ -863,11 +872,27 @@ int arch_set_info_guest(
>>>     }
>>>     else
>>>     {
>>> -        bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
>>> +        unsigned long pfn = pagetable_get_pfn(v->arch.guest_table);
>>> +        bool_t fail;
>>> 
>>> +        if ( !compat )
>>> +        {
>>> +            fail = xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[3];
>>>  #ifdef CONFIG_X86_64
>>> -        fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
>>> +            if ( pagetable_is_null(v->arch.guest_table_user) )
>>> +                fail |= c.nat->ctrlreg[1] || !(flags & VGCF_in_kernel);
>>> +            else
>>> +            {
>>> +                pfn = pagetable_get_pfn(v->arch.guest_table_user);
>>> +                fail |= xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[1];
>>> +            }
>>> +        } else {
>>> +            l4_pgentry_t *l4tab = __va(pfn_to_paddr(pfn));
>>> +
>>> +            pfn = l4e_get_pfn(*l4tab);
>>> +            fail = compat_pfn_to_cr3(pfn) != c.cmp->ctrlreg[3];
>>>  #endif
>>> +        }
>>> 
>>>         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
>>>             fail |= v->arch.pv_vcpu.gdt_frames[i] != c(gdt_frames[i]);
>>> @@ -907,15 +932,6 @@ int arch_set_info_guest(
>>>     v->arch.pv_vcpu.ctrlreg[0] &= X86_CR0_TS;
>>>     v->arch.pv_vcpu.ctrlreg[0] |= read_cr0() & ~X86_CR0_TS;
>>> 
>>> -    init_int80_direct_trap(v);
>>> -
>>> -    /* IOPL privileges are virtualised. */
>>> -    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
>>> -    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
>>> -
>>> -    /* Ensure real hardware interrupts are enabled. */
>>> -    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
>>> -
>>>     cr4 = v->arch.pv_vcpu.ctrlreg[4];
>>>     v->arch.pv_vcpu.ctrlreg[4] = cr4 ? pv_guest_cr4_fixup(v, cr4) :
>>>         real_cr4_to_pv_guest_cr4(mmu_cr4_features);
>>> 
>>> 
>> ok. this one works :). I ve tested with remus too :P. Can you please please
>> spin out a patch to be pushed into the repo ? thanks for the help.
>> 
>> shriram
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>> 
>> 

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Jan Beulich]

>>> On 24.05.11 at 22:09, Keir Fraser <keir.xen@gmail.com> wrote:
> It depends on c/s 23142, and I think it isn't even needed in the absence of
> that other patch.

Correct - no checking on the context was done up to 23142, so as
long as that one doesn't get backported the fix to it obviously doesn't
need to be either.

It's odd in the first place that you're asking, as it sort of implies you've
been seeing -EOPNOTSUPP problems on 4.1, which you'd need to
explain/investigate independently of the problem addressed here.

Jan

>  -- Keir
> 
> On 24/05/2011 20:24, "AP Xen" <apxeng@gmail.com> wrote:
> 
>> Is is possible for this (23348:3e8e1800d472) to be included in Xen 4.1.1?
>> 
>> Thanks,
>> AP
>> 
>> On Mon, May 16, 2011 at 6:48 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca>
>> wrote:
>>> On Mon, May 16, 2011 at 8:02 AM, Jan Beulich <JBeulich@novell.com> wrote:
>>>> 
>>>>>>> On 15.05.11 at 00:15, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
>>>>> This one works only for 64-bit domUs. 32bit domU (on 64bit dom0) fails
>>>>> with
>>>>> usual EOPNOTSUPP.
>>>> 
>>>> Next (hopefully final) try below.
>>>> 
>>>> Jan
>>>> 
>>>> --- a/xen/arch/x86/domain.c
>>>> +++ b/xen/arch/x86/domain.c
>>>> @@ -856,6 +856,15 @@ int arch_set_info_guest(
>>>>         goto out;
>>>>     }
>>>> 
>>>> +    init_int80_direct_trap(v);
>>>> +
>>>> +    /* IOPL privileges are virtualised. */
>>>> +    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
>>>> +    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
>>>> +
>>>> +    /* Ensure real hardware interrupts are enabled. */
>>>> +    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
>>>> +
>>>>     if ( !v->is_initialised )
>>>>     {
>>>>         v->arch.pv_vcpu.ldt_base = c(ldt_base);
>>>> @@ -863,11 +872,27 @@ int arch_set_info_guest(
>>>>     }
>>>>     else
>>>>     {
>>>> -        bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
>>>> +        unsigned long pfn = pagetable_get_pfn(v->arch.guest_table);
>>>> +        bool_t fail;
>>>> 
>>>> +        if ( !compat )
>>>> +        {
>>>> +            fail = xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[3];
>>>>  #ifdef CONFIG_X86_64
>>>> -        fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
>>>> +            if ( pagetable_is_null(v->arch.guest_table_user) )
>>>> +                fail |= c.nat->ctrlreg[1] || !(flags & VGCF_in_kernel);
>>>> +            else
>>>> +            {
>>>> +                pfn = pagetable_get_pfn(v->arch.guest_table_user);
>>>> +                fail |= xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[1];
>>>> +            }
>>>> +        } else {
>>>> +            l4_pgentry_t *l4tab = __va(pfn_to_paddr(pfn));
>>>> +
>>>> +            pfn = l4e_get_pfn(*l4tab);
>>>> +            fail = compat_pfn_to_cr3(pfn) != c.cmp->ctrlreg[3];
>>>>  #endif
>>>> +        }
>>>> 
>>>>         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
>>>>             fail |= v->arch.pv_vcpu.gdt_frames[i] != c(gdt_frames[i]);
>>>> @@ -907,15 +932,6 @@ int arch_set_info_guest(
>>>>     v->arch.pv_vcpu.ctrlreg[0] &= X86_CR0_TS;
>>>>     v->arch.pv_vcpu.ctrlreg[0] |= read_cr0() & ~X86_CR0_TS;
>>>> 
>>>> -    init_int80_direct_trap(v);
>>>> -
>>>> -    /* IOPL privileges are virtualised. */
>>>> -    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
>>>> -    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
>>>> -
>>>> -    /* Ensure real hardware interrupts are enabled. */
>>>> -    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
>>>> -
>>>>     cr4 = v->arch.pv_vcpu.ctrlreg[4];
>>>>     v->arch.pv_vcpu.ctrlreg[4] = cr4 ? pv_guest_cr4_fixup(v, cr4) :
>>>>         real_cr4_to_pv_guest_cr4(mmu_cr4_features);
>>>> 
>>>> 
>>> ok. this one works :). I ve tested with remus too :P. Can you please please
>>> spin out a patch to be pushed into the repo ? thanks for the help.
>>> 
>>> shriram
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xensource.com 
>>> http://lists.xensource.com/xen-devel 
>>> 
>>> 

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[AP Xen]

On Wed, May 25, 2011 at 12:15 AM, Jan Beulich <JBeulich@novell.com> wrote:
>>>> On 24.05.11 at 22:09, Keir Fraser <keir.xen@gmail.com> wrote:
>> It depends on c/s 23142, and I think it isn't even needed in the absence of
>> that other patch.
>
> Correct - no checking on the context was done up to 23142, so as
> long as that one doesn't get backported the fix to it obviously doesn't
> need to be either.
>
> It's odd in the first place that you're asking, as it sort of implies you've
> been seeing -EOPNOTSUPP problems on 4.1, which you'd need to
> explain/investigate independently of the problem addressed here.

You are right. I lost site of my original problem which is xl save -c
not working with 4.1. I did a quick get_vcpucontext and
set_vcpucontext test with a HVM domain and that seems to work without
any issues. So I think the root cause of the -c option not working is
something else.

AP

> Jan
>
>>  -- Keir
>>
>> On 24/05/2011 20:24, "AP Xen" <apxeng@gmail.com> wrote:
>>
>>> Is is possible for this (23348:3e8e1800d472) to be included in Xen 4.1.1?
>>>
>>> Thanks,
>>> AP
>>>
>>> On Mon, May 16, 2011 at 6:48 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca>
>>> wrote:
>>>> On Mon, May 16, 2011 at 8:02 AM, Jan Beulich <JBeulich@novell.com> wrote:
>>>>>
>>>>>>>> On 15.05.11 at 00:15, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
>>>>>> This one works only for 64-bit domUs. 32bit domU (on 64bit dom0) fails
>>>>>> with
>>>>>> usual EOPNOTSUPP.
>>>>>
>>>>> Next (hopefully final) try below.
>>>>>
>>>>> Jan
>>>>>
>>>>> --- a/xen/arch/x86/domain.c
>>>>> +++ b/xen/arch/x86/domain.c
>>>>> @@ -856,6 +856,15 @@ int arch_set_info_guest(
>>>>>         goto out;
>>>>>     }
>>>>>
>>>>> +    init_int80_direct_trap(v);
>>>>> +
>>>>> +    /* IOPL privileges are virtualised. */
>>>>> +    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
>>>>> +    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
>>>>> +
>>>>> +    /* Ensure real hardware interrupts are enabled. */
>>>>> +    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
>>>>> +
>>>>>     if ( !v->is_initialised )
>>>>>     {
>>>>>         v->arch.pv_vcpu.ldt_base = c(ldt_base);
>>>>> @@ -863,11 +872,27 @@ int arch_set_info_guest(
>>>>>     }
>>>>>     else
>>>>>     {
>>>>> -        bool_t fail = v->arch.pv_vcpu.ctrlreg[3] != c(ctrlreg[3]);
>>>>> +        unsigned long pfn = pagetable_get_pfn(v->arch.guest_table);
>>>>> +        bool_t fail;
>>>>>
>>>>> +        if ( !compat )
>>>>> +        {
>>>>> +            fail = xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[3];
>>>>>  #ifdef CONFIG_X86_64
>>>>> -        fail |= v->arch.pv_vcpu.ctrlreg[1] != c(ctrlreg[1]);
>>>>> +            if ( pagetable_is_null(v->arch.guest_table_user) )
>>>>> +                fail |= c.nat->ctrlreg[1] || !(flags & VGCF_in_kernel);
>>>>> +            else
>>>>> +            {
>>>>> +                pfn = pagetable_get_pfn(v->arch.guest_table_user);
>>>>> +                fail |= xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[1];
>>>>> +            }
>>>>> +        } else {
>>>>> +            l4_pgentry_t *l4tab = __va(pfn_to_paddr(pfn));
>>>>> +
>>>>> +            pfn = l4e_get_pfn(*l4tab);
>>>>> +            fail = compat_pfn_to_cr3(pfn) != c.cmp->ctrlreg[3];
>>>>>  #endif
>>>>> +        }
>>>>>
>>>>>         for ( i = 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
>>>>>             fail |= v->arch.pv_vcpu.gdt_frames[i] != c(gdt_frames[i]);
>>>>> @@ -907,15 +932,6 @@ int arch_set_info_guest(
>>>>>     v->arch.pv_vcpu.ctrlreg[0] &= X86_CR0_TS;
>>>>>     v->arch.pv_vcpu.ctrlreg[0] |= read_cr0() & ~X86_CR0_TS;
>>>>>
>>>>> -    init_int80_direct_trap(v);
>>>>> -
>>>>> -    /* IOPL privileges are virtualised. */
>>>>> -    v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
>>>>> -    v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
>>>>> -
>>>>> -    /* Ensure real hardware interrupts are enabled. */
>>>>> -    v->arch.user_regs.eflags |= X86_EFLAGS_IF;
>>>>> -
>>>>>     cr4 = v->arch.pv_vcpu.ctrlreg[4];
>>>>>     v->arch.pv_vcpu.ctrlreg[4] = cr4 ? pv_guest_cr4_fixup(v, cr4) :
>>>>>         real_cr4_to_pv_guest_cr4(mmu_cr4_features);
>>>>>
>>>>>
>>>> ok. this one works :). I ve tested with remus too :P. Can you please please
>>>> spin out a patch to be pushed into the repo ? thanks for the help.
>>>>
>>>> shriram
>>>>
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xensource.com
>>>> http://lists.xensource.com/xen-devel
>>>>
>>>>
>
>
>

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Keir Fraser]

On 11/05/2011 08:47, "Jan Beulich" <JBeulich@novell.com> wrote:

> The question is whether there are other inconsistencies lurking, and
> hence whether it wouldn't be better to mark a vCPU on which setting
> the context failed, not allowing it to resume or have its context
> obtained anymore. That appears quite drastic though - Keir, what's
> your opinion here?

Hm, I think overall we just need to get the error-path handling in this
function correct. I think it'll be a bit ugly to kludge around that.

 -- Keir

Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate)

[Jan Beulich]

[-- Attachment #1.1: Type: text/plain, Size: 250 bytes --]

>>> Shriram Rajagopalan  05/15/11 12:16 AM >>>
>This one works only for 64-bit domUs. 32bit domU (on 64bit dom0) fails with 
>usual EOPNOTSUPP.

Indeed, I see what I still missed. Will get you another one to try
hopefully later today.

Jan


[-- Attachment #1.2: HTML --]
[-- Type: text/html, Size: 558 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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