On Tue, Apr 16, 2024, Paolo Bonzini wrote:
> @@ -4711,8 +4722,21 @@ static void init_vmcs(struct vcpu_vmx *vmx)
>
> exec_controls_set(vmx, vmx_exec_control(vmx));
>
> - if (cpu_has_secondary_exec_ctrls())
> + if (cpu_has_secondary_exec_ctrls()) {
> secondary_exec_controls_set(vmx, vmx_secondary_exec_control(vmx));
> + if (vmx->ve_info) {
> + vmcs_write64(VE_INFORMATION_ADDRESS,
> + __pa(vmx->ve_info));
> + } else {
> + /*
> + * Because SECONDARY_EXEC_EPT_VIOLATION_VE is
> + * used only for debugging, it's okay to leave
> + * it disabled.
> + */
> + secondary_exec_controls_clearbit(vmx,
> + SECONDARY_EXEC_EPT_VIOLATION_VE);
As below, this is silly.
> + }
> + }
>
> if (cpu_has_tertiary_exec_ctrls())
> tertiary_exec_controls_set(vmx, vmx_tertiary_exec_control(vmx));
> @@ -5200,6 +5224,12 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu)
> if (is_invalid_opcode(intr_info))
> return handle_ud(vcpu);
>
> + /*
> + * #VE isn't supposed to happen. Block the VM if it does.
> + */
Doesn't need to be a multi-line comment. Though I would just drop the comment,
the KVM_BUG_ON() makes it pretty darn clear #VE is unexpected.
> + if (KVM_BUG_ON(is_ve_fault(intr_info), vcpu->kvm))
> + return -EIO;
> +
> error_code = 0;
> if (intr_info & INTR_INFO_DELIVER_CODE_MASK)
> error_code = vmcs_read32(VM_EXIT_INTR_ERROR_CODE);
> @@ -7474,6 +7504,8 @@ void vmx_vcpu_free(struct kvm_vcpu *vcpu)
> free_vpid(vmx->vpid);
> nested_vmx_free_vcpu(vcpu);
> free_loaded_vmcs(vmx->loaded_vmcs);
> + if (vmx->ve_info)
free_page() handles '0', though hopefully this becomes a moot point.
> + free_page((unsigned long)vmx->ve_info);
> }
>
> int vmx_vcpu_create(struct kvm_vcpu *vcpu)
> @@ -7567,6 +7599,19 @@ int vmx_vcpu_create(struct kvm_vcpu *vcpu)
> goto free_vmcs;
> }
>
> + if (vmcs_config.cpu_based_2nd_exec_ctrl & SECONDARY_EXEC_EPT_VIOLATION_VE) {
> + struct page *page;
> +
> + BUILD_BUG_ON(sizeof(*vmx->ve_info) > PAGE_SIZE);
> +
> + /* ve_info must be page aligned. */
> + page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);
> + if (page)
Can we please just treat this as an error. The odds of us screwing up checks
against vmx->ve_info are higher than the odds of someone enabling KVM_INTEL_PROVE_VE
on a machine with such high memory pressure that a 4KiB allocation fails, all
subequent memory allocations succeeding, *and* caring that VM creation fails.
The pr_err() in the failure path is even more ridiculous.
> + vmx->ve_info = page_to_virt(page);
> + else
> + pr_err("Failed to allocate ve_info. disabling EPT_VIOLATION_VE.\n");
> + }
> +
> if (vmx_can_use_ipiv(vcpu))
> WRITE_ONCE(to_kvm_vmx(vcpu->kvm)->pid_table[vcpu->vcpu_id],
> __pa(&vmx->pi_desc) | PID_TABLE_ENTRY_VALID);
