| Message ID | 20200812180615.22372-1-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: nVMX: Add VM-Enter failed tracepoints for super early checks | expand |
On Tue, Sep 01, 2020 at 10:21:15AM +0200, Vitaly Kuznetsov wrote: > Sean Christopherson <sean.j.christopherson@intel.com> writes: > > > Add tracepoints for the early consistency checks in nested_vmx_run(). > > The "VMLAUNCH vs. VMRESUME" check in particular is useful to trace, as > > there is no architectural way to check VMCS.LAUNCH_STATE, and subtle > > bugs such as VMCLEAR on the wrong HPA can lead to confusing errors in > > the L1 VMM. > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > --- > > arch/x86/kvm/vmx/nested.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > > index 23b58c28a1c92..fb37f0972e78a 100644 > > --- a/arch/x86/kvm/vmx/nested.c > > +++ b/arch/x86/kvm/vmx/nested.c > > @@ -3468,11 +3468,11 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) > > if (evmptrld_status == EVMPTRLD_ERROR) { > > Would it make sense to add 'CC' here too for, em, consistency? :-) #UD > is probably easy to spot anyway.. I'd prefer not to, purely because it's a #UD and not a VM-Fail.
On 12/08/20 20:06, Sean Christopherson wrote: > Add tracepoints for the early consistency checks in nested_vmx_run(). > The "VMLAUNCH vs. VMRESUME" check in particular is useful to trace, as > there is no architectural way to check VMCS.LAUNCH_STATE, and subtle > bugs such as VMCLEAR on the wrong HPA can lead to confusing errors in > the L1 VMM. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > arch/x86/kvm/vmx/nested.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index 23b58c28a1c92..fb37f0972e78a 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -3468,11 +3468,11 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) > if (evmptrld_status == EVMPTRLD_ERROR) { > kvm_queue_exception(vcpu, UD_VECTOR); > return 1; > - } else if (evmptrld_status == EVMPTRLD_VMFAIL) { > + } else if (CC(evmptrld_status == EVMPTRLD_VMFAIL)) { > return nested_vmx_failInvalid(vcpu); > } > > - if (!vmx->nested.hv_evmcs && vmx->nested.current_vmptr == -1ull) > + if (CC(!vmx->nested.hv_evmcs && vmx->nested.current_vmptr == -1ull)) > return nested_vmx_failInvalid(vcpu); > > vmcs12 = get_vmcs12(vcpu); > @@ -3483,7 +3483,7 @@ 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) > + if (CC(vmcs12->hdr.shadow_vmcs)) > return nested_vmx_failInvalid(vcpu); > > if (vmx->nested.hv_evmcs) { > @@ -3504,10 +3504,10 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) > * for misconfigurations which will anyway be caught by the processor > * when using the merged vmcs02. > */ > - if (interrupt_shadow & KVM_X86_SHADOW_INT_MOV_SS) > + if (CC(interrupt_shadow & KVM_X86_SHADOW_INT_MOV_SS)) > return nested_vmx_fail(vcpu, VMXERR_ENTRY_EVENTS_BLOCKED_BY_MOV_SS); > > - if (vmcs12->launch_state == launch) > + if (CC(vmcs12->launch_state == launch)) > return nested_vmx_fail(vcpu, > launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS > : VMXERR_VMRESUME_NONLAUNCHED_VMCS); > Queued, thanks. Paolo
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 23b58c28a1c92..fb37f0972e78a 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -3468,11 +3468,11 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) if (evmptrld_status == EVMPTRLD_ERROR) { kvm_queue_exception(vcpu, UD_VECTOR); return 1; - } else if (evmptrld_status == EVMPTRLD_VMFAIL) { + } else if (CC(evmptrld_status == EVMPTRLD_VMFAIL)) { return nested_vmx_failInvalid(vcpu); } - if (!vmx->nested.hv_evmcs && vmx->nested.current_vmptr == -1ull) + if (CC(!vmx->nested.hv_evmcs && vmx->nested.current_vmptr == -1ull)) return nested_vmx_failInvalid(vcpu); vmcs12 = get_vmcs12(vcpu); @@ -3483,7 +3483,7 @@ 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) + if (CC(vmcs12->hdr.shadow_vmcs)) return nested_vmx_failInvalid(vcpu); if (vmx->nested.hv_evmcs) { @@ -3504,10 +3504,10 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) * for misconfigurations which will anyway be caught by the processor * when using the merged vmcs02. */ - if (interrupt_shadow & KVM_X86_SHADOW_INT_MOV_SS) + if (CC(interrupt_shadow & KVM_X86_SHADOW_INT_MOV_SS)) return nested_vmx_fail(vcpu, VMXERR_ENTRY_EVENTS_BLOCKED_BY_MOV_SS); - if (vmcs12->launch_state == launch) + if (CC(vmcs12->launch_state == launch)) return nested_vmx_fail(vcpu, launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS : VMXERR_VMRESUME_NONLAUNCHED_VMCS);
Add tracepoints for the early consistency checks in nested_vmx_run(). The "VMLAUNCH vs. VMRESUME" check in particular is useful to trace, as there is no architectural way to check VMCS.LAUNCH_STATE, and subtle bugs such as VMCLEAR on the wrong HPA can lead to confusing errors in the L1 VMM. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kvm/vmx/nested.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)