• Sean Christopherson's avatar
    KVM: nVMX: Tweak handling of failure code for nested VM-Enter failure · 68cda40d
    Sean Christopherson authored
    Use an enum for passing around the failure code for a failed VM-Enter
    that results in VM-Exit to provide a level of indirection from the final
    resting place of the failure code, vmcs.EXIT_QUALIFICATION.  The exit
    qualification field is an unsigned long, e.g. passing around
    'u32 exit_qual' throws up red flags as it suggests KVM may be dropping
    bits when reporting errors to L1.  This is a red herring because the
    only defined failure codes are 0, 2, 3, and 4, i.e. don't come remotely
    close to overflowing a u32.
    
    Setting vmcs.EXIT_QUALIFICATION on entry failure is further complicated
    by the MSR load list, which returns the (1-based) entry that failed, and
    the number of MSRs to load is a 32-bit VMCS field.  At first blush, it
    would appear that overflowing a u32 is possible, but the number of MSRs
    that can be loaded is hardcapped at 4096 (limited by MSR_IA32_VMX_MISC).
    
    In other words, there are two completely disparate types of data that
    eventually get stuffed into vmcs.EXIT_QUALIFICATION, neither of which is
    an 'unsigned long' in nature.  This was presumably the reasoning for
    switching to 'u32' when the related code was refactored in commit
    ca0bde28 ("kvm: nVMX: Split VMCS checks from nested_vmx_run()").
    
    Using an enum for the failure code addresses the technically-possible-
    but-will-never-happen scenario where Intel defines a failure code that
    doesn't fit in a 32-bit integer.  The enum variables and values will
    either be automatically sized (gcc 5.4 behavior) or be subjected to some
    combination of truncation.  The former case will simply work, while the
    latter will trigger a compile-time warning unless the compiler is being
    particularly unhelpful.
    
    Separating the failure code from the failed MSR entry allows for
    disassociating both from vmcs.EXIT_QUALIFICATION, which avoids the
    conundrum where KVM has to choose between 'u32 exit_qual' and tracking
    values as 'unsigned long' that have no business being tracked as such.
    To cement the split, set vmcs12->exit_qualification directly from the
    entry error code or failed MSR index instead of bouncing through a local
    variable.
    
    Opportunistically rename the variables in load_vmcs12_host_state() and
    vmx_set_nested_state() to call out that they're ignored, set exit_reason
    on demand on nested VM-Enter failure, and add a comment in
    nested_vmx_load_msr() to call out that returning 'i + 1' can't wrap.
    
    No functional change intended.
    Reported-by: default avatarVitaly Kuznetsov <vkuznets@redhat.com>
    Cc: Jim Mattson <jmattson@google.com>
    Signed-off-by: default avatarSean Christopherson <sean.j.christopherson@intel.com>
    Message-Id: <20200511220529.11402-1-sean.j.christopherson@intel.com>
    Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
    68cda40d
nested.c 198 KB