Re: [PATCH v2] LoongArch: KVM: Allow to set pv_feature until vCPU run
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-10 02:37:29
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] A per-vCPU guard (`has_run`) is incorrectly used to protect a VM-wide state (`kvm->arch.pv_features`), allowing runtime mutation and conflicting configurations across vCPUs. -- commit c74c65cf1934e410a4858855575161a3136ee9d8 Author: Bibo Mao [off-list ref] LoongArch: KVM: Allow to set pv_feature until vCPU run This commit modifies the KVM LoongArch PV feature configuration so that features can be configured multiple times until the vCPU has run, which helps with VM migration.
quoted hunk ↗ jump to hunk
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c index 8e028be3f0a97..dbf87ac5252b6 100644 --- a/arch/loongarch/kvm/vcpu.c +++ b/arch/loongarch/kvm/vcpu.c@@ -1166,12 +1166,17 @@ static int kvm_loongarch_cpucfg_set_attr(struct kvm_vcpu *vcpu, /* All vCPUs need set the same PV features */ spin_lock(&kvm->arch.pv_setting_lock); - if ((kvm->arch.pv_features & LOONGARCH_PV_FEAT_UPDATED) - && ((kvm->arch.pv_features & valid) != val)) { + if ((kvm->arch.pv_features & valid) == val) { + spin_unlock(&kvm->arch.pv_setting_lock); + return 0; + }
[Severity: High] Can conflicting configurations silently overwrite the shared kvm->arch.pv_features without returning an error here, given the removed LOONGARCH_PV_FEAT_UPDATED cross-vCPU enforcement logic?
quoted hunk ↗ jump to hunk
+ + if (vcpu->arch.has_run) { spin_unlock(&kvm->arch.pv_setting_lock); return -EINVAL; }
[Severity: High] Since kvm->arch.pv_features is a VM-wide state, is it safe to use the per-vCPU guard vcpu->arch.has_run here in kvm_loongarch_cpucfg_set_attr()? If a userspace process configures VCPU0 with features A and starts it (vcpu0->arch.has_run becomes true), and while VCPU0 is running, userspace configures VCPU1 (which has not run yet) with conflicting features B: Because vcpu1->arch.has_run is false, wouldn't the ioctl succeed and overwrite kvm->arch.pv_features with B? This would override VCPU0's configuration and mutate the shared state while VCPU0 is running, which can lead to a state-machine mismatch and data race since kvm->arch.pv_features is accessed locklessly by running vCPUs.
quoted hunk ↗ jump to hunk
- kvm->arch.pv_features = val | LOONGARCH_PV_FEAT_UPDATED; + + kvm->arch.pv_features = val; spin_unlock(&kvm->arch.pv_setting_lock); return 0;
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260910022118.2339780-1-maobibo@loongson.cn?part=1