* [PATCH 0/2] KVM: nVMX: Fix VPID + !EPT TLB bugs
@ 2021-11-25 1:49 Sean Christopherson
2021-11-25 1:49 ` [PATCH 1/2] KVM: nVMX: Flush current VPID (L1 vs. L2) for KVM_REQ_TLB_FLUSH_GUEST Sean Christopherson
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Sean Christopherson @ 2021-11-25 1:49 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel, Lai Jiangshan
Fix two bugs reported by Lai where KVM mishandles guest-scoped TLB flushes
when L2 is active. Bugs confirmed (and confirmed fixed) by the VPID+access
test (patches posted for kvm-unit-tests).
Sean Christopherson (2):
KVM: nVMX: Flush current VPID (L1 vs. L2) for KVM_REQ_TLB_FLUSH_GUEST
KVM: nVMX: Emulate guest TLB flush on nested VM-Enter with new vpid12
arch/x86/kvm/vmx/nested.c | 45 +++++++++++++++++----------------------
arch/x86/kvm/vmx/vmx.c | 23 ++++++++++++--------
arch/x86/kvm/x86.c | 28 ++++++++++++++++++++----
arch/x86/kvm/x86.h | 7 +-----
4 files changed, 59 insertions(+), 44 deletions(-)
--
2.34.0.rc2.393.gf8c9666880-goog
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] KVM: nVMX: Flush current VPID (L1 vs. L2) for KVM_REQ_TLB_FLUSH_GUEST 2021-11-25 1:49 [PATCH 0/2] KVM: nVMX: Fix VPID + !EPT TLB bugs Sean Christopherson @ 2021-11-25 1:49 ` Sean Christopherson 2021-11-25 3:50 ` Lai Jiangshan 2021-11-25 1:49 ` [PATCH 2/2] KVM: nVMX: Emulate guest TLB flush on nested VM-Enter with new vpid12 Sean Christopherson 2021-11-26 12:11 ` [PATCH 0/2] KVM: nVMX: Fix VPID + !EPT TLB bugs Paolo Bonzini 2 siblings, 1 reply; 7+ messages in thread From: Sean Christopherson @ 2021-11-25 1:49 UTC (permalink / raw) To: Paolo Bonzini Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel, Lai Jiangshan Flush the current VPID when handling KVM_REQ_TLB_FLUSH_GUEST instead of always flushing vpid01. Any TLB flush that is triggered when L2 is active is scoped to L2's VPID (if it has one), e.g. if L2 toggles CR4.PGE and L1 doesn't intercept PGE writes, then KVM's emulation of the TLB flush needs to be applied to L2's VPID. Like KVM_REQ_TLB_FLUSH_CURRENT, the GUEST variant needs to be serviced at nested transitions, as KVM doesn't track requests for L1 vs L2. E.g. if there's a pending flush when a nested VM-Exit occurs, then the flush was requested in the context of L2 and needs to be handled before switching to L1, otherwise the flush for L2 would effectiely be lost. Opportunistically add a helper to handle CURRENT and GUEST as a pair, the logic for when they need to be serviced is identical as both requests are tied to L1 vs. L2, the only difference is the scope of the flush. Reported-by: Lai Jiangshan <jiangshanlai+lkml@gmail.com> Fixes: 07ffaf343e34 ("KVM: nVMX: Sync all PGDs on nested transition with shadow paging") Cc: stable@vger.kernel.org Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/vmx/nested.c | 8 +++----- arch/x86/kvm/vmx/vmx.c | 23 ++++++++++++++--------- arch/x86/kvm/x86.c | 28 ++++++++++++++++++++++++---- arch/x86/kvm/x86.h | 7 +------ 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 2d9565b37fe0..2ef1d5562a54 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -3344,8 +3344,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, }; u32 failed_index; - if (kvm_check_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu)) - kvm_vcpu_flush_tlb_current(vcpu); + kvm_service_local_tlb_flush_requests(vcpu); evaluate_pending_interrupts = exec_controls_get(vmx) & (CPU_BASED_INTR_WINDOW_EXITING | CPU_BASED_NMI_WINDOW_EXITING); @@ -4502,9 +4501,8 @@ void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason, (void)nested_get_evmcs_page(vcpu); } - /* Service the TLB flush request for L2 before switching to L1. */ - if (kvm_check_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu)) - kvm_vcpu_flush_tlb_current(vcpu); + /* Service pending TLB flush requests for L2 before switching to L1. */ + kvm_service_local_tlb_flush_requests(vcpu); /* * VCPU_EXREG_PDPTR will be clobbered in arch/x86/kvm/vmx/vmx.h between diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 3127c66a1651..226b06f1ddd1 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -2918,6 +2918,13 @@ static void vmx_flush_tlb_all(struct kvm_vcpu *vcpu) } } +static inline int vmx_get_current_vpid(struct kvm_vcpu *vcpu) +{ + if (is_guest_mode(vcpu)) + return nested_get_vpid02(vcpu); + return to_vmx(vcpu)->vpid; +} + static void vmx_flush_tlb_current(struct kvm_vcpu *vcpu) { struct kvm_mmu *mmu = vcpu->arch.mmu; @@ -2930,31 +2937,29 @@ static void vmx_flush_tlb_current(struct kvm_vcpu *vcpu) if (enable_ept) ept_sync_context(construct_eptp(vcpu, root_hpa, mmu->shadow_root_level)); - else if (!is_guest_mode(vcpu)) - vpid_sync_context(to_vmx(vcpu)->vpid); else - vpid_sync_context(nested_get_vpid02(vcpu)); + vpid_sync_context(vmx_get_current_vpid(vcpu)); } static void vmx_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t addr) { /* - * vpid_sync_vcpu_addr() is a nop if vmx->vpid==0, see the comment in + * vpid_sync_vcpu_addr() is a nop if vpid==0, see the comment in * vmx_flush_tlb_guest() for an explanation of why this is ok. */ - vpid_sync_vcpu_addr(to_vmx(vcpu)->vpid, addr); + vpid_sync_vcpu_addr(vmx_get_current_vpid(vcpu), addr); } static void vmx_flush_tlb_guest(struct kvm_vcpu *vcpu) { /* - * vpid_sync_context() is a nop if vmx->vpid==0, e.g. if enable_vpid==0 - * or a vpid couldn't be allocated for this vCPU. VM-Enter and VM-Exit - * are required to flush GVA->{G,H}PA mappings from the TLB if vpid is + * vpid_sync_context() is a nop if vpid==0, e.g. if enable_vpid==0 or a + * vpid couldn't be allocated for this vCPU. VM-Enter and VM-Exit are + * required to flush GVA->{G,H}PA mappings from the TLB if vpid is * disabled (VM-Enter with vpid enabled and vpid==0 is disallowed), * i.e. no explicit INVVPID is necessary. */ - vpid_sync_context(to_vmx(vcpu)->vpid); + vpid_sync_context(vmx_get_current_vpid(vcpu)); } void vmx_ept_load_pdptrs(struct kvm_vcpu *vcpu) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 04e8dabc187d..9b9b27ef3655 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3258,6 +3258,29 @@ static void kvm_vcpu_flush_tlb_guest(struct kvm_vcpu *vcpu) static_call(kvm_x86_tlb_flush_guest)(vcpu); } + +static inline void kvm_vcpu_flush_tlb_current(struct kvm_vcpu *vcpu) +{ + ++vcpu->stat.tlb_flush; + static_call(kvm_x86_tlb_flush_current)(vcpu); +} + +/* + * Service "local" TLB flush requests, which are specific to the current MMU + * context. In addition to the generic event handling in vcpu_enter_guest(), + * TLB flushes that are targeted at an MMU context also need to be serviced + * prior before nested VM-Enter/VM-Exit. + */ +void kvm_service_local_tlb_flush_requests(struct kvm_vcpu *vcpu) +{ + if (kvm_check_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu)) + kvm_vcpu_flush_tlb_current(vcpu); + + if (kvm_check_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu)) + kvm_vcpu_flush_tlb_guest(vcpu); +} +EXPORT_SYMBOL_GPL(kvm_service_local_tlb_flush_requests); + static void record_steal_time(struct kvm_vcpu *vcpu) { struct gfn_to_hva_cache *ghc = &vcpu->arch.st.cache; @@ -9653,10 +9676,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) /* Flushing all ASIDs flushes the current ASID... */ kvm_clear_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu); } - if (kvm_check_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu)) - kvm_vcpu_flush_tlb_current(vcpu); - if (kvm_check_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu)) - kvm_vcpu_flush_tlb_guest(vcpu); + kvm_service_local_tlb_flush_requests(vcpu); if (kvm_check_request(KVM_REQ_REPORT_TPR_ACCESS, vcpu)) { vcpu->run->exit_reason = KVM_EXIT_TPR_ACCESS; diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index 997669ae9caa..4abcd8d9836d 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -103,6 +103,7 @@ static inline unsigned int __shrink_ple_window(unsigned int val, #define MSR_IA32_CR_PAT_DEFAULT 0x0007040600070406ULL +void kvm_service_local_tlb_flush_requests(struct kvm_vcpu *vcpu); int kvm_check_nested_events(struct kvm_vcpu *vcpu); static inline void kvm_clear_exception_queue(struct kvm_vcpu *vcpu) @@ -185,12 +186,6 @@ static inline bool mmu_is_nested(struct kvm_vcpu *vcpu) return vcpu->arch.walk_mmu == &vcpu->arch.nested_mmu; } -static inline void kvm_vcpu_flush_tlb_current(struct kvm_vcpu *vcpu) -{ - ++vcpu->stat.tlb_flush; - static_call(kvm_x86_tlb_flush_current)(vcpu); -} - static inline int is_pae(struct kvm_vcpu *vcpu) { return kvm_read_cr4_bits(vcpu, X86_CR4_PAE); -- 2.34.0.rc2.393.gf8c9666880-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] KVM: nVMX: Flush current VPID (L1 vs. L2) for KVM_REQ_TLB_FLUSH_GUEST 2021-11-25 1:49 ` [PATCH 1/2] KVM: nVMX: Flush current VPID (L1 vs. L2) for KVM_REQ_TLB_FLUSH_GUEST Sean Christopherson @ 2021-11-25 3:50 ` Lai Jiangshan 0 siblings, 0 replies; 7+ messages in thread From: Lai Jiangshan @ 2021-11-25 3:50 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, LKML On Thu, Nov 25, 2021 at 9:49 AM Sean Christopherson <seanjc@google.com> wrote: > Reported-by: Lai Jiangshan <jiangshanlai+lkml@gmail.com> Oh, I just noticed that I made an incorrect configuration in Gmail. I hope it be Reported-by: Lai Jiangshan <jiangshanlai@gmail.com> when it comes to "Reviewed-by", "Reported-by" as my email address in the file MAINTAINERS. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] KVM: nVMX: Emulate guest TLB flush on nested VM-Enter with new vpid12 2021-11-25 1:49 [PATCH 0/2] KVM: nVMX: Fix VPID + !EPT TLB bugs Sean Christopherson 2021-11-25 1:49 ` [PATCH 1/2] KVM: nVMX: Flush current VPID (L1 vs. L2) for KVM_REQ_TLB_FLUSH_GUEST Sean Christopherson @ 2021-11-25 1:49 ` Sean Christopherson 2021-11-25 3:50 ` Lai Jiangshan 2021-11-26 12:11 ` [PATCH 0/2] KVM: nVMX: Fix VPID + !EPT TLB bugs Paolo Bonzini 2 siblings, 1 reply; 7+ messages in thread From: Sean Christopherson @ 2021-11-25 1:49 UTC (permalink / raw) To: Paolo Bonzini Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel, Lai Jiangshan Fully emulate a guest TLB flush on nested VM-Enter which changes vpid12, i.e. L2's VPID, instead of simply doing INVVPID to flush real hardware's TLB entries for vpid02. From L1's perspective, changing L2's VPID is effectively a TLB flush unless "hardware" has previously cached entries for the new vpid12. Because KVM tracks only a single vpid12, KVM doesn't know if the new vpid12 has been used in the past and so must treat it as a brand new, never been used VPID, i.e. must assume that the new vpid12 represents a TLB flush from L1's perspective. For example, if L1 and L2 share a CR3, the first VM-Enter to L2 (with a VPID) is effectively a TLB flush as hardware/KVM has never seen vpid12 and thus can't have cached entries in the TLB for vpid12. Reported-by: Lai Jiangshan <jiangshanlai+lkml@gmail.com> Fixes: 5c614b3583e7 ("KVM: nVMX: nested VPID emulation") Cc: stable@vger.kernel.org Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/vmx/nested.c | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 2ef1d5562a54..dafe5881ae51 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -1162,29 +1162,26 @@ static void nested_vmx_transition_tlb_flush(struct kvm_vcpu *vcpu, WARN_ON(!enable_vpid); /* - * If VPID is enabled and used by vmc12, but L2 does not have a unique - * TLB tag (ASID), i.e. EPT is disabled and KVM was unable to allocate - * a VPID for L2, flush the current context as the effective ASID is - * common to both L1 and L2. - * - * Defer the flush so that it runs after vmcs02.EPTP has been set by - * KVM_REQ_LOAD_MMU_PGD (if nested EPT is enabled) and to avoid - * redundant flushes further down the nested pipeline. - * - * If a TLB flush isn't required due to any of the above, and vpid12 is - * changing then the new "virtual" VPID (vpid12) will reuse the same - * "real" VPID (vpid02), and so needs to be flushed. There's no direct - * mapping between vpid02 and vpid12, vpid02 is per-vCPU and reused for - * all nested vCPUs. Remember, a flush on VM-Enter does not invalidate - * guest-physical mappings, so there is no need to sync the nEPT MMU. + * VPID is enabled and in use by vmcs12. If vpid12 is changing, then + * emulate a guest TLB flush as KVM does not track vpid12 history nor + * is the VPID incorporated into the MMU context. I.e. KVM must assume + * that the new vpid12 has never been used and thus represents a new + * guest ASID that cannot have entries in the TLB. */ - if (!nested_has_guest_tlb_tag(vcpu)) { - kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu); - } else if (is_vmenter && - vmcs12->virtual_processor_id != vmx->nested.last_vpid) { + if (is_vmenter && vmcs12->virtual_processor_id != vmx->nested.last_vpid) { vmx->nested.last_vpid = vmcs12->virtual_processor_id; - vpid_sync_context(nested_get_vpid02(vcpu)); + kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu); + return; } + + /* + * If VPID is enabled, used by vmc12, and vpid12 is not changing but + * does not have a unique TLB tag (ASID), i.e. EPT is disabled and + * KVM was unable to allocate a VPID for L2, flush the current context + * as the effective ASID is common to both L1 and L2. + */ + if (!nested_has_guest_tlb_tag(vcpu)) + kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu); } static bool is_bitwise_subset(u64 superset, u64 subset, u64 mask) -- 2.34.0.rc2.393.gf8c9666880-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] KVM: nVMX: Emulate guest TLB flush on nested VM-Enter with new vpid12 2021-11-25 1:49 ` [PATCH 2/2] KVM: nVMX: Emulate guest TLB flush on nested VM-Enter with new vpid12 Sean Christopherson @ 2021-11-25 3:50 ` Lai Jiangshan 2021-11-29 19:26 ` Sean Christopherson 0 siblings, 1 reply; 7+ messages in thread From: Lai Jiangshan @ 2021-11-25 3:50 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, LKML On Thu, Nov 25, 2021 at 9:49 AM Sean Christopherson <seanjc@google.com> wrote: > > Fully emulate a guest TLB flush on nested VM-Enter which changes vpid12, > i.e. L2's VPID, instead of simply doing INVVPID to flush real hardware's > TLB entries for vpid02. From L1's perspective, changing L2's VPID is > effectively a TLB flush unless "hardware" has previously cached entries > for the new vpid12. Because KVM tracks only a single vpid12, KVM doesn't > know if the new vpid12 has been used in the past and so must treat it as > a brand new, never been used VPID, i.e. must assume that the new vpid12 > represents a TLB flush from L1's perspective. > > For example, if L1 and L2 share a CR3, the first VM-Enter to L2 (with a > VPID) is effectively a TLB flush as hardware/KVM has never seen vpid12 > and thus can't have cached entries in the TLB for vpid12. > > Reported-by: Lai Jiangshan <jiangshanlai+lkml@gmail.com> > Fixes: 5c614b3583e7 ("KVM: nVMX: nested VPID emulation") > Cc: stable@vger.kernel.org > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/vmx/nested.c | 37 +++++++++++++++++-------------------- > 1 file changed, 17 insertions(+), 20 deletions(-) > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index 2ef1d5562a54..dafe5881ae51 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -1162,29 +1162,26 @@ static void nested_vmx_transition_tlb_flush(struct kvm_vcpu *vcpu, > WARN_ON(!enable_vpid); > > /* > - * If VPID is enabled and used by vmc12, but L2 does not have a unique > - * TLB tag (ASID), i.e. EPT is disabled and KVM was unable to allocate > - * a VPID for L2, flush the current context as the effective ASID is > - * common to both L1 and L2. > - * > - * Defer the flush so that it runs after vmcs02.EPTP has been set by > - * KVM_REQ_LOAD_MMU_PGD (if nested EPT is enabled) and to avoid > - * redundant flushes further down the nested pipeline. > - * > - * If a TLB flush isn't required due to any of the above, and vpid12 is > - * changing then the new "virtual" VPID (vpid12) will reuse the same > - * "real" VPID (vpid02), and so needs to be flushed. There's no direct > - * mapping between vpid02 and vpid12, vpid02 is per-vCPU and reused for > - * all nested vCPUs. Remember, a flush on VM-Enter does not invalidate > - * guest-physical mappings, so there is no need to sync the nEPT MMU. > + * VPID is enabled and in use by vmcs12. If vpid12 is changing, then > + * emulate a guest TLB flush as KVM does not track vpid12 history nor > + * is the VPID incorporated into the MMU context. I.e. KVM must assume > + * that the new vpid12 has never been used and thus represents a new > + * guest ASID that cannot have entries in the TLB. > */ > - if (!nested_has_guest_tlb_tag(vcpu)) { > - kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu); > - } else if (is_vmenter && > - vmcs12->virtual_processor_id != vmx->nested.last_vpid) { > + if (is_vmenter && vmcs12->virtual_processor_id != vmx->nested.last_vpid) { > vmx->nested.last_vpid = vmcs12->virtual_processor_id; How about when vmx->nested.last_vpid == vmcs12->virtual_processor_id == 0? I think KVM_REQ_TLB_FLUSH_GUEST is needed in this case too. > - vpid_sync_context(nested_get_vpid02(vcpu)); > + kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu); > + return; > } > + > + /* > + * If VPID is enabled, used by vmc12, and vpid12 is not changing but > + * does not have a unique TLB tag (ASID), i.e. EPT is disabled and > + * KVM was unable to allocate a VPID for L2, flush the current context > + * as the effective ASID is common to both L1 and L2. > + */ > + if (!nested_has_guest_tlb_tag(vcpu)) > + kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu); > } > > static bool is_bitwise_subset(u64 superset, u64 subset, u64 mask) > -- > 2.34.0.rc2.393.gf8c9666880-goog > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] KVM: nVMX: Emulate guest TLB flush on nested VM-Enter with new vpid12 2021-11-25 3:50 ` Lai Jiangshan @ 2021-11-29 19:26 ` Sean Christopherson 0 siblings, 0 replies; 7+ messages in thread From: Sean Christopherson @ 2021-11-29 19:26 UTC (permalink / raw) To: Lai Jiangshan Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, LKML On Thu, Nov 25, 2021, Lai Jiangshan wrote: > On Thu, Nov 25, 2021 at 9:49 AM Sean Christopherson <seanjc@google.com> wrote: > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > > index 2ef1d5562a54..dafe5881ae51 100644 > > --- a/arch/x86/kvm/vmx/nested.c > > +++ b/arch/x86/kvm/vmx/nested.c > > @@ -1162,29 +1162,26 @@ static void nested_vmx_transition_tlb_flush(struct kvm_vcpu *vcpu, > > WARN_ON(!enable_vpid); > > > > /* > > - * If VPID is enabled and used by vmc12, but L2 does not have a unique > > - * TLB tag (ASID), i.e. EPT is disabled and KVM was unable to allocate > > - * a VPID for L2, flush the current context as the effective ASID is > > - * common to both L1 and L2. > > - * > > - * Defer the flush so that it runs after vmcs02.EPTP has been set by > > - * KVM_REQ_LOAD_MMU_PGD (if nested EPT is enabled) and to avoid > > - * redundant flushes further down the nested pipeline. > > - * > > - * If a TLB flush isn't required due to any of the above, and vpid12 is > > - * changing then the new "virtual" VPID (vpid12) will reuse the same > > - * "real" VPID (vpid02), and so needs to be flushed. There's no direct > > - * mapping between vpid02 and vpid12, vpid02 is per-vCPU and reused for > > - * all nested vCPUs. Remember, a flush on VM-Enter does not invalidate > > - * guest-physical mappings, so there is no need to sync the nEPT MMU. > > + * VPID is enabled and in use by vmcs12. If vpid12 is changing, then > > + * emulate a guest TLB flush as KVM does not track vpid12 history nor > > + * is the VPID incorporated into the MMU context. I.e. KVM must assume > > + * that the new vpid12 has never been used and thus represents a new > > + * guest ASID that cannot have entries in the TLB. > > */ > > - if (!nested_has_guest_tlb_tag(vcpu)) { > > - kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu); > > - } else if (is_vmenter && > > - vmcs12->virtual_processor_id != vmx->nested.last_vpid) { > > + if (is_vmenter && vmcs12->virtual_processor_id != vmx->nested.last_vpid) { > > vmx->nested.last_vpid = vmcs12->virtual_processor_id; > > How about when vmx->nested.last_vpid == vmcs12->virtual_processor_id == 0? > > I think KVM_REQ_TLB_FLUSH_GUEST is needed in this case too. That's handled by code that's just out of sight in this diff. The first check in nested_vmx_transition_tlb_flush() is to see if vmcs12 has VPID enabled. If the code in this patch is reached, vmcs12->virtual_processor_id is guaranteed to be non-zero as VM-Enter fails if VPID is enabled but VPID==0. static void nested_vmx_transition_tlb_flush(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, bool is_vmenter) { struct vcpu_vmx *vmx = to_vmx(vcpu); /* * If vmcs12 doesn't use VPID, L1 expects linear and combined mappings * for *all* contexts to be flushed on VM-Enter/VM-Exit, i.e. it's a * full TLB flush from the guest's perspective. This is required even * if VPID is disabled in the host as KVM may need to synchronize the * MMU in response to the guest TLB flush. * * Note, using TLB_FLUSH_GUEST is correct even if nested EPT is in use. * EPT is a special snowflake, as guest-physical mappings aren't * flushed on VPID invalidations, including VM-Enter or VM-Exit with * VPID disabled. As a result, KVM _never_ needs to sync nEPT * entries on VM-Enter because L1 can't rely on VM-Enter to flush * those mappings. */ if (!nested_cpu_has_vpid(vmcs12)) { kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu); return; } ... } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] KVM: nVMX: Fix VPID + !EPT TLB bugs 2021-11-25 1:49 [PATCH 0/2] KVM: nVMX: Fix VPID + !EPT TLB bugs Sean Christopherson 2021-11-25 1:49 ` [PATCH 1/2] KVM: nVMX: Flush current VPID (L1 vs. L2) for KVM_REQ_TLB_FLUSH_GUEST Sean Christopherson 2021-11-25 1:49 ` [PATCH 2/2] KVM: nVMX: Emulate guest TLB flush on nested VM-Enter with new vpid12 Sean Christopherson @ 2021-11-26 12:11 ` Paolo Bonzini 2 siblings, 0 replies; 7+ messages in thread From: Paolo Bonzini @ 2021-11-26 12:11 UTC (permalink / raw) To: Sean Christopherson Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel, Lai Jiangshan On 11/25/21 02:49, Sean Christopherson wrote: > Fix two bugs reported by Lai where KVM mishandles guest-scoped TLB flushes > when L2 is active. Bugs confirmed (and confirmed fixed) by the VPID+access > test (patches posted for kvm-unit-tests). > > Sean Christopherson (2): > KVM: nVMX: Flush current VPID (L1 vs. L2) for KVM_REQ_TLB_FLUSH_GUEST > KVM: nVMX: Emulate guest TLB flush on nested VM-Enter with new vpid12 > > arch/x86/kvm/vmx/nested.c | 45 +++++++++++++++++---------------------- > arch/x86/kvm/vmx/vmx.c | 23 ++++++++++++-------- > arch/x86/kvm/x86.c | 28 ++++++++++++++++++++---- > arch/x86/kvm/x86.h | 7 +----- > 4 files changed, 59 insertions(+), 44 deletions(-) > Queued, thanks (but I split the first in two). Paolo ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-11-29 22:48 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-25 1:49 [PATCH 0/2] KVM: nVMX: Fix VPID + !EPT TLB bugs Sean Christopherson 2021-11-25 1:49 ` [PATCH 1/2] KVM: nVMX: Flush current VPID (L1 vs. L2) for KVM_REQ_TLB_FLUSH_GUEST Sean Christopherson 2021-11-25 3:50 ` Lai Jiangshan 2021-11-25 1:49 ` [PATCH 2/2] KVM: nVMX: Emulate guest TLB flush on nested VM-Enter with new vpid12 Sean Christopherson 2021-11-25 3:50 ` Lai Jiangshan 2021-11-29 19:26 ` Sean Christopherson 2021-11-26 12:11 ` [PATCH 0/2] KVM: nVMX: Fix VPID + !EPT TLB bugs Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).