<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jul 15, 2013 at 4:18 PM, Gedare Bloom <span dir="ltr"><<a href="mailto:gedare@rtems.org" target="_blank">gedare@rtems.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Overall this looks OK from what I have seen before. I have a few<br>
comments about how we might want to consider improving the interface<br>
though before we start to commit code.<br>
<div><div class="h5"><br>
On Sat, Jul 13, 2013 at 7:05 PM, Hesham AL-Matary<br>
<<a href="mailto:heshamelmatary@gmail.com">heshamelmatary@gmail.com</a>> wrote:<br>
> From: Hesham ALmatary <<a href="mailto:heshamelmatary@gmail.com">heshamelmatary@gmail.com</a>><br>
><br>
> ---<br>
> cpukit/score/Makefile.am | 2 +<br>
> cpukit/score/include/rtems/score/mm.h | 67 ++++++++++++++++++++++++++++<br>
> cpukit/score/inline/rtems/score/mm.inl | 80 ++++++++++++++++++++++++++++++++++<br>
> cpukit/score/<a href="http://preinstall.am" target="_blank">preinstall.am</a> | 8 ++++<br>
> 4 files changed, 157 insertions(+)<br>
> create mode 100644 cpukit/score/include/rtems/score/mm.h<br>
> create mode 100644 cpukit/score/inline/rtems/score/mm.inl<br>
><br>
> diff --git a/cpukit/score/Makefile.am b/cpukit/score/Makefile.am<br>
> index 3f6e686..58a865b 100644<br>
> --- a/cpukit/score/Makefile.am<br>
> +++ b/cpukit/score/Makefile.am<br>
> @@ -30,6 +30,7 @@ include_rtems_score_HEADERS += include/rtems/score/protectedheap.h<br>
> include_rtems_score_HEADERS += include/rtems/score/interr.h<br>
> include_rtems_score_HEADERS += include/rtems/score/isr.h<br>
> include_rtems_score_HEADERS += include/rtems/score/isrlevel.h<br>
> +include_rtems_score_HEADERS += include/rtems/score/mm.h<br>
> include_rtems_score_HEADERS += include/rtems/score/object.h<br>
> include_rtems_score_HEADERS += include/rtems/score/percpu.h<br>
> include_rtems_score_HEADERS += include/rtems/score/priority.h<br>
> @@ -90,6 +91,7 @@ include_rtems_score_HEADERS += inline/rtems/score/coremutex.inl<br>
> include_rtems_score_HEADERS += inline/rtems/score/coresem.inl<br>
> include_rtems_score_HEADERS += inline/rtems/score/heap.inl<br>
> include_rtems_score_HEADERS += inline/rtems/score/isr.inl<br>
> +include_rtems_score_HEADERS += inline/rtems/score/mm.inl<br>
> include_rtems_score_HEADERS += inline/rtems/score/object.inl<br>
> include_rtems_score_HEADERS += inline/rtems/score/priority.inl<br>
> include_rtems_score_HEADERS += inline/rtems/score/prioritybitmap.inl<br>
> diff --git a/cpukit/score/include/rtems/score/mm.h b/cpukit/score/include/rtems/score/mm.h<br>
> new file mode 100644<br>
> index 0000000..6163be6<br>
> --- /dev/null<br>
> +++ b/cpukit/score/include/rtems/score/mm.h<br>
> @@ -0,0 +1,67 @@<br>
> +/**<br>
> + * @file<br>
> + *<br>
> + * @brief Manages use of MPU/MMU units to provide memory management.<br>
> + */<br>
> +<br>
> +/*<br>
> + * Copyright (c) 2013 Hesham Al-Matary.<br>
> + * Copyright (c) 2013 Gedare Bloom.<br>
> + *<br>
> + * The license and distribution terms for this file may be<br>
> + * found in the file LICENSE in this distribution or at<br>
> + * <a href="http://www.rtems.com/license/LICENSE" target="_blank">http://www.rtems.com/license/LICENSE</a>.<br>
> + */<br>
> +<br>
> +#ifndef _RTEMS_SCORE_MM_H<br>
> +#define _RTEMS_SCORE_MM_H<br>
> +<br>
> +/* @defgroup SuperCoreMM Memory Management Support<br>
> + *<br>
> + * @ingroup Score<br>
> + */<br>
> +/**@{*/<br>
> +<br>
> +#include <inttypes.h><br>
> +<br>
> +#ifdef RTEMS_SMP<br>
> +#include <rtems/score/smplock.h><br>
> +#endif<br>
> +<br>
> +#ifdef __cplusplus<br>
> +extern "C" {<br>
> +#endif<br>
> +/**<br>
> + * @brief _Memory_management_Region Flags defs<br>
> + */<br>
> +#define RTEMS_MM_REGION_PROTECTION_READ_ONLY 0x1<br>
</div></div>This should maybe be "READ" instead of "READ_ONLY".<br>
<div class="im"><br>
> +#define RTEMS_MM_REGION_PROTECTION_WRITE 0x2<br>
> +//#define RTEMS_MM_REGION_PROTECTION_EXEC 0x4<br>
> +<br>
> +/**<br>
> + * @brief A managed _Memory_management_Region.<br>
> + */<br>
> +typedef struct {<br>
> + char *name;<br>
> + uintptr_t base;<br>
> + size_t size;<br>
> + bool installed;<br>
> + /* points to structure defining the BSP specific MM entry */<br>
> + void *bsp_mme;<br>
> +} Memory_management_Entry;<br>
> +<br>
</div>I wonder if we need "installed" and "bsp_mme".<br>
<div><div class="h5"><br></div></div></blockquote><div>The "installed" field can be used at high-level layers (and I use it at low-level arm libmm) to check if the entry to be installed is already installed or not. I think mme is important too at high-level layer if we want to keep track of high-level mme and keep them in a data structures like a free/busy list or similar data structure. </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div class="h5">
> +#ifdef RTEMS_SMP<br>
> +SMP_lock_Control mm_lock;<br>
> +#endif<br>
> +<br>
> +#ifndef __RTEMS_APPLICATION__<br>
> +#include <rtems/score/mm.inl><br>
> +#endif<br>
> +<br>
> +#ifdef __cplusplus<br>
> +}<br>
> +#endif<br>
> +<br>
> +/**@}*/<br>
> +<br>
> +#endif<br>
> diff --git a/cpukit/score/inline/rtems/score/mm.inl b/cpukit/score/inline/rtems/score/mm.inl<br>
> new file mode 100644<br>
> index 0000000..07b1dd7<br>
> --- /dev/null<br>
> +++ b/cpukit/score/inline/rtems/score/mm.inl<br>
> @@ -0,0 +1,80 @@<br>
> +/**<br>
> + * @file<br>
> + *<br>
> + * @brief Inlined Routines from the Memory Management Manager<br>
> + */<br>
> +<br>
> +/*<br>
> + * Copyright (c) 2013 Hesham AL-Matary<br>
> + * Copyright (c) 2013 Gedare Bloom.<br>
> + *<br>
> + * The license and distribution terms for this file may be<br>
> + * found in the file LICENSE in this distribution or at<br>
> + * <a href="http://www.rtems.com/license/LICENSE" target="_blank">http://www.rtems.com/license/LICENSE</a>.<br>
> + */<br>
> +<br>
> +#ifndef _RTEMS_SCORE_MM_H<br>
> +# error "Never use <rtems/score/mm.inl> directly; include <rtems/score/mm.h> instead."<br>
> +#endif<br>
> +<br>
> +#ifndef _RTEMS_SCORE_MM_INL<br>
> +#define _RTEMS_SCORE_MM_INL<br>
> +<br>
> +/**<br>
> + * @addtogroup SuperCoreMM<br>
> + */<br>
> +/**@{**/<br>
> +<br>
> +/**<br>
> + * @brief Calls _CPU_Memory_management_Initialize.<br>
> + */<br>
> +RTEMS_INLINE_ROUTINE void _Memory_management_Initialize( void )<br>
> +{<br>
> +#ifdef RTEMS_SMP<br>
> + _SMP_lock_Initialize( &mm_lock );<br>
> +#endif<br>
> +<br>
> + _CPU_Memory_management_Initialize();<br>
> +}<br>
> +<br>
> +/**<br>
> + * @brief Calls _CPU_Memory_management_Install_entry.<br>
> + */<br>
> +RTEMS_INLINE_ROUTINE void _Memory_management_Install_entry(<br>
> + Memory_management_Entry *mme,<br>
> + uint32_t attr<br>
> +)<br>
</div></div>We should consider whether these functions should instead take<br>
arguments directly for the start and size of memory regions. At what<br>
layer do we want to "strip off" the other metadata like name?<br></blockquote><div>Yes, I will convert the prototypes for this function to take base, size and attributes directly.</div><div>I think the stripping off process it should be at middle layer if we intend to high-level entries into a data structure.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div class="h5">> +{<br>
> +#ifdef RTEMS_SMP<br>
> + _SMP_lock_Acquire( &mm_lock );<br>
> +#endif<br>
> +<br>
> + _CPU_Memory_management_Install_entry(mme, attr);<br>
> +<br>
> +#ifdef RTEMS_SMP<br>
> + _SMP_lock_Release( &mm_lock );<br>
> +#endif<br>
> +}<br>
> +<br>
> +/**<br>
> + * @brief Calls _CPU_Memory_management_Uninstall_entry.<br>
> + */<br>
> +RTEMS_INLINE_ROUTINE void _Memory_management_Uninstall_entry(<br>
> + Memory_management_Entry *mme<br>
> +)<br>
> +{<br>
> +#ifdef RTEMS_SMP<br>
> + _SMP_lock_Acquire( &mm_lock );<br>
> +#endif<br>
> +<br>
> + _CPU_Memory_management_Uninstall_entry(mme);<br>
> +<br>
> +#ifdef RTEMS_SMP<br>
> + _SMP_lock_Release( &mm_lock );<br>
> +#endif<br>
> +<br>
> +}<br>
> +<br>
> +/** @}*/<br>
> +<br>
> +#endif<br>
> diff --git a/cpukit/score/<a href="http://preinstall.am" target="_blank">preinstall.am</a> b/cpukit/score/<a href="http://preinstall.am" target="_blank">preinstall.am</a><br>
> index dc84b21..100f090 100644<br>
> --- a/cpukit/score/<a href="http://preinstall.am" target="_blank">preinstall.am</a><br>
> +++ b/cpukit/score/<a href="http://preinstall.am" target="_blank">preinstall.am</a><br>
> @@ -103,6 +103,10 @@ $(PROJECT_INCLUDE)/rtems/score/isrlevel.h: include/rtems/score/isrlevel.h $(PROJ<br>
> $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/score/isrlevel.h<br>
> PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/score/isrlevel.h<br>
><br>
> +$(PROJECT_INCLUDE)/rtems/score/mm.h: include/rtems/score/mm.h $(PROJECT_INCLUDE)/rtems/score/$(dirstamp)<br>
> + $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/score/mm.h<br>
> +PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/score/mm.h<br>
> +<br>
> $(PROJECT_INCLUDE)/rtems/score/object.h: include/rtems/score/object.h $(PROJECT_INCLUDE)/rtems/score/$(dirstamp)<br>
> $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/score/object.h<br>
> PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/score/object.h<br>
> @@ -298,6 +302,10 @@ $(PROJECT_INCLUDE)/rtems/score/isr.inl: inline/rtems/score/isr.inl $(PROJECT_INC<br>
> $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/score/isr.inl<br>
> PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/score/isr.inl<br>
><br>
> +$(PROJECT_INCLUDE)/rtems/score/mm.inl: inline/rtems/score/mm.inl $(PROJECT_INCLUDE)/rtems/score/$(dirstamp)<br>
> + $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/score/mm.inl<br>
> +PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/score/mm.inl<br>
> +<br>
> $(PROJECT_INCLUDE)/rtems/score/object.inl: inline/rtems/score/object.inl $(PROJECT_INCLUDE)/rtems/score/$(dirstamp)<br>
> $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/rtems/score/object.inl<br>
> PREINSTALL_FILES += $(PROJECT_INCLUDE)/rtems/score/object.inl<br>
> --<br>
> 1.8.3.1<br>
><br>
</div></div>> _______________________________________________<br>
> rtems-devel mailing list<br>
> <a href="mailto:rtems-devel@rtems.org">rtems-devel@rtems.org</a><br>
> <a href="http://www.rtems.org/mailman/listinfo/rtems-devel" target="_blank">http://www.rtems.org/mailman/listinfo/rtems-devel</a><br>
</blockquote></div><br></div></div>