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/01/18 09:50:15 UTC

[GitHub] [incubator-nuttx] zhuyanlinzyl opened a new pull request #5261: xtensa: add svccall handler and use svc call in context switch

zhuyanlinzyl opened a new pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261


   ## Summary
   
   Add svccall handler and use svc call in context switch.
   
   # Reason why now context switch probability not work
   
   In some hardware Xtensa configurations, such as we used now, all timers have higher interrupt level than level 1.
   
   This is not the wrong case, but when timer interrupt happen,
   **the EXCM bit was not set by hardware in higher level**!!!
   
   1)  When in timer interrupt, if a context switch happen, the current interrupted context will 
         save by xtensa_savestate(rtcb->xcp.regs),  with **EXCM bit not set**
   
   2)   When this task become ready to run again, if it restore context from xtensa_context_restore.
         but with ps' EXCM bit not set. 
         after  ` wsr             a2, PS` instruction, the interrupt enable again...
         If interrupt happens at that time,  the context was corrupted。
             
   
   # Reason for use sw-interrupt as syscall interrupt
   
   The xtensa `syscall` instruction can cause SYSCALL interrupt. 
   **But** syscall interrupt is same interrupt level with level-one interrupt. It is design by hardware.
   Nuttx svcall must support when `enter_critical_section` ,  but can generate svcall interrupt.
   
   
   # Another way
   As we must find another way, we use software interrupt as svcall because: 
   Software interrupt may have a higher level than level-1 interrupt, this is design by hardware configuration.
   
   
   ## 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



[GitHub] [incubator-nuttx] zhuyanlinzyl edited a comment on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl edited a comment on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1017250336


   Yes,  As Xtensa is configurable cores, in hardware chip design, our chip let timer interrupt in level-2 and level-3 interrupt level.
   
   In interrupt higher than level one, the `EXCM` bit will not set by hardware when interrupt happen.
   
   I think it's the reason why we encounter this problem, but you not.
   
   In you product, the scheduler timer interrupt run in level-1 interrupt level.
   
   @Ouss4 
   


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



[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1017088834


   > @xiaoxiang781216 I will give this a good look during this week end if you can wait. Thanks.
   
   Sure, this is the most tough bug we hit on xtensa. The race happen here:
   https://github.com/apache/incubator-nuttx/pull/5261/files#diff-b1fb00346f50b11c84c36c75d51d0ae1551da08ed43959e4af932c7f6aac35d0L582-L586
   Line 582 restore PS(interrupt enable again) and the rest restore may be interrupted by the pending IRQ.
   The fix we want try is trigger a syscall call(like cortex-m or riscv) and reuse the interrupt handle process.
   The best trigger method is syscall intruction, but as @zhuyanlinzyl mention before, syscall block in the critical section, so we have to replace syscall with swint. You may have a better solution to fix this problem.


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



[GitHub] [incubator-nuttx] zhuyanlinzyl closed pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl closed pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261


   


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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1015268759


   @gustavonihei and @Ouss4 do you face the similar 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



[GitHub] [incubator-nuttx] zhuyanlinzyl closed pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl closed pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261


   


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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790458543



##########
File path: arch/xtensa/src/common/xtensa_svcall.c
##########
@@ -94,8 +101,71 @@ int xtensa_svcall(int irq, void *context, void *arg)
 
       case SYS_save_context:
         {
+          DEBUGASSERT(regs[REG_A3] != 0);

Review comment:
       This is becase "spill the register windows" was done in interrupt handler enter.
   
   As _xtensa_context_save will do _xtensa_window_spill all the time.
   
   




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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790385854



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -41,6 +64,194 @@
  * Inline functions
  ****************************************************************************/
 
+#ifndef __ASSEMBLY__
+
+/* SVC call with SYS_ call number and no parameters */
+
+static inline uintptr_t sys_call0(unsigned int nbr)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a3, %1\n"
+    "wsr a3, intset\n"

Review comment:
       @Ouss4 the key point is that the critical section also mask syscall, do you have better method to fix the problem?




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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790644686



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -41,6 +64,194 @@
  * Inline functions
  ****************************************************************************/
 
+#ifndef __ASSEMBLY__
+
+/* SVC call with SYS_ call number and no parameters */
+
+static inline uintptr_t sys_call0(unsigned int nbr)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a3, %1\n"
+    "wsr a3, intset\n"

Review comment:
       @Ouss4  Sorry for last reply,  we use sw interrupt in level 3,  and timer interrupt in level2.
   
   Yes.  A bit confused here, as I first want to use syscall, So I fixed syscall handler error, but  found I can't use it later, and still use the  syscall method
   




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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790560467



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -29,10 +29,33 @@
  * Included Files
  ****************************************************************************/
 
+#include <nuttx/config.h>
+#include <debug.h>
+#include <arch/irq.h>
+
+#ifndef __ASSEMBLY__
+#  include <stdint.h>
+#endif
+
 /****************************************************************************
  * Pre-processor Definitions
  ****************************************************************************/
 
+/* This is the value used as the argument to the SVC instruction.

Review comment:
       Yes, ESP32 has two software interrupts, one at level1 and one at level3




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



[GitHub] [incubator-nuttx] zhuyanlinzyl edited a comment on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl edited a comment on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1017250336


   Yes,  As Xtensa is configurable cores, in our hardware chip design, timer interrupt were connected to level-2 and level-3 interrupt level.
   
   In interrupt higher than level one, the `EXCM` bit will not set by hardware when interrupt happen.
   
   I think it's the reason why we encounter this problem, but you not.
   
   In you product, the scheduler timer interrupt run in level-1 interrupt level.
   
   @Ouss4 
   


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



[GitHub] [incubator-nuttx] zhuyanlinzyl edited a comment on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl edited a comment on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1018435835


   @Ouss4  the Build break it because
   This patch replace  `xtensa_context_restore` with `xtensa_switchcontext`
   You can add `xtensa_switchcontext.c` in Make.defs
   
   As toolchain `xtensa-esp32-elf-gcc` is not in my build environment.
   


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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1018435835


   @Ouss4  the Build break it because
   This patch replace  `xtensa_context_restore` with `xtensa_switchcontext`
   You can add `xtensa_context.S` in Make.defs
   
   As toolchain `xtensa-esp32-elf-gcc` is not in my build environment.
   


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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790379315



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -29,10 +29,33 @@
  * Included Files
  ****************************************************************************/
 
+#include <nuttx/config.h>
+#include <debug.h>
+#include <arch/irq.h>
+
+#ifndef __ASSEMBLY__
+#  include <stdint.h>
+#endif
+
 /****************************************************************************
  * Pre-processor Definitions
  ****************************************************************************/
 
+/* This is the value used as the argument to the SVC instruction.

Review comment:
       We implemented the windowed register ABI.




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



[GitHub] [incubator-nuttx] GUIDINGLI commented on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1018373717


   @Ouss4 Please review this for a bit quick~~, we are waiting for your suggestion.


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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r788333342



##########
File path: arch/xtensa/src/common/xtensa.h
##########
@@ -274,6 +274,8 @@ void xtensa_pause_handler(void);
 int xtensa_context_save(uint32_t *regs);
 void xtensa_context_restore(uint32_t *regs) noreturn_function;

Review comment:
       should we remove xtensa_context_save and xtensa_context_restore?




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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790380438



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -41,6 +64,194 @@
  * Inline functions
  ****************************************************************************/
 
+#ifndef __ASSEMBLY__
+
+/* SVC call with SYS_ call number and no parameters */
+
+static inline uintptr_t sys_call0(unsigned int nbr)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a3, %1\n"
+    "wsr a3, intset\n"

Review comment:
       Yes, we use software interrupt  instead of syscall.
   
   As I mentioned in title.
   
   `syscall` instruction has the same level (interruptlevel1) with other common interrupts (interruptlevel1).
   
   We must use a higher interrupt level that can generate interrupt when `enter_critical_section()`
   
   The sw interrupt in our hardware design, is hardwired to interrupt level2. 
   
   So We  can use it.
   




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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790383062



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -41,6 +64,194 @@
  * Inline functions
  ****************************************************************************/
 
+#ifndef __ASSEMBLY__
+
+/* SVC call with SYS_ call number and no parameters */
+
+static inline uintptr_t sys_call0(unsigned int nbr)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a3, %1\n"
+    "wsr a3, intset\n"

Review comment:
       The premise is that  in hardware design,  we have a software interrupt in higher level.
   
   This is not the very common solution
   
   But I can't find another method to solve this problem....
   




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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1017088834


   > @xiaoxiang781216 I will give this a good look during this week end if you can wait. Thanks.
   
   Sure, this is the most tough bug we hit on xtensa. The race happen here:
   https://github.com/apache/incubator-nuttx/pull/5261/files#diff-b1fb00346f50b11c84c36c75d51d0ae1551da08ed43959e4af932c7f6aac35d0L582-L586
   Line 582 restore PS(interrupt enable again) and the rest restore may be interrupted by the pending IRQ.
   The fix we want try is trigger a syscall call(like cortex-m or riscv) and reuse the interrupt handle process.


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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1017250336


   Yes,  As Xtensa is configurable cores, in hardware chip design, our chip let timer interrupt in level-2 and level-3 interrupt level 
   
   In interrupt higher than level one, the `EXCM` bit will not set by hardware when interrupt happen.
   
   I think it's the reason why we encounter this problem, but you do not.
   
   In you product, the timer interrupt run in level-1 interrupt level.
   
   @Ouss4 
   


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



[GitHub] [incubator-nuttx] zhuyanlinzyl edited a comment on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl edited a comment on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1020801767


   hi, @Ouss4 .
   You advice is very good, ​I upload a new patch, use `swint` to switch context. Please review again.
   thx.
   
   By the way, testing OK in our product.


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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1020801767


   hi, @Ouss4 .
   You advice is very good, ​I upload a new patch, use `swint` to switch context. Please review again.
   thx.


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



[GitHub] [incubator-nuttx] Ouss4 commented on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1016581535


   @zhuyanlinzyl can you please rebase this.  Your other PR has been merged.


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



[GitHub] [incubator-nuttx] Ouss4 commented on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1016498671


   @xiaoxiang781216 I will give this a good look during this week end if you can wait.  Thanks.


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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790644686



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -41,6 +64,194 @@
  * Inline functions
  ****************************************************************************/
 
+#ifndef __ASSEMBLY__
+
+/* SVC call with SYS_ call number and no parameters */
+
+static inline uintptr_t sys_call0(unsigned int nbr)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a3, %1\n"
+    "wsr a3, intset\n"

Review comment:
       @Ouss4  Sorry for last relay, We use sw interrupt in level 3,  and timer interrupt in level2.
   
   Yes.  A bit confused here, as I first want to use syscall, So I fixed syscall handler error, but  found I can't use it later.
   
   




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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790306728



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -41,6 +64,194 @@
  * Inline functions
  ****************************************************************************/
 
+#ifndef __ASSEMBLY__
+
+/* SVC call with SYS_ call number and no parameters */
+
+static inline uintptr_t sys_call0(unsigned int nbr)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a3, %1\n"
+    "wsr a3, intset\n"

Review comment:
       Here we are triggering a software interrupt, the cause will be interruptlevel1 or syscall?




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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790644686



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -41,6 +64,194 @@
  * Inline functions
  ****************************************************************************/
 
+#ifndef __ASSEMBLY__
+
+/* SVC call with SYS_ call number and no parameters */
+
+static inline uintptr_t sys_call0(unsigned int nbr)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a3, %1\n"
+    "wsr a3, intset\n"

Review comment:
       @Ouss4  Sorry for last reply,  we use sw interrupt in level 3,  and timer interrupt in level2.
   
   Yes.  A bit confused here, as I first want to use syscall, So I fixed syscall handler error, but  found I can't use it later.
   
   




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



[GitHub] [incubator-nuttx] Ouss4 commented on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1018395305


   > @Ouss4 Please review this for a bit quick~~, we are waiting for your suggestion.
   
   @GUIDINGLI I wanted to give a look in the week end to have enough time.  I will try to do that late today.  Meanwhile can you please check the build errors, seems there are some undefined references.


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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790384878



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -29,10 +29,33 @@
  * Included Files
  ****************************************************************************/
 
+#include <nuttx/config.h>
+#include <debug.h>
+#include <arch/irq.h>
+
+#ifndef __ASSEMBLY__
+#  include <stdint.h>
+#endif
+
 /****************************************************************************
  * Pre-processor Definitions
  ****************************************************************************/
 
+/* This is the value used as the argument to the SVC instruction.

Review comment:
       Your can have a try on ESP32.
   
   If  ESP32 have a software interrupt , and hardwired to higher level than interrupt-level1.
   @Ouss4 
   




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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790302830



##########
File path: arch/xtensa/src/common/xtensa_svcall.c
##########
@@ -94,8 +101,71 @@ int xtensa_svcall(int irq, void *context, void *arg)
 
       case SYS_save_context:
         {
+          DEBUGASSERT(regs[REG_A3] != 0);

Review comment:
       I see that `SYS_save_context` is equal to 0.  The TRM recommends to keep this syscall command to spill the register windows, to have a certain portability between different OSes.

##########
File path: arch/xtensa/include/syscall.h
##########
@@ -29,10 +29,33 @@
  * Included Files
  ****************************************************************************/
 
+#include <nuttx/config.h>
+#include <debug.h>
+#include <arch/irq.h>
+
+#ifndef __ASSEMBLY__
+#  include <stdint.h>
+#endif
+
 /****************************************************************************
  * Pre-processor Definitions
  ****************************************************************************/
 
+/* This is the value used as the argument to the SVC instruction.

Review comment:
       ```suggestion
   /* This is the value used as the argument to the SYSCALL instruction.
   ```




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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790578437



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -41,6 +64,194 @@
  * Inline functions
  ****************************************************************************/
 
+#ifndef __ASSEMBLY__
+
+/* SVC call with SYS_ call number and no parameters */
+
+static inline uintptr_t sys_call0(unsigned int nbr)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a3, %1\n"
+    "wsr a3, intset\n"

Review comment:
       @zhuyanlinzyl the namings are a bit confusing here.  The title says "Use SVC call in context swtich", SVC is an ARM thing, which I thought was meant to say `syscall`.  In your last PR you fixed the entry into the exception caused by `syscall` so I thought this is the continuation of that work.
   
   Anyway, back to the software interrupt.  So you have a software interrupt hardwired to interrupt level2 and it's attached to the `xtensa_svcall` ISR. I would suggest we fix the naming later, and call it `swint` as we do in RISC-V and others.
   
   @xiaoxiang781216 @zhuyanlinzyl what level is your timer interrupt? It has to be smaller than or same level as your software interrupt level for this to work?  Otherwise the software interrupt will still be masked. 




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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1020801767






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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790644686



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -41,6 +64,194 @@
  * Inline functions
  ****************************************************************************/
 
+#ifndef __ASSEMBLY__
+
+/* SVC call with SYS_ call number and no parameters */
+
+static inline uintptr_t sys_call0(unsigned int nbr)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a3, %1\n"
+    "wsr a3, intset\n"

Review comment:
       @Ouss4  Sorry for last relay, We use sw interrupt in level 3. and timer interrupt in level2.
   
   Yes.  A bit confused here, as I first want to use syscall, So I fixed syscall handler error, but  found I can't use it later.
   
   




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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#discussion_r790305567



##########
File path: arch/xtensa/include/syscall.h
##########
@@ -41,6 +64,194 @@
  * Inline functions
  ****************************************************************************/
 
+#ifndef __ASSEMBLY__
+
+/* SVC call with SYS_ call number and no parameters */
+
+static inline uintptr_t sys_call0(unsigned int nbr)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a3, %1\n"
+    "wsr a3, intset\n"
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    : "=r"(reg0)
+    : "i"(XCHAL_SW_SYSCALL), "r"(reg0)
+    : "a3", "memory"
+  );
+
+  return reg0;
+}
+
+/* SVC call with SYS_ call number and one parameter */
+
+static inline uintptr_t sys_call1(unsigned int nbr, uintptr_t parm1)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+  register long reg1 __asm__("a3") = (long)(parm1);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a4, %1\n"
+    "wsr a4, intset\n"
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    : "=r"(reg0)
+    : "i"(XCHAL_SW_SYSCALL), "r"(reg0), "r"(reg1)
+    : "a4", "memory"
+  );
+
+  return reg0;
+}
+
+/* SVC call with SYS_ call number and two parameters */
+
+static inline uintptr_t sys_call2(unsigned int nbr, uintptr_t parm1,
+                                  uintptr_t parm2)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+  register long reg2 __asm__("a4") = (long)(parm2);
+  register long reg1 __asm__("a3") = (long)(parm1);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a5, %1\n"
+    "wsr a5, intset\n"
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    : "=r"(reg0)
+    : "i"(XCHAL_SW_SYSCALL), "r"(reg0), "r"(reg1), "r"(reg2)
+    : "a5", "memory"
+  );
+
+  return reg0;
+}
+
+/* SVC call with SYS_ call number and three parameters */
+
+static inline uintptr_t sys_call4(unsigned int nbr, uintptr_t parm1,
+                                  uintptr_t parm2, uintptr_t parm3,
+                                  uintptr_t parm4);
+static inline uintptr_t sys_call3(unsigned int nbr, uintptr_t parm1,
+                                  uintptr_t parm2, uintptr_t parm3)
+{
+  return sys_call4(nbr, parm1, parm2, parm3, 0);
+}
+
+/* SVC call with SYS_ call number and four parameters.
+ *
+ * NOTE the nonstandard parameter passing:  parm4 is in R4
+ */
+
+static inline uintptr_t sys_call4(unsigned int nbr, uintptr_t parm1,
+                                  uintptr_t parm2, uintptr_t parm3,
+                                  uintptr_t parm4)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+  register long reg4 __asm__("a6") = (long)(parm4);
+  register long reg3 __asm__("a5") = (long)(parm3);
+  register long reg2 __asm__("a4") = (long)(parm2);
+  register long reg1 __asm__("a3") = (long)(parm1);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a7, %1\n"
+    "wsr a7, intset\n"
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    : "=r"(reg0)
+    : "i"(XCHAL_SW_SYSCALL), "r"(reg0), "r"(reg1), "r"(reg2),
+      "r"(reg3), "r"(reg4)
+    : "a7", "memory"
+  );
+
+  return reg0;
+}
+
+/* SVC call with SYS_ call number and five parameters.
+ *
+ * NOTE the nonstandard parameter passing:  parm4 and parm5 are in R4 and R5
+ */
+
+static inline uintptr_t sys_call6(unsigned int nbr, uintptr_t parm1,
+                                  uintptr_t parm2, uintptr_t parm3,
+                                  uintptr_t parm4, uintptr_t parm5,
+                                  uintptr_t parm6);
+static inline uintptr_t sys_call5(unsigned int nbr, uintptr_t parm1,
+                                  uintptr_t parm2, uintptr_t parm3,
+                                  uintptr_t parm4, uintptr_t parm5)
+{
+  return sys_call6(nbr, parm1, parm2, parm3, parm4, parm5, 0);
+}
+
+/* SVC call with SYS_ call number and six parameters.
+ *
+ * NOTE the nonstandard parameter passing:  parm4-parm6 are in R4-R6
+ */
+
+static inline uintptr_t sys_call6(unsigned int nbr, uintptr_t parm1,
+                                  uintptr_t parm2, uintptr_t parm3,
+                                  uintptr_t parm4, uintptr_t parm5,
+                                  uintptr_t parm6)
+{
+  register long reg0 __asm__("a2") = (long)(nbr);
+  register long reg6 __asm__("a8") = (long)(parm6);
+  register long reg5 __asm__("a7") = (long)(parm5);
+  register long reg4 __asm__("a5") = (long)(parm4);
+  register long reg3 __asm__("a4") = (long)(parm3);
+  register long reg2 __asm__("a4") = (long)(parm2);
+  register long reg1 __asm__("a3") = (long)(parm1);
+
+  __asm__ __volatile__
+  (
+    "isync\n"
+    "dsync\n"
+    "esync\n"
+    "memw\n"
+    "movi a7, %0\n"

Review comment:
       ```suggestion
       "movi a7, %1\n"
   ```




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



[GitHub] [incubator-nuttx] zhuyanlinzyl commented on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1020963777


   I got mistake, this PR is closed, please review in https://github.com/apache/incubator-nuttx/pull/5336
   
   @Ouss4  @gustavonihei 


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



[GitHub] [incubator-nuttx] zhuyanlinzyl edited a comment on pull request #5261: xtensa: add svccall handler and use svc call in context switch

Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl edited a comment on pull request #5261:
URL: https://github.com/apache/incubator-nuttx/pull/5261#issuecomment-1020801767


   hi, @Ouss4 .
   You advice is very good, ​I upload a new patch, use `swint` to switch context. Please review again.
   thx.
   
   By the way, testing OK in our product.


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