Re: [RFC PATCH v4 12/13] riscv: Add sigcontext save/restore for vector

[Guo Ren <guoren@kernel.org>]

Hi Greentime,

On Tue, May 26, 2020 at 3:03 PM Greentime Hu <greentime.hu@sifive.com> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> This patch adds sigcontext save/restore for vector. The vector registers
> will be saved in datap pointer. The datap pointer will be allocaed
> dynamically when the task needs in kernel space. The datap pointer will
> be set right after the __riscv_v_state data structure to save all the
> vector registers in the signal handler stack.
>
> [greentime.hu@sifive.com: add support for dynamic vlen]
> Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> ---
>  arch/riscv/include/uapi/asm/sigcontext.h |  2 +
>  arch/riscv/kernel/signal.c               | 92 +++++++++++++++++++++++-
>  2 files changed, 91 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/include/uapi/asm/sigcontext.h b/arch/riscv/include/uapi/asm/sigcontext.h
> index 84f2dfcfdbce..4217f3f1c8ba 100644
> --- a/arch/riscv/include/uapi/asm/sigcontext.h
> +++ b/arch/riscv/include/uapi/asm/sigcontext.h
> @@ -8,6 +8,7 @@
>
>  #include <asm/ptrace.h>
>
> +#define RVV_MAGIC      0x53465457
>  /*
>   * Signal context structure
>   *
> @@ -17,6 +18,7 @@
>  struct sigcontext {
>         struct user_regs_struct sc_regs;
>         union __riscv_fp_state sc_fpregs;
> +       struct __riscv_v_state sc_vregs;
>  };
>
>  #endif /* _UAPI_ASM_RISCV_SIGCONTEXT_H */
> diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
> index 17ba190e84a5..9ada6f74bb95 100644
> --- a/arch/riscv/kernel/signal.c
> +++ b/arch/riscv/kernel/signal.c
> @@ -83,6 +83,80 @@ static long save_fp_state(struct pt_regs *regs,
>  #define restore_fp_state(task, regs) (0)
>  #endif
>
> +#ifdef CONFIG_VECTOR
> +static long restore_v_state(struct pt_regs *regs, struct sigcontext *sc)
> +{
> +       long err;
> +       struct __riscv_v_state __user *state = &sc->sc_vregs;
> +       void *datap;
> +       __u32 magic;
> +
> +       /* Get magic number and check it. */
> +       err = __get_user(magic, &state->magic);
> +       if (unlikely(err))
> +               return err;
> +
> +       if (magic != RVV_MAGIC)
> +               return -EINVAL;
> +
> +       /* Copy everything of __riscv_v_state except datap. */
> +       err = __copy_from_user(&current->thread.vstate, state,
> +                              RISCV_V_STATE_DATAP);
> +       if (unlikely(err))
> +               return err;
> +
> +       /* Copy the pointer datap itself. */
> +       err = __get_user(datap, &state->datap);
> +       if (unlikely(err))
> +               return err;
> +
> +
> +       /* Copy the whole vector content from user space datap. */
> +       err = __copy_from_user(current->thread.vstate.datap, datap,
> +                              current->thread.vstate.size);
> +       if (unlikely(err))
> +               return err;
> +
> +       vstate_restore(current, regs);
> +
> +       return err;
> +}
> +
> +static long save_v_state(struct pt_regs *regs, struct sigcontext *sc)
> +{
> +       long err;
> +       struct __riscv_v_state __user *state = &sc->sc_vregs;
> +       /* Set the datap right after the sigcntext structure. */
> +       void *datap = sc + 1;
> +
> +       vstate_save(current, regs);
> +       /* Copy everything of vstate but datap. */
> +       err = __copy_to_user(state, &current->thread.vstate,
> +                            RISCV_V_STATE_DATAP);
> +       if (unlikely(err))
> +               return err;
> +
> +       /* Copy the magic number. */
> +       err = __put_user(RVV_MAGIC, &state->magic);
> +       if (unlikely(err))
> +               return err;
> +
> +       /* Copy the pointer datap itself. */
> +       err = __put_user(datap, &state->datap);
> +       if (unlikely(err))
> +               return err;
> +
> +       /* Copy the whole vector content to user space datap. */
> +       err = __copy_to_user(datap, current->thread.vstate.datap,
> +                            current->thread.vstate.size);
> +
> +       return err;
> +}
> +#else
> +#define save_v_state(task, regs) (0)
> +#define restore_v_state(task, regs) (0)
> +#endif
> +
>  static long restore_sigcontext(struct pt_regs *regs,
>         struct sigcontext __user *sc)
>  {
> @@ -92,6 +166,9 @@ static long restore_sigcontext(struct pt_regs *regs,
>         /* Restore the floating-point state. */
>         if (has_fpu)
>                 err |= restore_fp_state(regs, &sc->sc_fpregs);
> +       /* Restore the vector state. */
> +       if (has_vector)
> +               err |= restore_v_state(regs, sc);
>         return err;
>  }
>
> @@ -101,13 +178,16 @@ SYSCALL_DEFINE0(rt_sigreturn)
>         struct rt_sigframe __user *frame;
>         struct task_struct *task;
>         sigset_t set;
> +       size_t frame_size = sizeof(*frame);
>
>         /* Always make any pending restarted system calls return -EINTR */
>         current->restart_block.fn = do_no_restart_syscall;
>
>         frame = (struct rt_sigframe __user *)regs->sp;
>
> -       if (!access_ok(frame, sizeof(*frame)))
> +       if (has_vector)
> +               frame_size += current->thread.vstate.size;
Shall we force all applications save & restore vr regs ?  Maybe we can
utilize vs dirty bit

> +       if (!access_ok(frame, frame_size))
>                 goto badframe;
>
>         if (__copy_from_user(&set, &frame->uc.uc_sigmask, sizeof(set)))
> @@ -145,6 +225,9 @@ static long setup_sigcontext(struct rt_sigframe __user *frame,
>         /* Save the floating-point state. */
>         if (has_fpu)
>                 err |= save_fp_state(regs, &sc->sc_fpregs);
> +       /* Save the vector state. */
> +       if (has_vector)
> +               err |= save_v_state(regs, sc);
>         return err;
>  }
>
> @@ -176,9 +259,12 @@ static int setup_rt_frame(struct ksignal *ksig, sigset_t *set,
>  {
>         struct rt_sigframe __user *frame;
>         long err = 0;
> +       size_t frame_size = sizeof(*frame);
>
> -       frame = get_sigframe(ksig, regs, sizeof(*frame));
> -       if (!access_ok(frame, sizeof(*frame)))
> +       if (has_vector)
> +               frame_size += current->thread.vstate.size;
> +       frame = get_sigframe(ksig, regs, frame_size);
> +       if (!access_ok(frame, frame_size))
>                 return -EFAULT;
>
>         err |= copy_siginfo_to_user(&frame->info, &ksig->info);
> --
> 2.26.2
>
>


--
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/