Re: [[PATCH] v4 1/4] riscv: vector: refactor vector context operations

On Fri, May 29, 2026 at 12:41 AM Andy Chiu <[email protected]> wrote:
>
> Lift riscv_v_{enable,disable} out of __*vstate_{save,restore,discard} so
> that we can reuse some functions without repeatedly turning on/off
> vector.
>
> Also, refactor and document about the user context save in preempt_v to
> make code more readable.
>
> Signed-off-by: Andy Chiu <[email protected]>

For KVM RISC-V:
Acked-by: Anup Patel <[email protected]>

Regards,
Anup

> ---
> Changelog v4:
>  - fix an unused variable warning (Olof)
> Changelog v3:
>  - new patch since v3
> ---
>  arch/riscv/include/asm/kvm_vcpu_vector.h |  8 +++++--
>  arch/riscv/include/asm/vector.h          | 15 ++++++------
>  arch/riscv/kernel/kernel_mode_vector.c   | 29 +++++++++++++++++++-----
>  arch/riscv/kvm/vcpu_vector.c             |  8 +++----
>  4 files changed, 41 insertions(+), 19 deletions(-)
>
> diff --git a/arch/riscv/include/asm/kvm_vcpu_vector.h b/arch/riscv/include/asm/kvm_vcpu_vector.h
> index 57a798a4cb0d..e679869e2ba3 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_vector.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_vector.h
> @@ -16,14 +16,18 @@
>  #include <asm/vector.h>
>  #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();
>  }
>
>  void kvm_riscv_vcpu_vector_reset(struct kvm_vcpu *vcpu);
> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> index 00cb9c0982b1..45820cd900d6 100644
> --- a/arch/riscv/include/asm/vector.h
> +++ b/arch/riscv/include/asm/vector.h
> @@ -198,7 +198,6 @@ static inline void __riscv_v_vstate_save(struct __riscv_v_ext_state *save_to,
>  {
>         unsigned long vl;
>
> -       riscv_v_enable();
>         __vstate_csr_save(save_to);
>         if (has_xtheadvector()) {
>                 asm volatile (
> @@ -227,7 +226,6 @@ static inline void __riscv_v_vstate_save(struct __riscv_v_ext_state *save_to,
>                         ".option pop\n\t"
>                         : "=&r" (vl) : "r" (datap) : "memory");
>         }
> -       riscv_v_disable();
>  }
>
>  static inline void __riscv_v_vstate_restore(struct __riscv_v_ext_state *restore_from,
> @@ -235,7 +233,6 @@ static inline void __riscv_v_vstate_restore(struct __riscv_v_ext_state *restore_
>  {
>         unsigned long vl;
>
> -       riscv_v_enable();
>         if (has_xtheadvector()) {
>                 asm volatile (
>                         "mv t0, %0\n\t"
> @@ -264,14 +261,12 @@ static inline void __riscv_v_vstate_restore(struct __riscv_v_ext_state *restore_
>                         : "=&r" (vl) : "r" (datap) : "memory");
>         }
>         __vstate_csr_restore(restore_from);
> -       riscv_v_disable();
>  }
>
>  static inline void __riscv_v_vstate_discard(void)
>  {
>         unsigned long vl, vtype_inval = 1UL << (BITS_PER_LONG - 1);
>
> -       riscv_v_enable();
>         if (has_xtheadvector())
>                 asm volatile (THEAD_VSETVLI_T4X0E8M8D1 : : : "t4");
>         else
> @@ -291,14 +286,14 @@ static inline void __riscv_v_vstate_discard(void)
>                 "vsetvl         %0, x0, %1\n\t"
>                 ".option pop\n\t"
>                 : "=&r" (vl) : "r" (vtype_inval));
> -
> -       riscv_v_disable();
>  }
>
>  static inline void riscv_v_vstate_discard(struct pt_regs *regs)
>  {
>         if (riscv_v_vstate_query(regs)) {
> +               riscv_v_enable();
>                 __riscv_v_vstate_discard();
> +               riscv_v_disable();
>                 __riscv_v_vstate_dirty(regs);
>         }
>  }
> @@ -307,7 +302,9 @@ static inline void riscv_v_vstate_save(struct __riscv_v_ext_state *vstate,
>                                        struct pt_regs *regs)
>  {
>         if (__riscv_v_vstate_check(regs->status, DIRTY)) {
> +               riscv_v_enable();
>                 __riscv_v_vstate_save(vstate, vstate->datap);
> +               riscv_v_disable();
>                 __riscv_v_vstate_clean(regs);
>         }
>  }
> @@ -316,7 +313,9 @@ static inline void riscv_v_vstate_restore(struct __riscv_v_ext_state *vstate,
>                                           struct pt_regs *regs)
>  {
>         if (riscv_v_vstate_query(regs)) {
> +               riscv_v_enable();
>                 __riscv_v_vstate_restore(vstate, vstate->datap);
> +               riscv_v_disable();
>                 __riscv_v_vstate_clean(regs);
>         }
>  }
> @@ -378,8 +377,10 @@ static inline void __switch_to_vector(struct task_struct *prev,
>                         prev->thread.riscv_v_flags |= RISCV_PREEMPT_V_IN_SCHEDULE;
>                 }
>                 if (riscv_preempt_v_dirty(prev)) {
> +                       riscv_v_enable();
>                         __riscv_v_vstate_save(&prev->thread.kernel_vstate,
>                                               prev->thread.kernel_vstate.datap);
> +                       riscv_v_disable();
>                         riscv_preempt_v_clear_dirty(prev);
>                 }
>         } else {
> diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
> index 99972a48e86b..21127b33fea8 100644
> --- a/arch/riscv/kernel/kernel_mode_vector.c
> +++ b/arch/riscv/kernel/kernel_mode_vector.c
> @@ -123,7 +123,7 @@ static int riscv_v_stop_kernel_context(void)
>
>  static int riscv_v_start_kernel_context(bool *is_nested)
>  {
> -       struct __riscv_v_ext_state *kvstate, *uvstate;
> +       struct __riscv_v_ext_state *kvstate;
>
>         kvstate = &current->thread.kernel_vstate;
>         if (!kvstate->datap)
> @@ -134,20 +134,35 @@ static int riscv_v_start_kernel_context(bool *is_nested)
>                 *is_nested = true;
>                 get_cpu_vector_context();
>                 if (riscv_preempt_v_dirty(current)) {
> +                       riscv_v_enable();
>                         __riscv_v_vstate_save(kvstate, kvstate->datap);
> +                       riscv_v_disable();
>                         riscv_preempt_v_clear_dirty(current);
>                 }
>                 riscv_preempt_v_set_restore(current);
>                 return 0;
>         }
>
> -       /* Transfer the ownership of V from user to kernel, then save */
> -       riscv_v_start(RISCV_PREEMPT_V | RISCV_PREEMPT_V_DIRTY);
> +       /*
> +        * Skip saving user's context if it is not DIRTY. We would have to start KMV in "dirty" if
> +        * this check is performed after KMV starts, to protect user's ctx. Then, we could waste
> +        * time saving already "clean" context once KMV is started in "dirty".
> +        */
>         if (__riscv_v_vstate_check(task_pt_regs(current)->status, DIRTY)) {
> -               uvstate = &current->thread.vstate;
> -               __riscv_v_vstate_save(uvstate, uvstate->datap);
> +               /* Transfer the ownership of V from user to kernel, then save */
> +               riscv_v_start(RISCV_PREEMPT_V | RISCV_PREEMPT_V_DIRTY);
> +               /*
> +                * Calling the guarded version of vstate_save to make the code cleaner. Also, the
> +                * vstate check within the call is necessary as context switch may happen between
> +                * __riscv_v_vstate_check and riscv_v_start. In such case we are not supposed to
> +                * save the context again.
> +                */
> +               riscv_v_vstate_save(&current->thread.vstate, task_pt_regs(current));
> +               riscv_preempt_v_clear_dirty(current);
> +               return 0;
>         }
> -       riscv_preempt_v_clear_dirty(current);
> +
> +       riscv_v_start(RISCV_PREEMPT_V);
>         return 0;
>  }
>
> @@ -180,7 +195,9 @@ asmlinkage void riscv_v_context_nesting_end(struct pt_regs *regs)
>         depth = riscv_v_ctx_get_depth();
>         if (depth == 0) {
>                 if (riscv_preempt_v_restore(current)) {
> +                       riscv_v_enable();
>                         __riscv_v_vstate_restore(vstate, vstate->datap);
> +                       riscv_v_disable();
>                         __riscv_v_vstate_clean(regs);
>                         riscv_preempt_v_reset_flags();
>                 }
> diff --git a/arch/riscv/kvm/vcpu_vector.c b/arch/riscv/kvm/vcpu_vector.c
> index 62d2fb77bb9b..da6c6db846c1 100644
> --- a/arch/riscv/kvm/vcpu_vector.c
> +++ b/arch/riscv/kvm/vcpu_vector.c
> @@ -46,7 +46,7 @@ void kvm_riscv_vcpu_guest_vector_save(struct kvm_cpu_context *cntx,
>  {
>         if ((cntx->sstatus & SR_VS) == SR_VS_DIRTY) {
>                 if (riscv_isa_extension_available(isa, v))
> -                       __kvm_riscv_vector_save(cntx);
> +                       kvm_riscv_vector_save(cntx);
>                 kvm_riscv_vcpu_vector_clean(cntx);
>         }
>  }
> @@ -56,7 +56,7 @@ void kvm_riscv_vcpu_guest_vector_restore(struct kvm_cpu_context *cntx,
>  {
>         if ((cntx->sstatus & SR_VS) != SR_VS_OFF) {
>                 if (riscv_isa_extension_available(isa, v))
> -                       __kvm_riscv_vector_restore(cntx);
> +                       kvm_riscv_vector_restore(cntx);
>                 kvm_riscv_vcpu_vector_clean(cntx);
>         }
>  }
> @@ -65,13 +65,13 @@ void kvm_riscv_vcpu_host_vector_save(struct kvm_cpu_context *cntx)
>  {
>         /* No need to check host sstatus as it can be modified outside */
>         if (!kvm_riscv_isa_check_host(V))
> -               __kvm_riscv_vector_save(cntx);
> +               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);
> +               kvm_riscv_vector_restore(cntx);
>  }
>
>  int kvm_riscv_vcpu_alloc_vector_context(struct kvm_vcpu *vcpu)
> --
> 2.43.0
>

-- 
kvm-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kvm-riscv