Re: [PATCH] kvm: nVMX: Remove unnecessary sync_roots from handle_invept

[Jim Mattson <jmattson@google.com>]

On Thu, Jun 13, 2019 at 9:16 AM Jim Mattson <jmattson@google.com> wrote:
>
> When L0 is executing handle_invept(), the TDP MMU is active. Emulating
> an L1 INVEPT does require synchronizing the appropriate shadow EPT
> root(s), but a call to kvm_mmu_sync_roots in this context won't do
> that. Similarly, the hardware TLB and paging-structure-cache entries
> associated with the appropriate shadow EPT root(s) must be flushed,
> but requesting a TLB_FLUSH from this context won't do that either.
>
> How did this ever work? KVM always does a sync_roots and TLB flush (in
> the correct context) when transitioning from L1 to L2. That isn't the
> best choice for nested VM performance, but it effectively papers over
> the mistakes here.
>
> Remove the unnecessary operations and leave a comment to try to do
> better in the future.
>
> Reported-by: Junaid Shahid <junaids@google.com>
> Fixes: bfd0a56b90005f ("nEPT: Nested INVEPT")
> Cc: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> Cc: Nadav Har'El <nyh@il.ibm.com>
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> Cc: Xinhao Xu <xinhao.xu@intel.com>
> Cc: Yang Zhang <yang.z.zhang@Intel.com>
> Cc: Gleb Natapov <gleb@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by Peter Shier <pshier@google.com>
> Reviewed-by: Junaid Shahid <junaids@google.com>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/vmx/nested.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 1032f068f0b9..35621e73e726 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -4670,13 +4670,11 @@ static int handle_invept(struct kvm_vcpu *vcpu)
>
>         switch (type) {
>         case VMX_EPT_EXTENT_GLOBAL:
> +       case VMX_EPT_EXTENT_CONTEXT:
>         /*
> -        * TODO: track mappings and invalidate
> -        * single context requests appropriately
> +        * TODO: Sync the necessary shadow EPT roots here, rather than
> +        * at the next emulated VM-entry.
>          */
> -       case VMX_EPT_EXTENT_CONTEXT:
> -               kvm_mmu_sync_roots(vcpu);
> -               kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
>                 break;
>         default:
>                 BUG_ON(1);
> --
> 2.22.0.rc2.383.gf4fbbf30c2-goog
>

Ping.