| Message ID | 1251153439-17078-1-git-send-email-m.gamal005@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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<m.gamal005@gmail.com> > --- > 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).
On Wed, Aug 26, 2009 at 12:02 PM, Avi Kivity<avi@redhat.com> 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<m.gamal005@gmail.com> >> --- >> Â 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? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/26/2009 01:07 PM, Mohammed Gamal wrote: > On Wed, Aug 26, 2009 at 12:02 PM, Avi Kivity<avi@redhat.com> 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<m.gamal005@gmail.com> >>> --- >>> 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.
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; }
Return to userspace instead of repeatedly trying to emulate instructions that have already failed Signed-off-by: Mohammed Gamal <m.gamal005@gmail.com> --- arch/x86/kvm/vmx.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)