Re: [PATCH 00/10] KVM: x86: Clean up VMX's TLB flushing code

[Paolo Bonzini]

On 20/02/20 21:43, Sean Christopherson wrote:
> This series is technically x86 wide, but it only superficially affects
> SVM, the motivation and primary touchpoints are all about VMX.
> 
> The goal of this series to ultimately clean up __vmx_flush_tlb(), which,
> for me, manages to be extremely confusing despite being only ten lines of
> code.
> 
> The most confusing aspect of __vmx_flush_tlb() is that it is overloaded
> for multiple uses:
> 
>  1) TLB flushes in response to a change in KVM's MMU
> 
>  2) TLB flushes during nested VM-Enter/VM-Exit when VPID is enabled
> 
>  3) Guest-scoped TLB flushes for paravirt TLB flushing
> 
> Handling (2) and (3) in the same flow as (1) is kludgy, because the rules
> for (1) are quite different than the rules for (2) and (3).  They're all
> squeezed into __vmx_flush_tlb() via the @invalidate_gpa param, which means
> "invalidate gpa mappings", not "invalidate a specific gpa"; it took me
> forever and a day to realize that.
> 
> To clean things up, handle (2) by directly calling vpid_sync_context()
> instead of bouncing through __vmx_flush_tlb(), and handle (3) via a
> dedicated kvm_x86_ops hook.  This allows for a less tricky implementation
> of vmx_flush_tlb() for (1), and (hopefully) clarifies the rules for what
> mappings must be invalidated when.
> 
> Sean Christopherson (10):
>   KVM: VMX: Use vpid_sync_context() directly when possible
>   KVM: VMX: Move vpid_sync_vcpu_addr() down a few lines
>   KVM: VMX: Handle INVVPID fallback logic in vpid_sync_vcpu_addr()
>   KVM: VMX: Fold vpid_sync_vcpu_{single,global}() into
>     vpid_sync_context()
>   KVM: nVMX: Use vpid_sync_vcpu_addr() to emulate INVVPID with address
>   KVM: x86: Move "flush guest's TLB" logic to separate kvm_x86_ops hook
>   KVM: VMX: Clean up vmx_flush_tlb_gva()
>   KVM: x86: Drop @invalidate_gpa param from kvm_x86_ops' tlb_flush()
>   KVM: VMX: Drop @invalidate_gpa from __vmx_flush_tlb()
>   KVM: VMX: Fold __vmx_flush_tlb() into vmx_flush_tlb()
> 
>  arch/x86/include/asm/kvm_host.h |  8 +++++++-
>  arch/x86/kvm/mmu/mmu.c          |  2 +-
>  arch/x86/kvm/svm.c              | 14 ++++++++++----
>  arch/x86/kvm/vmx/nested.c       | 12 ++++--------
>  arch/x86/kvm/vmx/ops.h          | 32 +++++++++-----------------------
>  arch/x86/kvm/vmx/vmx.c          | 26 +++++++++++++++++---------
>  arch/x86/kvm/vmx/vmx.h          | 19 ++++++++++---------
>  arch/x86/kvm/x86.c              |  8 ++++----
>  8 files changed, 62 insertions(+), 59 deletions(-)
> 

Reviewed-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>