Re: [PATCH] VMX: Invalid guest state detection enhancements and bug fixes

[Avi Kivity <avi@redhat.com>]

On 05/10/2010 06:51 PM, Mohammed Gamal wrote:
> - Correct unusable flag check on SS, DS, ES, FS, GS, and LDTR
> - Add rflags checks
> - Report failed instruction on emulation failure
>
>    

Please post as separate patches.

> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 777e00d..968384b 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2122,7 +2122,7 @@ static bool stack_segment_valid(struct kvm_vcpu *vcpu)
>   	ss_rpl = ss.selector&  SELECTOR_RPL_MASK;
>
>   	if (ss.unusable)
> -		return true;
> +		return false;
>    

Where does it say that ss must be usable?

>   	if (ss.type != 3&&  ss.type != 7)
>   		return false;
>   	if (!ss.s)
> @@ -2144,7 +2144,7 @@ static bool data_segment_valid(struct kvm_vcpu *vcpu, int seg)
>   	rpl = var.selector&  SELECTOR_RPL_MASK;
>
>   	if (var.unusable)
> -		return true;
> +		return false;
>   	if (!var.s)
>   		return false;
>   	if (!var.present)
>    

Ditto.

> @@ -2185,7 +2185,7 @@ static bool ldtr_valid(struct kvm_vcpu *vcpu)
>   	vmx_get_segment(vcpu,&ldtr, VCPU_SREG_LDTR);
>
>   	if (ldtr.unusable)
> -		return true;
> +		return false;
>   	if (ldtr.selector&  SELECTOR_TI_MASK)	/* TI = 1 */
>   		return false;
>   	if (ldtr.type != 2)
>    

Ditto.

> @@ -2207,6 +2207,27 @@ 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 (is_long_mode(vcpu))
> +		if (rflags&  X86_EFLAGS_VM)
> +			return false;	
> +#endif
>    

This is architecturally illegal, not just for vmx entries.  The check 
should be made when emulating setting rflags and in KVM_SET_REGS.

These checks should assume the state is architecturally legal, and only 
check if they are legal for vmx entries.

> +	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;
> +	}
> +
>    

Ditto.

> @@ -3559,6 +3581,7 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
>   		}
>
>   		if (err != EMULATE_DONE) {
> +			kvm_report_emulation_failure(vcpu, "invalid guest state handler");
>   			vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
>   			vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
>   			vcpu->run->internal.ndata = 0;
>    

Uneeded, userspace can report it.

-- 
error compiling committee.c: too many arguments to function