Re: [RFC PATCH v2 17/25] KVM: nSVM: Service local TLB flushes before nested transitions

[Sean Christopherson <seanjc@google.com>]

On Thu, Jul 23, 2026, Yosry Ahmed wrote:
> On Wed, Jul 22, 2026 at 05:46:31PM -0700, Sean Christopherson wrote:
> > On Tue, Jun 16, 2026, Yosry Ahmed wrote:
> > > On Tue, Jun 16, 2026 at 11:06 AM Yosry Ahmed <yosry@kernel.org> wrote:
> > > > > > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > > > > > index 234724d8b4c54..7b19191e0e43f 100644
> > > > > > --- a/arch/x86/kvm/svm/nested.c
> > > > > > +++ b/arch/x86/kvm/svm/nested.c
> > > > > > @@ -717,9 +717,16 @@ static void svm_switch_vmcb(struct vcpu_svm *svm, struct kvm_vmcb_info *target_v
> > > > > >  {
> > > > > >       struct kvm_vcpu *vcpu = &svm->vcpu;
> > > > > >
> > > > > > +     /*
> > > > > > +      * TLB flushes are applied to the VMCB, so apply any pending TLB flushes
> > > > > > +      * on the current VMCB before switching to a new one..
> > > > > > +      */
> > > > > > +     kvm_service_local_tlb_flush_requests(vcpu);
> > > > >
> > > > > [Severity: High]
> > > > > Does this clear global TLB flush requests (like KVM_REQ_TLB_FLUSH_GUEST)
> > > > > while applying them only to the outgoing VMCB?
> > > > >
> > > > > If a host-initiated flush occurs (e.g., from page reclaim), the request
> > > > > bit will be consumed and applied to the outgoing VMCB, leaving the incoming
> > > > > VMCB to miss the flush and run with a stale TLB.
> > > >
> > > > Yeah it is intentional to apply the flushes to the
> > > > current/old/outgoing context. If there are flushes needed in the new
> > > > context, they are queued by nested_svm_{entry/exit}_tlb_flush().
> > > > That's similar to what VMX does.
> > > >
> > > > >
> > > > > Also, is there a context mismatch here during nested VM-Exit?
> > > > >
> > > > > In nested_svm_vmexit(), leave_guest_mode(vcpu) is called before
> > > > > svm_switch_vmcb(svm, &svm->vmcb01).
> > > > >
> > > > > Because of this, kvm_service_local_tlb_flush_requests() will see
> > > > > is_guest_mode(vcpu) as false. If Hyper-V is enabled, this means
> > > > > kvm_hv_purge_tlb_flush_fifo() will incorrectly target L1's FIFO while the
> > > > > hardware flushes are actually being applied to L2's vmcb02.
> > > >
> > > > Ugh.. yes. This is annoying. kvm_service_local_tlb_flush_requests()
> > > > needs to be called on both the current/old/outgoing VMCB *and* guest
> > > > mode. So we'll need to open-code the call in a bunch of places before
> > > > svm_switch_vmcb() and {enter/leave}_guest_mode(). I really liked
> > > > putting it in svm_switch_vmcb() together with
> > > > nested_svm_{entry/exit}_tlb_flush() so that all the TLB flushing logic
> > > > for nested transitions live in one place and the ordering needs to be
> > > > handled in one place.
> > > 
> > > Maybe we can just re-order the code to always call svm_switch_vmcb()
> > > before {enter/leave}_guest_mode(). We already do that on the entry
> > > side, and seems to be straightforward on the exit side.
> > 
> > As stated earlier, I'd prefer to explicitly do flushing stuff where it fits from
> > an architectural perspective.
> 
> I did it this way because (as you also stated) it's more robust, and it
> also documents the ordering requirements:
> - We need to service local flushes before switching to the new VMCB.

No, that's not the requirement.  The requirement is that KVM faithlyfully emulates
the SVM architecture.  For KVM's implementation, that _mostly_ aligns with
switching between vmcb01 and vmcb02, but that's not a hard guarantee.  Unlike VMX,
SVM doesn't force KVM doesn't need to switch the active VMCB in order to make
changes to a VMCB, so nSVM may never end up with as many switches as nVMX that
don't need to trigger a TLB flush, but conceptually it's still inaccurate.

And regarding robustness, the flaw Sashiko pointed out regarding servicing pending
flushes after leave_guest_mode(vcpu) highlights that burying architectural behaviors
in what are effectively utility functions can be dangerous.  The counter-argument is
that we ended up with a similar bug in nVMX where KVM straight up forgot to service
the flushes, but my point is that handling this in svm_switch_vmcb() isn't a silver
bullet.  And I really don't like adding an arbitrary constraint that the "new" VMCB
needs to "match" the current L1 vs. L2 mode.

> - We need to queue new flushes after servicing local flushes.
> 
> I like that it's all in one place. All that being said, I did go
> back-and-forth on this so I am not opposed to open-coding it. Let me know if
> the above argument swayed you or if you still prefer open-coding.

I still prefer open-coding the calls to perform/request/serivce flushes.