-me :-)

On Thu, Oct 28, 2021, Lai Jiangshan wrote:

On Sat, Mar 21, 2020 at 5:29 AM Sean Christopherson
[off-list ref] wrote:

quoted

+       if (!nested_cpu_has_vpid(vmcs12) || !nested_has_guest_tlb_tag(vcpu)) {
+               kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
+       } else 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));
+       }
+}

(I'm sorry to pick this old email to reply to, but the problem has
nothing to do with this patch nor 5c614b3583e7 and it exists since
nested vmx is introduced.)

I think kvm_mmu_free_guest_mode_roots() should be called
if (!enable_ept && vmcs12->virtual_processor_id != vmx->nested.last_vpid)
just because prev_roots doesn't cache the vpid12.
(prev_roots caches PCID, which is distinctive)

The problem hardly exists if L1's hypervisor is also kvm, but if L1's
hypervisor is different or is also kvm with some changes in the way how it
manages VPID.

Indeed.  A more straightforward error case would be if L1 and L2 share CR3, and
vmcs02.VPID is toggled (or used for the first time) on the L1 => L2 VM-Enter.

The fix should simply be:

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index eedcebf58004..574823370e7a 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c

@@ -1202,17 +1202,15 @@ static void nested_vmx_transition_tlb_flush(struct kvm_vcpu *vcpu,
         *
         * 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.
+        * "real" VPID (vpid02), and so needs to be flushed.  Like the !vpid02
+        * case above, this is a full TLB flush from the guest's perspective.
         */
        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) {
                vmx->nested.last_vpid = vmcs12->virtual_processor_id;
-               vpid_sync_context(nested_get_vpid02(vcpu));
+               kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu);
        }
 }