On Sun, Aug 25, 2013 at 8:14 PM, Hesham AL-Matary
<heshamelmat...@gmail.com> wrote:
> ---
> c/src/lib/libcpu/arm/shared/include/arm-cp15.h | 17 ++++
> .../libcpu/arm/shared/include/arm_cp15_print_fsr.h | 84 ++++++++++++++++
> c/src/lib/libcpu/arm/shared/src/armv7-mm-mpu.c | 108
> +++++++++++++++++++++
> c/src/lib/libcpu/arm/shared/src/mm.c | 83 ++++++++++++++++
> 5 files changed, 306 insertions(+), 1 deletion(-)
> create mode 100644 c/src/lib/libcpu/arm/shared/include/arm_cp15_print_fsr.h
> create mode 100644 c/src/lib/libcpu/arm/shared/src/armv7-mm-mpu.c
> create mode 100644 c/src/lib/libcpu/arm/shared/src/mm.c
>
> diff --git a/c/src/lib/libcpu/arm/shared/include/arm-cp15.h
> b/c/src/lib/libcpu/arm/shared/include/arm-cp15.h
> index 0117a5e..20eccbb 100644
> --- a/c/src/lib/libcpu/arm/shared/include/arm-cp15.h
> +++ b/c/src/lib/libcpu/arm/shared/include/arm-cp15.h
> @@ -7,6 +7,7 @@
> */
>
> /*
> + * Copyright (c) 2013 Hesham AL-Matary
> * Copyright (c) 2009-2013 embedded brains GmbH. All rights reserved.
> *
> * embedded brains GmbH
> @@ -139,6 +140,7 @@ extern "C" {
> #define ARM_CP15_CTRL_NMFI (1U << 27)
> #define ARM_CP15_CTRL_EE (1U << 25)
> #define ARM_CP15_CTRL_VE (1U << 24)
> +#define ARM_CP15_CTRL_XP (1U << 23) // ARMv6
> #define ARM_CP15_CTRL_U (1U << 22)
> #define ARM_CP15_CTRL_FI (1U << 21)
> #define ARM_CP15_CTRL_UWXN (1U << 20)
> @@ -250,6 +252,7 @@ static inline void arm_cp15_set_control(uint32_t val)
> *
> * @return The current control register value.
> */
> +
Avoid adding white space.
> static inline uint32_t arm_cp15_mmu_disable(uint32_t cls)
> {
> ARM_SWITCH_REGISTERS;
> @@ -1022,11 +1025,25 @@ uint32_t arm_cp15_set_translation_table_entries(
> uint32_t section_flags
> );
>
> +/**
> + * @brief Unsets the @a sections entry for the address range [@a begin, @a
> end).
> + * Unset section entry by set its value to Zero
> + */
See http://wiki.rtems.org/wiki/index.php/Doxygen_Recommendations for
advice on how to document functions using doxygen in RTEMS.
> +uint32_t arm_cp15_unset_translation_table_entries(
> + const void *begin,
> + const void *end
> +);
> +
> void arm_cp15_set_exception_handler(
> Arm_symbolic_exception_name exception,
> void (*handler)(void)
> );
>
> +/**
> + * @brief dummy exception handler for data aborts to help in debugging
> + */
> +void dummy_data_abort_exception_handler(void);
> +
Perhaps let's use "default" instead of "dummy" and make it conditional
on #if RTEMS_DEBUG. I'm not sure what the default behavior ought to
be...
> /** @} */
>
> #ifdef __cplusplus
> diff --git a/c/src/lib/libcpu/arm/shared/include/arm_cp15_print_fsr.h
> b/c/src/lib/libcpu/arm/shared/include/arm_cp15_print_fsr.h
> new file mode 100644
> index 0000000..5755a31
> --- /dev/null
> +++ b/c/src/lib/libcpu/arm/shared/include/arm_cp15_print_fsr.h
> @@ -0,0 +1,84 @@
> +/**
> + * @file
> + *
> + * @ingroup ScoreCPUARMCP15
> + *
> + * @brief ARM co-processor 15 (CP15) API.
> + */
> +
> +/*
> + * 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.
> + */
> +
> +#ifndef LICPU_SHARED_ARM_CP15_DATA_FAULT_INFO
> +#define LICPU_SHARED_ARM_CP15_DATA_FAULT_INFO
s/LICPU/LIBCPU
> +#endif
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif /* __cplusplus */
> +
> +/* Define mask for ARM CP15 fault status register */
> +#define ARM_CP15_FAULT_STATUS_MASK 0x040F
> +
> +/* Error States for ARM CP15 fault status register */
> +#define ARM_CP15_ALIGNMENT_FAULT 0x00000001
> +#define ARM_CP15_BACKGROUND_FAULT 0x0000
> +#define ARM_CP15_ACCESS_PERMISSION_FAULT 0x000D
> +#define ARM_CP15_PRECISE_EXTERNAL_ABORT_FAULT 0x0008
> +#define ARM_CP15_IMPRECISE_EXTERNAL_ABORT_FAULT 0x0406
> +#define ARM_CP15_PRECISE_PARITY_ERROR_EXCEPTION 0x0006
> +#define ARM_CP15_IMPRECISE_PARITY_ERROR_EXCEPTION 0x0408
> +#define ARM_CP15_DEBUG_EVENT 0x0002
> +
Should these defines just be added to the existing cp15.h file?
> +/* print error description */
> +
> +void arm_cp15_print_fault_status_description(uint32_t fsr)
> +{
> +
> + uint32_t error = fsr & ARM_CP15_FAULT_STATUS_MASK;
> + //printk("fsr before mask is 0x%x and after mask is 0x%x\n",fsr,error);
> + switch(error)
> + {
> + case ARM_CP15_ALIGNMENT_FAULT:
> + printk("Fault Status : Alignment fault !\n");
> + break;
> +
> + case ARM_CP15_BACKGROUND_FAULT:
> + printk("Fault Status : Background fault !\n");
> + break;
> +
> + case ARM_CP15_ACCESS_PERMISSION_FAULT:
> + printk("Fault Status : Memory Access Permission fault !\n");
> + break;
> + case ARM_CP15_PRECISE_EXTERNAL_ABORT_FAULT:
> + printk("Fault Status : Precise external abort !\n");
> + break;
> +
> + case ARM_CP15_IMPRECISE_EXTERNAL_ABORT_FAULT:
> + printk("Fault Status : Imprecise external abort !\n");
> + break;
> +
> + case ARM_CP15_PRECISE_PARITY_ERROR_EXCEPTION:
> + printk("Fault Status : Precise parity error excpetion !\n");
> + break;
> +
> + case ARM_CP15_IMPRECISE_PARITY_ERROR_EXCEPTION:
> + printk("Fault Status : Imprecise parity error excpetion !\n");
> + break;
> +
> + case ARM_CP15_DEBUG_EVENT:
> + printk("Fault Status : Debug event !\n");
> + break;
> +
> + default:
> + printk("Unknown Error !\n");
> + }
> +}
This should be an optional function defined in some .c file.
> +
> +#ifdef __cplusplus
> +#endif /* LICPU_SHARED_ARM_CP15_DATA_FAULT_INFO */
same typo here.
> diff --git a/c/src/lib/libcpu/arm/shared/src/armv7-mm-mpu.c
> b/c/src/lib/libcpu/arm/shared/src/armv7-mm-mpu.c
> new file mode 100644
> index 0000000..f4db385
> --- /dev/null
> +++ b/c/src/lib/libcpu/arm/shared/src/armv7-mm-mpu.c
> @@ -0,0 +1,108 @@
> +/**
> + * @file
> + *
> + * @ingroup libmm
> + *
> + * @brief Wrapper for ARMv7 MPU API.
> + */
> +
> +/*
> + * 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.
> + */
> +
> +#ifndef LIBCPU_ARM_MPU_LIBMM
> +#define LIBCPU_ARM_MPU_LIBMM
> +
> +#include <rtems/score/armv7m.h>
> +#include <libcpu/mm.h>
> +#include <bsp/start-config.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif /* __cplusplus */
> +
> +#define ALIGN_REGION_START_ADDRESS(addr,size) \
> + addr &= (2<<size)
> +
> +static void translate_attributes(
> + uint32_t high_level_attr,
> + uint32_t *ARM_CPU_ATTR
> +)
> +{
> +#ifdef ARM_MULTILIB_ARCH_V7M
> + /* Clear flags attributes */
> + *ARM_CPU_ATTR = 0;
> +
> + if( high_level_attr & 0x1 )
Instead of using the hard-coded values (0x1) you should use the
macro/enum names, like MM_READ_ONLY.
> + *ARM_CPU_ATTR |= ARMV7M_MPU_AP_PRIV_RO_USER_RO;
> +
> + /* Write access */
> + if ( high_level_attr & 0x2 )
Same.
> + *ARM_CPU_ATTR |= ARMV7M_MPU_AP_PRIV_RW_USER_RW;
> +#endif /* ARM_MULTILIB_ARCH_V7M */
> +}
> +
> +
Avoid more than two blank lines in a row.
> +void _CPU_Memory_management_Initialize(void)
> +{
> +#ifdef ARM_MULTILIB_ARCH_V7M
> + volatile ARMV7M_MPU *mpu = _ARMV7M_MPU;
> + size_t region_count = arm_start_config_mpu_region_count;
> + size_t i = 0;
> +
> + for (i = 0; i < region_count; ++i) {
> + mpu->rbar = arm_start_config_mpu_region [i].rbar;
> + mpu->rasr = arm_start_config_mpu_region [i].rasr;
Delete the blank space before [i].
> + }
> +
> + if (region_count > 0) {
> + mpu->ctrl = ARMV7M_MPU_CTRL_ENABLE;
> + }
> +#endif /* ARM_MULTILIB_ARCH_V7M */
> +}
> +
> +_CPU_Memory_management_Set_attributes(
> + uintptr_t base,
> + size_t size,
> + uint32_t attr
> +)
> +{
> +#ifdef ARM_MULTILIB_ARCH_V7M
> + volatile ARMV7M_MPU *mpu = _ARMV7M_MPU;
> + rtems_interrupt_level level;
> +
> + uint32_t mpu_attr;
> +
> + translate_attributes(attr, &mpu_attr);
> +
> +
Avoid multiple blank lines.
> + /* Disable MPU and interrupts */
> + rtems_interrupt_disable(level);
Should synchronization be handled by the higher level that calls
Set_attributes? In the SMP implementation there is a lock. Perhaps in
UP implementation it should disable interrupts around this entire
function? Or maybe the SMP should embed the lock within a structure
that gets passed to the lower level so that SMP can lock internally
here to avoid locking during the "translate attributes" function.
> + mpu-> = 0;
> +
> + ARMV7M_MPU_Region region =
> + ARMV7M_MPU_REGION_INITIALIZER(
> + 0,
> + base,
> + size,
> + mpu_attr
> + );
What does "0" mean here?
> +
> + mpu->rbar = region.basr;
> + mpu->rasr = region.rasr;
> +
> + /* Enable MPU and interrupts */
> + mpu->ctrl = ARMV7M_MPU_CTRL_ENABLE;
> + rtems_interrupt_enable(level);
> +#endif /* ARM_MULTILIB_ARCH_V7M */
> +}
> +
> +#ifdef __cplusplus
> +}
> +#endif /* __cplusplus */
> +
> +#endif /* LIBCPU_ARM_MPU_LIBMM */
> diff --git a/c/src/lib/libcpu/arm/shared/src/mm.c
> b/c/src/lib/libcpu/arm/shared/src/mm.c
> new file mode 100644
> index 0000000..f0be25f
> --- /dev/null
> +++ b/c/src/lib/libcpu/arm/shared/src/mm.c
> @@ -0,0 +1,83 @@
> +/*
> + * 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 <libcpu/arm-cp15.h>
> +#include <libcpu/mm.h>
> +#include <bsp/start.h>
> +#include <bsp/linker-symbols.h>
> +#include <libcpu/arm_cp15_print_fsr.h>
> +#include <bsp/mm_config_table.h>
> +
> +void _CPU_Memory_management_Initialize(void)
> +{
> + uint32_t ctrl = 0;
Don't need to initialize to 0.
> + uint32_t client_domain = 15;
This variable looks unused.
> + uint32_t *ttb;
> + 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,
> + &_cpu_mmu_config_table[0],
> + RTEMS_ARRAY_SIZE(&_cpu_mmu_config_table)
> + );
> +
> + arm_cp15_set_exception_handler(ARM_EXCEPTION_DATA_ABORT,
> + dummy_data_abort_exception_handler
> + );
> +}
> +
> +void _CPU_Memory_management_Set_attributes(
> + uintptr_t base,
> + size_t size,
> + uint32_t attr
> +)
> +{
> + uint32_t end = (uint32_t)base + (uint32_t)size;
> + uint32_t section_flags;
> + uint32_t cl_size = arm_cp15_get_min_cache_line_size();
> + uint32_t i = ARM_MMU_SECT_GET_INDEX(base);
> + uint32_t iend = ARM_MMU_SECT_GET_INDEX(ARM_MMU_SECT_MVA_ALIGN_UP(end));
> + uint32_t ctrl;
> + uint32_t cpsr = 0;
> + uint32_t *ttb = arm_cp15_get_translation_table_base();
> + uint32_t *table = (uint32_t *) bsp_section_vector_begin;
> + uint32_t *vend = (uint32_t *) bsp_section_vector_end;
> +
> + section_flags = translation_table[attr];
> + arm_cp15_set_translation_table_entries(base, end, section_flags);
> +}
> +
> +void dummy_data_abort_exception_handler(void)
> +{
> + uint32_t cpsr;
> +#if DEBUG
> + printf("Entered exception handler \n");
> +
> + __asm__ volatile (
> + ARM_SWITCH_TO_ARM
> + "mrs %[cpsr], cpsr\n"
> + ARM_SWITCH_BACK
> + : [cpsr] "=&r" (cpsr)
> + );
> +
> + printf("CPSR after enable MMU is 0x%x \n",cpsr);
These could be printk, but I'm not sure they should even be used. You
can't rely on printing if the MMU/MPU is not working.
> +
> + uint32_t address_fault = (uint32_t)arm_cp15_get_fault_address();
> + printk("Data fault address at 0x%x\n",address_fault);
> +
> + uint32_t fsr = (uint32_t) arm_cp15_get_data_fault_status();
> + arm_cp15_print_fault_status_description(fsr);
> +#endif
> +
> + //TODO: Call rtems_fatal
> + exit(0);
This should probably be made an actual fatal error.
> +}
> +
> --
> 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
