[PATCH 3/5] Shared libmm implemenation for ARM BSPs

Gedare Bloom gedare at rtems.org
Mon Aug 26 15:39:10 UTC 2013


On Sun, Aug 25, 2013 at 8:14 PM, Hesham AL-Matary
<heshamelmatary at 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 at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel



More information about the devel mailing list