From: dv <dv@openbsd.org>
Date: Fri, 20 May 2022 22:14:19 +0000 (+0000)
Subject: vmm: load the vmcs before reading vcpu registers
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=15777ce570af6acab3e81b6b56e2274aa5da4f85;p=openbsd

vmm: load the vmcs before reading vcpu registers

The vmm(4) ioctl for reading a vcpu's registers didn't flush and
load the vmcs before issuing vmread instructions. This adds a flag
to vcpu_readregs_vmx to signal if the vmcs needs to be synchronized
before attempting the reads. (This is the same approach used for
vcpu_writeregs_vmx function.)

This fixes `vmctl send` on Intel hosts using vmd(8).

While here, I noticed the vcpu_writeregs_vmx function doesn't
properly set the vmcs state variable to VMCS_CLEARED after running
a vmclear instruction. This can cause errors on vm re-entry.

ok mlarkin@
---

diff --git a/sys/arch/amd64/amd64/vmm.c b/sys/arch/amd64/amd64/vmm.c
index 2784aaf654f..b6ca04b2eca 100644
--- a/sys/arch/amd64/amd64/vmm.c
+++ b/sys/arch/amd64/amd64/vmm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmm.c,v 1.309 2022/05/13 18:19:32 dv Exp $	*/
+/*	$OpenBSD: vmm.c,v 1.310 2022/05/20 22:14:19 dv Exp $	*/
 /*
  * Copyright (c) 2014 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -140,7 +140,7 @@ int vm_rwregs(struct vm_rwregs_params *, int);
 int vm_mprotect_ept(struct vm_mprotect_ept_params *);
 int vm_rwvmparams(struct vm_rwvmparams_params *, int);
 int vm_find(uint32_t, struct vm **);
-int vcpu_readregs_vmx(struct vcpu *, uint64_t, struct vcpu_reg_state *);
+int vcpu_readregs_vmx(struct vcpu *, uint64_t, int, struct vcpu_reg_state *);
 int vcpu_readregs_svm(struct vcpu *, uint64_t, struct vcpu_reg_state *);
 int vcpu_writeregs_vmx(struct vcpu *, uint64_t, int, struct vcpu_reg_state *);
 int vcpu_writeregs_svm(struct vcpu *, uint64_t, struct vcpu_reg_state *);
@@ -978,7 +978,7 @@ vm_rwregs(struct vm_rwregs_params *vrwp, int dir)
 	if (vmm_softc->mode == VMM_MODE_VMX ||
 	    vmm_softc->mode == VMM_MODE_EPT)
 		ret = (dir == 0) ?
-		    vcpu_readregs_vmx(vcpu, vrwp->vrwp_mask, vrs) :
+		    vcpu_readregs_vmx(vcpu, vrwp->vrwp_mask, 1, vrs) :
 		    vcpu_writeregs_vmx(vcpu, vrwp->vrwp_mask, 1, vrs);
 	else if (vmm_softc->mode == VMM_MODE_SVM ||
 	    vmm_softc->mode == VMM_MODE_RVI)
@@ -1986,6 +1986,7 @@ vcpu_reload_vmcs_vmx(struct vcpu *vcpu)
  * Parameters:
  *  vcpu: the vcpu to read register values from
  *  regmask: the types of registers to read
+ *  loadvmcs: bit to indicate whether the VMCS has to be loaded first
  *  vrs: output parameter where register values are stored
  *
  * Return values:
@@ -1993,7 +1994,7 @@ vcpu_reload_vmcs_vmx(struct vcpu *vcpu)
  *  EINVAL: an error reading registers occurred
  */
 int
-vcpu_readregs_vmx(struct vcpu *vcpu, uint64_t regmask,
+vcpu_readregs_vmx(struct vcpu *vcpu, uint64_t regmask, int loadvmcs,
     struct vcpu_reg_state *vrs)
 {
 	int i, ret = 0;
@@ -2005,6 +2006,11 @@ vcpu_readregs_vmx(struct vcpu *vcpu, uint64_t regmask,
 	struct vcpu_segment_info *sregs = vrs->vrs_sregs;
 	struct vmx_msr_store *msr_store;
 
+	if (loadvmcs) {
+		if (vcpu_reload_vmcs_vmx(vcpu))
+			return (EINVAL);
+	}
+
 #ifdef VMM_DEBUG
 	/* VMCS should be loaded... */
 	paddr_t pa = 0ULL;
@@ -2393,6 +2399,7 @@ out:
 	if (loadvmcs) {
 		if (vmclear(&vcpu->vc_control_pa))
 			ret = EINVAL;
+		atomic_swap_uint(&vcpu->vc_vmx_vmcs_state, VMCS_CLEARED);
 	}
 	return (ret);
 }
@@ -4631,7 +4638,7 @@ vmm_translate_gva(struct vcpu *vcpu, uint64_t va, uint64_t *pa, int mode)
 
 	if (vmm_softc->mode == VMM_MODE_EPT ||
 	    vmm_softc->mode == VMM_MODE_VMX) {
-		if (vcpu_readregs_vmx(vcpu, VM_RWREGS_ALL, &vrs))
+		if (vcpu_readregs_vmx(vcpu, VM_RWREGS_ALL, 1, &vrs))
 			return (EINVAL);
 	} else if (vmm_softc->mode == VMM_MODE_RVI ||
 	    vmm_softc->mode == VMM_MODE_SVM) {
@@ -5111,7 +5118,7 @@ vcpu_run_vmx(struct vcpu *vcpu, struct vm_run_params *vrp)
 	vcpu->vc_last_pcpu = curcpu();
 
 	/* Copy the VCPU register state to the exit structure */
-	if (vcpu_readregs_vmx(vcpu, VM_RWREGS_ALL, &vcpu->vc_exit.vrs))
+	if (vcpu_readregs_vmx(vcpu, VM_RWREGS_ALL, 0, &vcpu->vc_exit.vrs))
 		ret = EINVAL;
 	vcpu->vc_exit.cpl = vmm_get_guest_cpu_cpl(vcpu);