[PATCH 3/3] score: Add SMP support to the cache manager

[Daniel Cederman]

Thank you for you comments Sebastian!

 > With this implementation cache routines must not be called from
 > interrupt context.  This should be mentioned in the documentation.
 >
 > It is extremely difficult to implement it in a way so that it can be
 > used from interrupt context.

There are some synchronization issues in the code that we need to take 
care of. For example if the task gets swapped out while holding the lock 
and gets called again on the same cpu. We need to do some rewriting of 
the code.

As there were no comments on the use of function pointers I will use 
them instead of function ids.

Daniel Cederman
Software Engineer
Aeroflex Gaisler AB
Aeroflex Microelectronic Solutions – HiRel
Kungsgatan 12
SE-411 19 Gothenburg, Sweden
Phone: +46 31 7758665
cederman at gaisler.com
www.Aeroflex.com/Gaisler

On 2014-07-04 08:38, Sebastian Huber wrote:
> On 2014-07-03 11:37, Daniel Cederman wrote:
>> Adds functions that allows the user to specify which cores that should
>> perform the cache operation. SMP messages are sent to all the specified
>> cores and the caller waits until all cores have acknowledged that they
>> have flushed their cache. Implementation is shown using both function
>> pointers and function ids together with a switch statement. One needs
>> to be dropped before committing. Any preference?
>> ---
>>   c/src/lib/libcpu/shared/src/cache_manager.c |  166
>> +++++++++++++++++++++++++++
>>   cpukit/rtems/include/rtems/rtems/cache.h    |   84 ++++++++++++++
>>   cpukit/score/include/rtems/score/smpimpl.h  |   13 +++
>>   3 files changed, 263 insertions(+)
>>
>> diff --git a/c/src/lib/libcpu/shared/src/cache_manager.c
>> b/c/src/lib/libcpu/shared/src/cache_manager.c
>> index 420a013..91e9f72 100644
>> --- a/c/src/lib/libcpu/shared/src/cache_manager.c
>> +++ b/c/src/lib/libcpu/shared/src/cache_manager.c
>> @@ -37,6 +37,172 @@
>>
>>   #include <rtems.h>
>>   #include "cache_.h"
>> +#include <rtems/score/smplock.h>
>> +#include <rtems/score/smpimpl.h>
>> +
>> +#if defined( RTEMS_SMP )
>> +
>> +typedef void (*cache_manager_func_t)(const void * d_addr, size_t
>> n_bytes);
>
> The *_t namespace is reserved by POSIX.  The names don't follow the
> score naming conventions.
>
> http://www.rtems.org/wiki/index.php/Naming_rules
>
>> +
>> +typedef enum {
>> +  FLUSH_MULTIPLE_DATA_LINES,
>> +  INVALIDATE_MULTIPLE_DATA_LINES,
>> +  INVALIDATE_MULTIPLE_INSTRUCTION_LINES,
>> +  FLUSH_ENTIRE_DATA,
>> +  INVALIDATE_ENTIRE_INSTRUCTION,
>> +  INVALIDATE_ENTIRE_DATA
>> +} cache_manager_func_id_t;
>> +
>> +typedef struct {
>> +  SMP_lock_Control lock;
>> +  cache_manager_func_t func;
>> +  cache_manager_func_id_t func_id;
>> +  Atomic_Uint count;
>> +  const void *addr;
>> +  size_t size;
>> +} Cache_Manager_SMP;
>> +
>> +static Cache_Manager_SMP _CM_SMP = {
>> +  .lock = SMP_LOCK_INITIALIZER("CacheMgr"),
>> +  .count = CPU_ATOMIC_INITIALIZER_UINT(0)
>> +};
>> +
>> +void
>> +_SMP_Cache_manager_ipi_handler(void)
>
> I would rather name this _SMP_Cache_manager_message_handler().
>
>> +{
>> +#ifdef USE_FUNCPTR
>> +  _CM_SMP.func( _CM_SMP.addr, _CM_SMP.size );
>> +#else
>> +  switch( _CM_SMP.func_id ) {
>> +    case FLUSH_MULTIPLE_DATA_LINES:
>> +      rtems_cache_flush_multiple_data_lines(
>> +          _CM_SMP.addr, _CM_SMP.size );
>> +      break;
>> +    case INVALIDATE_MULTIPLE_DATA_LINES:
>> +      rtems_cache_invalidate_multiple_data_lines(
>> +          _CM_SMP.addr, _CM_SMP.size );
>> +      break;
>> +    case INVALIDATE_MULTIPLE_INSTRUCTION_LINES:
>> +      rtems_cache_invalidate_multiple_instruction_lines(
>> +          _CM_SMP.addr, _CM_SMP.size );
>> +      break;
>> +    case FLUSH_ENTIRE_DATA:
>> +      rtems_cache_flush_entire_data();
>> +      break;
>> +    case INVALIDATE_ENTIRE_INSTRUCTION:
>> +      rtems_cache_invalidate_entire_instruction();
>> +      break;
>> +    case INVALIDATE_ENTIRE_DATA:
>> +      rtems_cache_invalidate_entire_data();
>> +      break;
>> +    default:
>> +      _Assert( 0 );
>> +      break;
>> +  }
>> +#endif
>> +
>> +  _Atomic_Fetch_sub_uint( &_CM_SMP.count, 1, ATOMIC_ORDER_RELEASE );
>> +}
>> +
>> +static void
>> +_cache_manager_send_ipi_msg( const cpu_set_t *set,
>> cache_manager_func_t func,
>> +    cache_manager_func_id_t func_id, const void * addr, size_t size )
>> +{
>> +  uint32_t i;
>> +  uint32_t set_size = 0;
>> +  SMP_lock_Context lock_context;
>> +
>> +  _Assert( _System_state_Is_up( _System_state_Get() ) );
>> +
>> +  for( i=0; i < _SMP_Get_processor_count(); ++i ) {
>> +    set_size += CPU_ISSET(i, set);
>> +  }
>> +
>> +  _SMP_lock_Acquire( &_CM_SMP.lock, &lock_context );
>
> With this implementation cache routines must not be called from
> interrupt context.  This should be mentioned in the documentation.
>
> It is extremely difficult to implement it in a way so that it can be
> used from interrupt context.
>
>> +
>> +  _CM_SMP.func = func;
>> +  _CM_SMP.func_id = func_id;
>> +  _CM_SMP.addr = addr;
>> +  _CM_SMP.size = size;
>> +  _Atomic_Store_uint( &_CM_SMP.count, set_size, ATOMIC_ORDER_RELEASE );
>> +  _Atomic_Fence( ATOMIC_ORDER_RELEASE );
>> +
>> +  _SMP_Send_message_cpu_set( set, SMP_MESSAGE_CACHE_MANAGER );
>> +
>> +  while(_Atomic_Load_uint( &_CM_SMP.count, ATOMIC_ORDER_ACQUIRE ) !=
>> 0 );
>> +
>> +  _SMP_lock_Release( &_CM_SMP.lock, &lock_context );
>> +}
>> +
>> +void
>> +rtems_cache_invalidate_entire_instruction_cpu_set ( const cpu_set_t
>> *set )
>
> This limits the API to the default cpu_set_t.  Other routines like
> pthread_setaffinity_np() don't have this limitation.
>
> On other places we don't use "cpu" instead we use "processor" it should
> be consistent in the high level RTEMS APIs.
>
> [...]
>