riscv: Add vector ISA support
Subsystems (3)
v9 cover riscv: Add vector ISA support
This patchset is implemented based on vector 1.0 spec to add vector support in riscv Linux kernel. There are some assumptions for this implementations.
1. We assume all harts has the same ISA in the system.
2. We use vector like FPU in some ways instead of using vector like a specific IP.
3. We disable vector in kernel space by default unless kernel uses kernel mode vector kernel_rvv_begin()/kernel_rvv_end().
4. We detect "riscv,isa" to determine whether vector is support or not.
We defined a new structure __riscv_v_state in struct thread_struct to save /restore the vector related registers. It is used for both kernel space and user space.
- In kernel space, the datap pointer in __riscv_v_state will be allocated to save vector registers.
- In user space,
- In signal handler of user space, datap will point to the address of the __riscv_v_state data structure to save vector registers in stack. We also create a __reserved[] array in sigcontext for future extensions.
- In ptrace, the data will be put in ubuf in which we use riscv_vr_get()/riscv_vr_set() to get or set the __riscv_v_state data structure from/to it, datap pointer would be zeroed and vector registers will be copied to the address right after the __riscv_v_state structure in ubuf.
This patchset also adds support for kernel mode vector, kernel XOR implementation with vector ISA and includes several bug fixes and code refinement.
This patchset is rebased to v5.15 and it is tested by running several vector programs simultaneously. It also can get the correct ucontext_t in signal handler and restore correct context after sigreturn. It is also tested with ptrace() syscall to use PTRACE_GETREGSET/PTRACE_SETREGSET to get/set the vector registers.
We have sent patches for vector support to glibc mailing list.
TODO
1. Refine the __riscv_v_state allocation in start_thread()
2. Refine the vector context switching functions by lazy save/restore
3. Add AMP support. To support harts with different ISA
[1] https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc
[2] https://sourceware.org/pipermail/libc-alpha/2021-September/130897.html
[3] https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v9
---
Changelog V9
- Rebase to v5.15
- Merge several patches
- Refine codes
- Fix a kernel panic issue
Changelog V8
- Rebase to v5.14
- Refine struct __riscv_v_state with struct __riscv_ctx_hdr
- Refine has_vector into a static key
- Defined __reserved space in struct sigcontext for vector and future extensions
Changelog V7
- Add support for kernel mode vector
- Add vector extension XOR implementation
- Optimize task switch codes of vector
- Allocate space for vector registers in start_thread()
- Fix an illegal instruction exception when accessing vlenb
- Optimize vector registers initialization
- Initialize vector registers with proper vsetvli then it can work normally
- Refine ptrace porting due to generic API changed
- Code clean up
Changelog V6
- Replace vle.v/vse.v instructions with vle8.v/vse8.v based on 0.9 spec
- Add comments based on mailinglist feedback
- Fix rv32 build error
Changelog V5
- Using regset_size() correctly in generic ptrace
- Fix the ptrace porting
- Fix compile warning
Changelog V4
- Support dynamic vlen
- Fix bugs: lazy save/resotre, not saving vtype
- Update VS bit offset based on latest vector spec
- Add new vector csr based on latest vector spec
- Code refine and removed unused macros
Changelog V3
- Rebase linux-5.6-rc3 and tested with qemu
- Seperate patches with Anup's advice
- Give out a ABI puzzle with unlimited vlen
Changelog V2
- Fixup typo "vecotr, fstate_save->vstate_save".
- Fixup wrong saved registers' length in vector.S.
- Seperate unrelated patches from this one.
Greentime Hu (11):
riscv: Add new csr defines related to vector extension
riscv: Add has_vector/riscv_vsize to save vector features.
riscv: Add vector struct and assembler definitions
riscv: Add task switch support for vector
riscv: Add ptrace vector support
riscv: Add sigcontext save/restore for vector
riscv: Add support for kernel mode vector
riscv: Use CSR_STATUS to replace sstatus in vector.S
riscv: Add vector extension XOR implementation
riscv: Fix an illegal instruction exception when accessing vlenb
without enable vector first
riscv: Fix a kernel panic issue if $s2 is set to a specific value
before entering Linux
Guo Ren (5):
riscv: Separate patch for cflags and aflags
riscv: Rename __switch_to_aux -> fpu
riscv: Extending cpufeature.c to detect V-extension
riscv: Add vector feature to compile
riscv: Reset vector register
Vincent Chen (1):
riscv: signal: Report signal frame size to userspace via auxv
arch/riscv/Kconfig | 9 ++ arch/riscv/Makefile | 19 ++- arch/riscv/include/asm/csr.h | 16 ++- arch/riscv/include/asm/elf.h | 41 +++--- arch/riscv/include/asm/processor.h | 3 + arch/riscv/include/asm/switch_to.h | 72 +++++++++- arch/riscv/include/asm/vector.h | 16 +++ arch/riscv/include/asm/xor.h | 66 +++++++++ arch/riscv/include/uapi/asm/auxvec.h | 1 + arch/riscv/include/uapi/asm/hwcap.h | 1 + arch/riscv/include/uapi/asm/ptrace.h | 25 ++++ arch/riscv/include/uapi/asm/sigcontext.h | 24 ++++ arch/riscv/kernel/Makefile | 2 + arch/riscv/kernel/asm-offsets.c | 8 ++ arch/riscv/kernel/cpufeature.c | 16 +++ arch/riscv/kernel/entry.S | 6 +- arch/riscv/kernel/head.S | 24 +++- arch/riscv/kernel/kernel_mode_vector.c | 146 +++++++++++++++++++ arch/riscv/kernel/process.c | 49 +++++++ arch/riscv/kernel/ptrace.c | 71 ++++++++++ arch/riscv/kernel/setup.c | 4 + arch/riscv/kernel/signal.c | 173 ++++++++++++++++++++++- arch/riscv/kernel/vector.S | 90 ++++++++++++ arch/riscv/lib/Makefile | 1 + arch/riscv/lib/xor.S | 81 +++++++++++ include/uapi/linux/elf.h | 1 + 26 files changed, 929 insertions(+), 36 deletions(-) create mode 100644 arch/riscv/include/asm/vector.h create mode 100644 arch/riscv/include/asm/xor.h create mode 100644 arch/riscv/kernel/kernel_mode_vector.c create mode 100644 arch/riscv/kernel/vector.S create mode 100644 arch/riscv/lib/xor.S
Signature · 1 line
2.31.1
PATCH v9 1/17 riscv: Separate patch for cflags and aflags
From: Guo Ren <[email protected]>
Use "subst fd" in Makefile is a hack way and it's not convenient to add new ISA feature. Just separate them into riscv-march-cflags and riscv-march-aflags.
--- arch/riscv/Makefile | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
arch/riscv/Makefile +12 -6
@@ -48,12 +48,18 @@ endif endif # ISA string setting -riscv-march-$(CONFIG_ARCH_RV32I) := rv32ima -riscv-march-$(CONFIG_ARCH_RV64I) := rv64ima -riscv-march-$(CONFIG_FPU) := $(riscv-march-y)fd -riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c -KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y)) -KBUILD_AFLAGS += -march=$(riscv-march-y) +riscv-march-cflags-$(CONFIG_ARCH_RV32I) := rv32ima +riscv-march-cflags-$(CONFIG_ARCH_RV64I) := rv64ima +riscv-march-$(CONFIG_FPU) := $(riscv-march-y)fd +riscv-march-cflags-$(CONFIG_RISCV_ISA_C) := $(riscv-march-cflags-y)c + +riscv-march-aflags-$(CONFIG_ARCH_RV32I) := rv32ima +riscv-march-aflags-$(CONFIG_ARCH_RV64I) := rv64ima +riscv-march-aflags-$(CONFIG_FPU) := $(riscv-march-aflags-y)fd +riscv-march-aflags-$(CONFIG_RISCV_ISA_C) := $(riscv-march-aflags-y)c + +KBUILD_CFLAGS += -march=$(riscv-march-cflags-y) +KBUILD_AFLAGS += -march=$(riscv-march-aflags-y) KBUILD_CFLAGS += -mno-save-restore KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
Signature · 1 line
2.31.1
Re: [PATCH v9 01/17] riscv: Separate patch for cflags and aflags
Quoted 32 lines · On Tue, 09 Nov 2021 01:48:13 PST (-0800), [email protected] wrote:
From: Guo Ren <[email protected]>
Use "subst fd" in
Makefileis a hack way and it's not convenient to add new ISA feature. Just separate them into riscv-march-cflags and riscv-march-aflags.Signed-off-by Guo Ren <[email protected]>Signed-off-by Guo Ren <[email protected]>Signed-off-by Greentime Hu <[email protected]>--- arch/riscv/Makefile | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)arch/riscv/Makefile +5
-6@@ -48,12 +48,18 @@ endif endif # ISA string setting -riscv-march-$(CONFIG_ARCH_RV32I) := rv32ima -riscv-march-$(CONFIG_ARCH_RV64I) := rv64ima -riscv-march-$(CONFIG_FPU) := $(riscv-march-y)fd -riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c -KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y)) -KBUILD_AFLAGS += -march=$(riscv-march-y) +riscv-march-cflags-$(CONFIG_ARCH_RV32I) := rv32ima +riscv-march-cflags-$(CONFIG_ARCH_RV64I) := rv64ima +riscv-march-$(CONFIG_FPU) := $(riscv-march-y)fd +riscv-march-cflags-$(CONFIG_RISCV_ISA_C) := $(riscv-march-cflags-y)c +
arch/riscv/Makefile endif · line 65
Quoted 10 lines
arch/riscv/Makefile +7
-0+riscv-march-aflags-$(CONFIG_ARCH_RV32I) := rv32ima +riscv-march-aflags-$(CONFIG_ARCH_RV64I) := rv64ima +riscv-march-aflags-$(CONFIG_FPU) := $(riscv-march-aflags-y)fd +riscv-march-aflags-$(CONFIG_RISCV_ISA_C) := $(riscv-march-aflags-y)c + +KBUILD_CFLAGS += -march=$(riscv-march-cflags-y) +KBUILD_AFLAGS += -march=$(riscv-march-aflags-y) KBUILD_CFLAGS += -mno-save-restore KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
PATCH v9 2/17 riscv: Rename __switch_to_aux -> fpu
In reply to Greentime HuFrom: Guo Ren <[email protected]>
The name of __switch_to_aux is not clear and rename it with the determine function: __switch_to_fpu. Next we could add other regs' switch.
--- arch/riscv/include/asm/switch_to.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
arch/riscv/include/asm/switch_to.h +3 -3
@@ -45,7 +45,7 @@ static inline void fstate_restore(struct task_struct *task, } } -static inline void __switch_to_aux (struct task_struct *prev, +static inline void __switch_to_fpu (struct task_struct *prev,struct task_struct *next) {struct pt_regs *regs;
@@ -65,7 +65,7 @@ static __always_inline bool has_fpu(void) static __always_inline bool has_fpu(void) { return false; }#define fstate_save (task, regs) do { } while (0)#define fstate_restore (task, regs) do { } while (0) -#define __switch_to_aux (__prev, __next) do { } while (0) +#define __switch_to_fpu (__prev, __next) do { } while (0)#endif extern struct task_struct *__switch_to(struct task_struct *,
@@ -76,7 +76,7 @@ do { \struct task_struct *__prev = (prev); \struct task_struct *__next = (next); \if (has_fpu ()) \ -__switch_to_aux (__prev, __next); \ +__switch_to_fpu (__prev, __next); \ ((last) =__switch_to (__prev, __next)); \ }while (0 )
Signature · 1 line
2.31.1
Re: [PATCH v9 02/17] riscv: Rename __switch_to_aux -> fpu
Quoted 38 lines · On Tue, 09 Nov 2021 01:48:14 PST (-0800), [email protected] wrote:
From: Guo Ren <[email protected]>
The name of
__switch_to_auxis not clear and rename it with the determine function:__switch_to_fpu. Next we could add other regs' switch.Signed-off-by Guo Ren <[email protected]>Signed-off-by Guo Ren <[email protected]>Reviewed-by Anup Patel <[email protected]>Reviewed-by Greentime Hu <[email protected]>Signed-off-by Greentime Hu <[email protected]>--- arch/riscv/include/asm/switch_to.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)arch/riscv/include/asm/switch_to.h +2
-2@@ -45,7 +45,7 @@ static inline void fstate_restore(struct task_struct *task, } } -static inline void __switch_to_aux (struct task_struct *prev, +static inline void __switch_to_fpu (struct task_struct *prev,struct task_struct *next) {struct pt_regs *regs;@@ -65,7 +65,7 @@ static __always_inline bool has_fpu(void) static __always_inline bool has_fpu(void) { return false; }#define fstate_save (task, regs) do { } while (0)#define fstate_restore (task, regs) do { } while (0) -#define __switch_to_aux (__prev, __next) do { } while (0) +#define __switch_to_fpu (__prev, __next) do { } while (0)#endif extern struct task_struct *__switch_to(struct task_struct *,
arch/riscv/include/asm/switch_to.h do { \ · line 81
Quoted 7 lines
arch/riscv/include/asm/switch_to.h +1
-1@@ -76,7 +76,7 @@ do { \struct task_struct *__prev = (prev); \struct task_struct *__next = (next); \if (has_fpu ()) \ -__switch_to_aux (__prev, __next); \ +__switch_to_fpu (__prev, __next); \ ((last) =__switch_to (__prev, __next)); \ }while (0 )
PATCH v9 3/17 riscv: Extending cpufeature.c to detect V-extension
In reply to Greentime HuFrom: Guo Ren <[email protected]>
Current cpufeature.c doesn't support detecting V-extension, because "rv64" also contain a 'v' letter and we need to skip it.
--- arch/riscv/include/uapi/asm/hwcap.h | 1 + arch/riscv/kernel/cpufeature.c | 1 + 2 files changed, 2 insertions(+)
arch/riscv/include/uapi/asm/hwcap.h +1 -0
@@ -21,5 +21,6 @@#define COMPAT_HWCAP_ISA_F (1 << ('F' - 'A'))#define COMPAT_HWCAP_ISA_D (1 << ('D' - 'A'))#define COMPAT_HWCAP_ISA_C (1 << ('C' - 'A')) +#define COMPAT_HWCAP_ISA_V (1 << ('V' - 'A'))#endif /* _UAPI_ASM_RISCV_HWCAP_H */
arch/riscv/kernel/cpufeature.c +1 -0
@@ -73,6 +73,7 @@ void __init riscv_fill_hwcap(void) isa2hwcap['f' ] = isa2hwcap['F' ] =COMPAT_HWCAP_ISA_F ; isa2hwcap['d' ] = isa2hwcap['D' ] =COMPAT_HWCAP_ISA_D ; isa2hwcap['c' ] = isa2hwcap['C' ] =COMPAT_HWCAP_ISA_C ; + isa2hwcap['v' ] = isa2hwcap['V' ] =COMPAT_HWCAP_ISA_V ; elf_hwcap =0 ;
Signature · 1 line
2.31.1
Re: [PATCH v9 03/17] riscv: Extending cpufeature.c to detect V-extension
Quoted 31 lines · On Tue, 09 Nov 2021 01:48:15 PST (-0800), [email protected] wrote:
From: Guo Ren <[email protected]>
Current
cpufeature.cdoesn't support detecting V-extension, because "rv64" also contain a 'v' letter and we need to skip it.Signed-off-by Guo Ren <[email protected]>Signed-off-by Guo Ren <[email protected]>Reviewed-by Anup Patel <[email protected]>Reviewed-by Greentime Hu <[email protected]>Signed-off-by Greentime Hu <[email protected]>--- arch/riscv/include/uapi/asm/hwcap.h | 1 + arch/riscv/kernel/cpufeature.c | 1 + 2 files changed, 2 insertions(+)arch/riscv/include/uapi/asm/hwcap.h +1
-0@@ -21,5 +21,6 @@#define COMPAT_HWCAP_ISA_F (1 << ('F' - 'A'))#define COMPAT_HWCAP_ISA_D (1 << ('D' - 'A'))#define COMPAT_HWCAP_ISA_C (1 << ('C' - 'A')) +#define COMPAT_HWCAP_ISA_V (1 << ('V' - 'A'))#endif /* _UAPI_ASM_RISCV_HWCAP_H */ arch/riscv/kernel/cpufeature.c +0
-0
arch/riscv/kernel/cpufeature.c void __init riscv_fill_hwcap(void) · line 78
Quoted 6 lines
arch/riscv/kernel/cpufeature.c +1
-0@@ -73,6 +73,7 @@ void __init riscv_fill_hwcap(void) isa2hwcap['f' ] = isa2hwcap['F' ] =COMPAT_HWCAP_ISA_F ; isa2hwcap['d' ] = isa2hwcap['D' ] =COMPAT_HWCAP_ISA_D ; isa2hwcap['c' ] = isa2hwcap['C' ] =COMPAT_HWCAP_ISA_C ; + isa2hwcap['v' ] = isa2hwcap['V' ] =COMPAT_HWCAP_ISA_V ; elf_hwcap =0 ;
PATCH v9 4/17 riscv: Add new csr defines related to vector extension
In reply to Greentime HuFollow the riscv vector spec to add new csr numbers.
--- arch/riscv/include/asm/csr.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
arch/riscv/include/asm/csr.h +14 -2
@@ -24,6 +24,12 @@#define SR_FS_CLEAN _AC(0x00004000, UL)#define SR_FS_DIRTY _AC(0x00006000, UL) +#define SR_VS _AC(0x00000600, UL)/* Vector Status */ +#define SR_VS_OFF _AC(0x00000000, UL) +#define SR_VS_INITIAL _AC(0x00000200, UL) +#define SR_VS_CLEAN _AC(0x00000400, UL) +#define SR_VS_DIRTY _AC(0x00000600, UL) +#define SR_XS _AC(0x00018000, UL)/* Extension Status */ #define SR_XS_OFF _AC(0x00000000, UL)#define SR_XS_INITIAL _AC(0x00008000, UL)
@@ -31,9 +37,9 @@#define SR_XS_DIRTY _AC(0x00018000, UL)#ifndef CONFIG_64BIT -#define SR_SD _AC(0x80000000, UL)/* FS/XS dirty */ +#define SR_SD _AC(0x80000000, UL)/* FS/VS/XS dirty */ #else -#define SR_SD _AC(0x8000000000000000, UL)/* FS/XS dirty */ +#define SR_SD _AC(0x8000000000000000, UL)/* FS/VS/XS dirty */ #endif /* SATP flags */
@@ -120,6 +126,12 @@#define CSR_MIMPID 0xf13#define CSR_MHARTID 0xf14 +#define CSR_VSTART 0x8 +#define CSR_VCSR 0xf +#define CSR_VL 0xc20 +#define CSR_VTYPE 0xc21 +#define CSR_VLENB 0xc22 +#ifdef CONFIG_RISCV_M_MODE # define CSR_STATUS CSR_MSTATUS# define CSR_IE CSR_MIE
Signature · 1 line
2.31.1
Re: [PATCH v9 04/17] riscv: Add new csr defines related to vector extension
Quoted 45 lines · On Tue, 09 Nov 2021 01:48:16 PST (-0800), [email protected] wrote:
Follow the riscv vector spec to add new csr numbers.
[[email protected]: first porting for new vector related csr]Signed-off-by Greentime Hu <[email protected]>Acked-by Guo Ren <[email protected]>Co-developed-by Guo Ren <[email protected]>Signed-off-by Guo Ren <[email protected]>Co-developed-by Vincent Chen <[email protected]>Signed-off-by Vincent Chen <[email protected]>--- arch/riscv/include/asm/csr.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)arch/riscv/include/asm/csr.h +8
-2@@ -24,6 +24,12 @@#define SR_FS_CLEAN _AC(0x00004000, UL)#define SR_FS_DIRTY _AC(0x00006000, UL) +#define SR_VS _AC(0x00000600, UL)/* Vector Status */ +#define SR_VS_OFF _AC(0x00000000, UL) +#define SR_VS_INITIAL _AC(0x00000200, UL) +#define SR_VS_CLEAN _AC(0x00000400, UL) +#define SR_VS_DIRTY _AC(0x00000600, UL) +#define SR_XS _AC(0x00018000, UL)/* Extension Status */ #define SR_XS_OFF _AC(0x00000000, UL)#define SR_XS_INITIAL _AC(0x00008000, UL)@@ -31,9 +37,9 @@#define SR_XS_DIRTY _AC(0x00018000, UL)#ifndef CONFIG_64BIT -#define SR_SD _AC(0x80000000, UL)/* FS/XS dirty */ +#define SR_SD _AC(0x80000000, UL)/* FS/VS/XS dirty */ #else -#define SR_SD _AC(0x8000000000000000, UL)/* FS/XS dirty */ +#define SR_SD _AC(0x8000000000000000, UL)/* FS/VS/XS dirty */ #endif /* SATP flags */ @@ -120,6 +126,12 @@#define CSR_MIMPID 0xf13#define CSR_MHARTID 0xf14
arch/riscv/include/asm/csr.h line 137
Quoted 10 lines
arch/riscv/include/asm/csr.h +6
-0+#define CSR_VSTART 0x8 +#define CSR_VCSR 0xf +#define CSR_VL 0xc20 +#define CSR_VTYPE 0xc21 +#define CSR_VLENB 0xc22 +#ifdef CONFIG_RISCV_M_MODE # define CSR_STATUS CSR_MSTATUS# define CSR_IE CSR_MIE
PATCH v9 5/17 riscv: Add vector feature to compile
In reply to Greentime HuFrom: Guo Ren <[email protected]>
This patch adds a new config option which could enable assembler's vector feature.
--- arch/riscv/Kconfig | 9 +++++++++ arch/riscv/Makefile | 1 + 2 files changed, 10 insertions(+)
arch/riscv/Kconfig +9 -0
@@ -389,6 +389,15 @@ config FPU If you don't know what to do here, say Y. +config VECTOR + bool "VECTOR support" + default n + help + Say N here if you want to disable all vector related procedure + in the kernel. + + If you don't know what to do here, say Y. + endmenu menu "Kernel features"
arch/riscv/Makefile +1 -0
@@ -57,6 +57,7 @@ riscv-march-aflags-$(CONFIG_ARCH_RV32I) := rv32ima riscv-march-aflags-$(CONFIG_ARCH_RV64I) := rv64ima riscv-march-aflags-$(CONFIG_FPU) := $(riscv-march-aflags-y)fd riscv-march-aflags-$(CONFIG_RISCV_ISA_C) := $(riscv-march-aflags-y)c +riscv-march-aflags-$(CONFIG_VECTOR) := $(riscv-march-aflags-y)v KBUILD_CFLAGS += -march=$(riscv-march-cflags-y) KBUILD_AFLAGS += -march=$(riscv-march-aflags-y)
Signature · 1 line
2.31.1
kernel test robot
build botRe: [PATCH v9 05/17] riscv: Add vector feature to compile
I love your patch! Yet something to improve:
Hi Greentime,
I love your patch! Yet something to improve:
[auto build test ERROR on v5.15]
[cannot apply to linus/master next-20211111]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Greentime-Hu/riscv-Add-vector-ISA-support/20211109-175222 base: DEBUG invalid remote for branch v5.15 8bb7eca972ad531c9b149c0a51ab43a417385813 config: riscv-allyesconfig (attached as .config) compiler: riscv64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/b164ca135b641a96cea32346baec67e96bf468a2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Greentime-Hu/riscv-Add-vector-ISA-support/20211109-175222 git checkout b164ca135b641a96cea32346baec67e96bf468a2
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=riscv prepare
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr] scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr] Assembler messages:
>> Error: cannot find default versions of the ISA extension `v' make[2]: *** [scripts/Makefile.build:379: arch/riscv/kernel/vdso/rt_sigreturn.o] Error 1 Assembler messages: >> Error: cannot find default versions of the ISA extension `v' Assembler messages: >> Error: cannot find default versions of the ISA extension `v' make[2]: *** [scripts/Makefile.build:379: arch/riscv/kernel/vdso/getcpu.o] Error 1 make[2]: *** [scripts/Makefile.build:379: arch/riscv/kernel/vdso/note.o] Error 1 Assembler messages: >> Error: cannot find default versions of the ISA extension `v'
make[2]: *** [scripts/Makefile.build:379: arch/riscv/kernel/vdso/flush_icache.o] Error 1 make[2]: Target 'include/generated/vdso-offsets.h' not remade because of errors. make[1]: *** [arch/riscv/Makefile:121: vdso_prepare] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:219: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Palmer Dabbelt
maintainerRe: [PATCH v9 05/17] riscv: Add vector feature to compile
In reply to Greentime HuQuoted 21 lines · On Tue, 09 Nov 2021 01:48:17 PST (-0800), [email protected] wrote:
From: Guo Ren <[email protected]>
This patch adds a new config option which could enable assembler's vector feature.
Signed-off-by Greentime Hu <[email protected]>Signed-off-by Guo Ren <[email protected]>Reviewed-by Greentime Hu <[email protected]>--- arch/riscv/Kconfig | 9 +++++++++ arch/riscv/Makefile | 1 + 2 files changed, 10 insertions(+)arch/riscv/Kconfig +0
-0@@ -389,6 +389,15 @@ config FPU If you don't know what to do here, say Y.
arch/riscv/Kconfig config FPU · line 401
Quoted 10 lines
arch/riscv/Kconfig +9
-0+config VECTOR + bool "VECTOR support" + default n + help + Say N here if you want to disable all vector related procedure + in the kernel. + + If you don't know what to do here, say Y. + endmenu
As the kernel build robot points out, this should have some sort of dependency on a vector toolchain. Not sure if there's a better way to do it that cc-option, looks like there are some other tests for assembler arguments that way.
Quoted 14 lines
arch/riscv/Kconfig +0
-0menu "Kernel features"arch/riscv/Makefile +1
-0@@ -57,6 +57,7 @@ riscv-march-aflags-$(CONFIG_ARCH_RV32I) := rv32ima riscv-march-aflags-$(CONFIG_ARCH_RV64I) := rv64ima riscv-march-aflags-$(CONFIG_FPU) := $(riscv-march-aflags-y)fd riscv-march-aflags-$(CONFIG_RISCV_ISA_C) := $(riscv-march-aflags-y)c +riscv-march-aflags-$(CONFIG_VECTOR) := $(riscv-march-aflags-y)v KBUILD_CFLAGS += -march=$(riscv-march-cflags-y) KBUILD_AFLAGS += -march=$(riscv-march-aflags-y)
PATCH v9 6/17 riscv: Add has_vector/riscv_vsize to save vector features.
In reply to Greentime HuThis patch is used to detect vector support status of CPU and use riscv_vsize to save the size of all the vector registers. It assumes all harts has the same capabilities in SMP system.
--- arch/riscv/kernel/cpufeature.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
arch/riscv/kernel/cpufeature.c +13 -0
@@ -21,6 +21,11 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;#ifdef CONFIG_FPU __ro_after_init DEFINE_STATIC_KEY_FALSE (cpu_hwcap_fpu);#endif +#ifdef CONFIG_VECTOR +#include <asm/vector.h> +__ro_after_init DEFINE_STATIC_KEY_FALSE (cpu_hwcap_vector); +unsigned long riscv_vsize__read_mostly ; +#endif /** * riscv_isa_extension_base() - Get base extension word
@@ -149,4 +154,12 @@ void __init riscv_fill_hwcap(void)if (elf_hwcap & (COMPAT_HWCAP_ISA_F |COMPAT_HWCAP_ISA_D ))static_branch_enable (&cpu_hwcap_fpu);#endif + +#ifdef CONFIG_VECTOR +if (elf_hwcap &COMPAT_HWCAP_ISA_V ) { +static_branch_enable (&cpu_hwcap_vector); +/* There are 32 vector registers with vlenb length. */ + riscv_vsize =csr_read (CSR_VLENB ) *32 ; + } +#endif }
Signature · 1 line
2.31.1
Re: [PATCH v9 06/17] riscv: Add has_vector/riscv_vsize to save vector features.
Quoted 10 lines · On Tue, 09 Nov 2021 01:48:18 PST (-0800), [email protected] wrote:
This patch is used to detect vector support status of CPU and use
riscv_vsizeto save the size of all the vector registers. It assumes all harts has the same capabilities in SMP system.[[email protected]: add has_vector checking]Signed-off-by Greentime Hu <[email protected]>Co-developed-by Guo Ren <[email protected]>Signed-off-by Guo Ren <[email protected]>Co-developed-by Vincent Chen <[email protected]>Signed-off-by Vincent Chen <[email protected]>
IMO those SOB flags are a bit out of order, but checkpatch isn't complaining so I'm not sure it matters. Otherwise:
Quoted 33 lines
--- arch/riscv/kernel/cpufeature.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)arch/riscv/kernel/cpufeature.c +13
-0@@ -21,6 +21,11 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;#ifdef CONFIG_FPU __ro_after_init DEFINE_STATIC_KEY_FALSE (cpu_hwcap_fpu);#endif +#ifdef CONFIG_VECTOR +#include <asm/vector.h> +__ro_after_init DEFINE_STATIC_KEY_FALSE (cpu_hwcap_vector); +unsigned long riscv_vsize__read_mostly ; +#endif /** * riscv_isa_extension_base() - Get base extension word @@ -149,4 +154,12 @@ void __init riscv_fill_hwcap(void)if (elf_hwcap & (COMPAT_HWCAP_ISA_F |COMPAT_HWCAP_ISA_D ))static_branch_enable (&cpu_hwcap_fpu);#endif + +#ifdef CONFIG_VECTOR +if (elf_hwcap &COMPAT_HWCAP_ISA_V ) { +static_branch_enable (&cpu_hwcap_vector); +/* There are 32 vector registers with vlenb length. */ + riscv_vsize =csr_read (CSR_VLENB ) *32 ; + } +#endif }
PATCH v9 7/17 riscv: Reset vector register
In reply to Greentime HuFrom: Guo Ren <[email protected]>
Reset vector registers at boot-time and disable vector instructions execution for kernel mode.
--- arch/riscv/kernel/entry.S | 6 +++--- arch/riscv/kernel/head.S | 22 ++++++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-)
arch/riscv/kernel/entry.S +3 -3
@@ -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
arch/riscv/kernel/head.S +20 -2
@@ -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 */ - 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 + 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 + /* note that the caller must clear SR_VS */ +#endif /* CONFIG_VECTOR */ + .Lreset_regs_done: ret END(reset_regs)
Signature · 1 line
2.31.1
Re: [PATCH v9 07/17] riscv: Reset vector register
Quoted 40 lines · On Tue, 09 Nov 2021 01:48:19 PST (-0800), [email protected] wrote:
From: Guo Ren <[email protected]>
Reset vector registers at boot-time and disable vector instructions execution for kernel mode.
Signed-off-by Guo Ren <[email protected]>Co-developed-by Vincent Chen <[email protected]>Signed-off-by Vincent Chen <[email protected]>Co-developed-by Han-Kuan Chen <[email protected]>Signed-off-by Han-Kuan Chen <[email protected]>Co-developed-by Greentime Hu <[email protected]>Signed-off-by Greentime Hu <[email protected]>--- arch/riscv/kernel/entry.S | 6 +++--- arch/riscv/kernel/head.S | 22 ++++++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-)arch/riscv/kernel/entry.S +3
-3@@ -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, t0arch/riscv/kernel/head.S +0
-0
arch/riscv/kernel/head.S pmp_done: · line 246
Quoted 6 lines
arch/riscv/kernel/head.S +1
-1@@ -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?
Quoted 7 lines
arch/riscv/kernel/head.S +1
-1*/ - li t0, SR_FS + li t0, SR_FS | SR_VS csrc CSR_STATUS, t0 #ifdef CONFIG_SMP
arch/riscv/kernel/head.S ENTRY(reset_regs) · line 440
Quoted 8 lines
arch/riscv/kernel/head.S +5
-0@@ -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.
arch/riscv/kernel/head.S ENTRY(reset_regs) · line 450
Quoted 10 lines
arch/riscv/kernel/head.S +10
-0+ 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.
arch/riscv/kernel/head.S ENTRY(reset_regs) · line 456
Quoted 6 lines
arch/riscv/kernel/head.S +3
-0+ /* note that the caller must clear SR_VS */ +#endif /* CONFIG_VECTOR */ + .Lreset_regs_done: ret END(reset_regs)
With those minor bits fixed,
Re: [PATCH v9 07/17] riscv: Reset vector register
Palmer Dabbelt <[email protected]> 於 2021年12月15日 週三 上午12:29寫道：
Quoted 44 lines · On Tue, 09 Nov 2021 01:48:19 PST (-0800), [email protected] wrote:
Quoted 42 lines
From: Guo Ren <[email protected]>
Reset vector registers at boot-time and disable vector instructions execution for kernel mode.
Signed-off-by Guo Ren <[email protected]>Co-developed-by Vincent Chen <[email protected]>Signed-off-by Vincent Chen <[email protected]>Co-developed-by Han-Kuan Chen <[email protected]>Signed-off-by Han-Kuan Chen <[email protected]>Co-developed-by Greentime Hu <[email protected]>Signed-off-by Greentime Hu <[email protected]>--- arch/riscv/kernel/entry.S | 6 +++--- arch/riscv/kernel/head.S | 22 ++++++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-)arch/riscv/kernel/entry.S +3
-3@@ -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, t0arch/riscv/kernel/head.S +0
-0@@ -242,10 +242,10 @@ pmp_done: .option pop
Quoted 7 lines
arch/riscv/kernel/head.S pmp_done: · line 246
Quoted 4 lines
arch/riscv/kernel/head.S +1
-1/* - * Disable FPU to detect illegal usage of + * Disable FPU & VECTOR to detect illegal usage of * floating point in kernel spacePresumably 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.
Quoted 5 lines
Quoted 5 lines
arch/riscv/kernel/head.S +1
-1*/ - li t0, SR_FS + li t0, SR_FS | SR_VS csrc CSR_STATUS, t0
Quoted 13 lines
arch/riscv/kernel/head.S ENTRY(reset_regs) · line 440
Quoted 10 lines
arch/riscv/kernel/head.S +5
-0#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, 16Why? 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_doneQuoted 11 lines
arch/riscv/kernel/head.S ENTRY(reset_regs) · line 450
Quoted 10 lines
arch/riscv/kernel/head.S +10
-0+ 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
Quoted 8 lines
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
Quoted 10 lines
arch/riscv/kernel/head.S ENTRY(reset_regs) · line 456
Quoted 6 lines
arch/riscv/kernel/head.S +3
-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 <[email protected]>
PATCH v9 8/17 riscv: Add vector struct and assembler definitions
In reply to Greentime HuAdd vector state context struct in struct thread and asm-offsets.c definitions.
The vector registers will be saved in datap pointer of __riscv_v_state. It will be dynamically allocated in kernel space. It will be put right after the __riscv_v_state data structure in user space.
--- arch/riscv/include/asm/processor.h | 1 + arch/riscv/include/uapi/asm/ptrace.h | 11 +++++++++++ arch/riscv/kernel/asm-offsets.c | 6 ++++++ 3 files changed, 18 insertions(+)
arch/riscv/include/asm/processor.h +1 -0
@@ -35,6 +35,7 @@ struct thread_struct {unsigned long s[12 ];/* s[0]: frame pointer */ struct __riscv_d_ext_state fstate;unsigned long bad_cause; +struct __riscv_v_state vstate; };/* Whitelist the fstate from the task_struct for hardened usercopy */
arch/riscv/include/uapi/asm/ptrace.h +11 -0
@@ -77,6 +77,17 @@ union __riscv_fp_state {struct __riscv_q_ext_state q; }; +struct __riscv_v_state { +unsigned long vstart; +unsigned long vl; +unsigned long vtype; +unsigned long vcsr; +void *datap; +#if __riscv_xlen ==32 +__u32 __padding; +#endif +}; +#endif /* __ASSEMBLY__ */ #endif /* _UAPI_ASM_RISCV_PTRACE_H */
arch/riscv/kernel/asm-offsets.c +6 -0
@@ -72,6 +72,12 @@ void asm_offsets(void)OFFSET (TSK_STACK_CANARY , task_struct, stack_canary);#endif +OFFSET (RISCV_V_STATE_VSTART , __riscv_v_state, vstart); +OFFSET (RISCV_V_STATE_VL , __riscv_v_state, vl); +OFFSET (RISCV_V_STATE_VTYPE , __riscv_v_state, vtype); +OFFSET (RISCV_V_STATE_VCSR , __riscv_v_state, vcsr); +OFFSET (RISCV_V_STATE_DATAP , __riscv_v_state, datap); +DEFINE (PT_SIZE ,sizeof (struct pt_regs ));OFFSET (PT_EPC , pt_regs, epc);OFFSET (PT_RA , pt_regs, ra);
Signature · 1 line
2.31.1
Palmer Dabbelt
maintainerRe: [PATCH v9 08/17] riscv: Add vector struct and assembler definitions
Quoted 33 lines · On Tue, 09 Nov 2021 01:48:20 PST (-0800), [email protected] wrote:
Add vector state context struct in struct thread and
asm-offsets.cdefinitions.The vector registers will be saved in datap pointer of
__riscv_v_state. It will be dynamically allocated in kernel space. It will be put right after the__riscv_v_statedata structure in user space.Co-developed-by Vincent Chen <[email protected]>Signed-off-by Vincent Chen <[email protected]>Signed-off-by Greentime Hu <[email protected]>--- arch/riscv/include/asm/processor.h | 1 + arch/riscv/include/uapi/asm/ptrace.h | 11 +++++++++++ arch/riscv/kernel/asm-offsets.c | 6 ++++++ 3 files changed, 18 insertions(+)arch/riscv/include/asm/processor.h +1
-0@@ -35,6 +35,7 @@ struct thread_struct {unsigned long s[12 ];/* s[0]: frame pointer */ struct __riscv_d_ext_state fstate;unsigned long bad_cause; +struct __riscv_v_state vstate; };/* Whitelist the fstate from the task_struct for hardened usercopy */ arch/riscv/include/uapi/asm/ptrace.h +0
-0
arch/riscv/include/uapi/asm/ptrace.h union __riscv_fp_state { · line 84
Quoted 8 lines
arch/riscv/include/uapi/asm/ptrace.h +5
-0@@ -77,6 +77,17 @@ union __riscv_fp_state {struct __riscv_q_ext_state q; }; +struct __riscv_v_state { +unsigned long vstart; +unsigned long vl; +unsigned long vtype; +unsigned long vcsr;
Don't we also need vlen to adequately determine the vector state? Otherwise we're going to end up dropping some state when vl isn't vlmax, which IIUC isn't legal.
arch/riscv/include/uapi/asm/ptrace.h union __riscv_fp_state { · line 88
Quoted 4 lines
arch/riscv/include/uapi/asm/ptrace.h +4
-0+void *datap; +#if __riscv_xlen ==32 +__u32 __padding; +#endif
Why is there padding?
Quoted 22 lines
arch/riscv/include/uapi/asm/ptrace.h +2
-0+}; +#endif /* __ASSEMBLY__ */ #endif /* _UAPI_ASM_RISCV_PTRACE_H */ arch/riscv/kernel/asm-offsets.c +6
-0@@ -72,6 +72,12 @@ void asm_offsets(void)OFFSET (TSK_STACK_CANARY , task_struct, stack_canary);#endif +OFFSET (RISCV_V_STATE_VSTART , __riscv_v_state, vstart); +OFFSET (RISCV_V_STATE_VL , __riscv_v_state, vl); +OFFSET (RISCV_V_STATE_VTYPE , __riscv_v_state, vtype); +OFFSET (RISCV_V_STATE_VCSR , __riscv_v_state, vcsr); +OFFSET (RISCV_V_STATE_DATAP , __riscv_v_state, datap); +DEFINE (PT_SIZE ,sizeof (struct pt_regs ));OFFSET (PT_EPC , pt_regs, epc);OFFSET (PT_RA , pt_regs, ra);
Re: [PATCH v9 08/17] riscv: Add vector struct and assembler definitions
Palmer Dabbelt <[email protected]> 於 2021年12月15日 週三 上午12:29寫道：
Quoted 38 lines · On Tue, 09 Nov 2021 01:48:20 PST (-0800), [email protected] wrote:
Quoted 36 lines
Add vector state context struct in struct thread and
asm-offsets.cdefinitions.The vector registers will be saved in datap pointer of
__riscv_v_state. It will be dynamically allocated in kernel space. It will be put right after the__riscv_v_statedata structure in user space.Co-developed-by Vincent Chen <[email protected]>Signed-off-by Vincent Chen <[email protected]>Signed-off-by Greentime Hu <[email protected]>--- arch/riscv/include/asm/processor.h | 1 + arch/riscv/include/uapi/asm/ptrace.h | 11 +++++++++++ arch/riscv/kernel/asm-offsets.c | 6 ++++++ 3 files changed, 18 insertions(+)arch/riscv/include/asm/processor.h +1
-0@@ -35,6 +35,7 @@ struct thread_struct {unsigned long s[12 ];/* s[0]: frame pointer */ struct __riscv_d_ext_state fstate;unsigned long bad_cause; +struct __riscv_v_state vstate; };/* Whitelist the fstate from the task_struct for hardened usercopy */ arch/riscv/include/uapi/asm/ptrace.h +0
-0@@ -77,6 +77,17 @@ union __riscv_fp_state {struct __riscv_q_ext_state q; };
Quoted 9 lines
arch/riscv/include/uapi/asm/ptrace.h union __riscv_fp_state { · line 84
Quoted 5 lines
arch/riscv/include/uapi/asm/ptrace.h +5
-0+struct __riscv_v_state { +unsigned long vstart; +unsigned long vl; +unsigned long vtype; +unsigned long vcsr;Don't we also need vlen to adequately determine the vector state? Otherwise we're going to end up dropping some state when vl isn't vlmax, which IIUC isn't legal.
Do you mean vlenb? Since it is a constant value, we don't need to save/restore it in the context.
Quoted 6 lines
arch/riscv/include/uapi/asm/ptrace.h union __riscv_fp_state { · line 88
Quoted 4 lines
arch/riscv/include/uapi/asm/ptrace.h +4
-0+void *datap; +#if __riscv_xlen ==32 +__u32 __padding; +#endif Why is there padding?
To keep vector registers saved in a 16-bytes aligned address for rv32.
struct __riscv_ctx_hdr {
__u32 magic;
__u32 size;
};
struct __sc_riscv_v_state {
struct __riscv_ctx_hdr head;
struct __riscv_v_state v_state;
} __attribute__((aligned(16)));rv64 => 48bytes -> 16byte aligned
rv32 => 32bytes -> 16byte aligned
This struct and vector registers will be copied to sigcontext.reserved[] for signal handler so we'd like to keep it is 16-byte aligned.
struct sigcontext {
struct user_regs_struct sc_regs;
union __riscv_fp_state sc_fpregs;
/*
* 4K + 128 reserved for vector state and future expansion.
* This space is enough to store the vector context whose VLENB
* is less or equal to 128.
* (The size of the vector context is 4144 byte as VLENB is 128)
*/
__u8 __reserved[4224] __attribute__((__aligned__(16)));
};Quoted 22 lines
Quoted 22 lines
arch/riscv/include/uapi/asm/ptrace.h +2
-0+}; +#endif /* __ASSEMBLY__ */ #endif /* _UAPI_ASM_RISCV_PTRACE_H */ arch/riscv/kernel/asm-offsets.c +6
-0@@ -72,6 +72,12 @@ void asm_offsets(void)OFFSET (TSK_STACK_CANARY , task_struct, stack_canary);#endif +OFFSET (RISCV_V_STATE_VSTART , __riscv_v_state, vstart); +OFFSET (RISCV_V_STATE_VL , __riscv_v_state, vl); +OFFSET (RISCV_V_STATE_VTYPE , __riscv_v_state, vtype); +OFFSET (RISCV_V_STATE_VCSR , __riscv_v_state, vcsr); +OFFSET (RISCV_V_STATE_DATAP , __riscv_v_state, datap); +DEFINE (PT_SIZE ,sizeof (struct pt_regs ));OFFSET (PT_EPC , pt_regs, epc);OFFSET (PT_RA , pt_regs, ra);
Re: [PATCH v9 08/17] riscv: Add vector struct and assembler definitions
Quoted 53 lines · Greentime Hu
Palmer Dabbelt <[email protected]> 於 2021年12月15日 週三 上午12:29寫道：
Quoted 38 lines · On Tue, 09 Nov 2021 01:48:20 PST (-0800), [email protected] wrote:
Quoted 36 lines
Add vector state context struct in struct thread and
asm-offsets.cdefinitions.The vector registers will be saved in datap pointer of
__riscv_v_state. It will be dynamically allocated in kernel space. It will be put right after the__riscv_v_statedata structure in user space.Co-developed-by Vincent Chen <[email protected]>Signed-off-by Vincent Chen <[email protected]>Signed-off-by Greentime Hu <[email protected]>---
arch/riscv/include/asm/processor.h | 1 + arch/riscv/include/uapi/asm/ptrace.h | 11 +++++++++++ arch/riscv/kernel/asm-offsets.c | 6 ++++++ 3 files changed, 18 insertions(+)arch/riscv/include/asm/processor.h +1
-0@@ -35,6 +35,7 @@ struct thread_struct {unsigned long s[12 ];/* s[0]: frame pointer */ struct __riscv_d_ext_state fstate;unsigned long bad_cause; +struct __riscv_v_state vstate; };/* Whitelist the fstate from the task_struct for hardened usercopy */ arch/riscv/include/uapi/asm/ptrace.h +0
-0@@ -77,6 +77,17 @@ union __riscv_fp_state {struct __riscv_q_ext_state q; };Quoted 9 lines
arch/riscv/include/uapi/asm/ptrace.h union __riscv_fp_state { · line 84
Quoted 5 lines
arch/riscv/include/uapi/asm/ptrace.h +5
-0+struct __riscv_v_state { +unsigned long vstart; +unsigned long vl; +unsigned long vtype; +unsigned long vcsr;Don't we also need vlen to adequately determine the vector state? Otherwise we're going to end up dropping some state when vl isn't vlmax, which IIUC isn't legal.
Do you mean vlenb? Since it is a constant value, we don't need to save/restore it in the context.
Quoted 8 lines
Quoted 6 lines
arch/riscv/include/uapi/asm/ptrace.h union __riscv_fp_state { · line 88
Quoted 4 lines
arch/riscv/include/uapi/asm/ptrace.h +4
-0+void *datap; +#if __riscv_xlen ==32 +__u32 __padding; +#endif Why is there padding?
To keep vector registers saved in a 16-bytes aligned address for rv32.
That struct has an alignment of 4 bytes. It doesn’t make sense to put the padding there; it should be wherever the 16 byte alignment is introduced, which looks like your __sc_riscv_v_state below (assuming you need to explicitly name the padding and can’t just rely on implicit compiler padding; presumably you need it so you can guarantee it’s zero when written to userspace memory?).
Especially since the amount of padding you need in __riscv_v_state if doing it this way depends on the size of __riscv_ctx_hdr, because that happens to be in __sc_riscv_v_state. This is quite fragile and non-obvious as it stands.
Jess
Quoted 56 lines
struct __riscv_ctx_hdr { __u32 magic; __u32 size; }; struct __sc_riscv_v_state { struct __riscv_ctx_hdr head; struct __riscv_v_state v_state; } __attribute__((aligned(16)));rv64 => 48bytes -> 16byte aligned
rv32 => 32bytes -> 16byte alignedThis struct and vector registers will be copied to sigcontext.reserved[] for signal handler so we'd like to keep it is 16-byte aligned.
struct sigcontext { struct user_regs_struct sc_regs; union __riscv_fp_state sc_fpregs; /* * 4K + 128 reserved for vector state and future expansion. * This space is enough to store the vector context whose VLENB * is less or equal to 128. * (The size of the vector context is 4144 byte as VLENB is 128) */ __u8 __reserved[4224] __attribute__((__aligned__(16))); };Quoted 13 lines
Quoted 13 lines
arch/riscv/include/uapi/asm/ptrace.h +2
-0+}; +#endif /* __ASSEMBLY__ */ #endif /* _UAPI_ASM_RISCV_PTRACE_H */ arch/riscv/kernel/asm-offsets.c +0
-0@@ -72,6 +72,12 @@ void asm_offsets(void)OFFSET (TSK_STACK_CANARY , task_struct, stack_canary);#endif Quoted 9 lines
arch/riscv/kernel/asm-offsets.c void asm_offsets(void) · line 83
Quoted 9 lines
arch/riscv/kernel/asm-offsets.c +6
-0+OFFSET (RISCV_V_STATE_VSTART , __riscv_v_state, vstart); +OFFSET (RISCV_V_STATE_VL , __riscv_v_state, vl); +OFFSET (RISCV_V_STATE_VTYPE , __riscv_v_state, vtype); +OFFSET (RISCV_V_STATE_VCSR , __riscv_v_state, vcsr); +OFFSET (RISCV_V_STATE_DATAP , __riscv_v_state, datap); +DEFINE (PT_SIZE ,sizeof (struct pt_regs ));OFFSET (PT_EPC , pt_regs, epc);OFFSET (PT_RA , pt_regs, ra);_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
PATCH v9 9/17 riscv: Add task switch support for vector
In reply to Greentime HuThis patch adds task switch support for vector. It supports partial lazy save and restore mechanism. It also supports all lengths of vlen.
[[email protected]: First available porting to support vector context switching]
[[email protected]: Rewrite vector.S to support dynamic vlen, xlen and code refine]
[[email protected]: Fix the might_sleep issue in vstate_save, vstate_restore]
[[email protected]: Optimize task switch codes of vector]
--- arch/riscv/include/asm/switch_to.h | 65 ++++++++++++++++++++++++ arch/riscv/kernel/Makefile | 1 + arch/riscv/kernel/process.c | 49 ++++++++++++++++++ arch/riscv/kernel/vector.S | 81 ++++++++++++++++++++++++++++++ 4 files changed, 196 insertions(+) create mode 100644 arch/riscv/kernel/vector.S
arch/riscv/include/asm/switch_to.h +65 -0
@@ -7,10 +7,12 @@#define _ASM_RISCV_SWITCH_TO_H#include <linux/jump_label.h> +#include <linux/slab.h> #include <linux/sched/task_stack.h> #include <asm/processor.h> #include <asm/ptrace.h> #include <asm/csr.h> +#include <asm/asm-offsets.h> #ifdef CONFIG_FPU extern void __fstate_save (struct task_struct *save_to);
@@ -68,6 +70,67 @@ static __always_inline bool has_fpu(void) { return false; }#define __switch_to_fpu (__prev, __next) do { } while (0)#endif +#ifdef CONFIG_VECTOR +extern struct static_key_false cpu_hwcap_vector; +static __always_inline bool has_vector(void) +{ +return static_branch_likely (&cpu_hwcap_vector); +} +extern unsigned long riscv_vsize; +extern void __vstate_save (struct __riscv_v_state *save_to,void *datap); +extern void __vstate_restore (struct __riscv_v_state *restore_from,void *datap); + +static inline void __vstate_clean (struct pt_regs *regs) +{ + regs->status = (regs->status & ~(SR_VS )) |SR_VS_CLEAN ; +} + +static inline void vstate_off (struct task_struct *task, +struct pt_regs *regs) +{ + regs->status = (regs->status & ~SR_VS ) |SR_VS_OFF ; +} + +static inline void vstate_save (struct task_struct *task, +struct pt_regs *regs) +{ +if ((regs->status &SR_VS ) ==SR_VS_DIRTY ) { +struct __riscv_v_state *vstate = &(task->thread.vstate); + +__vstate_save (vstate, vstate->datap); +__vstate_clean (regs); + } +} + +static inline void vstate_restore (struct task_struct *task, +struct pt_regs *regs) +{ +if ((regs->status &SR_VS ) !=SR_VS_OFF ) { +struct __riscv_v_state *vstate = &(task->thread.vstate); +__vstate_restore (vstate, vstate->datap); +__vstate_clean (regs); + } +} + +static inline void __switch_to_vector (struct task_struct *prev, +struct task_struct *next) +{ +struct pt_regs *regs; + + regs =task_pt_regs (prev); +if (unlikely (regs->status &SR_SD )) +vstate_save (prev, regs); +vstate_restore (next,task_pt_regs (next)); +} + +#else +static __always_inline bool has_vector(void) { return false; } +#define riscv_vsize (0) +#define vstate_save (task, regs) do { } while (0) +#define vstate_restore (task, regs) do { } while (0) +#define __switch_to_vector (__prev, __next) do { } while (0) +#endif +extern struct task_struct *__switch_to (struct task_struct *,struct task_struct *);
@@ -77,6 +140,8 @@ do { \struct task_struct *__next = (next); \if (has_fpu ()) \__switch_to_fpu (__prev, __next); \ +if (has_vector ()) \ +__switch_to_vector (__prev, __next); \ ((last) =__switch_to (__prev, __next)); \ }while (0 )
arch/riscv/kernel/Makefile +1 -0
@@ -40,6 +40,7 @@ obj-$(CONFIG_MMU) += vdso.o vdso/ obj-$(CONFIG_RISCV_M_MODE) += traps_misaligned.o obj-$(CONFIG_FPU) += fpu.o +obj-$(CONFIG_VECTOR) += vector.o obj-$(CONFIG_SMP) += smpboot.o obj-$(CONFIG_SMP) += smp.o obj-$(CONFIG_SMP) += cpu_ops.o
arch/riscv/kernel/process.c +49 -0
70 lines not on this page · raw message
arch/riscv/kernel/vector.S new file +81 -0
81 lines not on this page · raw message
Signature · 1 line
2.31.1
kernel test robot
build botRe: [PATCH v9 09/17] riscv: Add task switch support for vector
I love your patch! Yet something to improve:
Hi Greentime,
I love your patch! Yet something to improve:
[auto build test ERROR on v5.15]
[cannot apply to linus/master next-20211111]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Greentime-Hu/riscv-Add-vector-ISA-support/20211109-175222 base: DEBUG invalid remote for branch v5.15 8bb7eca972ad531c9b149c0a51ab43a417385813 config: riscv-buildonly-randconfig-r002-20211111 (attached as .config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 63ef0e17e28827eae53133b3467bdac7d9729318)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/93773de66911ee019c1fb31ae8e53a1221a540db
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Greentime-Hu/riscv-Add-vector-ISA-support/20211109-175222 git checkout 93773de66911ee019c1fb31ae8e53a1221a540db
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
>> arch/riscv/kernel/process.c:164:6: error: no previous prototype for function 'arch_release_task_struct' [-Werror,-Wmissing-prototypes]
void arch_release_task_struct(struct task_struct *tsk)
^ arch/riscv/kernel/process.c:164:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void arch_release_task_struct(struct task_struct *tsk)
^
static
1 error generated.
vim +/arch_release_task_struct +164 arch/riscv/kernel/process.c
163
> 164 void arch_release_task_struct(struct task_struct *tsk)
165 {
166 /* Free the vector context of datap. */
167 if (has_vector())
168 kfree(tsk->thread.vstate.datap);
169 }
170
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
kernel test robot
build botRe: [PATCH v9 09/17] riscv: Add task switch support for vector
In reply to Greentime HuI love your patch! Perhaps something to improve:
Hi Greentime,
I love your patch! Perhaps something to improve:
[auto build test WARNING on v5.15]
[cannot apply to linus/master next-20211111]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Greentime-Hu/riscv-Add-vector-ISA-support/20211109-175222 base: DEBUG invalid remote for branch v5.15 8bb7eca972ad531c9b149c0a51ab43a417385813 config: riscv-nommu_k210_defconfig (attached as .config) compiler: riscv64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/93773de66911ee019c1fb31ae8e53a1221a540db
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Greentime-Hu/riscv-Add-vector-ISA-support/20211109-175222 git checkout 93773de66911ee019c1fb31ae8e53a1221a540db
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
>> arch/riscv/kernel/process.c:164:6: warning: no previous prototype for 'arch_release_task_struct' [-Wmissing-prototypes]
164 | void arch_release_task_struct(struct task_struct *tsk)
| ^~~~~~~~~~~~~~~~~~~~~~~~
vim +/arch_release_task_struct +164 arch/riscv/kernel/process.c
163
> 164 void arch_release_task_struct(struct task_struct *tsk)
165 {
166 /* Free the vector context of datap. */
167 if (has_vector())
168 kfree(tsk->thread.vstate.datap);
169 }
170
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Palmer Dabbelt
maintainerRe: [PATCH v9 09/17] riscv: Add task switch support for vector
In reply to Greentime HuQuoted 172 lines · On Tue, 09 Nov 2021 01:48:21 PST (-0800), [email protected] wrote:
This patch adds task switch support for vector. It supports partial lazy save and restore mechanism. It also supports all lengths of vlen.
[[email protected]: First available porting to support vector context switching]
[[email protected]: Rewritevector.Sto support dynamic vlen, xlen and code refine]
[[email protected]: Fix themight_sleepissue invstate_save,vstate_restore]
[[email protected]: Optimize task switch codes of vector]Suggested-by Andrew Waterman <[email protected]>Co-developed-by Nick Knight <[email protected]>Signed-off-by Nick Knight <[email protected]>Co-developed-by Guo Ren <[email protected]>Signed-off-by Guo Ren <[email protected]>Co-developed-by Vincent Chen <[email protected]>Signed-off-by Vincent Chen <[email protected]>Signed-off-by Greentime Hu <[email protected]>--- arch/riscv/include/asm/switch_to.h | 65 ++++++++++++++++++++++++ arch/riscv/kernel/Makefile | 1 + arch/riscv/kernel/process.c | 49 ++++++++++++++++++ arch/riscv/kernel/vector.S | 81 ++++++++++++++++++++++++++++++ 4 files changed, 196 insertions(+) create mode 100644 arch/riscv/kernel/vector.Sarch/riscv/include/asm/switch_to.h +65
-0@@ -7,10 +7,12 @@#define _ASM_RISCV_SWITCH_TO_H#include <linux/jump_label.h> +#include <linux/slab.h> #include <linux/sched/task_stack.h> #include <asm/processor.h> #include <asm/ptrace.h> #include <asm/csr.h> +#include <asm/asm-offsets.h> #ifdef CONFIG_FPU extern void __fstate_save (struct task_struct *save_to);@@ -68,6 +70,67 @@ static __always_inline bool has_fpu(void) { return false; }#define __switch_to_fpu (__prev, __next) do { } while (0)#endif +#ifdef CONFIG_VECTOR +extern struct static_key_false cpu_hwcap_vector; +static __always_inline bool has_vector(void) +{ +return static_branch_likely (&cpu_hwcap_vector); +} +extern unsigned long riscv_vsize; +extern void __vstate_save (struct __riscv_v_state *save_to,void *datap); +extern void __vstate_restore (struct __riscv_v_state *restore_from,void *datap); + +static inline void __vstate_clean (struct pt_regs *regs) +{ + regs->status = (regs->status & ~(SR_VS )) |SR_VS_CLEAN ; +} + +static inline void vstate_off (struct task_struct *task, +struct pt_regs *regs) +{ + regs->status = (regs->status & ~SR_VS ) |SR_VS_OFF ; +} + +static inline void vstate_save (struct task_struct *task, +struct pt_regs *regs) +{ +if ((regs->status &SR_VS ) ==SR_VS_DIRTY ) { +struct __riscv_v_state *vstate = &(task->thread.vstate); + +__vstate_save (vstate, vstate->datap); +__vstate_clean (regs); + } +} + +static inline void vstate_restore (struct task_struct *task, +struct pt_regs *regs) +{ +if ((regs->status &SR_VS ) !=SR_VS_OFF ) { +struct __riscv_v_state *vstate = &(task->thread.vstate); +__vstate_restore (vstate, vstate->datap); +__vstate_clean (regs); + } +} + +static inline void __switch_to_vector (struct task_struct *prev, +struct task_struct *next) +{ +struct pt_regs *regs; + + regs =task_pt_regs (prev); +if (unlikely (regs->status &SR_SD )) +vstate_save (prev, regs); +vstate_restore (next,task_pt_regs (next)); +} + +#else +static __always_inline bool has_vector(void) { return false; } +#define riscv_vsize (0) +#define vstate_save (task, regs) do { } while (0) +#define vstate_restore (task, regs) do { } while (0) +#define __switch_to_vector (__prev, __next) do { } while (0) +#endif +extern struct task_struct *__switch_to (struct task_struct *,struct task_struct *);@@ -77,6 +140,8 @@ do { \struct task_struct *__next = (next); \if (has_fpu ()) \__switch_to_fpu (__prev, __next); \ +if (has_vector ()) \ +__switch_to_vector (__prev, __next); \ ((last) =__switch_to (__prev, __next)); \ }while (0 )arch/riscv/kernel/Makefile +1
-0@@ -40,6 +40,7 @@ obj-$(CONFIG_MMU) += vdso.o vdso/ obj-$(CONFIG_RISCV_M_MODE) += traps_misaligned.o obj-$(CONFIG_FPU) += fpu.o +obj-$(CONFIG_VECTOR) += vector.o obj-$(CONFIG_SMP) += smpboot.o obj-$(CONFIG_SMP) += smp.o obj-$(CONFIG_SMP) += cpu_ops.oarch/riscv/kernel/process.c +24
-0@@ -95,6 +95,25 @@ void start_thread(struct pt_regs *regs, unsigned long pc,*/ fstate_restore (current, regs); } + +if (has_vector ()) { +struct __riscv_v_state *vstate = &(current->thread.vstate); + +/* Enable vector and allocate memory for vector registers. */ +if (!vstate->datap) { + vstate->datap =kzalloc (riscv_vsize,GFP_KERNEL ); +if (WARN_ON (!vstate->datap)) +return ; + } + regs->status |=SR_VS_INITIAL ; + +/* +* Restore the initial value to the vector register +* before starting the user program. +*/ +vstate_restore (current, regs); + } + regs->epc = pc; regs->sp = sp; }@@ -110,15 +129,45 @@ void flush_thread(void)fstate_off (current,task_pt_regs (current));memset (¤t->thread.fstate,0 ,sizeof (current->thread.fstate));#endif +#ifdef CONFIG_VECTOR +/* Reset vector state and keep datap pointer. */ +vstate_off (current,task_pt_regs (current)); +memset (¤t->thread.vstate,0 ,RISCV_V_STATE_DATAP ); +if (current->thread.vstate.datap)
arch/riscv/kernel/process.c void flush_thread(void) · line 146
Quoted 10 lines
arch/riscv/kernel/process.c +5
-0+memset (current->thread.vstate.datap,0 , riscv_vsize); +#endif }int arch_dup_task_struct (struct task_struct *dst,struct task_struct *src) {fstate_save (src,task_pt_regs (src)); +if (has_vector ()) +/* To make sure every dirty vector context is saved. */ +vstate_save (src,task_pt_regs (src));
Isn't the has_vector() check unnecessary? It looks to be thes ame as fstate_save(), where we can just rely in the internal check.
arch/riscv/kernel/process.c void flush_thread(void) · line 149
Quoted 3 lines
arch/riscv/kernel/process.c +2
-0*dst = *src; +if (has_vector ()) { +/* Copy vector context to the forked task from parent. */
Unless I'm missing something, this shouldn't be necessary: we got here via some syscall, and since those all follow the standard ABI the V registers have all been clobbered. Thus we should be able to just toss the vector state here. arm64 goes a step farther and explicitly tosses the SVE state on syscalls, which seems reasonable to me as that'd allow us to avoid some unnecessary save/restore overhead.
A few bits of the V state are defined with thread-duration storage, so we'd need to split those out to be saved (I guess we could define them as clobbered by syscalls, but that seems like the wrong way to go to me).
Quoted 111 lines
arch/riscv/kernel/process.c +18
-0+if ((task_pt_regs (src)->status &SR_VS ) !=SR_VS_OFF ) { + dst->thread.vstate.datap =kzalloc (riscv_vsize,GFP_KERNEL ); +/* Failed to allocate memory. */ +if (!dst->thread.vstate.datap) +return -ENOMEM ; +/* Copy the src vector context to dst. */ +memcpy (dst->thread.vstate.datap, + src->thread.vstate.datap, riscv_vsize); + } + } +return 0 ; } +void arch_release_task_struct (struct task_struct *tsk) +{ +/* Free the vector context of datap. */ +if (has_vector ()) +kfree (tsk->thread.vstate.datap); +} +int copy_thread (unsigned long clone_flags,unsigned long usp,unsigned long arg,struct task_struct *p,unsigned long tls) {arch/riscv/kernel/vector.S new file +81
-0@@ -0,0 +1,81 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2012 Regents of the University of California + * Copyright (C) 2017 SiFive + * Copyright (C) 2019 Alibaba Group Holding Limited + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation, version 2. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/linkage.h> + +#include <asm/asm.h> +#include <asm/csr.h> +#include <asm/asm-offsets.h> + +#define vstatep a0 +#define datap a1 +#define x_vstart t0 +#define x_vtype t1 +#define x_vl t2 +#define x_vcsr t3 +#define incr t4 +#define status t5 + +ENTRY(__vstate_save) + li status, SR_VS + csrs sstatus, status + + csrr x_vstart, CSR_VSTART + csrr x_vtype, CSR_VTYPE + csrr x_vl, CSR_VL + csrr x_vcsr, CSR_VCSR + vsetvli incr, x0, e8, m8 + vse8.v v0, (datap) + add datap, datap, incr + vse8.v v8, (datap) + add datap, datap, incr + vse8.v v16, (datap) + add datap, datap, incr + vse8.v v24, (datap) + + REG_S x_vstart, RISCV_V_STATE_VSTART(vstatep) + REG_S x_vtype, RISCV_V_STATE_VTYPE(vstatep) + REG_S x_vl, RISCV_V_STATE_VL(vstatep) + REG_S x_vcsr, RISCV_V_STATE_VCSR(vstatep) + + csrc sstatus, status + ret +ENDPROC(__vstate_save) + +ENTRY(__vstate_restore) + li status, SR_VS + csrs sstatus, status + + vsetvli incr, x0, e8, m8 + vle8.v v0, (datap) + add datap, datap, incr + vle8.v v8, (datap) + add datap, datap, incr + vle8.v v16, (datap) + add datap, datap, incr + vle8.v v24, (datap) + + REG_L x_vstart, RISCV_V_STATE_VSTART(vstatep) + REG_L x_vtype, RISCV_V_STATE_VTYPE(vstatep) + REG_L x_vl, RISCV_V_STATE_VL(vstatep) + REG_L x_vcsr, RISCV_V_STATE_VCSR(vstatep) + vsetvl x0, x_vl, x_vtype + csrw CSR_VSTART, x_vstart + csrw CSR_VCSR, x_vcsr + + csrc sstatus, status + ret +ENDPROC(__vstate_restore)
PATCH v9 10/17 riscv: Add ptrace vector support
In reply to Greentime HuThis patch adds ptrace support for riscv vector. The vector registers will be saved in datap pointer of __riscv_v_state. This pointer will be set right after the __riscv_v_state data structure then it will be put in ubuf for ptrace system call to get or set. It will check if the datap got from ubuf is set to the correct address or not when the ptrace system call is trying to set the vector registers.
--- arch/riscv/include/uapi/asm/ptrace.h | 14 ++++++ arch/riscv/kernel/ptrace.c | 71 ++++++++++++++++++++++++++++ include/uapi/linux/elf.h | 1 + 3 files changed, 86 insertions(+)
arch/riscv/include/uapi/asm/ptrace.h +14 -0
@@ -83,11 +83,25 @@ struct __riscv_v_state {unsigned long vtype;unsigned long vcsr;void *datap; +/* +* In signal handler, datap will be set a correct user stack offset +* and vector registers will be copied to the address of datap +* pointer. +* +* In ptrace syscall, datap will be set to zero and the vector +* registers will be copied to the address right after this +* structure. +*/ #if __riscv_xlen ==32 __u32 __padding;#endif }; +/* +* To define a practical maximum vlenb for ptrace and it may need to be +* extended someday. +*/ +#define RISCV_MAX_VLENB (16384)#endif /* __ASSEMBLY__ */ #endif /* _UAPI_ASM_RISCV_PTRACE_H */
arch/riscv/kernel/ptrace.c +71 -0
89 lines not on this page · raw message
include/uapi/linux/elf.h +1 -0
@@ -432,6 +432,7 @@ typedef struct elf64_shdr {#define NT_MIPS_DSP 0x800/* MIPS DSP ASE registers */ #define NT_MIPS_FP_MODE 0x801/* MIPS floating-point mode */ #define NT_MIPS_MSA 0x802/* MIPS SIMD registers */ +#define NT_RISCV_VECTOR 0x900/* RISC-V vector registers */ /* Note types with note name "GNU" */ #define NT_GNU_PROPERTY_TYPE_0 5
Signature · 1 line
2.31.1
PATCH v9 11/17 riscv: Add sigcontext save/restore for vector
In reply to Greentime HuThis patch adds sigcontext save/restore for vector. The vector registers will be saved in datap pointer. The datap pointer will be allocated 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.
--- arch/riscv/include/uapi/asm/sigcontext.h | 24 ++++ arch/riscv/kernel/asm-offsets.c | 2 + arch/riscv/kernel/setup.c | 4 + arch/riscv/kernel/signal.c | 165 ++++++++++++++++++++++- 4 files changed, 191 insertions(+), 4 deletions(-)
arch/riscv/include/uapi/asm/sigcontext.h +24 -0
@@ -8,6 +8,23 @@#include <asm/ptrace.h> +/* The Magic number for signal context frame header. */ +#define RVV_MAGIC 0x53465457 +#define END_MAGIC 0x0 + +/* The size of END signal context header. */ +#define END_HDR_SIZE 0x0 + +struct __riscv_ctx_hdr { +__u32 magic; +__u32 size; +}; + +struct __sc_riscv_v_state { +struct __riscv_ctx_hdr head; +struct __riscv_v_state v_state; +} __attribute__((aligned (16 ))); +/* * Signal context structure *
@@ -17,6 +34,13 @@struct sigcontext {struct user_regs_struct sc_regs;union __riscv_fp_state sc_fpregs; +/* +* 4K + 128 reserved for vector state and future expansion. +* This space is enough to store the vector context whose VLENB +* is less or equal to 128. +* (The size of the vector context is 4144 byte as VLENB is 128) +*/ +__u8 __reserved[4224 ] __attribute__((__aligned__ (16 ))); };#endif /* _UAPI_ASM_RISCV_SIGCONTEXT_H */
arch/riscv/kernel/asm-offsets.c +2 -0
@@ -72,6 +72,8 @@ void asm_offsets(void)OFFSET (TSK_STACK_CANARY , task_struct, stack_canary);#endif +OFFSET (RISCV_V_STATE_MAGIC , __riscv_ctx_hdr, magic); +OFFSET (RISCV_V_STATE_SIZE , __riscv_ctx_hdr, size);OFFSET (RISCV_V_STATE_VSTART , __riscv_v_state, vstart);OFFSET (RISCV_V_STATE_VL , __riscv_v_state, vl);OFFSET (RISCV_V_STATE_VTYPE , __riscv_v_state, vtype);
arch/riscv/kernel/setup.c +4 -0
@@ -260,6 +260,8 @@ static void __init parse_dtb(void)#endif } +extern void __init init_rt_signal_env (void ); +void __init setup_arch (char **cmdline_p) {parse_dtb ();
@@ -295,6 +297,8 @@ void __init setup_arch(char **cmdline_p)#endif riscv_fill_hwcap (); + +init_rt_signal_env (); }static int __init topology_init (void )
arch/riscv/kernel/signal.c +161 -4
228 lines not on this page · raw message
Signature · 1 line
2.31.1
PATCH v9 12/17 riscv: signal: Report signal frame size to userspace via auxv
In reply to Greentime HuFrom: Vincent Chen <[email protected]>
The vector register belongs to the signal context. They need to be stored and restored as entering and leaving the signal handler. According to the V-extension specification, the maximum length of the vector registers can be 2^(XLEN-1). Hence, if userspace refers to the MINSIGSTKSZ to create a sigframe, it may not be enough. To resolve this problem, this patch refers to the commit 94b07c1f8c39c
("arm64: signal: Report signal frame size to userspace via auxv") to enable userspace to know the minimum required sigframe size through the auxiliary vector and use it to allocate enough memory for signal context.
--- arch/riscv/include/asm/elf.h | 41 +++++++++++++++++----------- arch/riscv/include/asm/processor.h | 2 ++ arch/riscv/include/uapi/asm/auxvec.h | 1 + arch/riscv/kernel/signal.c | 8 ++++++ 4 files changed, 36 insertions(+), 16 deletions(-)
arch/riscv/include/asm/elf.h +25 -16
@@ -61,22 +61,31 @@ extern unsigned long elf_hwcap;#define ELF_PLATFORM (NULL)#ifdef CONFIG_MMU -#define ARCH_DLINFO \ -do { \ - NEW_AUX_ENT(AT_SYSINFO_EHDR, \ - (elf_addr_t)current->mm->context.vdso); \ - NEW_AUX_ENT(AT_L1I_CACHESIZE, \ - get_cache_size(1, CACHE_TYPE_INST)); \ - NEW_AUX_ENT(AT_L1I_CACHEGEOMETRY, \ - get_cache_geometry(1, CACHE_TYPE_INST)); \ - NEW_AUX_ENT(AT_L1D_CACHESIZE, \ - get_cache_size(1, CACHE_TYPE_DATA)); \ - NEW_AUX_ENT(AT_L1D_CACHEGEOMETRY, \ - get_cache_geometry(1, CACHE_TYPE_DATA)); \ - NEW_AUX_ENT(AT_L2_CACHESIZE, \ - get_cache_size(2, CACHE_TYPE_UNIFIED)); \ - NEW_AUX_ENT(AT_L2_CACHEGEOMETRY, \ - get_cache_geometry(2, CACHE_TYPE_UNIFIED)); \ +#define ARCH_DLINFO \ +do { \ + NEW_AUX_ENT(AT_SYSINFO_EHDR, \ + (elf_addr_t)current->mm->context.vdso); \ + NEW_AUX_ENT(AT_L1I_CACHESIZE, \ + get_cache_size(1, CACHE_TYPE_INST)); \ + NEW_AUX_ENT(AT_L1I_CACHEGEOMETRY, \ + get_cache_geometry(1, CACHE_TYPE_INST)); \ + NEW_AUX_ENT(AT_L1D_CACHESIZE, \ + get_cache_size(1, CACHE_TYPE_DATA)); \ + NEW_AUX_ENT(AT_L1D_CACHEGEOMETRY, \ + get_cache_geometry(1, CACHE_TYPE_DATA)); \ + NEW_AUX_ENT(AT_L2_CACHESIZE, \ + get_cache_size(2, CACHE_TYPE_UNIFIED)); \ + NEW_AUX_ENT(AT_L2_CACHEGEOMETRY, \ + get_cache_geometry(2, CACHE_TYPE_UNIFIED)); \ +/* \ +* Should always be nonzero unless there's a kernel bug. \ +* If we haven't determined a sensible value to give to \ +* userspace, omit the entry: \ +*/ \ +if (likely (signal_minsigstksz)) \ +NEW_AUX_ENT (AT_MINSIGSTKSZ , signal_minsigstksz); \ +else \ +NEW_AUX_ENT (AT_IGNORE ,0 ); \ } while (0)#define ARCH_HAS_SETUP_ADDITIONAL_PAGES struct linux_binprm ;
arch/riscv/include/asm/processor.h +2 -0
@@ -7,6 +7,7 @@#define _ASM_RISCV_PROCESSOR_H#include <linux/const.h> +#include <linux/cache.h> #include <vdso/processor.h>
@@ -82,6 +83,7 @@ int riscv_of_parent_hartid(struct device_node *node);extern void riscv_fill_hwcap (void );extern int arch_dup_task_struct (struct task_struct *dst,struct task_struct *src); +extern unsigned long signal_minsigstksz__ro_after_init ;#endif /* __ASSEMBLY__ */ #endif /* _ASM_RISCV_PROCESSOR_H */
arch/riscv/include/uapi/asm/auxvec.h +1 -0
@@ -33,5 +33,6 @@/* entries in ARCH_DLINFO */ #define AT_VECTOR_SIZE_ARCH 7 +#define AT_MINSIGSTKSZ 51#endif /* _UAPI_ASM_RISCV_AUXVEC_H */
arch/riscv/kernel/signal.c +8 -0
@@ -471,8 +471,16 @@ asmlinkage __visible void do_notify_resume(struct pt_regs *regs,tracehook_notify_resume (regs); } +unsigned long __ro_after_init signal_minsigstksz; +void init_rt_signal_env (void );void __init init_rt_signal_env (void ) { rvv_sc_size =sizeof (struct __sc_riscv_v_state ) + riscv_vsize; +/* +* Determine the stack space required for guaranteed signal delivery. +* The signal_minsigstksz will be populated into the AT_MINSIGSTKSZ entry +* in the auxiliary array at process startup. +*/ + signal_minsigstksz =cal_rt_frame_size (); }
Signature · 1 line
2.31.1
PATCH v9 13/17 riscv: Add support for kernel mode vector
In reply to Greentime HuAdd <asm/vector.h> containing kernel_rvv_begin()/kernel_rvv_end() function declarations and corresponding definitions in kernel_mode_vector.c
These are needed to wrap uses of vector in kernel mode.
--- arch/riscv/include/asm/switch_to.h | 1 + arch/riscv/include/asm/vector.h | 14 +++ arch/riscv/kernel/Makefile | 1 + arch/riscv/kernel/kernel_mode_vector.c | 144 +++++++++++++++++++++++++ arch/riscv/kernel/vector.S | 9 ++ 5 files changed, 169 insertions(+) create mode 100644 arch/riscv/include/asm/vector.h create mode 100644 arch/riscv/kernel/kernel_mode_vector.c
arch/riscv/include/asm/switch_to.h +1 -0
@@ -79,6 +79,7 @@ static __always_inline bool has_vector(void)extern unsigned long riscv_vsize;extern void __vstate_save (struct __riscv_v_state *save_to,void *datap);extern void __vstate_restore (struct __riscv_v_state *restore_from,void *datap); +void vector_flush_cpu_state (void );static inline void __vstate_clean (struct pt_regs *regs) {
arch/riscv/include/asm/vector.h new file +14 -0
@@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* +* Copyright (C) 2020 SiFive +*/ + +#ifndef __ASM_RISCV_VECTOR_H +#define __ASM_RISCV_VECTOR_H + +#include <linux/types.h> + +void kernel_rvv_begin (void ); +void kernel_rvv_end (void ); + +#endif /* ! __ASM_RISCV_VECTOR_H */
arch/riscv/kernel/Makefile +1 -0
@@ -41,6 +41,7 @@ obj-$(CONFIG_MMU) += vdso.o vdso/ obj-$(CONFIG_RISCV_M_MODE) += traps_misaligned.o obj-$(CONFIG_FPU) += fpu.o obj-$(CONFIG_VECTOR) += vector.o +obj-$(CONFIG_VECTOR) += kernel_mode_vector.o obj-$(CONFIG_SMP) += smpboot.o obj-$(CONFIG_SMP) += smp.o obj-$(CONFIG_SMP) += cpu_ops.o
arch/riscv/kernel/kernel_mode_vector.c new file +144 -0
144 lines not on this page · raw message
arch/riscv/kernel/vector.S +9 -0
@@ -79,3 +79,12 @@ ENTRY(__vstate_restore) csrc sstatus, status ret ENDPROC(__vstate_restore) + +ENTRY(vector_flush_cpu_state) + vsetvli t0, x0, e8, m8 + vmv.v.i v0, 0 + vmv.v.i v8, 0 + vmv.v.i v16, 0 + vmv.v.i v24, 0 + ret +ENDPROC(vector_flush_cpu_state)
Signature · 1 line
2.31.1
PATCH v9 14/17 riscv: Use CSR_STATUS to replace sstatus in vector.S
In reply to Greentime HuIt should use the same logic here in both m-mode and s-mode.
--- arch/riscv/kernel/vector.S | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
arch/riscv/kernel/vector.S +4 -4
@@ -31,7 +31,7 @@ ENTRY(__vstate_save) li status, SR_VS - csrs sstatus, status + csrs CSR_STATUS, status csrr x_vstart, CSR_VSTART csrr x_vtype, CSR_VTYPE
@@ -51,13 +51,13 @@ ENTRY(__vstate_save) REG_S x_vl, RISCV_V_STATE_VL(vstatep) REG_S x_vcsr, RISCV_V_STATE_VCSR(vstatep) - csrc sstatus, status + csrc CSR_STATUS, status ret ENDPROC(__vstate_save) ENTRY(__vstate_restore) li status, SR_VS - csrs sstatus, status + csrs CSR_STATUS, status vsetvli incr, x0, e8, m8 vle8.v v0, (datap)
@@ -76,7 +76,7 @@ ENTRY(__vstate_restore) csrw CSR_VSTART, x_vstart csrw CSR_VCSR, x_vcsr - csrc sstatus, status + csrc CSR_STATUS, status ret ENDPROC(__vstate_restore)
Signature · 1 line
2.31.1
PATCH v9 15/17 riscv: Add vector extension XOR implementation
In reply to Greentime HuThis patch adds support for vector optimized XOR it is tested in spike and qemu.
Logs in spike:
[ 0.008365] xor: measuring software checksum speed [ 0.048885] 8regs : 1719.000 MB/sec [ 0.089080] 32regs : 1717.000 MB/sec [ 0.129275] rvv : 7043.000 MB/sec [ 0.129525] xor: using function: rvv (7043.000 MB/sec)
Logs in qemu:
[ 0.098943] xor: measuring software checksum speed [ 0.139391] 8regs : 2911.000 MB/sec [ 0.181079] 32regs : 2813.000 MB/sec [ 0.224260] rvv : 45.000 MB/sec [ 0.225586] xor: using function: 8regs (2911.000 MB/sec)
--- arch/riscv/include/asm/xor.h | 66 +++++++++++++++++++++++++++++ arch/riscv/lib/Makefile | 1 + arch/riscv/lib/xor.S | 81 ++++++++++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+) create mode 100644 arch/riscv/include/asm/xor.h create mode 100644 arch/riscv/lib/xor.S
arch/riscv/include/asm/xor.h new file +66 -0
@@ -0,0 +1,66 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* +* Copyright (C) 2021 SiFive +*/ + +#include <linux/hardirq.h> +#include <asm-generic/xor.h> +#ifdef CONFIG_VECTOR +#include <asm/vector.h> +#include <asm/switch_to.h> + +void xor_regs_2_ (unsigned long bytes,unsigned long *p1,unsigned long *p2); +void xor_regs_3_ (unsigned long bytes,unsigned long *p1,unsigned long *p2,unsigned long *p3); +void xor_regs_4_ (unsigned long bytes,unsigned long *p1,unsigned long *p2,unsigned long *p3, +unsigned long *p4); +void xor_regs_5_ (unsigned long bytes,unsigned long *p1,unsigned long *p2,unsigned long *p3, +unsigned long *p4,unsigned long *p5); + +static void xor_rvv_2 (unsigned long bytes,unsigned long *p1,unsigned long *p2) +{ +kernel_rvv_begin (); +xor_regs_2_ (bytes, p1, p2); +kernel_rvv_end (); +} + +static void xor_rvv_3 (unsigned long bytes,unsigned long *p1,unsigned long *p2,unsigned long *p3) +{ +kernel_rvv_begin (); +xor_regs_3_ (bytes, p1, p2, p3); +kernel_rvv_end (); +} + +static void xor_rvv_4 (unsigned long bytes,unsigned long *p1,unsigned long *p2,unsigned long *p3, +unsigned long *p4) +{ +kernel_rvv_begin (); +xor_regs_4_ (bytes, p1, p2, p3, p4); +kernel_rvv_end (); +} + +static void xor_rvv_5 (unsigned long bytes,unsigned long *p1,unsigned long *p2,unsigned long *p3, +unsigned long *p4,unsigned long *p5) +{ +kernel_rvv_begin (); +xor_regs_5_ (bytes, p1, p2, p3, p4, p5); +kernel_rvv_end (); +} + +static struct xor_block_template xor_block_rvv = { + .name ="rvv" , + .do_2 = xor_rvv_2, + .do_3 = xor_rvv_3, + .do_4 = xor_rvv_4, + .do_5 = xor_rvv_5 +}; + +#undef XOR_TRY_TEMPLATES +#define XOR_TRY_TEMPLATES \ + do { \ + xor_speed(&xor_block_8regs); \ + xor_speed(&xor_block_32regs); \ + if (has_vector()) { \ + xor_speed(&xor_block_rvv);\ + } \ + } while (0) +#endif
arch/riscv/lib/Makefile +1 -0
@@ -7,3 +7,4 @@ lib-$(CONFIG_MMU) += uaccess.o lib-$(CONFIG_64BIT) += tishift.o obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o +lib-$(CONFIG_VECTOR) += xor.o
arch/riscv/lib/xor.S new file +81 -0
81 lines not on this page · raw message
Signature · 1 line
2.31.1
PATCH v9 16/17 riscv: Fix an illegal instruction exception when accessing vlenb without enable vector first
In reply to Greentime HuIt triggered an illegal instruction exception when accessing vlenb CSR without enable vector first. To fix this issue, we should enable vector before using it and disable vector after using it.
--- arch/riscv/include/asm/vector.h | 2 ++ arch/riscv/kernel/cpufeature.c | 2 ++ arch/riscv/kernel/kernel_mode_vector.c | 6 ++++-- 3 files changed, 8 insertions(+), 2 deletions(-)
arch/riscv/include/asm/vector.h +2 -0
@@ -8,6 +8,8 @@#include <linux/types.h> +void rvv_enable (void ); +void rvv_disable (void );void kernel_rvv_begin (void );void kernel_rvv_end (void );
arch/riscv/kernel/cpufeature.c +2 -0
@@ -159,7 +159,9 @@ void __init riscv_fill_hwcap(void)if (elf_hwcap &COMPAT_HWCAP_ISA_V ) {static_branch_enable (&cpu_hwcap_vector);/* There are 32 vector registers with vlenb length. */ +rvv_enable (); riscv_vsize =csr_read (CSR_VLENB ) *32 ; +rvv_disable (); }#endif }
arch/riscv/kernel/kernel_mode_vector.c +4 -2
@@ -71,15 +71,17 @@ static void put_cpu_vector_context(void)preempt_enable (); } -static void rvv_enable (void ) +void rvv_enable (void ) {csr_set (CSR_STATUS ,SR_VS ); } +EXPORT_SYMBOL (rvv_enable); -static void rvv_disable (void ) +void rvv_disable (void ) {csr_clear (CSR_STATUS ,SR_VS ); } +EXPORT_SYMBOL (rvv_disable);/* * kernel_rvv_begin(): obtain the CPU vector registers for use by the calling
Signature · 1 line
2.31.1
Re: [PATCH v9 16/17] riscv: Fix an illegal instruction exception when accessing vlenb without enable vector first
Quoted 31 lines · Greentime Hu
It triggered an illegal instruction exception when accessing vlenb CSR without enable vector first. To fix this issue, we should enable vector before using it and disable vector after using it.
Co-developed-by Vincent Chen <[email protected]>Signed-off-by Vincent Chen <[email protected]>Signed-off-by Greentime Hu <[email protected]>--- arch/riscv/include/asm/vector.h | 2 ++ arch/riscv/kernel/cpufeature.c | 2 ++ arch/riscv/kernel/kernel_mode_vector.c | 6 ++++-- 3 files changed, 8 insertions(+), 2 deletions(-)arch/riscv/include/asm/vector.h +2
-0@@ -8,6 +8,8 @@#include <linux/types.h> +void rvv_enable (void ); +void rvv_disable (void );void kernel_rvv_begin (void );void kernel_rvv_end (void );arch/riscv/kernel/cpufeature.c +0
-0
arch/riscv/kernel/cpufeature.c void __init riscv_fill_hwcap(void) · line 166
Quoted 8 lines
arch/riscv/kernel/cpufeature.c +2
-0@@ -159,7 +159,9 @@ void __init riscv_fill_hwcap(void)if (elf_hwcap &COMPAT_HWCAP_ISA_V ) {static_branch_enable (&cpu_hwcap_vector);/* There are 32 vector registers with vlenb length. */ +rvv_enable (); riscv_vsize =csr_read (CSR_VLENB ) *32 ; +rvv_disable (); }#endif
Would it be better to enable this here, and then restore the original state instead of calling rvv_disable? If it was enabled then maybe something else is going to rely on that state?
Maybe something like:
prev = rvv_enable() riscv_vsize = csr_read(CSR_VLENB) * 32; rvv_restore(prev);
Quoted 26 lines
arch/riscv/kernel/cpufeature.c +0
-0}arch/riscv/kernel/kernel_mode_vector.c +4
-2@@ -71,15 +71,17 @@ static void put_cpu_vector_context(void)preempt_enable (); } -static void rvv_enable (void ) +void rvv_enable (void ) {csr_set (CSR_STATUS ,SR_VS ); } +EXPORT_SYMBOL (rvv_enable); -static void rvv_disable (void ) +void rvv_disable (void ) {csr_clear (CSR_STATUS ,SR_VS ); } +EXPORT_SYMBOL (rvv_disable);/* * kernel_rvv_begin(): obtain the CPU vector registers for use by the calling
Signature · 9 lines
Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius https://www.codethink.co.uk/privacy.html _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v9 16/17] riscv: Fix an illegal instruction exception when accessing vlenb without enable vector first
Ben Dooks <[email protected]> 於 2021年11月10日 週三 下午6:38寫道：
Quoted 46 lines · Greentime Hu
Quoted 31 lines
It triggered an illegal instruction exception when accessing vlenb CSR without enable vector first. To fix this issue, we should enable vector before using it and disable vector after using it.
Co-developed-by Vincent Chen <[email protected]>Signed-off-by Vincent Chen <[email protected]>Signed-off-by Greentime Hu <[email protected]>--- arch/riscv/include/asm/vector.h | 2 ++ arch/riscv/kernel/cpufeature.c | 2 ++ arch/riscv/kernel/kernel_mode_vector.c | 6 ++++-- 3 files changed, 8 insertions(+), 2 deletions(-)arch/riscv/include/asm/vector.h +2
-0@@ -8,6 +8,8 @@#include <linux/types.h> +void rvv_enable (void ); +void rvv_disable (void );void kernel_rvv_begin (void );void kernel_rvv_end (void );arch/riscv/kernel/cpufeature.c +0
-0arch/riscv/kernel/cpufeature.c void __init riscv_fill_hwcap(void) · line 166
Quoted 8 lines
arch/riscv/kernel/cpufeature.c +2
-0@@ -159,7 +159,9 @@ void __init riscv_fill_hwcap(void)if (elf_hwcap &COMPAT_HWCAP_ISA_V ) {static_branch_enable (&cpu_hwcap_vector);/* There are 32 vector registers with vlenb length. */ +rvv_enable (); riscv_vsize =csr_read (CSR_VLENB ) *32 ; +rvv_disable (); }#endif Would it be better to enable this here, and then restore the original state instead of calling
rvv_disable? If it was enabled then maybe something else is going to rely on that state?
Quoted 7 lines
Maybe something like:
prev = rvv_enable() riscv_vsize = csr_read(CSR_VLENB) * 32; rvv_restore(prev);
Hi Ben,
Thank you for reviewing this. The rvv won't be enabled here because we disable it in head.S at beginning and this stage is still doing some initialization work for rvv to set riscv_vsize, so we can assume that kernel mode vector should not be allowed to use vector yet.
Quoted 33 lines
Quoted 16 lines
arch/riscv/kernel/cpufeature.c +0
-0}arch/riscv/kernel/kernel_mode_vector.c +2
-1@@ -71,15 +71,17 @@ static void put_cpu_vector_context(void)preempt_enable (); } -static void rvv_enable (void ) +void rvv_enable (void ) {csr_set (CSR_STATUS ,SR_VS ); } +EXPORT_SYMBOL (rvv_enable);arch/riscv/kernel/kernel_mode_vector.c static void put_cpu_vector_context(void) · line 87
Quoted 10 lines
arch/riscv/kernel/kernel_mode_vector.c +2
-1-static void rvv_disable (void ) +void rvv_disable (void ) {csr_clear (CSR_STATUS ,SR_VS ); } +EXPORT_SYMBOL (rvv_disable);/* * kernel_rvv_begin(): obtain the CPU vector registers for use by the calling --
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
PATCH v9 17/17 riscv: Fix a kernel panic issue if $s2 is set to a specific value before entering Linux
In reply to Greentime HuPanic log:
[ 0.018707] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 [ 0.023060] Oops [#1] [ 0.023214] Modules linked in: [ 0.023725] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.14.0 #33 [ 0.023955] Hardware name: SiFive,FU800 (DT) [ 0.024150] epc : __vstate_save+0x1c/0x48 [ 0.024654] ra : arch_dup_task_struct+0x70/0x108 [ 0.024815] epc : ffffffff80005ad8 ra : ffffffff800035a8 sp : ffffffff81203d50 [ 0.025020] gp : ffffffff812e8290 tp : ffffffff8120bdc0 t0 : 0000000000000000 [ 0.025216] t1 : 0000000000000000 t2 : 0000000000000000 s0 : ffffffff81203d80 [ 0.025424] s1 : ffffffff8120bdc0 a0 : ffffffff8120c820 a1 : 0000000000000000 [ 0.025659] a2 : 0000000000001000 a3 : 0000000000000000 a4 : 0000000000000600 [ 0.025869] a5 : ffffffff8120cdc0 a6 : ffffffe00160b400 a7 : ffffffff80a1fe60 [ 0.026069] s2 : ffffffe0016b8000 s3 : ffffffff81204000 s4 : 0000000000004000 [ 0.026267] s5 : 0000000000000000 s6 : ffffffe0016b8000 s7 : ffffffe0016b9000 [ 0.026475] s8 : ffffffff81203ee0 s9 : 0000000000800300 s10: ffffffff812e9088 [ 0.026689] s11: ffffffd004008000 t3 : 0000000000000000 t4 : 0000000000000100 [ 0.026900] t5 : 0000000000000600 t6 : ffffffe00167bcc4 [ 0.027057] status: 8000000000000720 badaddr: 0000000000000000 cause: 000000000000000f [ 0.027344] [<ffffffff80005ad8>] __vstate_save+0x1c/0x48 [ 0.027567] [<ffffffff8000abe8>] copy_process+0x266/0x11a0 [ 0.027739] [<ffffffff8000bc98>] kernel_clone+0x90/0x2aa [ 0.027915] [<ffffffff8000c062>] kernel_thread+0x76/0x92 [ 0.028075] [<ffffffff8072e34c>] rest_init+0x26/0xfc [ 0.028242] [<ffffffff80800638>] arch_call_rest_init+0x10/0x18 [ 0.028423] [<ffffffff80800c4a>] start_kernel+0x5ce/0x5fe [ 0.029188] ---[ end trace 9a59af33f7ba3df4 ]--- [ 0.029479] Kernel panic - not syncing: Attempted to kill the idle task! [ 0.029907] ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---
The NULL pointer accessing caused the kernel panic. There is a NULL pointer is because in vstate_save() function it will check (regs->status & SR_VS) == SR_VS_DIRTY and this is true, but it shouldn't be true because vector is not used here. Since vector is not used, datap won't be allocated so it is NULL. The reason why regs->status is set to a wrong value is because pt_regs->status is put in stack and it is polluted after setup_vm() called.
In prologue of setup_vm(), we can observe it will save s2 to stack however s2 is meaningless here because the caller is assembly code and s2 is just some value from previous stage. The compiler will base on calling convention to save the register to stack. Then 0x80008638 in s2 is saved to stack. It might be any value. In this failure case it is 0x80008638 and it will accidentally cause SR_VS_DIRTY to call the vstate_save() function.
(gdb) info addr setup_vm
Symbol "setup_vm" is a function at address 0xffffffff80802c8a. (gdb) va2pa 0xffffffff80802c8a
$64 = 0x80a02c8a
(gdb) x/10i 0x80a02c8a
0x80a02c8a: addi sp,sp,-48 0x80a02c8c: li a3,-1 0x80a02c8e: auipc a5,0xff7fd 0x80a02c92: addi a5,a5,882 0x80a02c96: sd s0,32(sp) 0x80a02c98: sd s2,16(sp) <-- store to stack
After returning from setup_vm()
(gdb) x/20i 0x0000000080201138
0x80201138: mv a0,s1 0x8020113a: auipc ra,0x802 0x8020113e: jalr -1200(ra) <-- jump to setup_vm() 0x80201142: auipc a0,0xa03
(gdb) p/x $sp
$70 = 0x81404000
(gdb) p/x *(struct pt_regs*)($sp-0x120)
$71 = {
epc = 0x0,
ra = 0x0,
sp = 0x0,
gp = 0x0,
tp = 0x0,
t0 = 0x0,
t1 = 0x0,
t2 = 0x0,
s0 = 0x0,
s1 = 0x0,
a0 = 0x0,
a1 = 0x0,
a2 = 0x0,
a3 = 0x81403f90,
a4 = 0x80c04000,
a5 = 0x1,
a6 = 0xffffffff81337000,
a7 = 0x81096700,
s2 = 0x81400000,
s3 = 0xffffffff81200000,
s4 = 0x81403fd0,
s5 = 0x80a02c6c,
s6 = 0x8000000000006800,
s7 = 0x0,
s8 = 0xfffffffffffffff3,
s9 = 0x80c01000,
s10 = 0x81096700,
s11 = 0x82200000,
t3 = 0x81404000,
t4 = 0x80a02dea,
t5 = 0x0,
t6 = 0x82200000,
status = 0x80008638, <- Wrong value in stack!!!
badaddr = 0x82200000,
cause = 0x0,
orig_a0 = 0x80201142
}
(gdb) p/x $pc
$72 = 0x80201142
(gdb) p/x sizeof(struct pt_regs)
$73 = 0x120
--- arch/riscv/kernel/head.S | 2 ++ 1 file changed, 2 insertions(+)
arch/riscv/kernel/head.S +2 -0
@@ -303,6 +303,7 @@ clear_bss_done: /* Initialize page tables and relocate to virtual addresses */ la sp, init_thread_union + THREAD_SIZE XIP_FIXUP_OFFSET sp + addi sp, sp, -PT_SIZE #ifdef CONFIG_BUILTIN_DTB la a0, __dtb_start #else
@@ -320,6 +321,7 @@ clear_bss_done: la tp, init_task sw zero, TASK_TI_CPU(tp) la sp, init_thread_union + THREAD_SIZE + addi sp, sp, -PT_SIZE #ifdef CONFIG_KASAN call kasan_early_init
Signature · 1 line
2.31.1