[PATCH] RISC-V: Clobber V registers on syscalls

[Andy Chiu]

On Thu, Jun 22, 2023 at 5:40 AM Björn Töpel <bjorn at kernel.org> wrote:
>
> Andy Chiu <andy.chiu at sifive.com> writes:
>
> >> +static inline void riscv_v_vstate_discard(struct pt_regs *regs)
> >> +{
> >> +       unsigned long vl;
> >> +
> >> +       if (!riscv_v_vstate_query(regs))
> >> +               return;
> >> +
> >> +       riscv_v_vstate_on(regs);
> >
> > Do we need this riscv_v_vstate_on()?  If it is not on we'd return
> > early in the previous "if" statement, right?
>
> riscv_v_vstate_on() just set the state to Initial, right? Or do you mean
> that riscv_v_vstate_query() is too much, and we should only check if the
> state is dirty?
>
> >> +
> >> +       riscv_v_enable();
> >> +       asm volatile (
> >> +               ".option push\n\t"
> >> +               ".option arch, +v\n\t"
> >> +               "vsetvli        %0, x0, e8, m8, ta, ma\n\t"
> >> +               "vmv.v.i        v0, 0\n\t"
> >> +               "vmv.v.i        v8, 0\n\t"
> >> +               "vmv.v.i        v16, 0\n\t"
> >> +               "vmv.v.i        v24, 0\n\t"
> >> +               ".option pop\n\t"
> >> +               : "=&r" (vl) : : "memory");
> >> +       riscv_v_disable();
> >
> > Maybe consider cleaning the vstate (status.vs) here. As such we don't
> > have to save V during context switch.
>
> It's late, and I'm slower than usual. The regs are cleared, and the
> state is Initial. No save on context switch, but restore, right?

Yes, it's my bad, you are right. I sometime messed around the "real"
status.VS with the one in the userspace context :P

>
> > Or, maybe we could set vstate as off during syscall and discard V-reg
> > + restore status.VS when returning back to userspace?
>
> Hmm, interesting. We need to track the status.VS to restore somewhere...

Maybe something like this?

diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index 04c0b07bf6cd..79de9ca83391 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -43,6 +43,11 @@ static inline void riscv_v_vstate_on(struct pt_regs *regs)
 	regs->status = (regs->status & ~SR_VS) | SR_VS_INITIAL;
 }
 
+static inline void riscv_v_vstate_dirty(struct pt_regs *regs)
+{
+	regs->status = (regs->status & ~SR_VS) | SR_VS_DIRTY;
+}
+
 static inline bool riscv_v_vstate_query(struct pt_regs *regs)
 {
 	return (regs->status & SR_VS) != 0;
@@ -163,6 +168,24 @@ static inline void __switch_to_vector(struct task_struct *prev,
 void riscv_v_vstate_ctrl_init(struct task_struct *tsk);
 bool riscv_v_vstate_ctrl_user_allowed(void);
 
+static inline void riscv_v_vstate_discard(struct pt_regs *regs)
+{
+	unsigned long vl;
+
+	riscv_v_enable();
+	asm volatile (
+			".option push\n\t"
+			".option arch, +v\n\t"
+			"vsetvli        %0, x0, e8, m8, ta, ma\n\t"
+			"vmv.v.i        v0, 0\n\t"
+			"vmv.v.i        v8, 0\n\t"
+			"vmv.v.i        v16, 0\n\t"
+			"vmv.v.i        v24, 0\n\t"
+			".option pop\n\t"
+			: "=&r" (vl) : : "memory");
+	riscv_v_disable();
+}
+
 #else /* ! CONFIG_RISCV_ISA_V  */
 
 struct pt_regs;
@@ -178,6 +201,8 @@ static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; }
 #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)
+#define riscv_v_vstate_dirty(regs)		do {} while (0)
+#define riscv_v_vstate_discard(regs)		do {} while (0)
 
 #endif /* CONFIG_RISCV_ISA_V */
 
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 24d309c6ab8d..e36b69c9b07f 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -291,10 +291,14 @@ asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs)
 {
 	if (user_mode(regs)) {
 		ulong syscall = regs->a7;
+		bool v_is_on;
 
 		regs->epc += 4;
 		regs->orig_a0 = regs->a0;
 
+		v_is_on = riscv_v_vstate_query(regs);
+		riscv_v_vstate_off(regs);
+
 		syscall = syscall_enter_from_user_mode(regs, syscall);
 
 		if (syscall < NR_syscalls)
@@ -303,6 +307,10 @@ asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs)
 			regs->a0 = -ENOSYS;
 
 		syscall_exit_to_user_mode(regs);
+		if (v_is_on) {
+			riscv_v_vstate_discard(regs);
+			riscv_v_vstate_dirty(regs);
+		}
 	} else {
 		irqentry_state_t state = irqentry_nmi_enter(regs);
 
>
>
> Björn

Thanks,
Andy