From db090b68861dd75ec57d11a8eb88f6e6df9d4e54 Mon Sep 17 00:00:00 2001 From: dv Date: Mon, 29 Apr 2024 14:47:05 +0000 Subject: [PATCH] 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@ --- sys/arch/amd64/amd64/vmm_machdep.c | 141 ++++++++++------------------- sys/arch/amd64/include/vmmvar.h | 3 +- usr.sbin/vmd/vm.c | 5 +- 3 files changed, 50 insertions(+), 99 deletions(-) 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 * @@ -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 * @@ -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 @@ -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); } -- 2.20.1