[PATCH rtems v2 3/7] cpu/armv7m: Add table based init for ARMV7M_MPU

[Christian Mauderer]

Am 19.11.20 um 08:19 schrieb Sebastian Huber:
> On 18/11/2020 15:45, Christian Mauderer wrote:
> 
>> +/**
>> + * Higher level region configuration.
>> + *
>> + * Allows to configure with begin and end which is more convenient for
>> + * calculating the sizes from linker command file. Note that you 
>> still have to
>> + * follow the following rules:
>> + *
>> + * - Begin address has to be aligned to 0x20 (lower 5 bits set to 0)
>> + * - Sizes can only be a value of 2^x with a minimum of 32 Byte. If 
>> you have an
>> + *   end address that is not aligned, the region will get bigger.
>> + * - Later regions have higher priority.
>> + */
>> +typedef struct {
>> +  const void *begin;
>> +  const void *end;
>> +  uint32_t flags;
> 
> uint32_t rasr;
> 
> I guess the flags are actually a value stored directly to the RASR?
> 
>> +} ARMV7M_MPU_Region_Config;
> 
> ARMV7M_MPU_Region_config
> 
>> +
>>   typedef struct {
>>     uint32_t dhcsr;
>>     uint32_t dcrsr;
>> @@ -611,6 +632,85 @@ void _ARMV7M_Supervisor_call( void );
>>   void _ARMV7M_Clock_handler( void );
>> +static inline uint32_t _ARMV7M_MPU_get_region_size(uintptr_t size)
> 
> _ARMV7M_MPU_Get_region_size
> 
>> +{
>> +  if ((size & (size - 1)) == 0) {
>> +    return ARMV7M_MPU_RASR_SIZE(30 - __builtin_clz(size));
>> +  } else {
>> +    return ARMV7M_MPU_RASR_SIZE(31 - __builtin_clz(size));
>> +  }
>> +}
>> +
>> +static inline void _ARMV7M_MPU_set_region(
> 
> _ARMV7M_MPU_Set_region
> 
>> +  volatile ARMV7M_MPU *mpu,
>> +  uint32_t region,
>> +  uint32_t rasr,
>> +  const void *begin,
>> +  const void *end
>> +)
>> +{
>> +  uintptr_t size;
>> +  uint32_t rbar;
>> +
>> +  RTEMS_OBFUSCATE_VARIABLE(begin);
>> +  RTEMS_OBFUSCATE_VARIABLE(end);
>> +  size = (uintptr_t) end - (uintptr_t) begin;
>> +
>> +  if ( size > 0 ) {
>> +    rbar = (uintptr_t) begin | region | ARMV7M_MPU_RBAR_VALID;
>> +    rasr |= _ARMV7M_MPU_get_region_size(size);
>> +  } else {
>> +    rbar = region;
>> +    rasr = 0;
>> +  }
>> +
>> +  mpu->rbar = rbar;
>> +  mpu->rasr = rasr;
>> +}
>> +
>> +static inline void _ARMV7M_MPU_disable_region(
> 
> _ARMV7M_MPU_Disable_region
> 
>> +  volatile ARMV7M_MPU *mpu,
>> +  uint32_t region
>> +)
>> +{
>> +  mpu->rbar = ARMV7M_MPU_RBAR_VALID | region;
>> +  mpu->rasr = 0;
>> +}
>> +
>> +static inline void _ARMV7M_MPU_setup_from_config_and_enable(
>> +  const ARMV7M_MPU_Region_Config *cfg,
>> +  size_t cfg_count
>> +)
> 
> _ARMV7M_MPU_Setup
> 
> The config is implicit in the parameter. You don't necessarily enable a 
> configured region. It depends on the user provided RASR value.
> 
>> +{
>> +  volatile ARMV7M_MPU *mpu;
>> +  volatile ARMV7M_SCB *scb;
>> +  uint32_t region_count;
>> +  uint32_t region;
>> +
>> +  mpu = _ARMV7M_MPU;
>> +  scb = _ARMV7M_SCB;
>> +
>> +  region_count = ARMV7M_MPU_TYPE_DREGION_GET(mpu->type);
>> +
>> +  if (cfg_count > region_count) {
>> +    rtems_panic("invalid MPU config");
>> +  }
> 
> I think rtems_panic() is too heavy weight at this point. It uses 
> printk(). Maybe just use _Assert(). I think we need some light weight 
> general purpose fatal error which could just use the return address to 
> identify the error location.
> 
>> +
>> +  for (region = 0; region < region_count; ++region) {
> 
> for (region = cfg_count; region < region_count; ++region)
> 
> Only disable not configured regions?

I think I maybe have to disable the MPU first if I do that. Otherwise it 
is possible that there is (for example) a region 0 that prevents access 
to all memory and a high region (lets say 5) that allows access to 
program memory. As soon as I clear the high one but not the low one, the 
system will crash because it can't access the memory any more.

But most likely a similar situation can be constructed with cleaning all 
regions. I'll take a look at disabling MPU before setting it up.

> 
>> +    _ARMV7M_MPU_disable_region(mpu, region);
>> +  }
>> +
>> +  for (region = 0; region < cfg_count; ++region) {
>> +    _ARMV7M_MPU_set_region(mpu, region, cfg->flags, cfg->begin, 
>> cfg->end);
>> +  }
>> +
>> +  mpu->ctrl = ARMV7M_MPU_CTRL_ENABLE | ARMV7M_MPU_CTRL_PRIVDEFENA;
>> +  scb->shcsr |= ARMV7M_SCB_SHCSR_MEMFAULTENA;
>> +
>> +  _ARM_Data_synchronization_barrier();
>> +  _ARM_Instruction_synchronization_barrier();
>> +}
> 

-- 
--------------------------------------------
embedded brains GmbH
Christian MAUDERER
Dornierstr. 4
82178 Puchheim
Germany
email: christian.mauderer at embedded-brains.de
Phone: +49-89-18 94 741 - 18
Fax:   +49-89-18 94 741 - 08
PGP: Public key available on request.

embedded brains GmbH
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/