Re: [PATCH v2 11/29] KVM: SVM: Only flush the fallback ASID when used by a different vCPU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] kvm: data race on sd->fallback_asid_vcpu between pre_svm_run and svm_vcpu_free
--

commit aa8eca89b8d60f642d17b3f8439f52f2e1ae951d
Author: Yosry Ahmed <yosry@kernel.org>

KVM: SVM: Only flush the fallback ASID when used by a different vCPU

This commit optimizes the fallback ASID flushing logic by only flushing
when the last vCPU that used the fallback ASID on the pCPU changes. It
adds a per-CPU tracker fallback_asid_vcpu to determine when this occurs.

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 8f27722bc4faf..0dbb1962d32b6 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c

[ ... ]

> @@ -1422,6 +1424,11 @@ static void svm_vcpu_free(struct kvm_vcpu *vcpu)
>  	svm_vcpu_free_msrpm(svm->msrpm);
>  
>  	free_asid(svm->asid);
> +
> +	for_each_possible_cpu(cpu) {
> +		sd = per_cpu_ptr(&svm_data, cpu);
> +		cmpxchg(&sd->fallback_asid_vcpu, vcpu, NULL);
> +	}
>  }

[ ... ]

> @@ -3884,8 +3892,11 @@ static int pre_svm_run(struct kvm_vcpu *vcpu)
>  	if (is_sev_guest(vcpu))
>  		return pre_sev_run(svm, vcpu->cpu);
>  
> -	if (unlikely(svm->vmcb->control.asid == fallback_asid))
> +	if (unlikely(svm->vmcb->control.asid == fallback_asid &&
> +		     sd->fallback_asid_vcpu != vcpu)) {
>  		vmcb_set_flush_asid(svm->vmcb);
> +		sd->fallback_asid_vcpu = vcpu;

[Severity: Low]
Will this concurrent access to sd->fallback_asid_vcpu trigger a KCSAN data
race splat?

In svm_vcpu_free(), when a vCPU is being destroyed, it iterates over all
physical CPUs and modifies sd->fallback_asid_vcpu via cmpxchg() across CPUs.

Concurrently, another vCPU running on one of those CPUs will execute
pre_svm_run() and perform plain reads and writes of its local
sd->fallback_asid_vcpu.

Per the LKMM (C11 memory model), if an atomic RMW is used on one side
of a concurrent access, at least one side must use READ_ONCE() or
WRITE_ONCE() to prevent the compiler from fusing loads or reordering
plain stores.

Should the reads and writes in pre_svm_run() use READ_ONCE() and
WRITE_ONCE() to avoid this race?

> +	}
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008001425.2458927-1-yosry@kernel.org?part=11