[GitHub] [mynewt-core] utzig commented on pull request #2817: mcu/nrf5340: Add memory protection with MPU

2022-04-20 Thread GitBox 


utzig commented on PR #2817:
URL: https://github.com/apache/mynewt-core/pull/2817#issuecomment-1104473834

   
   
   
   ## Style check summary
   
   ### Our coding style is 
[here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
    hw/mcu/nordic/nrf5340/mpu/src/mpu.c
   
   
   ```diff
   @@ -46,7 +46,7 @@
ARM_MPU_SetMemAttr(0UL, ARM_MPU_ATTR(   /* Normal memory */
ARM_MPU_ATTR_MEMORY_(0UL, 1UL, 1UL, 1UL), /* Outer Write-Back 
transient with read and write allocate */
ARM_MPU_ATTR_MEMORY_(0UL, 0UL, 1UL, 1UL)  /* Inner Write-Through 
transient with read and write allocate */
   -));
   +   ));
/* RAM */
regions[0].RBAR = ARM_MPU_RBAR((uint32_t)&_ram_start, ARM_MPU_SH_OUTER, 
0UL, 1UL, 1UL);
regions[0].RLAR = ARM_MPU_RLAR(((uint32_t)&_ram_start + RAM_SIZE - 1), 
0UL);
   ```
   
   
   
    hw/mcu/nordic/nrf5340/src/system_nrf5340.c
   
   
   ```diff
   @@ -53,197 +53,189 @@
#define TRACE_TRACEDATA2_PIN TAD_PSEL_TRACEDATA2_PIN_Tracedata2
#define TRACE_TRACEDATA3_PIN TAD_PSEL_TRACEDATA3_PIN_Tracedata3

   -#if defined ( __CC_ARM )
   -uint32_t SystemCoreClock __attribute__((used)) = __SYSTEM_CLOCK_INITIAL;
   -#elif defined ( __ICCARM__ )
   -__root uint32_t SystemCoreClock = __SYSTEM_CLOCK_INITIAL;
   -#elif defined   ( __GNUC__ )
   -uint32_t SystemCoreClock __attribute__((used)) = __SYSTEM_CLOCK_INITIAL;
   +#if defined (__CC_ARM)
   +uint32_t SystemCoreClock __attribute__((used)) = __SYSTEM_CLOCK_INITIAL;
   +#elif defined (__ICCARM__)
   +__root uint32_t SystemCoreClock = __SYSTEM_CLOCK_INITIAL;
   +#elif defined   (__GNUC__)
   +uint32_t SystemCoreClock __attribute__((used)) = __SYSTEM_CLOCK_INITIAL;
#endif

/* stack limit provided by linker script */
extern uint32_t __StackLimit[];

   -void SystemCoreClockUpdate(void)
   +void
   +SystemCoreClockUpdate(void)
{
SystemCoreClock = __SYSTEM_CLOCK_MAX >> (NRF_CLOCK->HFCLKCTRL & 
(CLOCK_HFCLKCTRL_HCLK_Msk));
}

   -void SystemInit(void)
   +void
   +SystemInit(void)
{
#if !defined(NRF_TRUSTZONE_NONSECURE)
   -/* Perform Secure-mode initialization routines. */
   -
   -/* Set all ARM SAU regions to NonSecure if TrustZone extensions are 
enabled.
   -* Nordic SPU should handle Secure Attribution tasks */
   +/* Perform Secure-mode initialization routines. */
   +
   +/* Set all ARM SAU regions to NonSecure if TrustZone extensions are 
enabled.
   + * Nordic SPU should handle Secure Attribution tasks */
#if defined (__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U)
   -  SAU->CTRL |= (1 << SAU_CTRL_ALLNS_Pos);
   -#endif
   -
   -/* Workaround for Errata 97 "ERASEPROTECT, APPROTECT, or startup 
problems" found at the Errata document
   -   for your device located at 
https://infocenter.nordicsemi.com/index.jsp  */
   -if (nrf53_errata_97())
   -{
   -if (*((volatile uint32_t *)0x50004A20ul) == 0)
   -{
   -*((volatile uint32_t *)0x50004A20ul) = 0xDul;
   -*((volatile uint32_t *)0x5000491Cul) = 0x1ul;
   -*((volatile uint32_t *)0x5000491Cul) = 0x0ul;
   -}
   +SAU->CTRL |= (1 << SAU_CTRL_ALLNS_Pos);
   +#endif
   +
   +/* Workaround for Errata 97 "ERASEPROTECT, APPROTECT, or startup 
problems" found at the Errata document
   +   for your device located at 
https://infocenter.nordicsemi.com/index.jsp  */
   +if (nrf53_errata_97()) {
   +if (*((volatile uint32_t *)0x50004A20ul) == 0) {
   +*((volatile uint32_t *)0x50004A20ul) = 0xDul;
   +*((volatile uint32_t *)0x5000491Cul) = 0x1ul;
   +*((volatile uint32_t *)0x5000491Cul) = 0x0ul;
}
   -
   -/* Trimming of the device. Copy all the trimming values from FICR 
into the target addresses. Trim
   - until one ADDR is not initialized. */
   -uint32_t index = 0;
   -for (index = 0; index < 32ul && NRF_FICR_S->TRIMCNF[index].ADDR != 
(uint32_t *)0xul; index++){
   -#if defined ( __ICCARM__ )
   -/* IAR will complain about the order of volatile pointer 
accesses. */
   +}
   +
   +/* Trimming of the device. Copy all the trimming values from FICR into 
the target addresses. Trim
   +   until one ADDR is not initialized. */
   +uint32_t index = 0;
   +for (index = 0; index < 32ul && NRF_FICR_S->TRIMCNF[index].ADDR != 
(uint32_t *)0xul; index++) {
   +#if defined (__ICCARM__)
   +/* IAR will complain about the order of volatile pointer accesses. 
*/
#pragma diag_suppress=Pa082
#endif
   -*((volatile uint32_t *)NRF_FICR_S->TRIMCNF[index].ADDR) = 
NRF_FICR_S->TRIMCNF[index].DATA;
   -#if def

[GitHub] [mynewt-core] utzig commented on pull request #2817: mcu/nrf5340: Add memory protection with MPU

2022-04-21 Thread GitBox 


utzig commented on PR #2817:
URL: https://github.com/apache/mynewt-core/pull/2817#issuecomment-1104969822

   
   
   
   ## Style check summary
   
   ### Our coding style is 
[here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
    hw/mcu/nordic/nrf5340/src/system_nrf5340.c
   
   
   ```diff
   @@ -53,197 +53,189 @@
#define TRACE_TRACEDATA2_PIN TAD_PSEL_TRACEDATA2_PIN_Tracedata2
#define TRACE_TRACEDATA3_PIN TAD_PSEL_TRACEDATA3_PIN_Tracedata3

   -#if defined ( __CC_ARM )
   -uint32_t SystemCoreClock __attribute__((used)) = __SYSTEM_CLOCK_INITIAL;
   -#elif defined ( __ICCARM__ )
   -__root uint32_t SystemCoreClock = __SYSTEM_CLOCK_INITIAL;
   -#elif defined   ( __GNUC__ )
   -uint32_t SystemCoreClock __attribute__((used)) = __SYSTEM_CLOCK_INITIAL;
   +#if defined (__CC_ARM)
   +uint32_t SystemCoreClock __attribute__((used)) = __SYSTEM_CLOCK_INITIAL;
   +#elif defined (__ICCARM__)
   +__root uint32_t SystemCoreClock = __SYSTEM_CLOCK_INITIAL;
   +#elif defined   (__GNUC__)
   +uint32_t SystemCoreClock __attribute__((used)) = __SYSTEM_CLOCK_INITIAL;
#endif

/* stack limit provided by linker script */
extern uint32_t __StackLimit[];

   -void SystemCoreClockUpdate(void)
   +void
   +SystemCoreClockUpdate(void)
{
SystemCoreClock = __SYSTEM_CLOCK_MAX >> (NRF_CLOCK->HFCLKCTRL & 
(CLOCK_HFCLKCTRL_HCLK_Msk));
}

   -void SystemInit(void)
   +void
   +SystemInit(void)
{
#if !defined(NRF_TRUSTZONE_NONSECURE)
   -/* Perform Secure-mode initialization routines. */
   -
   -/* Set all ARM SAU regions to NonSecure if TrustZone extensions are 
enabled.
   -* Nordic SPU should handle Secure Attribution tasks */
   +/* Perform Secure-mode initialization routines. */
   +
   +/* Set all ARM SAU regions to NonSecure if TrustZone extensions are 
enabled.
   + * Nordic SPU should handle Secure Attribution tasks */
#if defined (__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U)
   -  SAU->CTRL |= (1 << SAU_CTRL_ALLNS_Pos);
   -#endif
   -
   -/* Workaround for Errata 97 "ERASEPROTECT, APPROTECT, or startup 
problems" found at the Errata document
   -   for your device located at 
https://infocenter.nordicsemi.com/index.jsp  */
   -if (nrf53_errata_97())
   -{
   -if (*((volatile uint32_t *)0x50004A20ul) == 0)
   -{
   -*((volatile uint32_t *)0x50004A20ul) = 0xDul;
   -*((volatile uint32_t *)0x5000491Cul) = 0x1ul;
   -*((volatile uint32_t *)0x5000491Cul) = 0x0ul;
   -}
   +SAU->CTRL |= (1 << SAU_CTRL_ALLNS_Pos);
   +#endif
   +
   +/* Workaround for Errata 97 "ERASEPROTECT, APPROTECT, or startup 
problems" found at the Errata document
   +   for your device located at 
https://infocenter.nordicsemi.com/index.jsp  */
   +if (nrf53_errata_97()) {
   +if (*((volatile uint32_t *)0x50004A20ul) == 0) {
   +*((volatile uint32_t *)0x50004A20ul) = 0xDul;
   +*((volatile uint32_t *)0x5000491Cul) = 0x1ul;
   +*((volatile uint32_t *)0x5000491Cul) = 0x0ul;
}
   -
   -/* Trimming of the device. Copy all the trimming values from FICR 
into the target addresses. Trim
   - until one ADDR is not initialized. */
   -uint32_t index = 0;
   -for (index = 0; index < 32ul && NRF_FICR_S->TRIMCNF[index].ADDR != 
(uint32_t *)0xul; index++){
   -#if defined ( __ICCARM__ )
   -/* IAR will complain about the order of volatile pointer 
accesses. */
   +}
   +
   +/* Trimming of the device. Copy all the trimming values from FICR into 
the target addresses. Trim
   +   until one ADDR is not initialized. */
   +uint32_t index = 0;
   +for (index = 0; index < 32ul && NRF_FICR_S->TRIMCNF[index].ADDR != 
(uint32_t *)0xul; index++) {
   +#if defined (__ICCARM__)
   +/* IAR will complain about the order of volatile pointer accesses. 
*/
#pragma diag_suppress=Pa082
#endif
   -*((volatile uint32_t *)NRF_FICR_S->TRIMCNF[index].ADDR) = 
NRF_FICR_S->TRIMCNF[index].DATA;
   -#if defined ( __ICCARM__ )
   +*((volatile uint32_t *)NRF_FICR_S->TRIMCNF[index].ADDR) = 
NRF_FICR_S->TRIMCNF[index].DATA;
   +#if defined (__ICCARM__)
#pragma diag_default=Pa082
#endif
   +}
   +
   +/* errata 64 must be before errata 42, as errata 42 is dependant on the 
changes in errata 64*/
   +/* Workaround for Errata 64 "VREGMAIN has invalid configuration when 
CPU is running at 128 MHz" found at the Errata document
   +   for your device located at 
https://infocenter.nordicsemi.com/index.jsp  */
   +if (nrf53_errata_64()) {
   +*((volatile uint32_t *)0x5000470Cul) = 0