This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] arch_set_info_guest() producing inconsistent state on x86?

To: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] arch_set_info_guest() producing inconsistent state on x86?
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Tue, 29 Mar 2011 09:01:56 +0100
Delivery-date: Tue, 29 Mar 2011 01:02:13 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
The pv guest code path has a v->is_initialised check in the middle,
skipping d->vm_assist assignment, actual GDT construction, actual
page table setup, and the call to update_domain_wallclock_time().

If indeed do_domctl(XEN_DOMCTL_setvcpucontext,...) (the only
code path where v->is_initialised can be false on entry) was called
twice for a vCPU, this would minimally yield stored GDT settings
(in struct vcpu) out of sync with what is in the per-domain page
tables, thus causing arch_get_info_guest() to return values
not actually in use.

In the course of shrinking struct vcpu to below PAGE_SIZE I need
to split the embedded struct vcpu_guest_context in struct vcpu
anyway (as it is by itself larger than a page on x86-64), which
made this so far hidden inconsistency visible.

The question is whether it must be considered legal to issue
XEN_DOMCTL_setvcpucontext on an already initialized vCPU
in the first place. If that isn't necessary, the fix is simple
(just remove that check, and add one at the top rejecting
the attempt). If it is necessary, but the subsequent code is
all legal to be run a second time, simply removing the check
would again be the way to go (and in the splitting patch I'm
working on the copying of the GDT data then could be
dropped altogether, as the calls to set_gdt() would then
fully take care of this.

Otherwise, I would think that at least the GDT setup needs to
be moved ahead of the check, but I would then wonder
whether the remaining stuff really is correct to be skipped.


Xen-devel mailing list