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

[Jim Mattson]

On Tue, Oct 13, 2026 at 7:49 AM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
>
> On Mon, Oct 12, 2026 at 4:58 PM Jim Mattson <jmattson@xxxxxxxxxx> wrote:
> >
> > On Mon, Oct 12, 2026 at 3:47 PM Yosry Ahmed <yosry@xxxxxxxxxx> 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.