Concerns about the use of spinlocks in the SMP Port of the RP2040

Kernel
1

Hey there !!

As said in a former topic (and I think I’ll ask all my questions in this topic from now on, to avoid flood) I’m writing a port of the SMP Configuration for the Cortex M7. I did a lot of progress imo. The checklist here is completed. Now I’m progressing in the debug of it.

I’m at a state where I need a reference, and the RP2040 seems to be all suited, as its very accessible ! So I just ordered one : I expect to use it with a debugger to observe what is supposed to be a correct behavior. While waiting it get delivered, I’m exploring the port without testing anything. and some parts are confusing me, mainly about the spinlock.

I’ve implemented my software spinlock, and it works, OK. I used it to define the macro portRELEASE_ISR_LOCK(), portRELEASE_TASK_LOCK(), portGET_ISR_LOCK(), and portGET_TASK_LOCK() spinlock_recursiveLock(&spinlock_TASK). In port.c file of the RP2040 view, I see there is two spinlock used, defined with static spin_lock_t * pxYieldSpinLock[ configNUMBER_OF_CORES ];. I’m surprised, because, if I look at the definition of the previous macro for RP2040, the used spinlock are accessed with pxSpinLock (in portmacro.h.) So the question is : Who are pxYieldSpinLock ? why are they here ? I note that they exist and are used in conditional compilation with configSUPPORT_PICO_SYNC_INTEROP and not configNUMBER_OF_CORES, which is the macro used to use or not the SMP Configuration. Can I ignore this for my SMP port ?

Moreover, there is two other spinlock used by the RP2040 :

    BaseType_t xPortStartScheduler( void )
   {
       configASSERT( ucPrimaryCoreNum == INVALID_PRIMARY_CORE_NUM );

       /* No one else should use these! */
       spin_lock_claim( configSMP_SPINLOCK_0 );
       spin_lock_claim( configSMP_SPINLOCK_1 );

       #if portRUNNING_ON_BOTH_CORES
           ucPrimaryCoreNum = configTICK_CORE;
           configASSERT( get_core_num() == 0 ); /* we must be started on core 0 */
           multicore_launch_core1( prvDisableInterruptsAndPortStartSchedulerOnCore );
       #else
           ucPrimaryCoreNum = get_core_num();
       #endif
       xPortStartSchedulerOnCore();

       /* Should not get here! */
       return 0;
   }

First, I the function used to take them is not the macro function portGET_ISR_LOCK nor vPortRecursiveLock (which is behind the macro.) Secondly, configSMP_SPINLOCK_0 and _1 are not pxSpinLock nor pxYieldSpinLock. Finally, I don’t understand the reason why “No one else should use these!”. It confuse me a little bit, to write my own port.
I take a look at the TI AM64 port, and I don’t see any mention of a spinlocks here. Instead, I see it disable the IT, which make more sense for me. I think I must do the same !!
I’ll probably get answers when I’ll have my RP2040, but I can’t wait one week to address such an issue. And as I’m currently reworking my xPortStartScheduler and xPortStartSchedulerOnCore while waiting, so I must start to think of it.

Thanks for reading !! :smiley: :heart:

1 Like
2

@TheSemaphoreNoob
The pico-sdk also support sem, mutex, queue etc. To call these APIs within a FreeRTOS task, the RP2040 port implements a FreeRTOS awareness lock_core interface. pxYieldSpinLock is used to implement Raspberry Pi Pico lock_core interface and the spinlock is allocated in pico-sdk, not in the port implementation. This feature is enabled when configSUPPORT_PICO_SYNC_INTEROP is set to 1. You can reference Raspberry Pi Pico C SDK and the pico-sdk for more information. It is port specific implementation. You may not require it in your port implementation.

configSMP_SPINLOCK_0 is the ISR lock and configSMP_SPINLOCK_1 is the TASK lock. The message /* No one else should use these! */ tries to specify that these two hardware spinlocks are used in FreeRTOS kernel only. It should not be used in user application.

3

Wow, I wasn’t aware the Pico-SDK offers such features aside of FreeRTOS Primitive ! :smiley: I never used a raspberry before, is it proper to the Pico or common to RP ecosystem ? I received my pico WH, and I’m setting up a work environment on my free time : I’ll soon be able to explore this. :smiley: So for now, I’ll consider I don’t have to care about pxYieldSpinLock.

OK I take this, it allows a simpler representation in my mind. So It’s about the spinlocks affected by the four macro I listed. It rises two questions :

  1. Why aren’t they taken with the defined macro portGET_ISR_LOCK and portGET_TASK_LOCK ? I can’t figure out an eventual technical reason.
  2. Why are they take now / here ? The meaning of the commentary reassure me, as it’s a remark I made to myself when I implemented my software spinlock. But with my comprehension, it doesn’t prevent the user app to take them. It locks them, and marks them as owned by the core executing this function = the main core = the core handling the sys tick IT and the updates of pxCurrentTCBs. I interpret this as, it prevents the secondary core from takeing them. Does the other core not require to take these locks ?
4

@TheSemaphoreNoob

spin_lock_claim is used for a cooperative claiming. There will be system panic if the spinlock is already claimed in RP2040. portGET/RELEASE_TASK/ISR_LOCK macro should be used to take or release the spinlock.