Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 23 Feb 2014 01:34:41 +0000 (UTC)
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r262352 - stable/10/sys/amd64/vmm/intel
Message-ID:  <201402230134.s1N1YfDp056165@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: jhb
Date: Sun Feb 23 01:34:40 2014
New Revision: 262352
URL: http://svnweb.freebsd.org/changeset/base/262352

Log:
  MFC 259542:
  Use vmcs_read() and vmcs_write() in preference to vmread() and vmwrite()
  respectively. The vmcs_xxx() functions provide inline error checking of
  all accesses to the VMCS.

Modified:
  stable/10/sys/amd64/vmm/intel/vmcs.c
  stable/10/sys/amd64/vmm/intel/vmcs.h
  stable/10/sys/amd64/vmm/intel/vmx.c
  stable/10/sys/amd64/vmm/intel/vmx_genassym.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/amd64/vmm/intel/vmcs.c
==============================================================================
--- stable/10/sys/amd64/vmm/intel/vmcs.c	Sun Feb 23 01:27:22 2014	(r262351)
+++ stable/10/sys/amd64/vmm/intel/vmcs.c	Sun Feb 23 01:34:40 2014	(r262352)
@@ -41,8 +41,8 @@ __FBSDID("$FreeBSD$");
 #include <machine/segments.h>
 #include <machine/vmm.h>
 #include "vmm_host.h"
-#include "vmcs.h"
 #include "vmx_cpufunc.h"
+#include "vmcs.h"
 #include "ept.h"
 #include "vmx.h"
 
@@ -454,19 +454,6 @@ done:
 	return (error);
 }
 
-uint64_t
-vmcs_read(uint32_t encoding)
-{
-	int error;
-	uint64_t val;
-
-	error = vmread(encoding, &val);
-	if (error != 0)
-		panic("vmcs_read(%u) error %d", encoding, error);
-
-	return (val);
-}
-
 #ifdef DDB
 extern int vmxon_enabled[];
 

Modified: stable/10/sys/amd64/vmm/intel/vmcs.h
==============================================================================
--- stable/10/sys/amd64/vmm/intel/vmcs.h	Sun Feb 23 01:27:22 2014	(r262351)
+++ stable/10/sys/amd64/vmm/intel/vmcs.h	Sun Feb 23 01:34:40 2014	(r262352)
@@ -58,7 +58,26 @@ int	vmcs_getdesc(struct vmcs *vmcs, int 
 		     struct seg_desc *desc);
 int	vmcs_setdesc(struct vmcs *vmcs, int ident,
 		     struct seg_desc *desc);
-uint64_t vmcs_read(uint32_t encoding);
+
+static __inline uint64_t
+vmcs_read(uint32_t encoding)
+{
+	int error;
+	uint64_t val;
+
+	error = vmread(encoding, &val);
+	KASSERT(error == 0, ("vmcs_read(%u) error %d", encoding, error));
+	return (val);
+}
+
+static __inline void
+vmcs_write(uint32_t encoding, uint64_t val)
+{
+	int error;
+
+	error = vmwrite(encoding, val);
+	KASSERT(error == 0, ("vmcs_write(%u) error %d", encoding, error));
+}
 
 #define	vmexit_instruction_length()	vmcs_read(VMCS_EXIT_INSTRUCTION_LENGTH)
 #define	vmcs_guest_rip()		vmcs_read(VMCS_GUEST_RIP)

Modified: stable/10/sys/amd64/vmm/intel/vmx.c
==============================================================================
--- stable/10/sys/amd64/vmm/intel/vmx.c	Sun Feb 23 01:27:22 2014	(r262351)
+++ stable/10/sys/amd64/vmm/intel/vmx.c	Sun Feb 23 01:34:40 2014	(r262352)
@@ -324,9 +324,8 @@ vmx_setjmp_trace(struct vmx *vmx, int vc
 	VCPU_CTR2((vmx)->vm, (vcpu), "setjmp return code %s(%d)",
 		 vmx_setjmp_rc2str(rc), rc);
 
-	host_rsp = host_rip = ~0;
-	vmread(VMCS_HOST_RIP, &host_rip);
-	vmread(VMCS_HOST_RSP, &host_rsp);
+	host_rip = vmcs_read(VMCS_HOST_RIP);
+	host_rsp = vmcs_read(VMCS_HOST_RSP);
 	VCPU_CTR2((vmx)->vm, (vcpu), "vmcs host_rip 0x%016lx, host_rsp %#lx",
 		 host_rip, host_rsp);
 
@@ -917,10 +916,10 @@ vmx_astpending_trace(struct vmx *vmx, in
 #endif
 }
 
-static int
+static void
 vmx_set_pcpu_defaults(struct vmx *vmx, int vcpu)
 {
-	int error, lastcpu;
+	int lastcpu;
 	struct vmxstate *vmxstate;
 	struct invvpid_desc invvpid_desc = { 0 };
 
@@ -928,24 +927,14 @@ vmx_set_pcpu_defaults(struct vmx *vmx, i
 	lastcpu = vmxstate->lastcpu;
 	vmxstate->lastcpu = curcpu;
 
-	if (lastcpu == curcpu) {
-		error = 0;
-		goto done;
-	}
+	if (lastcpu == curcpu)
+		return;
 
 	vmm_stat_incr(vmx->vm, vcpu, VCPU_MIGRATIONS, 1);
 
-	error = vmwrite(VMCS_HOST_TR_BASE, vmm_get_host_trbase());
-	if (error != 0)
-		goto done;
-
-	error = vmwrite(VMCS_HOST_GDTR_BASE, vmm_get_host_gdtrbase());
-	if (error != 0)
-		goto done;
-
-	error = vmwrite(VMCS_HOST_GS_BASE, vmm_get_host_gsbase());
-	if (error != 0)
-		goto done;
+	vmcs_write(VMCS_HOST_TR_BASE, vmm_get_host_trbase());
+	vmcs_write(VMCS_HOST_GDTR_BASE, vmm_get_host_gdtrbase());
+	vmcs_write(VMCS_HOST_GS_BASE, vmm_get_host_gsbase());
 
 	/*
 	 * If we are using VPIDs then invalidate all mappings tagged with 'vpid'
@@ -966,18 +955,6 @@ vmx_set_pcpu_defaults(struct vmx *vmx, i
 		invvpid_desc.vpid = vmxstate->vpid;
 		invvpid(INVVPID_TYPE_SINGLE_CONTEXT, invvpid_desc);
 	}
-done:
-	return (error);
-}
-
-static void 
-vm_exit_update_rip(struct vm_exit *vmexit)
-{
-	int error;
-
-	error = vmwrite(VMCS_GUEST_RIP, vmexit->rip + vmexit->inst_length);
-	if (error)
-		panic("vmx_run: error %d writing to VMCS_GUEST_RIP", error);
 }
 
 /*
@@ -988,66 +965,45 @@ CTASSERT((PROCBASED_CTLS_ONE_SETTING & P
 static void __inline
 vmx_set_int_window_exiting(struct vmx *vmx, int vcpu)
 {
-	int error;
 
 	vmx->cap[vcpu].proc_ctls |= PROCBASED_INT_WINDOW_EXITING;
-
-	error = vmwrite(VMCS_PRI_PROC_BASED_CTLS, vmx->cap[vcpu].proc_ctls);
-	if (error)
-		panic("vmx_set_int_window_exiting: vmwrite error %d", error);
+	vmcs_write(VMCS_PRI_PROC_BASED_CTLS, vmx->cap[vcpu].proc_ctls);
 }
 
 static void __inline
 vmx_clear_int_window_exiting(struct vmx *vmx, int vcpu)
 {
-	int error;
 
 	vmx->cap[vcpu].proc_ctls &= ~PROCBASED_INT_WINDOW_EXITING;
-
-	error = vmwrite(VMCS_PRI_PROC_BASED_CTLS, vmx->cap[vcpu].proc_ctls);
-	if (error)
-		panic("vmx_clear_int_window_exiting: vmwrite error %d", error);
+	vmcs_write(VMCS_PRI_PROC_BASED_CTLS, vmx->cap[vcpu].proc_ctls);
 }
 
 static void __inline
 vmx_set_nmi_window_exiting(struct vmx *vmx, int vcpu)
 {
-	int error;
 
 	vmx->cap[vcpu].proc_ctls |= PROCBASED_NMI_WINDOW_EXITING;
-
-	error = vmwrite(VMCS_PRI_PROC_BASED_CTLS, vmx->cap[vcpu].proc_ctls);
-	if (error)
-		panic("vmx_set_nmi_window_exiting: vmwrite error %d", error);
+	vmcs_write(VMCS_PRI_PROC_BASED_CTLS, vmx->cap[vcpu].proc_ctls);
 }
 
 static void __inline
 vmx_clear_nmi_window_exiting(struct vmx *vmx, int vcpu)
 {
-	int error;
 
 	vmx->cap[vcpu].proc_ctls &= ~PROCBASED_NMI_WINDOW_EXITING;
-
-	error = vmwrite(VMCS_PRI_PROC_BASED_CTLS, vmx->cap[vcpu].proc_ctls);
-	if (error)
-		panic("vmx_clear_nmi_window_exiting: vmwrite error %d", error);
+	vmcs_write(VMCS_PRI_PROC_BASED_CTLS, vmx->cap[vcpu].proc_ctls);
 }
 
 static int
 vmx_inject_nmi(struct vmx *vmx, int vcpu)
 {
-	int error;
 	uint64_t info, interruptibility;
 
 	/* Bail out if no NMI requested */
 	if (!vm_nmi_pending(vmx->vm, vcpu))
 		return (0);
 
-	error = vmread(VMCS_GUEST_INTERRUPTIBILITY, &interruptibility);
-	if (error) {
-		panic("vmx_inject_nmi: vmread(interruptibility) %d",
-			error);
-	}
+	interruptibility = vmcs_read(VMCS_GUEST_INTERRUPTIBILITY);
 	if (interruptibility & nmi_blocking_bits)
 		goto nmiblocked;
 
@@ -1057,10 +1013,7 @@ vmx_inject_nmi(struct vmx *vmx, int vcpu
 	 */
 	info = VMCS_INTERRUPTION_INFO_NMI | VMCS_INTERRUPTION_INFO_VALID;
 	info |= IDT_NMI;
-
-	error = vmwrite(VMCS_ENTRY_INTR_INFO, info);
-	if (error)
-		panic("vmx_inject_nmi: vmwrite(intrinfo) %d", error);
+	vmcs_write(VMCS_ENTRY_INTR_INFO, info);
 
 	VCPU_CTR0(vmx->vm, vcpu, "Injecting vNMI");
 
@@ -1082,7 +1035,7 @@ nmiblocked:
 static void
 vmx_inject_interrupts(struct vmx *vmx, int vcpu)
 {
-	int error, vector;
+	int vector;
 	uint64_t info, rflags, interruptibility;
 
 	const int HWINTR_BLOCKED = VMCS_INTERRUPTIBILITY_STI_BLOCKING |
@@ -1095,9 +1048,7 @@ vmx_inject_interrupts(struct vmx *vmx, i
 	 * VM entry but the actual entry into guest mode was aborted
 	 * because of a pending AST.
 	 */
-	error = vmread(VMCS_ENTRY_INTR_INFO, &info);
-	if (error)
-		panic("vmx_inject_interrupts: vmread(intrinfo) %d", error);
+	info = vmcs_read(VMCS_ENTRY_INTR_INFO);
 	if (info & VMCS_INTERRUPTION_INFO_VALID)
 		return;
 
@@ -1116,27 +1067,18 @@ vmx_inject_interrupts(struct vmx *vmx, i
 		panic("vmx_inject_interrupts: invalid vector %d\n", vector);
 
 	/* Check RFLAGS.IF and the interruptibility state of the guest */
-	error = vmread(VMCS_GUEST_RFLAGS, &rflags);
-	if (error)
-		panic("vmx_inject_interrupts: vmread(rflags) %d", error);
-
+	rflags = vmcs_read(VMCS_GUEST_RFLAGS);
 	if ((rflags & PSL_I) == 0)
 		goto cantinject;
 
-	error = vmread(VMCS_GUEST_INTERRUPTIBILITY, &interruptibility);
-	if (error) {
-		panic("vmx_inject_interrupts: vmread(interruptibility) %d",
-			error);
-	}
+	interruptibility = vmcs_read(VMCS_GUEST_INTERRUPTIBILITY);
 	if (interruptibility & HWINTR_BLOCKED)
 		goto cantinject;
 
 	/* Inject the interrupt */
 	info = VMCS_INTERRUPTION_INFO_HW_INTR | VMCS_INTERRUPTION_INFO_VALID;
 	info |= vector;
-	error = vmwrite(VMCS_ENTRY_INTR_INFO, info);
-	if (error)
-		panic("vmx_inject_interrupts: vmwrite(intrinfo) %d", error);
+	vmcs_write(VMCS_ENTRY_INTR_INFO, info);
 
 	/* Update the Local APIC ISR */
 	lapic_intr_accepted(vmx->vm, vcpu, vector);
@@ -1158,7 +1100,7 @@ cantinject:
 static int
 vmx_emulate_cr_access(struct vmx *vmx, int vcpu, uint64_t exitqual)
 {
-	int error, cr, vmcs_guest_cr, vmcs_shadow_cr;
+	int cr, vmcs_guest_cr, vmcs_shadow_cr;
 	uint64_t crval, regval, ones_mask, zeros_mask;
 	const struct vmxctx *vmxctx;
 
@@ -1173,7 +1115,7 @@ vmx_emulate_cr_access(struct vmx *vmx, i
 	vmxctx = &vmx->ctx[vcpu];
 
 	/*
-	 * We must use vmwrite() directly here because vmcs_setreg() will
+	 * We must use vmcs_write() directly here because vmcs_setreg() will
 	 * call vmclear(vmcs) as a side-effect which we certainly don't want.
 	 */
 	switch ((exitqual >> 8) & 0xf) {
@@ -1190,11 +1132,7 @@ vmx_emulate_cr_access(struct vmx *vmx, i
 		regval = vmxctx->guest_rbx;
 		break;
 	case 4:
-		error = vmread(VMCS_GUEST_RSP, &regval);
-		if (error) {
-			panic("vmx_emulate_cr_access: "
-			      "error %d reading guest rsp", error);
-		}
+		regval = vmcs_read(VMCS_GUEST_RSP);
 		break;
 	case 5:
 		regval = vmxctx->guest_rbp;
@@ -1242,20 +1180,11 @@ vmx_emulate_cr_access(struct vmx *vmx, i
 		vmcs_guest_cr = VMCS_GUEST_CR4;
 		vmcs_shadow_cr = VMCS_CR4_SHADOW;
 	}
-
-	error = vmwrite(vmcs_shadow_cr, regval);
-	if (error) {
-		panic("vmx_emulate_cr_access: error %d writing cr%d shadow",
-		      error, cr);
-	}
+	vmcs_write(vmcs_shadow_cr, regval);
 
 	crval = regval | ones_mask;
 	crval &= ~zeros_mask;
-	error = vmwrite(vmcs_guest_cr, crval);
-	if (error) {
-		panic("vmx_emulate_cr_access: error %d writing cr%d",
-		      error, cr);
-	}
+	vmcs_write(vmcs_guest_cr, crval);
 
 	if (cr == 0 && regval & CR0_PG) {
 		uint64_t efer, entry_ctls;
@@ -1265,29 +1194,13 @@ vmx_emulate_cr_access(struct vmx *vmx, i
 		 * the "IA-32e mode guest" bit in VM-entry control must be
 		 * equal.
 		 */
-		error = vmread(VMCS_GUEST_IA32_EFER, &efer);
-		if (error) {
-		  panic("vmx_emulate_cr_access: error %d efer read",
-			error);			
-		}
+		efer = vmcs_read(VMCS_GUEST_IA32_EFER);
 		if (efer & EFER_LME) {
 			efer |= EFER_LMA;
-			error = vmwrite(VMCS_GUEST_IA32_EFER, efer);
-			if (error) {
-				panic("vmx_emulate_cr_access: error %d"
-				      " efer write", error);
-			}
-			error = vmread(VMCS_ENTRY_CTLS, &entry_ctls);
-			if (error) {
-				panic("vmx_emulate_cr_access: error %d"
-				      " entry ctls read", error);
-			}
+			vmcs_write(VMCS_GUEST_IA32_EFER, efer);
+			entry_ctls = vmcs_read(VMCS_ENTRY_CTLS);
 			entry_ctls |= VM_ENTRY_GUEST_LMA;
-			error = vmwrite(VMCS_ENTRY_CTLS, entry_ctls);
-			if (error) {
-				panic("vmx_emulate_cr_access: error %d"
-				      " entry ctls write", error);
-			}
+			vmcs_write(VMCS_ENTRY_CTLS, entry_ctls);
 		}
 	}
 
@@ -1359,7 +1272,7 @@ vmx_exit_process(struct vmx *vmx, int vc
 	struct vmcs *vmcs;
 	struct vmxctx *vmxctx;
 	uint32_t eax, ecx, edx, idtvec_info, idtvec_err, reason;
-	uint64_t qual, gpa, rflags;
+	uint64_t qual, gpa;
 	bool retu;
 
 	handled = 0;
@@ -1388,12 +1301,13 @@ vmx_exit_process(struct vmx *vmx, int vc
 		idtvec_info = vmcs_idt_vectoring_info();
 		if (idtvec_info & VMCS_IDT_VEC_VALID) {
 			idtvec_info &= ~(1 << 12); /* clear undefined bit */
-			vmwrite(VMCS_ENTRY_INTR_INFO, idtvec_info);
+			vmcs_write(VMCS_ENTRY_INTR_INFO, idtvec_info);
 			if (idtvec_info & VMCS_IDT_VEC_ERRCODE_VALID) {
 				idtvec_err = vmcs_idt_vectoring_err();
-				vmwrite(VMCS_ENTRY_EXCEPTION_ERROR, idtvec_err);
+				vmcs_write(VMCS_ENTRY_EXCEPTION_ERROR,
+				    idtvec_err);
 			}
-			vmwrite(VMCS_ENTRY_INST_LENGTH, vmexit->inst_length);
+			vmcs_write(VMCS_ENTRY_INST_LENGTH, vmexit->inst_length);
 		}
 	default:
 		break;
@@ -1442,10 +1356,8 @@ vmx_exit_process(struct vmx *vmx, int vc
 		break;
 	case EXIT_REASON_HLT:
 		vmm_stat_incr(vmx->vm, vcpu, VMEXIT_HLT, 1);
-		if ((error = vmread(VMCS_GUEST_RFLAGS, &rflags)) != 0)
-			panic("vmx_exit_process: vmread(rflags) %d", error);
 		vmexit->exitcode = VM_EXITCODE_HLT;
-		vmexit->u.hlt.rflags = rflags;
+		vmexit->u.hlt.rflags = vmcs_read(VMCS_GUEST_RFLAGS);
 		break;
 	case EXIT_REASON_MTF:
 		vmm_stat_incr(vmx->vm, vcpu, VMEXIT_MTRAP, 1);
@@ -1533,9 +1445,9 @@ vmx_exit_process(struct vmx *vmx, int vc
 		 * the one we just processed. Therefore we update the
 		 * guest rip in the VMCS and in 'vmexit'.
 		 */
-		vm_exit_update_rip(vmexit);
 		vmexit->rip += vmexit->inst_length;
 		vmexit->inst_length = 0;
+		vmcs_write(VMCS_GUEST_RIP, vmexit->rip);
 	} else {
 		if (vmexit->exitcode == VM_EXITCODE_BOGUS) {
 			/*
@@ -1557,7 +1469,7 @@ vmx_exit_process(struct vmx *vmx, int vc
 static int
 vmx_run(void *arg, int vcpu, register_t rip, pmap_t pmap)
 {
-	int error, vie, rc, handled, astpending;
+	int vie, rc, handled, astpending;
 	uint32_t exit_reason;
 	struct vmx *vmx;
 	struct vmxctx *vmxctx;
@@ -1590,14 +1502,9 @@ vmx_run(void *arg, int vcpu, register_t 
 	 * If the life of a virtual machine was spent entirely in the context
 	 * of a single process we could do this once in vmcs_set_defaults().
 	 */
-	if ((error = vmwrite(VMCS_HOST_CR3, rcr3())) != 0)
-		panic("vmx_run: error %d writing to VMCS_HOST_CR3", error);
-
-	if ((error = vmwrite(VMCS_GUEST_RIP, rip)) != 0)
-		panic("vmx_run: error %d writing to VMCS_GUEST_RIP", error);
-
-	if ((error = vmx_set_pcpu_defaults(vmx, vcpu)) != 0)
-		panic("vmx_run: error %d setting up pcpu defaults", error);
+	vmcs_write(VMCS_HOST_CR3, rcr3());
+	vmcs_write(VMCS_GUEST_RIP, rip);
+	vmx_set_pcpu_defaults(vmx, vcpu);
 
 	do {
 		vmx_inject_interrupts(vmx, vcpu);

Modified: stable/10/sys/amd64/vmm/intel/vmx_genassym.c
==============================================================================
--- stable/10/sys/amd64/vmm/intel/vmx_genassym.c	Sun Feb 23 01:27:22 2014	(r262351)
+++ stable/10/sys/amd64/vmm/intel/vmx_genassym.c	Sun Feb 23 01:34:40 2014	(r262352)
@@ -39,8 +39,8 @@ __FBSDID("$FreeBSD$");
 #include <vm/pmap.h>
 
 #include <machine/vmm.h>
-#include "vmx.h"
 #include "vmx_cpufunc.h"
+#include "vmx.h"
 
 ASSYM(VMXCTX_TMPSTKTOP, offsetof(struct vmxctx, tmpstktop));
 ASSYM(VMXCTX_GUEST_RDI, offsetof(struct vmxctx, guest_rdi));



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201402230134.s1N1YfDp056165>