| Message ID | 20180828160459.14093-16-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: nVMX: add option to perform early consistency checks via H/W | expand |
On Tue, Aug 28, 2018 at 9:04 AM, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > ... as every invocation of nested_vmx_{fail,succeed} is immediately > followed by a call to kvm_skip_emulated_instruction(). This saves > a bit of code and eliminates some silly paths, e.g. nested_vmx_run() > ended up with a goto label purely used to call and return > kvm_skip_emulated_instruction(). > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- Very nice. Reviewed-by: Jim Mattson <jmattson@google.com> > arch/x86/kvm/vmx.c | 199 +++++++++++++++++---------------------------- > 1 file changed, 76 insertions(+), 123 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 8ab028fb1f5f..1edf82632832 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -8054,35 +8054,37 @@ static int handle_monitor(struct kvm_vcpu *vcpu) > > /* > * The following 3 functions, nested_vmx_succeed()/failValid()/failInvalid(), > - * set the success or error code of an emulated VMX instruction, as specified > - * by Vol 2B, VMX Instruction Reference, "Conventions". > + * set the success or error code of an emulated VMX instruction (as specified > + * by Vol 2B, VMX Instruction Reference, "Conventions"), and skip the emulated > + * instruction. > */ > -static void nested_vmx_succeed(struct kvm_vcpu *vcpu) > +static int nested_vmx_succeed(struct kvm_vcpu *vcpu) > { > vmx_set_rflags(vcpu, vmx_get_rflags(vcpu) > & ~(X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF | > X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF)); > + return kvm_skip_emulated_instruction(vcpu); > } > > -static void nested_vmx_failInvalid(struct kvm_vcpu *vcpu) > +static int nested_vmx_failInvalid(struct kvm_vcpu *vcpu) > { > vmx_set_rflags(vcpu, (vmx_get_rflags(vcpu) > & ~(X86_EFLAGS_PF | X86_EFLAGS_AF | X86_EFLAGS_ZF | > X86_EFLAGS_SF | X86_EFLAGS_OF)) > | X86_EFLAGS_CF); > + return kvm_skip_emulated_instruction(vcpu); > } > > -static void nested_vmx_failValid(struct kvm_vcpu *vcpu, > +static int nested_vmx_failValid(struct kvm_vcpu *vcpu, > u32 vm_instruction_error) > { > - if (to_vmx(vcpu)->nested.current_vmptr == -1ull) { > - /* > - * failValid writes the error number to the current VMCS, which > - * can't be done there isn't a current VMCS. > - */ > - nested_vmx_failInvalid(vcpu); > - return; > - } > + /* > + * failValid writes the error number to the current VMCS, which > + * can't be done if there isn't a current VMCS. > + */ > + if (to_vmx(vcpu)->nested.current_vmptr == -1ull) > + return nested_vmx_failInvalid(vcpu); > + > vmx_set_rflags(vcpu, (vmx_get_rflags(vcpu) > & ~(X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF | > X86_EFLAGS_SF | X86_EFLAGS_OF)) > @@ -8092,6 +8094,7 @@ static void nested_vmx_failValid(struct kvm_vcpu *vcpu, > * We don't need to force a shadow sync because > * VM_INSTRUCTION_ERROR is not shadowed > */ > + return kvm_skip_emulated_instruction(vcpu); > } > > static void nested_vmx_abort(struct kvm_vcpu *vcpu, u32 indicator) > @@ -8333,8 +8336,8 @@ static int handle_vmon(struct kvm_vcpu *vcpu) > } > > if (vmx->nested.vmxon) { > - nested_vmx_failValid(vcpu, VMXERR_VMXON_IN_VMX_ROOT_OPERATION); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_failValid(vcpu, > + VMXERR_VMXON_IN_VMX_ROOT_OPERATION); > } > > if ((vmx->msr_ia32_feature_control & VMXON_NEEDED_FEATURES) > @@ -8354,21 +8357,17 @@ static int handle_vmon(struct kvm_vcpu *vcpu) > * Note - IA32_VMX_BASIC[48] will never be 1 for the nested case; > * which replaces physical address width with 32 > */ > - if (!PAGE_ALIGNED(vmptr) || (vmptr >> cpuid_maxphyaddr(vcpu))) { > - nested_vmx_failInvalid(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > - } > + if (!PAGE_ALIGNED(vmptr) || (vmptr >> cpuid_maxphyaddr(vcpu))) > + return nested_vmx_failInvalid(vcpu); > > page = kvm_vcpu_gpa_to_page(vcpu, vmptr); > - if (is_error_page(page)) { > - nested_vmx_failInvalid(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > - } > + if (is_error_page(page)) > + return nested_vmx_failInvalid(vcpu); > + > if (*(u32 *)kmap(page) != VMCS12_REVISION) { > kunmap(page); > kvm_release_page_clean(page); > - nested_vmx_failInvalid(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_failInvalid(vcpu); > } > kunmap(page); > kvm_release_page_clean(page); > @@ -8378,8 +8377,7 @@ static int handle_vmon(struct kvm_vcpu *vcpu) > if (ret) > return ret; > > - nested_vmx_succeed(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_succeed(vcpu); > } > > /* > @@ -8479,8 +8477,7 @@ static int handle_vmoff(struct kvm_vcpu *vcpu) > if (!nested_vmx_check_permission(vcpu)) > return 1; > free_nested(to_vmx(vcpu)); > - nested_vmx_succeed(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_succeed(vcpu); > } > > /* Emulate the VMCLEAR instruction */ > @@ -8497,13 +8494,13 @@ static int handle_vmclear(struct kvm_vcpu *vcpu) > return 1; > > if (!PAGE_ALIGNED(vmptr) || (vmptr >> cpuid_maxphyaddr(vcpu))) { > - nested_vmx_failValid(vcpu, VMXERR_VMCLEAR_INVALID_ADDRESS); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_failValid(vcpu, > + VMXERR_VMCLEAR_INVALID_ADDRESS); > } > > if (vmptr == vmx->nested.vmxon_ptr) { > - nested_vmx_failValid(vcpu, VMXERR_VMCLEAR_VMXON_POINTER); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_failValid(vcpu, > + VMXERR_VMCLEAR_VMXON_POINTER); > } > > if (vmptr == vmx->nested.current_vmptr) > @@ -8513,8 +8510,7 @@ static int handle_vmclear(struct kvm_vcpu *vcpu) > vmptr + offsetof(struct vmcs12, launch_state), > &zero, sizeof(zero)); > > - nested_vmx_succeed(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_succeed(vcpu); > } > > static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch); > @@ -8670,20 +8666,6 @@ static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx) > vmcs_load(vmx->loaded_vmcs->vmcs); > } > > -/* > - * VMX instructions which assume a current vmcs12 (i.e., that VMPTRLD was > - * used before) all generate the same failure when it is missing. > - */ > -static int nested_vmx_check_vmcs12(struct kvm_vcpu *vcpu) > -{ > - struct vcpu_vmx *vmx = to_vmx(vcpu); > - if (vmx->nested.current_vmptr == -1ull) { > - nested_vmx_failInvalid(vcpu); > - return 0; > - } > - return 1; > -} > - > static int handle_vmread(struct kvm_vcpu *vcpu) > { > unsigned long field; > @@ -8696,8 +8678,8 @@ static int handle_vmread(struct kvm_vcpu *vcpu) > if (!nested_vmx_check_permission(vcpu)) > return 1; > > - if (!nested_vmx_check_vmcs12(vcpu)) > - return kvm_skip_emulated_instruction(vcpu); > + if (to_vmx(vcpu)->nested.current_vmptr == -1ull) > + return nested_vmx_failInvalid(vcpu); > > if (!is_guest_mode(vcpu)) > vmcs12 = get_vmcs12(vcpu); > @@ -8706,10 +8688,8 @@ static int handle_vmread(struct kvm_vcpu *vcpu) > * When vmcs->vmcs_link_pointer is -1ull, any VMREAD > * to shadowed-field sets the ALU flags for VMfailInvalid. > */ > - if (get_vmcs12(vcpu)->vmcs_link_pointer == -1ull) { > - nested_vmx_failInvalid(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > - } > + if (get_vmcs12(vcpu)->vmcs_link_pointer == -1ull) > + return nested_vmx_failInvalid(vcpu); > vmcs12 = get_shadow_vmcs12(vcpu); > } > > @@ -8717,9 +8697,10 @@ static int handle_vmread(struct kvm_vcpu *vcpu) > field = kvm_register_readl(vcpu, (((vmx_instruction_info) >> 28) & 0xf)); > /* Read the field, zero-extended to a u64 field_value */ > if (vmcs12_read_any(vmcs12, field, &field_value) < 0) { > - nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_failValid(vcpu, > + VMXERR_UNSUPPORTED_VMCS_COMPONENT); > } Nit: Remove the braces. > + > /* > * Now copy part of this value to register or memory, as requested. > * Note that the number of bits actually copied is 32 or 64 depending > @@ -8737,8 +8718,7 @@ static int handle_vmread(struct kvm_vcpu *vcpu) > (is_long_mode(vcpu) ? 8 : 4), NULL); > } > > - nested_vmx_succeed(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_succeed(vcpu); > } > > > @@ -8763,8 +8743,8 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) > if (!nested_vmx_check_permission(vcpu)) > return 1; > > - if (!nested_vmx_check_vmcs12(vcpu)) > - return kvm_skip_emulated_instruction(vcpu); > + if (vmx->nested.current_vmptr == -1ull) > + return nested_vmx_failInvalid(vcpu); > > if (vmx_instruction_info & (1u << 10)) > field_value = kvm_register_readl(vcpu, > @@ -8788,9 +8768,8 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) > */ > if (vmcs_field_readonly(field) && > !nested_cpu_has_vmwrite_any_field(vcpu)) { > - nested_vmx_failValid(vcpu, > + return nested_vmx_failValid(vcpu, > VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT); > - return kvm_skip_emulated_instruction(vcpu); > } Nit: Remove the braces. > > if (!is_guest_mode(vcpu)) > @@ -8800,17 +8779,14 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) > * When vmcs->vmcs_link_pointer is -1ull, any VMWRITE > * to shadowed-field sets the ALU flags for VMfailInvalid. > */ > - if (get_vmcs12(vcpu)->vmcs_link_pointer == -1ull) { > - nested_vmx_failInvalid(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > - } > + if (get_vmcs12(vcpu)->vmcs_link_pointer == -1ull) > + return nested_vmx_failInvalid(vcpu); > vmcs12 = get_shadow_vmcs12(vcpu); > - > } > > if (vmcs12_write_any(vmcs12, field, field_value) < 0) { > - nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_failValid(vcpu, > + VMXERR_UNSUPPORTED_VMCS_COMPONENT); > } Nit: Remove the braces. > > /* > @@ -8833,8 +8809,7 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) > } > } > > - nested_vmx_succeed(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_succeed(vcpu); > } > > static void set_current_vmptr(struct vcpu_vmx *vmx, gpa_t vmptr) > @@ -8863,32 +8838,30 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu) > return 1; > > if (!PAGE_ALIGNED(vmptr) || (vmptr >> cpuid_maxphyaddr(vcpu))) { > - nested_vmx_failValid(vcpu, VMXERR_VMPTRLD_INVALID_ADDRESS); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_failValid(vcpu, > + VMXERR_VMPTRLD_INVALID_ADDRESS); > } Nit: Braces. > > if (vmptr == vmx->nested.vmxon_ptr) { > - nested_vmx_failValid(vcpu, VMXERR_VMPTRLD_VMXON_POINTER); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_failValid(vcpu, > + VMXERR_VMPTRLD_VMXON_POINTER); > } Nit: braces. > > if (vmx->nested.current_vmptr != vmptr) { > struct vmcs12 *new_vmcs12; > struct page *page; > page = kvm_vcpu_gpa_to_page(vcpu, vmptr); > - if (is_error_page(page)) { > - nested_vmx_failInvalid(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > - } > + if (is_error_page(page)) > + return nested_vmx_failInvalid(vcpu); > + > new_vmcs12 = kmap(page); > if (new_vmcs12->hdr.revision_id != VMCS12_REVISION || > (new_vmcs12->hdr.shadow_vmcs && > !nested_cpu_has_vmx_shadow_vmcs(vcpu))) { > kunmap(page); > kvm_release_page_clean(page); > - nested_vmx_failValid(vcpu, > + return nested_vmx_failValid(vcpu, > VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID); > - return kvm_skip_emulated_instruction(vcpu); > } > > nested_release_vmcs12(vmx); > @@ -8903,8 +8876,7 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu) > set_current_vmptr(vmx, vmptr); > } > > - nested_vmx_succeed(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_succeed(vcpu); > } > > /* Emulate the VMPTRST instruction */ > @@ -8927,8 +8899,7 @@ static int handle_vmptrst(struct kvm_vcpu *vcpu) > kvm_inject_page_fault(vcpu, &e); > return 1; > } > - nested_vmx_succeed(vcpu); > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_succeed(vcpu); > } > > /* Emulate the INVEPT instruction */ > @@ -8959,9 +8930,8 @@ static int handle_invept(struct kvm_vcpu *vcpu) > types = (vmx->nested.msrs.ept_caps >> VMX_EPT_EXTENT_SHIFT) & 6; > > if (type >= 32 || !(types & (1 << type))) { > - nested_vmx_failValid(vcpu, > + return nested_vmx_failValid(vcpu, > VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); > - return kvm_skip_emulated_instruction(vcpu); > } Nit: Braces. > > /* According to the Intel VMX instruction reference, the memory > @@ -8984,14 +8954,13 @@ static int handle_invept(struct kvm_vcpu *vcpu) > case VMX_EPT_EXTENT_CONTEXT: > kvm_mmu_sync_roots(vcpu); > kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu); > - nested_vmx_succeed(vcpu); > break; > default: > BUG_ON(1); > break; > } > > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_succeed(vcpu); > } > > static int handle_invvpid(struct kvm_vcpu *vcpu) > @@ -9023,9 +8992,8 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) > VMX_VPID_EXTENT_SUPPORTED_MASK) >> 8; > > if (type >= 32 || !(types & (1 << type))) { > - nested_vmx_failValid(vcpu, > + return nested_vmx_failValid(vcpu, > VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); > - return kvm_skip_emulated_instruction(vcpu); > } Nit: Braces. > > /* according to the intel vmx instruction reference, the memory > @@ -9039,19 +9007,18 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) > return 1; > } > if (operand.vpid >> 16) { > - nested_vmx_failValid(vcpu, > + return nested_vmx_failValid(vcpu, > VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); > - return kvm_skip_emulated_instruction(vcpu); > } Nit: Braces. > > switch (type) { > case VMX_VPID_EXTENT_INDIVIDUAL_ADDR: > if (!operand.vpid || > is_noncanonical_address(operand.gla, vcpu)) { > - nested_vmx_failValid(vcpu, > + return nested_vmx_failValid(vcpu, > VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); > - return kvm_skip_emulated_instruction(vcpu); > } Nit: Braces. > + > if (cpu_has_vmx_invvpid_individual_addr() && > vmx->nested.vpid02) { > __invvpid(VMX_VPID_EXTENT_INDIVIDUAL_ADDR, > @@ -9062,9 +9029,8 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) > case VMX_VPID_EXTENT_SINGLE_CONTEXT: > case VMX_VPID_EXTENT_SINGLE_NON_GLOBAL: > if (!operand.vpid) { > - nested_vmx_failValid(vcpu, > + return nested_vmx_failValid(vcpu, > VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); > - return kvm_skip_emulated_instruction(vcpu); > } Nit: Braces. > __vmx_flush_tlb(vcpu, vmx->nested.vpid02, true); > break; > @@ -9076,9 +9042,7 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) > return kvm_skip_emulated_instruction(vcpu); > } > > - nested_vmx_succeed(vcpu); > - > - return kvm_skip_emulated_instruction(vcpu); > + return nested_vmx_succeed(vcpu); > } > > static int handle_invpcid(struct kvm_vcpu *vcpu) > @@ -12686,8 +12650,8 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) > if (!nested_vmx_check_permission(vcpu)) > return 1; > > - if (!nested_vmx_check_vmcs12(vcpu)) > - goto out; > + if (vmx->nested.current_vmptr == -1ull) > + return nested_vmx_failInvalid(vcpu); > > vmcs12 = get_vmcs12(vcpu); > > @@ -12697,10 +12661,8 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) > * rather than RFLAGS.ZF, and no error number is stored to the > * VM-instruction error field. > */ > - if (vmcs12->hdr.shadow_vmcs) { > - nested_vmx_failInvalid(vcpu); > - goto out; > - } > + if (vmcs12->hdr.shadow_vmcs) > + return nested_vmx_failInvalid(vcpu); > > if (enable_shadow_vmcs) > copy_shadow_to_vmcs12(vmx); > @@ -12716,23 +12678,19 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) > * when using the merged vmcs02. > */ > if (interrupt_shadow & KVM_X86_SHADOW_INT_MOV_SS) { > - nested_vmx_failValid(vcpu, > - VMXERR_ENTRY_EVENTS_BLOCKED_BY_MOV_SS); > - goto out; > + return nested_vmx_failValid(vcpu, > + VMXERR_ENTRY_EVENTS_BLOCKED_BY_MOV_SS); > } Nit: Braces. > > if (vmcs12->launch_state == launch) { > - nested_vmx_failValid(vcpu, > + return nested_vmx_failValid(vcpu, > launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS > : VMXERR_VMRESUME_NONLAUNCHED_VMCS); > - goto out; > } Nit: Braces.
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 8ab028fb1f5f..1edf82632832 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -8054,35 +8054,37 @@ static int handle_monitor(struct kvm_vcpu *vcpu) /* * The following 3 functions, nested_vmx_succeed()/failValid()/failInvalid(), - * set the success or error code of an emulated VMX instruction, as specified - * by Vol 2B, VMX Instruction Reference, "Conventions". + * set the success or error code of an emulated VMX instruction (as specified + * by Vol 2B, VMX Instruction Reference, "Conventions"), and skip the emulated + * instruction. */ -static void nested_vmx_succeed(struct kvm_vcpu *vcpu) +static int nested_vmx_succeed(struct kvm_vcpu *vcpu) { vmx_set_rflags(vcpu, vmx_get_rflags(vcpu) & ~(X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF | X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF)); + return kvm_skip_emulated_instruction(vcpu); } -static void nested_vmx_failInvalid(struct kvm_vcpu *vcpu) +static int nested_vmx_failInvalid(struct kvm_vcpu *vcpu) { vmx_set_rflags(vcpu, (vmx_get_rflags(vcpu) & ~(X86_EFLAGS_PF | X86_EFLAGS_AF | X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF)) | X86_EFLAGS_CF); + return kvm_skip_emulated_instruction(vcpu); } -static void nested_vmx_failValid(struct kvm_vcpu *vcpu, +static int nested_vmx_failValid(struct kvm_vcpu *vcpu, u32 vm_instruction_error) { - if (to_vmx(vcpu)->nested.current_vmptr == -1ull) { - /* - * failValid writes the error number to the current VMCS, which - * can't be done there isn't a current VMCS. - */ - nested_vmx_failInvalid(vcpu); - return; - } + /* + * failValid writes the error number to the current VMCS, which + * can't be done if there isn't a current VMCS. + */ + if (to_vmx(vcpu)->nested.current_vmptr == -1ull) + return nested_vmx_failInvalid(vcpu); + vmx_set_rflags(vcpu, (vmx_get_rflags(vcpu) & ~(X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF | X86_EFLAGS_SF | X86_EFLAGS_OF)) @@ -8092,6 +8094,7 @@ static void nested_vmx_failValid(struct kvm_vcpu *vcpu, * We don't need to force a shadow sync because * VM_INSTRUCTION_ERROR is not shadowed */ + return kvm_skip_emulated_instruction(vcpu); } static void nested_vmx_abort(struct kvm_vcpu *vcpu, u32 indicator) @@ -8333,8 +8336,8 @@ static int handle_vmon(struct kvm_vcpu *vcpu) } if (vmx->nested.vmxon) { - nested_vmx_failValid(vcpu, VMXERR_VMXON_IN_VMX_ROOT_OPERATION); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_failValid(vcpu, + VMXERR_VMXON_IN_VMX_ROOT_OPERATION); } if ((vmx->msr_ia32_feature_control & VMXON_NEEDED_FEATURES) @@ -8354,21 +8357,17 @@ static int handle_vmon(struct kvm_vcpu *vcpu) * Note - IA32_VMX_BASIC[48] will never be 1 for the nested case; * which replaces physical address width with 32 */ - if (!PAGE_ALIGNED(vmptr) || (vmptr >> cpuid_maxphyaddr(vcpu))) { - nested_vmx_failInvalid(vcpu); - return kvm_skip_emulated_instruction(vcpu); - } + if (!PAGE_ALIGNED(vmptr) || (vmptr >> cpuid_maxphyaddr(vcpu))) + return nested_vmx_failInvalid(vcpu); page = kvm_vcpu_gpa_to_page(vcpu, vmptr); - if (is_error_page(page)) { - nested_vmx_failInvalid(vcpu); - return kvm_skip_emulated_instruction(vcpu); - } + if (is_error_page(page)) + return nested_vmx_failInvalid(vcpu); + if (*(u32 *)kmap(page) != VMCS12_REVISION) { kunmap(page); kvm_release_page_clean(page); - nested_vmx_failInvalid(vcpu); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_failInvalid(vcpu); } kunmap(page); kvm_release_page_clean(page); @@ -8378,8 +8377,7 @@ static int handle_vmon(struct kvm_vcpu *vcpu) if (ret) return ret; - nested_vmx_succeed(vcpu); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_succeed(vcpu); } /* @@ -8479,8 +8477,7 @@ static int handle_vmoff(struct kvm_vcpu *vcpu) if (!nested_vmx_check_permission(vcpu)) return 1; free_nested(to_vmx(vcpu)); - nested_vmx_succeed(vcpu); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_succeed(vcpu); } /* Emulate the VMCLEAR instruction */ @@ -8497,13 +8494,13 @@ static int handle_vmclear(struct kvm_vcpu *vcpu) return 1; if (!PAGE_ALIGNED(vmptr) || (vmptr >> cpuid_maxphyaddr(vcpu))) { - nested_vmx_failValid(vcpu, VMXERR_VMCLEAR_INVALID_ADDRESS); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_failValid(vcpu, + VMXERR_VMCLEAR_INVALID_ADDRESS); } if (vmptr == vmx->nested.vmxon_ptr) { - nested_vmx_failValid(vcpu, VMXERR_VMCLEAR_VMXON_POINTER); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_failValid(vcpu, + VMXERR_VMCLEAR_VMXON_POINTER); } if (vmptr == vmx->nested.current_vmptr) @@ -8513,8 +8510,7 @@ static int handle_vmclear(struct kvm_vcpu *vcpu) vmptr + offsetof(struct vmcs12, launch_state), &zero, sizeof(zero)); - nested_vmx_succeed(vcpu); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_succeed(vcpu); } static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch); @@ -8670,20 +8666,6 @@ static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx) vmcs_load(vmx->loaded_vmcs->vmcs); } -/* - * VMX instructions which assume a current vmcs12 (i.e., that VMPTRLD was - * used before) all generate the same failure when it is missing. - */ -static int nested_vmx_check_vmcs12(struct kvm_vcpu *vcpu) -{ - struct vcpu_vmx *vmx = to_vmx(vcpu); - if (vmx->nested.current_vmptr == -1ull) { - nested_vmx_failInvalid(vcpu); - return 0; - } - return 1; -} - static int handle_vmread(struct kvm_vcpu *vcpu) { unsigned long field; @@ -8696,8 +8678,8 @@ static int handle_vmread(struct kvm_vcpu *vcpu) if (!nested_vmx_check_permission(vcpu)) return 1; - if (!nested_vmx_check_vmcs12(vcpu)) - return kvm_skip_emulated_instruction(vcpu); + if (to_vmx(vcpu)->nested.current_vmptr == -1ull) + return nested_vmx_failInvalid(vcpu); if (!is_guest_mode(vcpu)) vmcs12 = get_vmcs12(vcpu); @@ -8706,10 +8688,8 @@ static int handle_vmread(struct kvm_vcpu *vcpu) * When vmcs->vmcs_link_pointer is -1ull, any VMREAD * to shadowed-field sets the ALU flags for VMfailInvalid. */ - if (get_vmcs12(vcpu)->vmcs_link_pointer == -1ull) { - nested_vmx_failInvalid(vcpu); - return kvm_skip_emulated_instruction(vcpu); - } + if (get_vmcs12(vcpu)->vmcs_link_pointer == -1ull) + return nested_vmx_failInvalid(vcpu); vmcs12 = get_shadow_vmcs12(vcpu); } @@ -8717,9 +8697,10 @@ static int handle_vmread(struct kvm_vcpu *vcpu) field = kvm_register_readl(vcpu, (((vmx_instruction_info) >> 28) & 0xf)); /* Read the field, zero-extended to a u64 field_value */ if (vmcs12_read_any(vmcs12, field, &field_value) < 0) { - nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_failValid(vcpu, + VMXERR_UNSUPPORTED_VMCS_COMPONENT); } + /* * Now copy part of this value to register or memory, as requested. * Note that the number of bits actually copied is 32 or 64 depending @@ -8737,8 +8718,7 @@ static int handle_vmread(struct kvm_vcpu *vcpu) (is_long_mode(vcpu) ? 8 : 4), NULL); } - nested_vmx_succeed(vcpu); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_succeed(vcpu); } @@ -8763,8 +8743,8 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) if (!nested_vmx_check_permission(vcpu)) return 1; - if (!nested_vmx_check_vmcs12(vcpu)) - return kvm_skip_emulated_instruction(vcpu); + if (vmx->nested.current_vmptr == -1ull) + return nested_vmx_failInvalid(vcpu); if (vmx_instruction_info & (1u << 10)) field_value = kvm_register_readl(vcpu, @@ -8788,9 +8768,8 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) */ if (vmcs_field_readonly(field) && !nested_cpu_has_vmwrite_any_field(vcpu)) { - nested_vmx_failValid(vcpu, + return nested_vmx_failValid(vcpu, VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT); - return kvm_skip_emulated_instruction(vcpu); } if (!is_guest_mode(vcpu)) @@ -8800,17 +8779,14 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) * When vmcs->vmcs_link_pointer is -1ull, any VMWRITE * to shadowed-field sets the ALU flags for VMfailInvalid. */ - if (get_vmcs12(vcpu)->vmcs_link_pointer == -1ull) { - nested_vmx_failInvalid(vcpu); - return kvm_skip_emulated_instruction(vcpu); - } + if (get_vmcs12(vcpu)->vmcs_link_pointer == -1ull) + return nested_vmx_failInvalid(vcpu); vmcs12 = get_shadow_vmcs12(vcpu); - } if (vmcs12_write_any(vmcs12, field, field_value) < 0) { - nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_failValid(vcpu, + VMXERR_UNSUPPORTED_VMCS_COMPONENT); } /* @@ -8833,8 +8809,7 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) } } - nested_vmx_succeed(vcpu); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_succeed(vcpu); } static void set_current_vmptr(struct vcpu_vmx *vmx, gpa_t vmptr) @@ -8863,32 +8838,30 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu) return 1; if (!PAGE_ALIGNED(vmptr) || (vmptr >> cpuid_maxphyaddr(vcpu))) { - nested_vmx_failValid(vcpu, VMXERR_VMPTRLD_INVALID_ADDRESS); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_failValid(vcpu, + VMXERR_VMPTRLD_INVALID_ADDRESS); } if (vmptr == vmx->nested.vmxon_ptr) { - nested_vmx_failValid(vcpu, VMXERR_VMPTRLD_VMXON_POINTER); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_failValid(vcpu, + VMXERR_VMPTRLD_VMXON_POINTER); } if (vmx->nested.current_vmptr != vmptr) { struct vmcs12 *new_vmcs12; struct page *page; page = kvm_vcpu_gpa_to_page(vcpu, vmptr); - if (is_error_page(page)) { - nested_vmx_failInvalid(vcpu); - return kvm_skip_emulated_instruction(vcpu); - } + if (is_error_page(page)) + return nested_vmx_failInvalid(vcpu); + new_vmcs12 = kmap(page); if (new_vmcs12->hdr.revision_id != VMCS12_REVISION || (new_vmcs12->hdr.shadow_vmcs && !nested_cpu_has_vmx_shadow_vmcs(vcpu))) { kunmap(page); kvm_release_page_clean(page); - nested_vmx_failValid(vcpu, + return nested_vmx_failValid(vcpu, VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID); - return kvm_skip_emulated_instruction(vcpu); } nested_release_vmcs12(vmx); @@ -8903,8 +8876,7 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu) set_current_vmptr(vmx, vmptr); } - nested_vmx_succeed(vcpu); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_succeed(vcpu); } /* Emulate the VMPTRST instruction */ @@ -8927,8 +8899,7 @@ static int handle_vmptrst(struct kvm_vcpu *vcpu) kvm_inject_page_fault(vcpu, &e); return 1; } - nested_vmx_succeed(vcpu); - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_succeed(vcpu); } /* Emulate the INVEPT instruction */ @@ -8959,9 +8930,8 @@ static int handle_invept(struct kvm_vcpu *vcpu) types = (vmx->nested.msrs.ept_caps >> VMX_EPT_EXTENT_SHIFT) & 6; if (type >= 32 || !(types & (1 << type))) { - nested_vmx_failValid(vcpu, + return nested_vmx_failValid(vcpu, VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); - return kvm_skip_emulated_instruction(vcpu); } /* According to the Intel VMX instruction reference, the memory @@ -8984,14 +8954,13 @@ static int handle_invept(struct kvm_vcpu *vcpu) case VMX_EPT_EXTENT_CONTEXT: kvm_mmu_sync_roots(vcpu); kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu); - nested_vmx_succeed(vcpu); break; default: BUG_ON(1); break; } - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_succeed(vcpu); } static int handle_invvpid(struct kvm_vcpu *vcpu) @@ -9023,9 +8992,8 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) VMX_VPID_EXTENT_SUPPORTED_MASK) >> 8; if (type >= 32 || !(types & (1 << type))) { - nested_vmx_failValid(vcpu, + return nested_vmx_failValid(vcpu, VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); - return kvm_skip_emulated_instruction(vcpu); } /* according to the intel vmx instruction reference, the memory @@ -9039,19 +9007,18 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) return 1; } if (operand.vpid >> 16) { - nested_vmx_failValid(vcpu, + return nested_vmx_failValid(vcpu, VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); - return kvm_skip_emulated_instruction(vcpu); } switch (type) { case VMX_VPID_EXTENT_INDIVIDUAL_ADDR: if (!operand.vpid || is_noncanonical_address(operand.gla, vcpu)) { - nested_vmx_failValid(vcpu, + return nested_vmx_failValid(vcpu, VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); - return kvm_skip_emulated_instruction(vcpu); } + if (cpu_has_vmx_invvpid_individual_addr() && vmx->nested.vpid02) { __invvpid(VMX_VPID_EXTENT_INDIVIDUAL_ADDR, @@ -9062,9 +9029,8 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) case VMX_VPID_EXTENT_SINGLE_CONTEXT: case VMX_VPID_EXTENT_SINGLE_NON_GLOBAL: if (!operand.vpid) { - nested_vmx_failValid(vcpu, + return nested_vmx_failValid(vcpu, VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); - return kvm_skip_emulated_instruction(vcpu); } __vmx_flush_tlb(vcpu, vmx->nested.vpid02, true); break; @@ -9076,9 +9042,7 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) return kvm_skip_emulated_instruction(vcpu); } - nested_vmx_succeed(vcpu); - - return kvm_skip_emulated_instruction(vcpu); + return nested_vmx_succeed(vcpu); } static int handle_invpcid(struct kvm_vcpu *vcpu) @@ -12686,8 +12650,8 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) if (!nested_vmx_check_permission(vcpu)) return 1; - if (!nested_vmx_check_vmcs12(vcpu)) - goto out; + if (vmx->nested.current_vmptr == -1ull) + return nested_vmx_failInvalid(vcpu); vmcs12 = get_vmcs12(vcpu); @@ -12697,10 +12661,8 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) * rather than RFLAGS.ZF, and no error number is stored to the * VM-instruction error field. */ - if (vmcs12->hdr.shadow_vmcs) { - nested_vmx_failInvalid(vcpu); - goto out; - } + if (vmcs12->hdr.shadow_vmcs) + return nested_vmx_failInvalid(vcpu); if (enable_shadow_vmcs) copy_shadow_to_vmcs12(vmx); @@ -12716,23 +12678,19 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) * when using the merged vmcs02. */ if (interrupt_shadow & KVM_X86_SHADOW_INT_MOV_SS) { - nested_vmx_failValid(vcpu, - VMXERR_ENTRY_EVENTS_BLOCKED_BY_MOV_SS); - goto out; + return nested_vmx_failValid(vcpu, + VMXERR_ENTRY_EVENTS_BLOCKED_BY_MOV_SS); } if (vmcs12->launch_state == launch) { - nested_vmx_failValid(vcpu, + return nested_vmx_failValid(vcpu, launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS : VMXERR_VMRESUME_NONLAUNCHED_VMCS); - goto out; } ret = check_vmentry_prereqs(vcpu, vmcs12); - if (ret) { - nested_vmx_failValid(vcpu, ret); - goto out; - } + if (ret) + return nested_vmx_failValid(vcpu, ret); /* * We're finally done with prerequisite checking, and can start with @@ -12771,9 +12729,6 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) return kvm_vcpu_halt(vcpu); } return 1; - -out: - return kvm_skip_emulated_instruction(vcpu); } /* @@ -13376,7 +13331,7 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason, return; } - + /* * After an early L2 VM-entry failure, we're now back * in L1 which thinks it just finished a VMLAUNCH or @@ -13384,9 +13339,7 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason, * flag and the VM-instruction error field of the VMCS * accordingly, and skip the emulated instruction. */ - nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_CONTROL_FIELD); - - kvm_skip_emulated_instruction(vcpu); + (void)nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_CONTROL_FIELD); load_vmcs12_mmu_host_state(vcpu, vmcs12);
... as every invocation of nested_vmx_{fail,succeed} is immediately followed by a call to kvm_skip_emulated_instruction(). This saves a bit of code and eliminates some silly paths, e.g. nested_vmx_run() ended up with a goto label purely used to call and return kvm_skip_emulated_instruction(). Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kvm/vmx.c | 199 +++++++++++++++++---------------------------- 1 file changed, 76 insertions(+), 123 deletions(-)