(unknown)

[Mohammed Gamal <m.gamal005@gmail.com>]

laurent.vivier@bull.net, riel@surriel.com
Bcc: 
Subject: [RFC][PATCH] VMX: Add and enhance VMentry failure detection 
mechanism 
Reply-To: 

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 <switch_to_pm_20>:
    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().

Now enough talk, here is the patch

Signed-off-by: Guillaume Thouvenin <guillaume.thouvenin@ext.bull.net>
Signed-off-by: Laurent Vivier <laurent.vivier@bull.net>
Signed-off-by: Mohammed Gamal <m.gamal005@gmail.com>

---
 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];