Date:   Wed, 15 Apr 2020 10:55:09 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Sean Christopherson <sean.j.christopherson@...el.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, Xiaoyao Li <xiaoyao.li@...el.com>
Subject: [PATCH v2 00/10]  KVM: VMX: Unionize vcpu_vmx.exit_reason

Convert the exit_reason field in struct vcpu_vmx from a vanilla u32 to a
union, (ab)using the union to provide access to the basic exit reason and
flags.

There is a fairly substantial delta relative to v1, as I ran with Vitaly's
suggestion to split nested_vmx_exit_reflected() into VM-Fail, "L0 wants"
and "L1 wants", and move the tracepoint into nested_vmx_reflect_vmexit().
IMO, this yields cleaner and more understandable code overall, and helps
eliminate caching the basic exit reason (see below) by avoiding large
functions that repeatedly query the basic exit reason.  The refactoring
isn't strictly related to making exit_reason a union, but the code would
conflict horribly and the end code nicely demonstrates the value of using
a union for the exit reason.

There are three motivating factors for making exit_reason a union:

  - Help avoid bugs where a basic exit reason is compared against the full
    exit reason, e.g. there have been two bugs where MCE_DURING_VMENTRY
    was incorrectly compared against the full exit reason.

  - Clarify the intent of related flows, e.g. exit_reason is used for both
    "basic exit reason" and "full exit reason", and it's not always clear
    which of the two is intended without a fair bit of digging.

  - Prepare for future Intel features, e.g. SGX, that add new exit flags
    that are less restricted than FAILED_VMENTRY, i.e. can be set on what
    is otherwise a standard VM-Exit.

v2:
  - Don't snapshot the basic exit reason, i.e. either use vmx->exit_reason
    directly or snapshot the whole thing.  The resulting code is similar
    to Xiaoyao's original patch, e.g. vmx_handle_exit() now uses
    "exit_reason.basic" instead of "exit_reason" to reference the basic
    exit reason.
  - Split nested_vmx_exit_reflected() into VM-Fail, "L0 wants" and "L1
    wants", and move the tracepoint into nested_vmx_reflect_vmexit().
    [Vitaly]
  - Use a "union vmx_exit_reason exit_reason" to handle a consistency
    check VM-Exit on VM-Enter in nested_vmx_enter_non_root_mode() to avoid
    some implicit casting shenanigans. [Vitaly]
  - Collect tags. [Vitaly]

v1: https://lkml.kernel.org/r/20200312184521.24579-1-sean.j.christopherson@intel.com


Sean Christopherson (10):
  KVM: nVMX: Move reflection check into nested_vmx_reflect_vmexit()
  KVM: nVMX: Uninline nested_vmx_reflect_vmexit(), i.e. move it to
    nested.c
  KVM: nVMX: Move VM-Fail check out of nested_vmx_exit_reflected()
  KVM: nVMX: Move nested VM-Exit tracepoint into
    nested_vmx_reflect_vmexit()
  KVM: nVMX: Split VM-Exit reflection logic into L0 vs. L1 wants
  KVM: nVMX: Drop a superfluous WARN on reflecting EXTERNAL_INTERRUPT
  KVM: nVMX: Pull exit_reason from vcpu_vmx in
    nested_vmx_reflect_vmexit()
  KVM: nVMX: Cast exit_reason to u16 to check for nested
    EXTERNAL_INTERRUPT
  KVM: nVMX: Rename exit_reason to vm_exit_reason for nested VM-Exit
  KVM: VMX: Convert vcpu_vmx.exit_reason to a union

 arch/x86/kvm/vmx/nested.c | 237 +++++++++++++++++++++++++-------------
 arch/x86/kvm/vmx/nested.h |  32 +----
 arch/x86/kvm/vmx/vmx.c    |  66 ++++++-----
 arch/x86/kvm/vmx/vmx.h    |  25 +++-
 4 files changed, 219 insertions(+), 141 deletions(-)

-- 
2.26.0

Powered by blists - more mailing lists