[PATCH] RISC-V: KVM: Use raw_spinlock for VMID update critical section

[PATCH] RISC-V: KVM: Use raw_spinlock for VMID update critical section

[yhchen312]

From: "Yuhang.chen" <yhchen312@gmail.com>

The VMID update critical section performs an IPI broadcast via
on_each_cpu_mask() on VMID version rollover, which must not be preempted
under PREEMPT_RT. Convert vmid_lock to raw_spinlock_t and use plain
raw_spin_lock()/raw_spin_unlock() (interrupts stay enabled, since
on_each_cpu_mask() requires it).

Assisted-by: YuanSheng:deepseek-v4-pro
Co-developed-by: Quan Zhou <zhouquan@iscas.ac.cn>
Signed-off-by: Quan Zhou <zhouquan@iscas.ac.cn>
Signed-off-by: Yuhang.chen <yhchen312@gmail.com>
---
 arch/riscv/kvm/vmid.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kvm/vmid.c b/arch/riscv/kvm/vmid.c
index c15bdb1dd8be..21c2b276b55d 100644
--- a/arch/riscv/kvm/vmid.c
+++ b/arch/riscv/kvm/vmid.c
@@ -21,7 +21,7 @@
 static unsigned long vmid_version = 1;
 static unsigned long vmid_next;
 static unsigned long vmid_bits __ro_after_init;
-static DEFINE_SPINLOCK(vmid_lock);
+static DEFINE_RAW_SPINLOCK(vmid_lock);
 
 void __init kvm_riscv_gstage_vmid_detect(void)
 {
@@ -78,14 +78,14 @@ void kvm_riscv_gstage_vmid_update(struct kvm_vcpu *vcpu)
 	if (!kvm_riscv_gstage_vmid_ver_changed(vmid))
 		return;
 
-	spin_lock(&vmid_lock);
+	raw_spin_lock(&vmid_lock);
 
 	/*
 	 * We need to re-check the vmid_version here to ensure that if
 	 * another vcpu already allocated a valid vmid for this vm.
 	 */
 	if (!kvm_riscv_gstage_vmid_ver_changed(vmid)) {
-		spin_unlock(&vmid_lock);
+		raw_spin_unlock(&vmid_lock);
 		return;
 	}
 
@@ -117,7 +117,7 @@ void kvm_riscv_gstage_vmid_update(struct kvm_vcpu *vcpu)
 
 	WRITE_ONCE(vmid->vmid_version, READ_ONCE(vmid_version));
 
-	spin_unlock(&vmid_lock);
+	raw_spin_unlock(&vmid_lock);
 
 	/* Request G-stage page table update for all VCPUs */
 	kvm_for_each_vcpu(i, v, vcpu->kvm)
-- 
2.34.1

[PATCH] RISC-V: KVM: Use raw_spinlock for VMID update critical section

[yhchen312]

From: "Yuhang.chen" <yhchen312@gmail.com>

The VMID update critical section performs an IPI broadcast via
on_each_cpu_mask() on VMID version rollover, which must not be preempted
under PREEMPT_RT. Convert vmid_lock to raw_spinlock_t and use plain
raw_spin_lock()/raw_spin_unlock() (interrupts stay enabled, since
on_each_cpu_mask() requires it).

Assisted-by: YuanSheng:deepseek-v4-pro
Co-developed-by: Quan Zhou <zhouquan@iscas.ac.cn>
Signed-off-by: Quan Zhou <zhouquan@iscas.ac.cn>
Signed-off-by: Yuhang.chen <yhchen312@gmail.com>
---
 arch/riscv/kvm/vmid.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kvm/vmid.c b/arch/riscv/kvm/vmid.c
index c15bdb1dd8be..21c2b276b55d 100644
--- a/arch/riscv/kvm/vmid.c
+++ b/arch/riscv/kvm/vmid.c
@@ -21,7 +21,7 @@
 static unsigned long vmid_version = 1;
 static unsigned long vmid_next;
 static unsigned long vmid_bits __ro_after_init;
-static DEFINE_SPINLOCK(vmid_lock);
+static DEFINE_RAW_SPINLOCK(vmid_lock);
 
 void __init kvm_riscv_gstage_vmid_detect(void)
 {
@@ -78,14 +78,14 @@ void kvm_riscv_gstage_vmid_update(struct kvm_vcpu *vcpu)
 	if (!kvm_riscv_gstage_vmid_ver_changed(vmid))
 		return;
 
-	spin_lock(&vmid_lock);
+	raw_spin_lock(&vmid_lock);
 
 	/*
 	 * We need to re-check the vmid_version here to ensure that if
 	 * another vcpu already allocated a valid vmid for this vm.
 	 */
 	if (!kvm_riscv_gstage_vmid_ver_changed(vmid)) {
-		spin_unlock(&vmid_lock);
+		raw_spin_unlock(&vmid_lock);
 		return;
 	}
 
@@ -117,7 +117,7 @@ void kvm_riscv_gstage_vmid_update(struct kvm_vcpu *vcpu)
 
 	WRITE_ONCE(vmid->vmid_version, READ_ONCE(vmid_version));
 
-	spin_unlock(&vmid_lock);
+	raw_spin_unlock(&vmid_lock);
 
 	/* Request G-stage page table update for all VCPUs */
 	kvm_for_each_vcpu(i, v, vcpu->kvm)
-- 
2.34.1


-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

[PATCH] RISC-V: KVM: Use raw_spinlock for VMID update critical section

[yhchen312]

From: "Yuhang.chen" <yhchen312@gmail.com>

The VMID update critical section performs an IPI broadcast via
on_each_cpu_mask() on VMID version rollover, which must not be preempted
under PREEMPT_RT. Convert vmid_lock to raw_spinlock_t and use plain
raw_spin_lock()/raw_spin_unlock() (interrupts stay enabled, since
on_each_cpu_mask() requires it).

Assisted-by: YuanSheng:deepseek-v4-pro
Co-developed-by: Quan Zhou <zhouquan@iscas.ac.cn>
Signed-off-by: Quan Zhou <zhouquan@iscas.ac.cn>
Signed-off-by: Yuhang.chen <yhchen312@gmail.com>
---
 arch/riscv/kvm/vmid.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kvm/vmid.c b/arch/riscv/kvm/vmid.c
index c15bdb1dd8be..21c2b276b55d 100644
--- a/arch/riscv/kvm/vmid.c
+++ b/arch/riscv/kvm/vmid.c
@@ -21,7 +21,7 @@
 static unsigned long vmid_version = 1;
 static unsigned long vmid_next;
 static unsigned long vmid_bits __ro_after_init;
-static DEFINE_SPINLOCK(vmid_lock);
+static DEFINE_RAW_SPINLOCK(vmid_lock);
 
 void __init kvm_riscv_gstage_vmid_detect(void)
 {
@@ -78,14 +78,14 @@ void kvm_riscv_gstage_vmid_update(struct kvm_vcpu *vcpu)
 	if (!kvm_riscv_gstage_vmid_ver_changed(vmid))
 		return;
 
-	spin_lock(&vmid_lock);
+	raw_spin_lock(&vmid_lock);
 
 	/*
 	 * We need to re-check the vmid_version here to ensure that if
 	 * another vcpu already allocated a valid vmid for this vm.
 	 */
 	if (!kvm_riscv_gstage_vmid_ver_changed(vmid)) {
-		spin_unlock(&vmid_lock);
+		raw_spin_unlock(&vmid_lock);
 		return;
 	}
 
@@ -117,7 +117,7 @@ void kvm_riscv_gstage_vmid_update(struct kvm_vcpu *vcpu)
 
 	WRITE_ONCE(vmid->vmid_version, READ_ONCE(vmid_version));
 
-	spin_unlock(&vmid_lock);
+	raw_spin_unlock(&vmid_lock);
 
 	/* Request G-stage page table update for all VCPUs */
 	kvm_for_each_vcpu(i, v, vcpu->kvm)
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] RISC-V: KVM: Use raw_spinlock for VMID update critical section

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Race condition during VMID rollover allows a vCPU to transiently execute in guest mode with a stale VMID, compromising cross-VM isolation.
- [High] Integer overflow in the VMID generation counter (vmid_version) on 32-bit systems causes a cross-VM isolation break.
--

--- Patch [1]: [PATCH] RISC-V: KVM: Use raw_spinlock for VMID update critical section ---
commit 7d899ea43321661351b3cdd958ba47c38d85eeed
Author: Yuhang.chen <yhchen312@gmail.com>

RISC-V: KVM: Use raw_spinlock for VMID update critical section

This commit converts the vmid_lock to a raw_spinlock_t to prevent
preemption during the VMID update section under PREEMPT_RT. The update
path performs an IPI broadcast which requires preemption to be disabled.

> diff --git a/arch/riscv/kvm/vmid.c b/arch/riscv/kvm/vmid.c
> index 3b426c800480c..33efde4bb9d75 100644
> --- a/arch/riscv/kvm/vmid.c
> +++ b/arch/riscv/kvm/vmid.c
> @@ -20,7 +20,7 @@
>  static unsigned long vmid_version = 1;
>  static unsigned long vmid_next;
>  static unsigned long vmid_bits __ro_after_init;
> -static DEFINE_SPINLOCK(vmid_lock);
> +static DEFINE_RAW_SPINLOCK(vmid_lock);
>  
>  void __init kvm_riscv_gstage_vmid_detect(void)
>  {

[Severity: High]
This is a pre-existing issue, but does this unsigned long counter present an
overflow risk on 32-bit systems?

If a host user rapidly creates and destroys VMs, the 32-bit vmid_version could
wrap back to 0. Since a new VM's kvm_vmid struct is zero-initialized,
kvm_riscv_gstage_vmid_ver_changed() would evaluate (0 != 0) as false:

kvm_riscv_gstage_vmid_ver_changed() {
    ...
    return unlikely(READ_ONCE(vmid->vmid_version) !=
                    READ_ONCE(vmid_version));
}

Could this cause multiple new VMs to share VMID 0, breaking cross-VM isolation
by allowing their memory accesses to hit each other's TLB entries?

[ ... ]

> @@ -118,7 +118,7 @@ void kvm_riscv_gstage_vmid_update(struct kvm_vcpu *vcpu)
>  
>  	WRITE_ONCE(vmid->vmid_version, READ_ONCE(vmid_version));
>  
> -	spin_unlock(&vmid_lock);
> +	raw_spin_unlock(&vmid_lock);
>  
>  	/* Request G-stage page table update for all VCPUs */
>  	kvm_for_each_vcpu(i, v, vcpu->kvm)
>  		kvm_make_request(KVM_REQ_UPDATE_HGATP, v);

[Severity: Critical]
This isn't a bug introduced by this patch, but does dropping the lock before
making the requests open a race window?

If the lock is released after updating vmid_version but before setting
KVM_REQ_UPDATE_HGATP, could a concurrent vCPU from the same VM observe the
new vmid_version without the lock?

If it sees no pending requests, it might proceed to enter the guest with a
stale VMID. If a global VMID rollover just occurred, could this stale VMID
have been re-assigned to a different VM, causing an isolation break?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716064011.2387501-1-yhchen312@gmail.com?part=1