[PATCH 3/3] score: Add SMP support to the cache manager
Joel Sherrill
joel.sherrill at oarcorp.com
Thu Jul 3 15:35:01 UTC 2014
Comments inline. But mostly the same style comments.
Check that they aren't needed in other places. I didn't
note them everywhere.
On 7/3/2014 4:37 AM, 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);
> +
> +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)
> +{
> +#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 );
If any of these fit under 80 columns, join the line. I am pretty sure
the bottom two
don't fit though.
> + 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 )
Normally one parameter per line.
And this line appears to be too long.
> +{
> + 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 );
> +
> + _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 )
> +{
> +#if defined(CPU_INSTRUCTION_CACHE_ALIGNMENT)
> + _cache_manager_send_ipi_msg( set,
> + (cache_manager_func_t)rtems_cache_invalidate_entire_instruction,
> + INVALIDATE_ENTIRE_INSTRUCTION, 0, 0 );
> +#endif
I would have put each parameter on a separate line since you are already
on multiple ones.
Gedare will likely speak up and say "where's that written?" :)
> +}
> +
> +void
> +rtems_cache_flush_multiple_data_lines_cpu_set( const void *addr,
> + size_t size,
> + const cpu_set_t *set
> +)
> +{
> +#if defined(CPU_DATA_CACHE_ALIGNMENT)
> + _cache_manager_send_ipi_msg( set, rtems_cache_flush_multiple_data_lines,
> + FLUSH_MULTIPLE_DATA_LINES, addr, size );
> +#endif
> +}
> +
> +void
> +rtems_cache_invalidate_multiple_data_lines_cpu_set(
> + const void *addr,
> + size_t size,
> + const cpu_set_t *set
> +)
> +{
> +#if defined(CPU_DATA_CACHE_ALIGNMENT)
> + _cache_manager_send_ipi_msg( set, rtems_cache_invalidate_multiple_data_lines,
> + INVALIDATE_MULTIPLE_DATA_LINES, addr, size );
> +#endif
> +}
> +
> +void
> +rtems_cache_invalidate_multiple_instruction_lines_cpu_set(
> + const void *addr,
> + size_t size,
> + const cpu_set_t *set
> +)
> +{
> +#if defined(CPU_INSTRUCTION_CACHE_ALIGNMENT)
> + _cache_manager_send_ipi_msg( set,
> + rtems_cache_invalidate_multiple_instruction_lines,
> + INVALIDATE_MULTIPLE_INSTRUCTION_LINES, addr, size );
> +#endif
> +}
> +
> +void
> +rtems_cache_flush_entire_data_cpu_set( const cpu_set_t *set )
> +{
> +#if defined(CPU_DATA_CACHE_ALIGNMENT)
> + _cache_manager_send_ipi_msg( set,
> + (cache_manager_func_t)rtems_cache_flush_entire_data,
> + FLUSH_ENTIRE_DATA, 0, 0 );
> +#endif
> +}
> +
> +void
> +rtems_cache_invalidate_entire_data_cpu_set( const cpu_set_t *set )
> +{
> +#if defined(CPU_DATA_CACHE_ALIGNMENT)
> + _cache_manager_send_ipi_msg( set,
> + (cache_manager_func_t)rtems_cache_invalidate_entire_data,
> + INVALIDATE_ENTIRE_DATA, 0, 0 );
> +#endif
> +}
> +#endif
>
> /*
> * THESE FUNCTIONS ONLY HAVE BODIES IF WE HAVE A DATA CACHE
> diff --git a/cpukit/rtems/include/rtems/rtems/cache.h b/cpukit/rtems/include/rtems/rtems/cache.h
> index 05f6612..824a8c0 100644
> --- a/cpukit/rtems/include/rtems/rtems/cache.h
> +++ b/cpukit/rtems/include/rtems/rtems/cache.h
> @@ -188,6 +188,90 @@ void rtems_cache_disable_instruction( void );
> */
> void *rtems_cache_aligned_malloc ( size_t nbytes );
>
> +#if defined( RTEMS_SMP )
> +
> +/**
> + * @brief Handles cache invalidation/flush requests from a remote CPU
> + *
> + */
> +void _SMP_Cache_manager_ipi_handler(void);
> +
> +/**
> + * @brief Invalidates the entire instruction cache for a set of CPUs
> + *
> + * @see rtems_cache_invalidate_multiple_instruction_lines().
> + */
> +void rtems_cache_invalidate_entire_instruction_cpu_set( const cpu_set_t *set);
The above looks like a couple of the lines are too long.
> +/**
> + * @brief Flushes multiple data cache lines for a set of CPUs
> + *
> + * Dirty cache lines covering the area are transfered to memory. Depending on
> + * the cache implementation this may mark the lines as invalid.
> + *
The above looks like the line is too long.
> + * @param[in] addr The start address of the area to flush.
> + * @param[in] size The size in bytes of the area to flush.
> + */
> +void rtems_cache_flush_multiple_data_lines_cpu_set( const void *addr,
Bring this down to the next line.
> + size_t size,
> + const cpu_set_t *set
> +);
> +
> +/**
> + * @brief Invalidates multiple data cache lines for a set of CPUs
> + *
> + * The cache lines covering the area are marked as invalid. A later read
> + * access in the area will load the data from memory.
> + *
> + * In case the area is not aligned on cache line boundaries, then this
> + * operation may destroy unrelated data.
> + *
> + * @param[in] addr The start address of the area to invalidate.
> + * @param[in] size The size in bytes of the area to invalidate.
> + */
> +void rtems_cache_invalidate_multiple_data_lines_cpu_set(
> + const void *addr,
> + size_t size,
> + const cpu_set_t *set
> +);
> +
> +/**
> + * @brief Invalidates multiple instruction cache lines for a set of CPUs
> + *
> + * The cache lines covering the area are marked as invalid. A later
> + * instruction fetch from the area will result in a load from memory.
> + *
> + * @param[in] addr The start address of the area to invalidate.
> + * @param[in] size The size in bytes of the area to invalidate.
> + */
> +void rtems_cache_invalidate_multiple_instruction_lines_cpu_set(
> + const void *addr,
> + size_t size,
> + const cpu_set_t *set
> +);
> +
> +/**
> + * @brief Flushes the entire data cache for a set of CPUs
> + *
> + * @see rtems_cache_flush_multiple_data_lines().
> + */
> +void rtems_cache_flush_entire_data_cpu_set( const cpu_set_t *set );
> +
> +/**
> + * @brief Invalidates the entire instruction cache for a set of CPUs
> + *
> + * @see rtems_cache_invalidate_multiple_instruction_lines().
> + */
> +void rtems_cache_invalidate_entire_instruction_cpu_set( const cpu_set_t *set );
> +
> +/**
> + * This function is responsible for performing a data cache
> + * invalidate. It invalidates the entire cache for a set of CPUs.
> + */
> +void rtems_cache_invalidate_entire_data_cpu_set( const cpu_set_t *set );
> +
> +#endif
> +
> /**@}*/
>
> #ifdef __cplusplus
> diff --git a/cpukit/score/include/rtems/score/smpimpl.h b/cpukit/score/include/rtems/score/smpimpl.h
> index 51dd34e..5db89ac 100644
> --- a/cpukit/score/include/rtems/score/smpimpl.h
> +++ b/cpukit/score/include/rtems/score/smpimpl.h
> @@ -21,6 +21,7 @@
> #include <rtems/score/smp.h>
> #include <rtems/score/percpu.h>
> #include <rtems/fatal.h>
> +#include <rtems/rtems/cache.h>
>
> #ifdef __cplusplus
> extern "C" {
> @@ -51,6 +52,13 @@ extern "C" {
> #define SMP_MESSAGE_TEST 0x2UL
>
> /**
> + * @brief SMP message to request a cache manager invocation.
> + *
> + * @see _SMP_Send_message().
> + */
> +#define SMP_MESSAGE_CACHE_MANAGER 0x4UL
> +
> +/**
> * @brief SMP fatal codes.
> */
> typedef enum {
> @@ -148,6 +156,11 @@ static inline void _SMP_Inter_processor_interrupt_handler( void )
> if ( ( message & SMP_MESSAGE_TEST ) != 0 ) {
> ( *_SMP_Test_message_handler )( cpu_self );
> }
> +
> + if ( ( message & SMP_MESSAGE_CACHE_MANAGER ) != 0 ) {
> + _SMP_Cache_manager_ipi_handler();
> + }
> +
> }
> }
>
--
Joel Sherrill, Ph.D. Director of Research & Development
joel.sherrill at OARcorp.com On-Line Applications Research
Ask me about RTEMS: a free RTOS Huntsville AL 35805
Support Available (256) 722-9985
More information about the devel
mailing list