lkml.org 
[lkml]   [2020]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 00/10] KVM: VMX: Unionize vcpu_vmx.exit_reason
From
Date
On 15/04/20 19:55, Sean Christopherson wrote:
> 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

For now I committed only patches 1-9, just to limit the conflicts with
the other series. I would like to understand how you think the
conflicts should be fixed with the union.

Paolo

\
 
 \ /
  Last update: 2020-04-16 17:28    [W:0.092 / U:0.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site