Delete 108 lines of ASM from vmx_enter_guest() that predated lots

of later enhancements, removing the save/restore of flags, selectors,
and MSRs: flags are caller-saved and don't need restoring while
selectors and MSRs are auto-restored.  The FSBASE, GSBASE, and
KERNELGSBASE MSRs just need the correct values set with vmwrite()
in the "on new CPU?" block of vcpu_run_vmx().

Also, only rdmsr(MSR_MISC_ENABLE) once in vcpu_reset_regs_vmx(),
give symbolic names to the exit-load MSR slots, eliminate
VMX_NUM_MSR_STORE, and #if 0 the vc_vmx_msr_entry_load_{va,pa} code
and definitions as unused.

ok dv@

3 files changed, 67 insertions, 136 deletions

diff --git a/sys/arch/amd64/amd64/vmm_machdep.c b/sys/arch/amd64/amd64/vmm_machdep.c
index d94fa95ae04..55c775c65de 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.21 2024/03/12 02:31:15 guenther Exp $ */
+/* $OpenBSD: vmm_machdep.c,v 1.22 2024/04/01 05:11:49 guenther Exp $ */
 /*
  * Copyright (c) 2014 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -2329,7 +2329,7 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, struct vcpu_reg_state *vrs)
 	uint32_t cr0, cr4;
 	uint32_t pinbased, procbased, procbased2, exit, entry;
 	uint32_t want1, want0;
-	uint64_t ctrlval, cr3;
+	uint64_t ctrlval, cr3, msr_misc_enable;
 	uint16_t ctrl, vpid;
 	struct vmx_msr_store *msr_store;
 
@@ -2723,24 +2723,26 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, struct vcpu_reg_state *vrs)
 	vrs->vrs_crs[VCPU_REGS_CR0] = cr0;
 	vrs->vrs_crs[VCPU_REGS_CR4] = cr4;
 
+	msr_misc_enable = rdmsr(MSR_MISC_ENABLE);
+
 	/*
 	 * Select host MSRs to be loaded on exit
 	 */
 	msr_store = (struct vmx_msr_store *)vcpu->vc_vmx_msr_exit_load_va;
-	msr_store[0].vms_index = MSR_EFER;
-	msr_store[0].vms_data = rdmsr(MSR_EFER);
-	msr_store[1].vms_index = MSR_STAR;
-	msr_store[1].vms_data = rdmsr(MSR_STAR);
-	msr_store[2].vms_index = MSR_LSTAR;
-	msr_store[2].vms_data = rdmsr(MSR_LSTAR);
-	msr_store[3].vms_index = MSR_CSTAR;
-	msr_store[3].vms_data = 0;
-	msr_store[4].vms_index = MSR_SFMASK;
-	msr_store[4].vms_data = rdmsr(MSR_SFMASK);
-	msr_store[5].vms_index = MSR_KERNELGSBASE;
-	msr_store[5].vms_data = rdmsr(MSR_KERNELGSBASE);
-	msr_store[6].vms_index = MSR_MISC_ENABLE;
-	msr_store[6].vms_data = rdmsr(MSR_MISC_ENABLE);
+	msr_store[VCPU_HOST_REGS_EFER].vms_index = MSR_EFER;
+	msr_store[VCPU_HOST_REGS_EFER].vms_data = rdmsr(MSR_EFER);
+	msr_store[VCPU_HOST_REGS_STAR].vms_index = MSR_STAR;
+	msr_store[VCPU_HOST_REGS_STAR].vms_data = rdmsr(MSR_STAR);
+	msr_store[VCPU_HOST_REGS_LSTAR].vms_index = MSR_LSTAR;
+	msr_store[VCPU_HOST_REGS_LSTAR].vms_data = rdmsr(MSR_LSTAR);
+	msr_store[VCPU_HOST_REGS_CSTAR].vms_index = MSR_CSTAR;
+	msr_store[VCPU_HOST_REGS_CSTAR].vms_data = 0;
+	msr_store[VCPU_HOST_REGS_SFMASK].vms_index = MSR_SFMASK;
+	msr_store[VCPU_HOST_REGS_SFMASK].vms_data = rdmsr(MSR_SFMASK);
+	msr_store[VCPU_HOST_REGS_KGSBASE].vms_index = MSR_KERNELGSBASE;
+	msr_store[VCPU_HOST_REGS_KGSBASE].vms_data = 0;
+	msr_store[VCPU_HOST_REGS_MISC_ENABLE].vms_index = MSR_MISC_ENABLE;
+	msr_store[VCPU_HOST_REGS_MISC_ENABLE].vms_data = msr_misc_enable;
 
 	/*
 	 * Select guest MSRs to be loaded on entry / saved on exit
@@ -2759,7 +2761,7 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, struct vcpu_reg_state *vrs)
 	 * Initialize MSR_MISC_ENABLE as it can't be read and populated from vmd
 	 * and some of the content is based on the host.
 	 */
-	msr_store[VCPU_REGS_MISC_ENABLE].vms_data = rdmsr(MSR_MISC_ENABLE);
+	msr_store[VCPU_REGS_MISC_ENABLE].vms_data = msr_misc_enable;
 	msr_store[VCPU_REGS_MISC_ENABLE].vms_data &=
 	    ~(MISC_ENABLE_TCC | MISC_ENABLE_PERF_MON_AVAILABLE |
 	      MISC_ENABLE_EIST_ENABLED | MISC_ENABLE_ENABLE_MONITOR_FSM |
@@ -2768,24 +2770,26 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, struct vcpu_reg_state *vrs)
 	      MISC_ENABLE_BTS_UNAVAILABLE | MISC_ENABLE_PEBS_UNAVAILABLE;
 
 	/*
-	 * Currently we have the same count of entry/exit MSRs loads/stores
-	 * but this is not an architectural requirement.
+	 * Currently we use the same memory for guest MSRs (entry-load and
+	 * exit-store) so they have the same count.  We exit-load the same
+	 * host MSRs, so same count but different memory.  Those are just
+	 * our current choices, not architectural requirements.
 	 */
-	if (vmwrite(VMCS_EXIT_MSR_STORE_COUNT, VMX_NUM_MSR_STORE)) {
+	if (vmwrite(VMCS_EXIT_MSR_STORE_COUNT, VCPU_REGS_NMSRS)) {
 		DPRINTF("%s: error setting guest MSR exit store count\n",
 		    __func__);
 		ret = EINVAL;
 		goto exit;
 	}
 
-	if (vmwrite(VMCS_EXIT_MSR_LOAD_COUNT, VMX_NUM_MSR_STORE)) {
+	if (vmwrite(VMCS_EXIT_MSR_LOAD_COUNT, VCPU_HOST_REGS_NMSRS)) {
 		DPRINTF("%s: error setting guest MSR exit load count\n",
 		    __func__);
 		ret = EINVAL;
 		goto exit;
 	}
 
-	if (vmwrite(VMCS_ENTRY_MSR_LOAD_COUNT, VMX_NUM_MSR_STORE)) {
+	if (vmwrite(VMCS_ENTRY_MSR_LOAD_COUNT, VCPU_REGS_NMSRS)) {
 		DPRINTF("%s: error setting guest MSR entry load count\n",
 		    __func__);
 		ret = EINVAL;
@@ -2974,6 +2978,7 @@ vcpu_init_vmx(struct vcpu *vcpu)
 		goto exit;
 	}
 
+#if 0	/* XXX currently use msr_exit_save for msr_entry_load too */
 	/* Allocate MSR entry load area VA */
 	vcpu->vc_vmx_msr_entry_load_va = (vaddr_t)km_alloc(PAGE_SIZE, &kv_page,
 	   &kp_zero, &kd_waitok);
@@ -2989,6 +2994,7 @@ vcpu_init_vmx(struct vcpu *vcpu)
 		ret = ENOMEM;
 		goto exit;
 	}
+#endif
 
 	vmcs = (struct vmcs *)vcpu->vc_control_va;
 	vmcs->vmcs_revision = curcpu()->ci_vmm_cap.vcc_vmx.vmx_vmxon_revision;
@@ -3308,11 +3314,13 @@ vcpu_deinit_vmx(struct vcpu *vcpu)
 		    PAGE_SIZE, &kv_page, &kp_zero);
 		vcpu->vc_vmx_msr_exit_load_va = 0;
 	}
+#if 0
 	if (vcpu->vc_vmx_msr_entry_load_va) {
 		km_free((void *)vcpu->vc_vmx_msr_entry_load_va,
 		    PAGE_SIZE, &kv_page, &kp_zero);
 		vcpu->vc_vmx_msr_entry_load_va = 0;
 	}
+#endif
 
 	if (vcpu->vc_vmx_vpid_enabled)
 		vmm_free_vpid(vcpu->vc_vpid);
@@ -3954,8 +3962,9 @@ vcpu_run_vmx(struct vcpu *vcpu, struct vm_run_params *vrp)
 	int ret = 0, exitinfo;
 	struct region_descriptor gdt;
 	struct cpu_info *ci = NULL;
-	uint64_t exit_reason, cr3, insn_error;
+	uint64_t exit_reason, cr3, msr, insn_error;
 	struct schedstate_percpu *spc;
+	struct vmx_msr_store *msr_store;
 	struct vmx_invvpid_descriptor vid;
 	uint64_t eii, procbased, int_st;
 	uint16_t irq;
@@ -4091,6 +4100,7 @@ vcpu_run_vmx(struct vcpu *vcpu, struct vm_run_params *vrp)
 		}
 	}
 
+	msr_store = (struct vmx_msr_store *)vcpu->vc_vmx_msr_exit_load_va;
 	while (ret == 0) {
 #ifdef VMM_DEBUG
 		paddr_t pa = 0ULL;
@@ -4126,6 +4136,26 @@ vcpu_run_vmx(struct vcpu *vcpu, struct vm_run_params *vrp)
 				    (uint64_t)ci->ci_tss);
 				return (EINVAL);
 			}
+
+			/* Host GS.base (aka curcpu) */
+			if (vmwrite(VMCS_HOST_IA32_GS_BASE, (uint64_t)ci)) {
+				printf("%s: vmwrite(0x%04X, 0x%llx)\n",
+				    __func__, VMCS_HOST_IA32_GS_BASE,
+				    (uint64_t)ci);
+				return (EINVAL);
+			}
+
+			/* Host FS.base */
+			msr = rdmsr(MSR_FSBASE);
+			if (vmwrite(VMCS_HOST_IA32_FS_BASE, msr)) {
+				printf("%s: vmwrite(0x%04X, 0x%llx)\n",
+				    __func__, VMCS_HOST_IA32_FS_BASE, msr);
+				return (EINVAL);
+			}
+
+			/* Host KernelGS.base (userspace GS.base here) */
+			msr_store[VCPU_HOST_REGS_KGSBASE].vms_data =
+			    rdmsr(MSR_KERNELGSBASE);
 		}
 
 		/* Inject event if present */
@@ -8087,7 +8117,7 @@ vmx_vcpu_dump_regs(struct vcpu *vcpu)
 
 	msr_store = (struct vmx_msr_store *)vcpu->vc_vmx_msr_exit_save_va;
 
-	for (i = 0; i < VMX_NUM_MSR_STORE; i++) {
+	for (i = 0; i < VCPU_REGS_NMSRS; i++) {
 		DPRINTF("  MSR %d @ %p : 0x%08llx (%s), "
 		    "value=0x%016llx ",
 		    i, &msr_store[i], msr_store[i].vms_index,
diff --git a/sys/arch/amd64/amd64/vmm_support.S b/sys/arch/amd64/amd64/vmm_support.S
index 8da5ac88873..9c028630630 100644
--- a/sys/arch/amd64/amd64/vmm_support.S
+++ b/sys/arch/amd64/amd64/vmm_support.S
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmm_support.S,v 1.26 2024/03/17 05:49:41 guenther Exp $	*/
+/*	$OpenBSD: vmm_support.S,v 1.27 2024/04/01 05:11:49 guenther Exp $	*/
 /*
  * Copyright (c) 2014 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -177,59 +177,6 @@ ENTRY(vmx_enter_guest)
 	vmwrite %rax, %rdi	/* Host RIP */
 
 skip_init:
-	/*
-	 * XXX use msr list here for restore instead of all this
-	 * stack jiggery-pokery
-	 */
-
-	pushfq
-	popq	%rax
-	andq	$(~PSL_I), %rax
-	pushq	%rax
-
-	/*
-	 * Save (possibly) lazy-switched selectors
-	 */
-	movw	%es, %ax
-	pushw	%ax
-	movw	%ds, %ax
-	pushw	%ax
-	movw	%ss, %ax
-	pushw	%ax
-
-	movq	$MSR_FSBASE, %rcx
-	rdmsr
-	pushq	%rax
-	pushq	%rdx
-	pushw	%fs
-	movq	$MSR_GSBASE, %rcx
-	rdmsr
-	pushq	%rax
-	pushq	%rdx
-	pushw	%gs
-	movq	$MSR_KERNELGSBASE, %rcx
-	rdmsr
-	pushq	%rax
-	pushq	%rdx
-
-	/*
-	 * Save various MSRs
-	 */
-	movq	$MSR_STAR, %rcx
-	rdmsr
-	pushq	%rax
-	pushq	%rdx
-
-	movq	$MSR_LSTAR, %rcx
-	rdmsr
-	pushq	%rax
-	pushq	%rdx
-
-	movq	$MSR_SFMASK, %rcx
-	rdmsr
-	pushq	%rax
-	pushq	%rdx
-
 	RETGUARD_PUSH(r11)
 
 	/* Preserve callee-preserved registers as per AMD64 ABI */
@@ -486,61 +433,6 @@ restore_host:
 
 	RETGUARD_POP(r11)
 
-	/*
-	 * Restore saved MSRs
-	 */
-	popq	%rdx
-	popq	%rax
-	movq	$MSR_SFMASK, %rcx
-	wrmsr
-
-	/* make sure guest doesn't bleed into host */
-	xorl	%edx, %edx
-	xorl	%eax, %eax
-	movq	$MSR_CSTAR, %rcx
-	wrmsr
-
-	popq	%rdx
-	popq	%rax
-	movq	$MSR_LSTAR, %rcx
-	wrmsr
-
-	popq	%rdx
-	popq	%rax
-	movq	$MSR_STAR, %rcx
-	wrmsr
-
-	/*
-	 * popw %gs will reset gsbase to 0, so preserve it
-	 * first. This is to accommodate possibly lazy-switched
-	 * selectors from above
-	 */
-	popq	%rdx
-	popq	%rax
-	movq	$MSR_KERNELGSBASE, %rcx
-	wrmsr
-
-	popw	%gs
-	popq	%rdx
-	popq	%rax
-	movq	$MSR_GSBASE, %rcx
-	wrmsr
-
-	popw	%fs
-	popq	%rdx
-	popq	%rax
-	movq	$MSR_FSBASE, %rcx
-	wrmsr
-
-	popw	%ax
-	movw	%ax, %ss
-	popw	%ax
-	movw	%ax, %ds
-	popw	%ax
-	movw	%ax, %es
-
-	popfq
-
 	movq	%rdi, %rax
 	RETGUARD_CHECK(vmx_enter_guest, r11)
 	ret
diff --git a/sys/arch/amd64/include/vmmvar.h b/sys/arch/amd64/include/vmmvar.h
index e6a35211b0f..82aa105d395 100644
--- a/sys/arch/amd64/include/vmmvar.h
+++ b/sys/arch/amd64/include/vmmvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmmvar.h,v 1.98 2024/01/20 20:11:24 mlarkin Exp $	*/
+/*	$OpenBSD: vmmvar.h,v 1.99 2024/04/01 05:11:49 guenther Exp $	*/
 /*
  * Copyright (c) 2014 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -435,6 +435,15 @@ struct vcpu_reg_state {
 	struct vcpu_segment_info	vrs_idtr;
 };
 
+#define VCPU_HOST_REGS_EFER   		0
+#define VCPU_HOST_REGS_STAR   		1
+#define VCPU_HOST_REGS_LSTAR  		2
+#define VCPU_HOST_REGS_CSTAR  		3
+#define VCPU_HOST_REGS_SFMASK 		4
+#define VCPU_HOST_REGS_KGSBASE		5
+#define VCPU_HOST_REGS_MISC_ENABLE	6
+#define VCPU_HOST_REGS_NMSRS		(VCPU_HOST_REGS_MISC_ENABLE + 1)
+
 /*
  * struct vm_exit
  *
@@ -617,8 +626,6 @@ struct vm_mprotect_ept_params {
 #define VMX_FAIL_LAUNCH_INVALID_VMCS	2
 #define VMX_FAIL_LAUNCH_VALID_VMCS	3
 
-#define VMX_NUM_MSR_STORE		7
-
 /* MSR bitmap manipulation macros */
 #define VMX_MSRIDX(m)			((m) / 8)
 #define VMX_MSRBIT(m)			(1 << (m) % 8)
@@ -894,8 +901,10 @@ struct vcpu {
 	paddr_t vc_vmx_msr_exit_save_pa;
 	vaddr_t vc_vmx_msr_exit_load_va;
 	paddr_t vc_vmx_msr_exit_load_pa;
+#if 0	/* XXX currently use msr_exit_save for msr_entry_load too */
 	vaddr_t vc_vmx_msr_entry_load_va;
 	paddr_t vc_vmx_msr_entry_load_pa;
+#endif
 	uint8_t vc_vmx_vpid_enabled;
 	uint64_t vc_vmx_cr0_fixed1;
 	uint64_t vc_vmx_cr0_fixed0;