From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] VMX: Invalid guest state detection enhancements and bug fixes Date: Tue, 11 May 2010 12:24:30 +0300 Message-ID: <4BE9224E.5090305@redhat.com> References: <1273506685-2153-1-git-send-email-m.gamal005@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org To: Mohammed Gamal Return-path: Received: from mx1.redhat.com ([209.132.183.28]:59699 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754974Ab0EKJYc (ORCPT ); Tue, 11 May 2010 05:24:32 -0400 In-Reply-To: <1273506685-2153-1-git-send-email-m.gamal005@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: 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