riscv: use g_running_task store current regs

This commit fixes the regression from apache#13561

In order to determine whether a context switch has occurred,
we can use g_running_task to store the current regs.
This allows us to compare the current register state with the previously
stored state to identify if a context switch has taken place.

We added a percpu variable to hold the current context that needs
to be returned, because the running tcb->xcp.regs often
changes and is not suitable for preserving the context.

Signed-off-by: hujun5 <[email protected]>

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

@@ -58,6 +58,7 @@
 
 uintreg_t *riscv_doirq(int irq, uintreg_t *regs)
 {
+  struct tcb_s **running_task = &g_running_tasks[this_cpu()];
   struct tcb_s *tcb = this_task();
 
   board_autoled_on(LED_INIRQ);
@@ -71,9 +72,10 @@ uintreg_t *riscv_doirq(int irq, uintreg_t *regs)
     {
       regs[REG_EPC] += 4;
     }
-  else
+
+  if (*running_task != NULL)
     {
-      tcb->xcp.regs = regs;
+      (*running_task)->xcp.regs = regs;
     }
 
   /* Current regs non-zero indicates that we are processing an interrupt;
@@ -97,7 +99,7 @@ uintreg_t *riscv_doirq(int irq, uintreg_t *regs)
    * returning from the interrupt.
    */
 
-  if (regs != tcb->xcp.regs)
+  if ((*running_task) != tcb)
     {
 #ifdef CONFIG_ARCH_ADDRENV
       /* Make sure that the address environment for the previously
@@ -114,15 +116,7 @@ uintreg_t *riscv_doirq(int irq, uintreg_t *regs)
        * crashes.
        */
 
-      g_running_tasks[this_cpu()] = tcb;
-
-      /* If a context switch occurred while processing the interrupt then
-       * current_regs may have change value.  If we return any value
-       * different from the input regs, then the lower level will know
-       * that a context switch occurred during interrupt processing.
-       */
-
-      regs = tcb->xcp.regs;
+      *running_task = tcb;
     }
 
   /* Set current_regs to NULL to indicate that we are no longer in an
@@ -133,5 +127,5 @@ uintreg_t *riscv_doirq(int irq, uintreg_t *regs)
 
 #endif
   board_autoled_off(LED_INIRQ);
-  return regs;
+  return tcb->xcp.regs;
 }

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

@@ -174,6 +174,7 @@ exception_common:
   REGSTORE   s1, REG_EPC(sp)      /* Updated EPC to user context */
 
   csrr       tp, CSR_SCRATCH      /* Load kernel TP */
+  REGSTORE   sp, RISCV_PERCPU_REGS(tp)
   REGLOAD    tp, RISCV_PERCPU_TCB(tp)
 
   mv         a7, sp               /* a7 = context */

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

@@ -78,6 +78,10 @@ void up_exit(int status)
 
   nxsched_resume_scheduler(tcb);
 
+  /* g_running_tasks is not valid now */
+
+  g_running_tasks[this_cpu()] = NULL;
+
   /* Then switch contexts */
 
   riscv_fullcontextrestore(tcb);

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

@@ -35,6 +35,7 @@
 #include <arch/irq.h>
 
 #include "riscv_fork.h"
+#include "riscv_percpu.h"
 #include "riscv_internal.h"
 
 #include "sched/sched.h"
@@ -102,6 +103,7 @@
 
 pid_t riscv_fork(const struct fork_s *context)
 {
+  uintreg_t *parent_regs = (uintreg_t *)riscv_percpu_get_regs();
   struct tcb_s *parent = this_task();
   struct task_tcb_s *child;
   uintptr_t newsp;
@@ -120,7 +122,7 @@ pid_t riscv_fork(const struct fork_s *context)
 
   /* Allocate and initialize a TCB for the child task. */
 
-  child = nxtask_setup_fork((start_t)parent->xcp.regs[REG_RA]);
+  child = nxtask_setup_fork((start_t)parent_regs[REG_RA]);
   if (!child)
     {
       sinfo("nxtask_setup_fork failed\n");
@@ -130,8 +132,8 @@ pid_t riscv_fork(const struct fork_s *context)
   /* 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];
+  DEBUGASSERT(stacktop > parent_regs[REG_SP]);
+  stackutil = stacktop - parent_regs[REG_SP];
 
   /* Copy the parent stack contents (overwrites child's SP and TP) */
 
@@ -147,15 +149,15 @@ pid_t riscv_fork(const struct fork_s *context)
   /* Set up frame for context and copy the parent's user context there */
 
   memcpy((void *)(newsp - XCPTCONTEXT_SIZE),
-         parent->xcp.regs, XCPTCONTEXT_SIZE);
+         parent_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);
+  memcpy((void *)newsp, (const void *)parent_regs[REG_SP], stackutil);
 
   /* Set the new register restore area to the new stack top */
 

arch/risc-v/src/common/riscv_internal.h

@@ -109,10 +109,6 @@
 #define PMP_ACCESS_DENIED   (-1)    /* Access set and denied */
 #define PMP_ACCESS_FULL     (1)     /* Access set and allowed */
 
-/* Return values from riscv_swint */
-
-#define SWINT_CONTEXT_SWITCH (1)    /* Indicate we need context switch */
-
 #ifndef __ASSEMBLY__
 
 /* Use ASM as rv64ilp32 compiler generated address is limited */

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

@@ -52,6 +52,8 @@ static_assert(RISCV_PERCPU_USP == offsetof(riscv_percpu_t, usp),
               "RISCV_PERCPU_USP offset is wrong");
 static_assert(RISCV_PERCPU_KSP == offsetof(riscv_percpu_t, ksp),
               "RISCV_PERCPU_KSP offset is wrong");
+static_assert(RISCV_PERCPU_REGS == offsetof(riscv_percpu_t, regs),
+              "RISCV_PERCPU_REGS offset is wrong");
 
 /****************************************************************************
  * Private Data
@@ -177,6 +179,33 @@ uintptr_t riscv_percpu_get_hartid(void)
   return ((riscv_percpu_t *)scratch)->hartid;
 }
 
+/****************************************************************************
+ * Name: riscv_percpu_get_regs
+ *
+ * Description:
+ *   Get current regs by reading it from the per CPU area.
+ *
+ * Returned Value:
+ *   current regs
+ *
+ ****************************************************************************/
+
+uintptr_t riscv_percpu_get_regs(void)
+{
+  uintptr_t regs;
+  irqstate_t      flags;
+
+  flags   = up_irq_save();
+  uintptr_t scratch = READ_CSR(CSR_SCRATCH);
+
+  DEBUGASSERT(scratch >= (uintptr_t) &g_percpu &&
+              scratch <  (uintptr_t) &g_percpu + sizeof(g_percpu));
+  regs = (uintptr_t)((riscv_percpu_t *)scratch)->regs;
+
+  up_irq_restore(flags);
+  return regs;
+}
+
 /****************************************************************************
  * Name: riscv_percpu_get_irqstack
  *

arch/risc-v/src/common/riscv_percpu.h

@@ -52,6 +52,7 @@
 #define RISCV_PERCPU_IRQSTACK   (2 * INT_REG_SIZE)
 #define RISCV_PERCPU_USP        (3 * INT_REG_SIZE)
 #define RISCV_PERCPU_KSP        (4 * INT_REG_SIZE)
+#define RISCV_PERCPU_REGS       (5 * INT_REG_SIZE)
 
 #ifndef __ASSEMBLY__
 
@@ -75,6 +76,7 @@ union riscv_percpu_s
     uintreg_t           irq_stack; /* Interrupt stack */
     uintreg_t           usp;       /* Area to store user sp */
     uintreg_t           ksp;       /* Area to load kernel sp */
+    uintreg_t           regs;      /* Area to store user regs */
   };
 };
 
@@ -124,6 +126,21 @@ uintptr_t riscv_percpu_get_hartid(void);
 
 uintptr_t riscv_percpu_get_irqstack(void);
 
+/****************************************************************************
+ * Name: riscv_percpu_get_regs
+ *
+ * Description:
+ *   Get current regs by reading it from the per CPU area.
+ *
+ * Returned Value:
+ *   current regs
+ *
+ ****************************************************************************/
+
+uintptr_t riscv_percpu_get_regs(void);
+
+void riscv_percpu_set_regs(uintptr_t regs);
+
 /****************************************************************************
  * Name: riscv_percpu_set_kstack
  *

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

@@ -162,6 +162,10 @@ void riscv_sigdeliver(void)
   rtcb->irqcount--;
 #endif
 
+  /* g_running_tasks is not valid now */
+
+  g_running_tasks[this_cpu()] = NULL;
+
   rtcb->xcp.regs = regs;
   riscv_fullcontextrestore(rtcb);
 }

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

@@ -159,10 +159,6 @@ uintptr_t dispatch_syscall(unsigned int nbr, uintptr_t parm1,
       return -ENOSYS;
     }
 
-  /* Set the user register context to TCB */
-
-  rtcb->xcp.regs = context;
-
   /* Indicate that we are in a syscall handler */
 
   rtcb->flags |= TCB_FLAG_SYSCALL;
@@ -257,7 +253,6 @@ int riscv_swint(int irq, void *context, void *arg)
           struct tcb_s *next = (struct tcb_s *)(uintptr_t)regs[REG_A2];
 
           DEBUGASSERT(regs[REG_A1] != 0 && regs[REG_A2] != 0);
-          prev->xcp.regs = regs;
           riscv_savecontext(prev);
           new_regs = next->xcp.regs;
           riscv_restorecontext(next);
@@ -496,7 +491,6 @@ int riscv_swint(int irq, void *context, void *arg)
   if (regs != new_regs)
     {
       restore_critical_section(this_task(), this_cpu());
-      return SWINT_CONTEXT_SWITCH;
     }
 
   return OK;

arch/risc-v/src/common/supervisor/riscv_perform_syscall.c

@@ -37,19 +37,24 @@
 
 void *riscv_perform_syscall(uintreg_t *regs)
 {
+  struct tcb_s **running_task = &g_running_tasks[this_cpu()];
   struct tcb_s *tcb;
-  int cpu;
-  int ret;
+
+  if (*running_task != NULL)
+    {
+      (*running_task)->xcp.regs = regs;
+    }
 
   /* Set up the interrupt register set needed by swint() */
 
   up_set_current_regs(regs);
 
   /* Run the system call handler (swint) */
 
-  ret = riscv_swint(0, regs, NULL);
+  riscv_swint(0, regs, NULL);
+  tcb = this_task();
 
-  if (ret == SWINT_CONTEXT_SWITCH)
+  if ((*running_task) != tcb)
     {
 #ifdef CONFIG_ARCH_ADDRENV
       /* Make sure that the address environment for the previously
@@ -65,20 +70,10 @@ void *riscv_perform_syscall(uintreg_t *regs)
        * assertion logic for reporting crashes.
        */
 
-      cpu = this_cpu();
-      tcb = current_task(cpu);
-      g_running_tasks[cpu] = tcb;
-
-      /* If a context switch occurred while processing the interrupt then
-       * current_regs may have change value.  If we return any value
-       * different from the input regs, then the lower level will know
-       * that a context switch occurred during interrupt processing.
-       */
-
-      regs = tcb->xcp.regs;
+      *running_task = tcb;
     }
 
   up_set_current_regs(NULL);
 
-  return regs;
+  return tcb->xcp.regs;
 }