From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH] Add GDT, IDT and RFLAGS guest state validity checks
Date: Tue, 22 Sep 2009 10:13:26 +0300
Message-ID: <4AB87916.3000904@redhat.com>
References: <1253547235-25348-1-git-send-email-m.gamal005@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: mtosatti@redhat.com, kvm@vger.kernel.org
To: Mohammed Gamal <m.gamal005@gmail.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:27961 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751597AbZIVHN0 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 22 Sep 2009 03:13:26 -0400
In-Reply-To: <1253547235-25348-1-git-send-email-m.gamal005@gmail.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 09/21/2009 06:33 PM, Mohammed Gamal wrote:
> With emulate_invalid_guest_state=1 Windows XP exits with an invalid guest state
> due to rflags not being in a VMX-compliant state. This patch fixes this issue,
> although Windows XP doesn't boot yet with invalid state emulation on.
>
> Also added GDT and IDT checks while we're at it.
>
> Signed-off-by: Mohammed Gamal<m.gamal005@gmail.com>
> ---
>   arch/x86/kvm/vmx.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 56 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 3fe0d42..eaec4a5 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2022,6 +2022,23 @@ static bool ldtr_valid(struct kvm_vcpu *vcpu)
>   	return true;
>   }
>
> +static bool gdtr_idtr_valid(struct kvm_vcpu *vcpu)
> +{
> +	struct descriptor_table gdt;
> +	struct descriptor_table idt;
> +
> +	vmx_get_gdt(vcpu,&gdt);
> +	vmx_get_idt(vcpu,&idt);
> +
> +	if (gdt.limit&  0xffff0000)
> +		return false;
> +
> +	if (idt.limit&  0xffff0000)
> +		return false;
> +
> +	return true;
> +}
>    

gdt and idt limits cannot be > 0xffff, since the intstructions to load 
them always use a 16 bit quantity.

> +
>   static bool cs_ss_rpl_check(struct kvm_vcpu *vcpu)
>   {
>   	struct kvm_segment cs, ss;
> @@ -2033,6 +2050,41 @@ static bool cs_ss_rpl_check(struct kvm_vcpu *vcpu)
>   		 (ss.selector&  SELECTOR_RPL_MASK));
>   }
>
> +static bool rflags_valid(struct kvm_vcpu *vcpu)
> +{
> +	unsigned long rflags;
> +	u32 entry_intr_info;
> +
> +	rflags = vmcs_readl(GUEST_RFLAGS);
> +	entry_intr_info = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
> +#ifdef CONFIG_X86_64
> +	if (rflags&  0xffffffffffc00000)
> +		return false;
> +	if (is_long_mode(vcpu))
> +		if (rflags&  X86_EFLAGS_VM)
> +			return false;	
> +#else
> +	if (rflags&  0xffc00000)
> +		return false;
> +#endif
> +	if (rflags&  0x8000)
> +		return false;
> +	if (rflags&  0x20)
> +		return false;
> +	if (rflags&  0x8)
> +		return false;
> +	if (!(rflags&  0x2))
> +		return false;
> +
> +	if ((entry_intr_info&  INTR_INFO_INTR_TYPE_MASK) == INTR_TYPE_EXT_INTR
> +		&&  (entry_intr_info&  INTR_INFO_VALID_MASK)) {
> +		if (!(rflags&  X86_EFLAGS_IF))
> +			return false;
> +	}
> +
> +	return true;
> +}
>    

It's really difficult to tell what you are doing here since you're using 
numbers instead of symbolic constants.  But I think some of these are 
generally illegal.  !guest_state_valid() means "the state is valid for 
x86 but not valid for vmx entry"; if it's generally invalid all bets are 
off.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.