lkml.org 
[lkml]   [2021]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v9 07/17] riscv: Reset vector register
From
On Tue, 09 Nov 2021 01:48:19 PST (-0800), greentime.hu@sifive.com wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Reset vector registers at boot-time and disable vector instructions
> execution for kernel mode.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Co-developed-by: Vincent Chen <vincent.chen@sifive.com>
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> Co-developed-by: Han-Kuan Chen <hankuan.chen@sifive.com>
> Signed-off-by: Han-Kuan Chen <hankuan.chen@sifive.com>
> Co-developed-by: Greentime Hu <greentime.hu@sifive.com>
> Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
> ---
> arch/riscv/kernel/entry.S | 6 +++---
> arch/riscv/kernel/head.S | 22 ++++++++++++++++++++--
> 2 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index 98f502654edd..ad0fa80ada81 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -77,10 +77,10 @@ _save_context:
> * Disable user-mode memory access as it should only be set in the
> * actual user copy routines.
> *
> - * Disable the FPU to detect illegal usage of floating point in kernel
> - * space.
> + * Disable the FPU/Vector to detect illegal usage of floating point
> + * or vector in kernel space.
> */
> - li t0, SR_SUM | SR_FS
> + li t0, SR_SUM | SR_FS | SR_VS
>
> REG_L s0, TASK_TI_USER_SP(tp)
> csrrc s1, CSR_STATUS, t0
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index 52c5ff9804c5..551afe1de85e 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -242,10 +242,10 @@ pmp_done:
> .option pop
>
> /*
> - * Disable FPU to detect illegal usage of
> + * Disable FPU & VECTOR to detect illegal usage of
> * floating point in kernel space

Presumably that should be "floating point or vector", like the other
one?

> */
> - li t0, SR_FS
> + li t0, SR_FS | SR_VS
> csrc CSR_STATUS, t0
>
> #ifdef CONFIG_SMP
> @@ -433,6 +433,24 @@ ENTRY(reset_regs)
> csrw fcsr, 0
> /* note that the caller must clear SR_FS */
> #endif /* CONFIG_FPU */
> +
> +#ifdef CONFIG_VECTOR
> + csrr t0, CSR_MISA
> + li t1, (COMPAT_HWCAP_ISA_V >> 16)
> + slli t1, t1, 16

Why? Shouldn't the "li" pseudo handle generating that constant fine?
It generates the expected lui for me.

> + and t0, t0, t1
> + beqz t0, .Lreset_regs_done
> +
> + li t1, SR_VS
> + csrs CSR_STATUS, t1
> + vsetvli t1, x0, e8, m8
> + vmv.v.i v0, 0
> + vmv.v.i v8, 0
> + vmv.v.i v16, 0
> + vmv.v.i v24, 0

I don't see anything resetting vcsr here, which is explicitly required
by ISA manual.

Otherwise this looks OK to me: I wasn't actually sure this was guaranteed
to hit every bit in the vector register file, but IIUC it does -- VLMAX
has a defined value, VLEN is a constant, and this form of vsetvli is
defined to set vl to VLMAX. Probably worth a comment, though.

> + /* note that the caller must clear SR_VS */
> +#endif /* CONFIG_VECTOR */
> +
> .Lreset_regs_done:
> ret
> END(reset_regs)

With those minor bits fixed,

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>

\
 
 \ /
  Last update: 2021-12-14 17:30    [W:0.215 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site