This is super confusing to me.

The todo list says.

        Status:      Open
          Priority:    Low.  Things are working OK the way they are.  But the
design
                   could be improved and made a little more efficient with this
                       change.

So it sounds like no big deal. Hardfaulting on  set_errno() it is a big
deal.


Did we destroy active acrhs with a partial implementation and not realize
it?

Why was this work not done on a branch and tested?

Where is the process? Where is the PMC agreeing on these sweeping changes?

David



-----Original Message-----
From: Xiang Xiao [mailto:xiaoxiang781...@gmail.com]
Sent: Friday, May 22, 2020 4:51 AM
To: dev@nuttx.apache.org
Subject: Re: Arm Kinetis: set_errno in nx_start with CONFIG_TLS_ALIGNED not
set

The new implementation of set_errno require all thread(include IDLE)
initialize the stack related fields in tcb_s correctly. But many
chipset forget to setup them for IDLE thread. TODO contain an item
described this:
  Task:        IDLE THREAD TCB SETUP
  Description: There are issues with setting IDLE thread stacks:

               1. One problem is stack-related data in the IDLE threads TCB.
                  A solution might be to standardize the use of
g_idle_topstack.
                  That you could add initialization like this in nx_start:

                  @@ -344,6 +347,11 @@ void nx_start(void)
                     g_idleargv[1]  = NULL;
                     g_idletcb.argv = g_idleargv;

                  +  /* Set the IDLE task stack size */
                  +
                  +  g_idletcb.cmn.adj_stack_size =
CONFIG_IDLETHREAD_STACKSIZE;
                  +  g_idletcb.cmn.stack_alloc_ptr = (void
*)(g_idle_topstack - CONFIG_IDLETHREAD_STACKSIZE);
                  +
                     /* Then add the idle task's TCB to the head of
the ready to run list */

                     dq_addfirst((FAR dq_entry_t *)&g_idletcb, (FAR
dq_queue_t *)&g_readytorun);

                  The g_idle_topstack variable is available for almost
all architectures:

                  $ find . -name *.h | xargs grep g_idle_top
                  ./arm/src/common/up_internal.h:EXTERN const uint32_t
g_idle_topstack;
                  ./avr/src/avr/avr.h:extern uint16_t g_idle_topstack;
                  ./avr/src/avr32/avr32.h:extern uint32_t g_idle_topstack;
                  ./hc/src/common/up_internal.h:extern uint16_t
g_idle_topstack;
                  ./mips/src/common/up_internal.h:extern uint32_t
g_idle_topstack;
                  ./misoc/src/lm32/lm32.h:extern uint32_t g_idle_topstack;
                  ./renesas/src/common/up_internal.h:extern uint32_t
g_idle_topstack;
                  ./renesas/src/m16c/chip.h:extern uint32_t
g_idle_topstack; /* Start of the heap */
                  ./risc-v/src/common/up_internal.h:EXTERN uint32_t
g_idle_topstack;
                  ./x86/src/common/up_internal.h:extern uint32_t
g_idle_topstack;

                  That omits these architectures: sh1, sim, xtensa, z16,
z80,
                  ez80, and z8.  All would have to support this common
                  global variable.

                  Also, the stack itself may be 8-, 16-, or 32-bits wide,
                  depending upon the architecture and do have differing
                  alignment requirements.

               2. Another problem is colorizing that stack to use with
                 stack usage monitoring logic.  There is logic in some
                 start functions to do this in a function called
go_nx_start.
                 It is available in these architectures:

                 ./arm/src/efm32/efm32_start.c:static void
go_nx_start(void *pv, unsigned int nbytes)
                 ./arm/src/kinetis/kinetis_start.c:static void
go_nx_start(void *pv, unsigned int nbytes)
                 ./arm/src/sam34/sam_start.c:static void
go_nx_start(void *pv, unsigned int nbytes)
                 ./arm/src/samv7/sam_start.c:static void
go_nx_start(void *pv, unsigned int nbytes)
                 ./arm/src/stm32/stm32_start.c:static void
go_nx_start(void *pv, unsigned int nbytes)
                 ./arm/src/stm32f7/stm32_start.c:static void
go_nx_start(void *pv, unsigned int nbytes)
                 ./arm/src/stm32l4/stm32l4_start.c:static void
go_nx_start(void *pv, unsigned int nbytes)
                 ./arm/src/tms570/tms570_boot.c:static void
go_nx_start(void *pv, unsigned int nbytes)
                 ./arm/src/xmc4/xmc4_start.c:static void
go_nx_start(void *pv, unsigned int nbytes)
So, it become a block issue and need immedately fix now.

On Fri, May 22, 2020 at 7:40 PM Schock, Johannes - NIVUS GmbH
<johannes.sch...@nivus.com> wrote:
>
> Hello,
> I think there's a problem with set_errno() in nx_start(), when
> CONFIG_TLS_ALIGNED is not set:
> I disabled SERIAL_CONSOLE and DEV_CONSOLE, because I wanted to test
> CDCACM_CONSOLE. Now calling fs_fdopen() in group_setupstreams() leads to a
> call of set_errno().
> But since tls_get_info() returns NULL pointer for stack_alloc_ptr the
> board goes into hardfault.
> For the CONFIG_TLS_ALIGNED case it would return the actual stack pointer
> from arm_getsp() which accesses the stack pointer directly.
> I think thread local storage for nx_start() should be handled differently,
> or the stack_alloc_ptr should be set to a sane value.
> I attach my config for reference, perhaps I'm missing something.
>
> Regards, Johannes
>
>

Reply via email to