riscv: Fix fork() system call

arch/risc-v/Kconfig

@@ -520,7 +520,7 @@ config RISCV_MISALIGNED_HANDLER
 
 config RISCV_PERCPU_SCRATCH
 	bool "Enable Scratch-based Per-CPU storage"
-	default n
+	default y if LIB_SYSCALL
 	---help---
 		In some special chipsets, multiple CPUs may be bundled in one hardware
 		thread cluster, which results in hartid and cpuindex not being exactly

arch/risc-v/src/common/fork.S

@@ -90,6 +90,12 @@
 .type up_fork, function
 
 up_fork:
+
+#ifdef CONFIG_LIB_SYSCALL
+  /* When coming via system call, everything is in place already */
+
+  tail        riscv_fork
+#else
   /* Create a stack frame */
 
   addi        sp, sp, -FORK_SIZEOF
@@ -148,6 +154,7 @@ up_fork:
   REGLOAD     x1, FORK_RA_OFFSET(sp)
   addi        sp, sp, FORK_SIZEOF
   ret
+#endif
 
   .size  up_fork, .-up_fork
   .end

arch/risc-v/src/common/riscv_exception_common.S

@@ -55,6 +55,14 @@
 #  endif
 #endif
 
+/* System calls require the per CPU scratch area */
+
+#ifdef CONFIG_LIB_SYSCALL
+#  ifndef CONFIG_RISCV_PERCPU_SCRATCH
+#    error "CONFIG_RISCV_PERCPU_SCRATCH is needed for handling system calls"
+#  endif
+#endif
+
 /* Provide a default section for the exeception handler. */
 
 #ifndef EXCEPTION_SECTION
@@ -147,6 +155,7 @@ exception_common:
   csrr       tp, CSR_SCRATCH      /* Load kernel TP */
   REGLOAD    tp, RISCV_PERCPU_TCB(tp)
 
+  mv         a7, sp               /* a7 = context */
   call       x1, dispatch_syscall /* Dispatch the system call */
 
 return_from_syscall:
@@ -231,11 +240,7 @@ return_from_exception:
 
   load_ctx   sp
 
-#ifdef CONFIG_ARCH_KERNEL_STACK
   REGLOAD    sp, REG_SP(sp)      /* restore original sp */
-#else
-  addi       sp, sp, XCPTCONTEXT_SIZE
-#endif
 
   /* Return from exception */
 

arch/risc-v/src/common/riscv_fork.c

@@ -39,6 +39,8 @@
 
 #include "sched/sched.h"
 
+#ifdef CONFIG_ARCH_HAVE_FORK
+
 /****************************************************************************
  * Pre-processor Definitions
  ****************************************************************************/
@@ -96,7 +98,80 @@
  *
  ****************************************************************************/
 
-#ifdef CONFIG_ARCH_HAVE_FORK
+#ifdef CONFIG_LIB_SYSCALL
+
+pid_t riscv_fork(const struct fork_s *context)
+{
+  struct tcb_s *parent = this_task();
+  struct task_tcb_s *child;
+  uintptr_t newsp;
+  uintptr_t newtop;
+  uintptr_t stacktop;
+  uintptr_t stackutil;
+#ifdef CONFIG_SCHED_THREAD_LOCAL
+  uintptr_t tp;
+#endif
+  UNUSED(context);
+
+  /* Allocate and initialize a TCB for the child task. */
+
+  child = nxtask_setup_fork((start_t)parent->xcp.regs[REG_RA]);
+  if (!child)
+    {
+      sinfo("nxtask_setup_fork failed\n");
+      return (pid_t)ERROR;
+    }
+
+  /* Copy parent user stack to child */
+
+  stacktop = (uintptr_t)parent->stack_base_ptr + parent->adj_stack_size;
+  DEBUGASSERT(stacktop > parent->xcp.regs[REG_SP]);
+  stackutil = stacktop - parent->xcp.regs[REG_SP];
+
+  /* Copy the parent stack contents (overwrites child's SP and TP) */
+
+  newtop = (uintptr_t)child->cmn.stack_base_ptr + child->cmn.adj_stack_size;
+  newsp = newtop - stackutil;
+
+#ifdef CONFIG_SCHED_THREAD_LOCAL
+  /* Save child's thread pointer */
+
+  tp = child->cmn.xcp.regs[REG_TP];
+#endif
+
+  /* Set up frame for context and copy the parent's user context there */
+
+  memcpy((void *)(newsp - XCPTCONTEXT_SIZE),
+         parent->xcp.regs, XCPTCONTEXT_SIZE);
+
+  /* Save FPU */
+
+  riscv_savefpu(child->cmn.xcp.regs, riscv_fpuregs(&child->cmn));
+
+  /* Copy the parent stack contents */
+
+  memcpy((void *)newsp, (const void *)parent->xcp.regs[REG_SP], stackutil);
+
+  /* Set the new register restore area to the new stack top */
+
+  child->cmn.xcp.regs = (void *)(newsp - XCPTCONTEXT_SIZE);
+
+  /* Return 0 to child */
+
+  child->cmn.xcp.regs[REG_A0] = 0;
+  child->cmn.xcp.regs[REG_SP] = newsp;
+#ifdef CONFIG_SCHED_THREAD_LOCAL
+  child->cmn.xcp.regs[REG_TP] = tp;
+#endif
+
+  /* And, finally, start the child task.  On a failure, nxtask_start_fork()
+   * will discard the TCB by calling nxtask_abort_fork().
+   */
+
+  return nxtask_start_fork(child);
+}
+
+#else
 
 pid_t riscv_fork(const struct fork_s *context)
 {
@@ -171,14 +246,19 @@ pid_t riscv_fork(const struct fork_s *context)
   newtop = (uintptr_t)child->cmn.stack_base_ptr + child->cmn.adj_stack_size;
   newsp = newtop - stackutil;
 
-  /* Set up frame for context */
+  /* Set up frame for context and copy the initial context there */
 
   memcpy((void *)(newsp - XCPTCONTEXT_SIZE),
          child->cmn.xcp.regs, XCPTCONTEXT_SIZE);
 
-  child->cmn.xcp.regs = (void *)(newsp - XCPTCONTEXT_SIZE);
+  /* Copy the parent stack contents (overwrites child's SP and TP) */
+
   memcpy((void *)newsp, (const void *)(uintptr_t)context->sp, stackutil);
 
+  /* Set the new register restore area to the new stack top */
+
+  child->cmn.xcp.regs = (void *)(newsp - XCPTCONTEXT_SIZE);
+
   /* Was there a frame pointer in place before? */
 
 #ifdef CONFIG_RISCV_FRAMEPOINTER
@@ -246,19 +326,12 @@ pid_t riscv_fork(const struct fork_s *context)
   fregs[REG_FS11]               = context->fs11; /* Saved register fs11 */
 #endif
 
-#ifdef CONFIG_BUILD_PROTECTED
-  /* Forked task starts at `dispatch_syscall()`, which requires TP holding
-   * TCB pointer as per e6973c764c, so we please it here to support vfork.
-   */
-
-  child->cmn.xcp.regs[REG_TP]   = (uintptr_t)child;
-#endif
-
   /* And, finally, start the child task.  On a failure, nxtask_start_fork()
    * will discard the TCB by calling nxtask_abort_fork().
    */
 
   return nxtask_start_fork(child);
 }
 
+#endif /* CONFIG_LIB_SYSCALL */
 #endif /* CONFIG_ARCH_HAVE_FORK */

arch/risc-v/src/common/riscv_swint.c

@@ -48,16 +48,74 @@
  * Pre-processor Definitions
  ****************************************************************************/
 
-#ifdef CONFIG_LIB_SYSCALL
-#  define TCB_FLAGS_OFFSET offsetof(struct tcb_s, flags)
-#endif
-
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
 #ifdef CONFIG_LIB_SYSCALL
 
+/****************************************************************************
+ * Name: do_syscall
+ *
+ * Description:
+ *   Call the stub function corresponding to the system call.  NOTE the non-
+ *   standard parameter passing:
+ *
+ *     A0 = SYS_ call number
+ *     A1 = parm0
+ *     A2 = parm1
+ *     A3 = parm2
+ *     A4 = parm3
+ *     A5 = parm4
+ *     A6 = parm5
+ *
+ * Note:
+ *   Do not allow the compiler to inline this function, as it does a jump to
+ *   another procedure which can clobber any register and the compiler will
+ *   not understand it happens.
+ *
+ ****************************************************************************/
+
+static uintptr_t do_syscall(unsigned int nbr, uintptr_t parm1,
+                            uintptr_t parm2, uintptr_t parm3,
+                            uintptr_t parm4, uintptr_t parm5,
+                            uintptr_t parm6) noinline_function;
+static uintptr_t do_syscall(unsigned int nbr, uintptr_t parm1,
+                            uintptr_t parm2, uintptr_t parm3,
+                            uintptr_t parm4, uintptr_t parm5,
+                            uintptr_t parm6)
+{
+  register long a0 asm("a0") = (long)(nbr);
+  register long a1 asm("a1") = (long)(parm1);
+  register long a2 asm("a2") = (long)(parm2);
+  register long a3 asm("a3") = (long)(parm3);
+  register long a4 asm("a4") = (long)(parm4);
+  register long a5 asm("a5") = (long)(parm5);
+  register long a6 asm("a6") = (long)(parm6);
+
+  asm volatile
+    (
+     "la   t0, g_stublookup\n" /* t0=The base of the stub lookup table */
+#ifdef CONFIG_ARCH_RV32
+     "slli a0, a0, 2\n"        /* a0=Offset for the stub lookup table */
+#else
+     "slli a0, a0, 3\n"        /* a0=Offset for the stub lookup table */
+#endif
+     "add  t0, t0, a0\n"       /* t0=The address in the table */
+     REGLOAD " t0, 0(t0)\n"    /* t0=The address of the stub for this syscall */
+     "jalr ra, t0\n"           /* Call the stub (modifies ra) */
+     : "+r"(a0)
+     : "r"(a1), "r"(a2), "r"(a3), "r"(a4), "r"(a5), "r"(a6)
+     : "t0", "ra", "memory"
+  );
+
+  return a0;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
 /****************************************************************************
  * Name: dispatch_syscall
  *
@@ -72,13 +130,14 @@
  *     A4 = parm3
  *     A5 = parm4
  *     A6 = parm5
+ *     A7 = context (aka SP)
  *
  ****************************************************************************/
 
 uintptr_t dispatch_syscall(unsigned int nbr, uintptr_t parm1,
                            uintptr_t parm2, uintptr_t parm3,
                            uintptr_t parm4, uintptr_t parm5,
-                           uintptr_t parm6)
+                           uintptr_t parm6, void *context)
 {
   register long a0 asm("a0") = (long)(nbr);
   register long a1 asm("a1") = (long)(parm1);
@@ -87,7 +146,7 @@ uintptr_t dispatch_syscall(unsigned int nbr, uintptr_t parm1,
   register long a4 asm("a4") = (long)(parm4);
   register long a5 asm("a5") = (long)(parm5);
   register long a6 asm("a6") = (long)(parm6);
-
+  register struct tcb_s *rtcb asm("tp");
   uintptr_t ret;
 
   /* Valid system call ? */
@@ -99,37 +158,25 @@ uintptr_t dispatch_syscall(unsigned int nbr, uintptr_t parm1,
       return -ENOSYS;
     }
 
-  /* ra gets clobbered below, but it does not matter */
+  /* Set the user register context to TCB */
 
-  asm volatile
-    (
-     REGLOAD " t0, %1(tp)\n"                /* Load tcb->flags */
-     "ori  t0, t0, %2\n"                    /* tcb->flags |= TCB_FLAG_SYSCALL */
-     REGSTORE " t0, %1(tp)\n"
-     "addi a0, a0, -%3\n"                   /* Offset a0 to account for the reserved syscalls */
-     "la   t0, g_stublookup\n"              /* t0=The base of the stub lookup table */
-#ifdef CONFIG_ARCH_RV32
-     "slli a0, a0, 2\n"                     /* a0=Offset for the stub lookup table */
-#else
-     "slli a0, a0, 3\n"                     /* a0=Offset for the stub lookup table */
-#endif
-     "add  t0, t0, a0\n"                    /* t0=The address in the table */
-     REGLOAD " t0, 0(t0)\n"                 /* t0=The address of the stub for this syscall */
-     "jalr ra, t0\n"                        /* Call the stub (modifies ra) */
-     REGLOAD " t0, %1(tp)\n"                /* Load tcb->flags */
-     "andi  t0, t0, ~%2\n"                  /* tcb->flags &= ~TCB_FLAG_SYSCALL */
-     REGSTORE " t0, %1(tp)\n"
-     : "+r"(a0)
-     : "i"(TCB_FLAGS_OFFSET),
-       "i"(TCB_FLAG_SYSCALL),
-       "i"(CONFIG_SYS_RESERVED),
-       "r"(a1), "r"(a2), "r"(a3), "r"(a4), "r"(a5), "r"(a6)
-     : "t0", "memory"
-  );
+  rtcb->xcp.regs = context;
+
+  /* Indicate that we are in a syscall handler */
+
+  rtcb->flags |= TCB_FLAG_SYSCALL;
+
+  /* Offset a0 to account for the reserved syscalls */
 
-  /* a0 gets clobbered below, save it locally here */
+  a0 -= CONFIG_SYS_RESERVED;
 
-  ret = a0;
+  /* Run the system call, save return value locally */
+
+  ret = do_syscall(a0, a1, a2, a3, a4, a5, a6);
+
+  /* System call is now done */
+
+  rtcb->flags &= ~TCB_FLAG_SYSCALL;
 
   /* Unmask any pending signals now */
 
@@ -139,10 +186,6 @@ uintptr_t dispatch_syscall(unsigned int nbr, uintptr_t parm1,
 }
 #endif
 
-/****************************************************************************
- * Public Functions
- ****************************************************************************/
-
 /****************************************************************************
  * Name: riscv_swint
  *

boards/risc-v/qemu-rv/rv-virt/configs/pnsh/defconfig

@@ -60,7 +60,6 @@ CONFIG_PATH_INITIAL="/system/bin"
 CONFIG_RAM_SIZE=3145728
 CONFIG_RAM_START=0x80000000
 CONFIG_READLINE_CMD_HISTORY=y
-CONFIG_RISCV_PERCPU_SCRATCH=y
 CONFIG_RISCV_SEMIHOSTING_HOSTFS=y
 CONFIG_RR_INTERVAL=200
 CONFIG_SCHED_WAITPID=y

boards/risc-v/qemu-rv/rv-virt/configs/pnsh64/defconfig

@@ -59,7 +59,6 @@ CONFIG_PATH_INITIAL="/system/bin"
 CONFIG_RAM_SIZE=3145728
 CONFIG_RAM_START=0x80000000
 CONFIG_READLINE_CMD_HISTORY=y
-CONFIG_RISCV_PERCPU_SCRATCH=y
 CONFIG_RISCV_SEMIHOSTING_HOSTFS=y
 CONFIG_RR_INTERVAL=200
 CONFIG_SCHED_WAITPID=y