Messages in this thread | | | Subject | Re: [PATCH] KVM: X86: fix tlb_flush_guest() | From | Lai Jiangshan <> | Date | Fri, 28 May 2021 09:13:08 +0800 |
| |
On 2021/5/28 03:28, Sean Christopherson wrote: > On Thu, May 27, 2021, Sean Christopherson wrote: >>> KVM_REQ_MMU_RELOAD is overkill, nuking the shadow page tables will completely >>> offset the performance gains of the paravirtualized flush. > > Argh, I take that back. The PV KVM_VCPU_FLUSH_TLB flag doesn't distinguish > between flushing a specific mm and flushing the entire TLB. The HyperV usage > (via KVM_REQ) also throws everything into a single bucket. A full RELOAD still > isn't necessary as KVM just needs to sync all roots, not blast them away. For > previous roots, KVM doesn't have a mechanism to defer the sync, so the immediate > fix will need to unload those roots. > > And looking at KVM's other flows, __kvm_mmu_new_pgd() and kvm_set_cr3() are also > broken with respect to previous roots. E.g. if the guest does a MOV CR3 that > flushes the entire TLB, followed by a MOV CR3 with PCID_NOFLUSH=1, KVM will fail > to sync the MMU on the second flush even though the guest can technically rely > on the first MOV CR3 to have synchronized any previous changes relative to the > fisrt MOV CR3.
Could you elaborate the problem please? When can a MOV CR3 that needs to flush the entire TLB if PCID is enabled?
If CR4.PCIDE = 1 and bit 63 of the instruction’s source operand is 0, the instruction invalidates all TLB entries associated with the PCID specified in bits 11:0 of the instruction’s source operand except those for global pages. It also invalidates all entries in all paging-structure caches associated with that PCID. It is not required to invalidate entries in the TLBs and paging-structure caches that are associated with other PCIDs.
> > Lai, if it's ok with you, I'll massage this patch as discussed and fold it into > a larger series to fix the other bugs and do additional cleanup/improvements. > >>> I believe the minimal fix is: >>> >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>> index 81ab3b8f22e5..b0072063f9bf 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -3072,6 +3072,9 @@ static void kvm_vcpu_flush_tlb_all(struct kvm_vcpu *vcpu) >>> static void kvm_vcpu_flush_tlb_guest(struct kvm_vcpu *vcpu) >>> { >>> ++vcpu->stat.tlb_flush; >>> + >>> + if (!tdp_enabled) >>> + kvm_mmu_sync_roots(vcpu); >>> static_call(kvm_x86_tlb_flush_guest)(vcpu); >>> }
| |