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/02/18 06:35:06 UTC
[GitHub] [incubator-nuttx] zhuyanlinzyl opened a new pull request #5541: arch:xtensa: modify svcall to swint
zhuyanlinzyl opened a new pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541
Reason: xtensa svcall only have level-1 interrupt level.
Sush do not generate interrupt when up_irq_save.
Software int can generate interrupt when up_irq_save.
Signed-off-by: zhuyanlin <zh...@xiaomi.com>
## Summary
## Impact
## Testing
Testing on qemu xtensa and xtensa board
--
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 #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#issuecomment-1046786680
> > syscall is a general concept, arch can map to different instruction(.e.g. trap, syscall, ecall, swint...).
>
> This is what I meant, `syscall` is a general concept and we do have a syscall.h file for every arch, by the SVCall as far as I know is an ARM thing which other archs call it differently.
>
> > this macros not only can use by swint.c, also can be used by svcall.c.
>
> But we don't have an svcall.c file for Xtensa. If we create one I believe it will be called syscall.c
>
> I just wanted to note and clarify if we are using SVCall as a general term or if it refers to ARM's SVC exception.
Yes, I think syscall.* is better than svcall.*, how about we create a new patch after we merge this pr to rename all svcall.* to 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] xiaoxiang781216 commented on a change in pull request #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#discussion_r811553782
##########
File path: arch/xtensa/src/common/xtensa_swint.c
##########
@@ -94,16 +102,82 @@ int xtensa_svcall(int irq, void *context, void *arg)
case SYS_save_context:
{
+ DEBUGASSERT(regs[REG_A3] != 0);
+ memcpy((uint32_t *)regs[REG_A3], regs, (4 * XCPTCONTEXT_REGS));
Review comment:
need save cp state?
--
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 #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#discussion_r811553916
##########
File path: arch/xtensa/src/common/xtensa_swint.c
##########
@@ -123,10 +197,10 @@ int xtensa_svcall(int irq, void *context, void *arg)
svcinfo(" PC: %08x PS: %08x\n",
regs[REG_PC], regs[REG_PS]);
}
-# ifdef CONFIG_DEBUG_SVCALL
+# ifdef CONFIG_DEBUG_SYSCALL
else
{
- svcinfo("SVCall Return: %d\n", regs[REG_A0]);
+ svcinfo("SVCall Return: %d\n", regs[REG_A2]);
Review comment:
replace all SVCall to 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 pull request #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#issuecomment-1047457439
I have remove common/syscall.h and merge items to include/syscall.h
please review again
@xiaoxiang781216 @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 #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#discussion_r809863305
##########
File path: arch/xtensa/src/common/xtensa_swint.c
##########
@@ -94,16 +102,79 @@ int xtensa_svcall(int irq, void *context, void *arg)
case SYS_save_context:
{
+ DEBUGASSERT(regs[REG_A3] != 0);
+ memcpy((uint32_t *)regs[REG_A3], regs, (4 * XCPTCONTEXT_REGS));
}
break;
+
+ /* A2=SYS_restore_context: This a restore context command:
Review comment:
```suggestion
/* A2=SYS_restore_context: This is a restore context command:
```
##########
File path: arch/xtensa/src/common/xtensa_swint.c
##########
@@ -94,16 +102,79 @@ int xtensa_svcall(int irq, void *context, void *arg)
case SYS_save_context:
{
+ DEBUGASSERT(regs[REG_A3] != 0);
+ memcpy((uint32_t *)regs[REG_A3], regs, (4 * XCPTCONTEXT_REGS));
}
break;
+
+ /* A2=SYS_restore_context: This a restore context command:
+ *
+ * void xtensa_fullcontextrestore(uint32_t *restoreregs)
+ * noreturn_function;
+ *
+ * At this point, the following values are saved in context:
+ *
+ * A2 = SYS_restore_context
+ * A3 = restoreregs
+ *
+ * In this case, we simply need to set CURRENT_REGS to restore
+ * register area referenced in the saved A3. context == CURRENT_REGS
+ * is the normal exception return. By setting CURRENT_REGS =
+ * context[A3], we force the return to the saved context referenced
+ * in A3.
+ */
+
+ case SYS_restore_context:
+ {
+ DEBUGASSERT(regs[REG_A3] != 0);
+ CURRENT_REGS = (uint32_t *)regs[REG_A3];
+ }
+ break;
+
+ /* A2=SYS_switch_context: This a switch context command:
Review comment:
```suggestion
/* A2=SYS_switch_context: This is a switch context command:
```
##########
File path: arch/xtensa/src/common/xtensa_swi.h
##########
@@ -0,0 +1,68 @@
+/****************************************************************************
+ * arch/xtensa/src/common/xtensa_swi.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_XTENSA_SRC_COMMON_XTENSA_SWI_H
+#define __ARCH_XTENSA_SRC_COMMON_XTENSA_SWI_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <arch/xtensa/core.h>
+#include <arch/xtensa/xtensa_corebits.h>
+
+/* Select software interrupt number for context-switch.
+ * The SW interrupt level must larger than XCHAL_SYSCALL_LEVEL
+ * and not larger than XCHAL_EXCM_LEVEL.
+ * For that it can generate interupt when up_irq_save.
+ * and not generate interupt when up_irq_disable.
+ * Otherwise return error if no one interrupt find.
+ */
Review comment:
```suggestion
/* Select the software interrupt number for context-switch.
* The SW interrupt level must be greater than XCHAL_SYSCALL_LEVEL
* and less than XCHAL_EXCM_LEVEL.
* So that we can generate an interrupt when up_irq_save is called.
* and not generate interrupt when up_irq_disable is called.
* Return an error if no suitable software interrupt was found.
*/
```
--
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 #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl edited a comment on pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#issuecomment-1046428174
@Ouss4
but I think svcall.h may be a common include file.
The svcall.h only define marco SYS_save_context/SYS_XXX and so on,
this macros not only can use by swint.c, also can be used by svcall.c.
--
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] pkarashchenko commented on a change in pull request #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#discussion_r811907608
##########
File path: arch/xtensa/src/common/xtensa_swint.c
##########
@@ -94,22 +102,90 @@ int xtensa_svcall(int irq, void *context, void *arg)
case SYS_save_context:
{
+ DEBUGASSERT(regs[REG_A3] != 0);
+ memcpy((uint32_t *)regs[REG_A3], regs, (4 * XCPTCONTEXT_REGS));
+#if XCHAL_CP_NUM > 0
+ cpstate = (uintptr_t)regs[REG_A3] + cpstate_off;
+ xtensa_coproc_savestate((struct xtensa_cpstate_s *)cpstate);
+#endif
+ }
+
+ break;
+
+ /* A2=SYS_restore_context: This is a restore context command:
+ *
+ * void xtensa_fullcontextrestore(uint32_t *restoreregs)
+ * noreturn_function;
+ *
+ * At this point, the following values are saved in context:
+ *
+ * A2 = SYS_restore_context
+ * A3 = restoreregs
+ *
+ * In this case, we simply need to set CURRENT_REGS to restore
+ * register area referenced in the saved A3. context == CURRENT_REGS
+ * is the normal exception return. By setting CURRENT_REGS =
+ * context[A3], we force the return to the saved context referenced
+ * in A3.
+ */
+
+ case SYS_restore_context:
+ {
+#if XCHAL_CP_NUM > 0
+ cpstate = (uintptr_t)regs[REG_A3] + cpstate_off;
+ xtensa_coproc_restorestate((struct xtensa_cpstate_s *)cpstate);
+#endif
+ DEBUGASSERT(regs[REG_A3] != 0);
+ CURRENT_REGS = (uint32_t *)regs[REG_A3];
}
+
+ break;
+
+ /* A2=SYS_switch_context: This is a switch context command:
+ *
+ * void xtensa_switchcontext
+ * (uint32_t *saveregs, uint32_t *restoreregs);
+ *
+ * At this point, the following values are saved in context:
+ *
+ * A2 = SYS_switch_context
+ * A3 = saveregs
+ * A4 = restoreregs
+ *
+ * In this case, we do both: We save the context registers to the save
+ * register area reference by the saved contents of A3 and then set
+ * CURRENT_REGS to the save register area referenced by the saved
+ * contents of A4.
+ */
+
+ case SYS_switch_context:
+ {
+ DEBUGASSERT(regs[REG_A3] != 0 && regs[REG_A4] != 0);
+
+ memcpy((uint32_t *)regs[REG_A3], regs, (4 * XCPTCONTEXT_REGS));
+ CURRENT_REGS = (uint32_t *)regs[REG_A4];
+ }
+
break;
}
+ if (CURRENT_REGS[REG_PS] & PS_EXCM_MASK)
Review comment:
```suggestion
if ((CURRENT_REGS[REG_PS] & PS_EXCM_MASK) != 0)
```
--
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 #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#issuecomment-1044343074
@Ouss4 Thanks for you review,
Could you please review again ?
--
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 #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#discussion_r811592182
##########
File path: arch/xtensa/src/common/xtensa_swint.c
##########
@@ -94,22 +102,96 @@ int xtensa_svcall(int irq, void *context, void *arg)
case SYS_save_context:
{
+ DEBUGASSERT(regs[REG_A3] != 0);
+ memcpy((uint32_t *)regs[REG_A3], regs, (4 * XCPTCONTEXT_REGS));
+#if XCHAL_CP_NUM > 0
+ cpstate = (uintptr_t)regs[REG_A3] + cpstate_off;
+ xtensa_coproc_savestate((struct xtensa_cpstate_s *)cpstate);
+#endif
+ }
+
+ break;
+
+ /* A2=SYS_restore_context: This is a restore context command:
+ *
+ * void xtensa_fullcontextrestore(uint32_t *restoreregs)
+ * noreturn_function;
+ *
+ * At this point, the following values are saved in context:
+ *
+ * A2 = SYS_restore_context
+ * A3 = restoreregs
+ *
+ * In this case, we simply need to set CURRENT_REGS to restore
+ * register area referenced in the saved A3. context == CURRENT_REGS
+ * is the normal exception return. By setting CURRENT_REGS =
+ * context[A3], we force the return to the saved context referenced
+ * in A3.
+ */
+
+ case SYS_restore_context:
+ {
+#if XCHAL_CP_NUM > 0
+ cpstate = (uintptr_t)regs[REG_A3] + cpstate_off;
+ xtensa_coproc_restorestate((struct xtensa_cpstate_s *)cpstate);
+#endif
+ DEBUGASSERT(regs[REG_A3] != 0);
+ CURRENT_REGS = (uint32_t *)regs[REG_A3];
+ }
+
+ break;
+
+ /* A2=SYS_switch_context: This is a switch context command:
+ *
+ * void xtensa_switchcontext
+ * (uint32_t *saveregs, uint32_t *restoreregs);
+ *
+ * At this point, the following values are saved in context:
+ *
+ * A2 = SYS_switch_context
+ * A3 = saveregs
+ * A4 = restoreregs
+ *
+ * In this case, we do both: We save the context registers to the save
+ * register area reference by the saved contents of A3 and then set
+ * CURRENT_REGS to the save register area referenced by the saved
+ * contents of A4.
+ */
+
+ case SYS_switch_context:
+ {
+ DEBUGASSERT(regs[REG_A3] != 0 && regs[REG_A4] != 0);
+
+ memcpy((uint32_t *)regs[REG_A3], regs, (4 * XCPTCONTEXT_REGS));
+#if XCHAL_CP_NUM > 0
Review comment:
remvoe, dup in xtensa_irq_dispatch.
--
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 #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#issuecomment-1044833417
@zhuyanlinzyl we still have a file at `xtensa/src/common/svcall.h` should this one be renamed as well?
BTW, other architectures (like RISC-V and ceva) also have an `svcall.h` file. I think that was a copy/paste mistake from ARM. Not sure about ceva but for RISC-V I think it should be renamed to `ecall.h`.
cc. @xiaoxiang781216
--
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 #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#issuecomment-1046428174
@Ouss4
but I think svcall.h may be a common include file.
The svcall.h only define marco SYS_save_context/SYS_XXX and so on,
this macros not only can use by swint.c, also can be used by svcall.c.
@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 pull request #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#issuecomment-1046845945
> how about we create a new patch after we merge this pr to rename all svcall. to syscall.*?
Yes, we don't have to add it to 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
[GitHub] [incubator-nuttx] acassis merged pull request #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
acassis merged pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541
--
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 #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#issuecomment-1046514473
> @zhuyanlinzyl we still have a file at `xtensa/src/common/svcall.h` should this one be renamed as well?
>
> BTW, other architectures (like RISC-V and ceva) also have an `svcall.h` file. I think that was a copy/paste mistake from ARM. Not sure about ceva but for RISC-V I think it should be renamed to `ecall.h`. cc. @xiaoxiang781216
Here is the full list:
···
./arch/risc-v/src/common/svcall.h
./arch/ceva/src/common/svcall.h
./arch/xtensa/src/common/svcall.h
./arch/arm/src/armv7-m/svcall.h
./arch/arm/src/armv7-a/svcall.h
./arch/arm/src/armv7-r/svcall.h
./arch/arm/src/armv8-m/svcall.h
./arch/arm/src/armv6-m/svcall.h
···
syscall is a general concept, arch can map to different instruction(.e.g. trap, syscall, ecall, swint...).
--
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 #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#issuecomment-1046694653
> syscall is a general concept, arch can map to different instruction(.e.g. trap, syscall, ecall, swint...).
This is what I meant, `syscall` is a general concept and we do have a syscall.h file for every arch, by the SVCall as far as I know is an ARM thing which other archs call it differently.
> this macros not only can use by swint.c, also can be used by svcall.c.
But we don't have an svcall.c file for Xtensa. If we create one I believe it will be called syscall.c
I just wanted to note and clarify if we are using SVCall as a general term or if it refers to ARM's SVC exception.
--
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 #5541: arch:xtensa: modify svcall to swint
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on pull request #5541:
URL: https://github.com/apache/incubator-nuttx/pull/5541#issuecomment-1046849689
@Ouss4 I have rename svcall.h to syscall.h in arch/xtensa/src/common/svcall.h
As it is more common name.
--
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