[v1, 0/6] riscv: support kernel-mode Vector

[v1, 0/6] riscv: support kernel-mode Vector

[Andy Chiu]

This series provides support for running Vector code in kernel mode. The
implementation is based on the v12 series of the Vector series, but with
some additions. First, we introduce a mechanism to defer restoring
Vector context for userspace programs (patch 1). This is similar to
arm64 and x86's approaches when dealing with extra userspace register
context. And it is benefitial to both Vector in user and kernel-mode.
Then, patch 2, 3 add the kernel-mode Vector patch from v12 with minor
modifications. At the end of the series, patch 4, 5, 6 add supports for
making kernel-mode Vector code preemptible. We do this by adding
kernel-mode Vector context, and keeping track of the frame where V
context is last valid. We believe that enabling preemption of running V
is a critical path for getting V more generally available in the
kernel-mode. Besides, with status.VS, we can easily tell if
saving/restoring V is required. This reduce the level of cost when
running SIMD in kernel mode as compared to other arches. Other arches
usually do not have a way to tell if extra context is dirty. Thus, if
they also want to support running preemptible code with extra registers,
then they must save/restore extra context at each context switch even if
registers are not dirty.

The series is tested by loading a kernel module on a preemptive kernel.
The module launches multiple kworkers which run Vector operations and
verifies with scalar code. Also, the module provides userspace intefaces
via fops to verify if we can run Vector code on syscall path.

Changes from the vector v12 series (for patch 2, 3):
 - return a failure code when kernel_rvv_begin() fails.
 - Do not immediately restore user's V context.


Andy Chiu (4):
  riscv: sched: defer restoring Vector context for user
  riscv: vector: do not pass task_struct into
    riscv_v_vstate_{save,restore}()
  riscv: vector: allow kernel-mode Vector with preemption
  riscv: vector: enable preemptive kernel-mode Vector to be built

Greentime Hu (2):
  riscv: Add support for kernel mode vector
  riscv: Add vector extension XOR implementation

 arch/riscv/Kconfig                     |  10 ++
 arch/riscv/include/asm/entry-common.h  |  13 ++
 arch/riscv/include/asm/processor.h     |   2 +
 arch/riscv/include/asm/thread_info.h   |   6 +
 arch/riscv/include/asm/vector.h        |  50 +++++--
 arch/riscv/include/asm/xor.h           |  82 +++++++++++
 arch/riscv/kernel/Makefile             |   1 +
 arch/riscv/kernel/asm-offsets.c        |   2 +
 arch/riscv/kernel/entry.S              |  41 ++++++
 arch/riscv/kernel/kernel_mode_vector.c | 180 +++++++++++++++++++++++++
 arch/riscv/kernel/process.c            |  10 +-
 arch/riscv/kernel/ptrace.c             |   2 +-
 arch/riscv/kernel/signal.c             |   4 +-
 arch/riscv/kernel/vector.c             |   5 +-
 arch/riscv/lib/Makefile                |   1 +
 arch/riscv/lib/xor.S                   |  81 +++++++++++
 16 files changed, 473 insertions(+), 17 deletions(-)
 create mode 100644 arch/riscv/include/asm/xor.h
 create mode 100644 arch/riscv/kernel/kernel_mode_vector.c
 create mode 100644 arch/riscv/lib/xor.S

-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[v1, 1/6] riscv: sched: defer restoring Vector context for user

[Andy Chiu]

User's will use its Vector registers only after the kernel really
returns to the userspace. So we can delay restoring Vector registers as
long as we are still running in kernel mode. So, add a thread flag to
indicates the need of restoring Vector and do the restore at the last
arch-specific exit-to-user hook. This save the context restoring cost
when we switch over multiple processes that run V in kernel mode. For
example, if the kernel performs context swicth from A->B->C, and returns
to C's userspace, then there is no need for restoring B's V-register.

Besides, this also prevents us from repeatedly restoring V context when
executing kernel-mode Vector multiple times for the upcoming kenel-mode
Vector patches.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
 arch/riscv/include/asm/entry-common.h | 13 +++++++++++++
 arch/riscv/include/asm/thread_info.h  |  2 ++
 arch/riscv/include/asm/vector.h       | 11 ++++++++++-
 arch/riscv/kernel/process.c           |  2 ++
 arch/riscv/kernel/signal.c            |  2 +-
 arch/riscv/kernel/vector.c            |  2 +-
 6 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/include/asm/entry-common.h b/arch/riscv/include/asm/entry-common.h
index 6e4dee49d84b..52926f4d8d7c 100644
--- a/arch/riscv/include/asm/entry-common.h
+++ b/arch/riscv/include/asm/entry-common.h
@@ -4,6 +4,19 @@
 #define _ASM_RISCV_ENTRY_COMMON_H
 
 #include <asm/stacktrace.h>
+#include <asm/thread_info.h>
+#include <asm/vector.h>
+
+static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
+						  unsigned long ti_work)
+{
+	if (ti_work & _TIF_RISCV_V_DEFER_RESTORE) {
+		clear_thread_flag(TIF_RISCV_V_DEFER_RESTORE);
+		riscv_v_vstate_restore(current, regs);
+	}
+}
+
+#define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare
 
 void handle_page_fault(struct pt_regs *regs);
 void handle_break(struct pt_regs *regs);
diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index 97e6f65ec176..d83975efe866 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -101,12 +101,14 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
 #define TIF_NOTIFY_SIGNAL	9	/* signal notifications exist */
 #define TIF_UPROBE		10	/* uprobe breakpoint or singlestep */
 #define TIF_32BIT		11	/* compat-mode 32bit process */
+#define TIF_RISCV_V_DEFER_RESTORE	12
 
 #define _TIF_NOTIFY_RESUME	(1 << TIF_NOTIFY_RESUME)
 #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
 #define _TIF_NEED_RESCHED	(1 << TIF_NEED_RESCHED)
 #define _TIF_NOTIFY_SIGNAL	(1 << TIF_NOTIFY_SIGNAL)
 #define _TIF_UPROBE		(1 << TIF_UPROBE)
+#define _TIF_RISCV_V_DEFER_RESTORE	(1 << TIF_RISCV_V_DEFER_RESTORE)
 
 #define _TIF_WORK_MASK \
 	(_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | _TIF_NEED_RESCHED | \
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index 3d78930cab51..a4f3705fd144 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -183,6 +183,15 @@ static inline void riscv_v_vstate_restore(struct task_struct *task,
 	}
 }
 
+static inline void riscv_v_vstate_set_restore(struct task_struct *task,
+					      struct pt_regs *regs)
+{
+	if ((regs->status & SR_VS) != SR_VS_OFF) {
+		set_tsk_thread_flag(task, TIF_RISCV_V_DEFER_RESTORE);
+		riscv_v_vstate_on(regs);
+	}
+}
+
 static inline void __switch_to_vector(struct task_struct *prev,
 				      struct task_struct *next)
 {
@@ -190,7 +199,7 @@ static inline void __switch_to_vector(struct task_struct *prev,
 
 	regs = task_pt_regs(prev);
 	riscv_v_vstate_save(prev, regs);
-	riscv_v_vstate_restore(next, task_pt_regs(next));
+	riscv_v_vstate_set_restore(next, task_pt_regs(next));
 }
 
 void riscv_v_vstate_ctrl_init(struct task_struct *tsk);
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index e32d737e039f..ec89e7edb6fd 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -153,6 +153,7 @@ void flush_thread(void)
 	riscv_v_vstate_off(task_pt_regs(current));
 	kfree(current->thread.vstate.datap);
 	memset(&current->thread.vstate, 0, sizeof(struct __riscv_v_ext_state));
+	clear_tsk_thread_flag(current, TIF_RISCV_V_DEFER_RESTORE);
 #endif
 }
 
@@ -169,6 +170,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
 	*dst = *src;
 	/* clear entire V context, including datap for a new task */
 	memset(&dst->thread.vstate, 0, sizeof(struct __riscv_v_ext_state));
+	clear_tsk_thread_flag(dst, TIF_RISCV_V_DEFER_RESTORE);
 
 	return 0;
 }
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 180d951d3624..0fca2c128b5f 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -134,7 +134,7 @@ static long __restore_v_state(struct pt_regs *regs, void __user *sc_vec)
 	if (unlikely(err))
 		return err;
 
-	riscv_v_vstate_restore(current, regs);
+	riscv_v_vstate_set_restore(current, regs);
 
 	return err;
 }
diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
index 8d92fb6c522c..9d583b760db4 100644
--- a/arch/riscv/kernel/vector.c
+++ b/arch/riscv/kernel/vector.c
@@ -167,7 +167,7 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
 		return true;
 	}
 	riscv_v_vstate_on(regs);
-	riscv_v_vstate_restore(current, regs);
+	riscv_v_vstate_set_restore(current, regs);
 	return true;
 }
 
-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[v1, 2/6] riscv: Add support for kernel mode vector

[Andy Chiu]

From: Greentime Hu <greentime.hu@sifive.com>

Add kernel_rvv_begin() and kernel_rvv_end() function declarations
and corresponding definitions in kernel_mode_vector.c

These are needed to wrap uses of vector in kernel mode.

Co-developed-by: Vincent Chen <vincent.chen@sifive.com>
Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
 arch/riscv/include/asm/vector.h        |   2 +
 arch/riscv/kernel/Makefile             |   1 +
 arch/riscv/kernel/kernel_mode_vector.c | 129 +++++++++++++++++++++++++
 3 files changed, 132 insertions(+)
 create mode 100644 arch/riscv/kernel/kernel_mode_vector.c

diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index a4f3705fd144..9831b19153ae 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -22,6 +22,8 @@
 extern unsigned long riscv_v_vsize;
 int riscv_v_setup_vsize(void);
 bool riscv_v_first_use_handler(struct pt_regs *regs);
+int kernel_rvv_begin(void);
+void kernel_rvv_end(void);
 
 static __always_inline bool has_vector(void)
 {
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index a42951911067..b954bbf17c84 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_MMU) += vdso.o vdso/
 obj-$(CONFIG_RISCV_M_MODE)	+= traps_misaligned.o
 obj-$(CONFIG_FPU)		+= fpu.o
 obj-$(CONFIG_RISCV_ISA_V)	+= vector.o
+obj-$(CONFIG_RISCV_ISA_V)	+= kernel_mode_vector.o
 obj-$(CONFIG_SMP)		+= smpboot.o
 obj-$(CONFIG_SMP)		+= smp.o
 obj-$(CONFIG_SMP)		+= cpu_ops.o
diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
new file mode 100644
index 000000000000..c0c152c501a5
--- /dev/null
+++ b/arch/riscv/kernel/kernel_mode_vector.c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2012 ARM Ltd.
+ * Author: Catalin Marinas <catalin.marinas@arm.com>
+ * Copyright (C) 2017 Linaro Ltd. <ard.biesheuvel@linaro.org>
+ * Copyright (C) 2021 SiFive
+ */
+#include <linux/compiler.h>
+#include <linux/irqflags.h>
+#include <linux/percpu.h>
+#include <linux/preempt.h>
+#include <linux/types.h>
+
+#include <asm/vector.h>
+#include <asm/switch_to.h>
+
+DECLARE_PER_CPU(bool, vector_context_busy);
+DEFINE_PER_CPU(bool, vector_context_busy);
+
+/*
+ * may_use_vector - whether it is allowable at this time to issue vector
+ *                instructions or access the vector register file
+ *
+ * Callers must not assume that the result remains true beyond the next
+ * preempt_enable() or return from softirq context.
+ */
+static __must_check inline bool may_use_vector(void)
+{
+	/*
+	 * vector_context_busy is only set while preemption is disabled,
+	 * and is clear whenever preemption is enabled. Since
+	 * this_cpu_read() is atomic w.r.t. preemption, vector_context_busy
+	 * cannot change under our feet -- if it's set we cannot be
+	 * migrated, and if it's clear we cannot be migrated to a CPU
+	 * where it is set.
+	 */
+	return !in_irq() && !irqs_disabled() && !in_nmi() &&
+	       !this_cpu_read(vector_context_busy);
+}
+
+/*
+ * Claim ownership of the CPU vector context for use by the calling context.
+ *
+ * The caller may freely manipulate the vector context metadata until
+ * put_cpu_vector_context() is called.
+ */
+static void get_cpu_vector_context(void)
+{
+	bool busy;
+
+	preempt_disable();
+	busy = __this_cpu_xchg(vector_context_busy, true);
+
+	WARN_ON(busy);
+}
+
+/*
+ * Release the CPU vector context.
+ *
+ * Must be called from a context in which get_cpu_vector_context() was
+ * previously called, with no call to put_cpu_vector_context() in the
+ * meantime.
+ */
+static void put_cpu_vector_context(void)
+{
+	bool busy = __this_cpu_xchg(vector_context_busy, false);
+
+	WARN_ON(!busy);
+	preempt_enable();
+}
+
+/*
+ * kernel_rvv_begin(): obtain the CPU vector registers for use by the calling
+ * context
+ *
+ * Must not be called unless may_use_vector() returns true.
+ * Task context in the vector registers is saved back to memory as necessary.
+ *
+ * A matching call to kernel_rvv_end() must be made before returning from the
+ * calling context.
+ *
+ * The caller may freely use the vector registers until kernel_rvv_end() is
+ * called.
+ */
+int kernel_rvv_begin(void)
+{
+	if (!has_vector())
+		return -EOPNOTSUPP;
+
+	if (!may_use_vector())
+		return -EPERM;
+
+	/* Save vector state, if any */
+	riscv_v_vstate_save(current, task_pt_regs(current));
+
+	/* Acquire kernel mode vector */
+	get_cpu_vector_context();
+
+	/* Enable vector */
+	riscv_v_enable();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kernel_rvv_begin);
+
+/*
+ * kernel_rvv_end(): give the CPU vector registers back to the current task
+ *
+ * Must be called from a context in which kernel_rvv_begin() was previously
+ * called, with no call to kernel_rvv_end() in the meantime.
+ *
+ * The caller must not use the vector registers after this function is called,
+ * unless kernel_rvv_begin() is called again in the meantime.
+ */
+void kernel_rvv_end(void)
+{
+	if (WARN_ON(!has_vector()))
+		return;
+
+	/* Restore vector state, if any */
+	riscv_v_vstate_set_restore(current, task_pt_regs(current));
+
+	/* disable vector */
+	riscv_v_disable();
+
+	/* release kernel mode vector */
+	put_cpu_vector_context();
+}
+EXPORT_SYMBOL_GPL(kernel_rvv_end);
-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[v1, 3/6] riscv: Add vector extension XOR implementation

[Andy Chiu]

From: Greentime Hu <greentime.hu@sifive.com>

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

Co-developed-by: Han-Kuan Chen <hankuan.chen@sifive.com>
Signed-off-by: Han-Kuan Chen <hankuan.chen@sifive.com>
Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
 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..81b8837fa161
--- /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_RISCV_ISA_V
+#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 26cb2502ecf8..494f9cd1a00c 100644
--- a/arch/riscv/lib/Makefile
+++ b/arch/riscv/lib/Makefile
@@ -11,3 +11,4 @@ lib-$(CONFIG_64BIT)	+= tishift.o
 lib-$(CONFIG_RISCV_ISA_ZICBOZ)	+= clear_page.o
 
 obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
+lib-$(CONFIG_RISCV_ISA_V)	+= 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.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[v1, 4/6] riscv: vector: do not pass task_struct into riscv_v_vstate_{save,restore}()

[Andy Chiu]

riscv_v_vstate_{save,restore}() can operate only on the knowlege of
struct __riscv_v_ext_state, and struct pt_regs. Let the caller decides
which should be passed into the function. Meanwhile, the kernel-mode
Vector is going to introduce another vstate, so this also makes functions
potentially able to be reused.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
 arch/riscv/include/asm/entry-common.h  |  2 +-
 arch/riscv/include/asm/vector.h        | 14 +++++---------
 arch/riscv/kernel/kernel_mode_vector.c |  2 +-
 arch/riscv/kernel/ptrace.c             |  2 +-
 arch/riscv/kernel/signal.c             |  2 +-
 5 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/arch/riscv/include/asm/entry-common.h b/arch/riscv/include/asm/entry-common.h
index 52926f4d8d7c..aa1b9e50d6c8 100644
--- a/arch/riscv/include/asm/entry-common.h
+++ b/arch/riscv/include/asm/entry-common.h
@@ -12,7 +12,7 @@ static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
 {
 	if (ti_work & _TIF_RISCV_V_DEFER_RESTORE) {
 		clear_thread_flag(TIF_RISCV_V_DEFER_RESTORE);
-		riscv_v_vstate_restore(current, regs);
+		riscv_v_vstate_restore(&current->thread.vstate, regs);
 	}
 }
 
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index 9831b19153ae..50c556afd95a 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -163,23 +163,19 @@ static inline void riscv_v_vstate_discard(struct pt_regs *regs)
 	__riscv_v_vstate_dirty(regs);
 }
 
-static inline void riscv_v_vstate_save(struct task_struct *task,
+static inline void riscv_v_vstate_save(struct __riscv_v_ext_state *vstate,
 				       struct pt_regs *regs)
 {
 	if ((regs->status & SR_VS) == SR_VS_DIRTY) {
-		struct __riscv_v_ext_state *vstate = &task->thread.vstate;
-
 		__riscv_v_vstate_save(vstate, vstate->datap);
 		__riscv_v_vstate_clean(regs);
 	}
 }
 
-static inline void riscv_v_vstate_restore(struct task_struct *task,
+static inline void riscv_v_vstate_restore(struct __riscv_v_ext_state *vstate,
 					  struct pt_regs *regs)
 {
 	if ((regs->status & SR_VS) != SR_VS_OFF) {
-		struct __riscv_v_ext_state *vstate = &task->thread.vstate;
-
 		__riscv_v_vstate_restore(vstate, vstate->datap);
 		__riscv_v_vstate_clean(regs);
 	}
@@ -200,7 +196,7 @@ static inline void __switch_to_vector(struct task_struct *prev,
 	struct pt_regs *regs;
 
 	regs = task_pt_regs(prev);
-	riscv_v_vstate_save(prev, regs);
+	riscv_v_vstate_save(prev->thread.vstate, regs);
 	riscv_v_vstate_set_restore(next, task_pt_regs(next));
 }
 
@@ -218,8 +214,8 @@ static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; }
 static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; }
 #define riscv_v_vsize (0)
 #define riscv_v_vstate_discard(regs)		do {} while (0)
-#define riscv_v_vstate_save(task, regs)		do {} while (0)
-#define riscv_v_vstate_restore(task, regs)	do {} while (0)
+#define riscv_v_vstate_save(vstate, regs)	do {} while (0)
+#define riscv_v_vstate_restore(vstate, regs)	do {} while (0)
 #define __switch_to_vector(__prev, __next)	do {} while (0)
 #define riscv_v_vstate_off(regs)		do {} while (0)
 #define riscv_v_vstate_on(regs)			do {} while (0)
diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
index c0c152c501a5..30f1b861cac0 100644
--- a/arch/riscv/kernel/kernel_mode_vector.c
+++ b/arch/riscv/kernel/kernel_mode_vector.c
@@ -91,7 +91,7 @@ int kernel_rvv_begin(void)
 		return -EPERM;
 
 	/* Save vector state, if any */
-	riscv_v_vstate_save(current, task_pt_regs(current));
+	riscv_v_vstate_save(&current->thread.vstate, task_pt_regs(current));
 
 	/* Acquire kernel mode vector */
 	get_cpu_vector_context();
diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
index 1d572cf3140f..85e7167245cc 100644
--- a/arch/riscv/kernel/ptrace.c
+++ b/arch/riscv/kernel/ptrace.c
@@ -99,7 +99,7 @@ static int riscv_vr_get(struct task_struct *target,
 	 * copying them to membuf.
 	 */
 	if (target == current)
-		riscv_v_vstate_save(current, task_pt_regs(current));
+		riscv_v_vstate_save(&current->thread.vstate, task_pt_regs(current));
 
 	/* Copy vector header from vstate. */
 	membuf_write(&to, vstate, offsetof(struct __riscv_v_ext_state, datap));
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 0fca2c128b5f..75fd8cc05e10 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -86,7 +86,7 @@ static long save_v_state(struct pt_regs *regs, void __user **sc_vec)
 	/* datap is designed to be 16 byte aligned for better performance */
 	WARN_ON(unlikely(!IS_ALIGNED((unsigned long)datap, 16)));
 
-	riscv_v_vstate_save(current, regs);
+	riscv_v_vstate_save(&current->thread.vstate, regs);
 	/* Copy everything of vstate but datap. */
 	err = __copy_to_user(&state->v_state, &current->thread.vstate,
 			     offsetof(struct __riscv_v_ext_state, datap));
-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[v1, 5/6] riscv: vector: allow kernel-mode Vector with preemption

[Andy Chiu]

Add kernel_vstate to keep track of kernel-mode Vector registers when
trap introduced context switch happens. Also, provide trap_pt_regs to
let context save/restore routine reference status.VS at which the trap
takes place. The thread flag TIF_RISCV_V_KMV indicates whether a task is
running in kernel-mode Vector with preemption 'ON'. So context switch
routines know and would save V-regs to kernel_vstate and restore V-regs
immediately from kernel_vstate if the bit is set.

Apart from a task's preemption status, the capability of
running preemptive kernel-mode Vector is jointly controlled by the
RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE mask in the task's
thread.vstate_ctrl. This bit is masked whenever a trap takes place in
kernel mode while executing preemptive Vector code.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
 arch/riscv/include/asm/processor.h     |  2 +
 arch/riscv/include/asm/thread_info.h   |  4 ++
 arch/riscv/include/asm/vector.h        | 27 ++++++++++--
 arch/riscv/kernel/asm-offsets.c        |  2 +
 arch/riscv/kernel/entry.S              | 41 ++++++++++++++++++
 arch/riscv/kernel/kernel_mode_vector.c | 57 ++++++++++++++++++++++++--
 arch/riscv/kernel/process.c            |  8 +++-
 arch/riscv/kernel/vector.c             |  3 +-
 8 files changed, 136 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index e82af1097e26..d337b750f2ec 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -42,6 +42,8 @@ struct thread_struct {
 	unsigned long bad_cause;
 	unsigned long vstate_ctrl;
 	struct __riscv_v_ext_state vstate;
+	struct pt_regs *trap_pt_regs;
+	struct __riscv_v_ext_state kernel_vstate;
 };
 
 /* Whitelist the fstate from the task_struct for hardened usercopy */
diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index d83975efe866..59d88adfc4de 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -102,6 +102,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
 #define TIF_UPROBE		10	/* uprobe breakpoint or singlestep */
 #define TIF_32BIT		11	/* compat-mode 32bit process */
 #define TIF_RISCV_V_DEFER_RESTORE	12
+#define TIF_RISCV_V_KMV			13
 
 #define _TIF_NOTIFY_RESUME	(1 << TIF_NOTIFY_RESUME)
 #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
@@ -109,9 +110,12 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
 #define _TIF_NOTIFY_SIGNAL	(1 << TIF_NOTIFY_SIGNAL)
 #define _TIF_UPROBE		(1 << TIF_UPROBE)
 #define _TIF_RISCV_V_DEFER_RESTORE	(1 << TIF_RISCV_V_DEFER_RESTORE)
+#define _TIF_RISCV_V_KMV		(1 << TIF_RISCV_V_KMV_TASK)
 
 #define _TIF_WORK_MASK \
 	(_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | _TIF_NEED_RESCHED | \
 	 _TIF_NOTIFY_SIGNAL | _TIF_UPROBE)
 
+#define RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE	0x20
+
 #endif /* _ASM_RISCV_THREAD_INFO_H */
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index 50c556afd95a..d004c9fa6a57 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -25,6 +25,12 @@ bool riscv_v_first_use_handler(struct pt_regs *regs);
 int kernel_rvv_begin(void);
 void kernel_rvv_end(void);
 
+#ifdef CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV
+void riscv_v_vstate_ctrl_config_kmv(bool preemptive_kmv);
+#else
+#define riscv_v_vstate_ctrl_config_kmv(preemptive_kmv)	do {} while (0)
+#endif
+
 static __always_inline bool has_vector(void)
 {
 	return riscv_has_extension_unlikely(RISCV_ISA_EXT_v);
@@ -195,9 +201,24 @@ static inline void __switch_to_vector(struct task_struct *prev,
 {
 	struct pt_regs *regs;
 
-	regs = task_pt_regs(prev);
-	riscv_v_vstate_save(prev->thread.vstate, regs);
-	riscv_v_vstate_set_restore(next, task_pt_regs(next));
+	if (IS_ENABLED(CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV) &&
+	    test_tsk_thread_flag(prev, TIF_RISCV_V_KMV)) {
+		regs = prev->thread.trap_pt_regs;
+		WARN_ON(!regs);
+		riscv_v_vstate_save(&prev->thread.kernel_vstate, regs);
+	} else {
+		regs = task_pt_regs(prev);
+		riscv_v_vstate_save(&prev->thread.vstate, regs);
+	}
+
+	if (IS_ENABLED(CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV) &&
+	    test_tsk_thread_flag(next, TIF_RISCV_V_KMV)) {
+		regs = next->thread.trap_pt_regs;
+		WARN_ON(!regs);
+		riscv_v_vstate_restore(&next->thread.kernel_vstate, regs);
+	} else {
+		riscv_v_vstate_set_restore(next, task_pt_regs(next));
+	}
 }
 
 void riscv_v_vstate_ctrl_init(struct task_struct *tsk);
diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
index d6a75aac1d27..4b062f7741b2 100644
--- a/arch/riscv/kernel/asm-offsets.c
+++ b/arch/riscv/kernel/asm-offsets.c
@@ -38,6 +38,8 @@ void asm_offsets(void)
 	OFFSET(TASK_TI_PREEMPT_COUNT, task_struct, thread_info.preempt_count);
 	OFFSET(TASK_TI_KERNEL_SP, task_struct, thread_info.kernel_sp);
 	OFFSET(TASK_TI_USER_SP, task_struct, thread_info.user_sp);
+	OFFSET(TASK_THREAD_TRAP_REGP, task_struct, thread.trap_pt_regs);
+	OFFSET(TASK_THREAD_VSTATE_CTRL, task_struct, thread.vstate_ctrl);
 
 	OFFSET(TASK_THREAD_F0,  task_struct, thread.fstate.f[0]);
 	OFFSET(TASK_THREAD_F1,  task_struct, thread.fstate.f[1]);
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 143a2bb3e697..42b80b90626a 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -66,6 +66,27 @@ _save_context:
 	REG_S s4, PT_CAUSE(sp)
 	REG_S s5, PT_TP(sp)
 
+	/*
+	 * Reocrd the register set at the frame where in-kernel V registers are
+	 * last alive.
+	 */
+	REG_L s0, TASK_TI_FLAGS(tp)
+	li s1, 1 << TIF_RISCV_V_KMV
+	and s0, s0, s1
+	beqz s0, 1f
+	li s0, TASK_THREAD_TRAP_REGP
+	add s0, s0, tp
+	REG_L s1, (s0)
+	bnez s1, 1f
+	REG_S sp, (s0)
+	li s0, TASK_THREAD_VSTATE_CTRL
+	add s0, s0, tp
+	REG_L s1, (s0)
+	li s2, ~RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE
+	and s1, s1, s2
+	REG_S s1, (s0)
+1:
+
 	/*
 	 * Set the scratch register to 0, so that if a recursive exception
 	 * occurs, the exception vector knows it came from the kernel
@@ -129,6 +150,26 @@ SYM_CODE_START_NOALIGN(ret_from_exception)
 	 */
 	csrw CSR_SCRATCH, tp
 1:
+	/*
+	 * Clear tracking of the trap registers when we return to the frame
+	 * that uses kernel mode Vector.
+	 */
+	REG_L s0, TASK_TI_FLAGS(tp)
+	li s1, 1 << TIF_RISCV_V_KMV
+	and s0, s0, s1
+	beqz s0, 1f
+	li s0, TASK_THREAD_TRAP_REGP
+	add s0, s0, tp
+	REG_L s1, (s0)
+	bne s1, sp, 1f
+	REG_S x0, (s0)
+	li s0, TASK_THREAD_VSTATE_CTRL
+	add s0, s0, tp
+	REG_L s1, (s0)
+	ori s1, s1, RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE
+	REG_S s1, (s0)
+1:
+
 	REG_L a0, PT_STATUS(sp)
 	/*
 	 * The current load reservation is effectively part of the processor's
diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
index 30f1b861cac0..bcd6a69a5266 100644
--- a/arch/riscv/kernel/kernel_mode_vector.c
+++ b/arch/riscv/kernel/kernel_mode_vector.c
@@ -10,6 +10,7 @@
 #include <linux/percpu.h>
 #include <linux/preempt.h>
 #include <linux/types.h>
+#include <linux/slab.h>
 
 #include <asm/vector.h>
 #include <asm/switch_to.h>
@@ -35,7 +36,8 @@ static __must_check inline bool may_use_vector(void)
 	 * where it is set.
 	 */
 	return !in_irq() && !irqs_disabled() && !in_nmi() &&
-	       !this_cpu_read(vector_context_busy);
+	       !this_cpu_read(vector_context_busy) &&
+	       !test_thread_flag(TIF_RISCV_V_KMV);
 }
 
 /*
@@ -69,6 +71,47 @@ static void put_cpu_vector_context(void)
 	preempt_enable();
 }
 
+#ifdef CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV
+void riscv_v_vstate_ctrl_config_kmv(bool preemptive_kmv)
+{
+	if (preemptive_kmv)
+		current->thread.vstate_ctrl |= RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE;
+	else
+		current->thread.vstate_ctrl &= ~RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE;
+}
+
+static bool riscv_v_kmv_preempitble(void)
+{
+	return !!(current->thread.vstate_ctrl & RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE);
+}
+
+static int riscv_v_start_kernel_context(void)
+{
+	struct __riscv_v_ext_state *vstate;
+
+	vstate = &current->thread.kernel_vstate;
+	if (!vstate->datap) {
+		vstate->datap = kmalloc(riscv_v_vsize, GFP_KERNEL);
+		if (!vstate->datap)
+			return -ENOMEM;
+	}
+
+	current->thread.trap_pt_regs = NULL;
+	WARN_ON(test_and_set_thread_flag(TIF_RISCV_V_KMV));
+	return 0;
+}
+
+static void riscv_v_stop_kernel_context(void)
+{
+	WARN_ON(!test_and_clear_thread_flag(TIF_RISCV_V_KMV));
+	current->thread.trap_pt_regs = NULL;
+}
+#else
+#define riscv_v_kmv_preempitble()	(false)
+#define riscv_v_start_kernel_context()	(0)
+#define riscv_v_stop_kernel_context()	do {} while (0)
+#endif /* CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV */
+
 /*
  * kernel_rvv_begin(): obtain the CPU vector registers for use by the calling
  * context
@@ -94,7 +137,12 @@ int kernel_rvv_begin(void)
 	riscv_v_vstate_save(&current->thread.vstate, task_pt_regs(current));
 
 	/* Acquire kernel mode vector */
-	get_cpu_vector_context();
+	if (!preemptible() || !riscv_v_kmv_preempitble()) {
+		get_cpu_vector_context();
+	} else {
+		if (riscv_v_start_kernel_context())
+			get_cpu_vector_context();
+	}
 
 	/* Enable vector */
 	riscv_v_enable();
@@ -124,6 +172,9 @@ void kernel_rvv_end(void)
 	riscv_v_disable();
 
 	/* release kernel mode vector */
-	put_cpu_vector_context();
+	if (!test_thread_flag(TIF_RISCV_V_KMV))
+		put_cpu_vector_context();
+	else
+		riscv_v_stop_kernel_context();
 }
 EXPORT_SYMBOL_GPL(kernel_rvv_end);
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index ec89e7edb6fd..4db8cbc8abe9 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -160,8 +160,11 @@ void flush_thread(void)
 void arch_release_task_struct(struct task_struct *tsk)
 {
 	/* Free the vector context of datap. */
-	if (has_vector())
+	if (has_vector()) {
 		kfree(tsk->thread.vstate.datap);
+		if (IS_ENABLED(CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV))
+			kfree(tsk->thread.kernel_vstate.datap);
+	}
 }
 
 int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
@@ -170,7 +173,9 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
 	*dst = *src;
 	/* clear entire V context, including datap for a new task */
 	memset(&dst->thread.vstate, 0, sizeof(struct __riscv_v_ext_state));
+	memset(&dst->thread.kernel_vstate, 0, sizeof(struct __riscv_v_ext_state));
 	clear_tsk_thread_flag(dst, TIF_RISCV_V_DEFER_RESTORE);
+	clear_tsk_thread_flag(dst, TIF_RISCV_V_KMV);
 
 	return 0;
 }
@@ -205,6 +210,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 		childregs->a0 = 0; /* Return value of fork() */
 		p->thread.s[0] = 0;
 	}
+	riscv_v_vstate_ctrl_config_kmv(true);
 	p->thread.ra = (unsigned long)ret_from_fork;
 	p->thread.sp = (unsigned long)childregs; /* kernel sp */
 	return 0;
diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
index 9d583b760db4..42f227077ee5 100644
--- a/arch/riscv/kernel/vector.c
+++ b/arch/riscv/kernel/vector.c
@@ -122,7 +122,8 @@ static inline void riscv_v_ctrl_set(struct task_struct *tsk, int cur, int nxt,
 	ctrl |= VSTATE_CTRL_MAKE_NEXT(nxt);
 	if (inherit)
 		ctrl |= PR_RISCV_V_VSTATE_CTRL_INHERIT;
-	tsk->thread.vstate_ctrl = ctrl;
+	tsk->thread.vstate_ctrl &= ~PR_RISCV_V_VSTATE_CTRL_MASK;
+	tsk->thread.vstate_ctrl |= ctrl;
 }
 
 bool riscv_v_vstate_ctrl_user_allowed(void)
-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[v1, 6/6] riscv: vector: enable preemptive kernel-mode Vector to be built

[Andy Chiu]

Add a Kconfig to let user decides whether kernel-mode Vector in a
preemptive kernel should also run with preemption. If the config is 'N',
then all kernel-mode Vector code are run with preemption disabled.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
 arch/riscv/Kconfig | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index a9e8b697fefb..da6a45ea42ec 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -500,6 +500,16 @@ config RISCV_ISA_V_DEFAULT_ENABLE
 
 	  If you don't know what to do here, say Y.
 
+config RISCV_ISA_V_PREEMPTIVE_KMV
+	bool "Run kernel-mode Vector with kernel preemption"
+	depends on PREEMPTION
+	depends on RISCV_ISA_V
+	default y
+	help
+	  Ordinarily the kernel disables preemption before running in-kernel
+	  Vector code. This config frees the kernel from disabling preemption
+	  by adding meory on demand for tracking kernel's V-context.
+
 config TOOLCHAIN_HAS_ZBB
 	bool
 	default y
-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [v1, 0/6] riscv: support kernel-mode Vector

[Heiko Stuebner]

Am Samstag, 15. Juli 2023, 17:00:26 CEST schrieb Andy Chiu:
> This series provides support for running Vector code in kernel mode. The
> implementation is based on the v12 series of the Vector series, but with
> some additions. First, we introduce a mechanism to defer restoring
> Vector context for userspace programs (patch 1). This is similar to
> arm64 and x86's approaches when dealing with extra userspace register
> context. And it is benefitial to both Vector in user and kernel-mode.
> Then, patch 2, 3 add the kernel-mode Vector patch from v12 with minor
> modifications. At the end of the series, patch 4, 5, 6 add supports for
> making kernel-mode Vector code preemptible. We do this by adding
> kernel-mode Vector context, and keeping track of the frame where V
> context is last valid. We believe that enabling preemption of running V
> is a critical path for getting V more generally available in the
> kernel-mode. Besides, with status.VS, we can easily tell if
> saving/restoring V is required. This reduce the level of cost when
> running SIMD in kernel mode as compared to other arches. Other arches
> usually do not have a way to tell if extra context is dirty. Thus, if
> they also want to support running preemptible code with extra registers,
> then they must save/restore extra context at each context switch even if
> registers are not dirty.
> 
> The series is tested by loading a kernel module on a preemptive kernel.
> The module launches multiple kworkers which run Vector operations and
> verifies with scalar code. Also, the module provides userspace intefaces
> via fops to verify if we can run Vector code on syscall path.
> 
> Changes from the vector v12 series (for patch 2, 3):
>  - return a failure code when kernel_rvv_begin() fails.
>  - Do not immediately restore user's V context.

This works nicely with my vector crypto patchset rebased on
top of it:

Tested-by: Heiko Stuebner <heiko@sntech.de>



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [v1, 1/6] riscv: sched: defer restoring Vector context for user

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1757 bytes --]

Hey Andy,
Small bit of minor nitpickery..

On Sat, Jul 15, 2023 at 03:00:27PM +0000, Andy Chiu wrote:
> User's will use its Vector registers only after the kernel really

Looks like the ' here can be removed.

> returns to the userspace. So we can delay restoring Vector registers as
> long as we are still running in kernel mode. So, add a thread flag to
> indicates the need of restoring Vector and do the restore at the last
> arch-specific exit-to-user hook. This save the context restoring cost
> when we switch over multiple processes that run V in kernel mode. For
> example, if the kernel performs context swicth from A->B->C, and returns

"a context switch"

> to C's userspace, then there is no need for restoring B's V-register.

"to restore"

> 
> Besides, this also prevents us from repeatedly restoring V context when
> executing kernel-mode Vector multiple times for the upcoming kenel-mode
> Vector patches.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---

> diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> index 97e6f65ec176..d83975efe866 100644
> --- a/arch/riscv/include/asm/thread_info.h
> +++ b/arch/riscv/include/asm/thread_info.h
> @@ -101,12 +101,14 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
>  #define TIF_NOTIFY_SIGNAL	9	/* signal notifications exist */
>  #define TIF_UPROBE		10	/* uprobe breakpoint or singlestep */
>  #define TIF_32BIT		11	/* compat-mode 32bit process */
> +#define TIF_RISCV_V_DEFER_RESTORE	12

The rest of these have a comment, should the new addition?

Anyway, no meaningful comments from me here Andy,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [v1, 2/6] riscv: Add support for kernel mode vector

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 4090 bytes --]

On Sat, Jul 15, 2023 at 03:00:28PM +0000, Andy Chiu wrote:
> From: Greentime Hu <greentime.hu@sifive.com>
> 
> Add kernel_rvv_begin() and kernel_rvv_end() function declarations
> and corresponding definitions in kernel_mode_vector.c
> 
> These are needed to wrap uses of vector in kernel mode.
> 
> Co-developed-by: Vincent Chen <vincent.chen@sifive.com>
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
>  arch/riscv/include/asm/vector.h        |   2 +
>  arch/riscv/kernel/Makefile             |   1 +
>  arch/riscv/kernel/kernel_mode_vector.c | 129 +++++++++++++++++++++++++
>  3 files changed, 132 insertions(+)
>  create mode 100644 arch/riscv/kernel/kernel_mode_vector.c
> 
> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> index a4f3705fd144..9831b19153ae 100644
> --- a/arch/riscv/include/asm/vector.h
> +++ b/arch/riscv/include/asm/vector.h
> @@ -22,6 +22,8 @@
>  extern unsigned long riscv_v_vsize;
>  int riscv_v_setup_vsize(void);
>  bool riscv_v_first_use_handler(struct pt_regs *regs);
> +int kernel_rvv_begin(void);
> +void kernel_rvv_end(void);

So, we ditched all of the "rvv" stuff in the last series, using either
"vector" - has_vector() - or "riscv_v". I'd rather not introduce a third
naming scheme for vector related things...

Given what you add below is full of other things that use "vector", how
does s/rvv/vector/ sound here?


> diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
> new file mode 100644
> index 000000000000..c0c152c501a5
> --- /dev/null
> +++ b/arch/riscv/kernel/kernel_mode_vector.c

> +/*
> + * kernel_rvv_begin(): obtain the CPU vector registers for use by the calling
> + * context
> + *
> + * Must not be called unless may_use_vector() returns true.
> + * Task context in the vector registers is saved back to memory as necessary.
> + *
> + * A matching call to kernel_rvv_end() must be made before returning from the
> + * calling context.
> + *
> + * The caller may freely use the vector registers until kernel_rvv_end() is
> + * called.
> + */
> +int kernel_rvv_begin(void)

How come this returns an int, but you never actually check the result? The
other kernel_*_begin()s don't seem to return anything other than void.

> +{
> +	if (!has_vector())
> +		return -EOPNOTSUPP;
> +
> +	if (!may_use_vector())
> +		return -EPERM;

I notice arm64 takes a stronger approach. There, if the has() call
fails, it has a WARN_ON(). For the !may_use() case, it BUG()s.
Since users are forced to check may_use_vector() before calling
kernel_rvv_begin().

Is there a reason that we should not take the same approach?

> +
> +	/* Save vector state, if any */
> +	riscv_v_vstate_save(current, task_pt_regs(current));
> +
> +	/* Acquire kernel mode vector */
> +	get_cpu_vector_context();
> +
> +	/* Enable vector */

These three comments are mostly a statement of the obvious, no?

> +	riscv_v_enable();
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(kernel_rvv_begin);
> +
> +/*
> + * kernel_rvv_end(): give the CPU vector registers back to the current task
> + *
> + * Must be called from a context in which kernel_rvv_begin() was previously
> + * called, with no call to kernel_rvv_end() in the meantime.
> + *
> + * The caller must not use the vector registers after this function is called,
> + * unless kernel_rvv_begin() is called again in the meantime.
> + */
> +void kernel_rvv_end(void)
> +{
> +	if (WARN_ON(!has_vector()))

But there is a WARN_ON() here...

> +		return;
> +
> +	/* Restore vector state, if any */
> +	riscv_v_vstate_set_restore(current, task_pt_regs(current));
> +
> +	/* disable vector */
> +	riscv_v_disable();
> +
> +	/* release kernel mode vector */

Again, comments kinda state the obvious, no?

Otherwise, this stuff looks generally fine to me & similar to what is
being done elsewhere.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [v1, 3/6] riscv: Add vector extension XOR implementation

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2092 bytes --]

On Sat, Jul 15, 2023 at 03:00:29PM +0000, Andy Chiu wrote:
> From: Greentime Hu <greentime.hu@sifive.com>
> 
> This patch adds support for vector optimized XOR and it is tested in
> qemu.

Since this patch was originally written, has it been tested in hardware?

> Co-developed-by: Han-Kuan Chen <hankuan.chen@sifive.com>
> Signed-off-by: Han-Kuan Chen <hankuan.chen@sifive.com>
> Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
>  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..81b8837fa161
> --- /dev/null
> +++ b/arch/riscv/include/asm/xor.h

> +static void xor_rvv_2(unsigned long bytes, unsigned long *__restrict p1,
> +		      const unsigned long *__restrict p2)

> +static void xor_rvv_3(unsigned long bytes, unsigned long *__restrict p1,
> +		      const unsigned long *__restrict p2,
> +		      const unsigned long *__restrict p3)

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

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

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

Same naming scheme comments as the main vector patchset and 2/6 apply
here too.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [v1, 4/6] riscv: vector: do not pass task_struct into riscv_v_vstate_{save,restore}()

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 7571 bytes --]

On Sat, Jul 15, 2023 at 03:00:30PM +0000, Andy Chiu wrote:
> riscv_v_vstate_{save,restore}() can operate only on the knowlege of
> struct __riscv_v_ext_state, and struct pt_regs. Let the caller decides
> which should be passed into the function. Meanwhile, the kernel-mode
> Vector is going to introduce another vstate, so this also makes functions
> potentially able to be reused.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>

Breaks the build chief:
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'

rv64 allmodconfig w/ gcc.

Thanks,
Conor.

> ---
>  arch/riscv/include/asm/entry-common.h  |  2 +-
>  arch/riscv/include/asm/vector.h        | 14 +++++---------
>  arch/riscv/kernel/kernel_mode_vector.c |  2 +-
>  arch/riscv/kernel/ptrace.c             |  2 +-
>  arch/riscv/kernel/signal.c             |  2 +-
>  5 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/entry-common.h b/arch/riscv/include/asm/entry-common.h
> index 52926f4d8d7c..aa1b9e50d6c8 100644
> --- a/arch/riscv/include/asm/entry-common.h
> +++ b/arch/riscv/include/asm/entry-common.h
> @@ -12,7 +12,7 @@ static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
>  {
>  	if (ti_work & _TIF_RISCV_V_DEFER_RESTORE) {
>  		clear_thread_flag(TIF_RISCV_V_DEFER_RESTORE);
> -		riscv_v_vstate_restore(current, regs);
> +		riscv_v_vstate_restore(&current->thread.vstate, regs);
>  	}
>  }
>  
> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> index 9831b19153ae..50c556afd95a 100644
> --- a/arch/riscv/include/asm/vector.h
> +++ b/arch/riscv/include/asm/vector.h
> @@ -163,23 +163,19 @@ static inline void riscv_v_vstate_discard(struct pt_regs *regs)
>  	__riscv_v_vstate_dirty(regs);
>  }
>  
> -static inline void riscv_v_vstate_save(struct task_struct *task,
> +static inline void riscv_v_vstate_save(struct __riscv_v_ext_state *vstate,
>  				       struct pt_regs *regs)
>  {
>  	if ((regs->status & SR_VS) == SR_VS_DIRTY) {
> -		struct __riscv_v_ext_state *vstate = &task->thread.vstate;
> -
>  		__riscv_v_vstate_save(vstate, vstate->datap);
>  		__riscv_v_vstate_clean(regs);
>  	}
>  }
>  
> -static inline void riscv_v_vstate_restore(struct task_struct *task,
> +static inline void riscv_v_vstate_restore(struct __riscv_v_ext_state *vstate,
>  					  struct pt_regs *regs)
>  {
>  	if ((regs->status & SR_VS) != SR_VS_OFF) {
> -		struct __riscv_v_ext_state *vstate = &task->thread.vstate;
> -
>  		__riscv_v_vstate_restore(vstate, vstate->datap);
>  		__riscv_v_vstate_clean(regs);
>  	}
> @@ -200,7 +196,7 @@ static inline void __switch_to_vector(struct task_struct *prev,
>  	struct pt_regs *regs;
>  
>  	regs = task_pt_regs(prev);
> -	riscv_v_vstate_save(prev, regs);
> +	riscv_v_vstate_save(prev->thread.vstate, regs);
>  	riscv_v_vstate_set_restore(next, task_pt_regs(next));
>  }
>  
> @@ -218,8 +214,8 @@ static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; }
>  static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; }
>  #define riscv_v_vsize (0)
>  #define riscv_v_vstate_discard(regs)		do {} while (0)
> -#define riscv_v_vstate_save(task, regs)		do {} while (0)
> -#define riscv_v_vstate_restore(task, regs)	do {} while (0)
> +#define riscv_v_vstate_save(vstate, regs)	do {} while (0)
> +#define riscv_v_vstate_restore(vstate, regs)	do {} while (0)
>  #define __switch_to_vector(__prev, __next)	do {} while (0)
>  #define riscv_v_vstate_off(regs)		do {} while (0)
>  #define riscv_v_vstate_on(regs)			do {} while (0)
> diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
> index c0c152c501a5..30f1b861cac0 100644
> --- a/arch/riscv/kernel/kernel_mode_vector.c
> +++ b/arch/riscv/kernel/kernel_mode_vector.c
> @@ -91,7 +91,7 @@ int kernel_rvv_begin(void)
>  		return -EPERM;
>  
>  	/* Save vector state, if any */
> -	riscv_v_vstate_save(current, task_pt_regs(current));
> +	riscv_v_vstate_save(&current->thread.vstate, task_pt_regs(current));
>  
>  	/* Acquire kernel mode vector */
>  	get_cpu_vector_context();
> diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
> index 1d572cf3140f..85e7167245cc 100644
> --- a/arch/riscv/kernel/ptrace.c
> +++ b/arch/riscv/kernel/ptrace.c
> @@ -99,7 +99,7 @@ static int riscv_vr_get(struct task_struct *target,
>  	 * copying them to membuf.
>  	 */
>  	if (target == current)
> -		riscv_v_vstate_save(current, task_pt_regs(current));
> +		riscv_v_vstate_save(&current->thread.vstate, task_pt_regs(current));
>  
>  	/* Copy vector header from vstate. */
>  	membuf_write(&to, vstate, offsetof(struct __riscv_v_ext_state, datap));
> diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
> index 0fca2c128b5f..75fd8cc05e10 100644
> --- a/arch/riscv/kernel/signal.c
> +++ b/arch/riscv/kernel/signal.c
> @@ -86,7 +86,7 @@ static long save_v_state(struct pt_regs *regs, void __user **sc_vec)
>  	/* datap is designed to be 16 byte aligned for better performance */
>  	WARN_ON(unlikely(!IS_ALIGNED((unsigned long)datap, 16)));
>  
> -	riscv_v_vstate_save(current, regs);
> +	riscv_v_vstate_save(&current->thread.vstate, regs);
>  	/* Copy everything of vstate but datap. */
>  	err = __copy_to_user(&state->v_state, &current->thread.vstate,
>  			     offsetof(struct __riscv_v_ext_state, datap));
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [v1, 5/6] riscv: vector: allow kernel-mode Vector with preemption

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 8277 bytes --]

On Sat, Jul 15, 2023 at 03:00:31PM +0000, Andy Chiu wrote:
> Add kernel_vstate to keep track of kernel-mode Vector registers when
> trap introduced context switch happens. Also, provide trap_pt_regs to
> let context save/restore routine reference status.VS at which the trap
> takes place. The thread flag TIF_RISCV_V_KMV indicates whether a task is
> running in kernel-mode Vector with preemption 'ON'. So context switch
> routines know and would save V-regs to kernel_vstate and restore V-regs
> immediately from kernel_vstate if the bit is set.
> 
> Apart from a task's preemption status, the capability of
> running preemptive kernel-mode Vector is jointly controlled by the
> RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE mask in the task's
> thread.vstate_ctrl. This bit is masked whenever a trap takes place in
> kernel mode while executing preemptive Vector code.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
>  arch/riscv/include/asm/processor.h     |  2 +
>  arch/riscv/include/asm/thread_info.h   |  4 ++
>  arch/riscv/include/asm/vector.h        | 27 ++++++++++--
>  arch/riscv/kernel/asm-offsets.c        |  2 +
>  arch/riscv/kernel/entry.S              | 41 ++++++++++++++++++
>  arch/riscv/kernel/kernel_mode_vector.c | 57 ++++++++++++++++++++++++--
>  arch/riscv/kernel/process.c            |  8 +++-
>  arch/riscv/kernel/vector.c             |  3 +-
>  8 files changed, 136 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> index e82af1097e26..d337b750f2ec 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -42,6 +42,8 @@ struct thread_struct {
>  	unsigned long bad_cause;
>  	unsigned long vstate_ctrl;
>  	struct __riscv_v_ext_state vstate;
> +	struct pt_regs *trap_pt_regs;
> +	struct __riscv_v_ext_state kernel_vstate;
>  };
>  
>  /* Whitelist the fstate from the task_struct for hardened usercopy */
> diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> index d83975efe866..59d88adfc4de 100644
> --- a/arch/riscv/include/asm/thread_info.h
> +++ b/arch/riscv/include/asm/thread_info.h
> @@ -102,6 +102,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
>  #define TIF_UPROBE		10	/* uprobe breakpoint or singlestep */
>  #define TIF_32BIT		11	/* compat-mode 32bit process */
>  #define TIF_RISCV_V_DEFER_RESTORE	12
> +#define TIF_RISCV_V_KMV			13

Same comment about comments.

Also, the "V" here is a dupe, since you have RISCV_V in the name.
Ditto everywhere else, afaict. Perhaps you could do s/KMV/KERNEL_MODE/?

>  #define _TIF_NOTIFY_RESUME	(1 << TIF_NOTIFY_RESUME)
>  #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
> @@ -109,9 +110,12 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
>  #define _TIF_NOTIFY_SIGNAL	(1 << TIF_NOTIFY_SIGNAL)
>  #define _TIF_UPROBE		(1 << TIF_UPROBE)
>  #define _TIF_RISCV_V_DEFER_RESTORE	(1 << TIF_RISCV_V_DEFER_RESTORE)
> +#define _TIF_RISCV_V_KMV		(1 << TIF_RISCV_V_KMV_TASK)

Where is KMV_TASK defined?

>  
>  #define _TIF_WORK_MASK \
>  	(_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | _TIF_NEED_RESCHED | \
>  	 _TIF_NOTIFY_SIGNAL | _TIF_UPROBE)
>  
> +#define RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE	0x20
> +
>  #endif /* _ASM_RISCV_THREAD_INFO_H */
> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> index 50c556afd95a..d004c9fa6a57 100644
> --- a/arch/riscv/include/asm/vector.h
> +++ b/arch/riscv/include/asm/vector.h
> @@ -25,6 +25,12 @@ bool riscv_v_first_use_handler(struct pt_regs *regs);
>  int kernel_rvv_begin(void);
>  void kernel_rvv_end(void);
>  
> +#ifdef CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV
> +void riscv_v_vstate_ctrl_config_kmv(bool preemptive_kmv);
> +#else
> +#define riscv_v_vstate_ctrl_config_kmv(preemptive_kmv)	do {} while (0)
> +#endif

For clang/llvm allmodconfig:
../arch/riscv/kernel/process.c:213:2: error: call to undeclared function 'riscv_v_vstate_ctrl_config_kmv'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]

Probably also happens when vector is disabled?


> +
>  static __always_inline bool has_vector(void)
>  {
>  	return riscv_has_extension_unlikely(RISCV_ISA_EXT_v);
> @@ -195,9 +201,24 @@ static inline void __switch_to_vector(struct task_struct *prev,
>  {
>  	struct pt_regs *regs;
>  
> -	regs = task_pt_regs(prev);
> -	riscv_v_vstate_save(prev->thread.vstate, regs);
> -	riscv_v_vstate_set_restore(next, task_pt_regs(next));
> +	if (IS_ENABLED(CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV) &&

w.r.t. this symbol, just drop the KMV?

> +	    test_tsk_thread_flag(prev, TIF_RISCV_V_KMV)) {
> +		regs = prev->thread.trap_pt_regs;
> +		WARN_ON(!regs);
> +		riscv_v_vstate_save(&prev->thread.kernel_vstate, regs);
> +	} else {
> +		regs = task_pt_regs(prev);
> +		riscv_v_vstate_save(&prev->thread.vstate, regs);
> +	}
> +
> +	if (IS_ENABLED(CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV) &&

Possibly stupid question, but not explained by the patch, why would we
ever want to have RISCV_ISA_V_PREEMPTIVE_KMV disabled?

> +	    test_tsk_thread_flag(next, TIF_RISCV_V_KMV)) {
> +		regs = next->thread.trap_pt_regs;
> +		WARN_ON(!regs);
> +		riscv_v_vstate_restore(&next->thread.kernel_vstate, regs);
> +	} else {
> +		riscv_v_vstate_set_restore(next, task_pt_regs(next));
> +	}
>  }
>  
>  void riscv_v_vstate_ctrl_init(struct task_struct *tsk);
> diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
> index d6a75aac1d27..4b062f7741b2 100644
> --- a/arch/riscv/kernel/asm-offsets.c
> +++ b/arch/riscv/kernel/asm-offsets.c
> @@ -38,6 +38,8 @@ void asm_offsets(void)
>  	OFFSET(TASK_TI_PREEMPT_COUNT, task_struct, thread_info.preempt_count);
>  	OFFSET(TASK_TI_KERNEL_SP, task_struct, thread_info.kernel_sp);
>  	OFFSET(TASK_TI_USER_SP, task_struct, thread_info.user_sp);
> +	OFFSET(TASK_THREAD_TRAP_REGP, task_struct, thread.trap_pt_regs);
> +	OFFSET(TASK_THREAD_VSTATE_CTRL, task_struct, thread.vstate_ctrl);
>  
>  	OFFSET(TASK_THREAD_F0,  task_struct, thread.fstate.f[0]);
>  	OFFSET(TASK_THREAD_F1,  task_struct, thread.fstate.f[1]);
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index 143a2bb3e697..42b80b90626a 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -66,6 +66,27 @@ _save_context:
>  	REG_S s4, PT_CAUSE(sp)
>  	REG_S s5, PT_TP(sp)
>  
> +	/*
> +	 * Reocrd the register set at the frame where in-kernel V registers are

nit: s/Reocrd/Record/

> diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
> index 30f1b861cac0..bcd6a69a5266 100644
> --- a/arch/riscv/kernel/kernel_mode_vector.c
> +++ b/arch/riscv/kernel/kernel_mode_vector.c
> @@ -10,6 +10,7 @@
>  #include <linux/percpu.h>
>  #include <linux/preempt.h>
>  #include <linux/types.h>
> +#include <linux/slab.h>
>  
>  #include <asm/vector.h>
>  #include <asm/switch_to.h>
> @@ -35,7 +36,8 @@ static __must_check inline bool may_use_vector(void)
>  	 * where it is set.
>  	 */
>  	return !in_irq() && !irqs_disabled() && !in_nmi() &&
> -	       !this_cpu_read(vector_context_busy);
> +	       !this_cpu_read(vector_context_busy) &&
> +	       !test_thread_flag(TIF_RISCV_V_KMV);
>  }
>  
>  /*
> @@ -69,6 +71,47 @@ static void put_cpu_vector_context(void)
>  	preempt_enable();
>  }
>  
> +#ifdef CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV
> +void riscv_v_vstate_ctrl_config_kmv(bool preemptive_kmv)

I don't understand what this function is trying to do, based on the
function name. The lack of a verb in it is somewhat confusing.

> +{
> +	if (preemptive_kmv)
> +		current->thread.vstate_ctrl |= RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE;
> +	else
> +		current->thread.vstate_ctrl &= ~RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE;
> +}
> +
> +static bool riscv_v_kmv_preempitble(void)

Beyond the ible/able stuff, there's a typo in this function name.

> +{
> +	return !!(current->thread.vstate_ctrl & RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE);
> +}

Little comment on the rest, not qualified to do so :)

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [v1, 6/6] riscv: vector: enable preemptive kernel-mode Vector to be built

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1197 bytes --]

Hey Andy,

On Sat, Jul 15, 2023 at 03:00:32PM +0000, Andy Chiu wrote:
> Add a Kconfig to let user decides whether kernel-mode Vector in a
> preemptive kernel should also run with preemption. If the config is 'N',
> then all kernel-mode Vector code are run with preemption disabled.

nit: "is run".

> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
>  arch/riscv/Kconfig | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index a9e8b697fefb..da6a45ea42ec 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -500,6 +500,16 @@ config RISCV_ISA_V_DEFAULT_ENABLE
>  
>  	  If you don't know what to do here, say Y.
>  
> +config RISCV_ISA_V_PREEMPTIVE_KMV
> +	bool "Run kernel-mode Vector with kernel preemption"
> +	depends on PREEMPTION
> +	depends on RISCV_ISA_V
> +	default y
> +	help
> +	  Ordinarily the kernel disables preemption before running in-kernel
> +	  Vector code. This config frees the kernel from disabling preemption
> +	  by adding meory on demand for tracking kernel's V-context.

s/meory/memory/

This should be part of the previous patch.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [v1, 1/6] riscv: sched: defer restoring Vector context for user

[Andy Chiu]

On Mon, Jul 17, 2023 at 5:47 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> Hey Andy,
> Small bit of minor nitpickery..
>
> On Sat, Jul 15, 2023 at 03:00:27PM +0000, Andy Chiu wrote:
> > User's will use its Vector registers only after the kernel really
>
> Looks like the ' here can be removed.
>
> > returns to the userspace. So we can delay restoring Vector registers as
> > long as we are still running in kernel mode. So, add a thread flag to
> > indicates the need of restoring Vector and do the restore at the last
> > arch-specific exit-to-user hook. This save the context restoring cost
> > when we switch over multiple processes that run V in kernel mode. For
> > example, if the kernel performs context swicth from A->B->C, and returns
>
> "a context switch"
>
> > to C's userspace, then there is no need for restoring B's V-register.
>
> "to restore"

Sorry for the poor english. Let me fix it in the next spin.

>
> >
> > Besides, this also prevents us from repeatedly restoring V context when
> > executing kernel-mode Vector multiple times for the upcoming kenel-mode
> > Vector patches.
> >
> > Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> > ---
>
> > diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> > index 97e6f65ec176..d83975efe866 100644
> > --- a/arch/riscv/include/asm/thread_info.h
> > +++ b/arch/riscv/include/asm/thread_info.h
> > @@ -101,12 +101,14 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
> >  #define TIF_NOTIFY_SIGNAL    9       /* signal notifications exist */
> >  #define TIF_UPROBE           10      /* uprobe breakpoint or singlestep */
> >  #define TIF_32BIT            11      /* compat-mode 32bit process */
> > +#define TIF_RISCV_V_DEFER_RESTORE    12
>
> The rest of these have a comment, should the new addition?

Yes, it should. How about this "defer restoring process's V-context"

>
> Anyway, no meaningful comments from me here Andy,
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
>
> Thanks,
> Conor.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [v1, 2/6] riscv: Add support for kernel mode vector

[Andy Chiu]

On Mon, Jul 17, 2023 at 6:23 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Sat, Jul 15, 2023 at 03:00:28PM +0000, Andy Chiu wrote:
> > From: Greentime Hu <greentime.hu@sifive.com>
> >
> > Add kernel_rvv_begin() and kernel_rvv_end() function declarations
> > and corresponding definitions in kernel_mode_vector.c
> >
> > These are needed to wrap uses of vector in kernel mode.
> >
> > Co-developed-by: Vincent Chen <vincent.chen@sifive.com>
> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> > Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
> > Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> > ---
> >  arch/riscv/include/asm/vector.h        |   2 +
> >  arch/riscv/kernel/Makefile             |   1 +
> >  arch/riscv/kernel/kernel_mode_vector.c | 129 +++++++++++++++++++++++++
> >  3 files changed, 132 insertions(+)
> >  create mode 100644 arch/riscv/kernel/kernel_mode_vector.c
> >
> > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> > index a4f3705fd144..9831b19153ae 100644
> > --- a/arch/riscv/include/asm/vector.h
> > +++ b/arch/riscv/include/asm/vector.h
> > @@ -22,6 +22,8 @@
> >  extern unsigned long riscv_v_vsize;
> >  int riscv_v_setup_vsize(void);
> >  bool riscv_v_first_use_handler(struct pt_regs *regs);
> > +int kernel_rvv_begin(void);
> > +void kernel_rvv_end(void);
>
> So, we ditched all of the "rvv" stuff in the last series, using either
> "vector" - has_vector() - or "riscv_v". I'd rather not introduce a third
> naming scheme for vector related things...
>
> Given what you add below is full of other things that use "vector", how
> does s/rvv/vector/ sound here?

Yes, I agree. Let's use 'vector' instead of rvv.

>
>
> > diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
> > new file mode 100644
> > index 000000000000..c0c152c501a5
> > --- /dev/null
> > +++ b/arch/riscv/kernel/kernel_mode_vector.c
>
> > +/*
> > + * kernel_rvv_begin(): obtain the CPU vector registers for use by the calling
> > + * context
> > + *
> > + * Must not be called unless may_use_vector() returns true.
> > + * Task context in the vector registers is saved back to memory as necessary.
> > + *
> > + * A matching call to kernel_rvv_end() must be made before returning from the
> > + * calling context.
> > + *
> > + * The caller may freely use the vector registers until kernel_rvv_end() is
> > + * called.
> > + */
> > +int kernel_rvv_begin(void)
>
> How come this returns an int, but you never actually check the result? The
> other kernel_*_begin()s don't seem to return anything other than void.
>
> > +{
> > +     if (!has_vector())
> > +             return -EOPNOTSUPP;
> > +
> > +     if (!may_use_vector())
> > +             return -EPERM;
>
> I notice arm64 takes a stronger approach. There, if the has() call
> fails, it has a WARN_ON(). For the !may_use() case, it BUG()s.
> Since users are forced to check may_use_vector() before calling
> kernel_rvv_begin().
>
> Is there a reason that we should not take the same approach?

Yes, I agree that it is better to return nothing after some thinking.
Originally I was hoping to return a failure code if the preemptible
kernel-mode Vector failed to allocate memory in
riscv_v_start_kernel_context(). However, returning things here just
complicates the programming model for kernel-mode Vector. So, let's
just make it transparent to users of kernel_vector_begin() and return
nothing here.

>
> > +
> > +     /* Save vector state, if any */
> > +     riscv_v_vstate_save(current, task_pt_regs(current));
> > +
> > +     /* Acquire kernel mode vector */
> > +     get_cpu_vector_context();
> > +
> > +     /* Enable vector */
>
> These three comments are mostly a statement of the obvious, no?

Agree, I will drop those comments.

>
> > +     riscv_v_enable();
> > +
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(kernel_rvv_begin);
> > +
> > +/*
> > + * kernel_rvv_end(): give the CPU vector registers back to the current task
> > + *
> > + * Must be called from a context in which kernel_rvv_begin() was previously
> > + * called, with no call to kernel_rvv_end() in the meantime.
> > + *
> > + * The caller must not use the vector registers after this function is called,
> > + * unless kernel_rvv_begin() is called again in the meantime.
> > + */
> > +void kernel_rvv_end(void)
> > +{
> > +     if (WARN_ON(!has_vector()))
>
> But there is a WARN_ON() here...
>
> > +             return;
> > +
> > +     /* Restore vector state, if any */
> > +     riscv_v_vstate_set_restore(current, task_pt_regs(current));
> > +
> > +     /* disable vector */
> > +     riscv_v_disable();
> > +
> > +     /* release kernel mode vector */
>
> Again, comments kinda state the obvious, no?
>
> Otherwise, this stuff looks generally fine to me & similar to what is
> being done elsewhere.
>
> Thanks,
> Conor.

Thanks,
Andy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [v1, 3/6] riscv: Add vector extension XOR implementation

[Andy Chiu]

On Mon, Jul 17, 2023 at 6:26 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Sat, Jul 15, 2023 at 03:00:29PM +0000, Andy Chiu wrote:
> > From: Greentime Hu <greentime.hu@sifive.com>
> >
> > This patch adds support for vector optimized XOR and it is tested in
> > qemu.
>
> Since this patch was originally written, has it been tested in hardware?

We've run it on internal FPGAs but FPGAs don't count, right?  ;)

>
> > Co-developed-by: Han-Kuan Chen <hankuan.chen@sifive.com>
> > Signed-off-by: Han-Kuan Chen <hankuan.chen@sifive.com>
> > Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
> > Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> > ---
> >  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..81b8837fa161
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/xor.h
>
> > +static void xor_rvv_2(unsigned long bytes, unsigned long *__restrict p1,
> > +                   const unsigned long *__restrict p2)
>
> > +static void xor_rvv_3(unsigned long bytes, unsigned long *__restrict p1,
> > +                   const unsigned long *__restrict p2,
> > +                   const unsigned long *__restrict p3)
>
> > +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)
>
> > +
> > +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)
>
> > +
> > +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
> > +};
>
> Same naming scheme comments as the main vector patchset and 2/6 apply
> here too.

Yep, I'm doing s/xor_rvv/xor_vector

Thanks,
Andy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [v1, 4/6] riscv: vector: do not pass task_struct into riscv_v_vstate_{save,restore}()

[Andy Chiu]

On Mon, Jul 17, 2023 at 6:33 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Sat, Jul 15, 2023 at 03:00:30PM +0000, Andy Chiu wrote:
> > riscv_v_vstate_{save,restore}() can operate only on the knowlege of
> > struct __riscv_v_ext_state, and struct pt_regs. Let the caller decides
> > which should be passed into the function. Meanwhile, the kernel-mode
> > Vector is going to introduce another vstate, so this also makes functions
> > potentially able to be reused.
> >
> > Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
>
> Breaks the build chief:
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
> ../arch/riscv/include/asm/vector.h:199:41: error: incompatible type for argument 1 of 'riscv_v_vstate_save'
>
> rv64 allmodconfig w/ gcc.

Thanks for catching this. This bug was buried at the next patch and I
was not careful enough to carry the fix back to this patch.

>
> Thanks,
> Conor.
>
> > ---
> >  arch/riscv/include/asm/entry-common.h  |  2 +-
> >  arch/riscv/include/asm/vector.h        | 14 +++++---------
> >  arch/riscv/kernel/kernel_mode_vector.c |  2 +-
> >  arch/riscv/kernel/ptrace.c             |  2 +-
> >  arch/riscv/kernel/signal.c             |  2 +-
> >  5 files changed, 9 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/entry-common.h b/arch/riscv/include/asm/entry-common.h
> > index 52926f4d8d7c..aa1b9e50d6c8 100644
> > --- a/arch/riscv/include/asm/entry-common.h
> > +++ b/arch/riscv/include/asm/entry-common.h
> > @@ -12,7 +12,7 @@ static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
> >  {
> >       if (ti_work & _TIF_RISCV_V_DEFER_RESTORE) {
> >               clear_thread_flag(TIF_RISCV_V_DEFER_RESTORE);
> > -             riscv_v_vstate_restore(current, regs);
> > +             riscv_v_vstate_restore(&current->thread.vstate, regs);
> >       }
> >  }
> >
> > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> > index 9831b19153ae..50c556afd95a 100644
> > --- a/arch/riscv/include/asm/vector.h
> > +++ b/arch/riscv/include/asm/vector.h
> > @@ -163,23 +163,19 @@ static inline void riscv_v_vstate_discard(struct pt_regs *regs)
> >       __riscv_v_vstate_dirty(regs);
> >  }
> >
> > -static inline void riscv_v_vstate_save(struct task_struct *task,
> > +static inline void riscv_v_vstate_save(struct __riscv_v_ext_state *vstate,
> >                                      struct pt_regs *regs)
> >  {
> >       if ((regs->status & SR_VS) == SR_VS_DIRTY) {
> > -             struct __riscv_v_ext_state *vstate = &task->thread.vstate;
> > -
> >               __riscv_v_vstate_save(vstate, vstate->datap);
> >               __riscv_v_vstate_clean(regs);
> >       }
> >  }
> >
> > -static inline void riscv_v_vstate_restore(struct task_struct *task,
> > +static inline void riscv_v_vstate_restore(struct __riscv_v_ext_state *vstate,
> >                                         struct pt_regs *regs)
> >  {
> >       if ((regs->status & SR_VS) != SR_VS_OFF) {
> > -             struct __riscv_v_ext_state *vstate = &task->thread.vstate;
> > -
> >               __riscv_v_vstate_restore(vstate, vstate->datap);
> >               __riscv_v_vstate_clean(regs);
> >       }
> > @@ -200,7 +196,7 @@ static inline void __switch_to_vector(struct task_struct *prev,
> >       struct pt_regs *regs;
> >
> >       regs = task_pt_regs(prev);
> > -     riscv_v_vstate_save(prev, regs);
> > +     riscv_v_vstate_save(prev->thread.vstate, regs);
> >       riscv_v_vstate_set_restore(next, task_pt_regs(next));
> >  }
> >
> > @@ -218,8 +214,8 @@ static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; }
> >  static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; }
> >  #define riscv_v_vsize (0)
> >  #define riscv_v_vstate_discard(regs)         do {} while (0)
> > -#define riscv_v_vstate_save(task, regs)              do {} while (0)
> > -#define riscv_v_vstate_restore(task, regs)   do {} while (0)
> > +#define riscv_v_vstate_save(vstate, regs)    do {} while (0)
> > +#define riscv_v_vstate_restore(vstate, regs) do {} while (0)
> >  #define __switch_to_vector(__prev, __next)   do {} while (0)
> >  #define riscv_v_vstate_off(regs)             do {} while (0)
> >  #define riscv_v_vstate_on(regs)                      do {} while (0)
> > diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
> > index c0c152c501a5..30f1b861cac0 100644
> > --- a/arch/riscv/kernel/kernel_mode_vector.c
> > +++ b/arch/riscv/kernel/kernel_mode_vector.c
> > @@ -91,7 +91,7 @@ int kernel_rvv_begin(void)
> >               return -EPERM;
> >
> >       /* Save vector state, if any */
> > -     riscv_v_vstate_save(current, task_pt_regs(current));
> > +     riscv_v_vstate_save(&current->thread.vstate, task_pt_regs(current));
> >
> >       /* Acquire kernel mode vector */
> >       get_cpu_vector_context();
> > diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
> > index 1d572cf3140f..85e7167245cc 100644
> > --- a/arch/riscv/kernel/ptrace.c
> > +++ b/arch/riscv/kernel/ptrace.c
> > @@ -99,7 +99,7 @@ static int riscv_vr_get(struct task_struct *target,
> >        * copying them to membuf.
> >        */
> >       if (target == current)
> > -             riscv_v_vstate_save(current, task_pt_regs(current));
> > +             riscv_v_vstate_save(&current->thread.vstate, task_pt_regs(current));
> >
> >       /* Copy vector header from vstate. */
> >       membuf_write(&to, vstate, offsetof(struct __riscv_v_ext_state, datap));
> > diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
> > index 0fca2c128b5f..75fd8cc05e10 100644
> > --- a/arch/riscv/kernel/signal.c
> > +++ b/arch/riscv/kernel/signal.c
> > @@ -86,7 +86,7 @@ static long save_v_state(struct pt_regs *regs, void __user **sc_vec)
> >       /* datap is designed to be 16 byte aligned for better performance */
> >       WARN_ON(unlikely(!IS_ALIGNED((unsigned long)datap, 16)));
> >
> > -     riscv_v_vstate_save(current, regs);
> > +     riscv_v_vstate_save(&current->thread.vstate, regs);
> >       /* Copy everything of vstate but datap. */
> >       err = __copy_to_user(&state->v_state, &current->thread.vstate,
> >                            offsetof(struct __riscv_v_ext_state, datap));
> > --
> > 2.17.1
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Thanks,
Andy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [v1, 5/6] riscv: vector: allow kernel-mode Vector with preemption

[Andy Chiu]

On Mon, Jul 17, 2023 at 7:06 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Sat, Jul 15, 2023 at 03:00:31PM +0000, Andy Chiu wrote:
> > Add kernel_vstate to keep track of kernel-mode Vector registers when
> > trap introduced context switch happens. Also, provide trap_pt_regs to
> > let context save/restore routine reference status.VS at which the trap
> > takes place. The thread flag TIF_RISCV_V_KMV indicates whether a task is
> > running in kernel-mode Vector with preemption 'ON'. So context switch
> > routines know and would save V-regs to kernel_vstate and restore V-regs
> > immediately from kernel_vstate if the bit is set.
> >
> > Apart from a task's preemption status, the capability of
> > running preemptive kernel-mode Vector is jointly controlled by the
> > RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE mask in the task's
> > thread.vstate_ctrl. This bit is masked whenever a trap takes place in
> > kernel mode while executing preemptive Vector code.
> >
> > Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> > ---
> >  arch/riscv/include/asm/processor.h     |  2 +
> >  arch/riscv/include/asm/thread_info.h   |  4 ++
> >  arch/riscv/include/asm/vector.h        | 27 ++++++++++--
> >  arch/riscv/kernel/asm-offsets.c        |  2 +
> >  arch/riscv/kernel/entry.S              | 41 ++++++++++++++++++
> >  arch/riscv/kernel/kernel_mode_vector.c | 57 ++++++++++++++++++++++++--
> >  arch/riscv/kernel/process.c            |  8 +++-
> >  arch/riscv/kernel/vector.c             |  3 +-
> >  8 files changed, 136 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> > index e82af1097e26..d337b750f2ec 100644
> > --- a/arch/riscv/include/asm/processor.h
> > +++ b/arch/riscv/include/asm/processor.h
> > @@ -42,6 +42,8 @@ struct thread_struct {
> >       unsigned long bad_cause;
> >       unsigned long vstate_ctrl;
> >       struct __riscv_v_ext_state vstate;
> > +     struct pt_regs *trap_pt_regs;
> > +     struct __riscv_v_ext_state kernel_vstate;
> >  };
> >
> >  /* Whitelist the fstate from the task_struct for hardened usercopy */
> > diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> > index d83975efe866..59d88adfc4de 100644
> > --- a/arch/riscv/include/asm/thread_info.h
> > +++ b/arch/riscv/include/asm/thread_info.h
> > @@ -102,6 +102,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
> >  #define TIF_UPROBE           10      /* uprobe breakpoint or singlestep */
> >  #define TIF_32BIT            11      /* compat-mode 32bit process */
> >  #define TIF_RISCV_V_DEFER_RESTORE    12
> > +#define TIF_RISCV_V_KMV                      13
>
> Same comment about comments.

Adding /* kernel-mode Vector run with preemption-on */

>
> Also, the "V" here is a dupe, since you have RISCV_V in the name.
> Ditto everywhere else, afaict. Perhaps you could do s/KMV/KERNEL_MODE/?

Good idea.

>
> >  #define _TIF_NOTIFY_RESUME   (1 << TIF_NOTIFY_RESUME)
> >  #define _TIF_SIGPENDING              (1 << TIF_SIGPENDING)
> > @@ -109,9 +110,12 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
> >  #define _TIF_NOTIFY_SIGNAL   (1 << TIF_NOTIFY_SIGNAL)
> >  #define _TIF_UPROBE          (1 << TIF_UPROBE)
> >  #define _TIF_RISCV_V_DEFER_RESTORE   (1 << TIF_RISCV_V_DEFER_RESTORE)
> > +#define _TIF_RISCV_V_KMV             (1 << TIF_RISCV_V_KMV_TASK)
>
> Where is KMV_TASK defined?

My bad, it should be TIF_RISCV_V_KMV. Also, I'm changing it to
TIF_RISCV_V_KERNEL_MODE now.

>
> >
> >  #define _TIF_WORK_MASK \
> >       (_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | _TIF_NEED_RESCHED | \
> >        _TIF_NOTIFY_SIGNAL | _TIF_UPROBE)
> >
> > +#define RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE  0x20
> > +
> >  #endif /* _ASM_RISCV_THREAD_INFO_H */
> > diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> > index 50c556afd95a..d004c9fa6a57 100644
> > --- a/arch/riscv/include/asm/vector.h
> > +++ b/arch/riscv/include/asm/vector.h
> > @@ -25,6 +25,12 @@ bool riscv_v_first_use_handler(struct pt_regs *regs);
> >  int kernel_rvv_begin(void);
> >  void kernel_rvv_end(void);
> >
> > +#ifdef CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV
> > +void riscv_v_vstate_ctrl_config_kmv(bool preemptive_kmv);
> > +#else
> > +#define riscv_v_vstate_ctrl_config_kmv(preemptive_kmv)       do {} while (0)
> > +#endif
>
> For clang/llvm allmodconfig:
> ../arch/riscv/kernel/process.c:213:2: error: call to undeclared function 'riscv_v_vstate_ctrl_config_kmv'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>
> Probably also happens when vector is disabled?

Yes, I'm going to move the entire block out of CONFIG_RISCV_ISA_V to
resolve that.

>
>
> > +
> >  static __always_inline bool has_vector(void)
> >  {
> >       return riscv_has_extension_unlikely(RISCV_ISA_EXT_v);
> > @@ -195,9 +201,24 @@ static inline void __switch_to_vector(struct task_struct *prev,
> >  {
> >       struct pt_regs *regs;
> >
> > -     regs = task_pt_regs(prev);
> > -     riscv_v_vstate_save(prev->thread.vstate, regs);
> > -     riscv_v_vstate_set_restore(next, task_pt_regs(next));
> > +     if (IS_ENABLED(CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV) &&
>
> w.r.t. this symbol, just drop the KMV?
>
> > +         test_tsk_thread_flag(prev, TIF_RISCV_V_KMV)) {
> > +             regs = prev->thread.trap_pt_regs;
> > +             WARN_ON(!regs);
> > +             riscv_v_vstate_save(&prev->thread.kernel_vstate, regs);
> > +     } else {
> > +             regs = task_pt_regs(prev);
> > +             riscv_v_vstate_save(&prev->thread.vstate, regs);
> > +     }
> > +
> > +     if (IS_ENABLED(CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV) &&
>
> Possibly stupid question, but not explained by the patch, why would we
> ever want to have RISCV_ISA_V_PREEMPTIVE_KMV disabled?

Sorry, it's not obvious here. Below is the commit message that I will
add for describing usecase of RISCV_ISA_V_PREEMPTIVE_KMV (now
RISCV_ISA_V_PREEMPTIVE):

provide a config CONFIG_RISCV_ISA_V_PREEMPTIVE to give users an option
to disable preemptible kernel-mode Vector at build time. Users with
constraint memory may want to disable this config as preemptible
kernel-mode Vector needs extra space for tracking per thread's
kernel-mode V context. Or, users might as well want to disable it if
all
kernel-mode Vector code is time sensitive and cannot tolerate context
switch overhead.


>
> > +         test_tsk_thread_flag(next, TIF_RISCV_V_KMV)) {
> > +             regs = next->thread.trap_pt_regs;
> > +             WARN_ON(!regs);
> > +             riscv_v_vstate_restore(&next->thread.kernel_vstate, regs);
> > +     } else {
> > +             riscv_v_vstate_set_restore(next, task_pt_regs(next));
> > +     }
> >  }
> >
> >  void riscv_v_vstate_ctrl_init(struct task_struct *tsk);
> > diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
> > index d6a75aac1d27..4b062f7741b2 100644
> > --- a/arch/riscv/kernel/asm-offsets.c
> > +++ b/arch/riscv/kernel/asm-offsets.c
> > @@ -38,6 +38,8 @@ void asm_offsets(void)
> >       OFFSET(TASK_TI_PREEMPT_COUNT, task_struct, thread_info.preempt_count);
> >       OFFSET(TASK_TI_KERNEL_SP, task_struct, thread_info.kernel_sp);
> >       OFFSET(TASK_TI_USER_SP, task_struct, thread_info.user_sp);
> > +     OFFSET(TASK_THREAD_TRAP_REGP, task_struct, thread.trap_pt_regs);
> > +     OFFSET(TASK_THREAD_VSTATE_CTRL, task_struct, thread.vstate_ctrl);
> >
> >       OFFSET(TASK_THREAD_F0,  task_struct, thread.fstate.f[0]);
> >       OFFSET(TASK_THREAD_F1,  task_struct, thread.fstate.f[1]);
> > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> > index 143a2bb3e697..42b80b90626a 100644
> > --- a/arch/riscv/kernel/entry.S
> > +++ b/arch/riscv/kernel/entry.S
> > @@ -66,6 +66,27 @@ _save_context:
> >       REG_S s4, PT_CAUSE(sp)
> >       REG_S s5, PT_TP(sp)
> >
> > +     /*
> > +      * Reocrd the register set at the frame where in-kernel V registers are
>
> nit: s/Reocrd/Record/

Oops.

>
> > diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
> > index 30f1b861cac0..bcd6a69a5266 100644
> > --- a/arch/riscv/kernel/kernel_mode_vector.c
> > +++ b/arch/riscv/kernel/kernel_mode_vector.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/percpu.h>
> >  #include <linux/preempt.h>
> >  #include <linux/types.h>
> > +#include <linux/slab.h>
> >
> >  #include <asm/vector.h>
> >  #include <asm/switch_to.h>
> > @@ -35,7 +36,8 @@ static __must_check inline bool may_use_vector(void)
> >        * where it is set.
> >        */
> >       return !in_irq() && !irqs_disabled() && !in_nmi() &&
> > -            !this_cpu_read(vector_context_busy);
> > +            !this_cpu_read(vector_context_busy) &&
> > +            !test_thread_flag(TIF_RISCV_V_KMV);
> >  }
> >
> >  /*
> > @@ -69,6 +71,47 @@ static void put_cpu_vector_context(void)
> >       preempt_enable();
> >  }
> >
> > +#ifdef CONFIG_RISCV_ISA_V_PREEMPTIVE_KMV
> > +void riscv_v_vstate_ctrl_config_kmv(bool preemptive_kmv)
>
> I don't understand what this function is trying to do, based on the
> function name. The lack of a verb in it is somewhat confusing.

The purpose of this function is to allow/disallow kernel-mode Vector
to be executed with kernel preemption. I am going to change the
function name to kernel_vector_allow_preemption() since there is only
one user of this function and the only purpose is to initialize it to
be "allowed" when the config is y.

>
> > +{
> > +     if (preemptive_kmv)
> > +             current->thread.vstate_ctrl |= RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE;
> > +     else
> > +             current->thread.vstate_ctrl &= ~RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE;
> > +}
> > +
> > +static bool riscv_v_kmv_preempitble(void)
>
> Beyond the ible/able stuff, there's a typo in this function name.

I am going to change the function name to kernel_vector_preemptible to
match the naming scheme above.

>
> > +{
> > +     return !!(current->thread.vstate_ctrl & RISCV_V_VSTATE_CTRL_KMV_PREEMPTIBLE);
> > +}
>
> Little comment on the rest, not qualified to do so :)
>
> Thanks,
> Conor.

Thanks,
Andy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv