[PATCH 1/4] Add libmm score changes

[Gedare Bloom]

Overall this looks OK from what I have seen before. I have a few
comments about how we might want to consider improving the interface
though before we start to commit code.

On Sat, Jul 13, 2013 at 7:05 PM, Hesham AL-Matary
<heshamelmatary at gmail.com> wrote:
> From: Hesham ALmatary <heshamelmatary at gmail.com>
>
> ---
>  cpukit/score/Makefile.am               |  2 +
>  cpukit/score/include/rtems/score/mm.h  | 67 ++++++++++++++++++++++++++++
>  cpukit/score/inline/rtems/score/mm.inl | 80 ++++++++++++++++++++++++++++++++++
>  cpukit/score/preinstall.am             |  8 ++++
>  4 files changed, 157 insertions(+)
>  create mode 100644 cpukit/score/include/rtems/score/mm.h
>  create mode 100644 cpukit/score/inline/rtems/score/mm.inl
>
> diff --git a/cpukit/score/Makefile.am b/cpukit/score/Makefile.am
> index 3f6e686..58a865b 100644
> --- a/cpukit/score/Makefile.am
> +++ b/cpukit/score/Makefile.am
> @@ -30,6 +30,7 @@ include_rtems_score_HEADERS += include/rtems/score/protectedheap.h
>  include_rtems_score_HEADERS += include/rtems/score/interr.h
>  include_rtems_score_HEADERS += include/rtems/score/isr.h
>  include_rtems_score_HEADERS += include/rtems/score/isrlevel.h
> +include_rtems_score_HEADERS += include/rtems/score/mm.h
>  include_rtems_score_HEADERS += include/rtems/score/object.h
>  include_rtems_score_HEADERS += include/rtems/score/percpu.h
>  include_rtems_score_HEADERS += include/rtems/score/priority.h
> @@ -90,6 +91,7 @@ include_rtems_score_HEADERS += inline/rtems/score/coremutex.inl
>  include_rtems_score_HEADERS += inline/rtems/score/coresem.inl
>  include_rtems_score_HEADERS += inline/rtems/score/heap.inl
>  include_rtems_score_HEADERS += inline/rtems/score/isr.inl
> +include_rtems_score_HEADERS += inline/rtems/score/mm.inl
>  include_rtems_score_HEADERS += inline/rtems/score/object.inl
>  include_rtems_score_HEADERS += inline/rtems/score/priority.inl
>  include_rtems_score_HEADERS += inline/rtems/score/prioritybitmap.inl
> diff --git a/cpukit/score/include/rtems/score/mm.h b/cpukit/score/include/rtems/score/mm.h
> new file mode 100644
> index 0000000..6163be6
> --- /dev/null
> +++ b/cpukit/score/include/rtems/score/mm.h
> @@ -0,0 +1,67 @@
> +/**
> + * @file
> + *
> + * @brief Manages use of MPU/MMU units to provide memory management.
> + */
> +
> +/*
> + * Copyright (c) 2013 Hesham Al-Matary.
> + * Copyright (c) 2013 Gedare Bloom.
> + *
> + * The license and distribution terms for this file may be
> + * found in the file LICENSE in this distribution or at
> + * http://www.rtems.com/license/LICENSE.
> + */
> +
> +#ifndef _RTEMS_SCORE_MM_H
> +#define _RTEMS_SCORE_MM_H
> +
> +/* @defgroup SuperCoreMM Memory Management Support
> + *
> + * @ingroup Score
> + */
> +/**@{*/
> +
> +#include <inttypes.h>
> +
> +#ifdef RTEMS_SMP
> +#include <rtems/score/smplock.h>
> +#endif
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +/**
> + * @brief _Memory_management_Region Flags defs
> + */
> +#define RTEMS_MM_REGION_PROTECTION_READ_ONLY   0x1
This should maybe be "READ" instead of "READ_ONLY".

> +#define RTEMS_MM_REGION_PROTECTION_WRITE  0x2
> +//#define RTEMS_MM_REGION_PROTECTION_EXEC   0x4
> +
> +/**
> + * @brief A managed _Memory_management_Region.
> + */
> +typedef struct {
> +  char *name;
> +  uintptr_t base;
> +  size_t size;
> +  bool installed;
> +  /* points to structure defining the BSP specific MM entry */
> +  void *bsp_mme;
> +} Memory_management_Entry;
> +
I wonder if we need "installed" and "bsp_mme".

> +#ifdef RTEMS_SMP
> +SMP_lock_Control mm_lock;
> +#endif
> +
> +#ifndef __RTEMS_APPLICATION__
> +#include <rtems/score/mm.inl>
> +#endif
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +/**@}*/
> +
> +#endif
> diff --git a/cpukit/score/inline/rtems/score/mm.inl b/cpukit/score/inline/rtems/score/mm.inl
> new file mode 100644
> index 0000000..07b1dd7
> --- /dev/null
> +++ b/cpukit/score/inline/rtems/score/mm.inl
> @@ -0,0 +1,80 @@
> +/**
> + * @file
> + *
> + * @brief Inlined Routines from the Memory Management Manager
> + */
> +
> +/*
> + * Copyright (c) 2013 Hesham AL-Matary
> + * Copyright (c) 2013 Gedare Bloom.
> + *
> + * The license and distribution terms for this file may be
> + * found in the file LICENSE in this distribution or at
> + * http://www.rtems.com/license/LICENSE.
> + */
> +
> +#ifndef _RTEMS_SCORE_MM_H
> +# error "Never use <rtems/score/mm.inl> directly; include <rtems/score/mm.h> instead."
> +#endif
> +
> +#ifndef _RTEMS_SCORE_MM_INL
> +#define _RTEMS_SCORE_MM_INL
> +
> +/**
> + * @addtogroup SuperCoreMM
> + */
> +/**@{**/
> +
> +/**
> + * @brief Calls _CPU_Memory_management_Initialize.
> + */
> +RTEMS_INLINE_ROUTINE void _Memory_management_Initialize( void )
> +{
> +#ifdef RTEMS_SMP
> +   _SMP_lock_Initialize( &mm_lock );
> +#endif
> +
> +  _CPU_Memory_management_Initialize();
> +}
> +
> +/**
> + * @brief Calls _CPU_Memory_management_Install_entry.
> + */
> +RTEMS_INLINE_ROUTINE void _Memory_management_Install_entry(
> +  Memory_management_Entry *mme,
> +  uint32_t attr
> +)
We should consider whether these functions should instead take
arguments directly for the start and size of memory regions. At what
layer do we want to "strip off" the other metadata like name?
> +{
> +#ifdef RTEMS_SMP
> +  _SMP_lock_Acquire( &mm_lock );
> +#endif
> +
> +  _CPU_Memory_management_Install_entry(mme, attr);
> +
> +#ifdef RTEMS_SMP
> +  _SMP_lock_Release( &mm_lock );
> +#endif
> +}
> +
> +/**
> + * @brief Calls _CPU_Memory_management_Uninstall_entry.
> + */
> +RTEMS_INLINE_ROUTINE void _Memory_management_Uninstall_entry(
> +  Memory_management_Entry *mme
> +)
> +{
> +#ifdef RTEMS_SMP
> +  _SMP_lock_Acquire( &mm_lock );
> +#endif
> +
> +  _CPU_Memory_management_Uninstall_entry(mme);
> +
> +#ifdef RTEMS_SMP
> +  _SMP_lock_Release( &mm_lock );
> +#endif
> +
> +}
> +
> +/** @}*/
> +
> +#endif
> diff --git a/cpukit/score/preinstall.am b/cpukit/score/preinstall.am
> index dc84b21..100f090 100644
> --- a/cpukit/score/preinstall.am
> +++ b/cpukit/score/preinstall.am
> @@ -103,6 +103,10 @@ $(PROJECT_INCLUDE)/rtems/score/isrlevel.h: include/rtems/score/isrlevel.h $(PROJ
>         $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/score/isrlevel.h
>  PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/score/isrlevel.h
>
> +$(PROJECT_INCLUDE)/rtems/score/mm.h: include/rtems/score/mm.h $(PROJECT_INCLUDE)/rtems/score/$(dirstamp)
> +       $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/score/mm.h
> +PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/score/mm.h
> +
>  $(PROJECT_INCLUDE)/rtems/score/object.h: include/rtems/score/object.h $(PROJECT_INCLUDE)/rtems/score/$(dirstamp)
>         $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/score/object.h
>  PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/score/object.h
> @@ -298,6 +302,10 @@ $(PROJECT_INCLUDE)/rtems/score/isr.inl: inline/rtems/score/isr.inl $(PROJECT_INC
>         $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/score/isr.inl
>  PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/score/isr.inl
>
> +$(PROJECT_INCLUDE)/rtems/score/mm.inl: inline/rtems/score/mm.inl $(PROJECT_INCLUDE)/rtems/score/$(dirstamp)
> +       $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/score/mm.inl
> +PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/score/mm.inl
> +
>  $(PROJECT_INCLUDE)/rtems/score/object.inl: inline/rtems/score/object.inl $(PROJECT_INCLUDE)/rtems/score/$(dirstamp)
>         $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/score/object.inl
>  PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/score/object.inl
> --
> 1.8.3.1
>
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel