mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nikunj A Dadhania <nikunj@amd.com>
To: Yosry Ahmed <yosry@kernel.org>, Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, <kvm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <stable@vger.kernel.org>,
	Stefan Teodorescu <fane@google.com>
Subject: Re: [PATCH v2] KVM: SVM: Trigger new ASID allocation in both VMCBs on pCPU switch
Date: Fri, 25 Sep 2026 12:03:03 +0530	[thread overview]
Message-ID: <f0ff1193-8612-4eee-9997-4ef3ec215977@amd.com> (raw)
In-Reply-To: <20260903223258.486034-1-yosry@kernel.org>

On 9/4/2026 4:02 AM, Yosry Ahmed wrote:
> When the pCPU where the VMCB was mostly recently used is switched, reset

s/mostly recently/most recently/

> 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 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.
> 
> 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>
> ---
> 
> v1 -> v2:
> - Make resetting asid_generation in vmcb02 unconditional (Sean).
> 
> ---
>  arch/x86/kvm/svm/nested.c |  1 +
>  arch/x86/kvm/svm/svm.c    | 19 +++++++++++++++----
>  2 files changed, 16 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..762b6059eda31 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3765,14 +3765,25 @@ 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.
>  	 */

Nit, the example duplicates the one in the change log. How about dropping
the bullet list here and depend on the change log for example scenario?

Either way:

Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>

>  	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;
> +		svm->nested.vmcb02.asid_generation = 0;
>          }
>  
>  	if (is_sev_guest(vcpu))
> 
> base-commit: 76671054f9a1ff6abb976583cd8da37650acdc97


      reply	other threads:[~2026-09-25  6:33 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-03 22:32 Yosry Ahmed
2026-09-25  6:33 ` Nikunj A Dadhania [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f0ff1193-8612-4eee-9997-4ef3ec215977@amd.com \
    --to=nikunj@amd.com \
    --cc=fane@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=stable@vger.kernel.org \
    --cc=yosry@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®