You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "hujun260 (via GitHub)" <gi...@apache.org> on 2024/09/14 04:02:23 UTC

[PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

hujun260 opened a new pull request, #13444:
URL: https://github.com/apache/nuttx/pull/13444

   with other functionalities removed.
   
   reason:
   by doing this we can reduce context switch time,
   When we exit from an interrupt handler, we directly use tcb->xcp.regs
   
   ## Summary
   
   ## Impact
   
   ## Testing
   
   


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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#issuecomment-2372863546

   
   
   
   > @hujun260 I noticed that spresense:smp and raspberrypi-pico:smp do not boot with this PR.
   
   @masayuki2009 
   I found that this configuration (raspberrypi-pico:smp) cannot be started either. Even without this pr, it seems that this problem is not introduced by my pr?


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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#issuecomment-2372681389

   @hujun260 
   I noticed that spresense:smp and raspberrypi-pico:smp do not boot with this PR.
   


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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#discussion_r1761664427


##########
arch/arm/src/arm/arm_schedulesigaction.c:
##########
@@ -89,70 +89,15 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
        * being delivered to the currently executing task.
        */
 
-      sinfo("rtcb=%p current_regs=%p\n", this_task(), up_current_regs());
+      sinfo("rtcb=%p current_regs=%p\n", this_task(),
+            this_task()->xcp.regs);
 
-      if (tcb == this_task())
+      if (tcb == this_task() && !up_interrupt_context())
         {
-          /* CASE 1:  We are not in an interrupt handler and
-           * a task is signalling itself for some reason.
-           */
-
-          if (!up_current_regs())
-            {
-              /* In this case just deliver the signal now. */
-
-              sigdeliver(tcb);
-              tcb->xcp.sigdeliver = NULL;
-            }
-
-          /* CASE 2:  We are in an interrupt handler AND the
-           * interrupted task is the same as the one that
-           * must receive the signal, then we will have to modify
-           * the return state as well as the state in the TCB.
-           *
-           * Hmmm... there looks like a latent bug here: The following
-           * logic would fail in the strange case where we are in an
-           * interrupt handler, the thread is signalling itself, but
-           * a context switch to another task has occurred so that
-           * current_regs does not refer to the thread of this_task()!
-           */
-
-          else

Review Comment:
   why remove else branch



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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#discussion_r1759661734


##########
arch/arm/src/arm/arm_doirq.c:
##########
@@ -71,6 +73,7 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
    */
 
   up_set_current_regs(regs);
+  tcb->xcp.regs = regs;

Review Comment:
   why not mapping tcb->xcp.regs in up_current_regs?



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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#discussion_r1759703540


##########
arch/arm/src/arm/arm_doirq.c:
##########
@@ -71,6 +73,7 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
    */
 
   up_set_current_regs(regs);
+  tcb->xcp.regs = regs;

Review Comment:
   tcb->xcp.regs will not be NULL,  how do we determine if we are in irq
   
   In most ARM architectures, we will eventually deprecate the use of g_current_regs.
   context switching will directly manipulate tcb->xcp.regs.
   To determine if we are in irq, we use per-CPU registers or the interrupt status register.



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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#issuecomment-2369774065

   This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. 


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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#discussion_r1761668699


##########
arch/arm64/src/common/arm64_schedulesigaction.c:
##########
@@ -127,214 +127,46 @@ void arm64_init_signal_process(struct tcb_s *tcb, struct regs_context *regs)
  *
  ****************************************************************************/
 
-#ifndef CONFIG_SMP
 void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
 {
+  sinfo("tcb=%p sigdeliver=%p\n", tcb, sigdeliver);
+
   /* Refuse to handle nested signal actions */
 
   if (!tcb->xcp.sigdeliver)
     {
       tcb->xcp.sigdeliver = sigdeliver;
 
       /* First, handle some special cases when the signal is being delivered
-       * to task that is currently executing on this CPU.
+       * to task that is currently executing on any CPU.
        */
 
-      if (tcb == this_task())
-        {
-          /* CASE 1:  We are not in an interrupt handler and a task is
-           * signaling itself for some reason.
-           */
-
-          if (!up_current_regs())
-            {
-              /* In this case just deliver the signal now.
-               * REVISIT:  Signal handler will run in a critical section!
-               */
-
-              sigdeliver(tcb);
-              tcb->xcp.sigdeliver = NULL;
-            }
+      sinfo("rtcb=%p current_regs=%p\n", this_task(),

Review Comment:
   why add



##########
arch/arm64/src/common/arm64_schedulesigaction.c:
##########
@@ -127,214 +127,46 @@ void arm64_init_signal_process(struct tcb_s *tcb, struct regs_context *regs)
  *
  ****************************************************************************/
 
-#ifndef CONFIG_SMP
 void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
 {
+  sinfo("tcb=%p sigdeliver=%p\n", tcb, sigdeliver);
+
   /* Refuse to handle nested signal actions */
 
   if (!tcb->xcp.sigdeliver)
     {
       tcb->xcp.sigdeliver = sigdeliver;
 
       /* First, handle some special cases when the signal is being delivered
-       * to task that is currently executing on this CPU.
+       * to task that is currently executing on any CPU.
        */
 
-      if (tcb == this_task())
-        {
-          /* CASE 1:  We are not in an interrupt handler and a task is
-           * signaling itself for some reason.
-           */
-
-          if (!up_current_regs())
-            {
-              /* In this case just deliver the signal now.
-               * REVISIT:  Signal handler will run in a critical section!
-               */
-
-              sigdeliver(tcb);
-              tcb->xcp.sigdeliver = NULL;
-            }
+      sinfo("rtcb=%p current_regs=%p\n", this_task(),
+            this_task()->xcp.regs);
 
-          /* CASE 2:  We are in an interrupt handler AND the interrupted
-           * task is the same as the one that must receive the signal, then
-           * we will have to modify the return state as well as the state
-           * in the TCB.
-           *
-           * Hmmm... there looks like a latent bug here: The following logic
-           * would fail in the strange case where we are in an interrupt
-           * handler, the thread is signaling itself, but a context switch
-           * to another task has occurred so that current_regs does not
-           * refer to the thread of this_task()!
+      if (tcb == this_task() && !up_interrupt_context())
+        {
+          /* In this case just deliver the signal now.
+           * REVISIT:  Signal handler will run in a critical section!
            */
 
-          else

Review Comment:
   why remove else branch



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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#issuecomment-2368715130

   please fix conflict, @hujun260 


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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#issuecomment-2371302710

   This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. 


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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #13444:
URL: https://github.com/apache/nuttx/pull/13444


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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#issuecomment-2360081005

   > [**[Experimental Bot, please feedback here]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13494&type=pullrequests)
   > 
   > ## NuttX PR Requirements Review
   > While the provided summary gives a high-level idea of the change, it lacks some important details.
   > 
   > Here's a breakdown of what's missing and how to improve the PR:
   > 
   > **Summary:**
   > 
   > * **Clarity on the "Why":**  Instead of just stating functionalities were removed, explain the benefit of removing them. Why is reducing context switch time important? What kind of performance improvement does it bring?
   > * **Specificity on "What":** Which functionalities were removed exactly? This needs to be crystal clear.
   > * **Missing Information:**
   >   
   >   * **Related Issues:** Are there any related NuttX issues this PR addresses? If so, provide links.
   >   * **NuttX Apps Impact:**  Does this change impact any NuttX Apps? Be specific.
   > 
   > **Impact:**
   > 
   > * **Incomplete:** You need to address **all** the impact points listed in the requirements.  Even if the answer is "NO," state it explicitly. Don't leave any section blank.  For example:
   >   
   >   * **Is a new feature added?** NO
   >   * **Is an existing feature changed?** YES (Please describe how the removal of g_current_regs functionality impacts existing features)
   >   * **Impact on the user:** NO (or YES if users need to be aware of anything)
   >   * ...and so on for all the impact points.
   > 
   > **Testing:**
   > 
   > * **Insufficient Information:**
   >   
   >   * **Build Hosts:** Specify the operating system, CPU architecture, and compiler used for testing.
   >   * **Targets:** Be specific about the architecture (e.g., ARM Cortex-M4) and board configuration used.
   > * **Missing Logs:**
   >   
   >   * Provide actual testing logs from **before** and **after** the change. This helps reviewers understand how the code behaved before and after your changes. Include relevant logs that demonstrate the performance improvement (context switch time).
   > 
   > **Overall:**
   > 
   > The PR is incomplete and needs more details to meet the NuttX requirements. Provide the missing information and address all the impact points clearly. Detailed testing information, including logs, is crucial for reviewers to evaluate your changes effectively.
   
   i update the pr


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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "lupyuen (via GitHub)" <gi...@apache.org>.
lupyuen commented on PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#issuecomment-2359296999

   [**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13494&type=pullrequests)
   
   ## NuttX PR Requirements Review
   
   While the provided summary gives a high-level idea of the change, it lacks some important details. 
   
   Here's a breakdown of what's missing and how to improve the PR:
   
   **Summary:**
   
   * **Clarity on the "Why":**  Instead of just stating functionalities were removed, explain the benefit of removing them. Why is reducing context switch time important? What kind of performance improvement does it bring?
   * **Specificity on "What":** Which functionalities were removed exactly? This needs to be crystal clear.
   * **Missing Information:**
       * **Related Issues:** Are there any related NuttX issues this PR addresses? If so, provide links.
       * **NuttX Apps Impact:**  Does this change impact any NuttX Apps? Be specific.
   
   **Impact:**
   
   * **Incomplete:** You need to address **all** the impact points listed in the requirements.  Even if the answer is "NO," state it explicitly. Don't leave any section blank.  For example:
       * **Is a new feature added?** NO
       * **Is an existing feature changed?** YES (Please describe how the removal of g_current_regs functionality impacts existing features)
       * **Impact on the user:** NO (or YES if users need to be aware of anything)
       * ...and so on for all the impact points.
   
   **Testing:**
   
   * **Insufficient Information:** 
       * **Build Hosts:** Specify the operating system, CPU architecture, and compiler used for testing. 
       * **Targets:** Be specific about the architecture (e.g., ARM Cortex-M4) and board configuration used. 
   * **Missing Logs:** 
       * Provide actual testing logs from **before** and **after** the change. This helps reviewers understand how the code behaved before and after your changes. Include relevant logs that demonstrate the performance improvement (context switch time).
   
   **Overall:**
   
   The PR is incomplete and needs more details to meet the NuttX requirements.  Provide the missing information and address all the impact points clearly.  Detailed testing information, including logs, is crucial for reviewers to evaluate your changes effectively. 
   


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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#discussion_r1767822380


##########
arch/arm/src/tlsr82/tc32/tc32_doirq.c:
##########
@@ -35,6 +35,8 @@
 
 #include <arch/board/board.h>
 
+#include "sched/sched.h"

Review Comment:
   move to line 41



##########
arch/arm/src/armv8-m/arm_schedulesigaction.c:
##########
@@ -264,163 +116,70 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
            * state as well as the state in the TCB.
            */
 
-          else
-            {
-              /* If we signaling a task running on the other CPU, we have
-               * to PAUSE the other CPU.
-               */
-
-              if (cpu != me)
-                {
-                  /* Pause the CPU */
-
-                  up_cpu_pause(cpu);
-
-                  /* Now tcb on the other CPU can be accessed safely */
-
-                  /* Copy tcb->xcp.regs to tcp.xcp.saved. These will be
-                   * restored by the signal trampoline after the signal has
-                   * been delivered.
-                   */
-
-                  /* 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              = (void *)
-                                               ((uint32_t)tcb->xcp.regs -
-                                                          XCPTCONTEXT_SIZE);
-                  memcpy(tcb->xcp.regs, tcb->xcp.saved_regs,
-                         XCPTCONTEXT_SIZE);
-
-                  tcb->xcp.regs[REG_SP]      = (uint32_t)tcb->xcp.regs +
-                                                         XCPTCONTEXT_SIZE;
-
-                  /* 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_PC]      = (uint32_t)arm_sigdeliver;
-#ifdef CONFIG_ARMV8M_USEBASEPRI
-                  tcb->xcp.regs[REG_BASEPRI] = NVIC_SYSH_DISABLE_PRIORITY;
-#else
-                  tcb->xcp.regs[REG_PRIMASK] = 1;
-#endif
-                  tcb->xcp.regs[REG_XPSR]    = ARMV8M_XPSR_T;
-#ifdef CONFIG_BUILD_PROTECTED
-                  tcb->xcp.regs[REG_LR]      = EXC_RETURN_THREAD;
-                  tcb->xcp.regs[REG_CONTROL] = getcontrol() & ~CONTROL_NPRIV;
-#endif
-                }
-              else
-                {
-                  /* tcb is running on the same CPU */
-
-                  /* Save the return PC, CPSR and either the BASEPRI or
-                   * PRIMASK registers (and perhaps also the LR).  These
-                   * will be restored by the signal trampoline after the
-                   * signal has been delivered.
-                   */
-
-                  /* And make sure that the saved context in the TCB is the
-                   * same as the interrupt return context.
-                   */
-
-                  arm_savestate(tcb->xcp.saved_regs);
-
-                  /* Duplicate the register context.  These will be
-                   * restored by the signal trampoline after the signal has
-                   * been delivered.
-                   */
-
-                  up_set_current_regs(up_current_regs() - XCPTCONTEXT_REGS);
-                  memcpy(up_current_regs(), tcb->xcp.saved_regs,
-                         XCPTCONTEXT_SIZE);
-
-                  up_current_regs()[REG_SP] = (uint32_t)(up_current_regs()
-                                                        + XCPTCONTEXT_REGS);
-
-                  /* Then set up vector to the trampoline with interrupts
-                   * disabled.  The kernel-space trampoline must run in
-                   * privileged thread mode.
-                   */
+          /* If we signaling a task running on the other CPU, we have
+           * to PAUSE the other CPU.
+           */
 
-                  up_current_regs()[REG_PC]      = (uint32_t)arm_sigdeliver;
-#ifdef CONFIG_ARMV8M_USEBASEPRI
-                  up_current_regs()[REG_BASEPRI] =
-                  NVIC_SYSH_DISABLE_PRIORITY;
-#else
-                  up_current_regs()[REG_PRIMASK] = 1;
-#endif
-                  up_current_regs()[REG_XPSR]    = ARMV8M_XPSR_T;
-#ifdef CONFIG_BUILD_PROTECTED
-                  up_current_regs()[REG_LR]      = EXC_RETURN_THREAD;
-                  up_current_regs()[REG_CONTROL] = getcontrol() &
-                                                    ~CONTROL_NPRIV;
-#endif
-                }
+#ifdef CONFIG_SMP
+          int cpu = tcb->cpu;
+          int me  = this_cpu();
 
-              /* RESUME the other CPU if it was PAUSED */
+          if (cpu != me)
+            {
+              /* Pause the CPU */
 
-              if (cpu != me)
-                {
-                  up_cpu_resume(cpu);
-                }
+              up_cpu_pause(cpu);
             }
-        }
-
-      /* Otherwise, we are (1) signaling a task is not running from an
-       * interrupt handler or (2) we are not in an interrupt handler and the
-       * running task is signaling some other non-running task.
-       */
+#endif
 
-      else
-        {
           /* Save the return PC, CPSR and either the BASEPRI or PRIMASK
            * registers (and perhaps also the LR).  These will be restored
            * by the signal trampoline after the signal has been delivered.
            */
 
           /* Save the current register context location */
 
-          tcb->xcp.saved_regs        = tcb->xcp.regs;
+          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              = (void *)
-                                       ((uint32_t)tcb->xcp.regs -
-                                                  XCPTCONTEXT_SIZE);
+          tcb->xcp.regs                 = (void *)
+                                          ((uint32_t)tcb->xcp.regs -
+                                                     XCPTCONTEXT_SIZE);
           memcpy(tcb->xcp.regs, tcb->xcp.saved_regs, XCPTCONTEXT_SIZE);
 
-          tcb->xcp.regs[REG_SP]      = (uint32_t)tcb->xcp.regs +
-                                                 XCPTCONTEXT_SIZE;
+          tcb->xcp.regs[REG_SP]         = (uint32_t)tcb->xcp.regs +
+                                                    XCPTCONTEXT_SIZE;
 
           /* Then set up to vector to the trampoline with interrupts
            * disabled.  We must already be in privileged thread mode to be
            * here.
            */
 
-          tcb->xcp.regs[REG_PC]      = (uint32_t)arm_sigdeliver;
+          tcb->xcp.regs[REG_PC]         = (uint32_t)arm_sigdeliver;
 #ifdef CONFIG_ARMV8M_USEBASEPRI
-          tcb->xcp.regs[REG_BASEPRI] = NVIC_SYSH_DISABLE_PRIORITY;
+          tcb->xcp.regs[REG_BASEPRI]    = NVIC_SYSH_DISABLE_PRIORITY;
 #else
-          tcb->xcp.regs[REG_PRIMASK] = 1;
+          tcb->xcp.regs[REG_PRIMASK]    = 1;
 #endif
-          tcb->xcp.regs[REG_XPSR]    = ARMV8M_XPSR_T;
+          tcb->xcp.regs[REG_XPSR]       = ARMV8M_XPSR_T;
 #ifdef CONFIG_BUILD_PROTECTED
-          tcb->xcp.regs[REG_LR]      = EXC_RETURN_THREAD;
-          tcb->xcp.regs[REG_CONTROL] = getcontrol() & ~CONTROL_NPRIV;
+          tcb->xcp.regs[REG_LR]         = EXC_RETURN_THREAD;
+          tcb->xcp.regs[REG_EXC_RETURN] = EXC_RETURN_THREAD;

Review Comment:
   why add



##########
arch/arm/src/tlsr82/tc32/tc32_doirq.c:
##########
@@ -94,11 +101,9 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
 
   if (regs == NULL)
     {

Review Comment:
   why still keep this if statement?



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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#issuecomment-2372784017

   > @hujun260 I noticed that spresense:smp and raspberrypi-pico:smp do not boot with this PR.
   
   ok i'll look into this issue


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


Re: [PR] arm: g_current_regs is only used to determine if we are in irq, [nuttx]

Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on PR #13444:
URL: https://github.com/apache/nuttx/pull/13444#issuecomment-2372913088

   https://github.com/apache/nuttx/pull/13598 fix this issue


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