[PATCH] cpukit/smp: Prevent premature thread dispatch

[Kinsey Moore]

On 9/17/2021 00:39, Sebastian Huber wrote:
> On 16/09/2021 22:50, Kinsey Moore wrote:
>> There is currently a narrow window between the CPU state being set to UP
>> and the dispatch disable flag being set. It is possible to perform a
>> cross-processor thread dispatch in this window which catches the CPU in
>> a state which is not actually fully ready for that type of thread
>> dispatch.
>>
>> This moves the CPU state change to just before the CPU's first thread
>> dispatch and later than the dispatch disable flag change which closes
>> the window for the race condition.
>> ---
>>   cpukit/score/src/threadstartmultitasking.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/cpukit/score/src/threadstartmultitasking.c 
>> b/cpukit/score/src/threadstartmultitasking.c
>> index 094a535394..0c0951a243 100644
>> --- a/cpukit/score/src/threadstartmultitasking.c
>> +++ b/cpukit/score/src/threadstartmultitasking.c
>> @@ -29,8 +29,6 @@ void _Thread_Start_multitasking( void )
>>     Thread_Control  *heir;
>>     #if defined(RTEMS_SMP)
>> -  _Per_CPU_State_change( cpu_self, PER_CPU_STATE_UP );
>> -
>>     /*
>>      * Threads begin execution in the _Thread_Handler() function.   This
>>      * function will set the thread dispatch disable level to zero.
>> @@ -44,6 +42,8 @@ void _Thread_Start_multitasking( void )
>>     #if defined(RTEMS_SMP)
>>     _CPU_SMP_Prepare_start_multitasking();
>> +
>> +  _Per_CPU_State_change( cpu_self, PER_CPU_STATE_UP );
>>   #endif
>>     #if defined(_CPU_Start_multitasking)
>>
>
> For which branch is this patch? The code on the master is different.
I mistakenly sent this patch from the wrong branch.
>
> Which scenario caused a problem here? I guess there is a second bug 
> involved, since maskable interrupt should be disabled for this code 
> path. How could there be a race condition?
>
You asked the right questions and you are correct. This was caused by 
interrupts not being disabled when they should be. I thought I had read 
in the ARMv8 spec that interrupts are disabled on startup, but that is 
obviously not the case and I was misremembering information about 
interrupt status on exception. Fixing the interrupt startup status also 
resolved some other test failures I was seeing.

Thanks,

Kinsey