Re: [PATCH v7 6/6] riscv: KVM: do not save host vector context for vcpu

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] riscv: KVM: build failure in kvm_riscv_vcpu_enter_exit() when
  !CONFIG_RISCV_ISA_V
- [High] riscv: KVM: guest CSR_VCSR state leaks to host in
  kvm_riscv_vcpu_host_vector_restore()
--

commit e0f5ce69db9b4e847c776901421a3c6de8e57845
Author: Andy Chiu <tchiu@tenstorrent.com>

riscv: KVM: do not save host vector context for vcpu

This commit removes the per-vCPU host vector context buffer and replaces the
explicit host vector context save and restore in kvm_arch_vcpu_load() and
kvm_arch_vcpu_put() with deferred thread vector state restoration.

> diff --git a/arch/riscv/include/asm/kvm_vcpu_vector.h b/arch/riscv/include/asm/kvm_vcpu_vector.h
> index 60fa280309444..06409c870a069 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_vector.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_vector.h
[ ... ]
> @@ -77,11 +76,7 @@ static inline void kvm_riscv_vcpu_guest_vector_restore(struct kvm_cpu_context *c
>  {
>  }
>  
> -static inline void kvm_riscv_vcpu_host_vector_save(struct kvm_cpu_context *cntx)
> -{
> -}
> -
> -static inline void kvm_riscv_vcpu_host_vector_restore(struct kvm_cpu_context *cntx)
> +static inline void kvm_riscv_vcpu_host_vector_restore(void)
>  {
>  }

[Severity: High]
Does building with CONFIG_KVM enabled and CONFIG_RISCV_ISA_V disabled fail in
kvm_riscv_vcpu_enter_exit()?

Earlier in the series, commit "riscv: vector: refactor vector context
operations" renamed __kvm_riscv_vector_restore() to
kvm_riscv_vector_restore() under #ifdef CONFIG_RISCV_ISA_V and at the call
site in kvm_riscv_vcpu_enter_exit(), which is compiled unconditionally, but
the !CONFIG_RISCV_ISA_V stub in arch/riscv/include/asm/kvm_vcpu_vector.h is
still named __kvm_riscv_vector_restore().

[ ... ]

> diff --git a/arch/riscv/kvm/vcpu_vector.c b/arch/riscv/kvm/vcpu_vector.c
> index 730c32f7de504..6129a6e184049 100644
> --- a/arch/riscv/kvm/vcpu_vector.c
> +++ b/arch/riscv/kvm/vcpu_vector.c
> @@ -61,17 +61,9 @@ void kvm_riscv_vcpu_guest_vector_restore(struct kvm_cpu_context *cntx,
>  	}
>  }
>  
> -void kvm_riscv_vcpu_host_vector_save(struct kvm_cpu_context *cntx)
> +void kvm_riscv_vcpu_host_vector_restore(void)
>  {
> -	/* No need to check host sstatus as it can be modified outside */
> -	if (!kvm_riscv_isa_check_host(V))
> -		kvm_riscv_vector_save(cntx);
> -}
> -
> -void kvm_riscv_vcpu_host_vector_restore(struct kvm_cpu_context *cntx)
> -{
> -	if (!kvm_riscv_isa_check_host(V))
> -		kvm_riscv_vector_restore(cntx);
> +	riscv_v_vstate_set_restore(current, task_pt_regs(current));
>  	riscv_v_flags_set(riscv_v_flags() & ~(RISCV_V_VCPU_CTX | RISCV_V_VCPU_NEED_RESTORE));
>  }

[Severity: High]
Can the guest's CSR_VCSR value leak into host userspace after exiting the
guest?

When the host task enters ioctl(KVM_RUN) via do_trap_ecall_u(),
riscv_v_vstate_discard() sets task_pt_regs(current)->status to SR_VS_INITIAL
(or leaves it at SR_VS_OFF if the task has not used vector instructions).
While running the vCPU, kvm_riscv_vcpu_enter_exit() ->
kvm_riscv_vector_restore() loads the guest's vcsr into hardware CSR_VCSR (or
CSR_VXRM and CSR_VXSAT on XTheadVector), and the guest can also modify it
directly.

On vCPU put, kvm_riscv_vcpu_host_vector_restore() now only schedules a
deferred restore via riscv_v_vstate_set_restore().  When returning to host
userspace with SR_VS_INITIAL, riscv_v_vstate_restore() calls
__riscv_v_vstate_discard(), which resets v0..v31, vl, vtype, and vstart, but
never writes CSR_VCSR:

arch/riscv/include/asm/vector.h:riscv_v_vstate_restore() {
	...
	riscv_v_enable();
	if (__riscv_v_vstate_check(regs->status, INITIAL))
		__riscv_v_vstate_discard();
	else if (__riscv_v_vstate_check(regs->status, CLEAN))
		__riscv_v_vstate_restore(vstate, vstate->datap);
	riscv_v_disable();
	...
}

This leaves the guest's CSR_VCSR active in hardware when host userspace
resumes (and across riscv_v_ucontext_save() on signal delivery or ptrace), or
for the next task that hits __riscv_v_vstate_discard() on that CPU if the VMM
task had SR_VS_OFF.

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