Re: [PATCH v3 26/27] KVM: nSVM: Use different ASIDs for L1 and L2

[Yosry Ahmed]

On Tue, Oct 27, 2026 at 10:50 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Mon, Oct 26, 2026, Yosry Ahmed wrote:
> > On Mon, Oct 26, 2026 at 10:33 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > >
> > > On Thu, Oct 22, 2026, Yosry Ahmed wrote:
> > > > On Thu, Oct 22, 2026 at 3:44 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > > > >
> > > > > On Fri, Oct 16, 2026, Jim Mattson wrote:
> > > > > > On Mon, Oct 12, 2026 at 4:37 PM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
> > > > > > Moreover, does this change the semantics of PV TLB flush? In
> > > > > > record_steal_time(), we have:
> > > > > >
> > > > > > if (st_preempted & KVM_VCPU_FLUSH_TLB)
> > > > > >         kvm_vcpu_flush_tlb_guest(vcpu);
> > > > > >
> > > > > > IIUC, an idle vCPU that wakes into L2 will flush only L2's ASID. In
> > > > > > the past, with a shared ASID, L1's ASID was always flushed on emulated
> > > > > > VM-exit anyway. Now, L1's ASID will be unaffected. Though I can't find
> > > > > > any real documentation on PV TLB flush, it *seems* like the remote
> > > > > > vCPU would want L1's ASID to be flushed. OTOH, this change makes nSVM
> > > > > > behave just like nVMX.
> > > > >
> > > > > That's a bug that absolutely needs to be fixed, probably relatively urgently.  We
> > > > > should also document that passing through the steal_time shared information to L2
> > > > > to utilize PV TLB flushes for L2 is explicitly not supported.
> > > > >
> > > > > I hate to re-introduce deferred actions for L2 => L1 things, but that seems like
> > > > > the only sane way to handle this, because handling the !tdp_enabled case, where
> > > > > KVM needs to sync the current root, requires knowing what MMU is going to be in
> > > > > use when L1 regains control.  Yuck.
> > > >
> > > > Are you implying that *you* will be re-introducing deferred actions? :P
> > >
> > > Heh, probably not this calendar year.
> >
> > Are you opposed to this series landing before that is fixed, keeping
> > in mind that it will expose nSVM to the pre-existing bug?
>
> Nah, though I should just fix the damn bug.  It's not hard, I just don't want to
> do it because I don't want to reintroduce a deferred request.  Untested, but I
> think this is all that's needed.

Makes sense to the untrained eye, although I do wonder sometimes if
requests more generally need a way to key off guest mode. I am sure
you don't want to open this can of worms :)

>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 0378577d9d8b..696aae38f619 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -879,6 +879,7 @@ struct kvm_vcpu_arch {
>
>         struct {
>                 u8 preempted;
> +               bool flush_tlb_pending;
>                 u64 msr_val;
>                 u64 last_steal;
>                 struct gfn_to_hva_cache cache;
> diff --git a/arch/x86/kvm/regs.h b/arch/x86/kvm/regs.h
> index 97c17b120a57..efa54f9f9e9a 100644
> --- a/arch/x86/kvm/regs.h
> +++ b/arch/x86/kvm/regs.h
> @@ -469,6 +469,11 @@ static inline void leave_guest_mode(struct kvm_vcpu *vcpu)
>                 kvm_make_request(KVM_REQ_LOAD_EOI_EXITMAP, vcpu);
>         }
>
> +       if (vcpu->arch.st.flush_tlb_pending) {
> +               kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu);
> +               vcpu->arch.st.flush_tlb_pending = false;
> +       }
> +
>         vcpu->stat.guest_mode = 0;
>  }
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 32563a91fba9..15eb033b8e17 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2117,8 +2117,12 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
>
>                 trace_kvm_pv_tlb_flush(vcpu->vcpu_id,
>                                        st_preempted & KVM_VCPU_FLUSH_TLB);
> -               if (st_preempted & KVM_VCPU_FLUSH_TLB)
> -                       kvm_vcpu_flush_tlb_guest(vcpu);
> +               if (st_preempted & KVM_VCPU_FLUSH_TLB) {
> +                       if (!is_guest_mode(vcpu))
> +                               kvm_vcpu_flush_tlb_guest(vcpu);
> +                       else
> +                               vcpu->arch.st.flush_tlb_pending = true;
> +               }
>
>                 if (!user_access_begin(st, sizeof(*st)))
>                         goto dirty;