From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mohammed Gamal Subject: [RFC][PATCH] VMX: Add enhanced mechanism to detect vmentry failures Date: Tue, 29 Jul 2008 00:37:36 +0300 Message-ID: <20080728213736.GA7783@mohd-laptop> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: avi@qumranet.com, guillaume.thouvenin@ext.bull.net, laurent.vivier@ext.bull.net, riel@surriel.com To: kvm@vger.kernel.org Return-path: Received: from fg-out-1718.google.com ([72.14.220.154]:8386 "EHLO fg-out-1718.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759750AbYG1ViE (ORCPT ); Mon, 28 Jul 2008 17:38:04 -0400 Received: by fg-out-1718.google.com with SMTP id 19so2188638fgg.17 for ; Mon, 28 Jul 2008 14:38:03 -0700 (PDT) Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: This patch is *not* meant to be merged. This patch fixes the random crashes with gfxboot and it doesn't crash anymore at random instructions. It mainly does two things: 1- It handles all possible exit reasons before exiting for VMX failures 2- It handles vmentry failures avoiding external interrupts However, while this patch allows booting FreeDOS with HIMEM with no problems. It does occasionally crash with gfxboot at RIP 6e29, looking at the gfxboot code the instructions causing the crash is as follows: 00006e10 : 6e10: 66 b8 20 00 mov $0x20,%ax 6e14: 8e d8 mov %eax,%ds 6e16: 8c d0 mov %ss,%eax 6e18: 81 e4 ff ff 00 00 and $0xffff,%esp 6e1e: c1 e0 04 shl $0x4,%eax 6e21: 01 c4 add %eax,%esp 6e23: 66 b8 08 00 mov $0x8,%ax 6e27: 8e d0 mov %eax,%ss 6e29: 8e c0 mov %eax,%es 6e2b: 8e e0 mov %eax,%fs 6e2d: 8e e8 mov %eax,%gs 6e2f: 58 pop %eax 6e30: 66 9d popfw 6e32: 66 c3 retw So apparently to fix the problem we need to add other guest state checks -namely for ES, FS, GS- to invalid_guest_state(). Signed-off-by: Guillaume Thouvenin Signed-off-by: Laurent Vivier Signed-off-by: Mohammed Gamal --- arch/x86/kvm/vmx.c | 116 +++++++++++++++++++++++++++++++++++++++++--- arch/x86/kvm/vmx.h | 3 + include/asm-x86/kvm_host.h | 1 + 3 files changed, 112 insertions(+), 8 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index c4510fe..b438f94 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -1316,7 +1316,8 @@ static void enter_pmode(struct kvm_vcpu *vcpu) fix_pmode_dataseg(VCPU_SREG_GS, &vcpu->arch.rmode.gs); fix_pmode_dataseg(VCPU_SREG_FS, &vcpu->arch.rmode.fs); - vmcs_write16(GUEST_SS_SELECTOR, 0); + if (vcpu->arch.rmode_failed) + vmcs_write16(GUEST_SS_SELECTOR, 0); vmcs_write32(GUEST_SS_AR_BYTES, 0x93); vmcs_write16(GUEST_CS_SELECTOR, @@ -2708,6 +2709,93 @@ static int handle_nmi_window(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) return 1; } +static int invalid_guest_state(struct kvm_vcpu *vcpu, + struct kvm_run *kvm_run, u32 failure_reason) +{ + u16 ss, cs; + u8 opcodes[4]; + unsigned long rip = kvm_rip_read(vcpu); + unsigned long rip_linear; + + ss = vmcs_read16(GUEST_SS_SELECTOR); + cs = vmcs_read16(GUEST_CS_SELECTOR); + + if ((ss & 0x03) != (cs & 0x03)) { + int err; + rip_linear = rip + vmx_get_segment_base(vcpu, VCPU_SREG_CS); + emulator_read_std(rip_linear, (void *)opcodes, 4, vcpu); + err = emulate_instruction(vcpu, kvm_run, 0, 0, 0); + switch (err) { + case EMULATE_DONE: + return 1; + case EMULATE_DO_MMIO: + printk(KERN_INFO "mmio?\n"); + return 0; + default: + /* HACK: If we can not emulate the instruction + * we write a sane value on SS to pass sanity + * checks. The good thing to do is to emulate the + * instruction */ + kvm_report_emulation_failure(vcpu, "vmentry failure"); + printk(KERN_INFO " => Quit real mode emulation\n"); + vcpu->arch.rmode_failed = 1; + vmcs_write16(GUEST_SS_SELECTOR, 0); + return 1; + } + } + + kvm_run->exit_reason = KVM_EXIT_UNKNOWN; + kvm_run->hw.hardware_exit_reason = failure_reason; + printk(KERN_INFO "Failed to handle invalid guest state\n"); + return 0; +} + +/* + * Should be replaced with exit handlers for each individual case + */ +static int handle_vmentry_failure(struct kvm_vcpu *vcpu, + struct kvm_run *kvm_run, + u32 failure_reason) +{ + unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION); + switch (failure_reason) { + case EXIT_REASON_INVALID_GUEST_STATE: + return invalid_guest_state(vcpu, kvm_run, failure_reason); + case EXIT_REASON_MSR_LOADING: + printk("VMentry failure caused by MSR entry %ld loading.\n", + exit_qualification); + printk(" ... Not handled\n"); + break; + case EXIT_REASON_MACHINE_CHECK: + printk("VMentry failure caused by machine check.\n"); + printk(" ... Not handled\n"); + break; + default: + printk("reason not known yet!\n"); + break; + } + return 0; +} + +static int handle_invalid_guest_state(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) +{ + int rc; + u32 exit_reason = vmcs_read32(VM_EXIT_REASON); + + /* + * Disable interrupts to avoid occasional vmexits while + * handling vmentry failures + */ + spin_lock_irq(&vmx_vpid_lock); + if(exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY) + exit_reason &= ~VMX_EXIT_REASONS_FAILED_VMENTRY; + + rc = invalid_guest_state(vcpu, kvm_run, exit_reason); + spin_unlock_irq(&vmx_vpid_lock); + + return rc; +} + /* * The exit handlers return 1 if the exit was handled fully and guest execution * may resume. Otherwise they set the kvm_run parameter to indicate what needs @@ -2733,6 +2821,7 @@ static int (*kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu, [EXIT_REASON_WBINVD] = handle_wbinvd, [EXIT_REASON_TASK_SWITCH] = handle_task_switch, [EXIT_REASON_EPT_VIOLATION] = handle_ept_violation, + [EXIT_REASON_INVALID_GUEST_STATE] = handle_invalid_guest_state, }; static const int kvm_vmx_max_exit_handlers = @@ -2758,21 +2847,32 @@ static int kvm_handle_exit(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) ept_load_pdptrs(vcpu); } - if (unlikely(vmx->fail)) { - kvm_run->exit_reason = KVM_EXIT_FAIL_ENTRY; - kvm_run->fail_entry.hardware_entry_failure_reason - = vmcs_read32(VM_INSTRUCTION_ERROR); - return 0; - } - if ((vectoring_info & VECTORING_INFO_VALID_MASK) && (exit_reason != EXIT_REASON_EXCEPTION_NMI && exit_reason != EXIT_REASON_EPT_VIOLATION)) printk(KERN_WARNING "%s: unexpected, valid vectoring info and " "exit reason is 0x%x\n", __func__, exit_reason); + + /* + * Instead of using handle_vmentry_failure(), just clear + * the vmentry failure bit and leave it to the exit handlers + * to deal with the specific exit reason. + * The exit handlers other than invalid guest state handler + * will be added later. + */ + if ((exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY)) + exit_reason &= ~VMX_EXIT_REASONS_FAILED_VMENTRY; + + + /* Handle all possible exits first, handle failure later. */ if (exit_reason < kvm_vmx_max_exit_handlers && kvm_vmx_exit_handlers[exit_reason]) return kvm_vmx_exit_handlers[exit_reason](vcpu, kvm_run); + else if(unlikely(vmx->fail)) { + kvm_run->exit_reason = KVM_EXIT_FAIL_ENTRY; + kvm_run->fail_entry.hardware_entry_failure_reason + = vmcs_read32(VM_INSTRUCTION_ERROR); + } else { kvm_run->exit_reason = KVM_EXIT_UNKNOWN; kvm_run->hw.hardware_exit_reason = exit_reason; diff --git a/arch/x86/kvm/vmx.h b/arch/x86/kvm/vmx.h index 0c22e5f..cf8b771 100644 --- a/arch/x86/kvm/vmx.h +++ b/arch/x86/kvm/vmx.h @@ -239,7 +239,10 @@ enum vmcs_field { #define EXIT_REASON_IO_INSTRUCTION 30 #define EXIT_REASON_MSR_READ 31 #define EXIT_REASON_MSR_WRITE 32 +#define EXIT_REASON_INVALID_GUEST_STATE 33 +#define EXIT_REASON_MSR_LOADING 34 #define EXIT_REASON_MWAIT_INSTRUCTION 36 +#define EXIT_REASON_MACHINE_CHECK 41 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43 #define EXIT_REASON_APIC_ACCESS 44 #define EXIT_REASON_EPT_VIOLATION 48 diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h index 0b6b996..422d7c2 100644 --- a/include/asm-x86/kvm_host.h +++ b/include/asm-x86/kvm_host.h @@ -294,6 +294,7 @@ struct kvm_vcpu_arch { } tr, es, ds, fs, gs; } rmode; int halt_request; /* real mode on Intel only */ + int rmode_failed; int cpuid_nent; struct kvm_cpuid_entry2 cpuid_entries[KVM_MAX_CPUID_ENTRIES];