Re: [PATCH 1/3] KVM: nVMX: Always flush vpid02 on first use

[Huang, Kai]

On Wed, 2026-06-17 at 15:04 -0700, Yosry Ahmed wrote:
> On Wed, Jun 17, 2026 at 3:03 PM Huang, Kai <kai.huang@xxxxxxxxx> wrote:
> > 
> > On Wed, 2026-06-17 at 06:03 -0700, Sean Christopherson wrote:
> > > On Wed, Jun 17, 2026, Kai Huang wrote:
> > > > On Tue, 2026-06-16 at 21:46 +0000, Yosry Ahmed wrote:
> > > > > Make sure vpid02 is always flushed on first use by setting last_vpid=0
> > > > > when allocating vpid02.  nested_vmx_transition_tlb_flush() will always
> > > > > detect a VPID change on first VM-Enter after VMXON, because VPID=0 in
> > > > > vmcb12 is not allowed if L1 enables VPID.
> > > > 
> > > > vmcs12 :-)
> > > > 
> > > > > 
> > > > > This avoids using stale TLB entries from a previous lifetime of the
> > > > > VPID, that might have been associated with a different vCPU (or a
> > > > > completely different VM).
> > > > > 
> > > > > Note that last_vpid is already being initialized as 0 when the vCPU is
> > > > > created, but it is not reset when vpid02 is freed on VMXOFF. Hence, the
> > > > > problem can only occur if L1 does VMXOFF -> VMXON, runs an L2, and KVM
> > > > > happens to reuse a VPID that has TLB entries on the physical CPU.
> > > > 
> > > > Not sure whether it's better to set it to 0 in free_nested(), which also resets
> > > > some other nested fields to clean slate AFAICT?
> > > 
> > > It needs to be set on first use, for the same reason that kvm_mmu_load() flushes
> > > the root:
> > > 
> > >       /*
> > >        * Flush any TLB entries for the new root, the provenance of the root
> > >        * is unknown.  Even if KVM ensures there are no stale TLB entries
> > >        * for a freed root, in theory another hypervisor could have left
> > >        * stale entries.  Flushing on alloc also allows KVM to skip the TLB
> > >        * flush when freeing a root (see kvm_tdp_mmu_put_root()).
> > >        */
> > >       kvm_x86_call(flush_tlb_current)(vcpu);
> > 
> > I think you mean the "actual flush" needs to be done on the first use.  But
> > setting last_vpid to 0 is a setting which is to make sure the actual flush will
> > always be done on the first use, i.e., the actual flush will always be done on
> > the first use.  For this purpose seems to me there's no difference between
> > setting last_vpid to 0 in enter_vmx_operation() and free_nested(), but maybe I
> > am missing something.
> > 
> > But I guess doing it in enter_vmx_operation() matches the logic of "doing actual
> > flush on first use" more :-)
> 
> Yup. I thought about putting it free_nested() as it looks like
> cleanup, but semantically it makes more sense to put it in
> enter_vmx_operation().

Sounds good to me.  :-)