From: dv <dv@openbsd.org>
Date: Mon, 29 Apr 2024 14:47:05 +0000 (+0000)
Subject: vmm & vmd: drop "continue" flag to simplify running a vcpu.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=db090b68861dd75ec57d11a8eb88f6e6df9d4e54;p=openbsd

vmm & vmd: drop "continue" flag to simplify running a vcpu.

There's no need to distinguish the "first" time running a vcpu from
the subsequent times because vmm(4) uses in-kernel state tracking
the last vm exit reason to optimize the logic for updating vcpu
registers from userland. While here, clean up the DPRINTF's to make
the Intel VMX logic similar to the AMD SVM.

ok mlarkin@
---

diff --git a/sys/arch/amd64/amd64/vmm_machdep.c b/sys/arch/amd64/amd64/vmm_machdep.c
index 4769837796f..9338b96b02f 100644
--- a/sys/arch/amd64/amd64/vmm_machdep.c
+++ b/sys/arch/amd64/amd64/vmm_machdep.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: vmm_machdep.c,v 1.24 2024/04/13 21:57:22 dv Exp $ */
+/* $OpenBSD: vmm_machdep.c,v 1.25 2024/04/29 14:47:05 dv Exp $ */
 /*
  * Copyright (c) 2014 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -3691,18 +3691,14 @@ vm_run(struct vm_run_params *vrp)
 	}
 
 	/*
-	 * We may be returning from userland helping us from the last exit.
-	 * If so (vrp_continue == 1), copy in the exit data from vmd. The
-	 * exit data will be consumed before the next entry (this typically
-	 * comprises VCPU register changes as the result of vmd(8)'s actions).
+	 * We may be returning from userland helping us from the last
+	 * exit. Copy in the exit data from vmd. The exit data will be
+	 * consumed before the next entry (this typically comprises
+	 * VCPU register changes as the result of vmd(8)'s actions).
 	 */
-	if (vrp->vrp_continue) {
-		if (copyin(vrp->vrp_exit, &vcpu->vc_exit,
-		    sizeof(struct vm_exit)) == EFAULT) {
-			ret = EFAULT;
-			goto out_unlock;
-		}
-	}
+	ret = copyin(vrp->vrp_exit, &vcpu->vc_exit, sizeof(struct vm_exit));
+	if (ret)
+		goto out_unlock;
 
 	vcpu->vc_inject.vie_type = vrp->vrp_inject.vie_type;
 	vcpu->vc_inject.vie_vector = vrp->vrp_inject.vie_vector;
@@ -4001,67 +3997,28 @@ vcpu_run_vmx(struct vcpu *vcpu, struct vm_run_params *vrp)
 	else
 		vcpu->vc_intr = 0;
 
-	if (vrp->vrp_continue) {
-		switch (vcpu->vc_gueststate.vg_exit_reason) {
-		case VMX_EXIT_IO:
-			if (vcpu->vc_exit.vei.vei_dir == VEI_DIR_IN)
-				vcpu->vc_gueststate.vg_rax =
-				    vcpu->vc_exit.vei.vei_data;
-			vcpu->vc_gueststate.vg_rip =
-			    vcpu->vc_exit.vrs.vrs_gprs[VCPU_REGS_RIP];
-			if (vmwrite(VMCS_GUEST_IA32_RIP,
-			    vcpu->vc_gueststate.vg_rip)) {
-				printf("%s: failed to update rip\n", __func__);
-				return (EINVAL);
-			}
-			break;
-		case VMX_EXIT_EPT_VIOLATION:
-			ret = vcpu_writeregs_vmx(vcpu, VM_RWREGS_GPRS, 0,
-			    &vcpu->vc_exit.vrs);
-			if (ret) {
-				printf("%s: vm %d vcpu %d failed to update "
-				    "registers\n", __func__,
-				    vcpu->vc_parent->vm_id, vcpu->vc_id);
-				return (EINVAL);
-			}
-			break;
-		case VM_EXIT_NONE:
-		case VMX_EXIT_HLT:
-		case VMX_EXIT_INT_WINDOW:
-		case VMX_EXIT_EXTINT:
-		case VMX_EXIT_CPUID:
-		case VMX_EXIT_XSETBV:
-			break;
-#ifdef VMM_DEBUG
-		case VMX_EXIT_TRIPLE_FAULT:
-			DPRINTF("%s: vm %d vcpu %d triple fault\n",
-			    __func__, vcpu->vc_parent->vm_id,
-			    vcpu->vc_id);
-			vmx_vcpu_dump_regs(vcpu);
-			dump_vcpu(vcpu);
-			vmx_dump_vmcs(vcpu);
-			break;
-		case VMX_EXIT_ENTRY_FAILED_GUEST_STATE:
-			DPRINTF("%s: vm %d vcpu %d failed entry "
-			    "due to invalid guest state\n",
-			    __func__, vcpu->vc_parent->vm_id,
-			    vcpu->vc_id);
-			vmx_vcpu_dump_regs(vcpu);
-			dump_vcpu(vcpu);
+	switch (vcpu->vc_gueststate.vg_exit_reason) {
+	case VMX_EXIT_IO:
+		if (vcpu->vc_exit.vei.vei_dir == VEI_DIR_IN)
+			vcpu->vc_gueststate.vg_rax = vcpu->vc_exit.vei.vei_data;
+		vcpu->vc_gueststate.vg_rip =
+		    vcpu->vc_exit.vrs.vrs_gprs[VCPU_REGS_RIP];
+		if (vmwrite(VMCS_GUEST_IA32_RIP, vcpu->vc_gueststate.vg_rip)) {
+			printf("%s: failed to update rip\n", __func__);
+			return (EINVAL);
+		}
+		break;
+	case VMX_EXIT_EPT_VIOLATION:
+		ret = vcpu_writeregs_vmx(vcpu, VM_RWREGS_GPRS, 0,
+		    &vcpu->vc_exit.vrs);
+		if (ret) {
+			printf("%s: vm %d vcpu %d failed to update registers\n",
+			    __func__, vcpu->vc_parent->vm_id, vcpu->vc_id);
 			return (EINVAL);
-		default:
-			DPRINTF("%s: unimplemented exit type %d (%s)\n",
-			    __func__,
-			    vcpu->vc_gueststate.vg_exit_reason,
-			    vmx_exit_reason_decode(
-				vcpu->vc_gueststate.vg_exit_reason));
-			vmx_vcpu_dump_regs(vcpu);
-			dump_vcpu(vcpu);
-			break;
-#endif /* VMM_DEBUG */
 		}
-		memset(&vcpu->vc_exit, 0, sizeof(vcpu->vc_exit));
+		break;
 	}
+	memset(&vcpu->vc_exit, 0, sizeof(vcpu->vc_exit));
 
 	/* Host CR3 */
 	cr3 = rcr3();
@@ -6519,31 +6476,29 @@ vcpu_run_svm(struct vcpu *vcpu, struct vm_run_params *vrp)
 	 * needs to be fixed up depends on what vmd populated in the
 	 * exit data structure.
 	 */
-	if (vrp->vrp_continue) {
-		switch (vcpu->vc_gueststate.vg_exit_reason) {
-		case SVM_VMEXIT_IOIO:
-			if (vcpu->vc_exit.vei.vei_dir == VEI_DIR_IN) {
-				vcpu->vc_gueststate.vg_rax =
-				    vcpu->vc_exit.vei.vei_data;
-				vmcb->v_rax = vcpu->vc_gueststate.vg_rax;
-			}
-			vcpu->vc_gueststate.vg_rip =
-			    vcpu->vc_exit.vrs.vrs_gprs[VCPU_REGS_RIP];
-			vmcb->v_rip = vcpu->vc_gueststate.vg_rip;
-			break;
-		case SVM_VMEXIT_NPF:
-			ret = vcpu_writeregs_svm(vcpu, VM_RWREGS_GPRS,
-			    &vcpu->vc_exit.vrs);
-			if (ret) {
-				printf("%s: vm %d vcpu %d failed to update "
-				    "registers\n", __func__,
-				    vcpu->vc_parent->vm_id, vcpu->vc_id);
-				return (EINVAL);
-			}
-			break;
+	switch (vcpu->vc_gueststate.vg_exit_reason) {
+	case SVM_VMEXIT_IOIO:
+		if (vcpu->vc_exit.vei.vei_dir == VEI_DIR_IN) {
+			vcpu->vc_gueststate.vg_rax =
+			    vcpu->vc_exit.vei.vei_data;
+			vmcb->v_rax = vcpu->vc_gueststate.vg_rax;
 		}
-		memset(&vcpu->vc_exit, 0, sizeof(vcpu->vc_exit));
+		vcpu->vc_gueststate.vg_rip =
+		    vcpu->vc_exit.vrs.vrs_gprs[VCPU_REGS_RIP];
+		vmcb->v_rip = vcpu->vc_gueststate.vg_rip;
+		break;
+	case SVM_VMEXIT_NPF:
+		ret = vcpu_writeregs_svm(vcpu, VM_RWREGS_GPRS,
+		    &vcpu->vc_exit.vrs);
+		if (ret) {
+			printf("%s: vm %d vcpu %d failed to update "
+			    "registers\n", __func__,
+			    vcpu->vc_parent->vm_id, vcpu->vc_id);
+			return (EINVAL);
+		}
+		break;
 	}
+	memset(&vcpu->vc_exit, 0, sizeof(vcpu->vc_exit));
 
 	while (ret == 0) {
 		vmm_update_pvclock(vcpu);
diff --git a/sys/arch/amd64/include/vmmvar.h b/sys/arch/amd64/include/vmmvar.h
index 05567f5bec5..8426a5d94d1 100644
--- a/sys/arch/amd64/include/vmmvar.h
+++ b/sys/arch/amd64/include/vmmvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmmvar.h,v 1.100 2024/04/09 21:55:16 dv Exp $	*/
+/*	$OpenBSD: vmmvar.h,v 1.101 2024/04/29 14:47:05 dv Exp $	*/
 /*
  * Copyright (c) 2014 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -478,7 +478,6 @@ struct vm_run_params {
 	/* Input parameters to VMM_IOC_RUN */
 	uint32_t	vrp_vm_id;
 	uint32_t	vrp_vcpu_id;
-	uint8_t		vrp_continue;		/* Continuing from an exit */
 	struct vcpu_inject_event	vrp_inject;
 	uint8_t		vrp_intr_pending;	/* Additional intrs pending? */
 
diff --git a/usr.sbin/vmd/vm.c b/usr.sbin/vmd/vm.c
index d5bfe7a7688..f8e01288f01 100644
--- a/usr.sbin/vmd/vm.c
+++ b/usr.sbin/vmd/vm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vm.c,v 1.99 2024/04/09 21:55:16 dv Exp $	*/
+/*	$OpenBSD: vm.c,v 1.100 2024/04/29 14:47:06 dv Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -1538,7 +1538,6 @@ vcpu_run_loop(void *arg)
 	intptr_t ret = 0;
 	uint32_t n;
 
-	vrp->vrp_continue = 0;
 	n = vrp->vrp_vcpu_id;
 
 	for (;;) {
@@ -1917,8 +1916,6 @@ vcpu_exit(struct vm_run_params *vrp)
 		    __progname, vrp->vrp_exit_reason);
 	}
 
-	vrp->vrp_continue = 1;
-
 	return (0);
 }