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/17 10:53:47 UTC
[GitHub] [incubator-nuttx] jlaitine opened a new pull request #5247: Mpfs switch to exception common
jlaitine opened a new pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247
## Summary
Switch mpfs target to use riscv_exception_common to reduce code copies
Also fix riscv_exception_common to support more than 2 CPUs in SMP mode, and allow defining multiple IRQ stacs even when not using SMP.
This is needed by mpfs bootloader build, where nuttx is being booted only on one CPU, but the startup code still handles SW IRQs from all harts.
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r787735269
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -36,7 +36,15 @@
# define REGLOAD ld
# define REGSTORE sd
#endif
-
+
+#ifdef CONFIG_N_IRQ_STACKS
Review comment:
Sure!
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786699184
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
I can make it work; it is just a bit messy. Will need something like:
#ifndef CONFIG_SMP
# undef CONFIG_SMP_NCPUS
# define CONFIG_SMP_NCPUS 1
#endif
At least in sched/init/nx_start.c, since I don't want it to affect there. I just need the irq stacks, not SMP CPUs
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r785918264
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -99,15 +101,14 @@ exception_common:
csrr s0, mhartid
/* Switch to interrupt stack */
-
- bnez s0, 1f
- la sp, g_intstacktop
- j 2f
-1:
- la sp, g_intstacktop
- li t0, -(CONFIG_ARCH_INTERRUPTSTACK & ~15)
- add sp, sp, t0
-2:
+#if CONFIG_N_IRQ_STACKS > 1
+ li t0, ((CONFIG_ARCH_INTERRUPTSTACK) & ~15)
Review comment:
better have parenthesis, just in case someone goes and types "CONFIG_ARCH_INTERRUPTSTACK=2048+1024" or such in his .config file.
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786262409
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -49,7 +49,9 @@ exception_common:
addi sp, sp, -XCPTCONTEXT_SIZE
REGSTORE x1, REG_X1(sp) /* ra */
+#ifdef RISCV_SAVE_GP
Review comment:
No-where. But it is used elsewhere, like in riscv_vfork.c . And when the gp is not initialized there when creating a new task, it is also wrong to restore gp in here. I used the same macro in here so it can be enabled in the future
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r787041993
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
That's fine, but I still need 5 interrupt stacks even if SMP_NCPUS == 1 . This is what I am trying to accomplish here.
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r785919037
##########
File path: boards/risc-v/mpfs/icicle/scripts/ld-envm-opensbi.script
##########
@@ -59,7 +59,7 @@ SECTIONS
.text : {
_stext = ABSOLUTE(.);
- *(.vectors)
+ *mpfs_head.o
Review comment:
In this case it is actually not needed, you are right. I can remove 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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786686089
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
I will give it a try. I was a bit scared of that, since CONFIG_SMP_NCPUS can be found in so many places and in all architectures.
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786243630
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
I am actually in my setup booting two NuttX instances on the same SoC, one on e51 hart, which initializes the HW and triggers the other OSs, and another one starting on one of the u54 harts. None of those is running in SMP mode, but the one running on e51 needs the multiple irq stacks in the process of setting up the others.
--
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 #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r785857818
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -99,15 +101,14 @@ exception_common:
csrr s0, mhartid
/* Switch to interrupt stack */
-
- bnez s0, 1f
- la sp, g_intstacktop
- j 2f
-1:
- la sp, g_intstacktop
- li t0, -(CONFIG_ARCH_INTERRUPTSTACK & ~15)
- add sp, sp, t0
-2:
+#if CONFIG_N_IRQ_STACKS > 1
+ li t0, ((CONFIG_ARCH_INTERRUPTSTACK) & ~15)
Review comment:
```suggestion
li t0, (CONFIG_ARCH_INTERRUPTSTACK & ~15)
```
##########
File path: boards/risc-v/mpfs/icicle/scripts/ld-envm-opensbi.script
##########
@@ -59,7 +59,7 @@ SECTIONS
.text : {
_stext = ABSOLUTE(.);
- *(.vectors)
+ *mpfs_head.o
Review comment:
Why do we need `*` here?
--
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 #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786599397
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
How about we move SMP_NCPUS out of if SMP?
```
config SMP_NCPUS
int "Number of CPUs"
default 4 if SMP
default 1 if !SMP
range 1 32
---help---
This value identifies the number of CPUs supported by the processor
that will be used for SMP.
If CONFIG_DEBUG_FEATURES is enabled, then the value one is permitted
for CONFIG_SMP_NCPUS. This is not normally a valid setting for an
SMP configuration. However, running the SMP logic in a single CPU
configuration is useful during certain testing.
```
So, you can config SMP_NCPUS to other value without enabling SMP.
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786719918
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
no, it gets too scary. Maybe I will add some ARCH specific flag for this? CONFIG_ARCH_N_IRQ_STACKS, which can be defined only for mpfs for now?
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786750811
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
I modified the commit such that there is no longer change in sched, would this do? It keeps the same functionality for everything else, just if someone defines CONFIG_N_IRQ_STACKS it gets into use. And I only define that for MPFS bootloader. So this should be minimally intrusive
--
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 #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786914369
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
How about https://github.com/apache/incubator-nuttx/pull/5264? which should fix all place.
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r785932706
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -99,15 +101,14 @@ exception_common:
csrr s0, mhartid
/* Switch to interrupt stack */
-
- bnez s0, 1f
- la sp, g_intstacktop
- j 2f
-1:
- la sp, g_intstacktop
- li t0, -(CONFIG_ARCH_INTERRUPTSTACK & ~15)
- add sp, sp, t0
-2:
+#if CONFIG_N_IRQ_STACKS > 1
+ li t0, ((CONFIG_ARCH_INTERRUPTSTACK) & ~15)
Review comment:
Ok, removed the parenthesis. They are missing in other places as well, and the syntax for having something else than a single integer wouldn't work for defconfig file anyways (even though for .config it would work)
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786240912
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
The existence of this macro depends on CONFIG_SMP. I need multiple IRQ stacks in MPFS bootloader build, where it is not using SMP. It only boots nuttx on one core, but the startup logic still handles SW interrupts for other harts (the other harts end up in WFI, and continue booting only after when they receive the interrupt from the "bootloader" hart).
So, I figured that the best way is to just have an own config value for this, which defaults to SMP_NCPUS when SMP is selected. Otherwise board specific Kconfig files can override the default if needed.
--
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 #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r787714594
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
Yes, you are right. It's hard to mix your case with SMP. I will refine my patch to fix CONFIG_SMP_NCPUS to 1 in no SMP case, instead of undefined macro.
--
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 #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r787714594
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
Yes, you are right. It's hard to mix your case with SMP. I will refine my patch to enforce CONFIG_SMP_NCPUS to 1 in no SMP case, instead of undefined macro.
--
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 #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r787726821
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -36,7 +36,15 @@
# define REGLOAD ld
# define REGSTORE sd
#endif
-
+
+#ifdef CONFIG_N_IRQ_STACKS
Review comment:
CONFIG_N_IRQ_STACKS->CONFIG_IRQ_NSTACKS and N_IRQ_STACKS->IRQ_NSTACKS?
--
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 merged pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247
--
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 #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786217985
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -49,7 +49,9 @@ exception_common:
addi sp, sp, -XCPTCONTEXT_SIZE
REGSTORE x1, REG_X1(sp) /* ra */
+#ifdef RISCV_SAVE_GP
Review comment:
where define this macro?
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
why not use CONFIG_SMP_NCPUS?
##########
File path: arch/risc-v/src/mpfs/Kconfig
##########
@@ -46,6 +46,9 @@ config MPFS_BOOTLOADER
---help---
This NuttX image is used as a bootloader, which will boot only on one hart, putting the others in WFI
+config N_IRQ_STACKS
Review comment:
it's wrong to define N_IRQ_STACKS in here and sched/Kconfig?
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786242006
##########
File path: arch/risc-v/src/mpfs/Kconfig
##########
@@ -46,6 +46,9 @@ config MPFS_BOOTLOADER
---help---
This NuttX image is used as a bootloader, which will boot only on one hart, putting the others in WFI
+config N_IRQ_STACKS
Review comment:
No, this is actually a legal way to override the default value in another Kconfig file (board specific), https://www.kernel.org/doc/Documentation/kbuild/kconfig-language.txt:
"
A config option can be defined multiple times with the same
name, but every definition can have only a single input prompt and the
type must not conflict.
"
and
"
Default values are not limited to the menu entry where they are
defined. This means the default can be defined somewhere else or be
overridden by an earlier definition
"
--
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] jlaitine commented on pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#issuecomment-1014456243
Did the modifications suggested by @pkarashchenko
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r787362900
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
No, sorry. It would allocate all sched structs 5 times, go through all the loops e.g. in herehttps://github.com/apache/incubator-nuttx/blob/f2ee1c5b35a0fdd4c06b3daf4435194cef2757de/sched/init/nx_start.c#L378
and in many other places.
I only need 5 irq stacks for the mpfs_head.S use, I don't need 5 SMP CPUs.
It would also be extremely confusing to set SMP_NCPUS to > 1, when not having SMP, imho
Should I try not to use to common irqstacks, but define my own irqstacks somewhere else then? This is also confusing since then the exception_common would allocate one stack which wouldn't be used at all...
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786699184
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
> I can make it work; it is just a bit messy. Will need something like:
>
> #ifndef CONFIG_SMP
> # undef CONFIG_SMP_NCPUS
> # define CONFIG_SMP_NCPUS 1
> #endif
>
> At least in sched/init/nx_start.c, since I don't want it to affect there. I just need the irq stacks, not SMP CPUs
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r786699184
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
I can make it work; it is just a bit messy. Will need something like:
\#ifndef CONFIG_SMP
\# undef CONFIG_SMP_NCPUS
\# define CONFIG_SMP_NCPUS 1
\#endif
At least in sched/init/nx_start.c, since I don't want it to affect there. I just need the irq stacks, not SMP CPUs
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r787362900
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
No, sorry. It would allocate all sched structs 5 times, go through all the loops e.g. in here
https://github.com/apache/incubator-nuttx/blob/f2ee1c5b35a0fdd4c06b3daf4435194cef2757de/sched/init/nx_start.c#L378
and in many other places.
I only need 5 irq stacks for the mpfs_head.S use, I don't need 5 SMP CPUs.
It would also be extremely confusing to set SMP_NCPUS to > 1, when not having SMP, imho
Should I try not to use to common irqstacks, but define my own irqstacks somewhere else then? This is also confusing since then the exception_common would allocate one stack which wouldn't be used at all...
--
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] jlaitine commented on a change in pull request #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r787382077
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
Another solution that I can make is not to use the riscv_exception.S_common, but keep this file branched for mpfs: then we have control over the startup logic the way we need.
Things already broke for us badly previously in the commit
e47a915f4c129240bb5ed6b8e6e9bcec6c2342b2 which forces everyone to place the whole __start into the beginning of the image (we previously only had a single branch to actual starup there). I don't want to complicate things more now, but rather then just have own code for mpfs, if it is not possible to harmonize with the rest of risc-v targets.
--
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 #5247: Mpfs switch to exception common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5247:
URL: https://github.com/apache/incubator-nuttx/pull/5247#discussion_r787313601
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -178,16 +181,8 @@ exception_common:
.type g_intstackalloc, object
.type g_intstacktop, object
g_intstackalloc:
-#ifndef CONFIG_SMP
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 8) & ~15)
-#else
- .skip (((CONFIG_ARCH_INTERRUPTSTACK * CONFIG_SMP_NCPUS) + 8) & ~15)
Review comment:
But, you can set SMP_NCPUS to 5 without enable SMP now.
--
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