LinuxLists.cc - [PATCH v12 01/17] riscv: Rename __switch_to

[permalink] [raw]

Subject: [PATCH v12 13/17] riscv: Add vector extension XOR implementation

From: Greentime Hu <[email protected]>

This patch adds support for vector optimized XOR and it is tested in
qemu.

Co-developed-by: Han-Kuan Chen <[email protected]>
Signed-off-by: Han-Kuan Chen <[email protected]>
Signed-off-by: Greentime Hu <[email protected]>
---
arch/riscv/include/asm/xor.h | 82 ++++++++++++++++++++++++++++++++++++
arch/riscv/lib/Makefile | 1 +
arch/riscv/lib/xor.S | 81 +++++++++++++++++++++++++++++++++++
3 files changed, 164 insertions(+)
create mode 100644 arch/riscv/include/asm/xor.h
create mode 100644 arch/riscv/lib/xor.S

diff --git a/arch/riscv/include/asm/xor.h b/arch/riscv/include/asm/xor.h
new file mode 100644
index 000000000000..d1f2eeb14afb
--- /dev/null
+++ b/arch/riscv/include/asm/xor.h
@@ -0,0 +1,82 @@
+/* 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 *__restrict p1,
+ const unsigned long *__restrict p2);
+void xor_regs_3_(unsigned long bytes, unsigned long *__restrict p1,
+ const unsigned long *__restrict p2,
+ const unsigned long *__restrict p3);
+void xor_regs_4_(unsigned long bytes, unsigned long *__restrict p1,
+ const unsigned long *__restrict p2,
+ const unsigned long *__restrict p3,
+ const unsigned long *__restrict p4);
+void xor_regs_5_(unsigned long bytes, unsigned long *__restrict p1,
+ const unsigned long *__restrict p2,
+ const unsigned long *__restrict p3,
+ const unsigned long *__restrict p4,
+ const unsigned long *__restrict p5);
+
+static void xor_rvv_2(unsigned long bytes, unsigned long *__restrict p1,
+ const unsigned long *__restrict p2)
+{
+ kernel_rvv_begin();
+ xor_regs_2_(bytes, p1, p2);
+ kernel_rvv_end();
+}
+
+static void xor_rvv_3(unsigned long bytes, unsigned long *__restrict p1,
+ const unsigned long *__restrict p2,
+ const unsigned long *__restrict p3)
+{
+ kernel_rvv_begin();
+ xor_regs_3_(bytes, p1, p2, p3);
+ kernel_rvv_end();
+}
+
+static void xor_rvv_4(unsigned long bytes, unsigned long *__restrict p1,
+ const unsigned long *__restrict p2,
+ const unsigned long *__restrict p3,
+ const unsigned long *__restrict 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 *__restrict p1,
+ const unsigned long *__restrict p2,
+ const unsigned long *__restrict p3,
+ const unsigned long *__restrict p4,
+ const unsigned long *__restrict 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
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
index 25d5c9664e57..acd87ac86d24 100644
--- a/arch/riscv/lib/Makefile
+++ b/arch/riscv/lib/Makefile
@@ -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
diff --git a/arch/riscv/lib/xor.S b/arch/riscv/lib/xor.S
new file mode 100644
index 000000000000..3bc059e18171
--- /dev/null
+++ b/arch/riscv/lib/xor.S
@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2021 SiFive
+ */
+#include <linux/linkage.h>
+#include <asm-generic/export.h>
+#include <asm/asm.h>
+
+ENTRY(xor_regs_2_)
+ vsetvli a3, a0, e8, m8, ta, ma
+ vle8.v v0, (a1)
+ vle8.v v8, (a2)
+ sub a0, a0, a3
+ vxor.vv v16, v0, v8
+ add a2, a2, a3
+ vse8.v v16, (a1)
+ add a1, a1, a3
+ bnez a0, xor_regs_2_
+ ret
+END(xor_regs_2_)
+EXPORT_SYMBOL(xor_regs_2_)
+
+ENTRY(xor_regs_3_)
+ vsetvli a4, a0, e8, m8, ta, ma
+ vle8.v v0, (a1)
+ vle8.v v8, (a2)
+ sub a0, a0, a4
+ vxor.vv v0, v0, v8
+ vle8.v v16, (a3)
+ add a2, a2, a4
+ vxor.vv v16, v0, v16
+ add a3, a3, a4
+ vse8.v v16, (a1)
+ add a1, a1, a4
+ bnez a0, xor_regs_3_
+ ret
+END(xor_regs_3_)
+EXPORT_SYMBOL(xor_regs_3_)
+
+ENTRY(xor_regs_4_)
+ vsetvli a5, a0, e8, m8, ta, ma
+ vle8.v v0, (a1)
+ vle8.v v8, (a2)
+ sub a0, a0, a5
+ vxor.vv v0, v0, v8
+ vle8.v v16, (a3)
+ add a2, a2, a5
+ vxor.vv v0, v0, v16
+ vle8.v v24, (a4)
+ add a3, a3, a5
+ vxor.vv v16, v0, v24
+ add a4, a4, a5
+ vse8.v v16, (a1)
+ add a1, a1, a5
+ bnez a0, xor_regs_4_
+ ret
+END(xor_regs_4_)
+EXPORT_SYMBOL(xor_regs_4_)
+
+ENTRY(xor_regs_5_)
+ vsetvli a6, a0, e8, m8, ta, ma
+ vle8.v v0, (a1)
+ vle8.v v8, (a2)
+ sub a0, a0, a6
+ vxor.vv v0, v0, v8
+ vle8.v v16, (a3)
+ add a2, a2, a6
+ vxor.vv v0, v0, v16
+ vle8.v v24, (a4)
+ add a3, a3, a6
+ vxor.vv v0, v0, v24
+ vle8.v v8, (a5)
+ add a4, a4, a6
+ vxor.vv v16, v0, v8
+ add a5, a5, a6
+ vse8.v v16, (a1)
+ add a1, a1, a6
+ bnez a0, xor_regs_5_
+ ret
+END(xor_regs_5_)
+EXPORT_SYMBOL(xor_regs_5_)
--
2.25.1

2022-09-21 22:43:14

[permalink] [raw]

Subject: [PATCH v12 14/17] riscv: Fix a kernel panic issue if $s2 is set to a specific value before entering Linux

From: Greentime Hu <[email protected]>

Panic 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

Co-developed-by: ShihPo Hung <[email protected]>
Signed-off-by: ShihPo Hung <[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/kernel/head.S | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 2c81ca42ec4e..c7effef23f41 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -301,6 +301,7 @@ clear_bss_done:
la tp, init_task
la sp, init_thread_union + THREAD_SIZE
XIP_FIXUP_OFFSET sp
+ addi sp, sp, -PT_SIZE
#ifdef CONFIG_BUILTIN_DTB
la a0, __dtb_start
XIP_FIXUP_OFFSET a0
@@ -318,6 +319,7 @@ clear_bss_done:
/* Restore C environment */
la tp, init_task
la sp, init_thread_union + THREAD_SIZE
+ addi sp, sp, -PT_SIZE

#ifdef CONFIG_KASAN
call kasan_early_init
--
2.25.1

2022-09-21 22:43:44

[permalink] [raw]

Subject: [PATCH v12 16/17] riscv: KVM: Add vector lazy save/restore support

From: Vincent Chen <[email protected]>

This patch adds vector context save/restore for guest VCPUs. To reduce the
impact on KVM performance, the implementation imitates the FP context
switch mechanism to lazily store and restore the vector context only when
the kernel enters/exits the in-kernel run loop and not during the KVM
world switch.

Signed-off-by: Vincent Chen <[email protected]>
Signed-off-by: Greentime Hu <[email protected]>
---
arch/riscv/include/asm/kvm_host.h | 2 +
arch/riscv/include/asm/kvm_vcpu_vector.h | 65 +++++++++
arch/riscv/include/uapi/asm/kvm.h | 7 +
arch/riscv/kernel/asm-offsets.c | 7 +
arch/riscv/kvm/Makefile | 1 +
arch/riscv/kvm/vcpu.c | 32 +++++
arch/riscv/kvm/vcpu_switch.S | 69 +++++++++
arch/riscv/kvm/vcpu_vector.c | 173 +++++++++++++++++++++++
8 files changed, 356 insertions(+)
create mode 100644 arch/riscv/include/asm/kvm_vcpu_vector.h
create mode 100644 arch/riscv/kvm/vcpu_vector.c

diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
index 60c517e4d576..665ddb4cec62 100644
--- a/arch/riscv/include/asm/kvm_host.h
+++ b/arch/riscv/include/asm/kvm_host.h
@@ -17,6 +17,7 @@
#include <asm/hwcap.h>
#include <asm/kvm_vcpu_fp.h>
#include <asm/kvm_vcpu_insn.h>
+#include <asm/kvm_vcpu_vector.h>
#include <asm/kvm_vcpu_timer.h>

#define KVM_MAX_VCPUS 1024
@@ -143,6 +144,7 @@ struct kvm_cpu_context {
unsigned long sstatus;
unsigned long hstatus;
union __riscv_fp_state fp;
+ struct __riscv_v_state vector;
};

struct kvm_vcpu_csr {
diff --git a/arch/riscv/include/asm/kvm_vcpu_vector.h b/arch/riscv/include/asm/kvm_vcpu_vector.h
new file mode 100644
index 000000000000..1dcc1b2e05bb
--- /dev/null
+++ b/arch/riscv/include/asm/kvm_vcpu_vector.h
@@ -0,0 +1,65 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
+ * Copyright (C) 2022 SiFive
+ *
+ * Authors:
+ * Atish Patra <[email protected]>
+ * Anup Patel <[email protected]>
+ * Vincent Chen <[email protected]>
+ * Greentime Hu <[email protected]>
+ */
+
+#ifndef __KVM_VCPU_RISCV_VECTOR_H
+#define __KVM_VCPU_RISCV_VECTOR_H
+
+#include <linux/types.h>
+
+struct kvm_cpu_context;
+
+#ifdef CONFIG_VECTOR
+void __kvm_riscv_vector_save(struct kvm_cpu_context *context);
+void __kvm_riscv_vector_restore(struct kvm_cpu_context *context);
+void kvm_riscv_vcpu_vector_reset(struct kvm_vcpu *vcpu);
+void kvm_riscv_vcpu_guest_vector_save(struct kvm_cpu_context *cntx,
+ unsigned long isa);
+void kvm_riscv_vcpu_guest_vector_restore(struct kvm_cpu_context *cntx,
+ unsigned long isa);
+void kvm_riscv_vcpu_host_vector_save(struct kvm_cpu_context *cntx);
+void kvm_riscv_vcpu_host_vector_restore(struct kvm_cpu_context *cntx);
+void kvm_riscv_vcpu_free_vector_context(struct kvm_vcpu *vcpu);
+#else
+static inline void kvm_riscv_vcpu_vector_reset(struct kvm_vcpu *vcpu)
+{
+}
+
+static inline void kvm_riscv_vcpu_guest_vector_save(struct kvm_cpu_context *cntx,
+ unsigned long isa)
+{
+}
+
+static inline void kvm_riscv_vcpu_guest_vector_restore(struct kvm_cpu_context *cntx,
+ unsigned long isa)
+{
+}
+
+static inline void kvm_riscv_vcpu_host_vector_save(struct kvm_cpu_context *cntx)
+{
+}
+
+static inline void kvm_riscv_vcpu_host_vector_restore(struct kvm_cpu_context *cntx)
+{
+}
+
+static inline void kvm_riscv_vcpu_free_vector_context(struct kvm_vcpu *vcpu)
+{
+}
+#endif
+
+int kvm_riscv_vcpu_get_reg_vector(struct kvm_vcpu *vcpu,
+ const struct kvm_one_reg *reg,
+ unsigned long rtype);
+int kvm_riscv_vcpu_set_reg_vector(struct kvm_vcpu *vcpu,
+ const struct kvm_one_reg *reg,
+ unsigned long rtype);
+#endif
diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
index 7351417afd62..f4ba57b235a3 100644
--- a/arch/riscv/include/uapi/asm/kvm.h
+++ b/arch/riscv/include/uapi/asm/kvm.h
@@ -96,6 +96,7 @@ enum KVM_RISCV_ISA_EXT_ID {
KVM_RISCV_ISA_EXT_H,
KVM_RISCV_ISA_EXT_I,
KVM_RISCV_ISA_EXT_M,
+ KVM_RISCV_ISA_EXT_V,
KVM_RISCV_ISA_EXT_SVPBMT,
KVM_RISCV_ISA_EXT_SSTC,
KVM_RISCV_ISA_EXT_MAX,
@@ -145,6 +146,12 @@ enum KVM_RISCV_ISA_EXT_ID {
/* ISA Extension registers are mapped as type 7 */
#define KVM_REG_RISCV_ISA_EXT (0x07 << KVM_REG_RISCV_TYPE_SHIFT)

+/* V extension registers are mapped as type 7 */
+#define KVM_REG_RISCV_VECTOR (0x07 << KVM_REG_RISCV_TYPE_SHIFT)
+#define KVM_REG_RISCV_VECTOR_CSR_REG(name) \
+ (offsetof(struct __riscv_v_state, name) / sizeof(unsigned long))
+#define KVM_REG_RISCV_VECTOR_REG(n) \
+ ((n) + sizeof(struct __riscv_v_state) / sizeof(unsigned long))
#endif

#endif /* __LINUX_KVM_RISCV_H */
diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
index 80316ef7bb78..2540b9146072 100644
--- a/arch/riscv/kernel/asm-offsets.c
+++ b/arch/riscv/kernel/asm-offsets.c
@@ -278,6 +278,13 @@ void asm_offsets(void)
OFFSET(KVM_ARCH_FP_D_F31, kvm_cpu_context, fp.d.f[31]);
OFFSET(KVM_ARCH_FP_D_FCSR, kvm_cpu_context, fp.d.fcsr);

+ /* V extension */
+
+ OFFSET(KVM_ARCH_VECTOR_VSTART, kvm_cpu_context, vector.vstart);
+ OFFSET(KVM_ARCH_VECTOR_VL, kvm_cpu_context, vector.vl);
+ OFFSET(KVM_ARCH_VECTOR_VTYPE, kvm_cpu_context, vector.vtype);
+ OFFSET(KVM_ARCH_VECTOR_VCSR, kvm_cpu_context, vector.vcsr);
+ OFFSET(KVM_ARCH_VECTOR_DATAP, kvm_cpu_context, vector.datap);
/*
* THREAD_{F,X}* might be larger than a S-type offset can handle, but
* these are used in performance-sensitive assembly so we can't resort
diff --git a/arch/riscv/kvm/Makefile b/arch/riscv/kvm/Makefile
index 019df9208bdd..b26bc605a267 100644
--- a/arch/riscv/kvm/Makefile
+++ b/arch/riscv/kvm/Makefile
@@ -17,6 +17,7 @@ kvm-y += mmu.o
kvm-y += vcpu.o
kvm-y += vcpu_exit.o
kvm-y += vcpu_fp.o
+kvm-y += vcpu_vector.o
kvm-y += vcpu_insn.o
kvm-y += vcpu_switch.o
kvm-y += vcpu_sbi.o
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index d0f08d5b4282..76941937e745 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -19,6 +19,7 @@
#include <linux/kvm_host.h>
#include <asm/csr.h>
#include <asm/hwcap.h>
+#include <asm/switch_to.h>

const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
KVM_GENERIC_VCPU_STATS(),
@@ -51,6 +52,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
RISCV_ISA_EXT_h,
RISCV_ISA_EXT_i,
RISCV_ISA_EXT_m,
+ RISCV_ISA_EXT_v,
RISCV_ISA_EXT_SVPBMT,
RISCV_ISA_EXT_SSTC,
};
@@ -79,6 +81,7 @@ static bool kvm_riscv_vcpu_isa_enable_allowed(unsigned long ext)
return true;
}

+//CMS FIXME
static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
{
switch (ext) {
@@ -121,6 +124,8 @@ static void kvm_riscv_reset_vcpu(struct kvm_vcpu *vcpu)

kvm_riscv_vcpu_fp_reset(vcpu);

+ kvm_riscv_vcpu_vector_reset(vcpu);
+
kvm_riscv_vcpu_timer_reset(vcpu);

WRITE_ONCE(vcpu->arch.irqs_pending, 0);
@@ -171,6 +176,15 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
cntx->hstatus |= HSTATUS_SPVP;
cntx->hstatus |= HSTATUS_SPV;

+ if (has_vector()) {
+ cntx->vector.datap = kmalloc(riscv_vsize, GFP_KERNEL);
+ if (!cntx->vector.datap)
+ return -ENOMEM;
+ vcpu->arch.host_context.vector.datap = kzalloc(riscv_vsize, GFP_KERNEL);
+ if (!vcpu->arch.host_context.vector.datap)
+ return -ENOMEM;
+ }
+
/* By default, make CY, TM, and IR counters accessible in VU mode */
reset_csr->scounteren = 0x7;

@@ -201,6 +215,9 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)

/* Free unused pages pre-allocated for G-stage page table mappings */
kvm_mmu_free_memory_cache(&vcpu->arch.mmu_page_cache);
+
+ /* Free vector context space for host and guest kernel */
+ kvm_riscv_vcpu_free_vector_context(vcpu);
}

int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
@@ -539,6 +556,9 @@ static int kvm_riscv_vcpu_set_reg(struct kvm_vcpu *vcpu,
KVM_REG_RISCV_FP_D);
else if ((reg->id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_ISA_EXT)
return kvm_riscv_vcpu_set_reg_isa_ext(vcpu, reg);
+ else if ((reg->id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_VECTOR)
+ return kvm_riscv_vcpu_set_reg_vector(vcpu, reg,
+ KVM_REG_RISCV_VECTOR);

return -EINVAL;
}
@@ -562,6 +582,9 @@ static int kvm_riscv_vcpu_get_reg(struct kvm_vcpu *vcpu,
KVM_REG_RISCV_FP_D);
else if ((reg->id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_ISA_EXT)
return kvm_riscv_vcpu_get_reg_isa_ext(vcpu, reg);
+ else if ((reg->id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_VECTOR)
+ return kvm_riscv_vcpu_get_reg_vector(vcpu, reg,
+ KVM_REG_RISCV_VECTOR);

return -EINVAL;
}
@@ -818,6 +841,9 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
kvm_riscv_vcpu_host_fp_save(&vcpu->arch.host_context);
kvm_riscv_vcpu_guest_fp_restore(&vcpu->arch.guest_context,
vcpu->arch.isa);
+ kvm_riscv_vcpu_host_vector_save(&vcpu->arch.host_context);
+ kvm_riscv_vcpu_guest_vector_restore(&vcpu->arch.guest_context,
+ vcpu->arch.isa);

vcpu->cpu = cpu;
}
@@ -834,6 +860,12 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)

kvm_riscv_vcpu_timer_save(vcpu);

+ kvm_riscv_vcpu_guest_vector_save(&vcpu->arch.guest_context,
+ vcpu->arch.isa);
+ kvm_riscv_vcpu_host_vector_restore(&vcpu->arch.host_context);
+
+ csr_write(CSR_HGATP, 0);
+
csr->vsstatus = csr_read(CSR_VSSTATUS);
csr->vsie = csr_read(CSR_VSIE);
csr->vstvec = csr_read(CSR_VSTVEC);
diff --git a/arch/riscv/kvm/vcpu_switch.S b/arch/riscv/kvm/vcpu_switch.S
index d74df8eb4d71..730dc9b8c644 100644
--- a/arch/riscv/kvm/vcpu_switch.S
+++ b/arch/riscv/kvm/vcpu_switch.S
@@ -406,3 +406,72 @@ __kvm_riscv_fp_d_restore:
csrw CSR_SSTATUS, t2
ret
#endif
+
+#ifdef CONFIG_VECTOR
+
+#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(__kvm_riscv_vector_save)
+ li status, SR_VS
+ csrs CSR_STATUS, status
+
+ li a2, KVM_ARCH_VECTOR_DATAP
+ add datap, a0, a2
+ ld datap, (datap)
+ 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, ta, ma
+ 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, KVM_ARCH_VECTOR_VSTART(vstatep)
+ REG_S x_vtype, KVM_ARCH_VECTOR_VTYPE(vstatep)
+ REG_S x_vl, KVM_ARCH_VECTOR_VL(vstatep)
+ REG_S x_vcsr, KVM_ARCH_VECTOR_VCSR(vstatep)
+
+ csrc CSR_STATUS, status
+ ret
+ENDPROC(__kvm_riscv_vector_save)
+
+ENTRY(__kvm_riscv_vector_restore)
+ li status, SR_VS
+ csrs CSR_STATUS, status
+
+ li a2, KVM_ARCH_VECTOR_DATAP
+ add datap, a0, a2
+ ld datap, (datap)
+ vsetvli incr, x0, e8, m8, ta, ma
+ 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, KVM_ARCH_VECTOR_VSTART(vstatep)
+ REG_L x_vtype, KVM_ARCH_VECTOR_VTYPE(vstatep)
+ REG_L x_vl, KVM_ARCH_VECTOR_VL(vstatep)
+ REG_L x_vcsr, KVM_ARCH_VECTOR_VCSR(vstatep)
+ vsetvl x0, x_vl, x_vtype
+ csrw CSR_VSTART, x_vstart
+ csrw CSR_VCSR, x_vcsr
+
+ csrc CSR_STATUS, status
+ ret
+ENDPROC(__kvm_riscv_vector_restore)
+#endif
diff --git a/arch/riscv/kvm/vcpu_vector.c b/arch/riscv/kvm/vcpu_vector.c
new file mode 100644
index 000000000000..37bf4ffd47dd
--- /dev/null
+++ b/arch/riscv/kvm/vcpu_vector.c
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
+ * Copyright (C) 2022 SiFive
+ *
+ * Authors:
+ * Atish Patra <[email protected]>
+ * Anup Patel <[email protected]>
+ * Vincent Chen <[email protected]>
+ * Greentime Hu <[email protected]>
+ */
+
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <linux/kvm_host.h>
+#include <linux/uaccess.h>
+#include <asm/hwcap.h>
+
+#ifdef CONFIG_VECTOR
+extern unsigned long riscv_vsize;
+void kvm_riscv_vcpu_vector_reset(struct kvm_vcpu *vcpu)
+{
+ unsigned long isa = vcpu->arch.isa;
+ struct kvm_cpu_context *cntx = &vcpu->arch.guest_context;
+
+ cntx->sstatus &= ~SR_VS;
+ if (riscv_isa_extension_available(&isa, v))
+ cntx->sstatus |= SR_VS_INITIAL;
+ else
+ cntx->sstatus |= SR_VS_OFF;
+
+ memset(cntx->vector.datap, 0, riscv_vsize);
+}
+
+static void kvm_riscv_vcpu_vector_clean(struct kvm_cpu_context *cntx)
+{
+ cntx->sstatus &= ~SR_VS;
+ cntx->sstatus |= SR_VS_CLEAN;
+}
+
+void kvm_riscv_vcpu_guest_vector_save(struct kvm_cpu_context *cntx,
+ unsigned long isa)
+{
+ if ((cntx->sstatus & SR_VS) == SR_VS_DIRTY) {
+ if (riscv_isa_extension_available(&isa, v))
+ __kvm_riscv_vector_save(cntx);
+ kvm_riscv_vcpu_vector_clean(cntx);
+ }
+}
+
+void kvm_riscv_vcpu_guest_vector_restore(struct kvm_cpu_context *cntx,
+ unsigned long isa)
+{
+ if ((cntx->sstatus & SR_VS) != SR_VS_OFF) {
+ if (riscv_isa_extension_available(&isa, v))
+ __kvm_riscv_vector_restore(cntx);
+ kvm_riscv_vcpu_vector_clean(cntx);
+ }
+}
+
+void kvm_riscv_vcpu_host_vector_save(struct kvm_cpu_context *cntx)
+{
+ /* No need to check host sstatus as it can be modified outside */
+ __kvm_riscv_vector_save(cntx);
+}
+
+void kvm_riscv_vcpu_host_vector_restore(struct kvm_cpu_context *cntx)
+{
+ __kvm_riscv_vector_restore(cntx);
+}
+
+void kvm_riscv_vcpu_free_vector_context(struct kvm_vcpu *vcpu)
+{
+ kfree(vcpu->arch.guest_reset_context.vector.datap);
+ kfree(vcpu->arch.host_context.vector.datap);
+}
+#else
+#define riscv_vsize (0)
+#endif
+
+static void *kvm_riscv_vcpu_vreg_addr(struct kvm_vcpu *vcpu,
+ unsigned long reg_num,
+ size_t reg_size)
+{
+ struct kvm_cpu_context *cntx = &vcpu->arch.guest_context;
+ void *reg_val;
+ size_t vlenb = riscv_vsize / 32;
+
+ if (reg_num < KVM_REG_RISCV_VECTOR_REG(0)) {
+ if (reg_size != sizeof(unsigned long))
+ return NULL;
+ switch (reg_num) {
+ case KVM_REG_RISCV_VECTOR_CSR_REG(vstart):
+ reg_val = &cntx->vector.vstart;
+ break;
+ case KVM_REG_RISCV_VECTOR_CSR_REG(vl):
+ reg_val = &cntx->vector.vl;
+ break;
+ case KVM_REG_RISCV_VECTOR_CSR_REG(vtype):
+ reg_val = &cntx->vector.vtype;
+ break;
+ case KVM_REG_RISCV_VECTOR_CSR_REG(vcsr):
+ reg_val = &cntx->vector.vcsr;
+ break;
+ case KVM_REG_RISCV_VECTOR_CSR_REG(datap):
+ default:
+ return NULL;
+ }
+ } else if (reg_num <= KVM_REG_RISCV_VECTOR_REG(31)) {
+ if (reg_size != vlenb)
+ return NULL;
+ reg_val = cntx->vector.datap
+ + (reg_num - KVM_REG_RISCV_VECTOR_REG(0)) * vlenb;
+ } else {
+ return NULL;
+ }
+
+ return reg_val;
+}
+
+int kvm_riscv_vcpu_get_reg_vector(struct kvm_vcpu *vcpu,
+ const struct kvm_one_reg *reg,
+ unsigned long rtype)
+{
+ unsigned long isa = vcpu->arch.isa;
+ unsigned long __user *uaddr =
+ (unsigned long __user *)(unsigned long)reg->addr;
+ unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
+ KVM_REG_SIZE_MASK |
+ rtype);
+ void *reg_val;
+ size_t reg_size = KVM_REG_SIZE(reg->id);
+
+ if ((rtype == KVM_REG_RISCV_VECTOR) &&
+ riscv_isa_extension_available(&isa, v)) {
+ reg_val = kvm_riscv_vcpu_vreg_addr(vcpu, reg_num, reg_size);
+ }
+
+ if (!reg_val)
+ return -EINVAL;
+
+ if (copy_to_user(uaddr, reg_val, reg_size))
+ return -EFAULT;
+
+ return 0;
+}
+
+int kvm_riscv_vcpu_set_reg_vector(struct kvm_vcpu *vcpu,
+ const struct kvm_one_reg *reg,
+ unsigned long rtype)
+{
+ unsigned long isa = vcpu->arch.isa;
+ unsigned long __user *uaddr =
+ (unsigned long __user *)(unsigned long)reg->addr;
+ unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
+ KVM_REG_SIZE_MASK |
+ rtype);
+ void *reg_val = NULL;
+ size_t reg_size = KVM_REG_SIZE(reg->id);
+
+ if ((rtype == KVM_REG_RISCV_VECTOR) &&
+ riscv_isa_extension_available(&isa, v)) {
+ reg_val = kvm_riscv_vcpu_vreg_addr(vcpu, reg_num, reg_size);
+ }
+
+ if (!reg_val)
+ return -EINVAL;
+
+ if (copy_from_user(reg_val, uaddr, reg_size))
+ return -EFAULT;
+
+ return 0;
+}
--
2.25.1

2022-09-21 22:46:15

[permalink] [raw]

Subject: [PATCH v12 15/17] riscv: Add V extension to KVM ISA allow list

From: Vincent Chen <[email protected]>

Add V extension to KVM_RISCV_ISA_ALLOWED list to enable VCPU
to support V extension.

Signed-off-by: Vincent Chen <[email protected]>
Signed-off-by: Greentime Hu <[email protected]>
---
arch/riscv/include/asm/hwcap.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 6f59ec64175e..b242ed155262 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -35,6 +35,7 @@ extern unsigned long elf_hwcap;
#define RISCV_ISA_EXT_m ('m' - 'a')
#define RISCV_ISA_EXT_s ('s' - 'a')
#define RISCV_ISA_EXT_u ('u' - 'a')
+#define RISCV_ISA_EXT_v ('v' - 'a')

/*
* Increse this to higher value as kernel support more ISA extensions.
--
2.25.1

2022-09-21 22:47:57

[permalink] [raw]

Subject: [PATCH v12 07/17] riscv: Add vector struct and assembler definitions

From: Greentime Hu <[email protected]>

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

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 | 17 +++++++++++++++++
arch/riscv/kernel/asm-offsets.c | 6 ++++++
3 files changed, 24 insertions(+)

diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index 19eedd4af4cd..95917a2b24f9 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -39,6 +39,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 */
diff --git a/arch/riscv/include/uapi/asm/ptrace.h b/arch/riscv/include/uapi/asm/ptrace.h
index 882547f6bd5c..6ee1ca2edfa7 100644
--- a/arch/riscv/include/uapi/asm/ptrace.h
+++ b/arch/riscv/include/uapi/asm/ptrace.h
@@ -77,6 +77,23 @@ 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;
+ /*
+ * 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.
+ */
+};
+
#endif /* __ASSEMBLY__ */

#endif /* _UAPI_ASM_RISCV_PTRACE_H */
diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
index df9444397908..37e3e6a8d877 100644
--- a/arch/riscv/kernel/asm-offsets.c
+++ b/arch/riscv/kernel/asm-offsets.c
@@ -75,6 +75,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);
--
2.25.1

2022-09-21 22:48:08

[permalink] [raw]

Subject: [PATCH v12 08/17] riscv: Add task switch support for vector

From: Greentime Hu <[email protected]>

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]: 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]
[[email protected]: Fix the arch_release_task_struct free wrong
datap issue]

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]>
Co-developed-by: Ruinland Tsai <[email protected]>
Signed-off-by: Ruinland Tsai <[email protected]>
Signed-off-by: Greentime Hu <[email protected]>
Reported-by: kernel test robot <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
arch/riscv/include/asm/switch_to.h | 66 ++++++++++++++++++++++++++++++
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/process.c | 43 +++++++++++++++++++
3 files changed, 110 insertions(+)

diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
index df1aa589b7fd..527951c033d4 100644
--- a/arch/riscv/include/asm/switch_to.h
+++ b/arch/riscv/include/asm/switch_to.h
@@ -7,11 +7,13 @@
#define _ASM_RISCV_SWITCH_TO_H

#include <linux/jump_label.h>
+#include <linux/slab.h>
#include <linux/sched/task_stack.h>
#include <asm/hwcap.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,68 @@ 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 +141,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)

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index 33bb60a354cd..35752fb6d145 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -55,6 +55,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
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index ceb9ebab6558..e88a37fc77ed 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -124,6 +124,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;

@@ -148,15 +167,29 @@ void flush_thread(void)
fstate_off(current, task_pt_regs(current));
memset(&current->thread.fstate, 0, sizeof(current->thread.fstate));
#endif
+#ifdef CONFIG_VECTOR
+ /* Reset vector state */
+ vstate_off(current, task_pt_regs(current));
+ memset(&current->thread.vstate, 0, RISCV_V_STATE_DATAP);
+#endif
}

int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
{
fstate_save(src, task_pt_regs(src));
*dst = *src;
+ dst->thread.vstate.datap = NULL;
+
return 0;
}

+void arch_release_task_struct(struct task_struct *tsk)
+{
+ /* Free the vector context of datap. */
+ if (has_vector() && tsk->thread.vstate.datap)
+ kfree(tsk->thread.vstate.datap);
+}
+
int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
{
unsigned long clone_flags = args->flags;
@@ -175,7 +208,17 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
p->thread.ra = (unsigned long)ret_from_kernel_thread;
p->thread.s[0] = (unsigned long)args->fn;
p->thread.s[1] = (unsigned long)args->fn_arg;
+ p->thread.vstate.datap = NULL;
} else {
+ /* Allocate the datap for the user process if datap is NULL */
+ if (has_vector() && !p->thread.vstate.datap) {
+ void *datap = kzalloc(riscv_vsize, GFP_KERNEL);
+ /* Failed to allocate memory. */
+ if (!datap)
+ return -ENOMEM;
+ p->thread.vstate.datap = datap;
+ memset(&p->thread.vstate, 0, RISCV_V_STATE_DATAP);
+ }
*childregs = *(current_pt_regs());
if (usp) /* User fork */
childregs->sp = usp;
--
2.25.1

2022-09-21 22:49:08

[permalink] [raw]

Subject: [PATCH v12 02/17] riscv: Extending cpufeature.c to detect V-extension

From: 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.

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]>
Reviewed-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/include/uapi/asm/hwcap.h | 1 +
arch/riscv/kernel/cpufeature.c | 1 +
2 files changed, 2 insertions(+)

diff --git a/arch/riscv/include/uapi/asm/hwcap.h b/arch/riscv/include/uapi/asm/hwcap.h
index 46dc3f5ee99f..c52bb7bbbabe 100644
--- a/arch/riscv/include/uapi/asm/hwcap.h
+++ b/arch/riscv/include/uapi/asm/hwcap.h
@@ -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 */
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 553d755483ed..8d4448c2d4f4 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -83,6 +83,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;

--
2.25.1

2022-09-21 23:09:11

[permalink] [raw]

Subject: [PATCH v12 04/17] riscv: Add vector feature to compile

From: Guo Ren <[email protected]>

This patch adds a new config option which could enable assembler's
vector feature.

Signed-off-by: Guo Ren <[email protected]>
Co-developed-by: Greentime Hu <[email protected]>
Signed-off-by: Greentime Hu <[email protected]>
---
arch/riscv/Kconfig | 15 +++++++++++++--
arch/riscv/Makefile | 1 +
2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index ed66c31e4655..e294d85bfb7d 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -432,7 +432,17 @@ config FPU

If you don't know what to do here, say Y.

-endmenu # "Platform type"
+config VECTOR
+ bool "VECTOR support"
+ depends on GCC_VERSION >= 120000 || CLANG_VERSION >= 130000
+ 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"

@@ -556,6 +566,7 @@ config CMDLINE_EXTEND
cases where the provided arguments are insufficient and
you don't want to or cannot modify them.

+
config CMDLINE_FORCE
bool "Always use the default kernel command string"
help
@@ -648,7 +659,7 @@ config XIP_PHYS_ADDR
be linked for and stored to. This address is dependent on your
own flash usage.

-endmenu # "Boot options"
+endmenu

config BUILTIN_DTB
bool
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 3fa8ef336822..1ec17f3d6d09 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -50,6 +50,7 @@ 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
+riscv-march-$(CONFIG_VECTOR) := $(riscv-march-y)v

# Newer binutils versions default to ISA spec version 20191213 which moves some
# instructions from the I extension to the Zicsr and Zifencei extensions.
--
2.25.1

2022-09-21 23:09:11

[permalink] [raw]

Subject: [PATCH v12 06/17] riscv: Reset vector register

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]>
Reviewed-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/kernel/entry.S | 6 +++---
arch/riscv/kernel/head.S | 35 +++++++++++++++++++++++++++++------
2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index b9eda3fcbd6d..1e9987376591 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 b865046e4dbb..2c81ca42ec4e 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -140,10 +140,10 @@ secondary_start_sbi:
.option pop

/*
- * Disable FPU to detect illegal usage of
- * floating point in kernel space
+ * Disable FPU & VECTOR to detect illegal usage of
+ * floating point or vector in kernel space
*/
- li t0, SR_FS
+ li t0, SR_FS | SR_VS
csrc CSR_STATUS, t0

/* Set trap vector to spin forever to help debug */
@@ -234,10 +234,10 @@ pmp_done:
.option pop

/*
- * Disable FPU to detect illegal usage of
- * floating point in kernel space
+ * Disable FPU & VECTOR to detect illegal usage of
+ * floating point or vector in kernel space
*/
- li t0, SR_FS
+ li t0, SR_FS | SR_VS
csrc CSR_STATUS, t0

#ifdef CONFIG_RISCV_BOOT_SPINWAIT
@@ -431,6 +431,29 @@ 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
+ and t0, t0, t1
+ beqz t0, .Lreset_regs_done
+
+ /*
+ * 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, ta, ma
+ 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)
--
2.25.1

2022-11-04 05:41:33

[permalink] [raw]

Subject: Re: [PATCH v12 06/17] riscv: Reset vector register

On 9/21/22 14:43, Chris Stillson wrote:
> From: Guo Ren <[email protected]>
>
> Reset vector registers at boot-time and disable vector instructions
> execution for kernel mode.

Perhaps bike-shedding, but "Reset" has a different connotation in
kernel, this is clear registers IMO. And "Reset Vector ..." sounds
totally different at first glance.

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

Is VS writable in implementations not implementing V hardware.

Priv spec seems to be confusing. It states

"The FS[1:0] and VS[1:0] WARL fields..."

Above implies it can be written always but will read legal values only.
But then this follows.

"If neither the v registers nor S-mode is implemented, then VS
is read-only zero. If S-mode is implemented but the v registers
are not, VS may optionally be read-only zero"

What does optionally mean for software ?

>
> 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 b865046e4dbb..2c81ca42ec4e 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -140,10 +140,10 @@ secondary_start_sbi:
> .option pop
>
> /*
> - * Disable FPU to detect illegal usage of
> - * floating point in kernel space
> + * Disable FPU & VECTOR to detect illegal usage of
> + * floating point or vector in kernel space
> */
> - li t0, SR_FS
> + li t0, SR_FS | SR_VS
> csrc CSR_STATUS, t0
>
> /* Set trap vector to spin forever to help debug */
> @@ -234,10 +234,10 @@ pmp_done:
> .option pop
>
> /*
> - * Disable FPU to detect illegal usage of
> - * floating point in kernel space
> + * Disable FPU & VECTOR to detect illegal usage of
> + * floating point or vector in kernel space
> */
> - li t0, SR_FS
> + li t0, SR_FS | SR_VS
> csrc CSR_STATUS, t0

Third instance of duplicated SR_FS | SR_VS. Better to add a helper
SR_FS_VS or some such macro.

>
> #ifdef CONFIG_RISCV_BOOT_SPINWAIT
> @@ -431,6 +431,29 @@ 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
> + and t0, t0, t1
> + beqz t0, .Lreset_regs_done
> +
> + /*
> + * 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, ta, ma
> + 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 */

Is that actually happening ?

2022-11-04 05:43:33

[permalink] [raw]

Subject: Re: [PATCH v12 07/17] riscv: Add vector struct and assembler definitions

On 9/21/22 14:43, Chris Stillson wrote:
> From: Greentime Hu <[email protected]>
>
> Add 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.

"Vector state includes vector reg file and additional dynamic
configuration CSRs. To handle variable sized reg file context (due to
implementation defined ref size) and to enable lazy-allocation of this,
there's datap which points to appropriate location on user/kernel mode
stack as relevant..."

Something like above.

> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> index 19eedd4af4cd..95917a2b24f9 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -39,6 +39,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;

I think this patch should be preparatory, don't wire up the vstate in
thread_struct now. Only do it when the save/restore calls are wired up
in low level code.

> +struct __riscv_v_state {
> + unsigned long vstart;
> + unsigned long vl;
> + 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.
> + */

Nice.

>
> + 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);
> +

Ok.

Also move the __vstate_{save,restore} functions from patch 5/17 here.

2022-11-04 09:00:37

by Guo Ren

[permalink] [raw]

Subject: Re: [PATCH v12 06/17] riscv: Reset vector register

On Fri, Nov 4, 2022 at 1:01 PM Vineet Gupta <[email protected]> wrote:
>
> On 9/21/22 14:43, Chris Stillson wrote:
> > From: Guo Ren <[email protected]>
> >
> > Reset vector registers at boot-time and disable vector instructions
> > execution for kernel mode.
>
> Perhaps bike-shedding, but "Reset" has a different connotation in
> kernel, this is clear registers IMO. And "Reset Vector ..." sounds
> totally different at first glance.
Agree, "Clear vector registers" is okay.

>
>
> > - * 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
>
> Is VS writable in implementations not implementing V hardware.
>
> Priv spec seems to be confusing. It states
>
> "The FS[1:0] and VS[1:0] WARL fields..."
>
> Above implies it can be written always but will read legal values only.
> But then this follows.
>
> "If neither the v registers nor S-mode is implemented, then VS
> is read-only zero. If S-mode is implemented but the v registers
> are not, VS may optionally be read-only zero"
>
> What does optionally mean for software ?
The read-only zero bit is safe for writing 1, but the result is still
zero. So let's keep it for easier coding.

>
> >
> > 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 b865046e4dbb..2c81ca42ec4e 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -140,10 +140,10 @@ secondary_start_sbi:
> > .option pop
> >
> > /*
> > - * Disable FPU to detect illegal usage of
> > - * floating point in kernel space
> > + * Disable FPU & VECTOR to detect illegal usage of
> > + * floating point or vector in kernel space
> > */
> > - li t0, SR_FS
> > + li t0, SR_FS | SR_VS
> > csrc CSR_STATUS, t0
> >
> > /* Set trap vector to spin forever to help debug */
> > @@ -234,10 +234,10 @@ pmp_done:
> > .option pop
> >
> > /*
> > - * Disable FPU to detect illegal usage of
> > - * floating point in kernel space
> > + * Disable FPU & VECTOR to detect illegal usage of
> > + * floating point or vector in kernel space
> > */
> > - li t0, SR_FS
> > + li t0, SR_FS | SR_VS
> > csrc CSR_STATUS, t0
>
> Third instance of duplicated SR_FS | SR_VS. Better to add a helper
> SR_FS_VS or some such macro.
Good point. But we could move it to another patch and define a new
SR_AXS for all.

#define SR_AXS (SR_FS | SR_VS | SR_XS)

>
> >
> > #ifdef CONFIG_RISCV_BOOT_SPINWAIT
> > @@ -431,6 +431,29 @@ 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
> > + and t0, t0, t1
> > + beqz t0, .Lreset_regs_done
> > +
> > + /*
> > + * 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, ta, ma
> > + 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 */
>
> Is that actually happening ?
Yes, It's the same as FPU, see head.S _start_kernel:

ENTRY(_start_kernel)
..
/* Reset all registers except ra, a0, a1 */
call reset_regs
...

>
>

--
Best Regards
Guo Ren

2022-11-04 22:40:06

[permalink] [raw]

Subject: Re: [PATCH v12 08/17] riscv: Add task switch support for vector

On 9/21/22 14:43, Chris Stillson wrote:
> From: Greentime Hu <[email protected]>
>
> 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]: 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]
> [[email protected]: Fix the arch_release_task_struct free wrong
> datap issue]
>
> 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]>
> Co-developed-by: Ruinland Tsai <[email protected]>
> Signed-off-by: Ruinland Tsai <[email protected]>
> Signed-off-by: Greentime Hu <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> ---
> arch/riscv/include/asm/switch_to.h | 66 ++++++++++++++++++++++++++++++
> arch/riscv/kernel/Makefile | 1 +
> arch/riscv/kernel/process.c | 43 +++++++++++++++++++
> 3 files changed, 110 insertions(+)
>
> diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
> index df1aa589b7fd..527951c033d4 100644
> --- a/arch/riscv/include/asm/switch_to.h
> +++ b/arch/riscv/include/asm/switch_to.h
> @@ -7,11 +7,13 @@
> #define _ASM_RISCV_SWITCH_TO_H
>
> #include <linux/jump_label.h>
> +#include <linux/slab.h>
> #include <linux/sched/task_stack.h>
> #include <asm/hwcap.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,68 @@ 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

All of this needs to be moved into vector.h for better containment.
I would also wire in struct __riscv_v_state vstate in struct
thread_struct in this patch.

> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index 33bb60a354cd..35752fb6d145 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -55,6 +55,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

This needs to go into last patch which adds Kconfig/Makefile enabling.

> +
> + if (has_vector()) {

Would it make sense to add IS_ENABLED(CONFIG_VECTOR) inside this helper
- would help compiler remove the codegen completely for !VECTOR but
still having some build test coverage. Anyhow this is minor point and
can be added later.

> + 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);
> + }
> +

...

> +#ifdef CONFIG_VECTOR
> + /* Reset vector state */
> + vstate_off(current, task_pt_regs(current));
> + memset(&current->thread.vstate, 0, RISCV_V_STATE_DATAP);
> +#endif

This doesn't check has_vector() as we want to unconditionally clean
memory for security reasons ?

> }
>
> int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
> {
> fstate_save(src, task_pt_regs(src));
> *dst = *src;
> + dst->thread.vstate.datap = NULL;

has_vector() needed here ?

>
> +void arch_release_task_struct(struct task_struct *tsk)
> +{
> + /* Free the vector context of datap. */
> + if (has_vector() && tsk->thread.vstate.datap)
> + kfree(tsk->thread.vstate.datap);
> +}
> +
> int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
> {
> unsigned long clone_flags = args->flags;
> @@ -175,7 +208,17 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
> p->thread.ra = (unsigned long)ret_from_kernel_thread;
> p->thread.s[0] = (unsigned long)args->fn;
> p->thread.s[1] = (unsigned long)args->fn_arg;
> + p->thread.vstate.datap = NULL;
> } else {
> + /* Allocate the datap for the user process if datap is NULL */
> + if (has_vector() && !p->thread.vstate.datap) {
> + void *datap = kzalloc(riscv_vsize, GFP_KERNEL);
> + /* Failed to allocate memory. */
> + if (!datap)
> + return -ENOMEM;
> + p->thread.vstate.datap = datap;
> + memset(&p->thread.vstate, 0, RISCV_V_STATE_DATAP);
> + }
> *childregs = *(current_pt_regs());
> if (usp) /* User fork */
> childregs->sp = usp;

2022-11-07 17:49:52

by Björn Töpel

[permalink] [raw]

Subject: Re: [PATCH v12 04/17] riscv: Add vector feature to compile

[Cropping the huge Cc:-list.]

Chris Stillson <[email protected]> writes:

> From: Guo Ren <[email protected]>
>
> This patch adds a new config option which could enable assembler's
> vector feature.
>
> Signed-off-by: Guo Ren <[email protected]>
> Co-developed-by: Greentime Hu <[email protected]>
> Signed-off-by: Greentime Hu <[email protected]>
> ---
> arch/riscv/Kconfig | 15 +++++++++++++--
> arch/riscv/Makefile | 1 +
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index ed66c31e4655..e294d85bfb7d 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -432,7 +432,17 @@ config FPU
>
> If you don't know what to do here, say Y.
>
> -endmenu # "Platform type"
> +config VECTOR
> + bool "VECTOR support"
> + depends on GCC_VERSION >= 120000 || CLANG_VERSION >= 130000
> + 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

"VECTOR" is not really consistent to how the other configs are named;
RISCV_ISA_V, RISCV_ISA_VECTOR, RISCV_VECTOR?

Björn

2022-11-08 00:27:29

[permalink] [raw]

Subject: Re: [PATCH v12 04/17] riscv: Add vector feature to compile

+CC Andy, Conor

On 11/7/22 09:21, Björn Töpel wrote:
>> +config VECTOR
>> + bool "VECTOR support"
>> + depends on GCC_VERSION >= 120000 || CLANG_VERSION >= 130000
>> + 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
> "VECTOR" is not really consistent to how the other configs are named;
> RISCV_ISA_V, RISCV_ISA_VECTOR, RISCV_VECTOR?

Good point, I've changed it to RISCV_ISA_V to keep it consistent with
existing RISCV_ISA_C.

2022-11-08 08:26:51

[permalink] [raw]

Subject: Re: [PATCH v12 04/17] riscv: Add vector feature to compile

On Mon, Nov 07, 2022 at 04:04:28PM -0800, Vineet Gupta wrote:
> +CC Andy, Conor
>
> On 11/7/22 09:21, Bj?rn T?pel wrote:
> > > +config VECTOR
> > > + bool "VECTOR support"
> > > + depends on GCC_VERSION >= 120000 || CLANG_VERSION >= 130000
> > > + 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
> > "VECTOR" is not really consistent to how the other configs are named;
> > RISCV_ISA_V, RISCV_ISA_VECTOR, RISCV_VECTOR?
>
> Good point, I've changed it to RISCV_ISA_V to keep it consistent with
> existing RISCV_ISA_C.

Hey Vineet, kinda randomly replying here but the wording makes it look
like you're going to take this patchset on?
If so, please check out v10 (think it was from April) as there are some
comments on that version that IIRC remain un-resolved.
Thanks,
Conor.

2022-11-08 17:28:21

[permalink] [raw]

Subject: Re: [PATCH v12 04/17] riscv: Add vector feature to compile

On 11/7/22 23:56, Conor Dooley wrote:
> On Mon, Nov 07, 2022 at 04:04:28PM -0800, Vineet Gupta wrote:
>> +CC Andy, Conor
>>
>> On 11/7/22 09:21, Björn Töpel wrote:
>>>> +config VECTOR
>>>> + bool "VECTOR support"
>>>> + depends on GCC_VERSION >= 120000 || CLANG_VERSION >= 130000
>>>> + 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
>>> "VECTOR" is not really consistent to how the other configs are named;
>>> RISCV_ISA_V, RISCV_ISA_VECTOR, RISCV_VECTOR?
>>
>> Good point, I've changed it to RISCV_ISA_V to keep it consistent with
>> existing RISCV_ISA_C.
>
> Hey Vineet, kinda randomly replying here but the wording makes it look
> like you're going to take this patchset on?
> If so, please check out v10 (think it was from April) as there are some
> comments on that version that IIRC remain un-resolved.

Sure thing. Although I only see a few from Christoph and kernel build bot.

-Vineet

2022-11-08 18:12:44

[permalink] [raw]

Subject: Re: [PATCH v12 04/17] riscv: Add vector feature to compile

On Tue, Nov 08, 2022 at 09:17:26AM -0800, Vineet Gupta wrote:
> On 11/7/22 23:56, Conor Dooley wrote:
> > On Mon, Nov 07, 2022 at 04:04:28PM -0800, Vineet Gupta wrote:
> > > +CC Andy, Conor
> > >
> > > On 11/7/22 09:21, Bj?rn T?pel wrote:
> > > > > +config VECTOR
> > > > > + bool "VECTOR support"
> > > > > + depends on GCC_VERSION >= 120000 || CLANG_VERSION >= 130000
> > > > > + 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
> > > > "VECTOR" is not really consistent to how the other configs are named;
> > > > RISCV_ISA_V, RISCV_ISA_VECTOR, RISCV_VECTOR?
> > >
> > > Good point, I've changed it to RISCV_ISA_V to keep it consistent with
> > > existing RISCV_ISA_C.
> >
> > Hey Vineet, kinda randomly replying here but the wording makes it look
> > like you're going to take this patchset on?
> > If so, please check out v10 (think it was from April) as there are some
> > comments on that version that IIRC remain un-resolved.
>
> Sure thing. Although I only see a few from Christoph and kernel build bot.

Yup, they were minor - I just don't wanna see them get lost :)

2022-11-13 16:24:51

[permalink] [raw]

Subject: Re: [PATCH v12 04/17] riscv: Add vector feature to compile

On 07/11/2022 17:21, Björn Töpel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> [Cropping the huge Cc:-list.]
>
> Chris Stillson <[email protected]> writes:
>
>> From: Guo Ren <[email protected]>
>>
>> This patch adds a new config option which could enable assembler's
>> vector feature.
>>
>> Signed-off-by: Guo Ren <[email protected]>
>> Co-developed-by: Greentime Hu <[email protected]>
>> Signed-off-by: Greentime Hu <[email protected]>
>> ---
>> arch/riscv/Kconfig | 15 +++++++++++++--
>> arch/riscv/Makefile | 1 +
>> 2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index ed66c31e4655..e294d85bfb7d 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -432,7 +432,17 @@ config FPU
>>
>> If you don't know what to do here, say Y.
>>
>> -endmenu # "Platform type"
>> +config VECTOR
>> + bool "VECTOR support"
>> + depends on GCC_VERSION >= 120000 || CLANG_VERSION >= 130000
>> + 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
>
> "VECTOR" is not really consistent to how the other configs are named;
> RISCV_ISA_V, RISCV_ISA_VECTOR, RISCV_VECTOR?

It'd be RISCV_ISA_V to match the others single letter extentions, right?

The toolchain dependency check here also seems rather naive.

2022-11-15 17:52:27

[permalink] [raw]

Subject: Re: [PATCH v12 04/17] riscv: Add vector feature to compile

On 11/13/22 08:16, [email protected] wrote:
>>> +config VECTOR
>>> + bool "VECTOR support"
>>> + depends on GCC_VERSION >= 120000 || CLANG_VERSION >= 130000
>>> + 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
>> "VECTOR" is not really consistent to how the other configs are named;
>> RISCV_ISA_V, RISCV_ISA_VECTOR, RISCV_VECTOR?
> It'd be RISCV_ISA_V to match the others single letter extentions, right?

Yep.

> The toolchain dependency check here also seems rather naive.

Indeed. I can build the code just fine with gcc-11 (and gcc-12),
although my reworked patcheset doesn't include all the orig patches
including the in-kernel xor stuff.

-Vineet

2022-11-15 22:35:46