On Thu, Sep 12, 2013 at 2:26 PM, Hesham Moustafa
<heshamelmat...@gmail.com> wrote:
>
>
>
> On Thu, Sep 12, 2013 at 8:17 PM, Gedare Bloom <ged...@rtems.org> wrote:
>>
>> On Thu, Sep 12, 2013 at 2:10 PM, Hesham Moustafa
>> <heshamelmat...@gmail.com> wrote:
>> >
>> >
>> >
>> > On Thu, Sep 12, 2013 at 3:01 PM, Gedare Bloom <ged...@rtems.org> wrote:
>> >>
>> >> On Thu, Sep 12, 2013 at 2:59 AM, Hesham AL-Matary
>> >> <heshamelmat...@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.
>
> The change is that it's not static anymore because it's called from
> another file too.
inline functions should be declared static. Then they can be included
in multiple .c files without causing multiple definitions of the same
function.
>>
>> >
>> >>
>> >> > 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".
>>
> Yes for both application and low-level implementation code.
>>
>> >>
>> >> > +#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.
>>
> Sure, that means that the next set of patches should only include
> raspberrypi
> implementation only, Right ?
Yes I think it would be reasonable to start there.
>>
>> >>
>> >> > +}
>> >> > --
>> >> > 1.8.3.1
>> >> >
>> >> > _______________________________________________
>> >> > rtems-devel mailing list
>> >> > rtems-devel@rtems.org
>> >> > http://www.rtems.org/mailman/listinfo/rtems-devel
>> >
>> >
>
>
_______________________________________________
rtems-devel mailing list
rtems-devel@rtems.org
http://www.rtems.org/mailman/listinfo/rtems-devel
