Re: [PATCH v2 06/20] KVM: x86: Request the guest TLB flush from record_steal_time()

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 3714 bytes --]

On Tue, 2026-11-10 at 12:19 -0800, Sean Christopherson wrote:
> On Thu, Nov 05, 2026, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > record_steal_time() performs a TLB flush on the guest's behalf when
> > the
> > guest sets KVM_VCPU_FLUSH_TLB. With shadow paging,
> > kvm_vcpu_flush_tlb_guest() ends up in mmu_sync_children(), which
> > can
> > reschedule via cond_resched_rwlock_write(). That is why the flush
> > currently sits between a user_access_end() and a fresh
> > user_access_begin(): it cannot be done while the guest page is
> > mapped
> > for access.
> > 
> > Raise KVM_REQ_TLB_FLUSH_GUEST instead of flushing inline, and move
> > the
> > processing of KVM_REQ_STEAL_UPDATE ahead of where the TLB flush
> > requests
> > are serviced in vcpu_enter_guest() so that the flush still happens
> > before
> > the vCPU enters the guest. There is no requirement for it to
> > complete any
> > earlier than that.
> > 
> > kvm_make_request() on the vCPU which is running is nothing more
> > than a
> > barrier and a set_bit(), so unlike the flush itself it can be done
> > from
> > any context — which lets the surrounding access be simplified
> > later.
> > 
> > Note that the xchg() must stay where it is: it atomically consumes
> > any
> > KVM_VCPU_FLUSH_TLB which the guest has set, as required by commit
> > b043138246a4 ("x86/KVM: Make sure KVM_VCPU_FLUSH_TLB flag is not
> > missed").
> > 
> > Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
> > Closes:
> > https://lore.kernel.org/all/20260530061932.7849D1F00893@smtp.kernel.org
> > Suggested-by: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > ---
> >  arch/x86/kvm/x86.c | 26 +++++++++++++++++++++++---
> >  1 file changed, 23 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 6a1f0404f029..3c26c0eb9865 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -2117,8 +2117,22 @@ static void record_steal_time(struct
> > kvm_vcpu *vcpu)
> >  
> >  		trace_kvm_pv_tlb_flush(vcpu->vcpu_id,
> >  				       st_preempted &
> > KVM_VCPU_FLUSH_TLB);
> > +		/*
> > +		 * Request the flush instead of performing it
> > inline. With
> > +		 * shadow paging kvm_vcpu_flush_tlb_guest() can
> > reschedule
> > +		 * (via mmu_sync_children()), so it could not be
> > called while
> > +		 * the user access above was open. Requesting is
> > just a bit
> > +		 * set on the running vCPU, and the request is
> > serviced
> > +		 * before the vCPU enters the guest; nothing
> > requires the
> > +		 * flush to have completed any earlier than that.
> 
> Argh, I completely forgot about this.  It throws a big wrench in my plans to fix
> a mishandled TLB flush without using a deferred mechanism[1].  I really don't like
> having to re-introduce deferred flags, but the diff is a lot smaller[2].  On the
> other hand, I still don't like the idea of delaying the MMU sync until the nested
> VM-Exit.
> 
> I think I'll see how gross it is to do something like add a
> KVM_REQ_PV_TLB_FLUSH.
> 
> [1] https://lore.kernel.org/all/au0R41xbEJ_9W_9z@google.com
> [2] https://lore.kernel.org/all/auDkX94UTjrLNmqp@google.com

It's still OK just to do the TLB flush synchronously from the end of
record_steal_time() after clearing KVM_VCPU_FLUSH_TLB and dropping the
GPC lock.

You suggested using KVM_REQ_TLB_FLUSH_GUEST in
https://lore.kernel.org/all/aidiOemcna1Un9wG@google.com/ but if it now
makes you sad, I'm happy to go back to the original approach (noting
that it loses the coalescing of multiple flushes).

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]