option 3 looks good to me: +1 Best Łukasz
On Mon, 25 Nov 2019 at 21:11, Jerzy Kasenberg <jerzy.kasenb...@codecoup.pl> wrote: > > HI, > > I vote for option 3 > > My reason, apart what Andrzej mentioned (usefulness and debugging), > is that there is a notorious coverity warning popping up about usage of > address &t_tasktop[stack_size]. > > best regards > Jerzy > > pon., 25 lis 2019 o 16:28 Andrzej Kaczmarek <andrzej.kaczma...@codecoup.pl> > napisał(a): > > > Hi, > > > > I recently created a PR which adds support for hardware stack limit > > checking on Cortex-M33. It was already approved by few people but > > apparently there are different opinions of how this should be implemented > > so I would like to get more opinions on this since we likely will have more > > MCUs with this feature soon, so consider this as a voting. > > > > PR: https://github.com/apache/mynewt-core/pull/2108 > > > > Here are proposed ways to do this: > > 1. use stack top and stack size to calculate PSPLIM on every context switch > > this does not require any modification in os_task structure and adds 4 > > instructions on each context switch > > 2. add stack bottom to os_task > > this extends os_task struct by 4 bytes and adds 2 instructions on each > > context switch > > 3. change stack top to stack bottom in os_task > > this does not change the size of os_task struct and adds 2 instructions on > > each context switch, however it may break code which accesses os_task > > structure directly (like mcumgr does - it should be modified to use proper > > API anyway) > > > > My personal preference is option 3 since it is more useful to have stack > > bottom in os_task struct instead of stack top. For example, stack overflow > > or usage is determined using stack bottom. Also when debugging it's easier > > to inspect stack by using stack bottom rather than stack top. In general, > > seems like you only need stack top when initializing it but this is done > > only once (even os_task_init has 'stack_bottom' parameter, not > > 'stack_top'). > > > > Best, > > Andrzej > >
