Messages in this thread | | | Subject | Re: [PATCH v2 31/32] KVM: nVMX: Don't flush TLB on nested VM transition with EPT enabled | From | Paolo Bonzini <> | Date | Wed, 18 Mar 2020 18:38:56 +0100 |
| |
On 18/03/20 18:26, Sean Christopherson wrote: >>> >>> if (!nested_ept) >>> kvm_mmu_new_cr3(vcpu, cr3, enable_ept || >>> nested_cpu_has_vpid(vmcs12)); >> >> ... which is exactly nested_has_guest_tlb_tag(vcpu). Well, not exactly >> but it's a bug in your code above. :) > > I don't think it's a bug, it's intentionally different. When enable_ept=0, > nested_has_guest_tlb_tag() returns true if and only if L1 has enabled VPID > for L2 *and* L2 has been assigned a unique VPID by L0. > > For sync purposes, whether or not L2 has been assigned a unique VPID is > irrelevant. L0 needs to invalidate TLB entries to prevent resuing L1's > entries (assuming L1 has been assigned a VPID), but L0 doesn't need to sync > SPTEs because L2 doesn't expect them to be refreshed. ^^ L1
Yes you're right. So I would say keep your code, but we can simplify the comment. Something like:
/* * We can skip the TLB flush if we have EPT enabled (because...) and * also if L1 is using VPID, because then it doesn't expect SPTEs for L2 * to be refreshed. * * This is almost the same as nested_has_guest_tlb_tag(vcpu), but here * we don't care if L2 has been assigned a unique VPID; L1 doesn't know, * and will nevertheless do INVVPID to avoid reuse of stale page * table entries. */
Nevertheless it's scary in that this is a potential attack vector for reusing stale L0 SPTEs, so we should make sure it's all properly commented.
Thanks,
Paolo
>> It completely makes sense to use that as the third argument, and while a >> comment is still needed it will be much smaller. > Ya, agreed. >
| |