Module Name:    src
Committed By:   maxv
Date:           Thu Feb 14 09:37:32 UTC 2019

Modified Files:
        src/sys/dev/nvmm/x86: nvmm_x86_vmx.c

Log Message:
On AMD, the segments have a simple "present" bit. On Intel however there
is an extra "unusable" bit, which has a twisted meaning. We can't just
ignore this bit, because when unset, the CPU performs extra checks on the
other attributes, which may cause VMENTRY to fail and the guest to be
killed.

Typically, on Qemu, some guests like Windows XP trigger two consecutive
getstate+setstate calls, and while processing them, we end up wrongfully
removing the "unusable" bits that were previously set.

Fix that by forcing "unusable = !present". Each hypervisor I could check
does something different, but this seems to be the least problematic
solution for now.

While here, the fields of vmx_guest_segs are VMX indexes, so they should
be uint64_t (no functional change).


To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/sys/dev/nvmm/x86/nvmm_x86_vmx.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/nvmm/x86/nvmm_x86_vmx.c
diff -u src/sys/dev/nvmm/x86/nvmm_x86_vmx.c:1.1 src/sys/dev/nvmm/x86/nvmm_x86_vmx.c:1.2
--- src/sys/dev/nvmm/x86/nvmm_x86_vmx.c:1.1	Wed Feb 13 16:03:16 2019
+++ src/sys/dev/nvmm/x86/nvmm_x86_vmx.c	Thu Feb 14 09:37:31 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: nvmm_x86_vmx.c,v 1.1 2019/02/13 16:03:16 maxv Exp $	*/
+/*	$NetBSD: nvmm_x86_vmx.c,v 1.2 2019/02/14 09:37:31 maxv Exp $	*/
 
 /*
  * Copyright (c) 2018 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nvmm_x86_vmx.c,v 1.1 2019/02/13 16:03:16 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nvmm_x86_vmx.c,v 1.2 2019/02/14 09:37:31 maxv Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -674,9 +674,9 @@ struct vmx_cpudata {
 };
 
 static const struct {
-	uint16_t selector;
-	uint16_t attrib;
-	uint32_t limit;
+	uint64_t selector;
+	uint64_t attrib;
+	uint64_t limit;
 	uint64_t base;
 } vmx_guest_segs[NVMM_X64_NSEG] = {
 	[NVMM_X64_SEG_ES] = {
@@ -2113,7 +2113,8 @@ vmx_vcpu_setstate_seg(struct nvmm_x64_st
 	    __SHIFTIN(segs[idx].attrib.avl, VMX_SEG_ATTRIB_AVL) |
 	    __SHIFTIN(segs[idx].attrib.lng, VMX_SEG_ATTRIB_LONG) |
 	    __SHIFTIN(segs[idx].attrib.def32, VMX_SEG_ATTRIB_DEF32) |
-	    __SHIFTIN(segs[idx].attrib.gran, VMX_SEG_ATTRIB_GRAN);
+	    __SHIFTIN(segs[idx].attrib.gran, VMX_SEG_ATTRIB_GRAN) |
+	    (!segs[idx].attrib.p ? VMX_SEG_ATTRIB_UNUSABLE : 0);
 
 	if (idx != NVMM_X64_SEG_GDT && idx != NVMM_X64_SEG_IDT) {
 		vmx_vmwrite(vmx_guest_segs[idx].selector, segs[idx].selector);
@@ -2142,6 +2143,9 @@ vmx_vcpu_getstate_seg(struct nvmm_x64_st
 	segs[idx].attrib.lng = __SHIFTOUT(attrib, VMX_SEG_ATTRIB_LONG);
 	segs[idx].attrib.def32 = __SHIFTOUT(attrib, VMX_SEG_ATTRIB_DEF32);
 	segs[idx].attrib.gran = __SHIFTOUT(attrib, VMX_SEG_ATTRIB_GRAN);
+	if (attrib & VMX_SEG_ATTRIB_UNUSABLE) {
+		segs[idx].attrib.p = 0;
+	}
 }
 
 static inline bool

Reply via email to