From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] Fix up vmx_set_segment for booting older guests. Date: Tue, 20 Oct 2009 17:10:41 +0900 Message-ID: <4ADD7081.8030409@redhat.com> References: <1256025040-4941-1-git-send-email-clalance@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Chris Lalancette Return-path: Received: from mx1.redhat.com ([209.132.183.28]:12925 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750885AbZJTIKl (ORCPT ); Tue, 20 Oct 2009 04:10:41 -0400 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n9K8Ajct020338 for ; Tue, 20 Oct 2009 04:10:45 -0400 In-Reply-To: <1256025040-4941-1-git-send-email-clalance@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 10/20/2009 04:50 PM, Chris Lalancette wrote: > If a guest happens to be unlucky enough to use an address > such as 0xc0000000 in the CS base address field, the next attempt > to VM enter will fail. This is because the vmcs_writel() that > writes the base address into the VMCS will sign-extend the field > to 64-bits, and the Intel manual states that bits 63:32 of this > field *must* be 0. Use vmcs_write32() where appropriate. > This fixes booting of an absolutely ancient Red Hat Linux 5.2 > (not Enterprise Linux!) guest. > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 364263a..311afd4 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -1846,7 +1846,22 @@ static void vmx_set_segment(struct kvm_vcpu *vcpu, > vmx->rmode.tr.ar = vmx_segment_access_rights(var); > return; > } > - vmcs_writel(sf->base, var->base); > + > + /* Intel 64 and IA-32 Architecture Software Developer's Manual Vol. 3b, > + * section 22.3.1.2 states that VMENTRY will fail if bits 63:32 of the > + * base address for CS, SS, DS, ES are not 0 and the register is usable. > + * > + * If var->base happens to have bit 31 set, then it will get sign > + * extended on the vmcs_writel(), causing this check to fail. Make > + * sure to use the 32-bit version where appropriate. > + */ > + if (sf->base == GUEST_CS_BASE || > + ((~sf->ar_bytes& 0x00010000)&& (sf->base == GUEST_SS_BASE || > + sf->base == GUEST_DS_BASE || > + sf->base == GUEST_ES_BASE))) > + vmcs_write32(sf->base, var->base); > This will leave high bits untouched, so if any were set, this will fail. > + else > + vmcs_writel(sf->base, var->base); > vmcs_write32(sf->limit, var->limit); > I think the correct fix is to zero extend in vmcs_writel() rather than here. But as far as I can tell, it already does. Where does the sign extension occur? Perhaps in userspace? -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.