>
>- if (cpu_has_secondary_exec_ctrls())
>+ if (cpu_has_secondary_exec_ctrls()) {
> secondary_exec_controls_set(vmx, vmx_secondary_exec_control(vmx));
>+ if (secondary_exec_controls_get(vmx) &
>+ SECONDARY_EXEC_EPT_VIOLATION_VE) {
>+ if (!vmx->ve_info) {
how about allocating ve_info in vmx_vcpu_create()? It is better to me because:
a. symmetry. ve_info is free'd in vmx_vcpu_free().
b. no need to check if this is the first call of init_vmcs(). and ENOMEM can
be returned on allocation failure.
>+ /* ve_info must be page aligned. */
>+ struct page *page;
>+
>+ BUILD_BUG_ON(sizeof(*vmx->ve_info) > PAGE_SIZE);
>+ page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);
>+ if (page)
>+ vmx->ve_info = page_to_virt(page);
>+ }
>+ if (vmx->ve_info) {
>+ /*
>+ * Allow #VE delivery. CPU sets this field to
>+ * 0xFFFFFFFF on #VE delivery. Another #VE can
>+ * occur only if software clears the field.
>+ */
>+ vmx->ve_info->delivery = 0;
Is it necessary to reset ve_info->delivery to 0 given __GFP_ZERO?
>+ vmcs_write64(VE_INFORMATION_ADDRESS,
>+ __pa(vmx->ve_info));
I think the logic here should just be:
if (secondary_exec_controls_get(vmx) & SECONDARY_EXEC_EPT_VIOLATION_VE)
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.
>+ */
>+ pr_err("Failed to allocate ve_info. disabling EPT_VIOLATION_VE.\n");
>+ secondary_exec_controls_clearbit(vmx,
>+ SECONDARY_EXEC_EPT_VIOLATION_VE);
>+ }
>+ }
>+ }
>
> if (cpu_has_tertiary_exec_ctrls())
> tertiary_exec_controls_set(vmx, vmx_tertiary_exec_control(vmx));
>@@ -5200,6 +5243,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.
>+ */
>+ 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 +7523,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((unsigned long)vmx->ve_info);
> }
>
> int vmx_vcpu_create(struct kvm_vcpu *vcpu)
>diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
>index 65786dbe7d60..0da79a386825 100644
>--- a/arch/x86/kvm/vmx/vmx.h
>+++ b/arch/x86/kvm/vmx/vmx.h
>@@ -362,6 +362,9 @@ struct vcpu_vmx {
> DECLARE_BITMAP(read, MAX_POSSIBLE_PASSTHROUGH_MSRS);
> DECLARE_BITMAP(write, MAX_POSSIBLE_PASSTHROUGH_MSRS);
> } shadow_msr_intercept;
>+
>+ /* ve_info must be page aligned. */
this comment is not so useful. I think this should be placed above the call
of alloc_page().
>+ struct vmx_ve_information *ve_info;
> };
>
> struct kvm_vmx {
>@@ -574,7 +577,8 @@ static inline u8 vmx_get_rvi(void)
> SECONDARY_EXEC_ENABLE_VMFUNC | \
> SECONDARY_EXEC_BUS_LOCK_DETECTION | \
> SECONDARY_EXEC_NOTIFY_VM_EXITING | \
>- SECONDARY_EXEC_ENCLS_EXITING)
>+ SECONDARY_EXEC_ENCLS_EXITING | \
>+ SECONDARY_EXEC_EPT_VIOLATION_VE)
>
> #define KVM_REQUIRED_VMX_TERTIARY_VM_EXEC_CONTROL 0
> #define KVM_OPTIONAL_VMX_TERTIARY_VM_EXEC_CONTROL \
>--
>2.43.0
>
>
>
