lkml.org 
[lkml]   [2022]   [Jan]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v9 07/17] riscv: Reset vector register
    Palmer Dabbelt <palmer@dabbelt.com> 於 2021年12月15日 週三 上午12:29寫道:
    >
    > 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?

    Thank you, Palmer. I'll update this and I also found another issue in
    secondary_start_sbi.
    We should also disable vector there.

    > > */
    > > - 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.

    That's right.
    I'll update the code here to
    #ifdef CONFIG_VECTOR
    csrr t0, CSR_MISA
    li t1, COMPAT_HWCAP_ISA_V
    and t0, t0, t1
    beqz t0, .Lreset_regs_done


    > > + 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.
    >

    /*
    * Clear vector registers and reset vcsr
    * VLMAX has a defined value, VLEN is a constant,
    * and this form of vsetvli is defined to set vl to VLMAX.
    */
    li t1, SR_VS
    csrs CSR_STATUS, t1
    csrs CSR_VCSR, x0
    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


    > > + /* 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: 2022-09-17 16:12    [W:3.896 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site