riscv_fork.c: Fix race condition when handling parent integer registers

We need to record the parent's integer register context upon exception
entry to a separate non-volatile area. Why?

Because xcp.regs can move due to a context switch within the fork() system
call, be it either via interrupt or a synchronization point.

Fix this by adding a "sregs" area where the saved user context is placed.
The critical section within fork() is also unnecessary.

Documentation/components/drivers/special/ioexpander.rst

@@ -1,3 +1,35 @@
 ==========================
 IO Expander Device Drivers
 ==========================
+
+-  ``include/nuttx/ioexpander/ioexpander.h`` and ``include/nuttx/ioexpander/gpio.h``.
+   All structures and APIs needed to work with ioexpander drivers are provided in
+   this header file.
+
+-  ``struct ioexpander_ops_s``. Each ioexpand device driver must implement
+   an instance of ``struct ioexpander_ops_s``. That structure defines a
+   call table with the methods, and we also provide macros to help access methods.
+
+-  we also provide method ``gpio_lower_half`` to make ioexpander compatible with normal gpio.
+
+-  **Binding ioexpander Drivers**. ioexpander drivers are not normally directly
+   accessed by user code, we should always get lower level drivers, for example I2C,
+   and map extended gpio feature same asa normal gpio. See for example,
+   ``int nrf52_sx1509_initialize(void)``
+   in ``boards/arm/nrf52/thingy52/src/nrf52_sx1509.c``. In general, the binding
+   sequence is:
+
+   #. Get an instance of ``struct i2c_master_s`` from the
+      hardware-specific I2C device driver, and
+   #. Provide that instance and configurations to the ioexpander initialization method
+      to get the ``struct ioexpander_dev_s`` ioe device instance.
+   #. Then use ioe device instance to do ioexpander operations, or use ``gpio_lower_half``
+      to make ioexpand compatible with normal gpio.
+
+
+-  **Examples**: ``drivers/ioexpander/pca9555.c``,
+   ``drivers/input/aw86225.c``,
+   ``drivers/analog/lmp92001.c``,
+   ``drivers/ioexpander/ioe_rpmsg.c``,
+   ``boards/sim/sim/sim/src/sim_ioexpander.c``,
+   ``boards/arm/nrf52/thingy52/src/nrf52_sx1509.c`` etc.

arch/risc-v/include/irq.h

@@ -613,6 +613,12 @@ struct xcptcontext
 
   uintreg_t *regs;
 
+#ifdef CONFIG_LIB_SYSCALL
+  /* User integer registers upon system call entry */
+
+  uintreg_t *sregs;
+#endif
+
   /* FPU register save area */
 
 #if defined(CONFIG_ARCH_FPU) && defined(CONFIG_ARCH_LAZYFPU)

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_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

@@ -108,19 +108,14 @@ pid_t riscv_fork(const struct fork_s *context)
   uintptr_t newtop;
   uintptr_t stacktop;
   uintptr_t stackutil;
-  irqstate_t flags;
 #ifdef CONFIG_SCHED_THREAD_LOCAL
   uintptr_t tp;
 #endif
   UNUSED(context);
 
-  /* parent regs may change in irq, we should disable irq here */
-
-  flags = up_irq_save();
-
   /* 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->xcp.sregs[REG_RA]);
   if (!child)
     {
       sinfo("nxtask_setup_fork failed\n");
@@ -130,37 +125,51 @@ 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->xcp.sregs[REG_SP]);
+  stackutil = stacktop - parent->xcp.sregs[REG_SP];
 
-  /* Copy the parent stack contents (overwrites child's SP and TP) */
+  /* Copy goes to child's user stack top */
 
   newtop = (uintptr_t)child->cmn.stack_base_ptr + child->cmn.adj_stack_size;
   newsp = newtop - stackutil;
 
+  memcpy((void *)newsp, (const void *)parent->xcp.sregs[REG_SP], 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 */
+  /* Determine the integer context save area */
 
-  memcpy((void *)(newsp - XCPTCONTEXT_SIZE),
-         parent->xcp.regs, XCPTCONTEXT_SIZE);
+#ifdef CONFIG_ARCH_KERNEL_STACK
+  if (child->cmn.xcp.kstack)
+    {
+      /* Set context to kernel stack */
+
+      stacktop = (uintptr_t)child->cmn.xcp.ktopstk;
+    }
+  else
+#endif
+    {
+      /* Set context to user stack */
+
+      stacktop = newsp;
+    }
+
+  /* Set the new register restore area to the new stack top */
+
+  child->cmn.xcp.regs = (void *)(stacktop - XCPTCONTEXT_SIZE);
+
+  /* Copy the parent integer context (overwrites child's SP and TP) */
+
+  memcpy(child->cmn.xcp.regs, parent->xcp.sregs, 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;
@@ -169,8 +178,6 @@ pid_t riscv_fork(const struct fork_s *context)
   child->cmn.xcp.regs[REG_TP] = tp;
 #endif
 
-  up_irq_restore(flags);
-
   /* And, finally, start the child task.  On a failure, nxtask_start_fork()
    * will discard the TCB by calling nxtask_abort_fork().
    */

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_sigdeliver.c

@@ -162,6 +162,10 @@ retry:
   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

@@ -161,7 +161,7 @@ uintptr_t dispatch_syscall(unsigned int nbr, uintptr_t parm1,
 
   /* Set the user register context to TCB */
 
-  rtcb->xcp.regs = context;
+  rtcb->xcp.sregs = context;
 
   /* Indicate that we are in a syscall handler */
 
@@ -257,7 +257,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 +495,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,9 +37,13 @@
 
 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() */
 
@@ -47,9 +51,10 @@ void *riscv_perform_syscall(uintreg_t *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;
 }

drivers/ioexpander/gpio_lower_half.c

@@ -113,6 +113,33 @@ static const uint32_t g_gplh_inttype[GPIO_NPINTYPES] =
   IOEXPANDER_VAL_RISING,          /* GPIO_INTERRUPT_RISING_PIN */
   IOEXPANDER_VAL_FALLING,         /* GPIO_INTERRUPT_FALLING_PIN */
   IOEXPANDER_VAL_BOTH,            /* GPIO_INTERRUPT_BOTH_PIN */
+  CONFIG_GPIO_LOWER_HALF_INTTYPE, /* GPIO_INTERRUPT_PIN_WAKEUP */
+  IOEXPANDER_VAL_HIGH,            /* GPIO_INTERRUPT_HIGH_PIN_WAKEUP */
+  IOEXPANDER_VAL_LOW,             /* GPIO_INTERRUPT_LOW_PIN_WAKEUP */
+  IOEXPANDER_VAL_RISING,          /* GPIO_INTERRUPT_RISING_PIN_WAKEUP */
+  IOEXPANDER_VAL_FALLING,         /* GPIO_INTERRUPT_FALLING_PIN_WAKEUP */
+  IOEXPANDER_VAL_BOTH,            /* GPIO_INTERRUPT_BOTH_PIN_WAKEUP */
+};
+
+static const uint32_t g_gplh_wakeuptype[GPIO_NPINTYPES] =
+{
+  IOEXPANDER_WAKEUP_DISABLE,         /* GPIO_INPUT_PIN */
+  IOEXPANDER_WAKEUP_DISABLE,         /* GPIO_INPUT_PIN_PULLUP */
+  IOEXPANDER_WAKEUP_DISABLE,         /* GPIO_INPUT_PIN_PULLDOWN */
+  IOEXPANDER_WAKEUP_DISABLE,         /* GPIO_OUTPUT_PIN */
+  IOEXPANDER_WAKEUP_DISABLE,         /* GPIO_OUTPUT_PIN_OPENDRAIN */
+  IOEXPANDER_WAKEUP_DISABLE,         /* GPIO_INTERRUPT_PIN */
+  IOEXPANDER_WAKEUP_DISABLE,         /* GPIO_INTERRUPT_HIGH_PIN */
+  IOEXPANDER_WAKEUP_DISABLE,         /* GPIO_INTERRUPT_LOW_PIN */
+  IOEXPANDER_WAKEUP_DISABLE,         /* GPIO_INTERRUPT_RISING_PIN */
+  IOEXPANDER_WAKEUP_DISABLE,         /* GPIO_INTERRUPT_FALLING_PIN */
+  IOEXPANDER_WAKEUP_DISABLE,         /* GPIO_INTERRUPT_BOTH_PIN */
+  IOEXPANDER_WAKEUP_ENABLE,          /* GPIO_INTERRUPT_PIN_WAKEUP */
+  IOEXPANDER_WAKEUP_ENABLE,          /* GPIO_INTERRUPT_HIGH_PIN_WAKEUP */
+  IOEXPANDER_WAKEUP_ENABLE,          /* GPIO_INTERRUPT_LOW_PIN_WAKEUP */
+  IOEXPANDER_WAKEUP_ENABLE,          /* GPIO_INTERRUPT_RISING_PIN_WAKEUP */
+  IOEXPANDER_WAKEUP_ENABLE,          /* GPIO_INTERRUPT_FALLING_PIN_WAKEUP */
+  IOEXPANDER_WAKEUP_ENABLE,          /* GPIO_INTERRUPT_BOTH_PIN_WAKEUP */
 };
 
 /****************************************************************************
@@ -357,6 +384,9 @@ static int gplh_setpintype(FAR struct gpio_dev_s *gpio,
 
       IOEXP_SETOPTION(ioe, pin, IOEXPANDER_OPTION_INTCFG,
                       (FAR void *)(uintptr_t)g_gplh_inttype[pintype]);
+
+      IOEXP_SETOPTION(ioe, pin, IOEXPANDER_OPTION_WAKEUPCFG,
+                      (FAR void *)(uintptr_t)g_gplh_wakeuptype[pintype]);
     }
 
   gpio->gp_pintype = pintype;

drivers/sensors/sensor.c

@@ -161,36 +161,51 @@ static const struct sensor_meta_s g_sensor_meta[] =
   {0,                                         NULL},
   {sizeof(struct sensor_accel),               "accel"},
   {sizeof(struct sensor_mag),                 "mag"},
+  {sizeof(struct sensor_orientation),         "orientation"},
   {sizeof(struct sensor_gyro),                "gyro"},
   {sizeof(struct sensor_light),               "light"},
   {sizeof(struct sensor_baro),                "baro"},
+  {sizeof(struct sensor_noise),               "noise"},
   {sizeof(struct sensor_prox),                "prox"},
+  {sizeof(struct sensor_rgb),                 "rgb"},
+  {sizeof(struct sensor_accel),               "linear_accel"},
+  {sizeof(struct sensor_rotation),            "rotation"},
   {sizeof(struct sensor_humi),                "humi"},
   {sizeof(struct sensor_temp),                "temp"},
-  {sizeof(struct sensor_rgb),                 "rgb"},
-  {sizeof(struct sensor_hall),                "hall"},
-  {sizeof(struct sensor_ir),                  "ir"},
-  {sizeof(struct sensor_uv),                  "uv"},
-  {sizeof(struct sensor_noise),               "noise"},
   {sizeof(struct sensor_pm25),                "pm25"},
   {sizeof(struct sensor_pm1p0),               "pm1p0"},
   {sizeof(struct sensor_pm10),                "pm10"},
-  {sizeof(struct sensor_co2),                 "co2"},
+  {sizeof(struct sensor_event),               "motion_detect"},
+  {sizeof(struct sensor_event),               "step_detector"},
+  {sizeof(struct sensor_step_counter),        "step_counter"},
+  {sizeof(struct sensor_ph),                  "ph"},
+  {sizeof(struct sensor_hrate),               "hrate"},
+  {sizeof(struct sensor_event),               "tilt_detector"},
+  {sizeof(struct sensor_event),               "wake_gesture"},
+  {sizeof(struct sensor_event),               "glance_gesture"},
+  {sizeof(struct sensor_event),               "pickup_gesture"},
+  {sizeof(struct sensor_event),               "wrist_tilt"},
+  {sizeof(struct sensor_orientation),         "device_orientation"},
+  {sizeof(struct sensor_pose_6dof),           "pose_6dof"},
+  {sizeof(struct sensor_gas),                 "gas"},
+  {sizeof(struct sensor_event),               "significant_motion"},
+  {sizeof(struct sensor_hbeat),               "hbeat"},
+  {sizeof(struct sensor_force),               "force"},
+  {sizeof(struct sensor_hall),                "hall"},
+  {sizeof(struct sensor_event),               "offbody_detector"},
+  {sizeof(struct sensor_uv),                  "uv"},
+  {sizeof(struct sensor_angle),               "hinge_angle"},
+  {sizeof(struct sensor_ir),                  "ir"},
   {sizeof(struct sensor_hcho),                "hcho"},
   {sizeof(struct sensor_tvoc),                "tvoc"},
-  {sizeof(struct sensor_ph),                  "ph"},
   {sizeof(struct sensor_dust),                "dust"},
-  {sizeof(struct sensor_hrate),               "hrate"},
-  {sizeof(struct sensor_hbeat),               "hbeat"},
   {sizeof(struct sensor_ecg),                 "ecg"},
   {sizeof(struct sensor_ppgd),                "ppgd"},
   {sizeof(struct sensor_ppgq),                "ppgq"},
   {sizeof(struct sensor_impd),                "impd"},
   {sizeof(struct sensor_ots),                 "ots"},
-  {sizeof(struct sensor_wake_gesture),        "wake_gesture"},
+  {sizeof(struct sensor_co2),                 "co2"},
   {sizeof(struct sensor_cap),                 "cap"},
-  {sizeof(struct sensor_gas),                 "gas"},
-  {sizeof(struct sensor_force),               "force"},
   {sizeof(struct sensor_gnss),                "gnss"},
   {sizeof(struct sensor_gnss_satellite),      "gnss_satellite"},
   {sizeof(struct sensor_gnss_measurement),    "gnss_measurement"},

include/nuttx/ioexpander/gpio.h

@@ -100,6 +100,12 @@ enum gpio_pintype_e
   GPIO_INTERRUPT_RISING_PIN,
   GPIO_INTERRUPT_FALLING_PIN,
   GPIO_INTERRUPT_BOTH_PIN,
+  GPIO_INTERRUPT_PIN_WAKEUP,
+  GPIO_INTERRUPT_HIGH_PIN_WAKEUP,
+  GPIO_INTERRUPT_LOW_PIN_WAKEUP,
+  GPIO_INTERRUPT_RISING_PIN_WAKEUP,
+  GPIO_INTERRUPT_FALLING_PIN_WAKEUP,
+  GPIO_INTERRUPT_BOTH_PIN_WAKEUP,
   GPIO_NPINTYPES
 };
 

include/nuttx/ioexpander/ioexpander.h

@@ -77,6 +77,10 @@
                                           * structure to expander driver.
                                           */
 
+#define IOEXPANDER_OPTION_WAKEUPCFG   5  /* Configure interrupt for a pin to wake up the Soc */
+#define IOEXPANDER_WAKEUP_DISABLE     0  /* Do not cfg the pin as wake up source */
+#define IOEXPANDER_WAKEUP_ENABLE      1  /* Cfg the pin as wake up source */
+
 /* Access macros ************************************************************/
 
 /****************************************************************************