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.
Patch 1 legitimately tested on VMX (no SVM), everything else effectively
build tested only.
v3:
- 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.
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 | 2 +-
arch/x86/kvm/svm/svm.c | 4 +-
arch/x86/kvm/vmx/vmx.c | 134 ++++++++++++++++++--------------
arch/x86/kvm/vmx/vmx.h | 19 ++---
5 files changed, 87 insertions(+), 76 deletions(-)
--
2.28.0
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.
Signed-off-by: Sean Christopherson <[email protected]>
---
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 5b7c5b2fd2c7..40a67dd45c8c 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.28.0
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.
Signed-off-by: Sean Christopherson <[email protected]>
---
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 273a3206cef7..ebc87df4da4d 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 a2d143276603..9a25e83f8b96 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -301,6 +301,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.28.0
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 <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
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 4d9bc0d3a929..b684f45d6a78 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
@@ -3069,13 +3084,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.28.0
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).
Signed-off-by: Sean Christopherson <[email protected]>
---
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 b684f45d6a78..5b7c5b2fd2c7 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6955,8 +6955,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:
@@ -6973,7 +6974,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 cecc2a641e19..2bd86d8b2f4b 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -276,7 +276,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;
@@ -295,8 +297,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.28.0
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.
Signed-off-by: Sean Christopherson <[email protected]>
---
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 f5e9e2f61e10..17b228c4ba19 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.28.0
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 <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
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 ebc87df4da4d..a6442a861ffc 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.28.0
Sean Christopherson <[email protected]> writes:
> 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.
>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> 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 273a3206cef7..ebc87df4da4d 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 a2d143276603..9a25e83f8b96 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -301,6 +301,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;
> };
Reviewed-by: Vitaly Kuznetsov <[email protected]>
--
Vitaly
Sean Christopherson <[email protected]> writes:
> 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.
>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> 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 f5e9e2f61e10..17b228c4ba19 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);
> }
It seems this patch makes EPT_POINTERS_MISMATCH an alias for
EPT_POINTERS_CHECK but this all is gone in the next patch, so
Reviewed-by: Vitaly Kuznetsov <[email protected]>
--
Vitaly
Sean Christopherson <[email protected]> writes:
> 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).
>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> 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 b684f45d6a78..5b7c5b2fd2c7 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -6955,8 +6955,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:
> @@ -6973,7 +6974,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 cecc2a641e19..2bd86d8b2f4b 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -276,7 +276,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;
>
> @@ -295,8 +297,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);
Reviewed-by: Vitaly Kuznetsov <[email protected]>
--
Vitaly
Sean Christopherson <[email protected]> writes:
> 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.
>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> 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 5b7c5b2fd2c7..40a67dd45c8c 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;
> }
>
> /*
This should never happen (famous last words) but why not optimize the
impossibility :-)
Reviewed-by: Vitaly Kuznetsov <[email protected]>
--
Vitaly
On 27/10/20 22:23, 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.
>
> Patch 1 legitimately tested on VMX (no SVM), everything else effectively
> build tested only.
>
> v3:
> - 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.
>
> 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 | 2 +-
> arch/x86/kvm/svm/svm.c | 4 +-
> arch/x86/kvm/vmx/vmx.c | 134 ++++++++++++++++++--------------
> arch/x86/kvm/vmx/vmx.h | 19 ++---
> 5 files changed, 87 insertions(+), 76 deletions(-)
>
Queued, thanks.
Paolo
On Wed, Jan 27, 2021, Paolo Bonzini wrote:
> On 27/10/20 22:23, 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.
> >
> > Patch 1 legitimately tested on VMX (no SVM), everything else effectively
> > build tested only.
> >
> > v3:
> > - 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.
> > 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 | 2 +-
> > arch/x86/kvm/svm/svm.c | 4 +-
> > arch/x86/kvm/vmx/vmx.c | 134 ++++++++++++++++++--------------
> > arch/x86/kvm/vmx/vmx.h | 19 ++---
> > 5 files changed, 87 insertions(+), 76 deletions(-)
> >
>
> Queued, thanks.
Looks like this got shadow-banned, I'll send v4.