masayuki2009 commented on a change in pull request #2042: URL: https://github.com/apache/incubator-nuttx/pull/2042#discussion_r508901781
##########
File path: arch/arm/src/cxd56xx/cxd56_irq.c
##########
@@ -115,17 +112,17 @@ static uint64_t g_intstack_alloc[INTSTACK_ALLOC >> 3];
const uint32_t g_cpu_intstack_top[CONFIG_SMP_NCPUS] =
{
- (uint32_t)g_intstack_alloc + INTSTACK_SIZE,
+ (uint32_t)g_intstack_alloc + INTSTACK_SIZE - 8,
Review comment:
>so the pointer should point to after the end of stack. -8 make the code
complexity and waste a little memory, or is >there a special reason to do in
this way, @patacongo? I have worked with many RTOS and bootloader, but never
see >the type of special code.
@xiaoxiang781216
Thanks for your comments.
The situation is the same for non-SMP case as well.
The following code shows arch/arm/src/armv7-a/arm_vectors.S
but similar code exists for other ARM architectures.
As you can see and yes as you pointed out,
$sp is not subtracted in setirqstack then save a value to the stack.
```
...
if CONFIG_ARCH_INTERRUPTSTACK > 7
#ifndef CONFIG_SMP
.Lirqstackbase:
.word g_intstackbase
#endif
...
if !defined(CONFIG_SMP) && CONFIG_ARCH_INTERRUPTSTACK > 7
.macro setirqstack, tmp1, tmp2
ldr sp, .Lirqstackbase /* SP = IRQ stack top */
.endm
#endif
...
/* Call arm_decodeirq() on the interrupt stack */
setirqstack r1, r3 /* SP = IRQ stack top */
str r0, [sp] /* Save the user stack pointer */
mov r4, sp /* Save the SP in a preserved register
*/
bic sp, sp, #7 /* Force 8-byte alignment */
bl arm_decodeirq /* Call the handler */
...
```
Rather, in this caes g_intstackbase is subtracted explicitly (i.e. -4 but I
think this should be -8) here for non-SMP case.
```
...
g_intstackalloc:
.skip ((CONFIG_ARCH_INTERRUPTSTACK & ~7) - 4)
g_intstackbase:
.skip 4
.size g_intstackbase, 4
...
```
I agree that this kind of code is complicated, so should be more simple.
However, I think this should done as another task to refactor the code,
because the impact would be bigger than the current PR.
What do you think?
----------------------------------------------------------------
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
