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

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

[Yosry Ahmed]

Delay calling nested_vmx_transition_tlb_flush() in the nested VM-Enter
path until all the checks are completed, and performing any requested
TLB flushes for L2 is guaranteed (i.e.
kvm_service_local_tlb_flush_requests() is called in L2's context),
either before L2 is run in vcpu_enter_guest() or as part of a "full"
nested VM-Exit (i.e.  through __nested_vmx_vmexit()).

nested_vmx_transition_tlb_flush() checks if L1 changed L2's VPID in
vmcs12 (among other things), updates last_vpid accordingly, and requests
a TLB flush (through KVM_REQ_TLB_FLUSH_GUEST). With the current code
path, it is possible for the nested VM-Enter to fail after
nested_vmx_transition_tlb_flush() already updates last_vpid, but in this
case KVM will *not* call kvm_service_local_tlb_flush_requests() in L2's
context, and flushing L2's VPID will missed. If L1 later runs L2 with
the same VPID, nested_vmx_transition_tlb_flush() won't detect a change
in VPID, and L2's VPID will not be flushed.

Fixes: 5c614b3583e7 ("KVM: nVMX: nested VPID emulation")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org> # Internal review
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---

Note: I was able to reproduce the bug by hacking a nested TLB flushes
selftest I am working on as part of nSVM TLB optimizations. I haven't
sent out a new version that includes the selftest yet, but basically the
test includes one test case where L1 updates a mapping and changes L2's
VPID to effectively flush the TLB. Inserting a failed VM-Enter before
the correct VM-Enter makes that test case fail. This patch fixes it.

This is probably too vague, so I will reply to this patch when I send
out the selftest with exact diff needed to repro. This is me just
pointing out that a repro exists, and that I will add a pointer to it
later.


---
 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);
+
 	/*
 	 * Note no nested_vmx_succeed or nested_vmx_fail here. At this point
 	 * we are no longer running L1, and VMLAUNCH/VMRESUME has not yet

base-commit: 6bc96b971766fbbbbdd9fb2642cedacaf02da957
-- 
2.55.0.229.g6434b31f56-goog

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

[Sean Christopherson]

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.  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.

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:

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@google.com

> +
>  	/*
>  	 * Note no nested_vmx_succeed or nested_vmx_fail here. At this point
>  	 * we are no longer running L1, and VMLAUNCH/VMRESUME has not yet
> 
> base-commit: 6bc96b971766fbbbbdd9fb2642cedacaf02da957
> -- 
> 2.55.0.229.g6434b31f56-goog
> 

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@google.com> 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@google.com