Re: [PATCH] KVM: nVMX: Only update last_vpid on a successful nested VM-Enter

[Yosry Ahmed]

On Wed, Jul 22, 2026 at 2:41 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Fri, Jul 17, 2026, Yosry Ahmed wrote:
> > ---
> >  arch/x86/kvm/vmx/nested.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> > index b5460de4b1a72..0a4ea410b0483 100644
> > --- a/arch/x86/kvm/vmx/nested.c
> > +++ b/arch/x86/kvm/vmx/nested.c
> > @@ -2818,8 +2818,6 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
> >       if (kvm_caps.has_tsc_control)
> >               vmcs_write64(TSC_MULTIPLIER, vcpu->arch.tsc_scaling_ratio);
> >
> > -     nested_vmx_transition_tlb_flush(vcpu, vmcs12, true);
> > -
> >       if (nested_cpu_has_ept(vmcs12))
> >               nested_ept_init_mmu_context(vcpu);
> >
> > @@ -3739,6 +3737,8 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
> >               vmx_start_preemption_timer(vcpu, timer_value);
> >       }
> >
> > +     nested_vmx_transition_tlb_flush(vcpu, vmcs12, true);
>
> Hmm, so the SDM doesn't explicitly say _when_ TLB flushes happen, but I suspect
> this doesn't match how hardware behaves.  My guess is that any TLB flushes happen
> once VM-Enter has gotten past the VM-Fail consistency checks, i.e. once a VM-Exit
> is guaranteed.
>
> The SDM doesn't actually say anything about VM-Enter, so I don't think KVM *must*
> implement *that* specific behavior, but I do think we should leave the call to
> nested_vmx_transition_tlb_flush() where it's at, and instead service the local
> pending flushes in the pseudo-VM-Exit path.

Well, technically, changing the VPID is not a TLB flush. KVM has to
flush the TLB because under the hood it's using the same VPID, but
from L1's perspective, it's using a new VPID so any TLB entries
associated with the old VPID should not be used (for that VM entry). I
guess you're referring to other architectural flushes, like a VM entry
with VPID disabled.

The reason why I moved the call to nested_vmx_transition_tlb_flush()
is that it only makes sense (semantically) to update last_vpid when we
will actually use the VPID. Otherwise, if a VM entry fails, the CPU
couldn't have cached any translations associated with the new VPID,
and a flush is not needed if the VPID is changed again.

IOW, the choice was purely based on semantics and code readability.

> Because the other way the TLB flushes
> can be queued during VM-Enter is via the MSR load lists:
>
>   If any MSR is being loaded in such a way that would architecturally require
>   a TLB flush, the TLBs are updated so that, after VM entry, the logical
>   processor will not use any translations that were cached before the transition.
>
> E.g. if L1 successfully loads one or more MTRRs on VM-Enter to L2[*], then fails
> on a subsequent MSR, architecturally I believe L2 TLB entries are guaranteed to
> be flushed.

Hmm that is an interesting case. I guess the right thing to do here
depends on hardware, but yeah I think it makes sense in this case to
service local flushes in the failure path. It still annoys me that we
would update last_vpid even on failed nested VM entries, so part of me
still wants to move the call to nested_vmx_transition_tlb_flush() just
for that, but that may not make sense for the VPID disabled case.

I guess the "right" thing to do is break the last_vpid tracking out of
nested_vmx_transition_tlb_flush() and only update last_vpid on a
successful VM entry, but it's probably not worth it in terms of code
readability.

> I'm speculatingly heavily on all of this, but even if I'm wrong (or it's uarch-
> specific behavior), explicitly servicing pending flushes in the VM-Exit(ish) path
> feels safe in the long run:

All that being said, yeah I am convinced it's probably better to do
the below. I will send a v2 after testing it with my repro.

>
> diff --git arch/x86/kvm/vmx/nested.c arch/x86/kvm/vmx/nested.c
> index 3266c63046ee..49aeecc2f093 100644
> --- arch/x86/kvm/vmx/nested.c
> +++ arch/x86/kvm/vmx/nested.c
> @@ -3760,6 +3760,14 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
>  vmentry_fail_vmexit_guest_mode:
>         if (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETTING)
>                 vcpu->arch.tsc_offset -= vmcs12->tsc_offset;
> +
> +       /*
> +        * Handle any TLB flush requests that were queued for L2 if KVM made it
> +        * far enough along to switch to L2 context.  Note, loading host state
> +        * will generate any flushes for L1 required by VM-Exit.
> +        */
> +       kvm_service_local_tlb_flush_requests(vcpu);
> +
>         leave_guest_mode(vcpu);
>
>  vmentry_fail_vmexit:
>
>
> [*] https://lore.kernel.org/all/20260717230542.3555587-4-jmattson@xxxxxxxxxx