On Thu, Sep 20, 2018 at 10:05:19AM -0700, Jim Mattson wrote:
> On Tue, Aug 28, 2018 at 9:04 AM, Sean Christopherson
> <sean.j.christopherson@xxxxxxxxx> wrote:
> > In preparation of supporting checkpoint/restore for nested state,
> > commit ca0bde28f2ed ("kvm: nVMX: Split VMCS checks from nested_vmx_run()")
> > modified check_vmentry_postreqs() to only perform the guest EFER
> > consistency checks when nested_run_pending is true. But, in the
> > normal nested VMEntry flow, nested_run_pending is only set after
> > check_vmentry_postreqs(), i.e. the consistency check is being skipped.
> >
> > Alternatively, nested_run_pending could be set prior to calling
> > check_vmentry_postreqs() in nested_vmx_run(), but placing the
> > consistency checks in nested_vmx_enter_non_root_mode() allows us
> > to split prepare_vmcs02() and interleave the preparation with
> > the consistency checks without having to change the call sites
> > of nested_vmx_enter_non_root_mode(). In other words, the rest
> > of the consistency check code in nested_vmx_run() will be joining
> > the postreqs checks in future patches.
> >
> > Fixes: ca0bde28f2ed ("kvm: nVMX: Split VMCS checks from nested_vmx_run()")
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> > Cc: Jim Mattson <jmattson@xxxxxxxxxx>
> > ---
> > arch/x86/kvm/vmx.c | 18 ++++++++++--------
> > 1 file changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 5fe44462f713..43e87a2e172e 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -12556,7 +12556,16 @@ static int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, u32 *exit_qual)
> > struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
> > bool from_vmentry = !!exit_qual;
> > u32 dummy_exit_qual;
> > - int r = 0;
> > + int r;
> > +
> > + if (from_vmentry) {
> > + r = check_vmentry_postreqs(vcpu, vmcs12, exit_qual);
> > + if (r) {
> > + nested_vmx_entry_failure(vcpu, vmcs12,
> > + EXIT_REASON_INVALID_STATE, *exit_qual);
> > + return 1;
> > + }
> > + }
>
> Can this be simplified to:
>
> ...
> if (r)
> return EXIT_REASON_INVALID_STATE;
Can and should, nice catch!
> Reviewed-by: Jim Mattson <jmattson@xxxxxxxxxx>
