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

[Yosry Ahmed]

On Wed, Oct 28, 2026 at 12:55 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Tue, Oct 27, 2026, Yosry Ahmed wrote:
> > 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 :)
>
> 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.

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?