tests: arm thread swap: increase Idle Stack size for no-opt test-case

[ioannisg]

Executing the ARM thread swap test with NO_OPTIMIZATIONS
option set, leads to Idle thread stack overflow in certain
platforms. We increase the size of the Idle thread stack to
address this.

Signed-off-by: Ioannis Glaropoulos [email protected]

[ioannisg]

Here's the log on nRF52811_pca10056:

***** Booting Zephyr OS build zephyr-v2.0.0-633-g679130c86d95 *****
Running test suite arm_thread_swap
===================================================================
starting test - test_arm_thread_swap
PASS - test_arm_thread_swap
===================================================================
Test suite arm_thread_swap succeeded
===================================================================
PROJECT EXECUTION SUCCESSFUL
[00:00:00.041,290] <err> os: ***** MPU FAULT *****
[00:00:00.047,332] <err> os:   Stacking error (context area might be not valid)
[00:00:00.056,121] <err> os:   Data Access Violation
[00:00:00.062,377] <err> os:   MMFAR Address: 0x20001c1c
[00:00:00.069,000] <err> os: r0/a1:  0x20000f20  r1/a2:  0x00007d7d  r2/a3:  0xe6ddf62d
[00:00:00.078,582] <err> os: r3/a4:  0xa1818942 r12/ip:  0xd966ffbb r14/lr:  0x4e04813c
[00:00:00.088,165] <err> os:  xpsr:  0x08888559
[00:00:00.093,933] <err> os: Faulting instruction address (r15/pc): 0xdcfe6dd4
[00:00:00.102,661] <err> os: >>> ZEPHYR FATAL ERROR 2: Stack overflow
[00:00:00.110,534] <err> os: Current thread: 0x200003cc (idle)
[00:00:00.117,736] <err> os: Halting system

[pabigot]

Ugh. OK, so this works for the test. What about applications that might select that option, or future use in other tests?

One path would be to make it like KOBJECT_TEXT_AREA and have a conditional default in Kconfig. But since there are already five different defaults, doing that would be messy. It'd also be ignored for the boards that override the default in defconfig.

Another path is to modify kernel/init.c to conditionally add an architecture-specific increment at the point the local IDLE_STACK_SIZE is defined, if CONFIG_NO_OPTIMIZATIONS has been selected. That seems a little more robust.

I'm not loving either of those, but the second does have some appeal if there's any reasonable argument that somebody might select CONFIG_NO_OPTIMIZATIONS outside of this test (or coverage, which probably can't be run on real target hardware).

Thoughts?

[ioannisg]

Thanks for looking at this @pabigot !

Ugh. OK, so this works for the test. What about applications that might select that option, or future use in other tests?

Right. So currently, arm_thread_swap is the only (?) test that runs a test-case with NO_OPTIMIZATIONS. So that's why I didn't promote, initially, a more generic solution.

One path would be to make it like KOBJECT_TEXT_AREA and have a conditional default in Kconfig. But since there are already five different defaults, doing that would be messy. It'd also be ignored for the boards that override the default in defconfig.

This could be a solution, yes.
Currently, the IDLE Stack Size is a total mess:

config IDLE_STACK_SIZE
	int "Size of stack for idle thread"
	default 2048 if COVERAGE_GCOV
	default 1024 if XTENSA
	default 512 if RISCV
	default 320 if ARC || (ARM && CPU_HAS_FPU)
	default 256

because, some of the conditionals are ARCHEs, and some are not (e.g. COVERAGE_GCOV)
We can definitely add a line, like default 512 if ARM && NO_OPTIMIZATIONS
But then, I think this may solve the problem for ARM but does nothing for the other ARCHes, which may have the same issue.

Another path is to modify kernel/init.c to conditionally add an architecture-specific increment at the point the local IDLE_STACK_SIZE is defined, if CONFIG_NO_OPTIMIZATIONS has been selected. That seems a little more robust.

This would require an investigation of what the extra space would be for each ARCH.
But: isn't this similar to re-defining the IDLE_STACK_SIZE Kconfig option in the ARCH Kconfigs? (So we can avoid touching C code)

[pabigot]

Ugh. OK, so this works for the test. What about applications that might select that option, or future use in other tests?

Right. So currently, arm_thread_swap is the only (?) test that runs a test-case with NO_OPTIMIZATIONS. So that's why I didn't promote, initially, a more generic solution.

I'm less concerned with other tests than I am with users who choose to enable that option in order to do debugging/diagnostics for problems obscured by optimization. Being blocked by the fact that a swap then corrupts stack would add a big distractor to the task.

Another path is to modify kernel/init.c to conditionally add an architecture-specific increment at the point the local IDLE_STACK_SIZE is defined, if CONFIG_NO_OPTIMIZATIONS has been selected. That seems a little more robust.

This would require an investigation of what the extra space would be for each ARCH.
But: isn't this similar to re-defining the IDLE_STACK_SIZE Kconfig option in the ARCH Kconfigs? (So we can avoid touching C code)

Yes, but in Kconfig we'd have to do a per-architecture approach. We could simply assume that adding 256 or 320 to the architecture-specific offset in init.c is enough, until we have reason to believe it isn't.

I don't have strong feelings about this, so I'm gonna go ahead and approve it as is. If/when we run into the same problem again we should make a more serious effort to find a general solution.

[ioannisg]

For me, CONFIG_NO_OPTIMIZATIONS is something the developer will set if they really know what they are doing. And they should accept that this may require re-adjusting the core thread stacks (main, idle, ztest, etc.)

but i think I agree with your last comment, @pabigot - let's fix this for this sole test, now, and if we see in the near future that we run into similar problems, we can standardize the solution.