armv7-m/arm_mpu.c: MPU regions >= 2GB cannot be configured about nuttx

[anchao]

Hi @danielappiagyei-bc nice finding! Thank you very much?

I think the second option is a better approach, because will work for all cases without special cases or other inconveniences.

@anchao what do you think?

I also agree with the second option, could we add the uint64_t only for this function to handle the case of base + size overflow?

[danielappiagyei-bc]

for sure, will do

[acassis]

Hi @danielappiagyei-bc nice finding! Thank you very much?

I think the second option is a better approach, because will work for all cases without special cases or other inconveniences.

@anchao what do you think?

[acassis]

@danielappiagyei-bc please submit a PR with your fix

[danielappiagyei-bc]

I also agree with the second option, could we add the uint64_t only for this function to handle the case of base + size overflow?

@anchao The other issue is that because the argument base to this function is a uint32_t, you cannot specify a region size as 4GiB (because (uint32_t)(4 * 1024 * 1024 * 1024) overflows to 0). You may want to set a rule for the entire memory region due to erratum ERR011573 for Cortex-M7 MPUs specified here. The workaround states:

Workaround: Instead of leaving memory unmapped, software should use MPU region 0 to cover all
unmapped memory and make this region execute-never and inaccessible. That is,
MPU_RASR0 should be programmed with:
• MPU_RASR0.ENABLE = 1; MPU region 0 enable.
• MPU_RASR0.SIZE = b11111; MPU region 0 size = 2^32 bytes to cover entire memory.

To fix this, I need to make base uint64_t along with any function that base is passed to. So, mpu_subregion() and (maybe?) mpu_log2regionceil() need uint64_t args. Is this okay?

[anchao]

I also agree with the second option, could we add the uint64_t only for this function to handle the case of base + size overflow?

@anchao The other issue is that because the argument base to this function is a uint32_t, you cannot specify a region size as 4GiB (because (uint32_t)(4 * 1024 * 1024 * 1024) overflows to 0). You may want to set a rule for the entire memory region due to erratum ERR011573 for Cortex-M7 MPUs specified here. The workaround states:

Workaround: Instead of leaving memory unmapped, software should use MPU region 0 to cover all
unmapped memory and make this region execute-never and inaccessible. That is,
MPU_RASR0 should be programmed with:
• MPU_RASR0.ENABLE = 1; MPU region 0 enable.
• MPU_RASR0.SIZE = b11111; MPU region 0 size = 2^32 bytes to cover entire memory.

To fix this, I need to make base uint64_t along with any function that base is passed to. So, mpu_subregion() and (maybe?) mpu_log2regionceil() need uint64_t args. Is this okay?

This is a programming error and the compiler will report corresponding warning like Woverflow. BTW similar data types are used in popular RTOS such as Zephyr:
https://github.com/zephyrproject-rtos/zephyr/blob/main/arch/arm/include/kernel_arch_data.h#L52-L56

[danielappiagyei-bc]

I also agree with the second option, could we add the uint64_t only for this function to handle the case of base + size overflow?

@anchao The other issue is that because the argument base to this function is a uint32_t, you cannot specify a region size as 4GiB (because (uint32_t)(4 * 1024 * 1024 * 1024) overflows to 0). You may want to set a rule for the entire memory region due to erratum ERR011573 for Cortex-M7 MPUs specified here. The workaround states:

Workaround: Instead of leaving memory unmapped, software should use MPU region 0 to cover all

unmapped memory and make this region execute-never and inaccessible. That is,

MPU_RASR0 should be programmed with:

• MPU_RASR0.ENABLE = 1; MPU region 0 enable.

• MPU_RASR0.SIZE = b11111; MPU region 0 size = 2^32 bytes to cover entire memory.

To fix this, I need to make base uint64_t along with any function that base is passed to. So, mpu_subregion() and (maybe?) mpu_log2regionceil() need uint64_t args. Is this okay?

This is a programming error and the compiler will report corresponding warning like Woverflow. BTW similar data types are used in popular RTOS such as Zephyr:

https://github.com/zephyrproject-rtos/zephyr/blob/main/arch/arm/include/kernel_arch_data.h#L52-L56

Fair enough, I guess my point is that the architecture supports configuring a 4GiB region whereas this API (and zephyr's apparently) does not. But I understand we can't handle every special case while keeping the API generic, consistent between architectures, and simple. I'll make the requested changes 🙂

[danielappiagyei-bc]

Will get back to this in a few weeks, appreciate the patience, am busy with work and personal