[PATCH v2] LoongArch: KVM: Allow to set pv_feature until vCPU run

Subsystems: kernel virtual machine for loongarch (kvm/loongarch), loongarch, the rest

COOLING7d

3 messages, 3 authors, 7d ago · open the first message on its own page

[PATCH v2] LoongArch: KVM: Allow to set pv_feature until vCPU run

From: Bibo Mao <maobibo@loongson.cn>
Date: 2026-09-10 02:28:40 

Now pv_feature can be set only once, there is problem with VM migration.
Where it is set when vCPU is created and after migration, here it is
allowed to set for many times, until vCPU starts to run.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
v1 ... v2:
  1. Rename ran_atleast_once with has_run for simplity.
  2. Rebase on the latest version where there is pv_setting_lock
     protection when modify pv_features of VM.
---
 arch/loongarch/include/asm/kvm_host.h |  3 ++-
 arch/loongarch/kvm/vcpu.c             | 15 ++++++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
index 65d91c3ce313..4ee1a29a5ea0 100644
--- a/arch/loongarch/include/asm/kvm_host.h
+++ b/arch/loongarch/include/asm/kvm_host.h
@@ -164,7 +164,6 @@ enum emulation_result {
 #define KVM_LARCH_SWCSR_LATEST	(0x1 << 3)
 #define KVM_LARCH_HWCSR_USABLE	(0x1 << 4)
 
-#define LOONGARCH_PV_FEAT_UPDATED	BIT_ULL(63)
 #define LOONGARCH_PV_FEAT_MASK		(BIT(KVM_FEATURE_IPI) |		\
 					 BIT(KVM_FEATURE_PREEMPT) |	\
 					 BIT(KVM_FEATURE_STEAL_TIME) |	\
@@ -250,6 +249,8 @@ struct kvm_vcpu_arch {
 	struct dmsintc_state dmsintc_state;
 	/* cpucfg */
 	u32 cpucfg[KVM_MAX_CPUCFG_REGS];
+	/* VCPU ran at least once */
+	bool has_run;
 
 	/* paravirt steal time */
 	struct {
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 8e028be3f0a9..dbf87ac5252b 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;
+		}
+
+		if (vcpu->arch.has_run) {
 			spin_unlock(&kvm->arch.pv_setting_lock);
 			return -EINVAL;
 		}
-		kvm->arch.pv_features = val | LOONGARCH_PV_FEAT_UPDATED;
+
+		kvm->arch.pv_features = val;
 		spin_unlock(&kvm->arch.pv_setting_lock);
 		return 0;
 	default:
@@ -1858,6 +1863,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 	int r = -EINTR;
 	struct kvm_run *run = vcpu->run;
 
+	/* Mark this VCPU ran at least once */
+	if (!vcpu->arch.has_run)
+		vcpu->arch.has_run = true;
+
 	if (vcpu->mmio_needed) {
 		if (!vcpu->mmio_is_write)
 			kvm_complete_mmio_read(vcpu, run);

base-commit: df2908090cda368b01ff43709f51890076c56157
-- 
2.39.3

Re: [PATCH v2] LoongArch: KVM: Allow to set pv_feature until vCPU run

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
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
+
+		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
-		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

Re: [PATCH v2] LoongArch: KVM: Allow to set pv_feature until vCPU run

From: Huacai Chen <chenhuacai@kernel.org>
Date: 2026-10-03 04:02:28 

Applied, thanks.


Huacai

On Thu, Sep 10, 2026 at 10:28 AM Bibo Mao [off-list ref] wrote:
quoted hunk
Now pv_feature can be set only once, there is problem with VM migration.
Where it is set when vCPU is created and after migration, here it is
allowed to set for many times, until vCPU starts to run.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
v1 ... v2:
  1. Rename ran_atleast_once with has_run for simplity.
  2. Rebase on the latest version where there is pv_setting_lock
     protection when modify pv_features of VM.
---
 arch/loongarch/include/asm/kvm_host.h |  3 ++-
 arch/loongarch/kvm/vcpu.c             | 15 ++++++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
index 65d91c3ce313..4ee1a29a5ea0 100644
--- a/arch/loongarch/include/asm/kvm_host.h
+++ b/arch/loongarch/include/asm/kvm_host.h
@@ -164,7 +164,6 @@ enum emulation_result {
 #define KVM_LARCH_SWCSR_LATEST (0x1 << 3)
 #define KVM_LARCH_HWCSR_USABLE (0x1 << 4)

-#define LOONGARCH_PV_FEAT_UPDATED      BIT_ULL(63)
 #define LOONGARCH_PV_FEAT_MASK         (BIT(KVM_FEATURE_IPI) |         \
                                         BIT(KVM_FEATURE_PREEMPT) |     \
                                         BIT(KVM_FEATURE_STEAL_TIME) |  \
@@ -250,6 +249,8 @@ struct kvm_vcpu_arch {
        struct dmsintc_state dmsintc_state;
        /* cpucfg */
        u32 cpucfg[KVM_MAX_CPUCFG_REGS];
+       /* VCPU ran at least once */
+       bool has_run;

        /* paravirt steal time */
        struct {
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 8e028be3f0a9..dbf87ac5252b 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;
+               }
+
+               if (vcpu->arch.has_run) {
                        spin_unlock(&kvm->arch.pv_setting_lock);
                        return -EINVAL;
                }
-               kvm->arch.pv_features = val | LOONGARCH_PV_FEAT_UPDATED;
+
+               kvm->arch.pv_features = val;
                spin_unlock(&kvm->arch.pv_setting_lock);
                return 0;
        default:
@@ -1858,6 +1863,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
        int r = -EINTR;
        struct kvm_run *run = vcpu->run;

+       /* Mark this VCPU ran at least once */
+       if (!vcpu->arch.has_run)
+               vcpu->arch.has_run = true;
+
        if (vcpu->mmio_needed) {
                if (!vcpu->mmio_is_write)
                        kvm_complete_mmio_read(vcpu, run);

base-commit: df2908090cda368b01ff43709f51890076c56157
--
2.39.3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help