Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARM: initialization sequence might be not using all of interrupt stack #16210

Closed
ioannisg opened this issue May 16, 2019 · 10 comments · Fixed by #27343
Closed

ARM: initialization sequence might be not using all of interrupt stack #16210

ioannisg opened this issue May 16, 2019 · 10 comments · Fixed by #27343
Assignees
Labels
area: ARM ARM (32-bit) Architecture area: Memory Protection Enhancement Changes/Updates/Additions to existing features priority: low Low impact/importance bug
Milestone

Comments

@ioannisg
Copy link
Member

The interrupt stack is defined using the K_THREAD_STACK_DEFINE(<requested size>) kernel macro. In some architectures, for example the ARM architecture without requirement for stack start alignment, the size of the stack, defined with K_THREAD_STACK_DEFINE() is larger than <requested size> due to possibly a stack guard.

First, of, all, this is not yet needed - we do not support MPU-based stack-overflow protection in the ISR stack at the moment.
Then, the possible guard area is not taken into account in reset.S, so we do not use this area we have reserved for the interrupt stack. It's a (minor) memory wasting.

There are some straightforward ways to fix this issue;

  • don't use K_THREAD_STACK_DEFINE for the ISR stack
  • account for the actual size of the interrupt stack in reset.S
  • ....
@ioannisg ioannisg added bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug labels May 16, 2019
@ioannisg ioannisg changed the title ARM: ARM: wasting memory in interrupt stack May 16, 2019
@ioannisg
Copy link
Member Author

The reset.S code in the other ARCHes is pretty similar, but we won't see a bug as long as K_THREAD_STACK_DEFINE does not reserve any extra space besides what is requested by CONFIG_ISR_STACK_SIZE

@ioannisg
Copy link
Member Author

FYI @andrewboie

@andrewboie
Copy link
Contributor

Affects x86 as well since privilege mode stacks are part of the array created by K_THREAD_STACK_DEFINE(), we could save 4K.

Need to check on ARM but likely wasting both guard area and privileged stack.

@ioannisg
Copy link
Member Author

Don't know how much of an issue it is (might only affect boot code). But it's certainly something we might want to work on

@ioannisg
Copy link
Member Author

@andrewboie I was thinking to pick this up. Isn't the solution as simple as not using KThread Stack Define macro to define the interrupt stack?

@andrewboie
Copy link
Contributor

andrewboie commented Oct 18, 2019

@ioannisg I think so, but I think that #13637 will also fix it nicely, to the point where I suspect that this ticket could be treated as a duplicate. I'm going to get that done for 2.1

@bbolen
Copy link
Collaborator

bbolen commented Mar 5, 2020

Adding information from #23268 per request.

Describe the bug
When building with userspace enabled, gen_priv_stacks.py generates privileged stacks for system stacks that do not need it. For example, on Cortex-M platforms, there is no need for a privileged shadow stack of _interrupt_stack, z_main_stack, or z_idle_stack.

To Reproduce
Using recent code as of g219d9fc0829
west build -p auto --board=stm32f411e_disco samples/userspace/prod_consumer

~/zephyr-sdk/arm-zephyr-eabi/bin/arm-zephyr-eabi-nm build/zephyr/zephyr.elf |grep -e "priv_stack_[0-9]+"
20004ea0 b priv_stack_20001800
20004aa0 b priv_stack_20002000
200046a0 b priv_stack_20002800
200042a0 b priv_stack_20003000
20003ea0 b priv_stack_20003400

~/zephyr-sdk/arm-zephyr-eabi/bin/arm-zephyr-eabi-nm build/zephyr/zephyr.elf |grep -e _interrupt_stack -e z_main_stack -e z_idle_stack
20002800 B _interrupt_stack
20003400 B z_idle_stack
20003000 B z_main_stack

@ioannisg
Copy link
Member Author

This should not be an issue, any more, since we merged #24714 , and so the interrupt stack is defined using k_kernel_stack_define.
I need to see if there is a still a minor issue with the mpu guard.

@ioannisg
Copy link
Member Author

So, the only remaining issue, here, is the fact that during initialization not all interrupt stack area gets used by the boot process. After boot, the interrupt stack gets initialized to the upper boundary, anyway.
Considering this a minor enhancement.

@ioannisg ioannisg added Enhancement Changes/Updates/Additions to existing features and removed bug The issue is a bug, or the PR is fixing a bug labels Jul 31, 2020
@ioannisg ioannisg changed the title ARM: wasting memory in interrupt stack ARM: initialization sequence might be not using all of interrupt stack Jul 31, 2020
@ioannisg ioannisg added this to the v2.4.0 milestone Jul 31, 2020
@andrewboie
Copy link
Contributor

So, the only remaining issue, here, is the fact that during initialization not all interrupt stack area gets used by the boot process

Hmm, do we even need to fix it? We need the space anyway during normal kernel execution right? Suggest closing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARM ARM (32-bit) Architecture area: Memory Protection Enhancement Changes/Updates/Additions to existing features priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants