From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re: xl save -c issues with Windows 7 Ultimate) Date: Fri, 13 May 2011 11:00:07 +0100 Message-ID: <4DCD1D4702000078000413D0@vpn.id2.novell.com> References: <1305016915.26692.261.camel@zakaz.uk.xensource.com> <4DC96FA50200007800040C69@vpn.id2.novell.com> <4DC97E000200007800040CFF@vpn.id2.novell.com> <4DCA5B3A0200007800040EC4@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: rshriram@cs.ubc.ca Cc: "xen-devel@lists.xensource.com" , Keir Fraser , Ian Campbell List-Id: xen-devel@lists.xenproject.org >>> On 11.05.11 at 21:50, Shriram Rajagopalan 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] =3D xen_pfn_to_cr3( > pagetable_get_pfn(v->arch.guest_table)); > #ifdef __x86_64__ > c.nat->ctrlreg[1] =3D > 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 =3D > __va(pagetable_get_paddr(v->arch.guest_table)); > c.cmp->ctrlreg[3] =3D compat_pfn_to_cr3(l4e_get_pfn(*l4e)); > } >=20 > 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; } =20 + init_int80_direct_trap(v); + + /* IOPL privileges are virtualised. */ + v->arch.pv_vcpu.iopl =3D (v->arch.user_regs.eflags >> 12) & 3; + v->arch.user_regs.eflags &=3D ~X86_EFLAGS_IOPL; + + /* Ensure real hardware interrupts are enabled. */ + v->arch.user_regs.eflags |=3D X86_EFLAGS_IF; + if ( !v->is_initialised ) { v->arch.pv_vcpu.ldt_base =3D c(ldt_base); @@ -863,10 +872,25 @@ int arch_set_info_guest( } else { - bool_t fail =3D v->arch.pv_vcpu.ctrlreg[3] !=3D c(ctrlreg[3]); + unsigned long pfn =3D pagetable_get_pfn(v->arch.guest_table); + bool_t fail; =20 + if ( !compat ) + { + fail =3D xen_pfn_to_cr3(pfn) !=3D c.nat->ctrlreg[3]; #ifdef CONFIG_X86_64 - fail |=3D v->arch.pv_vcpu.ctrlreg[1] !=3D c(ctrlreg[1]); + if ( pagetable_is_null(v->arch.guest_table_user) ) + fail |=3D c.nat->ctrlreg[1] || !(flags & VGCF_in_kernel); + else + { + pfn =3D pagetable_get_pfn(v->arch.guest_table_user); + fail |=3D xen_pfn_to_cr3(pfn) !=3D c.nat->ctrlreg[1]; + } +#endif + } +#ifdef CONFIG_COMPAT + else + fail =3D compat_pfn_to_cr3(pfn) !=3D c.cmp->ctrlreg[3]; #endif =20 for ( i =3D 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] &=3D X86_CR0_TS; v->arch.pv_vcpu.ctrlreg[0] |=3D read_cr0() & ~X86_CR0_TS; =20 - init_int80_direct_trap(v); - - /* IOPL privileges are virtualised. */ - v->arch.pv_vcpu.iopl =3D (v->arch.user_regs.eflags >> 12) & 3; - v->arch.user_regs.eflags &=3D ~X86_EFLAGS_IOPL; - - /* Ensure real hardware interrupts are enabled. */ - v->arch.user_regs.eflags |=3D X86_EFLAGS_IF; - cr4 =3D v->arch.pv_vcpu.ctrlreg[4]; v->arch.pv_vcpu.ctrlreg[4] =3D cr4 ? pv_guest_cr4_fixup(v, cr4) : real_cr4_to_pv_guest_cr4(mmu_cr4_features); Jan