| Message ID | 20180828160459.14093-9-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: > 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@intel.com> > Cc: Jim Mattson <jmattson@google.com> > --- > 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; Reviewed-by: Jim Mattson <jmattson@google.com>
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@intel.com> 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@intel.com> > > Cc: Jim Mattson <jmattson@google.com> > > --- > > 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@google.com>
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; + } + } enter_guest_mode(vcpu); @@ -12681,13 +12690,6 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) */ skip_emulated_instruction(vcpu); - ret = check_vmentry_postreqs(vcpu, vmcs12, &exit_qual); - if (ret) { - nested_vmx_entry_failure(vcpu, vmcs12, - EXIT_REASON_INVALID_STATE, exit_qual); - return 1; - } - /* * We're finally done with prerequisite checking, and can start with * the nested entry.
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@intel.com> Cc: Jim Mattson <jmattson@google.com> --- arch/x86/kvm/vmx.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)