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

Master f7 stackcheck - result update stacks #11563

Merged
merged 3 commits into from
Mar 2, 2019
Merged

Conversation

davids5
Copy link
Member

@davids5 davids5 commented Feb 28, 2019

Fixes #11554 (comment) with SYS_AUTOSTART=4001

The cause of the stack detection fault is because of the
level of nesting in the start up script. We need to
determine the worst case configuration and set the
bar there.

This fault occurred some 42 calls deep due to script
calling script (repeat).

The HW stack check requires as a margin of 204 bytes. That is
ISR HW stacking of CPU(8) FPU(18) registers and SW stacking of
CPU(11) and FPU(16) registers. Total CPU(19) registers is
68 bytes and the total FPU(34) registers is 136 bytes. On
a system with a separate ISR stack This only needs to be 104
so there is 100 bytes of headroom. But as coded the detection
will give a false positive detection and fault. This does not
mean that the stack will be corrupted.

Adjustments to that stack can have no effect due to rounding.
A stack size of 2608 and 2616 can yield the exact same size stack.
So even when the failure is due to a 4 byte overflow, it can take
greater than a 16 bytes increase to fix it. Because the final
stack size is calculated with an 8 byte alignment after a 4 byte
decrease. So 2624 becomes 2620 at runtime and will boot
with SYS_AUTOSTART=4001.

David Sidrane added 3 commits February 28, 2019 12:48
   The cause of the stack detection fault is because of the
   level of nesting in the start up script. We need to
   determine the worst case configuration and set the
   bar there.

   This fault occurred some 42 calls deep due to script
   calling script (repeat).

   The HW stack check requires as a margin of 204 bytes. That is
   ISR HW stacking of CPU(8) FPU(18) registers and SW stacking of
   CPU(11) and FPU(16) registers. Total CPU(19) registers is
   68 bytes and the total FPU(34) registers is 136 bytes.  On
   a system with a separate ISR stack This only needs to be 104
   so there is 100 bytes of headroom. But as coded the detection
   will give a false positive detection and fault. This does not
   mean that the stack will be corrupted.

   Adjustments to that stack can have no effect due to rounding.
   A stack size of 2608 and 2616 can yield the exact same size stack.
   So even when the failure is due to a 4 byte overflow, it can take
   greater than a 16 bytes increase to fix it. Because the final
   stack size is calculated with an 8 byte alignment after a 4 byte
   decrease. So 2624 becomes 2620 at runtime and will boot
   with SYS_AUTOSTART=4001.
@davids5 davids5 force-pushed the master_f7_stackcheck branch from d0030ea to 38b6336 Compare February 28, 2019 20:49
@davids5
Copy link
Member Author

davids5 commented Feb 28, 2019

I am hoping to get this in upstream:
https://bitbucket.org/nuttx/nuttx/pull-requests/834/armv7-m-stackcheck-allow-faulting-stack/diff

image

Subtract R11 from R10 and add 12. Then that number to the failed stack size.

@davids5
Copy link
Member Author

davids5 commented Feb 28, 2019

@davids5
Copy link
Member Author

davids5 commented Mar 1, 2019

@dagar Doe it make sense to take one more drink from upstream on #11537 to get the stack info in the up_assert display?

@dagar
Copy link
Member

dagar commented Mar 2, 2019

@dagar Doe it make sense to take one more drink from upstream on #11537 to get the stack info in the up_assert display?

Yes I'll update #11537 again.

@dagar dagar merged commit dc50a56 into master Mar 2, 2019
@dagar dagar deleted the master_f7_stackcheck branch March 2, 2019 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants