Re: [PATCH v3 3/3] RISC-V: KVM: fix vcpu vector context handling for kernel-mode vector

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Non-atomic read-modify-write on thread flags leads to double-flush and guest vector state corruption.
--

--- Patch [3]: [PATCH v3 3/3] RISC-V: KVM: fix vcpu vector context handling for kernel-mode vector ---
commit a8a50591677d45441201aae73b3e95504bcf9fd4
Author: Andy Chiu <tchiu@tenstorrent.com>

RISC-V: KVM: fix vcpu vector context handling for kernel-mode vector

Running vector workloads like perf + mcf on KVM can result in an
unexpected termination due to a vtype corruption. This happens because
the kernel-mode vector (KMV) misattributes the guest's vcpu context as
the user's context and source from a wrong status.VS.

> diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
> index f76e52de11178..be0f6c7f78da2 100644
> --- a/arch/riscv/kernel/kernel_mode_vector.c
> +++ b/arch/riscv/kernel/kernel_mode_vector.c

[ ... ]

> @@ -147,13 +178,10 @@ static int riscv_v_start_kernel_context(void)
>  	}
>  
>  	/* Transfer the ownership of V from user to kernel, then save */
> -	riscv_v_start(RISCV_PREEMPT_V | RISCV_PREEMPT_V_DIRTY);
> -	if (__riscv_v_vstate_check(task_pt_regs(current)->status, DIRTY)) {
> -		uvstate = &current->thread.vstate;
> -		__riscv_v_vstate_save(uvstate, uvstate->datap);
> -	}
> -	riscv_preempt_v_clear_dirty(current);
> -	riscv_v_vstate_set_restore(current, task_pt_regs(current));
> +	get_cpu_vector_context();
> +	__riscv_flush_vector_context();
> +	riscv_v_start(RISCV_PREEMPT_V);
> +	put_cpu_vector_context();

[Severity: Critical]
Can a race condition occur here if a hardware interrupt is triggered?

It looks like get_cpu_vector_context() performs a non-atomic read-modify-write
of the thread flags (via riscv_v_start()). If a vector-using hardware interrupt
occurs right after the read, the interrupt handler could successfully flush the
guest vector state and clobber the hardware vector registers with host data.

When the interrupt returns, would this VCPU thread write back the stale
riscv_v_flags, and then proceed to call __riscv_flush_vector_context() a
second time?

[ ... ]

> diff --git a/arch/riscv/kvm/vcpu_vector.c b/arch/riscv/kvm/vcpu_vector.c
> index 62d2fb77bb9b9..ef2eee6ec3086 100644
> --- a/arch/riscv/kvm/vcpu_vector.c
> +++ b/arch/riscv/kvm/vcpu_vector.c

[ ... ]

> +void kvm_riscv_vcpu_flush_vector(void)
> +{
> +	struct kvm_vcpu *vcpu = *this_cpu_ptr(kvm_get_running_vcpus());
> +
> +	/*
> +	 * Only reached from __riscv_flush_vector_context() when RISCV_V_VCPU_CTX is set, which
> +	 * always have kvm_get_running_vcpus non-NULL.
> +	 */
> +	if (WARN_ON_ONCE(!vcpu))
> +		return;
> +
> +	kvm_riscv_vcpu_guest_vector_save(&vcpu->arch.guest_context, vcpu->arch.isa);

[Severity: Critical]
If kvm_riscv_vcpu_flush_vector() is executed a second time due to the race
described above, could this blindly save the interrupt-clobbered hardware
registers over the valid guest state in memory?

This seems like it might leak host vector data to the guest and completely
corrupt the guest's vector state.

> +
> +	if ((vcpu->arch.guest_context.sstatus & SR_VS) != SR_VS_OFF)
> +		riscv_v_flags_set(riscv_v_flags() | RISCV_V_VCPU_NEED_RESTORE);
> +}
> +
>  #endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724165001.2317788-1-tchiu@tenstorrent.com?part=3