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

["Jan Beulich" <JBeulich@novell.com>]

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