xiaoxiang781216 commented on a change in pull request #2061:
URL: https://github.com/apache/incubator-nuttx/pull/2061#discussion_r510279488
##########
File path: arch/arm/src/armv7-a/smp.h
##########
@@ -58,7 +58,7 @@
#define SMP_STACK_MASK 7
#define SMP_STACK_SIZE ((CONFIG_SMP_IDLETHREAD_STACKSIZE + 7) & ~7)
#define SMP_STACK_WORDS (SMP_STACK_SIZE >> 2)
-#define SMP_STACK_TOP (SMP_STACK_SIZE - 8)
+#define SMP_STACK_TOP (SMP_STACK_SIZE)
Review comment:
let's replace all SMP_STACK_TOP with SMP_STACK_SIZE?
##########
File path: arch/arm/src/arm/arm_vectors.S
##########
@@ -132,7 +132,8 @@ arm_vectorirq:
#if CONFIG_ARCH_INTERRUPTSTACK > 3
ldr sp, .Lirqstackbase /* SP = interrupt stack base */
- str r0, [sp] /* Save the user stack pointer */
+ sub sp, #4 /* Adjust SP (minus 4-byte
offset) */
+ str r0, [sp] /* Save the xcp address */
Review comment:
"str r0, [sp, #-4]!"
##########
File path: arch/arm/src/armv8-m/arm_exception.S
##########
@@ -213,6 +213,7 @@ exception_common:
*/
setintstack r2, r3
+ sub sp, #8 /* Adjust SP
(minus 8-byte offset) */
Review comment:
why sub 8?
##########
File path: arch/arm/src/armv7-a/arm_assert.c
##########
@@ -265,24 +265,10 @@ static void up_dumpstate(void)
if (sp > istackbase - istacksize && sp < istackbase)
{
- uint32_t *stackbase;
Review comment:
should we remove -4 at line 255?
##########
File path: arch/arm/src/armv7-m/gnu/arm_exception.S
##########
@@ -197,7 +197,8 @@ exception_common:
* here prohibits nested interrupts without some additional logic!
*/
- setintstack r2, r3
+ setintstack r2, r3 /* SP = IRQ stack top */
+ sub sp, #8 /* Adjust SP
(minus 8-byte offset) */
Review comment:
why sub 8?
##########
File path: arch/arm/src/armv7-m/gnu/arm_exception.S
##########
@@ -321,9 +322,9 @@ exception_common:
.bss
.global g_intstackalloc
.global g_intstackbase
- .align 8
+ .balign 8
g_intstackalloc:
- .skip ((CONFIG_ARCH_INTERRUPTSTACK + 4) & ~7)
+ .skip (CONFIG_ARCH_INTERRUPTSTACK & ~7)
Review comment:
the alignment need sync with the code in stack setup and dump. for
example here is line 70:
```
# if defined(CONFIG_BUILD_PROTECTED) && CONFIG_ARCH_INTERRUPTSTACK < 8
```
if we select the align up, the above logic is wrong and should change to
CONFIG_ARCH_INTERRUPTSTACK < 1.
So, I would like to keep the logic as simple as possible instead bringing up
more inconsistentence like this. The round down logic is more suitable choice
because I don't think there is much difference betwen 2000 and 2008 if user set
CONFIG_ARCH_INTERRUPTSTACK to 2006. The consistent round strategy across all
related code is more important.
##########
File path: arch/arm/src/armv7-r/arm_vectors.S
##########
@@ -890,9 +891,10 @@ arm_vectorfiq:
mov fp, #0 /* Init frame
pointer */
mov r0, sp /* Get r0=xcp */
-#if CONFIG_ARCH_INTERRUPTSTACK > 3
+#if CONFIG_ARCH_INTERRUPTSTACK > 7
ldr sp, .Lfiqstackbase /* SP = interrupt stack
base */
- str r0, [sp] /* Save the
user stack pointer */
+ sub sp, #8 /* Adjust SP
(minus 8-byte offset) */
+ str r0, [sp] /* Save the xcp
address */
Review comment:
merge
##########
File path: arch/arm/src/armv7-r/arm_vectors.S
##########
@@ -179,9 +179,10 @@ arm_vectorirq:
mov fp, #0 /* Init frame
pointer */
mov r0, sp /* Get r0=xcp */
-#if CONFIG_ARCH_INTERRUPTSTACK > 3
+#if CONFIG_ARCH_INTERRUPTSTACK > 7
ldr sp, .Lirqstackbase /* SP = interrupt stack
base */
- str r0, [sp] /* Save the
user stack pointer */
+ sub sp, #8 /* Adjust SP
(minus 8-byte offset) */
+ str r0, [sp] /* Save the xcp
address */
Review comment:
merge into one instruction like armv7-a
##########
File path: arch/arm/src/armv7-m/gnu/arm_lazyexception.S
##########
@@ -192,7 +192,8 @@ exception_common:
* here prohibits nested interrupts without some additional logic!
*/
- setintstack r2, r3
+ setintstack r2, r3 /* SP = IRQ stack top */
+ sub sp, #8 /* Adjust SP
(minus 8-byte offset) */
Review comment:
why need sub 8 here?
##########
File path: arch/arm/src/armv7-a/arm_vectors.S
##########
@@ -261,7 +261,8 @@ arm_vectorirq:
/* Call arm_decodeirq() on the interrupt stack */
setirqstack r1, r3 /* SP = IRQ stack top */
- str r0, [sp] /* Save the
user stack pointer */
+ sub sp, #8 /* Adjust SP
(minus 8-byte offset) */
+ str r0, [sp] /* Save the xcp
address */
Review comment:
merge to "str r0, [sp + #-8]!"
##########
File path: arch/arm/src/armv6-m/arm_exception.S
##########
@@ -153,6 +153,7 @@ exception_common:
#if CONFIG_ARCH_INTERRUPTSTACK > 3
ldr r7, =g_intstackbase /* R7=Base of the
interrupt stack */
+ sub r7, #4 /* Adjust R7
(minus 4-byte offset) */
Review comment:
don't need, since push {r1} at line 158 equal:
sub sp, #4
str r1, [sp]
##########
File path: arch/arm/src/c5471/c5471_vectors.S
##########
@@ -162,7 +162,8 @@ arm_vectorirq:
#if CONFIG_ARCH_INTERRUPTSTACK > 3
ldr sp, .Lirqstackbase /* SP = interrupt stack base */
- str r1, [sp] /* Save the user stack pointer */
+ sub sp, #4 /* Adjust SP (minus 4-byte offset) */
+ str r1, [sp] /* Save the xcp address */
Review comment:
same as line 136
##########
File path: arch/arm/src/armv7-a/arm_vectors.S
##########
@@ -1004,7 +1005,8 @@ arm_vectorfiq:
#if CONFIG_ARCH_INTERRUPTSTACK > 7
setfiqstack r1, r4 /* SP = FIQ stack top */
- str r0, [sp] /* Save the
user stack pointer */
+ sub sp, #8 /* Adjust SP
(minus 8-byte offset) */
+ str r0, [sp] /* Save the xcp
address */
Review comment:
same comment as line 265
##########
File path: arch/arm/src/armv8-m/arm_lazyexception.S
##########
@@ -210,6 +210,7 @@ exception_common:
*/
setintstack r2, r3
+ sub sp, #8 /* Adjust SP
(minus 8-byte offset) */
Review comment:
why sub 8?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
