Re: [PATCH] Fix up vmx_set_segment for booting older guests.

[Avi Kivity <avi@redhat.com>]

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.