* [PATCH v3 01/11] KVM: x86: Get active PCID only when writing a CR3 value
2020-10-27 21:23 [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
@ 2020-10-27 21:23 ` Sean Christopherson
2020-11-12 10:11 ` Vitaly Kuznetsov
2021-01-27 17:30 ` Paolo Bonzini
2020-10-27 21:23 ` [PATCH v3 02/11] KVM: VMX: Track common EPTP for Hyper-V's paravirt TLB flush Sean Christopherson
` (10 subsequent siblings)
11 siblings, 2 replies; 23+ messages in thread
From: Sean Christopherson @ 2020-10-27 21:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
Retrieve the active PCID only when writing a guest CR3 value, i.e. don't
get the PCID when using EPT. The PCID is not used when EPT is enabled,
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>
---
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 | 13 ++++++-------
arch/x86/kvm/vmx/vmx.h | 3 +--
5 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index d44858b69353..33b2acfd7869 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1181,8 +1181,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 9c4a9c8e43d9..add537a39177 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -95,7 +95,7 @@ static inline void kvm_mmu_load_pgd(struct kvm_vcpu *vcpu)
if (!VALID_PAGE(root_hpa))
return;
- kvm_x86_ops.load_mmu_pgd(vcpu, root_hpa | kvm_get_active_pcid(vcpu),
+ kvm_x86_ops.load_mmu_pgd(vcpu, root_hpa,
vcpu->arch.mmu->shadow_root_level);
}
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index cf951e588dd1..4a6a5a3dc963 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3667,13 +3667,13 @@ 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;
- cr3 = __sme_set(root);
+ cr3 = __sme_set(root_hpa) | kvm_get_active_pcid(vcpu);
if (npt_enabled) {
svm->vmcb->control.nested_cr3 = cr3;
vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 281c405c7ea3..273a3206cef7 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -3043,8 +3043,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;
@@ -3053,13 +3052,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;
@@ -3067,7 +3066,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) {
@@ -3086,7 +3085,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 f6f66e5c6510..a2d143276603 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -326,8 +326,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 update_exception_bitmap(struct kvm_vcpu *vcpu);
void vmx_update_msr_bitmap(struct kvm_vcpu *vcpu);
--
2.28.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 01/11] KVM: x86: Get active PCID only when writing a CR3 value
2020-10-27 21:23 ` [PATCH v3 01/11] KVM: x86: Get active PCID only when writing a CR3 value Sean Christopherson
@ 2020-11-12 10:11 ` Vitaly Kuznetsov
2021-01-27 17:30 ` Paolo Bonzini
1 sibling, 0 replies; 23+ messages in thread
From: Vitaly Kuznetsov @ 2020-11-12 10:11 UTC (permalink / raw)
To: Sean Christopherson
Cc: Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel, Paolo Bonzini
Sean Christopherson <sean.j.christopherson@intel.com> writes:
> Retrieve the active PCID only when writing a guest CR3 value, i.e. don't
> get the PCID when using EPT. The PCID is not used when EPT is enabled,
> 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>
> ---
> 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 | 13 ++++++-------
> arch/x86/kvm/vmx/vmx.h | 3 +--
> 5 files changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index d44858b69353..33b2acfd7869 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1181,8 +1181,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 9c4a9c8e43d9..add537a39177 100644
> --- a/arch/x86/kvm/mmu.h
> +++ b/arch/x86/kvm/mmu.h
> @@ -95,7 +95,7 @@ static inline void kvm_mmu_load_pgd(struct kvm_vcpu *vcpu)
> if (!VALID_PAGE(root_hpa))
> return;
>
> - kvm_x86_ops.load_mmu_pgd(vcpu, root_hpa | kvm_get_active_pcid(vcpu),
> + kvm_x86_ops.load_mmu_pgd(vcpu, root_hpa,
> vcpu->arch.mmu->shadow_root_level);
> }
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index cf951e588dd1..4a6a5a3dc963 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3667,13 +3667,13 @@ 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;
>
> - cr3 = __sme_set(root);
> + cr3 = __sme_set(root_hpa) | kvm_get_active_pcid(vcpu);
> if (npt_enabled) {
> svm->vmcb->control.nested_cr3 = cr3;
> vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 281c405c7ea3..273a3206cef7 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -3043,8 +3043,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;
>
> @@ -3053,13 +3052,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;
(Nitpicking according to personal taste):
It looks a bit weird that we start building 'eptp' from flags and add
pointer as the last step, I'd suggest we re-write the function as:
u64 construct_eptp(struct kvm_vcpu *vcpu, hpa_t root_hpa, int root_level)
{
u64 eptp = root_hpa | VMX_EPTP_MT_WB;
eptp |= (root_level == 5) ? VMX_EPTP_PWL_5 : VMX_EPTP_PWL_4;
if (enable_ept_ad_bits &&
(!is_guest_mode(vcpu) || nested_ept_ad_enabled(vcpu)))
eptp |= VMX_EPTP_AD_ENABLE_BIT;
return eptp;
}
>
> 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;
> @@ -3067,7 +3066,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) {
> @@ -3086,7 +3085,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 f6f66e5c6510..a2d143276603 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -326,8 +326,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 update_exception_bitmap(struct kvm_vcpu *vcpu);
> void vmx_update_msr_bitmap(struct kvm_vcpu *vcpu);
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 01/11] KVM: x86: Get active PCID only when writing a CR3 value
2020-10-27 21:23 ` [PATCH v3 01/11] KVM: x86: Get active PCID only when writing a CR3 value Sean Christopherson
2020-11-12 10:11 ` Vitaly Kuznetsov
@ 2021-01-27 17:30 ` Paolo Bonzini
1 sibling, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2021-01-27 17:30 UTC (permalink / raw)
To: Sean Christopherson
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel
On 27/10/20 22:23, Sean Christopherson wrote:
>
> +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;
>
> - cr3 = __sme_set(root);
> + cr3 = __sme_set(root_hpa) | kvm_get_active_pcid(vcpu);
> if (npt_enabled) {
> svm->vmcb->control.nested_cr3 = cr3;
SVM uses the name "nested CR3" so this variable actually could represent
an NPT value that does not need the PCID.
Therefore, this change must be done in an else branch, which I've done
on applying the patch.
Paolo
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 02/11] KVM: VMX: Track common EPTP for Hyper-V's paravirt TLB flush
2020-10-27 21:23 [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
2020-10-27 21:23 ` [PATCH v3 01/11] KVM: x86: Get active PCID only when writing a CR3 value Sean Christopherson
@ 2020-10-27 21:23 ` Sean Christopherson
2020-11-12 10:27 ` Vitaly Kuznetsov
2020-10-27 21:23 ` [PATCH v3 03/11] KVM: VMX: Stash kvm_vmx in a local variable for Hyper-V " Sean Christopherson
` (9 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Sean Christopherson @ 2020-10-27 21:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
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 <sean.j.christopherson@intel.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 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 02/11] KVM: VMX: Track common EPTP for Hyper-V's paravirt TLB flush
2020-10-27 21:23 ` [PATCH v3 02/11] KVM: VMX: Track common EPTP for Hyper-V's paravirt TLB flush Sean Christopherson
@ 2020-11-12 10:27 ` Vitaly Kuznetsov
0 siblings, 0 replies; 23+ messages in thread
From: Vitaly Kuznetsov @ 2020-11-12 10:27 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel
Sean Christopherson <sean.j.christopherson@intel.com> 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 <sean.j.christopherson@intel.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 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 <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 03/11] KVM: VMX: Stash kvm_vmx in a local variable for Hyper-V paravirt TLB flush
2020-10-27 21:23 [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
2020-10-27 21:23 ` [PATCH v3 01/11] KVM: x86: Get active PCID only when writing a CR3 value Sean Christopherson
2020-10-27 21:23 ` [PATCH v3 02/11] KVM: VMX: Track common EPTP for Hyper-V's paravirt TLB flush Sean Christopherson
@ 2020-10-27 21:23 ` Sean Christopherson
2020-10-27 21:23 ` [PATCH v3 04/11] KVM: VMX: Fold Hyper-V EPTP checking into it's only caller Sean Christopherson
` (8 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Sean Christopherson @ 2020-10-27 21:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
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>
---
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 04/11] KVM: VMX: Fold Hyper-V EPTP checking into it's only caller
2020-10-27 21:23 [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
` (2 preceding siblings ...)
2020-10-27 21:23 ` [PATCH v3 03/11] KVM: VMX: Stash kvm_vmx in a local variable for Hyper-V " Sean Christopherson
@ 2020-10-27 21:23 ` Sean Christopherson
2020-11-12 10:41 ` Vitaly Kuznetsov
2020-10-27 21:23 ` [PATCH v3 05/11] KVM: VMX: Do Hyper-V TLB flush iff vCPU's EPTP hasn't been flushed Sean Christopherson
` (7 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Sean Christopherson @ 2020-10-27 21:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
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.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.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 a6442a861ffc..f5e9e2f61e10 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.28.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 04/11] KVM: VMX: Fold Hyper-V EPTP checking into it's only caller
2020-10-27 21:23 ` [PATCH v3 04/11] KVM: VMX: Fold Hyper-V EPTP checking into it's only caller Sean Christopherson
@ 2020-11-12 10:41 ` Vitaly Kuznetsov
0 siblings, 0 replies; 23+ messages in thread
From: Vitaly Kuznetsov @ 2020-11-12 10:41 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel
Sean Christopherson <sean.j.christopherson@intel.com> writes:
> 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.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.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 a6442a861ffc..f5e9e2f61e10 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) {
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 05/11] KVM: VMX: Do Hyper-V TLB flush iff vCPU's EPTP hasn't been flushed
2020-10-27 21:23 [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
` (3 preceding siblings ...)
2020-10-27 21:23 ` [PATCH v3 04/11] KVM: VMX: Fold Hyper-V EPTP checking into it's only caller Sean Christopherson
@ 2020-10-27 21:23 ` Sean Christopherson
2020-11-12 10:47 ` Vitaly Kuznetsov
2020-10-27 21:23 ` [PATCH v3 06/11] KVM: VMX: Invalidate hv_tlb_eptp to denote an EPTP mismatch Sean Christopherson
` (6 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Sean Christopherson @ 2020-10-27 21:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
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 <sean.j.christopherson@intel.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 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 05/11] KVM: VMX: Do Hyper-V TLB flush iff vCPU's EPTP hasn't been flushed
2020-10-27 21:23 ` [PATCH v3 05/11] KVM: VMX: Do Hyper-V TLB flush iff vCPU's EPTP hasn't been flushed Sean Christopherson
@ 2020-11-12 10:47 ` Vitaly Kuznetsov
0 siblings, 0 replies; 23+ messages in thread
From: Vitaly Kuznetsov @ 2020-11-12 10:47 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel
Sean Christopherson <sean.j.christopherson@intel.com> 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 <sean.j.christopherson@intel.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 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 <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 06/11] KVM: VMX: Invalidate hv_tlb_eptp to denote an EPTP mismatch
2020-10-27 21:23 [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
` (4 preceding siblings ...)
2020-10-27 21:23 ` [PATCH v3 05/11] KVM: VMX: Do Hyper-V TLB flush iff vCPU's EPTP hasn't been flushed Sean Christopherson
@ 2020-10-27 21:23 ` Sean Christopherson
2020-11-12 10:50 ` Vitaly Kuznetsov
2020-10-27 21:23 ` [PATCH v3 07/11] KVM: VMX: Don't invalidate hv_tlb_eptp if the new EPTP matches Sean Christopherson
` (5 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Sean Christopherson @ 2020-10-27 21:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
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.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.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 17b228c4ba19..25a714cda662 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);
}
@@ -3060,8 +3072,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 9a25e83f8b96..cecc2a641e19 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -288,12 +288,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;
@@ -302,7 +296,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.28.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 06/11] KVM: VMX: Invalidate hv_tlb_eptp to denote an EPTP mismatch
2020-10-27 21:23 ` [PATCH v3 06/11] KVM: VMX: Invalidate hv_tlb_eptp to denote an EPTP mismatch Sean Christopherson
@ 2020-11-12 10:50 ` Vitaly Kuznetsov
0 siblings, 0 replies; 23+ messages in thread
From: Vitaly Kuznetsov @ 2020-11-12 10:50 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel
Sean Christopherson <sean.j.christopherson@intel.com> writes:
> 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.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.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 17b228c4ba19..25a714cda662 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);
> }
>
> @@ -3060,8 +3072,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 9a25e83f8b96..cecc2a641e19 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -288,12 +288,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;
>
> @@ -302,7 +296,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;
> };
This looks really neat and straighforward now, thanks!
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 07/11] KVM: VMX: Don't invalidate hv_tlb_eptp if the new EPTP matches
2020-10-27 21:23 [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
` (5 preceding siblings ...)
2020-10-27 21:23 ` [PATCH v3 06/11] KVM: VMX: Invalidate hv_tlb_eptp to denote an EPTP mismatch Sean Christopherson
@ 2020-10-27 21:23 ` Sean Christopherson
2020-10-27 21:23 ` [PATCH v3 08/11] KVM: VMX: Explicitly check for hv_remote_flush_tlb when loading pgd Sean Christopherson
` (4 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Sean Christopherson @ 2020-10-27 21:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
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>
---
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 25a714cda662..4d9bc0d3a929 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -3072,7 +3072,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.28.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 08/11] KVM: VMX: Explicitly check for hv_remote_flush_tlb when loading pgd
2020-10-27 21:23 [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
` (6 preceding siblings ...)
2020-10-27 21:23 ` [PATCH v3 07/11] KVM: VMX: Don't invalidate hv_tlb_eptp if the new EPTP matches Sean Christopherson
@ 2020-10-27 21:23 ` Sean Christopherson
2020-10-27 21:23 ` [PATCH v3 09/11] KVM: VMX: Define Hyper-V paravirt TLB flush fields iff Hyper-V is enabled Sean Christopherson
` (3 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Sean Christopherson @ 2020-10-27 21:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
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>
---
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 09/11] KVM: VMX: Define Hyper-V paravirt TLB flush fields iff Hyper-V is enabled
2020-10-27 21:23 [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
` (7 preceding siblings ...)
2020-10-27 21:23 ` [PATCH v3 08/11] KVM: VMX: Explicitly check for hv_remote_flush_tlb when loading pgd Sean Christopherson
@ 2020-10-27 21:23 ` Sean Christopherson
2020-11-12 10:52 ` Vitaly Kuznetsov
2020-10-27 21:23 ` [PATCH v3 10/11] KVM: VMX: Skip additional Hyper-V TLB EPTP flushes if one fails Sean Christopherson
` (2 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Sean Christopherson @ 2020-10-27 21:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
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 <sean.j.christopherson@intel.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 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 09/11] KVM: VMX: Define Hyper-V paravirt TLB flush fields iff Hyper-V is enabled
2020-10-27 21:23 ` [PATCH v3 09/11] KVM: VMX: Define Hyper-V paravirt TLB flush fields iff Hyper-V is enabled Sean Christopherson
@ 2020-11-12 10:52 ` Vitaly Kuznetsov
0 siblings, 0 replies; 23+ messages in thread
From: Vitaly Kuznetsov @ 2020-11-12 10:52 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel
Sean Christopherson <sean.j.christopherson@intel.com> 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 <sean.j.christopherson@intel.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 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 <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 10/11] KVM: VMX: Skip additional Hyper-V TLB EPTP flushes if one fails
2020-10-27 21:23 [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
` (8 preceding siblings ...)
2020-10-27 21:23 ` [PATCH v3 09/11] KVM: VMX: Define Hyper-V paravirt TLB flush fields iff Hyper-V is enabled Sean Christopherson
@ 2020-10-27 21:23 ` Sean Christopherson
2020-11-12 10:59 ` Vitaly Kuznetsov
2020-10-27 21:23 ` [PATCH v3 11/11] KVM: VMX: Track root HPA instead of EPTP for paravirt Hyper-V TLB flush Sean Christopherson
2021-01-27 18:10 ` [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV " Paolo Bonzini
11 siblings, 1 reply; 23+ messages in thread
From: Sean Christopherson @ 2020-10-27 21:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
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 <sean.j.christopherson@intel.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 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 10/11] KVM: VMX: Skip additional Hyper-V TLB EPTP flushes if one fails
2020-10-27 21:23 ` [PATCH v3 10/11] KVM: VMX: Skip additional Hyper-V TLB EPTP flushes if one fails Sean Christopherson
@ 2020-11-12 10:59 ` Vitaly Kuznetsov
0 siblings, 0 replies; 23+ messages in thread
From: Vitaly Kuznetsov @ 2020-11-12 10:59 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel
Sean Christopherson <sean.j.christopherson@intel.com> 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 <sean.j.christopherson@intel.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 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 <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 11/11] KVM: VMX: Track root HPA instead of EPTP for paravirt Hyper-V TLB flush
2020-10-27 21:23 [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
` (9 preceding siblings ...)
2020-10-27 21:23 ` [PATCH v3 10/11] KVM: VMX: Skip additional Hyper-V TLB EPTP flushes if one fails Sean Christopherson
@ 2020-10-27 21:23 ` Sean Christopherson
2020-11-12 11:02 ` Vitaly Kuznetsov
2021-01-27 18:10 ` [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV " Paolo Bonzini
11 siblings, 1 reply; 23+ messages in thread
From: Sean Christopherson @ 2020-10-27 21:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
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.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.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 40a67dd45c8c..330d42ac5e02 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
}
@@ -3092,7 +3087,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;
@@ -6964,7 +6959,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;
@@ -6983,7 +6978,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 2bd86d8b2f4b..5c98d16a0c6f 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -277,7 +277,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;
@@ -298,8 +298,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.28.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 11/11] KVM: VMX: Track root HPA instead of EPTP for paravirt Hyper-V TLB flush
2020-10-27 21:23 ` [PATCH v3 11/11] KVM: VMX: Track root HPA instead of EPTP for paravirt Hyper-V TLB flush Sean Christopherson
@ 2020-11-12 11:02 ` Vitaly Kuznetsov
0 siblings, 0 replies; 23+ messages in thread
From: Vitaly Kuznetsov @ 2020-11-12 11:02 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel
Sean Christopherson <sean.j.christopherson@intel.com> writes:
> 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.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.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 40a67dd45c8c..330d42ac5e02 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
> }
> @@ -3092,7 +3087,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;
> @@ -6964,7 +6959,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;
>
> @@ -6983,7 +6978,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 2bd86d8b2f4b..5c98d16a0c6f 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -277,7 +277,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;
> @@ -298,8 +298,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
> };
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush
2020-10-27 21:23 [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush Sean Christopherson
` (10 preceding siblings ...)
2020-10-27 21:23 ` [PATCH v3 11/11] KVM: VMX: Track root HPA instead of EPTP for paravirt Hyper-V TLB flush Sean Christopherson
@ 2021-01-27 18:10 ` Paolo Bonzini
2021-03-02 18:56 ` Sean Christopherson
11 siblings, 1 reply; 23+ messages in thread
From: Paolo Bonzini @ 2021-01-27 18:10 UTC (permalink / raw)
To: Sean Christopherson
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV TLB flush
2021-01-27 18:10 ` [PATCH v3 00/11] KVM: VMX: Clean up Hyper-V PV " Paolo Bonzini
@ 2021-03-02 18:56 ` Sean Christopherson
0 siblings, 0 replies; 23+ messages in thread
From: Sean Christopherson @ 2021-03-02 18:56 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel
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.
^ permalink raw reply [flat|nested] 23+ messages in thread