You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/03/13 13:01:34 UTC

[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5731: arch/risc-v: Improve speed of context switch

xiaoxiang781216 commented on a change in pull request #5731:
URL: https://github.com/apache/incubator-nuttx/pull/5731#discussion_r825443766



##########
File path: arch/risc-v/src/qemu-rv/qemu_rv_head.S
##########
@@ -77,9 +77,9 @@ __start:
   ld   sp, 0(t0)
 #endif
 
-  /* sp (stack top) = sp + idle stack size */
+  /* sp (stack top) = sp + idle stack size - XCPTCONTEXT_SIZE */
 
-  li   t0, CONFIG_IDLETHREAD_STACKSIZE
+  li   t0, CONFIG_IDLETHREAD_STACKSIZE - XCPTCONTEXT_SIZE

Review comment:
       don't need reserve xcpt for idle thread

##########
File path: arch/risc-v/src/k210/k210_head.S
##########
@@ -73,9 +73,9 @@ __start:
 
   ld   sp, 0(t0)
 
-  /* sp (stack top) = sp + idle stack size */
+  /* sp (stack top) = sp + idle stack size - XCPTCONTEXT_SIZE */
 
-  li   t0, CONFIG_IDLETHREAD_STACKSIZE
+  li   t0, CONFIG_IDLETHREAD_STACKSIZE - XCPTCONTEXT_SIZE

Review comment:
       don't preserve

##########
File path: arch/risc-v/src/common/riscv_schedulesigaction.c
##########
@@ -267,20 +258,34 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
                    * been delivered.
                    */
 
-                  tcb->xcp.sigdeliver        = (void *)sigdeliver;
-                  tcb->xcp.saved_epc         = tcb->xcp.regs[REG_EPC];
-                  tcb->xcp.saved_int_ctx     = tcb->xcp.regs[REG_INT_CTX];
+                  tcb->xcp.sigdeliver = sigdeliver;
 
                   /* Then set up vector to the trampoline with interrupts
                    * disabled.  We must already be in privileged thread mode
                    * to be here.
                    */
 
-                  tcb->xcp.regs[REG_EPC]      = (uintptr_t)riscv_sigdeliver;
+                  /* Save the current register context location */
+
+                  tcb->xcp.saved_regs = tcb->xcp.regs;
+
+                  /* Duplicate the register context.  These will be
+                   * restored by the signal trampoline after the signal has
+                   * been delivered.
+                   */
+
+                  tcb->xcp.regs = (uintptr_t *)STACK_ALIGN_DOWN(
+                                    (uintptr_t)tcb->xcp.regs -
+                                    XCPTCONTEXT_SIZE);

Review comment:
       merge to line 278

##########
File path: arch/risc-v/src/common/riscv_schedulesigaction.c
##########
@@ -172,26 +162,27 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
            * been delivered.
            */
 
-          tcb->xcp.sigdeliver       = sigdeliver;
-          tcb->xcp.saved_epc        = tcb->xcp.regs[REG_EPC];
-          tcb->xcp.saved_int_ctx    = tcb->xcp.regs[REG_INT_CTX];
+          tcb->xcp.sigdeliver    = sigdeliver;

Review comment:
       remove the extra space before =

##########
File path: arch/risc-v/src/common/riscv_schedulesigaction.c
##########
@@ -353,8 +370,22 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
            */
 
           tcb->xcp.sigdeliver        = (void *)sigdeliver;
-          tcb->xcp.saved_epc         = tcb->xcp.regs[REG_EPC];
-          tcb->xcp.saved_int_ctx     = tcb->xcp.regs[REG_INT_CTX];
+
+          /* Save the current register context location */
+
+          tcb->xcp.saved_regs        = tcb->xcp.regs;
+
+          /* Duplicate the register context.  These will be
+           * restored by the signal trampoline after the signal has been
+           * delivered.
+           */
+
+          tcb->xcp.regs = (uintptr_t *)STACK_ALIGN_DOWN(
+                            (uintptr_t)tcb->xcp.regs -
+                            XCPTCONTEXT_SIZE);

Review comment:
       merge to line 384

##########
File path: arch/risc-v/src/common/riscv_schedulesigaction.c
##########
@@ -293,8 +298,26 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
                    */
 
                   tcb->xcp.sigdeliver       = (void *)sigdeliver;
-                  tcb->xcp.saved_epc        = CURRENT_REGS[REG_EPC];
-                  tcb->xcp.saved_int_ctx    = CURRENT_REGS[REG_INT_CTX];
+
+                  /* And make sure that the saved context in the TCB is the
+                   * same as the interrupt return context.
+                   */
+
+                  riscv_savestate(tcb->xcp.saved_regs);
+
+                  /* Duplicate the register context.  These will be
+                   * restored by the signal trampoline after the signal has
+                   * been delivered.
+                   */
+
+                  CURRENT_REGS = (uintptr_t *)STACK_ALIGN_DOWN(
+                                   (uintptr_t)CURRENT_REGS -
+                                  XCPTCONTEXT_SIZE);

Review comment:
       align with line 314

##########
File path: arch/risc-v/src/common/riscv_schedulesigaction.c
##########
@@ -172,26 +162,27 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
            * been delivered.
            */
 
-          tcb->xcp.sigdeliver       = sigdeliver;
-          tcb->xcp.saved_epc        = tcb->xcp.regs[REG_EPC];
-          tcb->xcp.saved_int_ctx    = tcb->xcp.regs[REG_INT_CTX];
+          tcb->xcp.sigdeliver    = sigdeliver;
 
-          /* Then set up to vector to the trampoline with interrupts
-           * disabled.  We must already be in privileged thread mode to be
-           * here.
+          /* Save the current register context location */
+
+          tcb->xcp.saved_regs = tcb->xcp.regs;
+
+          /* Duplicate the register context.  These will be
+           * restored by the signal trampoline after the signal has been
+           * delivered.
            */
 
-          tcb->xcp.regs[REG_EPC]      = (uintptr_t)riscv_sigdeliver;
+          tcb->xcp.regs = (uintptr_t *)STACK_ALIGN_DOWN(
+                          (uintptr_t)tcb->xcp.regs - XCPTCONTEXT_SIZE);
+          memcpy(tcb->xcp.regs, tcb->xcp.saved_regs, XCPTCONTEXT_SIZE);
+
+          tcb->xcp.regs[REG_SP] = (uintptr_t)tcb->xcp.regs;

Review comment:
       keep = at line 180 align with line 182

##########
File path: arch/risc-v/src/common/riscv_schedulesigaction.c
##########
@@ -267,20 +258,34 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
                    * been delivered.
                    */
 
-                  tcb->xcp.sigdeliver        = (void *)sigdeliver;
-                  tcb->xcp.saved_epc         = tcb->xcp.regs[REG_EPC];
-                  tcb->xcp.saved_int_ctx     = tcb->xcp.regs[REG_INT_CTX];
+                  tcb->xcp.sigdeliver = sigdeliver;
 
                   /* Then set up vector to the trampoline with interrupts
                    * disabled.  We must already be in privileged thread mode
                    * to be here.
                    */
 
-                  tcb->xcp.regs[REG_EPC]      = (uintptr_t)riscv_sigdeliver;
+                  /* Save the current register context location */
+
+                  tcb->xcp.saved_regs = tcb->xcp.regs;
+
+                  /* Duplicate the register context.  These will be
+                   * restored by the signal trampoline after the signal has
+                   * been delivered.
+                   */
+
+                  tcb->xcp.regs = (uintptr_t *)STACK_ALIGN_DOWN(
+                                    (uintptr_t)tcb->xcp.regs -
+                                    XCPTCONTEXT_SIZE);
 
-                  int_ctx                     = tcb->xcp.regs[REG_INT_CTX];
-                  int_ctx                    &= ~MSTATUS_MPIE;
+                  memcpy(tcb->xcp.regs, tcb->xcp.saved_regs,
+                         XCPTCONTEXT_SIZE);

Review comment:
       merge to line 281

##########
File path: arch/risc-v/src/common/riscv_schedulesigaction.c
##########
@@ -293,8 +298,26 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
                    */
 
                   tcb->xcp.sigdeliver       = (void *)sigdeliver;
-                  tcb->xcp.saved_epc        = CURRENT_REGS[REG_EPC];
-                  tcb->xcp.saved_int_ctx    = CURRENT_REGS[REG_INT_CTX];
+
+                  /* And make sure that the saved context in the TCB is the
+                   * same as the interrupt return context.
+                   */
+
+                  riscv_savestate(tcb->xcp.saved_regs);
+
+                  /* Duplicate the register context.  These will be
+                   * restored by the signal trampoline after the signal has
+                   * been delivered.
+                   */
+
+                  CURRENT_REGS = (uintptr_t *)STACK_ALIGN_DOWN(
+                                   (uintptr_t)CURRENT_REGS -
+                                  XCPTCONTEXT_SIZE);
+
+                  memcpy((uintptr_t *)CURRENT_REGS, tcb->xcp.saved_regs,
+                         XCPTCONTEXT_SIZE);
+
+                  CURRENT_REGS[REG_SP] = (uintptr_t)CURRENT_REGS;

Review comment:
       align = with line 327

##########
File path: arch/risc-v/src/common/riscv_initialstate.c
##########
@@ -74,24 +78,21 @@ void up_initial_state(struct tcb_s *tcb)
 
       riscv_stack_color(tcb->stack_alloc_ptr, 0);
 #endif /* CONFIG_STACK_COLORATION */
+      return;
     }
 
-  /* Initialize the initial exception register context structure */
-
-  memset(xcp, 0, sizeof(struct xcptcontext));
+  xcp->regs = (uintptr_t *)(STACK_ALIGN_DOWN(
+                            (uintptr_t)tcb->stack_base_ptr +
+                                     tcb->adj_stack_size -
+                                     XCPTCONTEXT_SIZE));
 
-  /* Save the initial stack pointer.  Hmmm.. the stack is set to the very
-   * beginning of the stack region.  Some functions may want to store data on
-   * the caller's stack and it might be good to reserve some space.  However,
-   * only the start function would do that and we have control over that one
-   */
+  /* Save the initial stack pointer */
 
-  xcp->regs[REG_SP]      = (uintptr_t)tcb->stack_base_ptr +
-                                      tcb->adj_stack_size;
+  xcp->regs[REG_SP] = (uintptr_t)xcp->regs;
 
   /* Save the task entry point */
 
-  xcp->regs[REG_EPC]     = (uintptr_t)tcb->start;
+  xcp->regs[REG_EPC] = (uintptr_t)tcb->start;

Review comment:
       remove the space at before = at line 100 too




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org