From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] VMX: Return to userspace on invalid state emulation failure Date: Wed, 26 Aug 2009 13:11:45 +0300 Message-ID: <4A950A61.4090606@redhat.com> References: <1251153439-17078-1-git-send-email-m.gamal005@gmail.com> <4A94FA2D.7020206@redhat.com> <52d4a3890908260307y4c5e4836g4b25e0426b6f722f@mail.gmail.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: Mohammed Gamal Return-path: Received: from mx1.redhat.com ([209.132.183.28]:24949 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756333AbZHZKLr (ORCPT ); Wed, 26 Aug 2009 06:11:47 -0400 In-Reply-To: <52d4a3890908260307y4c5e4836g4b25e0426b6f722f@mail.gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On 08/26/2009 01:07 PM, Mohammed Gamal wrote: > On Wed, Aug 26, 2009 at 12:02 PM, Avi Kivity wrote: > >> On 08/25/2009 01:37 AM, Mohammed Gamal wrote: >> >>> Return to userspace instead of repeatedly trying to emulate >>> instructions that have already failed >>> >>> Signed-off-by: Mohammed Gamal >>> --- >>> arch/x86/kvm/vmx.c | 6 +++++- >>> 1 files changed, 5 insertions(+), 1 deletions(-) >>> >>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>> index 6b57eed..c559bb7 100644 >>> --- a/arch/x86/kvm/vmx.c >>> +++ b/arch/x86/kvm/vmx.c >>> @@ -3337,6 +3337,8 @@ static void handle_invalid_guest_state(struct >>> kvm_vcpu *vcpu) >>> >>> if (err != EMULATE_DONE) { >>> kvm_report_emulation_failure(vcpu, "emulation >>> failure"); >>> + vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; >>> + vcpu->run->internal.suberror = >>> KVM_INTERNAL_ERROR_EMULATION; >>> break; >>> } >>> >>> @@ -3607,7 +3609,9 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu) >>> vmx->entry_time = ktime_get(); >>> >>> /* Handle invalid guest state instead of entering VMX */ >>> - if (vmx->emulation_required&& emulate_invalid_guest_state) { >>> + if (vmx->emulation_required&& emulate_invalid_guest_state >>> +&& !(vcpu->run->exit_reason == KVM_EXIT_INTERNAL_ERROR&& >>> + vcpu->run->internal.suberror == >>> KVM_INTERNAL_ERROR_EMULATION)) { >>> handle_invalid_guest_state(vcpu); >>> return; >>> } >>> >>> >> Still suffers from the same problem. You don't always update >> vcpu->run->exit_reason, so you can't test it. Best to return a value from >> handle_invalid_guest_state() (the standard return codes for exit handlers >> are 1 for return-to-guest, 0 for return-to-host, and -errno to return with >> an error). >> >> > I was thinking of the same idea since I was also concerned about > vcpu->run->exit_reason not being updated. But how can we interpret the > return values of handle_invalid_guest_state() inside vmx_vcpu_run() > since it doesn't have a return value. Or would it be better to move > handle_invalid_guest_state() to the standard vmx exit handlers? > We can move the call to vmx_handle_exit(). We have a check for emulate_invalid_guest_state there anyway. I don't think it should be a standard exit handler since there is no exit_reason for it. -- error compiling committee.c: too many arguments to function