From: Björn Töpel <[email protected]>
The RISC-V vector specification states:
Executing a system call causes all caller-saved vector registers
(v0-v31, vl, vtype) and vstart to become unspecified.
The vector registers are set to all 1s, vill is set (invalid), and the
vector status is set to Dirty.
That way we can prevent userspace from accidentally relying on the
stated save.
Rémi pointed out [1] that writing to the registers might be
superfluous, and setting vill is sufficient.
Link: https://lore.kernel.org/linux-riscv/[email protected]/ # [1]
Suggested-by: Darius Rad <[email protected]>
Suggested-by: Palmer Dabbelt <[email protected]>
Suggested-by: Rémi Denis-Courmont <[email protected]>
Signed-off-by: Björn Töpel <[email protected]>
---
v2->v3:
Set state to Dirty after discard, for proper ptrace() handling
(Andy)
v1->v2:
Proper register restore for initial state (Andy)
Set registers to 1s, and not 0s (Darius)
---
arch/riscv/include/asm/vector.h | 33 +++++++++++++++++++++++++++++++++
arch/riscv/kernel/traps.c | 2 ++
2 files changed, 35 insertions(+)
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index 04c0b07bf6cd..0b23056503c5 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -33,6 +33,11 @@ static inline void __riscv_v_vstate_clean(struct pt_regs *regs)
regs->status = (regs->status & ~SR_VS) | SR_VS_CLEAN;
}
+static inline void __riscv_v_vstate_dirty(struct pt_regs *regs)
+{
+ regs->status = (regs->status & ~SR_VS) | SR_VS_DIRTY;
+}
+
static inline void riscv_v_vstate_off(struct pt_regs *regs)
{
regs->status = (regs->status & ~SR_VS) | SR_VS_OFF;
@@ -128,6 +133,34 @@ static inline void __riscv_v_vstate_restore(struct __riscv_v_ext_state *restore_
riscv_v_disable();
}
+static inline void __riscv_v_vstate_discard(void)
+{
+ unsigned long vl, vtype_inval = 1UL << (BITS_PER_LONG - 1);
+
+ 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, -1\n\t"
+ "vmv.v.i v8, -1\n\t"
+ "vmv.v.i v16, -1\n\t"
+ "vmv.v.i v24, -1\n\t"
+ "vsetvl %0, x0, %1\n\t"
+ ".option pop\n\t"
+ : "=&r" (vl) : "r" (vtype_inval) : "memory");
+ riscv_v_disable();
+}
+
+static inline void riscv_v_vstate_discard(struct pt_regs *regs)
+{
+ if ((regs->status & SR_VS) == SR_VS_OFF)
+ return;
+
+ __riscv_v_vstate_discard();
+ __riscv_v_vstate_dirty(regs);
+}
+
static inline void riscv_v_vstate_save(struct task_struct *task,
struct pt_regs *regs)
{
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 5158961ea977..5ff63a784a6d 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -296,6 +296,8 @@ asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs)
regs->epc += 4;
regs->orig_a0 = regs->a0;
+ riscv_v_vstate_discard(regs);
+
syscall = syscall_enter_from_user_mode(regs, syscall);
if (syscall < NR_syscalls)
base-commit: 488833ccdcac118da16701f4ee0673b20ba47fe3
--
2.39.2
Hey,
On Thu, Jun 29, 2023 at 08:27:30AM +0200, Bj?rn T?pel wrote:
> From: Bj?rn T?pel <[email protected]>
>
> The RISC-V vector specification states:
> Executing a system call causes all caller-saved vector registers
> (v0-v31, vl, vtype) and vstart to become unspecified.
>
> The vector registers are set to all 1s, vill is set (invalid), and the
> vector status is set to Dirty.
>
> That way we can prevent userspace from accidentally relying on the
> stated save.
>
> R?mi pointed out [1] that writing to the registers might be
> superfluous, and setting vill is sufficient.
>
> Link: https://lore.kernel.org/linux-riscv/[email protected]/ # [1]
> Suggested-by: Darius Rad <[email protected]>
> Suggested-by: Palmer Dabbelt <[email protected]>
> Suggested-by: R?mi Denis-Courmont <[email protected]>
> Signed-off-by: Bj?rn T?pel <[email protected]>
clang allmodconfig and rv32_defconfig fail to build with this patch,
according to patchwork:
../arch/riscv/kernel/traps.c:299:3: error: call to undeclared function 'riscv_v_vstate_discard'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
Cheers,
Conor.
> ---
>
> v2->v3:
> Set state to Dirty after discard, for proper ptrace() handling
> (Andy)
>
> v1->v2:
> Proper register restore for initial state (Andy)
> Set registers to 1s, and not 0s (Darius)
>
> ---
> arch/riscv/include/asm/vector.h | 33 +++++++++++++++++++++++++++++++++
> arch/riscv/kernel/traps.c | 2 ++
> 2 files changed, 35 insertions(+)
>
> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> index 04c0b07bf6cd..0b23056503c5 100644
> --- a/arch/riscv/include/asm/vector.h
> +++ b/arch/riscv/include/asm/vector.h
> @@ -33,6 +33,11 @@ static inline void __riscv_v_vstate_clean(struct pt_regs *regs)
> regs->status = (regs->status & ~SR_VS) | SR_VS_CLEAN;
> }
>
> +static inline void __riscv_v_vstate_dirty(struct pt_regs *regs)
> +{
> + regs->status = (regs->status & ~SR_VS) | SR_VS_DIRTY;
> +}
> +
> static inline void riscv_v_vstate_off(struct pt_regs *regs)
> {
> regs->status = (regs->status & ~SR_VS) | SR_VS_OFF;
> @@ -128,6 +133,34 @@ static inline void __riscv_v_vstate_restore(struct __riscv_v_ext_state *restore_
> riscv_v_disable();
> }
>
> +static inline void __riscv_v_vstate_discard(void)
> +{
> + unsigned long vl, vtype_inval = 1UL << (BITS_PER_LONG - 1);
> +
> + 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, -1\n\t"
> + "vmv.v.i v8, -1\n\t"
> + "vmv.v.i v16, -1\n\t"
> + "vmv.v.i v24, -1\n\t"
> + "vsetvl %0, x0, %1\n\t"
> + ".option pop\n\t"
> + : "=&r" (vl) : "r" (vtype_inval) : "memory");
> + riscv_v_disable();
> +}
> +
> +static inline void riscv_v_vstate_discard(struct pt_regs *regs)
> +{
> + if ((regs->status & SR_VS) == SR_VS_OFF)
> + return;
> +
> + __riscv_v_vstate_discard();
> + __riscv_v_vstate_dirty(regs);
> +}
> +
> static inline void riscv_v_vstate_save(struct task_struct *task,
> struct pt_regs *regs)
> {
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index 5158961ea977..5ff63a784a6d 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -296,6 +296,8 @@ asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs)
> regs->epc += 4;
> regs->orig_a0 = regs->a0;
>
> + riscv_v_vstate_discard(regs);
> +
> syscall = syscall_enter_from_user_mode(regs, syscall);
>
> if (syscall < NR_syscalls)
>
> base-commit: 488833ccdcac118da16701f4ee0673b20ba47fe3
> --
> 2.39.2
>
Hi Bj?rn,
kernel test robot noticed the following build errors:
[auto build test ERROR on 488833ccdcac118da16701f4ee0673b20ba47fe3]
url: https://github.com/intel-lab-lkp/linux/commits/Bj-rn-T-pel/riscv-Discard-vector-state-on-syscalls/20230629-142852
base: 488833ccdcac118da16701f4ee0673b20ba47fe3
patch link: https://lore.kernel.org/r/20230629062730.985184-1-bjorn%40kernel.org
patch subject: [PATCH v3] riscv: Discard vector state on syscalls
config: riscv-randconfig-r042-20230629 (https://download.01.org/0day-ci/archive/20230629/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20230629/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
| ~~~~~~~~~~ ^
In file included from arch/riscv/kernel/traps.c:15:
In file included from include/linux/kprobes.h:28:
In file included from include/linux/ftrace.h:10:
In file included from include/linux/trace_recursion.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:751:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
751 | insw(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw'
105 | #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from arch/riscv/kernel/traps.c:15:
In file included from include/linux/kprobes.h:28:
In file included from include/linux/ftrace.h:10:
In file included from include/linux/trace_recursion.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:759:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
759 | insl(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:106:53: note: expanded from macro 'insl'
106 | #define insl(addr, buffer, count) __insl(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from arch/riscv/kernel/traps.c:15:
In file included from include/linux/kprobes.h:28:
In file included from include/linux/ftrace.h:10:
In file included from include/linux/trace_recursion.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:768:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
768 | outsb(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:118:55: note: expanded from macro 'outsb'
118 | #define outsb(addr, buffer, count) __outsb(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from arch/riscv/kernel/traps.c:15:
In file included from include/linux/kprobes.h:28:
In file included from include/linux/ftrace.h:10:
In file included from include/linux/trace_recursion.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:777:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
777 | outsw(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:119:55: note: expanded from macro 'outsw'
119 | #define outsw(addr, buffer, count) __outsw(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from arch/riscv/kernel/traps.c:15:
In file included from include/linux/kprobes.h:28:
In file included from include/linux/ftrace.h:10:
In file included from include/linux/trace_recursion.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:786:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
786 | outsl(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:120:55: note: expanded from macro 'outsl'
120 | #define outsl(addr, buffer, count) __outsl(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from arch/riscv/kernel/traps.c:15:
In file included from include/linux/kprobes.h:28:
In file included from include/linux/ftrace.h:10:
In file included from include/linux/trace_recursion.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:1134:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
1134 | return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
| ~~~~~~~~~~ ^
>> arch/riscv/kernel/traps.c:299:3: error: call to undeclared function 'riscv_v_vstate_discard'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
299 | riscv_v_vstate_discard(regs);
| ^
arch/riscv/kernel/traps.c:299:3: note: did you mean 'riscv_v_vstate_query'?
arch/riscv/include/asm/vector.h:206:20: note: 'riscv_v_vstate_query' declared here
206 | static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; }
| ^
13 warnings and 1 error generated.
vim +/riscv_v_vstate_discard +299 arch/riscv/kernel/traps.c
290
291 asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs)
292 {
293 if (user_mode(regs)) {
294 ulong syscall = regs->a7;
295
296 regs->epc += 4;
297 regs->orig_a0 = regs->a0;
298
> 299 riscv_v_vstate_discard(regs);
300
301 syscall = syscall_enter_from_user_mode(regs, syscall);
302
303 if (syscall < NR_syscalls)
304 syscall_handler(regs, syscall);
305 else
306 regs->a0 = -ENOSYS;
307
308 syscall_exit_to_user_mode(regs);
309 } else {
310 irqentry_state_t state = irqentry_nmi_enter(regs);
311
312 do_trap_error(regs, SIGILL, ILL_ILLTRP, regs->epc,
313 "Oops - environment call from U-mode");
314
315 irqentry_nmi_exit(regs, state);
316 }
317
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Bj?rn,
kernel test robot noticed the following build errors:
[auto build test ERROR on 488833ccdcac118da16701f4ee0673b20ba47fe3]
url: https://github.com/intel-lab-lkp/linux/commits/Bj-rn-T-pel/riscv-Discard-vector-state-on-syscalls/20230629-142852
base: 488833ccdcac118da16701f4ee0673b20ba47fe3
patch link: https://lore.kernel.org/r/20230629062730.985184-1-bjorn%40kernel.org
patch subject: [PATCH v3] riscv: Discard vector state on syscalls
config: riscv-allnoconfig (https://download.01.org/0day-ci/archive/20230629/[email protected]/config)
compiler: riscv64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230629/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
arch/riscv/kernel/traps.c: In function 'do_trap_ecall_u':
>> arch/riscv/kernel/traps.c:299:17: error: implicit declaration of function 'riscv_v_vstate_discard'; did you mean 'riscv_v_vstate_restore'? [-Werror=implicit-function-declaration]
299 | riscv_v_vstate_discard(regs);
| ^~~~~~~~~~~~~~~~~~~~~~
| riscv_v_vstate_restore
cc1: some warnings being treated as errors
vim +299 arch/riscv/kernel/traps.c
290
291 asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs)
292 {
293 if (user_mode(regs)) {
294 ulong syscall = regs->a7;
295
296 regs->epc += 4;
297 regs->orig_a0 = regs->a0;
298
> 299 riscv_v_vstate_discard(regs);
300
301 syscall = syscall_enter_from_user_mode(regs, syscall);
302
303 if (syscall < NR_syscalls)
304 syscall_handler(regs, syscall);
305 else
306 regs->a0 = -ENOSYS;
307
308 syscall_exit_to_user_mode(regs);
309 } else {
310 irqentry_state_t state = irqentry_nmi_enter(regs);
311
312 do_trap_error(regs, SIGILL, ILL_ILLTRP, regs->epc,
313 "Oops - environment call from U-mode");
314
315 irqentry_nmi_exit(regs, state);
316 }
317
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Conor Dooley <[email protected]> writes:
> clang allmodconfig and rv32_defconfig fail to build with this patch,
> according to patchwork:
> ../arch/riscv/kernel/traps.c:299:3: error: call to undeclared function 'riscv_v_vstate_discard'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
Ugh. Sloppy. :-(
Thank you!
Björn