All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush
@ 2021-03-05 18:31 Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 01/11] KVM: x86: Get active PCID only when writing a CR3 value Sean Christopherson
                   ` (12 more replies)
  0 siblings, 13 replies; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Sean Christopherson

Clean up KVM's PV TLB flushing when running with EPT on Hyper-V, i.e. as
a nested VMM.  No real goal in mind other than the sole patch in v1, which
is a minor change to avoid a future mixup when TDX also wants to define
.remote_flush_tlb.  Everything else is opportunistic clean up.

NOTE: Based on my NPT+SME bug fix series[*] due to multiple conflicts with
      non-trivial resolutions.

[*] https://lkml.kernel.org/r/20210305011101.3597423-1-seanjc@google.com


Patch 1 legitimately tested on VMX and SVM (including i386).  Patches 2+
smoke tested by hacking usage of the relevant flows without actually
routing to the Hyper-V hypercalls (partial hack-a-patch below).

-static inline int hv_remote_flush_root_ept(hpa_t root_ept,
+static inline int hv_remote_flush_root_ept(struct kvm *kvm, hpa_t root_ept,
                                           struct kvm_tlb_range *range)
 {
-       if (range)
-               return hyperv_flush_guest_mapping_range(root_ept,
-                               kvm_fill_hv_flush_list_func, (void *)range);
-       else
-               return hyperv_flush_guest_mapping(root_ept);
+       if (range) {
+               kvm_make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH);
+               return 0;
+       }
+
+       return -ENOMEM;
 }
 
 static int hv_remote_flush_tlb_with_range(struct kvm *kvm,
@@ -7753,8 +7754,7 @@ static __init int hardware_setup(void)
                vmx_x86_ops.update_cr8_intercept = NULL;
 
 #if IS_ENABLED(CONFIG_HYPERV)
-       if (ms_hyperv.nested_features & HV_X64_NESTED_GUEST_MAPPING_FLUSH
-           && enable_ept) {
+       if (enable_ept) {
                vmx_x86_ops.tlb_remote_flush = hv_remote_flush_tlb;
                vmx_x86_ops.tlb_remote_flush_with_range =
                                hv_remote_flush_tlb_with_range;

v4: 
  - Rebased to kvm/queue, commit fe5f0041c026 ("KVM/SVM: Move vmenter.S
    exception fixups out of line"), plus the aforementioned series.
  - Don't grab PCID for nested_cr3 (NPT). [Paolo]
  - Collect reviews. [Vitaly]

v3:
  - https://lkml.kernel.org/r/20201027212346.23409-1-sean.j.christopherson@intel.com
  - Add a patch to pass the root_hpa instead of pgd to vmx_load_mmu_pgd()
    and retrieve the active PCID only when necessary.  [Vitaly]
  - Selectively collects reviews (skipped a few due to changes). [Vitaly]
  - Explicitly invalidate hv_tlb_eptp instead of leaving it valid when
    the mismatch tracker "knows" it's invalid. [Vitaly]
  - Change the last patch to use "hv_root_ept" instead of "hv_tlb_pgd"
    to better reflect what is actually being tracked.

v2:
  - Rewrite everything.
  - https://lkml.kernel.org/r/20201020215613.8972-1-sean.j.christopherson@intel.com

v1: ???

Sean Christopherson (11):
  KVM: x86: Get active PCID only when writing a CR3 value
  KVM: VMX: Track common EPTP for Hyper-V's paravirt TLB flush
  KVM: VMX: Stash kvm_vmx in a local variable for Hyper-V paravirt TLB
    flush
  KVM: VMX: Fold Hyper-V EPTP checking into it's only caller
  KVM: VMX: Do Hyper-V TLB flush iff vCPU's EPTP hasn't been flushed
  KVM: VMX: Invalidate hv_tlb_eptp to denote an EPTP mismatch
  KVM: VMX: Don't invalidate hv_tlb_eptp if the new EPTP matches
  KVM: VMX: Explicitly check for hv_remote_flush_tlb when loading pgd
  KVM: VMX: Define Hyper-V paravirt TLB flush fields iff Hyper-V is
    enabled
  KVM: VMX: Skip additional Hyper-V TLB EPTP flushes if one fails
  KVM: VMX: Track root HPA instead of EPTP for paravirt Hyper-V TLB
    flush

 arch/x86/include/asm/kvm_host.h |   4 +-
 arch/x86/kvm/mmu.h              |   4 +-
 arch/x86/kvm/svm/svm.c          |  10 ++-
 arch/x86/kvm/vmx/vmx.c          | 134 ++++++++++++++++++--------------
 arch/x86/kvm/vmx/vmx.h          |  19 ++---
 5 files changed, 92 insertions(+), 79 deletions(-)

-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4 01/11] KVM: x86: Get active PCID only when writing a CR3 value
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
@ 2021-03-05 18:31 ` Sean Christopherson
  2021-03-08 10:05   ` Vitaly Kuznetsov
  2021-03-05 18:31 ` [PATCH v4 02/11] KVM: VMX: Track common EPTP for Hyper-V's paravirt TLB flush Sean Christopherson
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Sean Christopherson

From: Sean Christopherson <sean.j.christopherson@intel.com>

Retrieve the active PCID only when writing a guest CR3 value, i.e. don't
get the PCID when using EPT or NPT.  The PCID is especially problematic
for EPT as the bits have different meaning, and so the PCID and must be
manually stripped, which is annoying and unnecessary.  And on VMX,
getting the active PCID also involves reading the guest's CR3 and
CR4.PCIDE, i.e. may add pointless VMREADs.

Opportunistically rename the pgd/pgd_level params to root_hpa and
root_level to better reflect their new roles.  Keep the function names,
as "load the guest PGD" is still accurate/correct.

Last, and probably least, pass root_hpa as a hpa_t/u64 instead of an
unsigned long.  The EPTP holds a 64-bit value, even in 32-bit mode, so
in theory EPT could support HIGHMEM for 32-bit KVM.  Never mind that
doing so would require changing the MMU page allocators and reworking
the MMU to use kmap().

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/kvm_host.h |  4 ++--
 arch/x86/kvm/mmu.h              |  4 ++--
 arch/x86/kvm/svm/svm.c          | 10 ++++++----
 arch/x86/kvm/vmx/vmx.c          | 13 ++++++-------
 arch/x86/kvm/vmx/vmx.h          |  3 +--
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 2da6c9f5935a..51725e994451 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1245,8 +1245,8 @@ struct kvm_x86_ops {
 	int (*set_identity_map_addr)(struct kvm *kvm, u64 ident_addr);
 	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
 
-	void (*load_mmu_pgd)(struct kvm_vcpu *vcpu, unsigned long pgd,
-			     int pgd_level);
+	void (*load_mmu_pgd)(struct kvm_vcpu *vcpu, hpa_t root_hpa,
+			     int root_level);
 
 	bool (*has_wbinvd_exit)(void);
 
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 67e8c7c7a6ce..88d0ed5225a4 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -107,8 +107,8 @@ static inline void kvm_mmu_load_pgd(struct kvm_vcpu *vcpu)
 	if (!VALID_PAGE(root_hpa))
 		return;
 
-	static_call(kvm_x86_load_mmu_pgd)(vcpu, root_hpa | kvm_get_active_pcid(vcpu),
-				 vcpu->arch.mmu->shadow_root_level);
+	static_call(kvm_x86_load_mmu_pgd)(vcpu, root_hpa,
+					  vcpu->arch.mmu->shadow_root_level);
 }
 
 int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u32 error_code,
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index dfc8fe231e8b..c7a685bf6862 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3902,14 +3902,14 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
 	return svm_exit_handlers_fastpath(vcpu);
 }
 
-static void svm_load_mmu_pgd(struct kvm_vcpu *vcpu, unsigned long root,
+static void svm_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
 			     int root_level)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 	unsigned long cr3;
 
 	if (npt_enabled) {
-		svm->vmcb->control.nested_cr3 = __sme_set(root);
+		svm->vmcb->control.nested_cr3 = __sme_set(root_hpa);
 		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
 
 		/* Loading L2's CR3 is handled by enter_svm_guest_mode.  */
@@ -3917,9 +3917,11 @@ static void svm_load_mmu_pgd(struct kvm_vcpu *vcpu, unsigned long root,
 			return;
 		cr3 = vcpu->arch.cr3;
 	} else if (vcpu->arch.mmu->shadow_root_level >= PT64_ROOT_4LEVEL) {
-		cr3 = __sme_set(root);
+		cr3 = __sme_set(root_hpa) | kvm_get_active_pcid(vcpu);
 	} else {
-		cr3 = root;
+		/* PCID in the guest should be impossible with a 32-bit MMU. */
+		WARN_ON_ONCE(kvm_get_active_pcid(vcpu));
+		cr3 = root_hpa;
 	}
 
 	svm->vmcb->save.cr3 = cr3;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 6d7e760fdfa0..dde2ebc7cf3a 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -3088,8 +3088,7 @@ static int vmx_get_max_tdp_level(void)
 	return 4;
 }
 
-u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa,
-		   int root_level)
+u64 construct_eptp(struct kvm_vcpu *vcpu, hpa_t root_hpa, int root_level)
 {
 	u64 eptp = VMX_EPTP_MT_WB;
 
@@ -3098,13 +3097,13 @@ u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa,
 	if (enable_ept_ad_bits &&
 	    (!is_guest_mode(vcpu) || nested_ept_ad_enabled(vcpu)))
 		eptp |= VMX_EPTP_AD_ENABLE_BIT;
-	eptp |= (root_hpa & PAGE_MASK);
+	eptp |= root_hpa;
 
 	return eptp;
 }
 
-static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, unsigned long pgd,
-			     int pgd_level)
+static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
+			     int root_level)
 {
 	struct kvm *kvm = vcpu->kvm;
 	bool update_guest_cr3 = true;
@@ -3112,7 +3111,7 @@ static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, unsigned long pgd,
 	u64 eptp;
 
 	if (enable_ept) {
-		eptp = construct_eptp(vcpu, pgd, pgd_level);
+		eptp = construct_eptp(vcpu, root_hpa, root_level);
 		vmcs_write64(EPT_POINTER, eptp);
 
 		if (kvm_x86_ops.tlb_remote_flush) {
@@ -3131,7 +3130,7 @@ static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, unsigned long pgd,
 			update_guest_cr3 = false;
 		vmx_ept_load_pdptrs(vcpu);
 	} else {
-		guest_cr3 = pgd;
+		guest_cr3 = root_hpa | kvm_get_active_pcid(vcpu);
 	}
 
 	if (update_guest_cr3)
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 89da5e1251f1..4795955490cb 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -376,8 +376,7 @@ void set_cr4_guest_host_mask(struct vcpu_vmx *vmx);
 void ept_save_pdptrs(struct kvm_vcpu *vcpu);
 void vmx_get_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int seg);
 void vmx_set_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int seg);
-u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa,
-		   int root_level);
+u64 construct_eptp(struct kvm_vcpu *vcpu, hpa_t root_hpa, int root_level);
 
 void vmx_update_exception_bitmap(struct kvm_vcpu *vcpu);
 void vmx_update_msr_bitmap(struct kvm_vcpu *vcpu);
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 02/11] KVM: VMX: Track common EPTP for Hyper-V's paravirt TLB flush
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 01/11] KVM: x86: Get active PCID only when writing a CR3 value Sean Christopherson
@ 2021-03-05 18:31 ` Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 03/11] KVM: VMX: Stash kvm_vmx in a local variable for Hyper-V " Sean Christopherson
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Sean Christopherson

From: Sean Christopherson <sean.j.christopherson@intel.com>

Explicitly track the EPTP that is common to all vCPUs instead of
grabbing vCPU0's EPTP when invoking Hyper-V's paravirt TLB flush.
Tracking the EPTP will allow optimizing the checks when loading a new
EPTP and will also allow dropping ept_pointer_match, e.g. by marking
the common EPTP as invalid.

This also technically fixes a bug where KVM could theoretically flush an
invalid GPA if all vCPUs have an invalid root.  In practice, it's likely
impossible to trigger a remote TLB flush in such a scenario.  In any
case, the superfluous flush is completely benign.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 20 +++++++++-----------
 arch/x86/kvm/vmx/vmx.h |  1 +
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index dde2ebc7cf3a..4082c7a26612 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -483,12 +483,14 @@ static void check_ept_pointer_match(struct kvm *kvm)
 		if (!VALID_PAGE(tmp_eptp)) {
 			tmp_eptp = to_vmx(vcpu)->ept_pointer;
 		} else if (tmp_eptp != to_vmx(vcpu)->ept_pointer) {
+			to_kvm_vmx(kvm)->hv_tlb_eptp = INVALID_PAGE;
 			to_kvm_vmx(kvm)->ept_pointers_match
 				= EPT_POINTERS_MISMATCH;
 			return;
 		}
 	}
 
+	to_kvm_vmx(kvm)->hv_tlb_eptp = tmp_eptp;
 	to_kvm_vmx(kvm)->ept_pointers_match = EPT_POINTERS_MATCH;
 }
 
@@ -501,21 +503,18 @@ static int kvm_fill_hv_flush_list_func(struct hv_guest_mapping_flush_list *flush
 			range->pages);
 }
 
-static inline int __hv_remote_flush_tlb_with_range(struct kvm *kvm,
-		struct kvm_vcpu *vcpu, struct kvm_tlb_range *range)
+static inline int hv_remote_flush_eptp(u64 eptp, struct kvm_tlb_range *range)
 {
-	u64 ept_pointer = to_vmx(vcpu)->ept_pointer;
-
 	/*
 	 * FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE hypercall needs address
 	 * of the base of EPT PML4 table, strip off EPT configuration
 	 * information.
 	 */
 	if (range)
-		return hyperv_flush_guest_mapping_range(ept_pointer & PAGE_MASK,
+		return hyperv_flush_guest_mapping_range(eptp & PAGE_MASK,
 				kvm_fill_hv_flush_list_func, (void *)range);
 	else
-		return hyperv_flush_guest_mapping(ept_pointer & PAGE_MASK);
+		return hyperv_flush_guest_mapping(eptp & PAGE_MASK);
 }
 
 static int hv_remote_flush_tlb_with_range(struct kvm *kvm,
@@ -533,12 +532,11 @@ static int hv_remote_flush_tlb_with_range(struct kvm *kvm,
 		kvm_for_each_vcpu(i, vcpu, kvm) {
 			/* If ept_pointer is invalid pointer, bypass flush request. */
 			if (VALID_PAGE(to_vmx(vcpu)->ept_pointer))
-				ret |= __hv_remote_flush_tlb_with_range(
-					kvm, vcpu, range);
+				ret |= hv_remote_flush_eptp(to_vmx(vcpu)->ept_pointer,
+							    range);
 		}
-	} else {
-		ret = __hv_remote_flush_tlb_with_range(kvm,
-				kvm_get_vcpu(kvm, 0), range);
+	} else if (VALID_PAGE(to_kvm_vmx(kvm)->hv_tlb_eptp)) {
+		ret = hv_remote_flush_eptp(to_kvm_vmx(kvm)->hv_tlb_eptp, range);
 	}
 
 	spin_unlock(&to_kvm_vmx(kvm)->ept_pointer_lock);
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 4795955490cb..f846cf3a5d25 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -351,6 +351,7 @@ struct kvm_vmx {
 	bool ept_identity_pagetable_done;
 	gpa_t ept_identity_map_addr;
 
+	hpa_t hv_tlb_eptp;
 	enum ept_pointers_status ept_pointers_match;
 	spinlock_t ept_pointer_lock;
 };
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 03/11] KVM: VMX: Stash kvm_vmx in a local variable for Hyper-V paravirt TLB flush
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 01/11] KVM: x86: Get active PCID only when writing a CR3 value Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 02/11] KVM: VMX: Track common EPTP for Hyper-V's paravirt TLB flush Sean Christopherson
@ 2021-03-05 18:31 ` Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 04/11] KVM: VMX: Fold Hyper-V EPTP checking into it's only caller Sean Christopherson
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Sean Christopherson

From: Sean Christopherson <sean.j.christopherson@intel.com>

Capture kvm_vmx in a local variable instead of polluting
hv_remote_flush_tlb_with_range() with to_kvm_vmx(kvm).

No functional change intended.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 4082c7a26612..cf79fc6c01bb 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -520,26 +520,27 @@ static inline int hv_remote_flush_eptp(u64 eptp, struct kvm_tlb_range *range)
 static int hv_remote_flush_tlb_with_range(struct kvm *kvm,
 		struct kvm_tlb_range *range)
 {
+	struct kvm_vmx *kvm_vmx = to_kvm_vmx(kvm);
 	struct kvm_vcpu *vcpu;
 	int ret = 0, i;
 
-	spin_lock(&to_kvm_vmx(kvm)->ept_pointer_lock);
+	spin_lock(&kvm_vmx->ept_pointer_lock);
 
-	if (to_kvm_vmx(kvm)->ept_pointers_match == EPT_POINTERS_CHECK)
+	if (kvm_vmx->ept_pointers_match == EPT_POINTERS_CHECK)
 		check_ept_pointer_match(kvm);
 
-	if (to_kvm_vmx(kvm)->ept_pointers_match != EPT_POINTERS_MATCH) {
+	if (kvm_vmx->ept_pointers_match != EPT_POINTERS_MATCH) {
 		kvm_for_each_vcpu(i, vcpu, kvm) {
 			/* If ept_pointer is invalid pointer, bypass flush request. */
 			if (VALID_PAGE(to_vmx(vcpu)->ept_pointer))
 				ret |= hv_remote_flush_eptp(to_vmx(vcpu)->ept_pointer,
 							    range);
 		}
-	} else if (VALID_PAGE(to_kvm_vmx(kvm)->hv_tlb_eptp)) {
-		ret = hv_remote_flush_eptp(to_kvm_vmx(kvm)->hv_tlb_eptp, range);
+	} else if (VALID_PAGE(kvm_vmx->hv_tlb_eptp)) {
+		ret = hv_remote_flush_eptp(kvm_vmx->hv_tlb_eptp, range);
 	}
 
-	spin_unlock(&to_kvm_vmx(kvm)->ept_pointer_lock);
+	spin_unlock(&kvm_vmx->ept_pointer_lock);
 	return ret;
 }
 static int hv_remote_flush_tlb(struct kvm *kvm)
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 04/11] KVM: VMX: Fold Hyper-V EPTP checking into it's only caller
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
                   ` (2 preceding siblings ...)
  2021-03-05 18:31 ` [PATCH v4 03/11] KVM: VMX: Stash kvm_vmx in a local variable for Hyper-V " Sean Christopherson
@ 2021-03-05 18:31 ` Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 05/11] KVM: VMX: Do Hyper-V TLB flush iff vCPU's EPTP hasn't been flushed Sean Christopherson
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Sean Christopherson

From: Sean Christopherson <sean.j.christopherson@intel.com>

Fold check_ept_pointer_match() into hv_remote_flush_tlb_with_range() in
preparation for combining the kvm_for_each_vcpu loops of the ==CHECK and
!=MATCH statements.

No functional change intended.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 44 +++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index cf79fc6c01bb..a1c7ba0918e7 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -472,28 +472,6 @@ static const u32 vmx_uret_msrs_list[] = {
 static bool __read_mostly enlightened_vmcs = true;
 module_param(enlightened_vmcs, bool, 0444);
 
-/* check_ept_pointer() should be under protection of ept_pointer_lock. */
-static void check_ept_pointer_match(struct kvm *kvm)
-{
-	struct kvm_vcpu *vcpu;
-	u64 tmp_eptp = INVALID_PAGE;
-	int i;
-
-	kvm_for_each_vcpu(i, vcpu, kvm) {
-		if (!VALID_PAGE(tmp_eptp)) {
-			tmp_eptp = to_vmx(vcpu)->ept_pointer;
-		} else if (tmp_eptp != to_vmx(vcpu)->ept_pointer) {
-			to_kvm_vmx(kvm)->hv_tlb_eptp = INVALID_PAGE;
-			to_kvm_vmx(kvm)->ept_pointers_match
-				= EPT_POINTERS_MISMATCH;
-			return;
-		}
-	}
-
-	to_kvm_vmx(kvm)->hv_tlb_eptp = tmp_eptp;
-	to_kvm_vmx(kvm)->ept_pointers_match = EPT_POINTERS_MATCH;
-}
-
 static int kvm_fill_hv_flush_list_func(struct hv_guest_mapping_flush_list *flush,
 		void *data)
 {
@@ -523,11 +501,29 @@ static int hv_remote_flush_tlb_with_range(struct kvm *kvm,
 	struct kvm_vmx *kvm_vmx = to_kvm_vmx(kvm);
 	struct kvm_vcpu *vcpu;
 	int ret = 0, i;
+	u64 tmp_eptp;
 
 	spin_lock(&kvm_vmx->ept_pointer_lock);
 
-	if (kvm_vmx->ept_pointers_match == EPT_POINTERS_CHECK)
-		check_ept_pointer_match(kvm);
+	if (kvm_vmx->ept_pointers_match == EPT_POINTERS_CHECK) {
+		kvm_vmx->ept_pointers_match = EPT_POINTERS_MATCH;
+		kvm_vmx->hv_tlb_eptp = INVALID_PAGE;
+
+		kvm_for_each_vcpu(i, vcpu, kvm) {
+			tmp_eptp = to_vmx(vcpu)->ept_pointer;
+			if (!VALID_PAGE(tmp_eptp))
+				continue;
+
+			if (!VALID_PAGE(kvm_vmx->hv_tlb_eptp)) {
+				kvm_vmx->hv_tlb_eptp = tmp_eptp;
+			} else if (kvm_vmx->hv_tlb_eptp != tmp_eptp) {
+				kvm_vmx->hv_tlb_eptp = INVALID_PAGE;
+				kvm_vmx->ept_pointers_match
+					= EPT_POINTERS_MISMATCH;
+				break;
+			}
+		}
+	}
 
 	if (kvm_vmx->ept_pointers_match != EPT_POINTERS_MATCH) {
 		kvm_for_each_vcpu(i, vcpu, kvm) {
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 05/11] KVM: VMX: Do Hyper-V TLB flush iff vCPU's EPTP hasn't been flushed
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
                   ` (3 preceding siblings ...)
  2021-03-05 18:31 ` [PATCH v4 04/11] KVM: VMX: Fold Hyper-V EPTP checking into it's only caller Sean Christopherson
@ 2021-03-05 18:31 ` Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 06/11] KVM: VMX: Invalidate hv_tlb_eptp to denote an EPTP mismatch Sean Christopherson
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Sean Christopherson

From: Sean Christopherson <sean.j.christopherson@intel.com>

Combine the for-loops for Hyper-V TLB EPTP checking and flushing, and in
doing so skip flushes for vCPUs whose EPTP matches the target EPTP.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index a1c7ba0918e7..a1a5b411c903 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -505,33 +505,26 @@ static int hv_remote_flush_tlb_with_range(struct kvm *kvm,
 
 	spin_lock(&kvm_vmx->ept_pointer_lock);
 
-	if (kvm_vmx->ept_pointers_match == EPT_POINTERS_CHECK) {
+	if (kvm_vmx->ept_pointers_match != EPT_POINTERS_MATCH) {
 		kvm_vmx->ept_pointers_match = EPT_POINTERS_MATCH;
 		kvm_vmx->hv_tlb_eptp = INVALID_PAGE;
 
 		kvm_for_each_vcpu(i, vcpu, kvm) {
 			tmp_eptp = to_vmx(vcpu)->ept_pointer;
-			if (!VALID_PAGE(tmp_eptp))
+			if (!VALID_PAGE(tmp_eptp) ||
+			    tmp_eptp == kvm_vmx->hv_tlb_eptp)
 				continue;
 
-			if (!VALID_PAGE(kvm_vmx->hv_tlb_eptp)) {
+			if (!VALID_PAGE(kvm_vmx->hv_tlb_eptp))
 				kvm_vmx->hv_tlb_eptp = tmp_eptp;
-			} else if (kvm_vmx->hv_tlb_eptp != tmp_eptp) {
-				kvm_vmx->hv_tlb_eptp = INVALID_PAGE;
+			else
 				kvm_vmx->ept_pointers_match
 					= EPT_POINTERS_MISMATCH;
-				break;
-			}
-		}
-	}
 
-	if (kvm_vmx->ept_pointers_match != EPT_POINTERS_MATCH) {
-		kvm_for_each_vcpu(i, vcpu, kvm) {
-			/* If ept_pointer is invalid pointer, bypass flush request. */
-			if (VALID_PAGE(to_vmx(vcpu)->ept_pointer))
-				ret |= hv_remote_flush_eptp(to_vmx(vcpu)->ept_pointer,
-							    range);
+			ret |= hv_remote_flush_eptp(tmp_eptp, range);
 		}
+		if (kvm_vmx->ept_pointers_match == EPT_POINTERS_MISMATCH)
+			kvm_vmx->hv_tlb_eptp = INVALID_PAGE;
 	} else if (VALID_PAGE(kvm_vmx->hv_tlb_eptp)) {
 		ret = hv_remote_flush_eptp(kvm_vmx->hv_tlb_eptp, range);
 	}
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 06/11] KVM: VMX: Invalidate hv_tlb_eptp to denote an EPTP mismatch
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
                   ` (4 preceding siblings ...)
  2021-03-05 18:31 ` [PATCH v4 05/11] KVM: VMX: Do Hyper-V TLB flush iff vCPU's EPTP hasn't been flushed Sean Christopherson
@ 2021-03-05 18:31 ` Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 07/11] KVM: VMX: Don't invalidate hv_tlb_eptp if the new EPTP matches Sean Christopherson
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Sean Christopherson

From: Sean Christopherson <sean.j.christopherson@intel.com>

Drop the dedicated 'ept_pointers_match' field in favor of stuffing
'hv_tlb_eptp' with INVALID_PAGE to mark it as invalid, i.e. to denote
that there is at least one EPTP mismatch.  Use a local variable to
track whether or not a mismatch is detected so that hv_tlb_eptp can be
used to skip redundant flushes.

No functional change intended.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 35 +++++++++++++++++++++++------------
 arch/x86/kvm/vmx/vmx.h |  7 -------
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index a1a5b411c903..2582df5ae64d 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -500,32 +500,44 @@ static int hv_remote_flush_tlb_with_range(struct kvm *kvm,
 {
 	struct kvm_vmx *kvm_vmx = to_kvm_vmx(kvm);
 	struct kvm_vcpu *vcpu;
-	int ret = 0, i;
+	int ret = 0, i, nr_unique_valid_eptps;
 	u64 tmp_eptp;
 
 	spin_lock(&kvm_vmx->ept_pointer_lock);
 
-	if (kvm_vmx->ept_pointers_match != EPT_POINTERS_MATCH) {
-		kvm_vmx->ept_pointers_match = EPT_POINTERS_MATCH;
-		kvm_vmx->hv_tlb_eptp = INVALID_PAGE;
+	if (!VALID_PAGE(kvm_vmx->hv_tlb_eptp)) {
+		nr_unique_valid_eptps = 0;
 
+		/*
+		 * Flush all valid EPTPs, and see if all vCPUs have converged
+		 * on a common EPTP, in which case future flushes can skip the
+		 * loop and flush the common EPTP.
+		 */
 		kvm_for_each_vcpu(i, vcpu, kvm) {
 			tmp_eptp = to_vmx(vcpu)->ept_pointer;
 			if (!VALID_PAGE(tmp_eptp) ||
 			    tmp_eptp == kvm_vmx->hv_tlb_eptp)
 				continue;
 
-			if (!VALID_PAGE(kvm_vmx->hv_tlb_eptp))
+			/*
+			 * Set the tracked EPTP to the first valid EPTP.  Keep
+			 * this EPTP for the entirety of the loop even if more
+			 * EPTPs are encountered as a low effort optimization
+			 * to avoid flushing the same (first) EPTP again.
+			 */
+			if (++nr_unique_valid_eptps == 1)
 				kvm_vmx->hv_tlb_eptp = tmp_eptp;
-			else
-				kvm_vmx->ept_pointers_match
-					= EPT_POINTERS_MISMATCH;
 
 			ret |= hv_remote_flush_eptp(tmp_eptp, range);
 		}
-		if (kvm_vmx->ept_pointers_match == EPT_POINTERS_MISMATCH)
+
+		/*
+		 * The optimized flush of a single EPTP can't be used if there
+		 * are multiple valid EPTPs (obviously).
+		 */
+		if (nr_unique_valid_eptps > 1)
 			kvm_vmx->hv_tlb_eptp = INVALID_PAGE;
-	} else if (VALID_PAGE(kvm_vmx->hv_tlb_eptp)) {
+	} else {
 		ret = hv_remote_flush_eptp(kvm_vmx->hv_tlb_eptp, range);
 	}
 
@@ -3105,8 +3117,7 @@ static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
 		if (kvm_x86_ops.tlb_remote_flush) {
 			spin_lock(&to_kvm_vmx(kvm)->ept_pointer_lock);
 			to_vmx(vcpu)->ept_pointer = eptp;
-			to_kvm_vmx(kvm)->ept_pointers_match
-				= EPT_POINTERS_CHECK;
+			to_kvm_vmx(kvm)->hv_tlb_eptp = INVALID_PAGE;
 			spin_unlock(&to_kvm_vmx(kvm)->ept_pointer_lock);
 		}
 
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index f846cf3a5d25..fb7b2000bd0e 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -338,12 +338,6 @@ struct vcpu_vmx {
 	} shadow_msr_intercept;
 };
 
-enum ept_pointers_status {
-	EPT_POINTERS_CHECK = 0,
-	EPT_POINTERS_MATCH = 1,
-	EPT_POINTERS_MISMATCH = 2
-};
-
 struct kvm_vmx {
 	struct kvm kvm;
 
@@ -352,7 +346,6 @@ struct kvm_vmx {
 	gpa_t ept_identity_map_addr;
 
 	hpa_t hv_tlb_eptp;
-	enum ept_pointers_status ept_pointers_match;
 	spinlock_t ept_pointer_lock;
 };
 
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 07/11] KVM: VMX: Don't invalidate hv_tlb_eptp if the new EPTP matches
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
                   ` (5 preceding siblings ...)
  2021-03-05 18:31 ` [PATCH v4 06/11] KVM: VMX: Invalidate hv_tlb_eptp to denote an EPTP mismatch Sean Christopherson
@ 2021-03-05 18:31 ` Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 08/11] KVM: VMX: Explicitly check for hv_remote_flush_tlb when loading pgd Sean Christopherson
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Sean Christopherson

From: Sean Christopherson <sean.j.christopherson@intel.com>

Don't invalidate the common EPTP, and thus trigger rechecking of EPTPs
across all vCPUs, if the new EPTP matches the old/common EPTP.  In all
likelihood this is a meaningless optimization, but there are (uncommon)
scenarios where KVM can reload the same EPTP.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 2582df5ae64d..cd0d787b0a85 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -3117,7 +3117,8 @@ static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
 		if (kvm_x86_ops.tlb_remote_flush) {
 			spin_lock(&to_kvm_vmx(kvm)->ept_pointer_lock);
 			to_vmx(vcpu)->ept_pointer = eptp;
-			to_kvm_vmx(kvm)->hv_tlb_eptp = INVALID_PAGE;
+			if (eptp != to_kvm_vmx(kvm)->hv_tlb_eptp)
+				to_kvm_vmx(kvm)->hv_tlb_eptp = INVALID_PAGE;
 			spin_unlock(&to_kvm_vmx(kvm)->ept_pointer_lock);
 		}
 
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 08/11] KVM: VMX: Explicitly check for hv_remote_flush_tlb when loading pgd
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
                   ` (6 preceding siblings ...)
  2021-03-05 18:31 ` [PATCH v4 07/11] KVM: VMX: Don't invalidate hv_tlb_eptp if the new EPTP matches Sean Christopherson
@ 2021-03-05 18:31 ` Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 09/11] KVM: VMX: Define Hyper-V paravirt TLB flush fields iff Hyper-V is enabled Sean Christopherson
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Sean Christopherson

From: Sean Christopherson <sean.j.christopherson@intel.com>

Explicitly check that kvm_x86_ops.tlb_remote_flush() points at Hyper-V's
implementation for PV flushing instead of assuming that a non-NULL
implementation means running on Hyper-V.  Wrap the related logic in
ifdeffery as hv_remote_flush_tlb() is defined iff CONFIG_HYPERV!=n.

Short term, the explicit check makes it more obvious why a non-NULL
tlb_remote_flush() triggers EPTP shenanigans.  Long term, this will
allow TDX to define its own implementation of tlb_remote_flush() without
running afoul of Hyper-V.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index cd0d787b0a85..aca6849c409a 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -576,6 +576,21 @@ static int hv_enable_direct_tlbflush(struct kvm_vcpu *vcpu)
 
 #endif /* IS_ENABLED(CONFIG_HYPERV) */
 
+static void hv_load_mmu_eptp(struct kvm_vcpu *vcpu, u64 eptp)
+{
+#if IS_ENABLED(CONFIG_HYPERV)
+	struct kvm_vmx *kvm_vmx = to_kvm_vmx(vcpu->kvm);
+
+	if (kvm_x86_ops.tlb_remote_flush == hv_remote_flush_tlb) {
+		spin_lock(&kvm_vmx->ept_pointer_lock);
+		to_vmx(vcpu)->ept_pointer = eptp;
+		if (eptp != kvm_vmx->hv_tlb_eptp)
+			kvm_vmx->hv_tlb_eptp = INVALID_PAGE;
+		spin_unlock(&kvm_vmx->ept_pointer_lock);
+	}
+#endif
+}
+
 /*
  * Comment's format: document - errata name - stepping - processor name.
  * Refer from
@@ -3114,13 +3129,7 @@ static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
 		eptp = construct_eptp(vcpu, root_hpa, root_level);
 		vmcs_write64(EPT_POINTER, eptp);
 
-		if (kvm_x86_ops.tlb_remote_flush) {
-			spin_lock(&to_kvm_vmx(kvm)->ept_pointer_lock);
-			to_vmx(vcpu)->ept_pointer = eptp;
-			if (eptp != to_kvm_vmx(kvm)->hv_tlb_eptp)
-				to_kvm_vmx(kvm)->hv_tlb_eptp = INVALID_PAGE;
-			spin_unlock(&to_kvm_vmx(kvm)->ept_pointer_lock);
-		}
+		hv_load_mmu_eptp(vcpu, eptp);
 
 		if (!enable_unrestricted_guest && !is_paging(vcpu))
 			guest_cr3 = to_kvm_vmx(kvm)->ept_identity_map_addr;
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 09/11] KVM: VMX: Define Hyper-V paravirt TLB flush fields iff Hyper-V is enabled
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
                   ` (7 preceding siblings ...)
  2021-03-05 18:31 ` [PATCH v4 08/11] KVM: VMX: Explicitly check for hv_remote_flush_tlb when loading pgd Sean Christopherson
@ 2021-03-05 18:31 ` Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 10/11] KVM: VMX: Skip additional Hyper-V TLB EPTP flushes if one fails Sean Christopherson
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Sean Christopherson

From: Sean Christopherson <sean.j.christopherson@intel.com>

Ifdef away the Hyper-V specific fields in structs kvm_vmx and vcpu_vmx
as each field has only a single reference outside of the struct itself
that isn't already wrapped in ifdeffery (and both are initialization).

vcpu_vmx.ept_pointer in particular should be wrapped as it is valid if
and only if Hyper-v is active, i.e. non-Hyper-V code cannot rely on it
to actually track the current EPTP (without additional code changes).

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 5 ++++-
 arch/x86/kvm/vmx/vmx.h | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index aca6849c409a..78bda73173d2 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6920,8 +6920,9 @@ static int vmx_create_vcpu(struct kvm_vcpu *vcpu)
 	vmx->pi_desc.nv = POSTED_INTR_VECTOR;
 	vmx->pi_desc.sn = 1;
 
+#if IS_ENABLED(CONFIG_HYPERV)
 	vmx->ept_pointer = INVALID_PAGE;
-
+#endif
 	return 0;
 
 free_vmcs:
@@ -6938,7 +6939,9 @@ static int vmx_create_vcpu(struct kvm_vcpu *vcpu)
 
 static int vmx_vm_init(struct kvm *kvm)
 {
+#if IS_ENABLED(CONFIG_HYPERV)
 	spin_lock_init(&to_kvm_vmx(kvm)->ept_pointer_lock);
+#endif
 
 	if (!ple_gap)
 		kvm->arch.pause_in_guest = true;
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index fb7b2000bd0e..6d97b5a64b62 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -325,7 +325,9 @@ struct vcpu_vmx {
 	 */
 	u64 msr_ia32_feature_control;
 	u64 msr_ia32_feature_control_valid_bits;
+#if IS_ENABLED(CONFIG_HYPERV)
 	u64 ept_pointer;
+#endif
 
 	struct pt_desc pt_desc;
 	struct lbr_desc lbr_desc;
@@ -345,8 +347,10 @@ struct kvm_vmx {
 	bool ept_identity_pagetable_done;
 	gpa_t ept_identity_map_addr;
 
+#if IS_ENABLED(CONFIG_HYPERV)
 	hpa_t hv_tlb_eptp;
 	spinlock_t ept_pointer_lock;
+#endif
 };
 
 bool nested_vmx_allowed(struct kvm_vcpu *vcpu);
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 10/11] KVM: VMX: Skip additional Hyper-V TLB EPTP flushes if one fails
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
                   ` (8 preceding siblings ...)
  2021-03-05 18:31 ` [PATCH v4 09/11] KVM: VMX: Define Hyper-V paravirt TLB flush fields iff Hyper-V is enabled Sean Christopherson
@ 2021-03-05 18:31 ` Sean Christopherson
  2021-03-05 18:31 ` [PATCH v4 11/11] KVM: VMX: Track root HPA instead of EPTP for paravirt Hyper-V TLB flush Sean Christopherson
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Sean Christopherson

From: Sean Christopherson <sean.j.christopherson@intel.com>

Skip additional EPTP flushes if one fails when processing EPTPs for
Hyper-V's paravirt TLB flushing.  If _any_ flush fails, KVM falls back
to a full global flush, i.e. additional flushes are unnecessary (and
will likely fail anyways).

Continue processing the loop unless a mismatch was already detected,
e.g. to handle the case where the first flush fails and there is a
yet-to-be-detected mismatch.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 78bda73173d2..720dcfe2a57d 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -528,7 +528,15 @@ static int hv_remote_flush_tlb_with_range(struct kvm *kvm,
 			if (++nr_unique_valid_eptps == 1)
 				kvm_vmx->hv_tlb_eptp = tmp_eptp;
 
-			ret |= hv_remote_flush_eptp(tmp_eptp, range);
+			if (!ret)
+				ret = hv_remote_flush_eptp(tmp_eptp, range);
+
+			/*
+			 * Stop processing EPTPs if a failure occurred and
+			 * there is already a detected EPTP mismatch.
+			 */
+			if (ret && nr_unique_valid_eptps > 1)
+				break;
 		}
 
 		/*
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 11/11] KVM: VMX: Track root HPA instead of EPTP for paravirt Hyper-V TLB flush
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
                   ` (9 preceding siblings ...)
  2021-03-05 18:31 ` [PATCH v4 10/11] KVM: VMX: Skip additional Hyper-V TLB EPTP flushes if one fails Sean Christopherson
@ 2021-03-05 18:31 ` Sean Christopherson
  2021-03-05 18:33 ` [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV " Sean Christopherson
  2021-03-08 11:50 ` Paolo Bonzini
  12 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Sean Christopherson

From: Sean Christopherson <sean.j.christopherson@intel.com>

Track the address of the top-level EPT struct, a.k.a. the root HPA,
instead of the EPTP itself for Hyper-V's paravirt TLB flush.  The
paravirt API takes only the address, not the full EPTP, and in theory
tracking the EPTP could lead to false negatives, e.g. if the HPA matched
but the attributes in the EPTP do not.  In practice, such a mismatch is
extremely unlikely, if not flat out impossible, given how KVM generates
the EPTP.

Opportunsitically rename the related fields to use the 'root'
nomenclature, and to prefix them with 'hv_' to connect them to Hyper-V's
paravirt TLB flushing.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 83 ++++++++++++++++++++----------------------
 arch/x86/kvm/vmx/vmx.h |  6 +--
 2 files changed, 42 insertions(+), 47 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 720dcfe2a57d..ef826594365f 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -481,18 +481,14 @@ static int kvm_fill_hv_flush_list_func(struct hv_guest_mapping_flush_list *flush
 			range->pages);
 }
 
-static inline int hv_remote_flush_eptp(u64 eptp, struct kvm_tlb_range *range)
+static inline int hv_remote_flush_root_ept(hpa_t root_ept,
+					   struct kvm_tlb_range *range)
 {
-	/*
-	 * FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE hypercall needs address
-	 * of the base of EPT PML4 table, strip off EPT configuration
-	 * information.
-	 */
 	if (range)
-		return hyperv_flush_guest_mapping_range(eptp & PAGE_MASK,
+		return hyperv_flush_guest_mapping_range(root_ept,
 				kvm_fill_hv_flush_list_func, (void *)range);
 	else
-		return hyperv_flush_guest_mapping(eptp & PAGE_MASK);
+		return hyperv_flush_guest_mapping(root_ept);
 }
 
 static int hv_remote_flush_tlb_with_range(struct kvm *kvm,
@@ -500,56 +496,55 @@ static int hv_remote_flush_tlb_with_range(struct kvm *kvm,
 {
 	struct kvm_vmx *kvm_vmx = to_kvm_vmx(kvm);
 	struct kvm_vcpu *vcpu;
-	int ret = 0, i, nr_unique_valid_eptps;
-	u64 tmp_eptp;
+	int ret = 0, i, nr_unique_valid_roots;
+	hpa_t root;
 
-	spin_lock(&kvm_vmx->ept_pointer_lock);
+	spin_lock(&kvm_vmx->hv_root_ept_lock);
 
-	if (!VALID_PAGE(kvm_vmx->hv_tlb_eptp)) {
-		nr_unique_valid_eptps = 0;
+	if (!VALID_PAGE(kvm_vmx->hv_root_ept)) {
+		nr_unique_valid_roots = 0;
 
 		/*
-		 * Flush all valid EPTPs, and see if all vCPUs have converged
-		 * on a common EPTP, in which case future flushes can skip the
-		 * loop and flush the common EPTP.
+		 * Flush all valid roots, and see if all vCPUs have converged
+		 * on a common root, in which case future flushes can skip the
+		 * loop and flush the common root.
 		 */
 		kvm_for_each_vcpu(i, vcpu, kvm) {
-			tmp_eptp = to_vmx(vcpu)->ept_pointer;
-			if (!VALID_PAGE(tmp_eptp) ||
-			    tmp_eptp == kvm_vmx->hv_tlb_eptp)
+			root = to_vmx(vcpu)->hv_root_ept;
+			if (!VALID_PAGE(root) || root == kvm_vmx->hv_root_ept)
 				continue;
 
 			/*
-			 * Set the tracked EPTP to the first valid EPTP.  Keep
-			 * this EPTP for the entirety of the loop even if more
-			 * EPTPs are encountered as a low effort optimization
-			 * to avoid flushing the same (first) EPTP again.
+			 * Set the tracked root to the first valid root.  Keep
+			 * this root for the entirety of the loop even if more
+			 * roots are encountered as a low effort optimization
+			 * to avoid flushing the same (first) root again.
 			 */
-			if (++nr_unique_valid_eptps == 1)
-				kvm_vmx->hv_tlb_eptp = tmp_eptp;
+			if (++nr_unique_valid_roots == 1)
+				kvm_vmx->hv_root_ept = root;
 
 			if (!ret)
-				ret = hv_remote_flush_eptp(tmp_eptp, range);
+				ret = hv_remote_flush_root_ept(root, range);
 
 			/*
-			 * Stop processing EPTPs if a failure occurred and
-			 * there is already a detected EPTP mismatch.
+			 * Stop processing roots if a failure occurred and
+			 * multiple valid roots have already been detected.
 			 */
-			if (ret && nr_unique_valid_eptps > 1)
+			if (ret && nr_unique_valid_roots > 1)
 				break;
 		}
 
 		/*
-		 * The optimized flush of a single EPTP can't be used if there
-		 * are multiple valid EPTPs (obviously).
+		 * The optimized flush of a single root can't be used if there
+		 * are multiple valid roots (obviously).
 		 */
-		if (nr_unique_valid_eptps > 1)
-			kvm_vmx->hv_tlb_eptp = INVALID_PAGE;
+		if (nr_unique_valid_roots > 1)
+			kvm_vmx->hv_root_ept = INVALID_PAGE;
 	} else {
-		ret = hv_remote_flush_eptp(kvm_vmx->hv_tlb_eptp, range);
+		ret = hv_remote_flush_root_ept(kvm_vmx->hv_root_ept, range);
 	}
 
-	spin_unlock(&kvm_vmx->ept_pointer_lock);
+	spin_unlock(&kvm_vmx->hv_root_ept_lock);
 	return ret;
 }
 static int hv_remote_flush_tlb(struct kvm *kvm)
@@ -584,17 +579,17 @@ static int hv_enable_direct_tlbflush(struct kvm_vcpu *vcpu)
 
 #endif /* IS_ENABLED(CONFIG_HYPERV) */
 
-static void hv_load_mmu_eptp(struct kvm_vcpu *vcpu, u64 eptp)
+static void hv_track_root_ept(struct kvm_vcpu *vcpu, hpa_t root_ept)
 {
 #if IS_ENABLED(CONFIG_HYPERV)
 	struct kvm_vmx *kvm_vmx = to_kvm_vmx(vcpu->kvm);
 
 	if (kvm_x86_ops.tlb_remote_flush == hv_remote_flush_tlb) {
-		spin_lock(&kvm_vmx->ept_pointer_lock);
-		to_vmx(vcpu)->ept_pointer = eptp;
-		if (eptp != kvm_vmx->hv_tlb_eptp)
-			kvm_vmx->hv_tlb_eptp = INVALID_PAGE;
-		spin_unlock(&kvm_vmx->ept_pointer_lock);
+		spin_lock(&kvm_vmx->hv_root_ept_lock);
+		to_vmx(vcpu)->hv_root_ept = root_ept;
+		if (root_ept != kvm_vmx->hv_root_ept)
+			kvm_vmx->hv_root_ept = INVALID_PAGE;
+		spin_unlock(&kvm_vmx->hv_root_ept_lock);
 	}
 #endif
 }
@@ -3137,7 +3132,7 @@ static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
 		eptp = construct_eptp(vcpu, root_hpa, root_level);
 		vmcs_write64(EPT_POINTER, eptp);
 
-		hv_load_mmu_eptp(vcpu, eptp);
+		hv_track_root_ept(vcpu, root_hpa);
 
 		if (!enable_unrestricted_guest && !is_paging(vcpu))
 			guest_cr3 = to_kvm_vmx(kvm)->ept_identity_map_addr;
@@ -6929,7 +6924,7 @@ static int vmx_create_vcpu(struct kvm_vcpu *vcpu)
 	vmx->pi_desc.sn = 1;
 
 #if IS_ENABLED(CONFIG_HYPERV)
-	vmx->ept_pointer = INVALID_PAGE;
+	vmx->hv_root_ept = INVALID_PAGE;
 #endif
 	return 0;
 
@@ -6948,7 +6943,7 @@ static int vmx_create_vcpu(struct kvm_vcpu *vcpu)
 static int vmx_vm_init(struct kvm *kvm)
 {
 #if IS_ENABLED(CONFIG_HYPERV)
-	spin_lock_init(&to_kvm_vmx(kvm)->ept_pointer_lock);
+	spin_lock_init(&to_kvm_vmx(kvm)->hv_root_ept_lock);
 #endif
 
 	if (!ple_gap)
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 6d97b5a64b62..0fb3236b0283 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -326,7 +326,7 @@ struct vcpu_vmx {
 	u64 msr_ia32_feature_control;
 	u64 msr_ia32_feature_control_valid_bits;
 #if IS_ENABLED(CONFIG_HYPERV)
-	u64 ept_pointer;
+	u64 hv_root_ept;
 #endif
 
 	struct pt_desc pt_desc;
@@ -348,8 +348,8 @@ struct kvm_vmx {
 	gpa_t ept_identity_map_addr;
 
 #if IS_ENABLED(CONFIG_HYPERV)
-	hpa_t hv_tlb_eptp;
-	spinlock_t ept_pointer_lock;
+	hpa_t hv_root_ept;
+	spinlock_t hv_root_ept_lock;
 #endif
 };
 
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
                   ` (10 preceding siblings ...)
  2021-03-05 18:31 ` [PATCH v4 11/11] KVM: VMX: Track root HPA instead of EPTP for paravirt Hyper-V TLB flush Sean Christopherson
@ 2021-03-05 18:33 ` Sean Christopherson
  2021-03-08 11:50 ` Paolo Bonzini
  12 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2021-03-05 18:33 UTC (permalink / raw)
  To: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

I'm an idiot and Cc'd my old @intel.com address on everything.  Apologies in
advance for the inevitable bounces. :-/

On Fri, Mar 05, 2021, Sean Christopherson wrote:
> Clean up KVM's PV TLB flushing when running with EPT on Hyper-V, i.e. as
> a nested VMM.  No real goal in mind other than the sole patch in v1, which
> is a minor change to avoid a future mixup when TDX also wants to define
> .remote_flush_tlb.  Everything else is opportunistic clean up.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 01/11] KVM: x86: Get active PCID only when writing a CR3 value
  2021-03-05 18:31 ` [PATCH v4 01/11] KVM: x86: Get active PCID only when writing a CR3 value Sean Christopherson
@ 2021-03-08 10:05   ` Vitaly Kuznetsov
  0 siblings, 0 replies; 17+ messages in thread
From: Vitaly Kuznetsov @ 2021-03-08 10:05 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel

Sean Christopherson <seanjc@google.com> writes:

> From: Sean Christopherson <sean.j.christopherson@intel.com>
>
> Retrieve the active PCID only when writing a guest CR3 value, i.e. don't
> get the PCID when using EPT or NPT.  The PCID is especially problematic
> for EPT as the bits have different meaning, and so the PCID and must be
> manually stripped, which is annoying and unnecessary.  And on VMX,
> getting the active PCID also involves reading the guest's CR3 and
> CR4.PCIDE, i.e. may add pointless VMREADs.
>
> Opportunistically rename the pgd/pgd_level params to root_hpa and
> root_level to better reflect their new roles.  Keep the function names,
> as "load the guest PGD" is still accurate/correct.
>
> Last, and probably least, pass root_hpa as a hpa_t/u64 instead of an
> unsigned long.  The EPTP holds a 64-bit value, even in 32-bit mode, so
> in theory EPT could support HIGHMEM for 32-bit KVM.  Never mind that
> doing so would require changing the MMU page allocators and reworking
> the MMU to use kmap().
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  4 ++--
>  arch/x86/kvm/mmu.h              |  4 ++--
>  arch/x86/kvm/svm/svm.c          | 10 ++++++----
>  arch/x86/kvm/vmx/vmx.c          | 13 ++++++-------
>  arch/x86/kvm/vmx/vmx.h          |  3 +--
>  5 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 2da6c9f5935a..51725e994451 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1245,8 +1245,8 @@ struct kvm_x86_ops {
>  	int (*set_identity_map_addr)(struct kvm *kvm, u64 ident_addr);
>  	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
>  
> -	void (*load_mmu_pgd)(struct kvm_vcpu *vcpu, unsigned long pgd,
> -			     int pgd_level);
> +	void (*load_mmu_pgd)(struct kvm_vcpu *vcpu, hpa_t root_hpa,
> +			     int root_level);
>  
>  	bool (*has_wbinvd_exit)(void);
>  
> diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
> index 67e8c7c7a6ce..88d0ed5225a4 100644
> --- a/arch/x86/kvm/mmu.h
> +++ b/arch/x86/kvm/mmu.h
> @@ -107,8 +107,8 @@ static inline void kvm_mmu_load_pgd(struct kvm_vcpu *vcpu)
>  	if (!VALID_PAGE(root_hpa))
>  		return;
>  
> -	static_call(kvm_x86_load_mmu_pgd)(vcpu, root_hpa | kvm_get_active_pcid(vcpu),
> -				 vcpu->arch.mmu->shadow_root_level);
> +	static_call(kvm_x86_load_mmu_pgd)(vcpu, root_hpa,
> +					  vcpu->arch.mmu->shadow_root_level);
>  }
>  
>  int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u32 error_code,
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index dfc8fe231e8b..c7a685bf6862 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3902,14 +3902,14 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
>  	return svm_exit_handlers_fastpath(vcpu);
>  }
>  
> -static void svm_load_mmu_pgd(struct kvm_vcpu *vcpu, unsigned long root,
> +static void svm_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
>  			     int root_level)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  	unsigned long cr3;
>  
>  	if (npt_enabled) {
> -		svm->vmcb->control.nested_cr3 = __sme_set(root);
> +		svm->vmcb->control.nested_cr3 = __sme_set(root_hpa);
>  		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
>  
>  		/* Loading L2's CR3 is handled by enter_svm_guest_mode.  */
> @@ -3917,9 +3917,11 @@ static void svm_load_mmu_pgd(struct kvm_vcpu *vcpu, unsigned long root,
>  			return;
>  		cr3 = vcpu->arch.cr3;
>  	} else if (vcpu->arch.mmu->shadow_root_level >= PT64_ROOT_4LEVEL) {
> -		cr3 = __sme_set(root);
> +		cr3 = __sme_set(root_hpa) | kvm_get_active_pcid(vcpu);
>  	} else {
> -		cr3 = root;
> +		/* PCID in the guest should be impossible with a 32-bit MMU. */
> +		WARN_ON_ONCE(kvm_get_active_pcid(vcpu));
> +		cr3 = root_hpa;
>  	}
>  
>  	svm->vmcb->save.cr3 = cr3;
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 6d7e760fdfa0..dde2ebc7cf3a 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -3088,8 +3088,7 @@ static int vmx_get_max_tdp_level(void)
>  	return 4;
>  }
>  
> -u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa,
> -		   int root_level)
> +u64 construct_eptp(struct kvm_vcpu *vcpu, hpa_t root_hpa, int root_level)
>  {
>  	u64 eptp = VMX_EPTP_MT_WB;
>  
> @@ -3098,13 +3097,13 @@ u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa,
>  	if (enable_ept_ad_bits &&
>  	    (!is_guest_mode(vcpu) || nested_ept_ad_enabled(vcpu)))
>  		eptp |= VMX_EPTP_AD_ENABLE_BIT;
> -	eptp |= (root_hpa & PAGE_MASK);
> +	eptp |= root_hpa;
>  
>  	return eptp;
>  }
>  
> -static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, unsigned long pgd,
> -			     int pgd_level)
> +static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
> +			     int root_level)
>  {
>  	struct kvm *kvm = vcpu->kvm;
>  	bool update_guest_cr3 = true;
> @@ -3112,7 +3111,7 @@ static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, unsigned long pgd,
>  	u64 eptp;
>  
>  	if (enable_ept) {
> -		eptp = construct_eptp(vcpu, pgd, pgd_level);
> +		eptp = construct_eptp(vcpu, root_hpa, root_level);
>  		vmcs_write64(EPT_POINTER, eptp);
>  
>  		if (kvm_x86_ops.tlb_remote_flush) {
> @@ -3131,7 +3130,7 @@ static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, unsigned long pgd,
>  			update_guest_cr3 = false;
>  		vmx_ept_load_pdptrs(vcpu);
>  	} else {
> -		guest_cr3 = pgd;
> +		guest_cr3 = root_hpa | kvm_get_active_pcid(vcpu);
>  	}
>  
>  	if (update_guest_cr3)
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index 89da5e1251f1..4795955490cb 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -376,8 +376,7 @@ void set_cr4_guest_host_mask(struct vcpu_vmx *vmx);
>  void ept_save_pdptrs(struct kvm_vcpu *vcpu);
>  void vmx_get_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int seg);
>  void vmx_set_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int seg);
> -u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa,
> -		   int root_level);
> +u64 construct_eptp(struct kvm_vcpu *vcpu, hpa_t root_hpa, int root_level);
>  
>  void vmx_update_exception_bitmap(struct kvm_vcpu *vcpu);
>  void vmx_update_msr_bitmap(struct kvm_vcpu *vcpu);

The rest of the series carries my R-b so let's not make this patch feel
sad. Personally, I'd suggest we split this into 're-name/re-type' and
'PCID' patches but it's not a big deal.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

-- 
Vitaly


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush
  2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
                   ` (11 preceding siblings ...)
  2021-03-05 18:33 ` [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV " Sean Christopherson
@ 2021-03-08 11:50 ` Paolo Bonzini
  2021-03-09  1:18   ` Sean Christopherson
  12 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2021-03-08 11:50 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
	linux-kernel, Sean Christopherson

On 05/03/21 19:31, Sean Christopherson wrote:
> Clean up KVM's PV TLB flushing when running with EPT on Hyper-V, i.e. as
> a nested VMM.  No real goal in mind other than the sole patch in v1, which
> is a minor change to avoid a future mixup when TDX also wants to define
> .remote_flush_tlb.  Everything else is opportunistic clean up.
> 
> NOTE: Based on my NPT+SME bug fix series[*] due to multiple conflicts with
>        non-trivial resolutions.
> 
> [*] https://lkml.kernel.org/r/20210305011101.3597423-1-seanjc@google.com
> 
> 
> Patch 1 legitimately tested on VMX and SVM (including i386).  Patches 2+
> smoke tested by hacking usage of the relevant flows without actually
> routing to the Hyper-V hypercalls (partial hack-a-patch below).
> 
> -static inline int hv_remote_flush_root_ept(hpa_t root_ept,
> +static inline int hv_remote_flush_root_ept(struct kvm *kvm, hpa_t root_ept,
>                                             struct kvm_tlb_range *range)
>   {
> -       if (range)
> -               return hyperv_flush_guest_mapping_range(root_ept,
> -                               kvm_fill_hv_flush_list_func, (void *)range);
> -       else
> -               return hyperv_flush_guest_mapping(root_ept);
> +       if (range) {
> +               kvm_make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH);
> +               return 0;
> +       }
> +
> +       return -ENOMEM;
>   }
>   
>   static int hv_remote_flush_tlb_with_range(struct kvm *kvm,
> @@ -7753,8 +7754,7 @@ static __init int hardware_setup(void)
>                  vmx_x86_ops.update_cr8_intercept = NULL;
>   
>   #if IS_ENABLED(CONFIG_HYPERV)
> -       if (ms_hyperv.nested_features & HV_X64_NESTED_GUEST_MAPPING_FLUSH
> -           && enable_ept) {
> +       if (enable_ept) {
>                  vmx_x86_ops.tlb_remote_flush = hv_remote_flush_tlb;
>                  vmx_x86_ops.tlb_remote_flush_with_range =
>                                  hv_remote_flush_tlb_with_range;
> 
> v4:
>    - Rebased to kvm/queue, commit fe5f0041c026 ("KVM/SVM: Move vmenter.S
>      exception fixups out of line"), plus the aforementioned series.
>    - Don't grab PCID for nested_cr3 (NPT). [Paolo]
>    - Collect reviews. [Vitaly]
> 
> v3:
>    - https://lkml.kernel.org/r/20201027212346.23409-1-sean.j.christopherson@intel.com
>    - Add a patch to pass the root_hpa instead of pgd to vmx_load_mmu_pgd()
>      and retrieve the active PCID only when necessary.  [Vitaly]
>    - Selectively collects reviews (skipped a few due to changes). [Vitaly]
>    - Explicitly invalidate hv_tlb_eptp instead of leaving it valid when
>      the mismatch tracker "knows" it's invalid. [Vitaly]
>    - Change the last patch to use "hv_root_ept" instead of "hv_tlb_pgd"
>      to better reflect what is actually being tracked.
> 
> v2:
>    - Rewrite everything.
>    - https://lkml.kernel.org/r/20201020215613.8972-1-sean.j.christopherson@intel.com
> 
> v1: ???
> 
> Sean Christopherson (11):
>    KVM: x86: Get active PCID only when writing a CR3 value
>    KVM: VMX: Track common EPTP for Hyper-V's paravirt TLB flush
>    KVM: VMX: Stash kvm_vmx in a local variable for Hyper-V paravirt TLB
>      flush
>    KVM: VMX: Fold Hyper-V EPTP checking into it's only caller
>    KVM: VMX: Do Hyper-V TLB flush iff vCPU's EPTP hasn't been flushed
>    KVM: VMX: Invalidate hv_tlb_eptp to denote an EPTP mismatch
>    KVM: VMX: Don't invalidate hv_tlb_eptp if the new EPTP matches
>    KVM: VMX: Explicitly check for hv_remote_flush_tlb when loading pgd
>    KVM: VMX: Define Hyper-V paravirt TLB flush fields iff Hyper-V is
>      enabled
>    KVM: VMX: Skip additional Hyper-V TLB EPTP flushes if one fails
>    KVM: VMX: Track root HPA instead of EPTP for paravirt Hyper-V TLB
>      flush
> 
>   arch/x86/include/asm/kvm_host.h |   4 +-
>   arch/x86/kvm/mmu.h              |   4 +-
>   arch/x86/kvm/svm/svm.c          |  10 ++-
>   arch/x86/kvm/vmx/vmx.c          | 134 ++++++++++++++++++--------------
>   arch/x86/kvm/vmx/vmx.h          |  19 ++---
>   5 files changed, 92 insertions(+), 79 deletions(-)
> 

Huh, I was sure I had queued this already for 5.12.  Well, done so now.

Paolo


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush
  2021-03-08 11:50 ` Paolo Bonzini
@ 2021-03-09  1:18   ` Sean Christopherson
  2021-03-09  8:35     ` Paolo Bonzini
  0 siblings, 1 reply; 17+ messages in thread
From: Sean Christopherson @ 2021-03-09  1:18 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
	linux-kernel, Sean Christopherson

On Mon, Mar 08, 2021, Paolo Bonzini wrote:
> On 05/03/21 19:31, Sean Christopherson wrote:
> > Sean Christopherson (11):
> >    KVM: x86: Get active PCID only when writing a CR3 value

...

> Huh, I was sure I had queued this already for 5.12.  Well, done so now.

Maybe this series is cursed.  The first patch got mangled and broke SME.
It shows up as two commits with the same changelog, so maybe you intended to
split the patch and things went sideways?

Anyways, commit a16241ae56fa ("KVM: x86: Get active PCID only when writing a
CR3 value") breaks SME and PCID.  The kvm/queue code looks like this:


	cr3 = __sme_set(root_hpa);
	if (npt_enabled) {
		svm->vmcb->control.nested_cr3 = root_hpa;
		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);

		/* Loading L2's CR3 is handled by enter_svm_guest_mode.  */
		if (!test_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail))
			return;
		cr3 = vcpu->arch.cr3;
	}

	svm->vmcb->save.cr3 = cr3;
	vmcb_mark_dirty(svm->vmcb, VMCB_CR);

but it should look like this:

	if (npt_enabled) {
		svm->vmcb->control.nested_cr3 = __sme_set(root);
		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);

		/* Loading L2's CR3 is handled by enter_svm_guest_mode.  */
		if (!test_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail))
			return;
		cr3 = vcpu->arch.cr3;
	} else if (vcpu->arch.mmu->shadow_root_level >= PT64_ROOT_4LEVEL) {
		cr3 = __sme_set(root);
	} else {
		cr3 = root;
	}

	svm->vmcb->save.cr3 = cr3;
	vmcb_mark_dirty(svm->vmcb, VMCB_CR);

I'll generate a delta patch, and test and post, just in case there is other
stuff that got lost.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush
  2021-03-09  1:18   ` Sean Christopherson
@ 2021-03-09  8:35     ` Paolo Bonzini
  0 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-03-09  8:35 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
	linux-kernel, Sean Christopherson

On 09/03/21 02:18, Sean Christopherson wrote:
> Maybe this series is cursed.  The first patch got mangled and broke SME.
> It shows up as two commits with the same changelog, so maybe you intended to
> split the patch and things went sideways?

There was a conflict.  I admit kvm/queue is not always that good, 
usually I try to test it but yesterday I just didn't have time.

I'll fix up everything (also 20/24 in the other series).

Oh well, you have to break eggs to make an omelette. :)

Paolo

> Anyways, commit a16241ae56fa ("KVM: x86: Get active PCID only when writing a
> CR3 value") breaks SME and PCID.  The kvm/queue code looks like this:
> 
> 
> 	cr3 = __sme_set(root_hpa);
> 	if (npt_enabled) {
> 		svm->vmcb->control.nested_cr3 = root_hpa;
> 		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
> 
> 		/* Loading L2's CR3 is handled by enter_svm_guest_mode.  */
> 		if (!test_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail))
> 			return;
> 		cr3 = vcpu->arch.cr3;
> 	}
> 
> 	svm->vmcb->save.cr3 = cr3;
> 	vmcb_mark_dirty(svm->vmcb, VMCB_CR);
> 
> but it should look like this:
> 
> 	if (npt_enabled) {
> 		svm->vmcb->control.nested_cr3 = __sme_set(root);
> 		vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
> 
> 		/* Loading L2's CR3 is handled by enter_svm_guest_mode.  */
> 		if (!test_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail))
> 			return;
> 		cr3 = vcpu->arch.cr3;
> 	} else if (vcpu->arch.mmu->shadow_root_level >= PT64_ROOT_4LEVEL) {
> 		cr3 = __sme_set(root);
> 	} else {
> 		cr3 = root;
> 	}
> 
> 	svm->vmcb->save.cr3 = cr3;
> 	vmcb_mark_dirty(svm->vmcb, VMCB_CR);
> 
> I'll generate a delta patch, and test and post, just in case there is other
> stuff that got lost.
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-03-09  8:35 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-05 18:31 [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
2021-03-05 18:31 ` [PATCH v4 01/11] KVM: x86: Get active PCID only when writing a CR3 value Sean Christopherson
2021-03-08 10:05   ` Vitaly Kuznetsov
2021-03-05 18:31 ` [PATCH v4 02/11] KVM: VMX: Track common EPTP for Hyper-V's paravirt TLB flush Sean Christopherson
2021-03-05 18:31 ` [PATCH v4 03/11] KVM: VMX: Stash kvm_vmx in a local variable for Hyper-V " Sean Christopherson
2021-03-05 18:31 ` [PATCH v4 04/11] KVM: VMX: Fold Hyper-V EPTP checking into it's only caller Sean Christopherson
2021-03-05 18:31 ` [PATCH v4 05/11] KVM: VMX: Do Hyper-V TLB flush iff vCPU's EPTP hasn't been flushed Sean Christopherson
2021-03-05 18:31 ` [PATCH v4 06/11] KVM: VMX: Invalidate hv_tlb_eptp to denote an EPTP mismatch Sean Christopherson
2021-03-05 18:31 ` [PATCH v4 07/11] KVM: VMX: Don't invalidate hv_tlb_eptp if the new EPTP matches Sean Christopherson
2021-03-05 18:31 ` [PATCH v4 08/11] KVM: VMX: Explicitly check for hv_remote_flush_tlb when loading pgd Sean Christopherson
2021-03-05 18:31 ` [PATCH v4 09/11] KVM: VMX: Define Hyper-V paravirt TLB flush fields iff Hyper-V is enabled Sean Christopherson
2021-03-05 18:31 ` [PATCH v4 10/11] KVM: VMX: Skip additional Hyper-V TLB EPTP flushes if one fails Sean Christopherson
2021-03-05 18:31 ` [PATCH v4 11/11] KVM: VMX: Track root HPA instead of EPTP for paravirt Hyper-V TLB flush Sean Christopherson
2021-03-05 18:33 ` [PATCH v4 00/11] KVM: VMX: Clean up Hyper-V PV " Sean Christopherson
2021-03-08 11:50 ` Paolo Bonzini
2021-03-09  1:18   ` Sean Christopherson
2021-03-09  8:35     ` Paolo Bonzini

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.