Re: [PATCH v2] RISC-V: KVM: Avoid synchronous IPIs on VMID rollover

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Race condition during VMID rollover allows vCPU to enter guest with stale hardware VMID, breaking memory isolation.
--

commit f9683a31ea50a94b9a73a513d36b5718f5b167c1
Author: Wenlong Li <wenlongli486@gmail.com>
RISC-V: KVM: Avoid synchronous IPIs on VMID rollover

This commit makes VMID rollover lazy to avoid synchronous cross-CPU IPIs on
all online CPUs. It tracks active and reserved software VMIDs, performing
pending local HFENCE.GVMAs on the next VMID activation before guest entry.

> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index e062ca19f9d8f..85542c1a5ec3f 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -943,14 +943,14 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>  			continue;
>  		ret = 1;
>  
> -		kvm_riscv_gstage_vmid_update(vcpu);
> -
>  		ret = kvm_riscv_check_vcpu_requests(vcpu);
>  		if (ret <= 0)
>  			continue;
>  
>  		preempt_disable();
>  
> +		kvm_riscv_gstage_vmid_update(vcpu);
> +

[Severity: Critical]
Does moving kvm_riscv_gstage_vmid_update() after kvm_riscv_check_vcpu_requests()
create a window where a vCPU misses a pending request?

Later in kvm_arch_vcpu_ioctl_run(), the vCPU checks for pending requests:

    if (kvm_riscv_gstage_vmid_ver_changed(&vcpu->kvm->arch.vmid) ||
        kvm_request_pending(vcpu) ||
        xfer_to_guest_mode_work_pending()) {
        vcpu->mode = OUTSIDE_GUEST_MODE;

If another vCPU updates the shared VMID generation locklessly,
kvm_request_pending() can evaluate to false here if the KVM_REQ_UPDATE_HGATP
request from the other vCPU has not yet been written. This would allow the
vCPU to enter the guest with a stale hardware VMID.

>  		/* Update AIA HW state before entering guest */
>  		ret = kvm_riscv_vcpu_aia_update(vcpu);

[ ... ]

> diff --git a/arch/riscv/kvm/vmid.c b/arch/riscv/kvm/vmid.c
> index c15bdb1dd8bef..7b86a6fbe4a69 100644
> --- a/arch/riscv/kvm/vmid.c
> +++ b/arch/riscv/kvm/vmid.c
> @@ -201,31 +201,31 @@ void kvm_riscv_gstage_vmid_update(struct kvm_vcpu *vcpu)

[ ... ]

> +	/*
> +	 * Publish the CPU-local active identity only after the required
> +	 * local invalidation is complete.
> +	 */
> +	atomic64_set(active, vmid);
> +
> +	raw_spin_unlock_irqrestore(&vmid_lock, flags);

[Severity: Critical]
Is it safe to update the admission state and drop the lock here before 
issuing the HGATP update request?

If the current vCPU drops the lock, another vCPU can observe the new 
generation, update its own admission state, and bypass the 
KVM_REQ_UPDATE_HGATP check since the request has not yet been made. The 
second vCPU would then proceed to enter the guest.

> +
> +	/*
> +	 * A new software VMID may contain a different hardware VMID, so
> +	 * request an HGATP update for every vCPU in the VM.
> +	 */
> +	if (vmid_changed) {
> +		kvm_for_each_vcpu(i, v, vcpu->kvm)
> +			kvm_make_request(KVM_REQ_UPDATE_HGATP, v);
>  	}

[Severity: Critical]
Since this cross-vCPU request is issued locklessly, can this cause a 
regression where two VMs might share the same hardware VMID?

Because the second vCPU mentioned above advertises the new VMID in its 
admission state but still runs with the old hardware VMID in the guest, the 
old hardware VMID is no longer protected. It could be immediately 
reallocated to another VM, breaking spatial isolation.

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