mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Tianyu Lan <ltykernel@gmail.com>,
	Michael Kelley <mikelley@microsoft.com>
Subject: Re: [PATCH] KVM: SVM: Disable TDP MMU when running on Hyper-V
Date: Tue, 7 Mar 2023 09:36:49 -0800	[thread overview]
Message-ID: <ZAd2MRNLw1JAXmOf@google.com> (raw)
In-Reply-To: <20230227171751.1211786-1-jpiotrowski@linux.microsoft.com>

On Mon, Feb 27, 2023, Jeremi Piotrowski wrote:
> Disable TDP MMU when using SVM Hyper-V for the time being while we
> search for a better fix.

...

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index c91ee2927dd7..5c0e28a7a3bc 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -5787,14 +5787,15 @@ void kvm_mmu_invpcid_gva(struct kvm_vcpu *vcpu, gva_t gva, unsigned long pcid)
>  }
>  
>  void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level,
> -		       int tdp_max_root_level, int tdp_huge_page_level)
> +		       int tdp_max_root_level, int tdp_huge_page_level,
> +		       bool enable_tdp_mmu)
>  {
>  	tdp_enabled = enable_tdp;
>  	tdp_root_level = tdp_forced_root_level;
>  	max_tdp_level = tdp_max_root_level;
>  
>  #ifdef CONFIG_X86_64
> -	tdp_mmu_enabled = tdp_mmu_allowed && tdp_enabled;
> +	tdp_mmu_enabled = tdp_mmu_allowed && tdp_enabled && enable_tdp_mmu;
>  #endif

Thinking about this more, I would rather revert commit 1e0c7d40758b ("KVM: SVM:
hyper-v: Remote TLB flush for SVM") or fix the thing properly straitaway.  KVM
doesn't magically handle the flushes correctly for the shadow/legacy MMU, KVM just
happens to get lucky and not run afoul of the underlying bugs.  The revert appears
to be reasonably straightforward (see bottom).

And _if_ we want to hack-a-fix it, then I would strongly prefer a very isolated,
obviously hacky fix, e.g.

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 36e4561554ca..a9ba4ae14fda 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5779,8 +5779,13 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level,
        tdp_root_level = tdp_forced_root_level;
        max_tdp_level = tdp_max_root_level;
 
+       /*
+        * FIXME: Remove the enlightened TLB restriction when KVM properly
+        * handles TLB flushes for said enlightenment.
+        */.
 #ifdef CONFIG_X86_64
-       tdp_mmu_enabled = tdp_mmu_allowed && tdp_enabled;
+       tdp_mmu_enabled = tdp_mmu_allowed && tdp_enabled &&
+                         !(ms_hyperv.nested_features & HV_X64_NESTED_ENLIGHTENED_TLB);
 #endif
        /*
         * max_huge_page_level reflects KVM's MMU capabilities irrespective




The revert...

---
 arch/x86/kvm/svm/svm.c          |  3 ---
 arch/x86/kvm/svm/svm_onhyperv.h | 27 ---------------------------
 2 files changed, 30 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 11068e8eb969..292650dc85a0 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1320,7 +1320,6 @@ static void init_vmcb(struct kvm_vcpu *vcpu)
 	if (sev_guest(vcpu->kvm))
 		sev_init_vmcb(svm);
 
-	svm_hv_init_vmcb(vmcb);
 	init_vmcb_after_set_cpuid(vcpu);
 
 	vmcb_mark_all_dirty(vmcb);
@@ -4075,8 +4074,6 @@ static void svm_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
 		svm->vmcb->control.nested_cr3 = __sme_set(root_hpa);
 		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
 
-		hv_track_root_tdp(vcpu, root_hpa);
-
 		cr3 = vcpu->arch.cr3;
 	} else if (root_level >= PT64_ROOT_4LEVEL) {
 		cr3 = __sme_set(root_hpa) | kvm_get_active_pcid(vcpu);
diff --git a/arch/x86/kvm/svm/svm_onhyperv.h b/arch/x86/kvm/svm/svm_onhyperv.h
index 6981c1e9a809..5118fd273e73 100644
--- a/arch/x86/kvm/svm/svm_onhyperv.h
+++ b/arch/x86/kvm/svm/svm_onhyperv.h
@@ -15,31 +15,8 @@ static struct kvm_x86_ops svm_x86_ops;
 
 int svm_hv_enable_l2_tlb_flush(struct kvm_vcpu *vcpu);
 
-static inline void svm_hv_init_vmcb(struct vmcb *vmcb)
-{
-	struct hv_vmcb_enlightenments *hve = &vmcb->control.hv_enlightenments;
-
-	BUILD_BUG_ON(sizeof(vmcb->control.hv_enlightenments) !=
-		     sizeof(vmcb->control.reserved_sw));
-
-	if (npt_enabled &&
-	    ms_hyperv.nested_features & HV_X64_NESTED_ENLIGHTENED_TLB)
-		hve->hv_enlightenments_control.enlightened_npt_tlb = 1;
-
-	if (ms_hyperv.nested_features & HV_X64_NESTED_MSR_BITMAP)
-		hve->hv_enlightenments_control.msr_bitmap = 1;
-}
-
 static inline void svm_hv_hardware_setup(void)
 {
-	if (npt_enabled &&
-	    ms_hyperv.nested_features & HV_X64_NESTED_ENLIGHTENED_TLB) {
-		pr_info(KBUILD_MODNAME ": Hyper-V enlightened NPT TLB flush enabled\n");
-		svm_x86_ops.tlb_remote_flush = hv_remote_flush_tlb;
-		svm_x86_ops.tlb_remote_flush_with_range =
-				hv_remote_flush_tlb_with_range;
-	}
-
 	if (ms_hyperv.nested_features & HV_X64_NESTED_DIRECT_FLUSH) {
 		int cpu;
 
@@ -80,10 +57,6 @@ static inline void svm_hv_update_vp_id(struct vmcb *vmcb, struct kvm_vcpu *vcpu)
 }
 #else
 
-static inline void svm_hv_init_vmcb(struct vmcb *vmcb)
-{
-}
-
 static inline void svm_hv_hardware_setup(void)
 {
 }

base-commit: cb8748a781fe983e451f616ce4861a1c49ce79dd
-- 


  parent reply	other threads:[~2023-03-07 17:41 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-27 17:17 Jeremi Piotrowski
2023-03-06 17:52 ` Vitaly Kuznetsov
2023-03-06 18:31   ` Jeremi Piotrowski
2023-03-07 10:07     ` Vitaly Kuznetsov
2023-03-08 15:42       ` Jeremi Piotrowski
2023-03-07 17:36 ` Sean Christopherson [this message]
2023-03-08  0:00   ` Paolo Bonzini
2023-03-08  0:39     ` Sean Christopherson
2023-03-08 15:55       ` Jeremi Piotrowski
2023-03-08 17:22         ` Jeremi Piotrowski
2023-03-08 19:20           ` Sean Christopherson
2023-03-08 19:11         ` Sean Christopherson
2023-03-09 17:58           ` Jeremi Piotrowski
2023-03-12 17:42             ` Alexander Grest
2023-03-08 15:48     ` Jeremi Piotrowski
2023-04-05 16:43   ` Jeremi Piotrowski
2023-04-10 23:25     ` Sean Christopherson
2023-04-11 14:22       ` Jeremi Piotrowski
2023-04-11 16:02         ` Sean Christopherson
2023-04-13  9:53           ` Jeremi Piotrowski
2023-04-13 17:24             ` Sean Christopherson
2023-04-13 18:49               ` Sean Christopherson
2023-04-13 19:09               ` Sean Christopherson
2023-04-13 20:21                 ` David Matlack
2023-04-13 20:58                   ` Sean Christopherson

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=ZAd2MRNLw1JAXmOf@google.com \
    --to=seanjc@google.com \
    --cc=jpiotrowski@linux.microsoft.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ltykernel@gmail.com \
    --cc=mikelley@microsoft.com \
    --cc=pbonzini@redhat.com \
    --cc=vkuznets@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®