Re: [PATCH v7 1/6] riscv: vector: refactor vector context operations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] riscv: kvm: unrenamed __kvm_riscv_vector_restore() stub breaks
  !ISA_V build
- [Low] riscv: vector: commit message describes preempt_v changes not in
  diff
--

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

riscv: vector: refactor vector context operations

This commit lifts riscv_v_enable() and riscv_v_disable() out of the
low-level __riscv_v_vstate_{save,restore,discard}() helpers into their
callers and renames the KVM vector save/restore helpers.

>     Also, refactor and document about the user context save in preempt_v to
>     make code more readable.

[Severity: Low]
This isn't a bug, but the preempt_v refactoring and comment described here
are not in this patch; they were already included in commit b5060a4aa33d
("RISC-V: KVM: fix vcpu vector context handling for kernel-mode vector").

> diff --git a/arch/riscv/include/asm/kvm_vcpu_vector.h b/arch/riscv/include/asm/kvm_vcpu_vector.h
> index 50d4789b9cac9..60fa280309444 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_vector.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_vector.h
> @@ -16,14 +16,18 @@
>  #ifdef CONFIG_RISCV_ISA_V
>  #include <asm/kvm_host.h>
>  
> -static __always_inline void __kvm_riscv_vector_save(struct kvm_cpu_context *context)
> +static __always_inline void kvm_riscv_vector_save(struct kvm_cpu_context *context)
>  {
> +	riscv_v_enable();
>  	__riscv_v_vstate_save(&context->vector, context->vector.datap);
> +	riscv_v_disable();
>  }
>  
> -static __always_inline void __kvm_riscv_vector_restore(struct kvm_cpu_context *context)
> +static __always_inline void kvm_riscv_vector_restore(struct kvm_cpu_context *context)
>  {
> +	riscv_v_enable();
>  	__riscv_v_vstate_restore(&context->vector, context->vector.datap);
> +	riscv_v_disable();
>  }

[ ... ]

> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index dd786f758eb76..caf6e0ad72efd 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -825,7 +825,7 @@ static void noinstr kvm_riscv_vcpu_enter_exit(struct kvm_vcpu *vcpu,
>  	if (current->thread.riscv_v_flags & RISCV_V_VCPU_NEED_RESTORE) {
>  		current->thread.riscv_v_flags &= ~RISCV_V_VCPU_NEED_RESTORE;
>  		current->thread.riscv_v_flags |= RISCV_V_VCPU_CTX;
> -		__kvm_riscv_vector_restore(gcntx);
> +		kvm_riscv_vector_restore(gcntx);
>  		gcntx->sstatus = (gcntx->sstatus & ~SR_VS) | SR_VS_CLEAN;
>  	}

[Severity: High]
Does this break the build when CONFIG_KVM is enabled and CONFIG_RISCV_ISA_V
is disabled?  The stub in the #else branch of
arch/riscv/include/asm/kvm_vcpu_vector.h is still named
__kvm_riscv_vector_restore(), so kvm_riscv_vcpu_enter_exit() calls an
undeclared function.

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