[PATCH] mm: Add infrastructure for basic memory management

[Gedare Bloom]

Thanks Sebastian. Responses below.

On Wed, Feb 20, 2013 at 3:51 AM, Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
> On 02/18/2013 06:47 PM, Gedare Bloom wrote:
>>
>> +/**
>> + * @brief A managed _Memory_management_Region.
>> + */
>> +typedef struct {
>> +  char *name;
>
>
> Why not const char *?
>
That should be fine.

>
>> +  uintptr_t base;
>> +  size_t size;
>
>
> Why different types for base and size?
I suppose uintptr_t size might be sensible enough.

>
>> +  bool installed;
>
>
> Do we really need this state?  The entry install and uninstall functions
> should be idempotent.
>
I'm not sure or not. We can eliminate it for now.

>
>> +  /* points to structure defining the BSP specific MM entry */
>> +  void *bsp_mme;
>> +} Memory_management_Entry;
>
>
> Where does this bsp_mme value come from?  I would not use the name BSP here.
> Do we really need such a context pointer?
>
Either we provide the CPU / BSP with a context pointer in this
structure, or the CPU/BSP mm code has to translate the information
(base, size) into its own context. That might be OK in many cases.
I'll have to look to see if we use this pointer in any way that makes
it hard to recover this MME metadata.

>> +
>> +#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..abe065d
>> --- /dev/null
>> +++ b/cpukit/score/inline/rtems/score/mm.inl
>> @@ -0,0 +1,77 @@
>> +/**
>> + * @file
>> + *
>> + * @brief Inlined Routines from the Memory Management Manager
>> + */
>> +
>> +/*
>> + * 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 )
>> +{
>> +  _CPU_Memory_management_Initialize();
>> +}
>> +
>> +/**
>> + * @brief Calls _CPU_Memory_management_Install_entry.
>> + */
>> +RTEMS_INLINE_ROUTINE void _Memory_management_Install_entry(
>> +  Memory_management_Entry *mme
>> +)
>> +{
>> +  _CPU_Memory_management_Install_entry(mme);
>> +}
>> +
>> +/**
>> + * @brief Calls _CPU_Memory_management_Uninstall_entry.
>> + */
>> +RTEMS_INLINE_ROUTINE void _Memory_management_Uninstall_entry(
>> +  Memory_management_Entry *mme
>> +)
>> +{
>> +  _CPU_Memory_management_Uninstall_entry(mme);
>> +}
>> +
>> +/**
>> + * @brief Calls _CPU_Memory_management_Set_write.
>> + */
>> +RTEMS_INLINE_ROUTINE void _Memory_management_Set_write(
>> +  Memory_management_Entry *mme
>> +)
>> +{
>> +  _CPU_Memory_management_Set_write(mme);
>> +}
>> +
>> +/**
>> + * @brief Calls _CPU_Memory_management_Set_read_only
>> + */
>> +RTEMS_INLINE_ROUTINE void _Memory_management_Set_read_only(
>> +  Memory_management_Entry *mme
>> +)
>> +{
>> +  _CPU_Memory_management_Set_read_only(mme);
>> +}
>
>
> I would not use set methods for attributes.  I would use integer flags so we
> can initialize an entry completely static in read-only memory.
>
Yes, we had quite a long discussion about how to provide the
attributes. Using integer flags requires that MMU-specific code
translates the flags, but I'm seeing that it might be more sensible
from a flexibility point of view. Since either way the MMU specific
code would need to translate something to get the attributes set
correctly.

> I miss attributes to determine cache enable/disable, cache strategy, guarded
> (non well-behaved memory), cache consistency (MSI, MESI, etc.), and
> executable for example.
>
Yes, we should be able to support these and converting to an integer
format would make it easier to extend for each attribute we need to
support.

Thanks again, I will look at making these corrections.

> --
> Sebastian Huber, embedded brains GmbH
>
> Address : Dornierstr. 4, D-82178 Puchheim, Germany
> Phone   : +49 89 189 47 41-16
> Fax     : +49 89 189 47 41-09
> E-Mail  : sebastian.huber at embedded-brains.de
> PGP     : Public key available on request.
>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel