apache / nuttx

Apache NuttX is a mature, real-time embedded operating system (RTOS)
https://nuttx.apache.org/
Apache License 2.0
2.74k stars 1.14k forks source link

STM32F7 MPU Memory Restrictions (Shareable attribute) #8288

Open NevynUK opened 1 year ago

NevynUK commented 1 year ago

Came across a problem configuring the MPU for a board running the STM32F777. The system locked the board and I believe that the lock was caused by theMPU_RASR_S attribute being used on a memory region that didi not support sharing.

The code was configuring the MPU using the mpu_user_intsram macro in mpu.h which expands to the following:

#define mpu_user_intsram(base, size) \
  do \
    { \
      /* The configure the region */ \
      mpu_configure_region(base, size, \
                           MPU_RASR_TEX_SO   | /* Ordered            */ \
                           MPU_RASR_C        | /* Cacheable          */ \
                                               /* Not Bufferable     */ \
                           MPU_RASR_S        | /* Shareable          */ \
                           MPU_RASR_AP_RWRW    /* P:RW   U:RW        */ \
                                               /* Instruction access */); \
    } while (0)

This passes the MPU_RASR_S attribute to mpu_configure_region which attempts to set the appropriate attributes for the memory region.

I was executing mpu_user_intsram(0x20070000, 0x8000) and this address is in a region that is not shareable and this caused my application to lock.

I finally resolved this by removing the shareable attribute to get the system running.

Now according to PM0253 - STM32 Programming Manual, the majority of the memory map is not shareable (Table 17 in Rev 5 of this document). In fact only 0xA0000000-0xBFFFFFFF and 0xE0000000-0xE00FFFFF are allowed to be shareable.

Given this restriction I think it would be advisable to make shareable an explicit option as these macros only work correctly for a small portion of the memory map.

acassis commented 1 year ago

@xiaoxiang781216 @davids5 @raiden00pl what do you think? Should we remove the MPU_RASR_S from mpu_user_intsram() or make some validation test to confirm that it is using an area that can be shared?

xiaoxiang781216 commented 1 year ago

As far as I understand, the sharable attribute is only suitable for SMP system, which indicate that the memory will be accessed by multiple CPU concurrently and cache system must do the sync in hardware level. Since ARM never officially support SMP for armvx-m architecture, MPU_RASR_S doesn't make sense for cortex-m at all.

masayuki2009 commented 1 year ago

https://developer.arm.com/documentation/dui0646/a/Cortex-M7-Peripherals/Optional-Memory-Protection-Unit?lang=en

image

NevynUK commented 1 year ago

It should be noted that MPU_RASR_S is used in a number of other macros in mpu.h as well.

NevynUK commented 1 year ago

As far as I understand, the sharable attribute is only suitable for SMP system, which indicate that the memory will be accessed by multiple CPU concurrently and cache system must do the sync in hardware level. Since ARM never officially support SMP for armvx-m architecture, MPU_RASR_S doesn't make sense for cortex-m at all.

What about DMA? Doesn't shareable allow multiple DMA requests to the same memory block?

masayuki2009 commented 1 year ago

http://www.st.com/resource/en/application_note/DM00272913.pdf

image
NevynUK commented 1 year ago

My preference would be to remove the shared attribute from all of the macros except maybe the peripheral sections (I can't remember if that can be shared) as you are more likely to create a problem with the current code than you are to result in a working system.

If you need the memory region to be shared then you can always do this by calling mpu_configure_region with the appropriate attributes.

masayuki2009 commented 1 year ago

https://www.st.com/resource/en/application_note/an4838-managing-memory-protection-unit-in-stm32-mcus-stmicroelectronics.pdf

image
xiaoxiang781216 commented 1 year ago

http://www.st.com/resource/en/application_note/DM00272913.pdf

image

Is this ST specific extension? DMA normally need do the cache clean/invalidate explicitly. Only very complex/smart DMA master(e.g. very recent ARM GPU: https://developer.arm.com/documentation/101574/0300/optimizing-opencl-for-mali-gpus/optimizing-memory-allocation/sharing-memory-in-a-fully-coherent-system) which implement AXI ACE extension can participate with CPU cache coherency system: https://developer.arm.com/documentation/ihi0022/e/ACE-Protocol-Specification/About-ACE/Coherency-overview?lang=en I don't believe that Cortex-M IP will implement ACE.

masayuki2009 commented 1 year ago

@xiaoxiang781216 I think it's not ST-specific but ARM Cortex-M7-specific. In fact, in the application note for i.mxrt, as you can also seen in https://www.nxp.com/docs/en/application-note/AN12042.pdf

It says that Cache policy is fixed to Non-cacheable when Shareable bit is set, no matter what’s the TEX/C/B value. A full cache policy settings table can be found in ARM Cortex-M7 Processor User Guide

image

So I think to make the normal memory region cacheable, the shareable bit in MPU should be 0. In this case, cache coherency between CPU and DMA must be ensured by software. (i.e. clean/invalidate)

xiaoxiang781216 commented 1 year ago

Yes, I think so. The memory tagged with both cacheable and shareable isn't really implemented on Cortex-M architecture.

davids5 commented 1 year ago

For the ARM Cortex-M7

So I think to make the normal memory region cacheable, the shareable bit in MPU should be 0. In this case, cache coherency between CPU and DMA must be ensured by software. (i.e. clean/invalidate)

I believe this to be correct and that mpu_user_intsram is incorrect.