Convert all top-level nested VM-Enter consistency check functions to use explicit parameters to pass failure information to the caller. Using an explicit parameter achieves several goals: - Provides consistent prototypes for all functions. - Self-documents the net effect of failure, e.g. without the explicit parameter it may not be obvious that nested_vmx_check_guest_state() leads to a VM-Exit. - Does not give the false impression that failure information is always consumed and/or relevant, e.g. vmx_set_nested_state() only cares whether or not the checks were successful. Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx> --- arch/x86/kvm/vmx/nested.c | 61 ++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index b22605d5ee9e..16cff40456ee 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -817,7 +817,8 @@ static int nested_vmx_store_msr_check(struct kvm_vcpu *vcpu, * Load guest's/host's msr at nested entry/exit. * return 0 for success, entry index for failure. */ -static u32 nested_vmx_load_msr(struct kvm_vcpu *vcpu, u64 gpa, u32 count) +static int nested_vmx_load_msr(struct kvm_vcpu *vcpu, u64 gpa, u32 count, + u32 *exit_reason, u32 *exit_qual) { u32 i; struct vmx_msr_entry e; @@ -849,7 +850,9 @@ static u32 nested_vmx_load_msr(struct kvm_vcpu *vcpu, u64 gpa, u32 count) } return 0; fail: - return i + 1; + *exit_reason = EXIT_REASON_MSR_LOAD_FAIL; + *exit_qual = i + 1; + return -EINVAL; } static int nested_vmx_store_msr(struct kvm_vcpu *vcpu, u64 gpa, u32 count) @@ -2574,12 +2577,15 @@ static int nested_check_vm_entry_controls(struct kvm_vcpu *vcpu, } static int nested_vmx_check_controls(struct kvm_vcpu *vcpu, - struct vmcs12 *vmcs12) + struct vmcs12 *vmcs12, + u32 *vm_instruction_error) { + *vm_instruction_error = VMXERR_ENTRY_INVALID_CONTROL_FIELD; + if (nested_check_vm_execution_controls(vcpu, vmcs12) || nested_check_vm_exit_controls(vcpu, vmcs12) || nested_check_vm_entry_controls(vcpu, vmcs12)) - return VMXERR_ENTRY_INVALID_CONTROL_FIELD; + return -EINVAL; return 0; } @@ -2624,10 +2630,13 @@ static int nested_check_host_control_regs(struct kvm_vcpu *vcpu, } static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu, - struct vmcs12 *vmcs12) + struct vmcs12 *vmcs12, + u32 *vm_instruction_error) { + *vm_instruction_error = VMXERR_ENTRY_INVALID_HOST_STATE_FIELD; + if (nested_check_host_control_regs(vcpu, vmcs12)) - return VMXERR_ENTRY_INVALID_HOST_STATE_FIELD; + return -EINVAL; return 0; } @@ -2673,10 +2682,12 @@ static int nested_check_guest_non_reg_state(struct vmcs12 *vmcs12) static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, + u32 *exit_reason, u32 *exit_qual) { bool ia32e; + *exit_reason = EXIT_REASON_INVALID_STATE; *exit_qual = ENTRY_FAIL_DEFAULT; if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) || @@ -2965,7 +2976,7 @@ int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry) struct vcpu_vmx *vmx = to_vmx(vcpu); struct vmcs12 *vmcs12 = get_vmcs12(vcpu); bool evaluate_pending_interrupts; - u32 exit_reason = EXIT_REASON_INVALID_STATE; + u32 exit_reason; u32 exit_qual; evaluate_pending_interrupts = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL) & @@ -2991,7 +3002,8 @@ int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry) return -1; } - if (nested_vmx_check_guest_state(vcpu, vmcs12, &exit_qual)) + if (nested_vmx_check_guest_state(vcpu, vmcs12, + &exit_reason, &exit_qual)) goto vmentry_fail_vmexit; } @@ -3003,11 +3015,9 @@ int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry) goto vmentry_fail_vmexit_guest_mode; if (from_vmentry) { - exit_reason = EXIT_REASON_MSR_LOAD_FAIL; - exit_qual = nested_vmx_load_msr(vcpu, - vmcs12->vm_entry_msr_load_addr, - vmcs12->vm_entry_msr_load_count); - if (exit_qual) + if (nested_vmx_load_msr(vcpu, vmcs12->vm_entry_msr_load_addr, + vmcs12->vm_entry_msr_load_count, + &exit_reason, &exit_qual)) goto vmentry_fail_vmexit_guest_mode; } else { /* @@ -3087,6 +3097,7 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) struct vmcs12 *vmcs12; struct vcpu_vmx *vmx = to_vmx(vcpu); u32 interrupt_shadow = vmx_get_interrupt_shadow(vcpu); + u32 vm_instruction_error; int ret; if (!nested_vmx_check_permission(vcpu)) @@ -3136,13 +3147,11 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS : VMXERR_VMRESUME_NONLAUNCHED_VMCS); - ret = nested_vmx_check_controls(vcpu, vmcs12); - if (ret) - return nested_vmx_failValid(vcpu, ret); + if (nested_vmx_check_controls(vcpu, vmcs12, &vm_instruction_error)) + return nested_vmx_failValid(vcpu, vm_instruction_error); - ret = nested_vmx_check_host_state(vcpu, vmcs12); - if (ret) - return nested_vmx_failValid(vcpu, ret); + if (nested_vmx_check_host_state(vcpu, vmcs12, &vm_instruction_error)) + return nested_vmx_failValid(vcpu, vm_instruction_error); /* * We're finally done with prerequisite checking, and can start with @@ -3594,7 +3603,7 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12) { struct kvm_segment seg; - u32 entry_failure_code; + u32 ign; if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) vcpu->arch.efer = vmcs12->host_ia32_efer; @@ -3629,7 +3638,7 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu, * Only PDPTE load can fail as the value of cr3 was checked on entry and * couldn't have changed. */ - if (nested_vmx_load_cr3(vcpu, vmcs12->host_cr3, false, &entry_failure_code)) + if (nested_vmx_load_cr3(vcpu, vmcs12->host_cr3, false, &ign)) nested_vmx_abort(vcpu, VMX_ABORT_LOAD_HOST_PDPTE_FAIL); if (!enable_ept) @@ -3727,7 +3736,7 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu, vmx_update_msr_bitmap(vcpu); if (nested_vmx_load_msr(vcpu, vmcs12->vm_exit_msr_load_addr, - vmcs12->vm_exit_msr_load_count)) + vmcs12->vm_exit_msr_load_count, &ign, &ign)) nested_vmx_abort(vcpu, VMX_ABORT_LOAD_HOST_MSR_FAIL); } @@ -5352,7 +5361,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, { struct vcpu_vmx *vmx = to_vmx(vcpu); struct vmcs12 *vmcs12; - u32 exit_qual; + u32 ign; int ret; if (kvm_state->format != 0) @@ -5465,9 +5474,9 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, return -EINVAL; } - if (nested_vmx_check_controls(vcpu, vmcs12) || - nested_vmx_check_host_state(vcpu, vmcs12) || - nested_vmx_check_guest_state(vcpu, vmcs12, &exit_qual)) + if (nested_vmx_check_controls(vcpu, vmcs12, &ign) || + nested_vmx_check_host_state(vcpu, vmcs12, &ign) || + nested_vmx_check_guest_state(vcpu, vmcs12, &ign, &ign)) return -EINVAL; vmx->nested.dirty_vmcs12 = true; -- 2.21.0