[PATCH 4/6] libmm-libbsp-arm-shared. libmm low-level implementation that can be shared between many ARM targets. mminit.c contains initialization function for arm-cp15 and setup page tables. After that it enables MMU and Cache unit part of BSP initialization process. mm.c translates attributes from high-level to CPU specific attributes and apply them dynamically.

Thu Sep 12 18:17:30 UTC 2013

On Thu, Sep 12, 2013 at 2:10 PM, Hesham Moustafa
<heshamelmatary at gmail.com> wrote:
>
>
>
> On Thu, Sep 12, 2013 at 3:01 PM, Gedare Bloom <gedare at rtems.org> wrote:
>>
>> On Thu, Sep 12, 2013 at 2:59 AM, Hesham AL-Matary
>> <heshamelmatary at gmail.com> wrote:
>> > ---
>> >  .../lib/libbsp/arm/shared/include/arm-cp15-start.h | 11 ++++--
>> >  c/src/lib/libbsp/arm/shared/mm.c                   | 46
>> > ++++++++++++++++++++++
>> >  c/src/lib/libbsp/arm/shared/mminit.c               | 27 +++++++++++++
>> >  3 files changed, 80 insertions(+), 4 deletions(-)
>> >  create mode 100644 c/src/lib/libbsp/arm/shared/mm.c
>> >  create mode 100644 c/src/lib/libbsp/arm/shared/mminit.c
>> >
>> > diff --git a/c/src/lib/libbsp/arm/shared/include/arm-cp15-start.h
>> > b/c/src/lib/libbsp/arm/shared/include/arm-cp15-start.h
>> > index 01f3104..b463b1f 100644
>> > --- a/c/src/lib/libbsp/arm/shared/include/arm-cp15-start.h
>> > +++ b/c/src/lib/libbsp/arm/shared/include/arm-cp15-start.h
>> > @@ -1,4 +1,5 @@
>> > -/*
>> > +/*
>> > + * Copyright (c) 2013 Hesham AL-Matary.
>> >   * Copyright (c) 2009-2013 embedded brains GmbH.  All rights reserved.
>> >   *
>> >   *  embedded brains GmbH
>> > @@ -89,7 +90,7 @@
>> > arm_cp15_start_setup_translation_table_and_enable_mmu_and_cache(
>> >
>> >    /* Initialize translation table with invalid entries */
>> Fix this comment (assuming the change below now populates the ttb with
>> valid entries).
>>
>> >    for (i = 0; i < ARM_MMU_TRANSLATION_TABLE_ENTRY_COUNT; ++i) {
>> > -    ttb [i] = 0;
>> > +    ttb [i] = (i << ARM_MMU_SECT_BASE_SHIFT) |
>> > ARMV7_MMU_DATA_READ_WRITE;
>> >    }
>> >
>> >    for (i = 0; i < config_count; ++i) {
>> > @@ -97,11 +98,13 @@
>> > arm_cp15_start_setup_translation_table_and_enable_mmu_and_cache(
>> >    }
>> >
>> >    /* Enable MMU and cache */
>> > -  ctrl |= ARM_CP15_CTRL_I | ARM_CP15_CTRL_C | ARM_CP15_CTRL_M;
>> > +  ctrl |= ARM_CP15_CTRL_AFE | ARM_CP15_CTRL_S | ARM_CP15_CTRL_I |
>> > +          ARM_CP15_CTRL_C | ARM_CP15_CTRL_M  | ARM_CP15_CTRL_XP;
>> > +
>> >    arm_cp15_set_control(ctrl);
>> >  }
>> >
>> > -BSP_START_TEXT_SECTION static inline uint32_t
>> > +BSP_START_TEXT_SECTION inline uint32_t
>> undo this change.
>
> I already did, but that caused undefined reference to
> arm_cp15_start_setup_mmu_and_cache
> compilation error at arm-a9mpcore-start.h:89 when I was compiling realview
> with --enable-smp
> option.
You should figure out why that happened. Changing this function to
non-inline does not seem like the right answer.

>
>>
>> >  arm_cp15_start_setup_mmu_and_cache(uint32_t ctrl_clear, uint32_t
>> > ctrl_set)
>> >  {
>> >    uint32_t ctrl = arm_cp15_get_control();
>> > diff --git a/c/src/lib/libbsp/arm/shared/mm.c
>> > b/c/src/lib/libbsp/arm/shared/mm.c
>> > new file mode 100644
>> > index 0000000..36e90be
>> > --- /dev/null
>> > +++ b/c/src/lib/libbsp/arm/shared/mm.c
>> > @@ -0,0 +1,46 @@
>> > +/*
>> > + * Copyright (c) 2013 Hesham AL-Matary.
>> > + *
>> > + * 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.
>> > + */
>> > +
>> > +#include <bsp/mm.h>
>> > +#include <libcpu/arm-cp15.h>
>> > +#include <rtems/score/mm.h>
>> > +
>> > +#define LIBMM_REGION_IS_CACHE_ENABLED(attr) \
>> > +  (((attr) >> RTEMS_MM_REGION_BIT_CACHE) & 1U)
>> > +#define LIBMM_REGION_IS_PROTECTION_WRITE_ENABLED(attr) \
>> > +  (((attr) >> RTEMS_MM_REGION_BIT_WRITE) & 1U)
>> > +#define LIBMM_REGION_IS_DEVICE_TYPE(attr) \
>> > +  (((attr) >> RTEMS_MM_REGION_BIT_DEVICE) & 1U)
>> > +#define LIBMM_REGION_IS_SHARED_MEMORY(attr) \
>> > +  (((attr) >> RTEMS_MM_REGION_BIT_SHARED) & 1U)
>> > +
>> Should these macros be provided by the score/mm.h?
>>
> mm.h or mmimpl.h whatever. It's used by applications calls as well as
> mm.c which used these definitions to translate attributes to CPU specific
> ones.
mm.h so that the macros are "visible".

>>
>> > +#define ARM_MMU_BIND_SECTION_TO_CLIENT_DOMAIN \
>> > +  (ARM_MMU_DEFAULT_CLIENT_DOMAIN << ARM_MMU_SECT_DOMAIN_SHIFT)
>> > +
>> > +#define LIBMM_TRANSLATE_ATTRIBUTES_TO_CPU(attr) \
>> > +  ((ARM_MMU_BIND_SECTION_TO_CLIENT_DOMAIN) \
>> > +  | (ARM_MMU_SECT_AP_0) \
>> > +  | (LIBMM_REGION_IS_PROTECTION_WRITE_ENABLED(attr) ? \
>> > +  0U : (ARM_MMU_SECT_AP_2)) \
>> > +  | (LIBMM_REGION_IS_CACHE_ENABLED(attr) ? \
>> > +  (ARM_MMU_SECT_TEX_0|ARM_MMU_SECT_C|ARM_MMU_SECT_B) : 0U) \
>> > +  | (LIBMM_REGION_IS_DEVICE_TYPE(attr) ? ARM_MMU_SECT_B : 0U) \
>> > +  | (LIBMM_REGION_IS_SHARED_MEMORY(attr) ? ARM_MMU_SECT_S : 0U) \
>> > +  | (ARM_MMU_SECT_DEFAULT))
>> > +
>> This macro could properly be named something simpler since it is local
>> to this .c file.
>>
> Sure, I just wanted to give it a self-describing name.
>>
>> > +void bsp_memory_management_set_attributes(
>> > +  uintptr_t base,
>> > +  size_t size,
>> > +  uint32_t attr
>> > +)
>> > +{
>> > +  uintptr_t end = (uint32_t)base + (uint32_t)size;
>> Don't cast base and size here.
>>
>> > +  uint32_t section_flags = LIBMM_TRANSLATE_ATTRIBUTES_TO_CPU(attr);
>> > +
>> > +  arm_cp15_set_translation_table_entries(base, end, section_flags);
>> > +}
>> > diff --git a/c/src/lib/libbsp/arm/shared/mminit.c
>> > b/c/src/lib/libbsp/arm/shared/mminit.c
>> > new file mode 100644
>> > index 0000000..333d65c
>> > --- /dev/null
>> > +++ b/c/src/lib/libbsp/arm/shared/mminit.c
>> > @@ -0,0 +1,27 @@
>> > +/*
>> > + * Copyright (c) 2013 Hesham AL-Matary.
>> > + *
>> > + * 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.
>> > + */
>> > +#include <bsp/arm-cp15-start.h>
>> > +#include <bsp/linker-symbols.h>
>> > +#include <bsp/mm.h>
>> > +#include <bsp/start.h>
>> > +
>> > +extern const mm_init_start_config bsp_mm_config_table[];
>> > +extern const size_t bsp_mm_config_table_size;
>> > +
>> > +BSP_START_TEXT_SECTION void bsp_memory_management_initialize(void)
>> > +{
>> > +  uint32_t ctrl = arm_cp15_get_control();
>> > +
>> > +  arm_cp15_start_setup_translation_table_and_enable_mmu_and_cache(
>> > +  ctrl,
>> > +  (uint32_t *) bsp_translation_table_base,
>> > +  ARM_MMU_DEFAULT_CLIENT_DOMAIN,
>> > +  &bsp_mm_config_table[0],
>> > +  bsp_mm_config_table_size
>> > +  );
>> Fix indentation level of nested lines of arguments. Also, this
>> mminit.c file does not make sense until you add at least one of the
>> ARM bsps using it.
>>
> realview, xilinx, and raspberry currently
> calls the initialization function within this
> mminit.c
Right, except they have not been added prior to this patch. Without
knowledge about them, this code does not make sense, right? Anyway, I
think it might be useful to re-arrange the order of these patches in
order to add the raspberrypi mmu handling first, and then show how to
consolidate the ARM MMU handling, and then offer some generic routines
in the bsp layer (e.g. bsp_memory_management_initialize) that can
unify the other BSPs like powerpc ones that will also have MMU/MPU
requirements.

>>
>> > +}
>> > --
>> > 1.8.3.1
>> >
>> > _______________________________________________
>> > rtems-devel mailing list
>> > rtems-devel at rtems.org
>> > http://www.rtems.org/mailman/listinfo/rtems-devel
>
>