From: Dave Voutila Subject: Move VMX vpid allocation to fix vpid leak in vmm. To: tech Cc: Mike Larkin , Hans-J?rg H?xer Date: Sun, 01 Sep 2024 21:46:16 -0400 I noticed this while Hans-Joerg was working on SEV support for vmm(4). Since vpid allocation is in the reset register ioctl handler, and we don't free any previous vpid, a program on an Intel host that keeps resetting a vcpu via VMM_IOC_RESETCPU will exhaust vpids. Instead of a check & free dance, this just moves the allocation to the vcpu initialization since vmm frees vpids/asids in vcpu de-init. (The SEV changes moved the SVM ASID stuff similarly.) It also removes the DPRINTF (also on the SVM side) since the allocation function already has a printf on failure. Plus, since we're de-coupling allocation from configuration, the VMCS reload can be avoided, making this a net-negative diff. ok? diffstat refs/heads/master refs/heads/vpid-leak M sys/arch/amd64/amd64/vmm_machdep.c | 7+ 21- 1 file changed, 7 insertions(+), 21 deletions(-) diff refs/heads/master refs/heads/vpid-leak commit - 24056440d1d6e8d3aa8785990526387386042fb4 commit + a01d423d1b92501a0e6b5cc4b9fe22a9e3983795 blob - c84fbb3273c799116e4628a1097db4eb9f135955 blob + a472790c5d18da3cc8c7954a2c5006b745009c53 --- sys/arch/amd64/amd64/vmm_machdep.c +++ sys/arch/amd64/amd64/vmm_machdep.c @@ -2253,7 +2253,7 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, struct vcpu_reg uint32_t pinbased, procbased, procbased2, exit, entry; uint32_t want1, want0; uint64_t ctrlval, cr3, msr_misc_enable; - uint16_t ctrl, vpid; + uint16_t ctrl; struct vmx_msr_store *msr_store; rw_assert_wrlock(&vcpu->vc_lock); @@ -2516,30 +2516,12 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, struct vcpu_reg IA32_VMX_ACTIVATE_SECONDARY_CONTROLS, 1)) { if (vcpu_vmx_check_cap(vcpu, IA32_VMX_PROCBASED2_CTLS, IA32_VMX_ENABLE_VPID, 1)) { - - /* We may sleep during allocation, so reload VMCS. */ - vcpu->vc_last_pcpu = curcpu(); - ret = vmm_alloc_vpid(&vpid); - if (vcpu_reload_vmcs_vmx(vcpu)) { - printf("%s: failed to reload vmcs\n", __func__); - ret = EINVAL; - goto exit; - } - if (ret) { - DPRINTF("%s: could not allocate VPID\n", - __func__); - ret = EINVAL; - goto exit; - } - - if (vmwrite(VMCS_GUEST_VPID, vpid)) { + if (vmwrite(VMCS_GUEST_VPID, vcpu->vc_vpid)) { DPRINTF("%s: error setting guest VPID\n", __func__); ret = EINVAL; goto exit; } - - vcpu->vc_vpid = vpid; } } @@ -2832,6 +2814,11 @@ vcpu_init_vmx(struct vcpu *vcpu) uint32_t cr0, cr4; int ret = 0; + /* Allocate a VPID early because there's no cleanup to do on failure. */ + ret = vmm_alloc_vpid(&vcpu->vc_vpid); + if (ret) + return (ret); + /* Allocate VMCS VA */ vcpu->vc_control_va = (vaddr_t)km_alloc(PAGE_SIZE, &kv_page, &kp_zero, &kd_waitok); @@ -3175,7 +3162,6 @@ vcpu_init_svm(struct vcpu *vcpu, struct vm_create_para /* Guest VCPU ASID */ if (vmm_alloc_vpid(&asid)) { - DPRINTF("%s: could not allocate asid\n", __func__); ret = EINVAL; goto exit; }