From: Greentime Hu <greentime.hu@sifive.com> To: Palmer Dabbelt <palmer@dabbelt.com> Cc: Paul Walmsley <paul.walmsley@sifive.com>, linux-riscv <linux-riscv@lists.infradead.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Albert Ou <aou@eecs.berkeley.edu> Subject: Re: [PATCH v9 07/17] riscv: Reset vector register Date: Tue, 4 Jan 2022 14:02:39 +0800 [thread overview] Message-ID: <CAHCEehK-oQqsi0fivFfKMUcHrzVoLEGHd29=UKFuGFCBAqCJXw@mail.gmail.com> (raw) In-Reply-To: <mhng-591e217f-9290-464e-ab17-91fd84bed22b@palmer-ri-x1c9> 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>
WARNING: multiple messages have this Message-ID (diff)
From: Greentime Hu <greentime.hu@sifive.com> To: Palmer Dabbelt <palmer@dabbelt.com> Cc: Paul Walmsley <paul.walmsley@sifive.com>, linux-riscv <linux-riscv@lists.infradead.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Albert Ou <aou@eecs.berkeley.edu> Subject: Re: [PATCH v9 07/17] riscv: Reset vector register Date: Tue, 4 Jan 2022 14:02:39 +0800 [thread overview] Message-ID: <CAHCEehK-oQqsi0fivFfKMUcHrzVoLEGHd29=UKFuGFCBAqCJXw@mail.gmail.com> (raw) In-Reply-To: <mhng-591e217f-9290-464e-ab17-91fd84bed22b@palmer-ri-x1c9> 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> _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2022-01-04 6:02 UTC|newest] Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-11-09 9:48 [PATCH v9 00/17] riscv: Add vector ISA support Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-11-09 9:48 ` [PATCH v9 01/17] riscv: Separate patch for cflags and aflags Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-12-14 16:29 ` Palmer Dabbelt 2021-12-14 16:29 ` Palmer Dabbelt 2021-11-09 9:48 ` [PATCH v9 02/17] riscv: Rename __switch_to_aux -> fpu Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-12-14 16:29 ` Palmer Dabbelt 2021-12-14 16:29 ` Palmer Dabbelt 2021-11-09 9:48 ` [PATCH v9 03/17] riscv: Extending cpufeature.c to detect V-extension Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-12-14 16:29 ` Palmer Dabbelt 2021-12-14 16:29 ` Palmer Dabbelt 2021-11-09 9:48 ` [PATCH v9 04/17] riscv: Add new csr defines related to vector extension Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-12-14 16:29 ` Palmer Dabbelt 2021-12-14 16:29 ` Palmer Dabbelt 2021-11-09 9:48 ` [PATCH v9 05/17] riscv: Add vector feature to compile Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-11-12 0:54 ` kernel test robot 2021-11-12 0:54 ` kernel test robot 2021-11-12 0:54 ` kernel test robot 2021-12-14 16:29 ` Palmer Dabbelt 2021-12-14 16:29 ` Palmer Dabbelt 2021-11-09 9:48 ` [PATCH v9 06/17] riscv: Add has_vector/riscv_vsize to save vector features Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-12-14 16:29 ` Palmer Dabbelt 2021-12-14 16:29 ` Palmer Dabbelt 2021-11-09 9:48 ` [PATCH v9 07/17] riscv: Reset vector register Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-12-14 16:29 ` Palmer Dabbelt 2021-12-14 16:29 ` Palmer Dabbelt 2022-01-04 6:02 ` Greentime Hu [this message] 2022-01-04 6:02 ` Greentime Hu 2021-11-09 9:48 ` [PATCH v9 08/17] riscv: Add vector struct and assembler definitions Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-12-14 16:29 ` Palmer Dabbelt 2021-12-14 16:29 ` Palmer Dabbelt 2022-01-07 13:28 ` Greentime Hu 2022-01-07 13:28 ` Greentime Hu 2022-01-07 13:53 ` Jessica Clarke 2022-01-07 13:53 ` Jessica Clarke 2021-11-09 9:48 ` [PATCH v9 09/17] riscv: Add task switch support for vector Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-11-11 13:49 ` kernel test robot 2021-11-11 13:49 ` kernel test robot 2021-11-11 13:49 ` kernel test robot 2021-11-11 19:16 ` kernel test robot 2021-11-11 19:16 ` kernel test robot 2021-11-11 19:16 ` kernel test robot 2021-12-15 18:38 ` Palmer Dabbelt 2021-12-15 18:38 ` Palmer Dabbelt 2021-11-09 9:48 ` [PATCH v9 10/17] riscv: Add ptrace vector support Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-11-09 9:48 ` [PATCH v9 11/17] riscv: Add sigcontext save/restore for vector Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-11-09 9:48 ` [PATCH v9 12/17] riscv: signal: Report signal frame size to userspace via auxv Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-11-09 9:48 ` [PATCH v9 13/17] riscv: Add support for kernel mode vector Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-11-09 9:48 ` [PATCH v9 14/17] riscv: Use CSR_STATUS to replace sstatus in vector.S Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-11-09 9:48 ` [PATCH v9 15/17] riscv: Add vector extension XOR implementation Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-11-09 9:48 ` [PATCH v9 16/17] riscv: Fix an illegal instruction exception when accessing vlenb without enable vector first Greentime Hu 2021-11-09 9:48 ` Greentime Hu 2021-11-10 10:38 ` Ben Dooks 2021-11-10 10:38 ` Ben Dooks 2021-11-16 13:14 ` Greentime Hu 2021-11-16 13:14 ` Greentime Hu 2021-11-09 9:48 ` [PATCH v9 17/17] riscv: Fix a kernel panic issue if $s2 is set to a specific value before entering Linux Greentime Hu 2021-11-09 9:48 ` Greentime Hu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAHCEehK-oQqsi0fivFfKMUcHrzVoLEGHd29=UKFuGFCBAqCJXw@mail.gmail.com' \ --to=greentime.hu@sifive.com \ --cc=aou@eecs.berkeley.edu \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.