* [PATCH] KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch
@ 2026-09-03 17:58 Yosry Ahmed
2026-09-03 21:53 ` Sean Christopherson
0 siblings, 1 reply; 6+ messages in thread
From: Yosry Ahmed @ 2026-09-03 17:58 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, kvm, linux-kernel, Yosry Ahmed, stable,
Stefan Teodorescu
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@linux.dev/
[2]https://lore.kernel.org/kvm/20260728003557.1136583-1-yosry@kernel.org/
Cc: stable@vger.kernel.org
Reported-by: Stefan Teodorescu <fane@google.com>
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
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.
---
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)
+ svm->nested.vmcb02.asid_generation = 0;
}
if (is_sev_guest(vcpu))
--
2.55.0.979.g7e5102b832-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch
2026-09-03 17:58 [PATCH] KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch Yosry Ahmed
@ 2026-09-03 21:53 ` Sean Christopherson
2026-09-03 22:00 ` Yosry Ahmed
0 siblings, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2026-09-03 21:53 UTC (permalink / raw)
To: Yosry Ahmed; +Cc: Paolo Bonzini, kvm, linux-kernel, stable, Stefan Teodorescu
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@linux.dev/
> [2]https://lore.kernel.org/kvm/20260728003557.1136583-1-yosry@kernel.org/
>
Fixes: 193015adf40d ("KVM: nSVM: Track the ASID generation of the vmcb vmrun through the vmcb")
> Cc: stable@vger.kernel.org
> Reported-by: Stefan Teodorescu <fane@google.com>
> Signed-off-by: Yosry Ahmed <yosry@kernel.org>
> ---
>
> 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@redhat.com
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch
2026-09-03 21:53 ` Sean Christopherson
@ 2026-09-03 22:00 ` Yosry Ahmed
2026-09-03 22:18 ` Sean Christopherson
0 siblings, 1 reply; 6+ messages in thread
From: Yosry Ahmed @ 2026-09-03 22:00 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, kvm, linux-kernel, stable, Stefan Teodorescu
On Thu, Sep 3, 2026 at 2:53 PM Sean Christopherson <seanjc@google.com> wrote:
>
> 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@linux.dev/
> > [2]https://lore.kernel.org/kvm/20260728003557.1136583-1-yosry@kernel.org/
> >
>
> Fixes: 193015adf40d ("KVM: nSVM: Track the ASID generation of the vmcb vmrun through the vmcb")
>
> > Cc: stable@vger.kernel.org
> > Reported-by: Stefan Teodorescu <fane@google.com>
> > Signed-off-by: Yosry Ahmed <yosry@kernel.org>
> > ---
> >
> > 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.
Oh yeah it's not needed. I thought the generation is only allocated
when svm->nested.initialized, but that's not true it's always there.
So yeah we can just always zero it, and then we can drop the change in
svm_allocate_nested() too, or keep it as hardening (it seems like a
good idea in general)?
>
> - 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.
IIUC, here userspace wrote EFER.SVME=0..
> - 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.
..and here it wrote EFER.SVME=1, in which case svm_allocate_nested()
should set svm->nested.vmcb02.cpu = -1..
> - vCPUx does VMRUN on vmcb02 with asid=2.
..and we allocate a new ASID here?
> - 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.
I can send a new version dropping conditioning the reset on
svm->nested.initialized and dropping the change in
svm_allocate_nested(), is this what you had in mind?
>
> [*] https://lore.kernel.org/all/20210112164313.4204-1-cavery@redhat.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch
2026-09-03 22:00 ` Yosry Ahmed
@ 2026-09-03 22:18 ` Sean Christopherson
2026-09-03 22:20 ` Yosry Ahmed
0 siblings, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2026-09-03 22:18 UTC (permalink / raw)
To: Yosry Ahmed; +Cc: Paolo Bonzini, kvm, linux-kernel, stable, Stefan Teodorescu
On Thu, Sep 03, 2026, Yosry Ahmed wrote:
> On Thu, Sep 3, 2026 at 2:53 PM Sean Christopherson <seanjc@google.com> wrote:
> > > 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.
>
> Oh yeah it's not needed. I thought the generation is only allocated
> when svm->nested.initialized, but that's not true it's always there.
> So yeah we can just always zero it, and then we can drop the change in
> svm_allocate_nested() too, or keep it as hardening (it seems like a
> good idea in general)?
Definitely keep it as hardening.
> > - 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.
>
> IIUC, here userspace wrote EFER.SVME=0..
>
> > - 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.
>
> ..and here it wrote EFER.SVME=1, in which case svm_allocate_nested()
> should set svm->nested.vmcb02.cpu = -1..
>
> > - vCPUx does VMRUN on vmcb02 with asid=2.
>
> ..and we allocate a new ASID here?
Oh, right. Yeesh, that's subtle. All the more reason to burn with fire.
> > - 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.
>
> I can send a new version dropping conditioning the reset on
> svm->nested.initialized and dropping the change in
> svm_allocate_nested(), is this what you had in mind?
Keep the change in svm_allocate_nested(). Or just do nothing, I'll happily drop
the svm->nested.initialized check when applying.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch
2026-09-03 22:18 ` Sean Christopherson
@ 2026-09-03 22:20 ` Yosry Ahmed
2026-09-03 22:26 ` Sean Christopherson
0 siblings, 1 reply; 6+ messages in thread
From: Yosry Ahmed @ 2026-09-03 22:20 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, kvm, linux-kernel, stable, Stefan Teodorescu
On Thu, Sep 3, 2026 at 3:18 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Sep 03, 2026, Yosry Ahmed wrote:
> > On Thu, Sep 3, 2026 at 2:53 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > 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.
> >
> > Oh yeah it's not needed. I thought the generation is only allocated
> > when svm->nested.initialized, but that's not true it's always there.
> > So yeah we can just always zero it, and then we can drop the change in
> > svm_allocate_nested() too, or keep it as hardening (it seems like a
> > good idea in general)?
>
> Definitely keep it as hardening.
>
> > > - 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.
> >
> > IIUC, here userspace wrote EFER.SVME=0..
> >
> > > - 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.
> >
> > ..and here it wrote EFER.SVME=1, in which case svm_allocate_nested()
> > should set svm->nested.vmcb02.cpu = -1..
> >
> > > - vCPUx does VMRUN on vmcb02 with asid=2.
> >
> > ..and we allocate a new ASID here?
>
> Oh, right. Yeesh, that's subtle. All the more reason to burn with fire.
>
> > > - 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.
> >
> > I can send a new version dropping conditioning the reset on
> > svm->nested.initialized and dropping the change in
> > svm_allocate_nested(), is this what you had in mind?
>
> Keep the change in svm_allocate_nested(). Or just do nothing, I'll happily drop
> the svm->nested.initialized check when applying.
You'll need to update the changelog too.
Instead of:
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.
Maybe:
Additionally, initialize the tracked pCPU for vmcb02 to -1 on nested
enablement as hardening, so that a new ASID allocation is always
triggered when nested is disabled and re-enabled.
Let me know if you still wanna fixup when applying or a v2 :)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch
2026-09-03 22:20 ` Yosry Ahmed
@ 2026-09-03 22:26 ` Sean Christopherson
0 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2026-09-03 22:26 UTC (permalink / raw)
To: Yosry Ahmed; +Cc: Paolo Bonzini, kvm, linux-kernel, stable, Stefan Teodorescu
On Thu, Sep 03, 2026, Yosry Ahmed wrote:
> On Thu, Sep 3, 2026 at 3:18 PM Sean Christopherson <seanjc@google.com> wrote:
> > Keep the change in svm_allocate_nested(). Or just do nothing, I'll happily drop
> > the svm->nested.initialized check when applying.
>
> You'll need to update the changelog too.
>
> Instead of:
>
> 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.
>
> Maybe:
>
> Additionally, initialize the tracked pCPU for vmcb02 to -1 on nested
> enablement as hardening, so that a new ASID allocation is always
> triggered when nested is disabled and re-enabled.
>
> Let me know if you still wanna fixup when applying or a v2 :)
LOL, just send a v2.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-03 22:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03 17:58 [PATCH] KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch Yosry Ahmed
2026-09-03 21:53 ` Sean Christopherson
2026-09-03 22:00 ` Yosry Ahmed
2026-09-03 22:18 ` Sean Christopherson
2026-09-03 22:20 ` Yosry Ahmed
2026-09-03 22:26 ` Sean Christopherson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).