Move nested_vmx_load_msr() into prepare_vmcs02() and remove the "fail" label now that the inner section of nested_vmx_enter_non_root_mode() has a single error path. Move the call to vmx_segment_cache_clear() above the call to enter_guest_mode() so that it is explicitly clear that the state being restored in the prepare_vmcs02() error path is the state that is set immediately before the call to prepare_vmcs02. Eliminating the "fail" label resolves the oddity of having multiple entry points into the consistency check VMExit handling in nested_vmx_enter_non_root_mode() and reduces the probability of breaking the related code (speaking from experience). Pass exit_reason to check_vmentry_postreqs() and prepare_vmcs02(), and unconditionally set exit_reason and exit_qual in both functions. This fixes a subtle bug where exit_qual would be used uninitialized when vmx->nested.nested_run_pending is false and the MSR load fails, and hopefully prevents such issues in the future, e.g. a similar bug from the past: https://www.spinics.net/lists/kvm/msg165988.html. Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx> --- arch/x86/kvm/vmx.c | 49 ++++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index d5328518b1c6..7c3bdd9deb63 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -11469,10 +11469,13 @@ static void prepare_vmcs02_full(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12) * is assigned to entry_failure_code on failure. */ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, - u32 *entry_failure_code) + u32 *exit_reason, u32 *entry_failure_code) { struct vcpu_vmx *vmx = to_vmx(vcpu); + *exit_reason = EXIT_REASON_INVALID_STATE; + *entry_failure_code = ENTRY_FAIL_DEFAULT; + if (vmx->nested.dirty_vmcs12) { prepare_vmcs02_full(vmx, vmcs12); vmx->nested.dirty_vmcs12 = false; @@ -11572,10 +11575,8 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, * which means L1 attempted VMEntry to L2 with invalid state. * Fail the VMEntry. */ - if (vmx->emulation_required) { - *entry_failure_code = ENTRY_FAIL_DEFAULT; + if (vmx->emulation_required) return 1; - } /* Shadow page tables on either EPT or shadow page tables. */ if (nested_vmx_load_cr3(vcpu, vmcs12->guest_cr3, nested_cpu_has_ept(vmcs12), @@ -11587,6 +11588,12 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, kvm_register_write(vcpu, VCPU_REGS_RSP, vmcs12->guest_rsp); kvm_register_write(vcpu, VCPU_REGS_RIP, vmcs12->guest_rip); + + if (nested_vmx_load_msr(vcpu, vmcs12->vm_entry_msr_load_addr, + vmcs12->vm_entry_msr_load_count)) { + *exit_reason = EXIT_REASON_MSR_LOAD_FAIL; + return 1; + } return 0; } @@ -11753,10 +11760,11 @@ static int check_vmentry_prereqs(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12) } static int check_vmentry_postreqs(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, - u32 *exit_qual) + 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) || @@ -11804,9 +11812,7 @@ static int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu) struct vcpu_vmx *vmx = to_vmx(vcpu); struct vmcs12 *vmcs12 = get_vmcs12(vcpu); bool is_vmentry = vmx->nested.nested_run_pending; - u32 exit_reason = EXIT_REASON_INVALID_STATE; - u32 msr_entry_idx; - u32 exit_qual; + u32 exit_qual, exit_reason; if (!(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_DEBUG_CONTROLS)) vmx->nested.vmcs01_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL); @@ -11820,26 +11826,22 @@ static int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu) if (!is_vmentry) goto enter_non_root_mode; - if (check_vmentry_postreqs(vcpu, vmcs12, &exit_qual)) + if (check_vmentry_postreqs(vcpu, vmcs12, &exit_reason, &exit_qual)) goto consistency_check_vmexit; enter_non_root_mode: + vmx_segment_cache_clear(vmx); + enter_guest_mode(vcpu); - - vmx_segment_cache_clear(vmx); - if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETING) vcpu->arch.tsc_offset += vmcs12->tsc_offset; - if (prepare_vmcs02(vcpu, vmcs12, &exit_qual)) - goto fail; - - exit_reason = EXIT_REASON_MSR_LOAD_FAIL; - msr_entry_idx = nested_vmx_load_msr(vcpu, - vmcs12->vm_entry_msr_load_addr, - vmcs12->vm_entry_msr_load_count); - if (msr_entry_idx) - goto fail; + if (prepare_vmcs02(vcpu, vmcs12, &exit_reason, &exit_qual)) { + if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETING) + vcpu->arch.tsc_offset -= vmcs12->tsc_offset; + leave_guest_mode(vcpu); + goto consistency_check_vmexit; + } /* * Note no nested_vmx_succeed or nested_vmx_fail here. At this point @@ -11849,11 +11851,6 @@ static int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu) */ return 0; -fail: - if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETING) - vcpu->arch.tsc_offset -= vmcs12->tsc_offset; - leave_guest_mode(vcpu); - /* * A consistency check VMExit during L1's VMEnter to L2 is a subset * of a normal VMexit, as explained in 23.7 "VM-entry failures during -- 2.18.0
