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

[Sean Christopherson <seanjc@google.com>]

On Wed, Oct 28, 2026, Yosry Ahmed wrote:
> On Wed, Oct 28, 2026 at 12:55 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Tue, Oct 27, 2026, Yosry Ahmed wrote:
> > > On Tue, Oct 27, 2026 at 10:50 AM Sean Christopherson <seanjc@google.com> wrote:
> > > >
> > > > On Mon, Oct 26, 2026, Yosry Ahmed wrote:
> > > > > On Mon, Oct 26, 2026 at 10:33 AM Sean Christopherson <seanjc@google.com> wrote:
> > > > > >
> > > > > > On Thu, Oct 22, 2026, Yosry Ahmed wrote:
> > > > > > > On Thu, Oct 22, 2026 at 3:44 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > > > > >
> > > > > > > > On Fri, Oct 16, 2026, Jim Mattson wrote:
> > > > > > > > > On Mon, Oct 12, 2026 at 4:37 PM Yosry Ahmed <yosry@kernel.org> 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 :)
> >
> > It's not so much a can of worms as it is infeasible to come up with a generic
> > scheme that would add value.  The hardest part in all of this is identifying when
> > an action/request *always* needs to be serviced in the context of L1, e.g. this
> > code has been around for ages and it wasn't until Jimshiko came along that anyone
> > noticed there's a bug.
> >
> > The actual servicing of the request in L1 is straightforward and vendor specific
> > for the vast majority of cases.  E.g. guard(vmx_vmcs01) is essentially what you're
> > asking for, but it's obviously VMX specific beause SVM can simply grab the pointer
> > to vmcb01.  In other words, there isn't a general need to defer requests or have a
> > framework for servicing requests while L2 is active.
> >
> > This one is special because of legacy shadow paging, and specifically the need to
> > synchronizing the MMU.  In theory, we could temporarily switch to L1's MMU, a la
> > vmcs01, but it's entirely possible L1's MMU doesn't even "exist", e.g. because
> > the host state that will be loaded from the VMCS on nested VM-Exit is different
> > than the pre-VM-Enter state (which is possible on Intel).  Even if the MMU context
> > was known, switching MMUs at this point in the KVM_RUN flow is terrifying.
> >
> > So, while I detest the deferred approach, I also think it's absolutely the right
> > thing to do for a fix that needs to go to stable@, and probably the least awful
> > fix long term as well.  But it's very much a one-off snowflake, not something we
> > should generally encourage.
> 
> The other example I thought about is
> kvm_service_local_tlb_flush_requests(), AFAICT this only exists
> because we don't have a way to specify a request for L1/L2. But yeah,
> I agree if these are the exceptions rather than the rule.

Yes and no.  Doing the "obvious" thing by tracking L1 vs. L2 requests would be
subtly wrong.  E.g. if L2 triggers a flush with virtual ASID=1, then KVM must
service the flush in exactly that context.  If KVM shoved the request into e.g.
l2_requests and serviced those at the next entry to L2, then KVM could process
the TLB flush using the wrong virtual ASID.

So while I get where you're coming from, I think the current approach of using
a unified requests queue and then servicing the requests that are "local" to L2
on nested VM-Exit is the "right" approach.

> In my head, there was also a request we clear for exceptions on nested
> VM-Exit, but looking at the code again, it's not a request, we clear
> the exception and interrupt queues. I think this is the right thing to
> do architecturally anyway though?

Yep.  If an event is supposed to be preserved, then it needs to be saved into
the exit fields of the VMC{B,S}.  KVM's software model should always be reset.