All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	 Vitaly Kuznetsov <vkuznets@redhat.com>,
	Alexander Lougovski <alougovsk@redhat.com>,
	 Yosry Ahmed <yosry@kernel.org>
Subject: Re: [PATCH] KVM: SVM: make svm_flush_tlb_gva do a full asid flush if NPT enabled
Date: Fri, 24 Jul 2026 15:36:41 -0700	[thread overview]
Message-ID: <amPo-XdhFGIZquES@google.com> (raw)
In-Reply-To: <20260723094419.630204-1-pbonzini@redhat.com>

+Yosry, who has been digging deep on SVM TLB crud.

On Thu, Jul 23, 2026, Paolo Bonzini wrote:
> The flush is issued from kvm_hv_vcpu_flush_tlb(), which receives the
> cross-CPU requests from the Hyper-V TLB flush hypercalls via a kfifo
> and is invoked by the KVM_REQ_HV_TLB_FLUSH request.  The mechanism is
> the same for both Intel and AMD, and the handler for both vendors is
> a simple INVVPID(ADDR)/INVLPGA instruction.
> 
> Because the request is handled on the destination CPU, there is a question
> of what happens if the VM is migrated across physical CPUs.  In that case,
> the INVLPGA instruction would use a stale svm->vmcb->control.asid; but
> if anything that might do an *unnecessary* flush (on an asid that's being
> used for another VM) and then pre_svm_run() would force a full TLB rebuild.
> 
> So, for lack of better ideas, this patch forces a full ASID bump in
> svm_flush_tlb_gva().  To avoid paying the price on Intel and also to
> avoid unnecessary loops on AMD, the flush_tlb_gva op now returns whether
> it did a full flush or not; kvm_hv_vcpu_flush_tlb() takes note and exits
> its loops immediately.  While there is an obvious performance impact,
> about half of the benefit from Hyper-V tlbflush is preserved (10% vs. 20%
> on the SQL Server workload).
> 
> kvm_mmu_invalidate_addr() is the only other caller of the flush_tlb_gva op.
> The change would have a performance impact on every intercepted INVLPG and,
> for nested SVM, on every L1 INVLPGA.  For INVLPGA specifically, this covers
> the same suspected issue but for nested hypervisors, so it is correct to
> apply the workaround; for INVLPG on shadow paging, instead, the impact
> would be stronger and, due to lack of data, for now the use of INVLPGA is
> left in place in svm_flush_tlb_gva().
> 
> Analyzed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Analyzed-by: Alexander Lougovski <alougovsk@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  2 +-
>  arch/x86/kvm/hyperv.c           |  7 ++++---
>  arch/x86/kvm/mmu/mmu.c          |  2 +-
>  arch/x86/kvm/svm/svm.c          | 27 ++++++++++++++++++++-------
>  arch/x86/kvm/vmx/main.c         |  4 ++--
>  arch/x86/kvm/vmx/vmx.c          |  2 +-
>  arch/x86/kvm/vmx/x86_ops.h      |  2 +-
>  7 files changed, 30 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index b517257a6315..eca04d4b974e 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1751,7 +1751,7 @@ struct kvm_x86_ops {
>  	 * Can potentially get non-canonical addresses through INVLPGs, which
>  	 * the implementation may choose to ignore if appropriate.
>  	 */
> -	void (*flush_tlb_gva)(struct kvm_vcpu *vcpu, gva_t addr);
> +	void (*flush_tlb_gva)(struct kvm_vcpu *vcpu, gva_t addr, bool *full);

LOL, why on earth are you using an out-param?  If we do this at runtime, just
return a bool, at least that way we don't have to churn every call-site.

But I would much rather handle this by nuking .flush_tlb_gva at setup, e.g.


diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index 3776cf5382a2..4235331b71a0 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -61,7 +61,7 @@ KVM_X86_OP(flush_tlb_current)
 KVM_X86_OP_OPTIONAL(flush_remote_tlbs)
 KVM_X86_OP_OPTIONAL(flush_remote_tlbs_range)
 #endif
-KVM_X86_OP(flush_tlb_gva)
+KVM_X86_OP_OPTIONAL(flush_tlb_gva)
 KVM_X86_OP(flush_tlb_guest)
 KVM_X86_OP(vcpu_pre_run)
 KVM_X86_OP(vcpu_run)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 4438ecac9a89..7b1c6391f878 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1978,7 +1978,8 @@ int kvm_hv_vcpu_flush_tlb(struct kvm_vcpu *vcpu)
        count = kfifo_out(&tlb_flush_fifo->entries, entries, KVM_HV_TLB_FLUSH_FIFO_SIZE);
 
        for (i = 0; i < count; i++) {
-               if (entries[i] == KVM_HV_TLB_FLUSHALL_ENTRY)
+               if (entries[i] == KVM_HV_TLB_FLUSHALL_ENTRY ||
+                   !kvm_x86_ops.flush_tlb_gva)
                        goto out_flush_all;
 
                /*
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index f0144ae8d891..bf16119640dc 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6555,7 +6555,10 @@ void kvm_mmu_invalidate_addr(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
                if (is_noncanonical_invlpg_address(addr, vcpu))
                        return;
 
-               kvm_x86_call(flush_tlb_gva)(vcpu, addr);
+               if (kvm_x86_ops.flush_tlb_gva)
+                       kvm_x86_call(flush_tlb_gva)(vcpu, addr);
+               else
+                       kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu);
        }
 
        if (!mmu->sync_spte)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index c46a34aeb3df..9e04f016d939 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5688,6 +5688,15 @@ static __init int svm_hardware_setup(void)
        if (!enable_pmu)
                pr_info("PMU virtualization is disabled\n");
 
+       /*
+        * INVLPGA has had errata on Genoa and Turin, and even on older
+        * generations there were reports of Windows BSODs if INVLPGA
+        * was used for Hyper-V tlbflush.  Use it only for shadow paging
+        * where it seems to be okay.
+        */
+       if (npt_enabled)
+               svm_x86_ops.flush_tlb_gva = NULL;
+
        svm_set_cpu_caps();
 
        kvm_caps.inapplicable_quirks &= ~KVM_X86_QUIRK_CD_NW_CLEARED;

  reply	other threads:[~2026-07-24 22:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-23  9:44 [PATCH] KVM: SVM: make svm_flush_tlb_gva do a full asid flush if NPT enabled Paolo Bonzini
2026-07-24 22:36 ` Sean Christopherson [this message]
2026-07-25 13:37   ` Paolo Bonzini
2026-07-24 23:42 ` Yosry Ahmed
2026-07-24 23:50   ` Yosry Ahmed
2026-07-25 13:32   ` Paolo Bonzini
2026-07-25 21:33     ` Yosry Ahmed
2026-07-27 19:23 ` Tycho Andersen
2026-07-27 19:58   ` Yosry Ahmed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amPo-XdhFGIZquES@google.com \
    --to=seanjc@google.com \
    --cc=alougovsk@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=vkuznets@redhat.com \
    --cc=yosry@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.