Re: [PATCH] KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch
From: Sean Christopherson
Date: Thu Sep 03 2026 - 17:56:20 EST
On Thu, Sep 03, 2026, Yosry Ahmed wrote:
> When the pCPU where the VMCB was mostly recently used is switched, reset
> the ASID generation in both VMCBs, triggering new ASID allocation for
> the immediate VMRUN as well as the next VMRUN on the other VMCB.
>
> The ASID is shared between vmcb01 and vmcb02, and gets flushed on every
> nested transition. However, since pCPU tracking is done per VMCB, it is
> possible for one VMCB to allocate a new ASID when migrated to a new
> pCPU, and then the other VMCB reuses that ASID on the old pCPU. This can
> result in the same ASID being used by multiple vCPUs on the old pCPU.
>
> Example scenario:
> - vCPU runs on pCPU A, vmcb01 is active, asid=1.
> - vCPU migrates to pCPU B, vmcb01 pCPU changes, new asid=2.
> - Another vCPU runs on pCPU A and allocates asid=2 as well.
> - vCPU migrates back to pCPU A, and then switches to vmcb02 before it
> runs again with vmcb01.
> - No pCPU switch is detected for vmcb02, so VMRUN is done with asid=2.
> - Two vCPUs end up using asid=2 on the same pCPU.
>
> Keep the VMCB dirtying to the active VMCB only. Clean bits are tracked
> by a pCPU for each VMCB, so do not unnecessarily dirty a VMCB if its
> pCPU does not change.
>
> Additionally, initialize the tracker pCPU for vmcb02 to -1 on nested
> enablement, so that the new ASID allocation in the scenario above
> happens even if EFER.SVME is disabled in L1 before migrating to the new
> pCPU (so asid_generation in vmcb02 is not reset), but enabled before
> returning to the old pCPU.
>
> No performance regression was noticed when overcommitting L1 vCPUs in L0
> (to force rescheduling), pinning L1 <-> L2 vCPUs, and running CPUID in a
> tight loop bouncing between 2 vCPUs in L2.
>
> An alternative (and perhaps more proper) fix would be tracking the ASID
> per-VMCB instead (e.g. [1]). However, that's a more involved change, and
> it would result in having different ASIDs for L1 and L2 without actually
> properly maintaining them. It would probably work because all TLB
> flushes target the current VMCB, and the other VMCB is always flushed on
> nested transitions, but the code ends up in an arguably more fragile
> state. Punt a proper clean fix to an incoming (and overdue) overhaul of
> SVM's ASID usage [2].
>
> [1]https://lore.kernel.org/lkml/20250205182402.2147495-2-yosry.ahmed@xxxxxxxxx/
> [2]https://lore.kernel.org/kvm/20260728003557.1136583-1-yosry@xxxxxxxxxx/
>
Fixes: 193015adf40d ("KVM: nSVM: Track the ASID generation of the vmcb vmrun through the vmcb")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Stefan Teodorescu <fane@xxxxxxxxxx>
> Signed-off-by: Yosry Ahmed <yosry@xxxxxxxxxx>
> ---
>
> I wasn't sure if the last paragraph (or parts of it) fit in the
> changelog or below ---, so I just put it all in the changelog, but feel
> free to move things around.
I like having the alternative(s) listed in the changelog, it saves having to
find the alternative when digging through git (if the reader is even aware there
was/is an alternative).
I agree with your assessment, tracking per-VMCB is absolutely the right approach
given that the asid_generation is tracked per-VMCB. But I hate how SVM manages
ASIDs and want to burn it with fire. Taking a quick-and-dirty approach will be
good motivation for landing the overhaul of ASIDs.
I _was_ going to propose an alternative solution, but it subtly doesn't work.
More below.
> ---
> arch/x86/kvm/svm/nested.c | 1 +
> arch/x86/kvm/svm/svm.c | 20 ++++++++++++++++----
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 73f37b050d0a0..0c55c71fc6010 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1494,6 +1494,7 @@ int svm_allocate_nested(struct vcpu_svm *svm)
> if (!svm->nested.msrpm)
> goto err_free_vmcb02;
>
> + svm->nested.vmcb02.cpu = -1;
> svm->nested.initialized = true;
> return 0;
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index ea647938a2a65..c388c6598463e 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3765,14 +3765,26 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
> struct vcpu_svm *svm = to_svm(vcpu);
>
> /*
> - * If the previous vmrun of the vmcb occurred on a different physical
> - * cpu, then mark the vmcb dirty and assign a new asid. Hardware's
> - * vmcb clean bits are per logical CPU, as are KVM's asid assignments.
> + * If the previous VMRUN of the VMCB occurred on a different physical
> + * cpu, then mark the VMCB dirty as hardware's clean bits are per pCPU.
> + *
> + * Reset the ASID generation in both VMCBs. This will lead to assigning
> + * a new ASID now, and then again when switching to the other VMCB.
> + * However, this is needed as the ASID is shared between the VMCBs, and
> + * otherwise it would be possible to use an ASID allocated on one pCPU
> + * on another, for example:
> + * - vCPU migrates from pCPU A to pCPU B, allocates a new ASID.
> + * - vCPU migrates back to pCPU A, and then switches the VMCB.
> + * - The new VMCB does not detect a pCPU change and runs on pCPU A with
> + * the new ASID allocated on pCPU B, which is potentially used by
> + * another vCPU/VM.
> */
> if (unlikely(svm->current_vmcb->cpu != vcpu->cpu)) {
> - svm->current_vmcb->asid_generation = 0;
> vmcb_mark_all_dirty(svm->vmcb);
> svm->current_vmcb->cpu = vcpu->cpu;
> + svm->vmcb01.asid_generation = 0;
> + if (svm->nested.initialized)
Isn't conditioning the clear on nested.initialized wrong? It's stupidly contrived,
but I think it can happen? Even if it can't, I don't see any reason to conditionally
zero vmcb02.asid_generation. Either it's buggy or it's a wash in terms of performance.
- vCPUx runs on pCPU A, vmcb02 is active, asid=1.
- vCPUx migrates to pCPU B, vmcb02 pCPU changes, new asid=2.
- vCPUz runs on pCPU A and allocates asid=2 as well.
- vCPUx switches to vmbc01 and disabled nested, but doesn't run, e.g. because
userspace stuffs EFER.
- vCPUx migrates back to pCPU A, and zeroes vmcb01.asid_generation, but not
vmcb02.asid_generation.
- vCPUx switches to vmcb02, without running vmcb01, again thanks to userspace.
- vCPUx does VMRUN on vmcb02 with asid=2.
- Two vCPUs end up using asid=2 on the same pCPU.
The "svm->current_vmcb->cpu != vcpu->cpu" check is also sketchy, but I don't think
it's outright wrong?
Ugh. Jumping back a bit, I _was_ going to say that we could revert 193015adf40d
and then do:
diff --git arch/x86/kvm/svm/svm.c arch/x86/kvm/svm/svm.c
index b4845e452e69..4c8e2fcd378e 100644
--- arch/x86/kvm/svm/svm.c
+++ arch/x86/kvm/svm/svm.c
@@ -1310,12 +1310,6 @@ void svm_switch_vmcb(struct vcpu_svm *svm, struct kvm_vmcb_info *target_vmcb)
{
svm->current_vmcb = target_vmcb;
svm->vmcb = target_vmcb->ptr;
-
- /*
- * Workaround: we don't yet track the ASID generation
- * that was active the last time target_vmcb was run.
- */
- svm->asid_generation = 0;
}
static int svm_vcpu_create(struct kvm_vcpu *vcpu)
@@ -4531,6 +4525,9 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
sync_lapic_to_cr8(vcpu);
if (unlikely(svm->asid != svm->vmcb->control.asid)) {
+ if (svm->vmcb->control.tlb_ctl != TLB_CONTROL_FLUSH_ALL_ASID)
+ svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ASID;
+
svm->vmcb->control.asid = svm->asid;
vmcb_mark_dirty(svm->vmcb, VMCB_ASID);
}
But after reading the cover letter[*], I can't tell if 193015adf40d was a bug
fix for a dirty/clean bits bug, a bug fix for ASID reuse, or an optimization (I
thought it was an optimization until reading the cover letter and looking more
at commit af18fa775d07 ("KVM: nSVM: Track the physical cpu of the vmcb vmrun
through the vmcb").
So yeah, hit this with a hammer and defer the proper fix to your cleanup series,
because I have low confidence that doing a proper fix is the safest approach for
LTS kernels.
[*] https://lore.kernel.org/all/20210112164313.4204-1-cavery@xxxxxxxxxx