stm32h7:sdmmc: prevent SRAM123 and SRAM4 from using by SDMMC1 IDMA

[a-lunev]

Summary

Preventing SRAM123 and SRAM4 from using by SDMMC1 IDMA.

Impact

STM32H7 SDMMC

[davids5]

@a-lunev - Why is this needed when stm32_dmapreflight already excludes it?

[a-lunev]

@a-lunev - Why is this needed when stm32_dmapreflight already excludes it?

Hi David,
stm32_dmapreflight() depends on the CONFIG_STM32H7_SDMMC_IDMA option that as I understand should not do, because IDMA is still in use not depending on the option.
Also stm32_dmapreflight() is invoked only if CONFIG_DEBUG_ASSERTIONS option is enabled.
If my understanding is correct, concerning namely SDMMC1 IDMA, stm32_dmapreflight() is useful for debug purposes to check if IDMA is trying to access not allowed memory regions like SRAM123 and SRAM4. This can be caught only if CONFIG_DEBUG_ASSERTIONS option is enabled.
In a normal build CONFIG_DEBUG_ASSERTIONS option is not enabled. How then the system should be prevented from the IDMA access failure?
On another hand, if CONFIG_DEBUG_ASSERTIONS option is enabled and the assert has been triggered, what the further developer's actions should be in this particular case with SDMMC1 IDMA? As I understand, normally there are two possible options:

• either to exclude SRAM123 and SRAM4 from the heap allocation;
• or to use SDMMC2 instead of SDMMC1 as SDMMC2 does not have the access limitation.

Please let me know what do you think.

[davids5]

@a-lunev stm32_dmapreflight is used by the high level SDMMC driver https://github.com/apache/incubator-nuttx/blob/master/drivers/mmcsd/mmcsd_sdio.c#L1392 . The debug assert is in addition to that just for debugging.

[davids5]

@a-lunev - the stm32_dmapreflight does the right thing. The lame FIFO requires IDMA. But to a buffer that is not from the heap.

Please let me know what do you think.

I think the exclusion from the heap for this drive is not the correct solution. The solution is in place. Did not work or is this just a misunderstanding or configuration issue on your part?

You may want to add the Exclusion as an option in a separate PR but it can not come in here

[hartmannathan]

@a-lunev what are you trying to achieve? Is this similar to the BDMA/SRAM4 heap clobbering issue fixed in 4716fc929d7 / PR-3198?

[a-lunev]

@a-lunev stm32_dmapreflight is used by the high level SDMMC driver https://github.com/apache/incubator-nuttx/blob/master/drivers/mmcsd/mmcsd_sdio.c#L1392 . The debug assert is in addition to that just for debugging.

@davids5 concerning the high level SDMMC driver (drivers/mmcsd/mmcsd_sdio.c) there is e.g. mmcsd_read_csd() function that has a local buffer (uint8_t buffer[512] aligned_data(16);) that is placed on the stack. The stack is allocated within the heap. If SRAM123 or SRAM4 is included into the total heap, and the local buffer is allocated somewhere in SRAM123 or SRAM4 areas, then the buffer address will be passed to SDIO_DMAPREFLIGHT(), the validation will trigger error and the target sdio operation will fail.
There are multiple similar places in NuttX where a not allowed buffer address may be passed to dmapreflight() or directly to stm32h7 sdmmc driver that will result in a failure.
It's still not clear what is the strategy to fix those situations?

[a-lunev]

@a-lunev what are you trying to achieve? Is this similar to the BDMA/SRAM4 heap clobbering issue fixed in 4716fc929d7 / PR-3198?

Hi @hartmannathan,
Your PR-3198 is different to my PR.
In your PR SRAM4 should be used by BDMA because BDMA allows access only to SRAM4 that was clobbered by the heap and vice versa.
In my PR SRAM123 and SRAM4 should not be used by SDMMC1 IDMA because SDMMC1 IDMA does not provide access to SRAM123 and SRAM4 (stm32h7 hardware limitation). SDMMC1 IDMA can access only AXI-SRAM memory region, that is the main part of the heap, and there is not a clobbering issue.

[davids5]

It's still not clear what is the strategy to fix those situations?

On the buffer. It is not dache-aligned either. Using the granular allocator, is the best approach to get DMA & aligned buffers.

We use SDMMMC2 So I would have to look at this again to see what happens on SDMMC1 on mmcsd_read_csd() call.

[xiaoxiang781216]

@davids5 @a-lunev what's the next step for this PR?

[davids5]

@a-lunev I am looking into this on HW that supports SDMMC1. The code path where mmcsd_read_csd is for CONFIG_MMCSD_MMCSUPPORT. I have CONFIG_MMCSD_MMCSUPPORT disabled since there was a commit that broke the detection (while still in bitbucket). Have you a config you can test this code path?

As I see it way to solve this are to a) use CONFIG_GRAN or b) statically define the buffer using locate_data. The latter is more wasteful.

My suggestion is to add CONFIG_GRAN and a dma allocattor. (see https://github.com/PX4/PX4-Autopilot/blob/master/platforms/nuttx/src/px4/common/board_dma_alloc.c) Then in mmcsd_read_csd add compile time code to call board_dma_alloc to alocate the buffer, then free it after use with board_dma_free.

[a-lunev]

Hi David,
I've checked User Manuals for stm32h7 boards.
STM32H745I-DISCO, STM32H750B-DK and STM32H7B3I-DK have eMMC memory chip connected to SDMMC1, however I have neither of them.

One more possible option could be to solder wires to SD or MMC connector, insert MMC card and connect the wires to some stm32h7 board (e.g. NUCLEO-H743ZI) to its SDMMC1 interface.

[a-lunev]

This PR is still actual. I am going to return to it later.