[PATCH] KVM: SVM: Update tracked pCPU on pCPU switch

[PATCH] KVM: SVM: Update tracked pCPU on pCPU switch

[Yosry Ahmed]

When a pCPU switch is detected in pre_svm_run(), update the tracked pCPU
and mark all fields dirty in both VMCBs (if nested is initialized), in
addition to resetting the ASID generation in both VMCBs.

Commit bf260dc9e7b8 ("KVM: SVM: Trigger new ASID allocation in both
VMCBs on pCPU switch") reset the ASID generation in both VMCBs on a pCPU
switch, but only updated the tracked pCPU for the active VMCB to avoid
unnecessarily dirtying the inactive VMCB. Leaving the inactive VMCB's
tracked pCPU set to an old pCPU still allows an ASID allocated on one
pCPU to be used on another:

- vCPU runs with both vmcb01 and vmcb02 on pCPU A (both track cpu=A).
- With vmcb01 active, vCPU migrates to pCPU B. vmcb01.cpu is updated to
B, asid_generation is reset to 0 in both VMCBs, and a new ASID is
allocated on pCPU B (setting vmcb01.asid_generation = gen_B).
vmcb02.cpu remains A.
- vCPU switches to vmcb02 and migrates back to pCPU A before
pre_svm_run(). Because vmcb02.cpu is still A, no pCPU switch is
detected, leaving vmcb01 with cpu=B and asid_generation=gen_B.
Because vmcb02.asid_generation is 0, a new ASID (asid_A) is allocated
on pCPU A.
- vCPU switches back to vmcb01 and migrates to pCPU B. Neither a pCPU
change nor a generation mismatch is detected for vmcb01, so VMRUN on
pCPU B uses asid_A (allocated on pCPU A), which may collide with
another vCPU on pCPU B.

Because the per-VMCB cpu tracking is shared between ASID management and
per-pCPU hardware VMCB clean bits, also mark both VMCBs dirty when
updating their tracked pCPU so that the inactive VMCB does not run with
stale clean bits when later entered on the new pCPU.

Fixes: bf260dc9e7b8 ("KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch")
Cc: stable@vger.kernel.org
Reported-by: Stefan Teodorescu <fane@google.com>
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 arch/x86/kvm/svm/svm.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 136069dfcd44f..0a0af0cbb6bba 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3880,7 +3880,8 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
 
 	/*
 	 * 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.
+	 * cpu, mark the VMCB dirty and reset the ASID generation, as hardware's
+	 * clean bits and ASIDs 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.
@@ -3892,13 +3893,22 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
 	 * - 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.
+	 *
+	 * Note that dirtying both VMCBs is not strictly needed, but the per-CPU
+	 * tracking is shared for both ASID and clean bits, and that CPU must
+	 * match where the ASID was allocated.
 	 */
 	if (unlikely(svm->current_vmcb->cpu != vcpu->cpu)) {
-		vmcb_mark_all_dirty(svm->vmcb);
-		svm->current_vmcb->cpu = vcpu->cpu;
+		vmcb_mark_all_dirty(svm->vmcb01.ptr);
+		svm->vmcb01.cpu = vcpu->cpu;
 		svm->vmcb01.asid_generation = 0;
-		svm->nested.vmcb02.asid_generation = 0;
-        }
+
+		if (svm->nested.initialized) {
+			svm->nested.vmcb02.cpu = vcpu->cpu;
+			svm->nested.vmcb02.asid_generation = 0;
+			vmcb_mark_all_dirty(svm->nested.vmcb02.ptr);
+		}
+	}
 
 	if (is_sev_guest(vcpu))
 		return pre_sev_run(svm, vcpu->cpu);
-- 
2.56.0.385.gd3acb90ef8-goog

Re: [PATCH] KVM: SVM: Update tracked pCPU on pCPU switch

[Jim Mattson]

On Mon, Oct 12, 2026 at 3:47 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> When a pCPU switch is detected in pre_svm_run(), update the tracked pCPU
> and mark all fields dirty in both VMCBs (if nested is initialized), in
> addition to resetting the ASID generation in both VMCBs.
>
> Commit bf260dc9e7b8 ("KVM: SVM: Trigger new ASID allocation in both
> VMCBs on pCPU switch") reset the ASID generation in both VMCBs on a pCPU
> switch, but only updated the tracked pCPU for the active VMCB to avoid
> unnecessarily dirtying the inactive VMCB. Leaving the inactive VMCB's
> tracked pCPU set to an old pCPU still allows an ASID allocated on one
> pCPU to be used on another:
>
> - vCPU runs with both vmcb01 and vmcb02 on pCPU A (both track cpu=A).
> - With vmcb01 active, vCPU migrates to pCPU B. vmcb01.cpu is updated to
> B, asid_generation is reset to 0 in both VMCBs, and a new ASID is
> allocated on pCPU B (setting vmcb01.asid_generation = gen_B).
> vmcb02.cpu remains A.
> - vCPU switches to vmcb02 and migrates back to pCPU A before
> pre_svm_run(). Because vmcb02.cpu is still A, no pCPU switch is
> detected, leaving vmcb01 with cpu=B and asid_generation=gen_B.
> Because vmcb02.asid_generation is 0, a new ASID (asid_A) is allocated
> on pCPU A.
> - vCPU switches back to vmcb01 and migrates to pCPU B. Neither a pCPU
> change nor a generation mismatch is detected for vmcb01, so VMRUN on
> pCPU B uses asid_A (allocated on pCPU A), which may collide with
> another vCPU on pCPU B.

By "collide," you mean that two vCPUs, potentially from different VMs,
will share TLB and paging-structure cache entries? Yikes!

> Because the per-VMCB cpu tracking is shared between ASID management and
> per-pCPU hardware VMCB clean bits, also mark both VMCBs dirty when
> updating their tracked pCPU so that the inactive VMCB does not run with
> stale clean bits when later entered on the new pCPU.
>
> Fixes: bf260dc9e7b8 ("KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch")

Commit bf260dc9e7b8 hasn't made it to Linus yet, has it? Perhaps Sean
can just squash this in, rather than carrying a fix for a fix.

> Cc: stable@vger.kernel.org
> Reported-by: Stefan Teodorescu <fane@google.com>
> Signed-off-by: Yosry Ahmed <yosry@kernel.org>
> ---
>  arch/x86/kvm/svm/svm.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 136069dfcd44f..0a0af0cbb6bba 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3880,7 +3880,8 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
>
>         /*
>          * 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.
> +        * cpu, mark the VMCB dirty and reset the ASID generation, as hardware's
> +        * clean bits and ASIDs 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.
> @@ -3892,13 +3893,22 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
>          * - 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.
> +        *
> +        * Note that dirtying both VMCBs is not strictly needed, but the per-CPU
> +        * tracking is shared for both ASID and clean bits, and that CPU must
> +        * match where the ASID was allocated.

Is dirtying both VMCBs not strictly needed?

Per APM vol. 2 (rev. 3.45), section 15.15.2:

> The hypervisor must clear the entire VMCB [Clean] field to 0 for a guest, under the following circumstances:
> ...
>  - The hypervisor executes the guest on a different CPU core than one used the last time that guest was executed.
> ...
> Failure to clear the VMCB Clean bits to zero in these cases may result in undefined behavior.

Suppose vmcb02 runs on pCPU A, migrates to (and runs on) pCPU B, and
then migrates back to pCPU A with some clean bits set. At the next
VMRUN, pCPU A will happily load stale state for anything marked clean.

At some point before that VMRUN, something has to clear the clean bits!

Re: [PATCH] KVM: SVM: Update tracked pCPU on pCPU switch

[Yosry Ahmed]

On Mon, Oct 12, 2026 at 4:58 PM Jim Mattson <jmattson@google.com> wrote:
>
> On Mon, Oct 12, 2026 at 3:47 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > When a pCPU switch is detected in pre_svm_run(), update the tracked pCPU
> > and mark all fields dirty in both VMCBs (if nested is initialized), in
> > addition to resetting the ASID generation in both VMCBs.
> >
> > Commit bf260dc9e7b8 ("KVM: SVM: Trigger new ASID allocation in both
> > VMCBs on pCPU switch") reset the ASID generation in both VMCBs on a pCPU
> > switch, but only updated the tracked pCPU for the active VMCB to avoid
> > unnecessarily dirtying the inactive VMCB. Leaving the inactive VMCB's
> > tracked pCPU set to an old pCPU still allows an ASID allocated on one
> > pCPU to be used on another:
> >
> > - vCPU runs with both vmcb01 and vmcb02 on pCPU A (both track cpu=A).
> > - With vmcb01 active, vCPU migrates to pCPU B. vmcb01.cpu is updated to
> > B, asid_generation is reset to 0 in both VMCBs, and a new ASID is
> > allocated on pCPU B (setting vmcb01.asid_generation = gen_B).
> > vmcb02.cpu remains A.
> > - vCPU switches to vmcb02 and migrates back to pCPU A before
> > pre_svm_run(). Because vmcb02.cpu is still A, no pCPU switch is
> > detected, leaving vmcb01 with cpu=B and asid_generation=gen_B.
> > Because vmcb02.asid_generation is 0, a new ASID (asid_A) is allocated
> > on pCPU A.
> > - vCPU switches back to vmcb01 and migrates to pCPU B. Neither a pCPU
> > change nor a generation mismatch is detected for vmcb01, so VMRUN on
> > pCPU B uses asid_A (allocated on pCPU A), which may collide with
> > another vCPU on pCPU B.
>
> By "collide," you mean that two vCPUs, potentially from different VMs,
> will share TLB and paging-structure cache entries? Yikes!

Potentially, yes.

>
> > Because the per-VMCB cpu tracking is shared between ASID management and
> > per-pCPU hardware VMCB clean bits, also mark both VMCBs dirty when
> > updating their tracked pCPU so that the inactive VMCB does not run with
> > stale clean bits when later entered on the new pCPU.
> >
> > Fixes: bf260dc9e7b8 ("KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch")
>
> Commit bf260dc9e7b8 hasn't made it to Linus yet, has it? Perhaps Sean
> can just squash this in, rather than carrying a fix for a fix.

That would be ideal.

>
> > Cc: stable@vger.kernel.org
> > Reported-by: Stefan Teodorescu <fane@google.com>
> > Signed-off-by: Yosry Ahmed <yosry@kernel.org>
> > ---
> >  arch/x86/kvm/svm/svm.c | 20 +++++++++++++++-----
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 136069dfcd44f..0a0af0cbb6bba 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -3880,7 +3880,8 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
> >
> >         /*
> >          * 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.
> > +        * cpu, mark the VMCB dirty and reset the ASID generation, as hardware's
> > +        * clean bits and ASIDs 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.
> > @@ -3892,13 +3893,22 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
> >          * - 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.
> > +        *
> > +        * Note that dirtying both VMCBs is not strictly needed, but the per-CPU
> > +        * tracking is shared for both ASID and clean bits, and that CPU must
> > +        * match where the ASID was allocated.
>
> Is dirtying both VMCBs not strictly needed?
>
> Per APM vol. 2 (rev. 3.45), section 15.15.2:
>
> > The hypervisor must clear the entire VMCB [Clean] field to 0 for a guest, under the following circumstances:
> > ...
> >  - The hypervisor executes the guest on a different CPU core than one used the last time that guest was executed.
> > ...
> > Failure to clear the VMCB Clean bits to zero in these cases may result in undefined behavior.
>
> Suppose vmcb02 runs on pCPU A, migrates to (and runs on) pCPU B, and
> then migrates back to pCPU A with some clean bits set. At the next
> VMRUN, pCPU A will happily load stale state for anything marked clean.
>
> At some point before that VMRUN, something has to clear the clean bits!

Dirtying both VMCBs is not needed in all cases covered with the new
code here. IIUC, clean bits are VMCB-specific, unlike the ASID here
which is shared.

Because ASID is shared, we have to reset the ASID generation for both
VMCBs when a new ASID is allocated through either, and update the cpu
variable tracking which pCPU allocated that ASID. Since that cpu
variable is also shared for tracking which pCPU ran the VMCB last, we
also need to clear the clean bits every time we updated. IOW, if we
had two variables, asid_cpu and vmcb_dirty_cpu, we would need to
update asid_cpu on pCPU migration for both VMCBs, but only update
vmcb_run_cpu for the current VMCB.

This is all annoying and confusing, but that should go away in the
static ASIDs series, and we should go back to only updating pCPU
tracking, flushing the ASID, and clearing the clean bits only for the
current VMCB.

Re: [PATCH] KVM: SVM: Update tracked pCPU on pCPU switch

[Jim Mattson]

On Tue, Oct 13, 2026 at 7:49 AM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Mon, Oct 12, 2026 at 4:58 PM Jim Mattson <jmattson@google.com> wrote:
> >
> > On Mon, Oct 12, 2026 at 3:47 PM Yosry Ahmed <yosry@kernel.org> wrote:
[ ... ]
> > > @@ -3892,13 +3893,22 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
> > >          * - 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.
> > > +        *
> > > +        * Note that dirtying both VMCBs is not strictly needed, but the per-CPU
> > > +        * tracking is shared for both ASID and clean bits, and that CPU must
> > > +        * match where the ASID was allocated.
> >
> > Is dirtying both VMCBs not strictly needed?
> >
> > Per APM vol. 2 (rev. 3.45), section 15.15.2:
> >
> > > The hypervisor must clear the entire VMCB [Clean] field to 0 for a guest, under the following circumstances:
> > > ...
> > >  - The hypervisor executes the guest on a different CPU core than one used the last time that guest was executed.
> > > ...
> > > Failure to clear the VMCB Clean bits to zero in these cases may result in undefined behavior.
> >
> > Suppose vmcb02 runs on pCPU A, migrates to (and runs on) pCPU B, and
> > then migrates back to pCPU A with some clean bits set. At the next
> > VMRUN, pCPU A will happily load stale state for anything marked clean.
> >
> > At some point before that VMRUN, something has to clear the clean bits!
>
> Dirtying both VMCBs is not needed in all cases covered with the new
> code here. IIUC, clean bits are VMCB-specific, unlike the ASID here
> which is shared.
>
> Because ASID is shared, we have to reset the ASID generation for both
> VMCBs when a new ASID is allocated through either, and update the cpu
> variable tracking which pCPU allocated that ASID. Since that cpu
> variable is also shared for tracking which pCPU ran the VMCB last, we
> also need to clear the clean bits every time we updated. IOW, if we
> had two variables, asid_cpu and vmcb_dirty_cpu, we would need to
> update asid_cpu on pCPU migration for both VMCBs, but only update
> vmcb_run_cpu for the current VMCB.
>
> This is all annoying and confusing, but that should go away in the
> static ASIDs series, and we should go back to only updating pCPU
> tracking, flushing the ASID, and clearing the clean bits only for the
> current VMCB.

I haven't read the static ASIDs series, but in any case, this patch
should stand on its own, in case that series never gets committed or
gets committed much later.

My complaint is with the claim, "dirtying both VMCBs is not strictly
needed," which seems to imply that dirtying the inactive VMCB here is
optional. In fact, once you update vmcb02.cpu/vmcb01.cpu for both
VMCBs, dirtying both *is* strictly needed so the inactive VMCB doesn't
later miss a pCPU switch.

Re: [PATCH] KVM: SVM: Update tracked pCPU on pCPU switch

[Yosry Ahmed]

On Tue, Oct 13, 2026 at 10:55 AM Jim Mattson <jmattson@google.com> wrote:
>
> On Tue, Oct 13, 2026 at 7:49 AM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Mon, Oct 12, 2026 at 4:58 PM Jim Mattson <jmattson@google.com> wrote:
> > >
> > > On Mon, Oct 12, 2026 at 3:47 PM Yosry Ahmed <yosry@kernel.org> wrote:
> [ ... ]
> > > > @@ -3892,13 +3893,22 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
> > > >          * - 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.
> > > > +        *
> > > > +        * Note that dirtying both VMCBs is not strictly needed, but the per-CPU
> > > > +        * tracking is shared for both ASID and clean bits, and that CPU must
> > > > +        * match where the ASID was allocated.
> > >
> > > Is dirtying both VMCBs not strictly needed?
> > >
> > > Per APM vol. 2 (rev. 3.45), section 15.15.2:
> > >
> > > > The hypervisor must clear the entire VMCB [Clean] field to 0 for a guest, under the following circumstances:
> > > > ...
> > > >  - The hypervisor executes the guest on a different CPU core than one used the last time that guest was executed.
> > > > ...
> > > > Failure to clear the VMCB Clean bits to zero in these cases may result in undefined behavior.
> > >
> > > Suppose vmcb02 runs on pCPU A, migrates to (and runs on) pCPU B, and
> > > then migrates back to pCPU A with some clean bits set. At the next
> > > VMRUN, pCPU A will happily load stale state for anything marked clean.
> > >
> > > At some point before that VMRUN, something has to clear the clean bits!
> >
> > Dirtying both VMCBs is not needed in all cases covered with the new
> > code here. IIUC, clean bits are VMCB-specific, unlike the ASID here
> > which is shared.
> >
> > Because ASID is shared, we have to reset the ASID generation for both
> > VMCBs when a new ASID is allocated through either, and update the cpu
> > variable tracking which pCPU allocated that ASID. Since that cpu
> > variable is also shared for tracking which pCPU ran the VMCB last, we
> > also need to clear the clean bits every time we updated. IOW, if we
> > had two variables, asid_cpu and vmcb_dirty_cpu, we would need to
> > update asid_cpu on pCPU migration for both VMCBs, but only update
> > vmcb_run_cpu for the current VMCB.
> >
> > This is all annoying and confusing, but that should go away in the
> > static ASIDs series, and we should go back to only updating pCPU
> > tracking, flushing the ASID, and clearing the clean bits only for the
> > current VMCB.
>
> I haven't read the static ASIDs series, but in any case, this patch
> should stand on its own, in case that series never gets committed or
> gets committed much later.

It does. I just didn't spend time trying to improve things beyond the fix.

>
> My complaint is with the claim, "dirtying both VMCBs is not strictly
> needed," which seems to imply that dirtying the inactive VMCB here is
> optional. In fact, once you update vmcb02.cpu/vmcb01.cpu for both
> VMCBs, dirtying both *is* strictly needed so the inactive VMCB doesn't
> later miss a pCPU switch.

Yeah I guess it can be verbed more accurately, something like this:

Dirtying both VMCBs is generally not required if only one VMCB
switches pCPUs, but since the pCPU tracking is shared between ASID and
VMCB clean bits, it must be done once pCPU tracking is updated.

I won't respin until Sean takes a look, in case he wants more changes
or prefers to make changes while applying.

Re: [PATCH] KVM: SVM: Update tracked pCPU on pCPU switch

[Jim Mattson]

On Tue, Oct 13, 2026 at 11:11 AM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Tue, Oct 13, 2026 at 10:55 AM Jim Mattson <jmattson@google.com> wrote:
> >
> > On Tue, Oct 13, 2026 at 7:49 AM Yosry Ahmed <yosry@kernel.org> wrote:
> > >
> > > On Mon, Oct 12, 2026 at 4:58 PM Jim Mattson <jmattson@google.com> wrote:
> > > >
> > > > On Mon, Oct 12, 2026 at 3:47 PM Yosry Ahmed <yosry@kernel.org> wrote:
> > [ ... ]
> > > > > @@ -3892,13 +3893,22 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
> > > > >          * - 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.
> > > > > +        *
> > > > > +        * Note that dirtying both VMCBs is not strictly needed, but the per-CPU
> > > > > +        * tracking is shared for both ASID and clean bits, and that CPU must
> > > > > +        * match where the ASID was allocated.
> > > >
> > > > Is dirtying both VMCBs not strictly needed?
> > > >
> > > > Per APM vol. 2 (rev. 3.45), section 15.15.2:
> > > >
> > > > > The hypervisor must clear the entire VMCB [Clean] field to 0 for a guest, under the following circumstances:
> > > > > ...
> > > > >  - The hypervisor executes the guest on a different CPU core than one used the last time that guest was executed.
> > > > > ...
> > > > > Failure to clear the VMCB Clean bits to zero in these cases may result in undefined behavior.
> > > >
> > > > Suppose vmcb02 runs on pCPU A, migrates to (and runs on) pCPU B, and
> > > > then migrates back to pCPU A with some clean bits set. At the next
> > > > VMRUN, pCPU A will happily load stale state for anything marked clean.
> > > >
> > > > At some point before that VMRUN, something has to clear the clean bits!
> > >
> > > Dirtying both VMCBs is not needed in all cases covered with the new
> > > code here. IIUC, clean bits are VMCB-specific, unlike the ASID here
> > > which is shared.
> > >
> > > Because ASID is shared, we have to reset the ASID generation for both
> > > VMCBs when a new ASID is allocated through either, and update the cpu
> > > variable tracking which pCPU allocated that ASID. Since that cpu
> > > variable is also shared for tracking which pCPU ran the VMCB last, we
> > > also need to clear the clean bits every time we updated. IOW, if we
> > > had two variables, asid_cpu and vmcb_dirty_cpu, we would need to
> > > update asid_cpu on pCPU migration for both VMCBs, but only update
> > > vmcb_run_cpu for the current VMCB.
> > >
> > > This is all annoying and confusing, but that should go away in the
> > > static ASIDs series, and we should go back to only updating pCPU
> > > tracking, flushing the ASID, and clearing the clean bits only for the
> > > current VMCB.
> >
> > I haven't read the static ASIDs series, but in any case, this patch
> > should stand on its own, in case that series never gets committed or
> > gets committed much later.
>
> It does. I just didn't spend time trying to improve things beyond the fix.
>
> >
> > My complaint is with the claim, "dirtying both VMCBs is not strictly
> > needed," which seems to imply that dirtying the inactive VMCB here is
> > optional. In fact, once you update vmcb02.cpu/vmcb01.cpu for both
> > VMCBs, dirtying both *is* strictly needed so the inactive VMCB doesn't
> > later miss a pCPU switch.
>
> Yeah I guess it can be verbed more accurately, something like this:

I don't understand your prevarication. The comment, as written, isn't
"inaccurately verbed." It's wrong. With the current implementation,
dirtying both VMCBs *is* strictly needed.

> Dirtying both VMCBs is generally not required if only one VMCB
> switches pCPUs, but since the pCPU tracking is shared between ASID and
> VMCB clean bits, it must be done once pCPU tracking is updated.

Clear as mud. :)

Re: [PATCH] KVM: SVM: Update tracked pCPU on pCPU switch

[Yosry Ahmed]

On Tue, Oct 13, 2026 at 12:00 PM Jim Mattson <jmattson@google.com> wrote:
>
> On Tue, Oct 13, 2026 at 11:11 AM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Tue, Oct 13, 2026 at 10:55 AM Jim Mattson <jmattson@google.com> wrote:
> > >
> > > On Tue, Oct 13, 2026 at 7:49 AM Yosry Ahmed <yosry@kernel.org> wrote:
> > > >
> > > > On Mon, Oct 12, 2026 at 4:58 PM Jim Mattson <jmattson@google.com> wrote:
> > > > >
> > > > > On Mon, Oct 12, 2026 at 3:47 PM Yosry Ahmed <yosry@kernel.org> wrote:
> > > [ ... ]
> > > > > > @@ -3892,13 +3893,22 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
> > > > > >          * - 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.
> > > > > > +        *
> > > > > > +        * Note that dirtying both VMCBs is not strictly needed, but the per-CPU
> > > > > > +        * tracking is shared for both ASID and clean bits, and that CPU must
> > > > > > +        * match where the ASID was allocated.
> > > > >
> > > > > Is dirtying both VMCBs not strictly needed?
> > > > >
> > > > > Per APM vol. 2 (rev. 3.45), section 15.15.2:
> > > > >
> > > > > > The hypervisor must clear the entire VMCB [Clean] field to 0 for a guest, under the following circumstances:
> > > > > > ...
> > > > > >  - The hypervisor executes the guest on a different CPU core than one used the last time that guest was executed.
> > > > > > ...
> > > > > > Failure to clear the VMCB Clean bits to zero in these cases may result in undefined behavior.
> > > > >
> > > > > Suppose vmcb02 runs on pCPU A, migrates to (and runs on) pCPU B, and
> > > > > then migrates back to pCPU A with some clean bits set. At the next
> > > > > VMRUN, pCPU A will happily load stale state for anything marked clean.
> > > > >
> > > > > At some point before that VMRUN, something has to clear the clean bits!
> > > >
> > > > Dirtying both VMCBs is not needed in all cases covered with the new
> > > > code here. IIUC, clean bits are VMCB-specific, unlike the ASID here
> > > > which is shared.
> > > >
> > > > Because ASID is shared, we have to reset the ASID generation for both
> > > > VMCBs when a new ASID is allocated through either, and update the cpu
> > > > variable tracking which pCPU allocated that ASID. Since that cpu
> > > > variable is also shared for tracking which pCPU ran the VMCB last, we
> > > > also need to clear the clean bits every time we updated. IOW, if we
> > > > had two variables, asid_cpu and vmcb_dirty_cpu, we would need to
> > > > update asid_cpu on pCPU migration for both VMCBs, but only update
> > > > vmcb_run_cpu for the current VMCB.
> > > >
> > > > This is all annoying and confusing, but that should go away in the
> > > > static ASIDs series, and we should go back to only updating pCPU
> > > > tracking, flushing the ASID, and clearing the clean bits only for the
> > > > current VMCB.
> > >
> > > I haven't read the static ASIDs series, but in any case, this patch
> > > should stand on its own, in case that series never gets committed or
> > > gets committed much later.
> >
> > It does. I just didn't spend time trying to improve things beyond the fix.
> >
> > >
> > > My complaint is with the claim, "dirtying both VMCBs is not strictly
> > > needed," which seems to imply that dirtying the inactive VMCB here is
> > > optional. In fact, once you update vmcb02.cpu/vmcb01.cpu for both
> > > VMCBs, dirtying both *is* strictly needed so the inactive VMCB doesn't
> > > later miss a pCPU switch.
> >
> > Yeah I guess it can be verbed more accurately, something like this:
>
> I don't understand your prevarication. The comment, as written, isn't
> "inaccurately verbed." It's wrong. With the current implementation,
> dirtying both VMCBs *is* strictly needed.

Yeah I am just trying to clarify that ASID is shared, so we must flush
both VMCBs, but the clean bits aren't, we are only clearing them
because of the shared pCPU tracking. That being said, no comment is
better than a confusing comment, so I can just drop the note at the
end of the comment.

>
> > Dirtying both VMCBs is generally not required if only one VMCB
> > switches pCPUs, but since the pCPU tracking is shared between ASID and
> > VMCB clean bits, it must be done once pCPU tracking is updated.
>
> Clear as mud. :)
>