From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@novell.com>
Subject: Re: xl/xm save -c fails - set_vcpucontext EOPNOTSUPP (was Re:
	xl save -c issues with Windows 7 Ultimate)
Date: Tue, 10 May 2011 16:02:29 +0100
Message-ID: <4DC96FA50200007800040C69@vpn.id2.novell.com>
References: <BANLkTi=a4=uNLYSA+0FEX+oX=iBmStn3aA@mail.gmail.com>
	<1305016915.26692.261.camel@zakaz.uk.xensource.com>
	<BANLkTinURoEKajLD54zQQVNf1=7uKaXb3A@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: quoted-printable
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <BANLkTinURoEKajLD54zQQVNf1=7uKaXb3A@mail.gmail.com>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=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" <xen-devel@lists.xensource.com>, Ian Campbell <Ian.Campbell@citrix.com>
List-Id: xen-devel@lists.xenproject.org

>>> 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>wr=
ote:
>> 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 =3D v->arch.pv_vcpu.ctrlreg[3] !=3D c(ctrlreg[3]);
>=20
> #ifdef CONFIG_X86_64
>         fail |=3D v->arch.pv_vcpu.ctrlreg[1] !=3D c(ctrlreg[1]);
> #endif
>=20
>         for ( i =3D 0; i < ARRAY_SIZE(v->arch.pv_vcpu.gdt_frames); ++i )
>             fail |=3D v->arch.pv_vcpu.gdt_frames[i] !=3D c(gdt_frames[i])=
;
>         fail |=3D v->arch.pv_vcpu.gdt_ents !=3D c(gdt_ents);
>=20
>         fail |=3D v->arch.pv_vcpu.ldt_base !=3D c(ldt_base);
>         fail |=3D v->arch.pv_vcpu.ldt_ents !=3D c(ldt_ents);
>=20
>         if ( fail )
>            return -EOPNOTSUPP;
>     }
>=20
> 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
>=20
> 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