On Tue, Sep 01, 2009 at 12:48:18PM +0200, Mohammed Gamal wrote:
> - Change returned handle_invalid_guest_state() to return relevant exit codes
> - Move triggering the emulation from vmx_vcpu_run() to vmx_handle_exit()
> - Return to userspace instead of repeatedly trying to emulate instructions that have already failed
>
> Signed-off-by: Mohammed Gamal <m.gamal005@xxxxxxxxx>
Mohammed,
The handle_invalid_guest_state loop is potentially problematic. It would
be more appropriate to use the __vcpu_run loop.
Can't you set vmx->emulation_required depending on the result
of one call to emulate_instruction and get rid of the while
(!guest_state_valid(vcpu)) loop?
> ---
> arch/x86/kvm/vmx.c | 44 ++++++++++++++++++++------------------------
> 1 files changed, 20 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 78101dd..6265098 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -107,7 +107,6 @@ struct vcpu_vmx {
> } rmode;
> int vpid;
> bool emulation_required;
> - enum emulation_result invalid_state_emulation_result;
>
> /* Support for vnmi-less CPUs */
> int soft_vnmi_blocked;
> @@ -3318,35 +3317,37 @@ static int handle_nmi_window(struct kvm_vcpu *vcpu)
> return 1;
> }
>
> -static void handle_invalid_guest_state(struct kvm_vcpu *vcpu)
> +static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> enum emulation_result err = EMULATE_DONE;
> -
> - local_irq_enable();
> - preempt_enable();
> + int ret = 1;
>
> while (!guest_state_valid(vcpu)) {
> err = emulate_instruction(vcpu, 0, 0, 0);
>
> - if (err == EMULATE_DO_MMIO)
> - break;
> + if (err == EMULATE_DO_MMIO) {
> + ret = 0;
> + goto out;
> + }
>
> if (err != EMULATE_DONE) {
> kvm_report_emulation_failure(vcpu, "emulation failure");
> - break;
> + vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
> + vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
> + ret = 0;
> + goto out;
> }
>
> if (signal_pending(current))
> - break;
> + goto out;
> if (need_resched())
> schedule();
> }
>
> - preempt_disable();
> - local_irq_disable();
> -
> - vmx->invalid_state_emulation_result = err;
> + vmx->emulation_required = 0;
> +out:
> + return ret;
> }
>
> /*
> @@ -3402,13 +3403,9 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
>
> trace_kvm_exit(exit_reason, kvm_rip_read(vcpu));
>
> - /* If we need to emulate an MMIO from handle_invalid_guest_state
> - * we just return 0 */
> - if (vmx->emulation_required && emulate_invalid_guest_state) {
> - if (guest_state_valid(vcpu))
> - vmx->emulation_required = 0;
> - return vmx->invalid_state_emulation_result != EMULATE_DO_MMIO;
> - }
> + /* If guest state is invalid, start emulating */
> + if (vmx->emulation_required && emulate_invalid_guest_state)
> + return handle_invalid_guest_state(vcpu);
>
> /* Access CR3 don't cause VMExit in paging mode, so we need
> * to sync with guest real CR3. */
> @@ -3603,11 +3600,10 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
> if (unlikely(!cpu_has_virtual_nmis() && vmx->soft_vnmi_blocked))
> vmx->entry_time = ktime_get();
>
> - /* Handle invalid guest state instead of entering VMX */
> - if (vmx->emulation_required && emulate_invalid_guest_state) {
> - handle_invalid_guest_state(vcpu);
> + /* Don't enter VMX if guest state is invalid, let the exit handler
> + start emulation until we arrive back to a valid state */
> + if (vmx->emulation_required && emulate_invalid_guest_state)
> return;
> - }
>
> if (test_bit(VCPU_REGS_RSP, (unsigned long *)&vcpu->arch.regs_dirty))
> vmcs_writel(GUEST_RSP, vcpu->arch.regs[VCPU_REGS_RSP]);
> --
> 1.6.0.4
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
