riscv: exception code mega simplification and optimization

Complete revamp of the exception entry code, including syscall handling.
Proper syscall frame exception trigger. Many correctness fixes, hacks
removal, etc. etc.

I tried to make this into several commits, but this stuff is all
inter-related and a pain to split.

The diffstat summary:

 14 files changed, 250 insertions(+), 802 deletions(-)

Binary size (before):

   text	   data	    bss	    dec	    hex	filename
   1104	      0	      0	   1104	    450	isr.S.obj
     64	      0	      0	     64	     40	userspace.S.obj

Binary size (after):

   text	   data	    bss	    dec	    hex	filename
    600	      0	      0	    600	    258	isr.S.obj
     36	      0	      0	     36	     24	userspace.S.obj

Run of samples/userspace/syscall_perf (before):

*** Booting Zephyr OS build zephyr-v3.0.0-325-g3748accae018  ***
Main Thread started; qemu_riscv32
Supervisor thread started
User thread started
Supervisor thread(0x80010048):       384 cycles	     509 instructions
User thread(0x80010140):           77312 cycles	   77437 instructions

Run of samples/userspace/syscall_perf (after):

*** Booting Zephyr OS build zephyr-v3.0.0-326-g4c877a2753b3  ***
Main Thread started; qemu_riscv32
Supervisor thread started
User thread started
Supervisor thread(0x80010048):       384 cycles	     509 instructions
User thread(0x80010138):            7040 cycles     7165 instructions

Yes, that's more than a 10x speed-up!

Signed-off-by: Nicolas Pitre <npitre@baylibre.com>

arch/riscv/core/fatal.c

@@ -39,7 +39,11 @@ FUNC_NORETURN void z_riscv_fatal_error(unsigned int reason,
 		LOG_ERR("     a5: " PR_REG "    t5: " PR_REG, esf->a5, esf->t5);
 		LOG_ERR("     a6: " PR_REG "    t6: " PR_REG, esf->a6, esf->t6);
 		LOG_ERR("     a7: " PR_REG, esf->a7);
+#ifdef CONFIG_USERSPACE
+		LOG_ERR("     sp: " PR_REG "    tp: " PR_REG, esf->sp, esf->tp);
+#else
 		LOG_ERR("         " NO_REG "    tp: " PR_REG, esf->tp);
+#endif
 		LOG_ERR("     ra: " PR_REG, esf->ra);
 		LOG_ERR("   mepc: " PR_REG, esf->mepc);
 		LOG_ERR("mstatus: " PR_REG, esf->mstatus);
@@ -119,15 +123,7 @@ void _Fault(z_arch_esf_t *esf)
 	LOG_ERR("  mtval: %lx", mtval);
 #endif
 
-	unsigned int reason = K_ERR_CPU_EXCEPTION;
+	z_riscv_fatal_error(K_ERR_CPU_EXCEPTION, esf);
-
-#if !defined(CONFIG_USERSPACE)
-	if (esf->t5 == ARCH_EXCEPT_MARKER) {
-		reason = esf->t6;
-	}
-#endif
-
-	z_riscv_fatal_error(reason, esf);
 }
 
 #ifdef CONFIG_USERSPACE

arch/riscv/core/irq_offload.c

@@ -6,29 +6,17 @@
 
 #include <irq.h>
 #include <irq_offload.h>
-#include <sys/printk.h>
+#include <arch/riscv/syscall.h>
 
-volatile irq_offload_routine_t _offload_routine;
+static irq_offload_routine_t offload_routine;
-static volatile const void *offload_param;
+static const void *offload_param;
 
 /*
  * Called by _enter_irq
- *
- * Just in case the offload routine itself generates an unhandled
- * exception, clear the offload_routine global before executing.
  */
 void z_irq_do_offload(void)
 {
-	irq_offload_routine_t tmp;
+	offload_routine(offload_param);
-
-	if (!_offload_routine) {
-		return;
-	}
-
-	tmp = _offload_routine;
-	_offload_routine = NULL;
-
-	tmp((const void *)offload_param);
 }
 
 void arch_irq_offload(irq_offload_routine_t routine, const void *parameter)
@@ -36,10 +24,8 @@ void arch_irq_offload(irq_offload_routine_t routine, const void *parameter)
 	unsigned int key;
 
 	key = irq_lock();
-	_offload_routine = routine;
+	offload_routine = routine;
 	offload_param = parameter;
-
+	arch_syscall_invoke0(RV_ECALL_IRQ_OFFLOAD);
-	__asm__ volatile ("ecall");
-
 	irq_unlock(key);
 }

arch/riscv/core/offsets/offsets.c

@@ -29,7 +29,6 @@
 GEN_OFFSET_SYM(_thread_arch_t, swap_return_value);
 #if defined(CONFIG_USERSPACE)
 GEN_OFFSET_SYM(_thread_arch_t, priv_stack_start);
-GEN_OFFSET_SYM(_thread_arch_t, user_sp);
 #endif
 
 /* struct coop member offsets */
@@ -85,8 +84,11 @@ GEN_OFFSET_SYM(z_arch_esf_t, a7);
 GEN_OFFSET_SYM(z_arch_esf_t, mepc);
 GEN_OFFSET_SYM(z_arch_esf_t, mstatus);
 
+#ifdef CONFIG_USERSPACE
+GEN_OFFSET_SYM(z_arch_esf_t, sp);
+#endif
+
 #if defined(CONFIG_FPU) && defined(CONFIG_FPU_SHARING)
-GEN_OFFSET_SYM(z_arch_esf_t, fp_state);
 GEN_OFFSET_SYM(z_arch_esf_t, ft0);
 GEN_OFFSET_SYM(z_arch_esf_t, ft1);
 GEN_OFFSET_SYM(z_arch_esf_t, ft2);

arch/riscv/core/reset.S

@@ -85,8 +85,6 @@ aa_loop:
 	li t0, CONFIG_ISR_STACK_SIZE
 	add sp, sp, t0
 
-	csrw mscratch, sp
-
 #ifdef CONFIG_WDOG_INIT
 	call _WdogInit
 #endif

arch/riscv/core/swap.S

@@ -8,11 +8,11 @@
 #include <linker/sections.h>
 #include <offsets_short.h>
 #include <arch/cpu.h>
+#include <arch/riscv/syscall.h>
 #include "asm_macros.inc"
 
 /* exports */
 GTEXT(arch_swap)
-GTEXT(z_thread_entry_wrapper)
 
 /* Use ABI name of registers for the sake of simplicity */
 
@@ -25,6 +25,7 @@ GTEXT(z_thread_entry_wrapper)
 SECTION_FUNC(exception.other, arch_swap)
 
 	/* Make a system call to perform context switch */
+	li a7, RV_ECALL_CONTEXT_SWITCH
 	ecall
 
 	/*
@@ -49,28 +50,10 @@ SECTION_FUNC(exception.other, arch_swap)
 	 * Use atomic instruction csrrs to do so.
 	 */
 	andi a0, a0, MSTATUS_IEN
-	csrrs t0, mstatus, a0
+	csrs mstatus, a0
 
 	/* Set value of return register a0 to value of register t2 */
 	mv a0, t2
 
 	/* Return */
 	ret
-
-
-/*
- * void z_thread_entry_wrapper(k_thread_entry_t, void *, void *, void *)
- */
-SECTION_FUNC(TEXT, z_thread_entry_wrapper)
-	/*
-	 * z_thread_entry_wrapper is called for every new thread upon the return
-	 * of arch_swap or ISR. Its address, as well as its input function
-	 * arguments thread_entry_t, void *, void *, void * are restored from
-	 * the thread stack (initialized via function _thread).
-	 * In this case, thread_entry_t, * void *, void * and void * are stored
-	 * in registers a0, a1, a2 and a3. These registers are used as arguments
-	 * to function z_thread_entry. Hence, just call z_thread_entry with
-	 * return address set to 0 to indicate a non-returning function call.
-	 */
-
-	j z_thread_entry

arch/riscv/core/thread.c

@@ -12,20 +12,13 @@
 #include <core_pmp.h>
 
 #ifdef CONFIG_USERSPACE
-
 /*
  * Glogal variable used to know the current mode running.
  * Is not boolean because it must match the PMP granularity of the arch.
  */
 uint32_t is_user_mode;
-bool irq_flag;
 #endif
 
-void z_thread_entry_wrapper(k_thread_entry_t thread,
-			    void *arg1,
-			    void *arg2,
-			    void *arg3);
-
 void arch_new_thread(struct k_thread *thread, k_thread_stack_t *stack,
 		     char *stack_ptr, k_thread_entry_t entry,
 		     void *p1, void *p2, void *p3)
@@ -64,13 +57,13 @@ void arch_new_thread(struct k_thread *thread, k_thread_stack_t *stack,
 	 * within the RISCV architecture implementation, initially set:
 	 * 1) MSTATUS to MSTATUS_DEF_RESTORE in the thread stack to enable
 	 *    interrupts when the newly created thread will be scheduled;
-	 * 2) MEPC to the address of the z_thread_entry_wrapper in the thread
+	 * 2) MEPC to the address of the z_thread_entry in the thread
 	 *    stack.
 	 * Hence, when going out of an interrupt/exception/context-switch,
 	 * after scheduling the newly created thread:
 	 * 1) interrupts will be enabled, as the MSTATUS register will be
 	 *    restored following the MSTATUS value set within the thread stack;
-	 * 2) the core will jump to z_thread_entry_wrapper, as the program
+	 * 2) the core will jump to z_thread_entry, as the program
 	 *    counter will be restored following the MEPC value set within the
 	 *    thread stack.
 	 */
@@ -81,7 +74,6 @@ void arch_new_thread(struct k_thread *thread, k_thread_stack_t *stack,
 	if ((thread->base.user_options & K_FP_REGS) != 0) {
 		stack_init->mstatus |= MSTATUS_FS_INIT;
 	}
-	stack_init->fp_state = 0;
 	thread->callee_saved.fcsr = 0;
 #elif defined(CONFIG_FPU)
 	/* Unshared FP mode: enable FPU of each thread. */
@@ -96,7 +88,9 @@ void arch_new_thread(struct k_thread *thread, k_thread_stack_t *stack,
 #if defined(CONFIG_USERSPACE)
 	/* Clear user thread context */
 	thread->arch.priv_stack_start = 0;
-	thread->arch.user_sp = 0;
+
+	/* the unwound stack pointer upon exiting exception */
+	stack_init->sp = (ulong_t)(stack_init + 1);
 #endif /* CONFIG_USERSPACE */
 
 	/* Assign thread entry point and mstatus.MPRV mode. */
@@ -107,7 +101,7 @@ void arch_new_thread(struct k_thread *thread, k_thread_stack_t *stack,
 
 	} else {
 		/* Supervisor thread */
-		stack_init->mepc = (ulong_t)z_thread_entry_wrapper;
+		stack_init->mepc = (ulong_t)z_thread_entry;
 
 #if defined(CONFIG_PMP_STACK_GUARD)
 		/* Enable PMP in mstatus.MPRV mode for RISC-V machine mode
@@ -197,20 +191,6 @@ int arch_float_enable(struct k_thread *thread, unsigned int options)
 
 #ifdef CONFIG_USERSPACE
 
-/* Function used by Zephyr to switch a supervisor thread to a user thread */
-FUNC_NORETURN void arch_user_mode_enter(k_thread_entry_t user_entry,
-	void *p1, void *p2, void *p3)
-{
-	arch_syscall_invoke5((uintptr_t) arch_user_mode_enter,
-		(uintptr_t) user_entry,
-		(uintptr_t) p1,
-		(uintptr_t) p2,
-		(uintptr_t) p3,
-		FORCE_SYSCALL_ID);
-
-	CODE_UNREACHABLE;
-}
-
 /*
  * User space entry function
  *
@@ -218,34 +198,43 @@ FUNC_NORETURN void arch_user_mode_enter(k_thread_entry_t user_entry,
  * The conversion is one way, and threads which transition to user mode do
  * not transition back later, unless they are doing system calls.
  */
-FUNC_NORETURN void z_riscv_user_mode_enter_syscall(k_thread_entry_t user_entry,
+FUNC_NORETURN void arch_user_mode_enter(k_thread_entry_t user_entry,
-	void *p1, void *p2, void *p3)
+					void *p1, void *p2, void *p3)
 {
-	ulong_t top_of_user_stack = 0U;
+	ulong_t top_of_user_stack, top_of_priv_stack;
-	uintptr_t status;
+	ulong_t status;
 
 	/* Set up privileged stack */
 #ifdef CONFIG_GEN_PRIV_STACKS
-		_current->arch.priv_stack_start =
+	_current->arch.priv_stack_start =
 			(ulong_t)z_priv_stack_find(_current->stack_obj);
 #else
-		_current->arch.priv_stack_start =
+	_current->arch.priv_stack_start =
 			(ulong_t)(_current->stack_obj) +
 			Z_RISCV_STACK_GUARD_SIZE;
 #endif /* CONFIG_GEN_PRIV_STACKS */
+	top_of_priv_stack = Z_STACK_PTR_ALIGN(_current->arch.priv_stack_start
+					      + CONFIG_PRIVILEGED_STACK_SIZE);
 
 	top_of_user_stack = Z_STACK_PTR_ALIGN(
 				_current->stack_info.start +
 				_current->stack_info.size -
 				_current->stack_info.delta);
 
-	/* Set next CPU status to user mode */
 	status = csr_read(mstatus);
+
+	/* Set next CPU status to user mode */
 	status = INSERT_FIELD(status, MSTATUS_MPP, PRV_U);
-	status = INSERT_FIELD(status, MSTATUS_MPRV, 0);
+	/* Enable IRQs for user mode */
+	status = INSERT_FIELD(status, MSTATUS_MPIE, 1);
+	/* Disable IRQs for m-mode until the mode switch */
+	status = INSERT_FIELD(status, MSTATUS_MIE, 0);
 
 	csr_write(mstatus, status);
-	csr_write(mepc, z_thread_entry_wrapper);
+	csr_write(mepc, z_thread_entry);
+
+	/* exception stack has to be in mscratch */
+	csr_write(mscratch, top_of_priv_stack);
 
 	/* Set up Physical Memory Protection */
 #if defined(CONFIG_PMP_STACK_GUARD)
@@ -257,32 +246,16 @@ FUNC_NORETURN void z_riscv_user_mode_enter_syscall(k_thread_entry_t user_entry,
 
 	is_user_mode = true;
 
-	__asm__ volatile ("mv a0, %1"
+	register void *a0 __asm__("a0") = user_entry;
-			  : "=r" (user_entry)
+	register void *a1 __asm__("a1") = p1;
-			  : "r" (user_entry)
+	register void *a2 __asm__("a2") = p2;
-			  : "memory");
+	register void *a3 __asm__("a3") = p3;
 
-	__asm__ volatile ("mv a1, %1"
+	__asm__ volatile (
-			  : "=r" (p1)
+	"mv sp, %4; mret"
-			  : "r" (p1)
+	:
-			  : "memory");
+	: "r" (a0), "r" (a1), "r" (a2), "r" (a3), "r" (top_of_user_stack)
-
+	: "memory");
-	__asm__ volatile ("mv a2, %1"
-			  : "=r" (p2)
-			  : "r" (p2)
-			  : "memory");
-
-	__asm__ volatile ("mv a3, %1"
-			  : "=r" (p3)
-			  : "r" (p3)
-			  : "memory");
-
-	__asm__ volatile ("mv sp, %1"
-			  : "=r" (top_of_user_stack)
-			  : "r" (top_of_user_stack)
-			  : "memory");
-
-	__asm__ volatile ("mret");
 
 	CODE_UNREACHABLE;
 }

arch/riscv/core/userspace.S

@@ -16,31 +16,10 @@
 #include "asm_macros.inc"
 
 /* exports */
-GTEXT(z_riscv_do_syscall)
 GTEXT(arch_user_string_nlen)
 GTEXT(z_riscv_user_string_nlen_fault_start)
 GTEXT(z_riscv_user_string_nlen_fault_end)
 GTEXT(z_riscv_user_string_nlen_fixup)
-GTEXT(z_riscv_do_syscall_start)
-GTEXT(z_riscv_do_syscall_end)
-
-/* Imports */
-GDATA(_k_syscall_table)
-
-SECTION_FUNC(TEXT,z_riscv_do_syscall)
-	la	t0, _k_syscall_table
-
-	slli	t1, a7, RV_REGSHIFT	# Determine offset from indice value
-	add	t0, t0, t1		# Table addr + offset = function addr
-	lr	t3, 0(t0)	# Load function address
-
-	/* Execute syscall function */
-	jalr 	t3
-
-	/* Return to ISR environment to switch-back in user mode */
-z_riscv_do_syscall_start:
-	ECALL
-z_riscv_do_syscall_end:
 
 /*
  * size_t arch_user_string_nlen(const char *s, size_t maxsize, int *err_arg)

arch/riscv/include/kernel_arch_func.h

@@ -29,6 +29,9 @@ void z_riscv_configure_static_pmp_regions(void);
 
 static ALWAYS_INLINE void arch_kernel_init(void)
 {
+#ifdef CONFIG_USERSPACE
+	csr_write(mscratch, 0);
+#endif
 #ifdef CONFIG_RISCV_PMP
 	z_riscv_configure_static_pmp_regions();
 #endif

boards/riscv/qemu_riscv64/qemu_riscv64_defconfig

@@ -3,6 +3,7 @@
 CONFIG_SOC_SERIES_RISCV_VIRT=y
 CONFIG_SOC_RISCV_VIRT=y
 CONFIG_BOARD_QEMU_RISCV64=y
+CONFIG_PRIVILEGED_STACK_SIZE=2048
 CONFIG_CONSOLE=y
 CONFIG_SERIAL=y
 CONFIG_UART_NS16550=y

include/arch/riscv/error.h

@@ -24,41 +24,20 @@ extern "C" {
 
 #ifdef CONFIG_USERSPACE
 
-/*
- * Kernel features like canary (software stack guard) are built
- * with an argument to bypass the test before syscall (test if CPU
- * is running in user or kernel) and directly execute the function.
- * Then if this kind of code wishes to trigger a CPU exception,
- * the implemented syscall is useless because  the function is directly
- * called even if the CPU is running in user (which happens during
- * sanity check). To fix that, I bypass the generated test code by writing
- * the test myself to remove the bypass ability.
- */
-
 #define ARCH_EXCEPT(reason_p)	do {			\
 		if (k_is_user_context()) {		\
 			arch_syscall_invoke1(reason_p,	\
 				K_SYSCALL_USER_FAULT);	\
 		} else {				\
 			compiler_barrier();		\
-			z_impl_user_fault(reason_p);	\
+			arch_syscall_invoke1(reason_p,	\
+				RV_ECALL_RUNTIME_EXCEPT);\
 		}					\
 		CODE_UNREACHABLE; /* LCOV_EXCL_LINE */	\
 	} while (false)
 #else
-/*
+#define ARCH_EXCEPT(reason_p) \
- * Raise an illegal instruction exception so that mepc will hold expected value in
+	arch_syscall_invoke1(reason_p, RV_ECALL_RUNTIME_EXCEPT)
- * exception handler, and generated coredump can reconstruct the failing stack.
- * Store reason_p in register t6, marker in t5
- */
-#define ARCH_EXCEPT_MARKER 0x00DEAD00
-#define ARCH_EXCEPT(reason_p)	do {			\
-		__asm__ volatile("addi t5, %[marker], 0"	\
-			: : [marker] "r" (ARCH_EXCEPT_MARKER));	\
-		__asm__ volatile("addi t6, %[reason], 0"	\
-			: : [reason] "r" (reason_p));	\
-		__asm__ volatile("unimp");		\
-	} while (false)
 #endif
 
 __syscall void user_fault(unsigned int reason);

include/arch/riscv/exp.h

@@ -73,8 +73,11 @@ struct __esf {
 	ulong_t mepc;		/* machine exception program counter */
 	ulong_t mstatus;	/* machine status register */
 
+#ifdef CONFIG_USERSPACE
+	ulong_t sp;		/* preserved (user or kernel) stack pointer */
+#endif
+
 #if defined(CONFIG_FPU) && defined(CONFIG_FPU_SHARING)
-	uint8_t fp_state;	/* Floating-point saved context state. */
 	RV_FP_TYPE ft0;		/* Caller-saved temporary floating register */
 	RV_FP_TYPE ft1;		/* Caller-saved temporary floating register */
 	RV_FP_TYPE ft2;		/* Caller-saved temporary floating register */

include/arch/riscv/syscall.h

@@ -16,14 +16,13 @@
 #ifndef ZEPHYR_INCLUDE_ARCH_RISCV_SYSCALL_H_
 #define ZEPHYR_INCLUDE_ARCH_RISCV_SYSCALL_H_
 
-#define _SVC_CALL_CONTEXT_SWITCH	0
+/*
-#define _SVC_CALL_IRQ_OFFLOAD		1
+ * Privileged mode system calls
-#define _SVC_CALL_RUNTIME_EXCEPT	2
+ */
-#define _SVC_CALL_SYSTEM_CALL		3
+#define RV_ECALL_CONTEXT_SWITCH		0
+#define RV_ECALL_IRQ_OFFLOAD		1
+#define RV_ECALL_RUNTIME_EXCEPT		2
 
-#define FORCE_SYSCALL_ID		-1
-
-#ifdef CONFIG_USERSPACE
 #ifndef _ASMLANGUAGE
 
 #include <zephyr/types.h>
@@ -142,23 +141,24 @@ static inline uintptr_t arch_syscall_invoke0(uintptr_t call_id)
 	register ulong_t a7 __asm__ ("a7") = call_id;
 
 	__asm__ volatile ("ecall"
-			  : "+r" (a0)
+			  : "=r" (a0)
 			  : "r" (a7)
 			  : "memory");
 	return a0;
 }
 
+#ifdef CONFIG_USERSPACE
 static inline bool arch_is_user_context(void)
 {
 	/* Defined in arch/riscv/core/thread.c */
 	extern uint32_t is_user_mode;
 	return is_user_mode;
 }
+#endif
 
 #ifdef __cplusplus
 }
 #endif
 
 #endif /* _ASMLANGUAGE */
-#endif /* CONFIG_USERSPACE */
 #endif /* ZEPHYR_INCLUDE_ARCH_RISCV_SYSCALL_H_ */

include/arch/riscv/thread.h

@@ -98,8 +98,6 @@ struct _thread_arch {
 
 #ifdef CONFIG_USERSPACE
 	ulong_t priv_stack_start;
-	ulong_t user_sp;
-	ulong_t unfinished_syscall;
 	ulong_t u_pmpcfg[RISCV_PMP_CFG_NUM];
 	ulong_t u_pmpaddr[CONFIG_PMP_SLOT];
 #endif