From: dv <dv@openbsd.org>
Date: Wed, 27 Apr 2022 14:23:37 +0000 (+0000)
Subject: vmm(4): dt(4) tracepoints for vm exit reporting previous exit.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=54a3c02af71382a882044f3f821dca3827ee12a5;p=openbsd

vmm(4): dt(4) tracepoints for vm exit reporting previous exit.

On Intel/VMX, the tracepoint for vm exits was before extracting the
vm exit reason meaning we either reported stack garbage (on first
exit) or the prior exit reason. Move the tracepoint to after
extraction and refactor the logic to collect all exit info processing
in one place. (We were extracting the guest RFLAGS register state
before checking the exit info extraction success.)

On AMD/SVM, the tracepoint was always triggered even on unsuccessful
vm entry leading to garbage data. Tuck the tracepoint into the If
block and merge them.

ok mlarkin@
---

diff --git a/sys/arch/amd64/amd64/vmm.c b/sys/arch/amd64/amd64/vmm.c
index 5b2aa6ee38a..765fc19bca5 100644
--- a/sys/arch/amd64/amd64/vmm.c
+++ b/sys/arch/amd64/amd64/vmm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmm.c,v 1.305 2022/03/28 06:28:47 tb Exp $	*/
+/*	$OpenBSD: vmm.c,v 1.306 2022/04/27 14:23:37 dv Exp $	*/
 /*
  * Copyright (c) 2014 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -4988,45 +4988,39 @@ vcpu_run_vmx(struct vcpu *vcpu, struct vm_run_params *vrp)
 		vmm_fpusave(vcpu);
 		intr_restore(s);
 
-		TRACEPOINT(vmm, guest_exit, vcpu, vrp, exit_reason);
-
 		atomic_swap_uint(&vcpu->vc_vmx_vmcs_state, VMCS_LAUNCHED);
 		exit_reason = VM_EXIT_NONE;
 
 		/* If we exited successfully ... */
 		if (ret == 0) {
-			/*
-			 * ret == 0 implies we entered the guest, and later
-			 * exited for some valid reason
-			 */
 			exitinfo = vmx_get_exit_info(
 			    &vcpu->vc_gueststate.vg_rip, &exit_reason);
-			if (vmread(VMCS_GUEST_IA32_RFLAGS,
-			    &vcpu->vc_gueststate.vg_rflags)) {
-				printf("%s: can't read guest rflags during "
-				    "exit\n", __func__);
-				ret = EINVAL;
-				break;
-                        }
-
-			/* Update our state */
 			if (!(exitinfo & VMX_EXIT_INFO_HAVE_RIP)) {
 				printf("%s: cannot read guest rip\n", __func__);
 				ret = EINVAL;
 				break;
 			}
-
 			if (!(exitinfo & VMX_EXIT_INFO_HAVE_REASON)) {
 				printf("%s: cant read exit reason\n", __func__);
 				ret = EINVAL;
 				break;
 			}
+			vcpu->vc_gueststate.vg_exit_reason = exit_reason;
+			TRACEPOINT(vmm, guest_exit, vcpu, vrp, exit_reason);
+
+			/* Update our state */
+			if (vmread(VMCS_GUEST_IA32_RFLAGS,
+			    &vcpu->vc_gueststate.vg_rflags)) {
+				printf("%s: can't read guest rflags during "
+				    "exit\n", __func__);
+				ret = EINVAL;
+				break;
+                        }
 
 			/*
 			 * Handle the exit. This will alter "ret" to EAGAIN if
 			 * the exit handler determines help from vmd is needed.
 			 */
-			vcpu->vc_gueststate.vg_exit_reason = exit_reason;
 			ret = vmx_handle_exit(vcpu);
 
 			if (vcpu->vc_gueststate.vg_rflags & PSL_I)
@@ -7325,16 +7319,12 @@ vcpu_run_svm(struct vcpu *vcpu, struct vm_run_params *vrp)
 		vmcb->v_tlb_control = SVM_TLB_CONTROL_FLUSH_NONE;
 		svm_set_clean(vcpu, SVM_CLEANBITS_ALL);
 
-		/* Record the exit reason on successful exit */
+		/* If we exited successfully ... */
 		if (ret == 0) {
 			exit_reason = vmcb->v_exitcode;
 			vcpu->vc_gueststate.vg_exit_reason = exit_reason;
-		}
+			TRACEPOINT(vmm, guest_exit, vcpu, vrp, exit_reason);
 
-		TRACEPOINT(vmm, guest_exit, vcpu, vrp, exit_reason);
-
-		/* If we exited successfully ... */
-		if (ret == 0) {
 			vcpu->vc_gueststate.vg_rflags = vmcb->v_rflags;
 
 			/*