On Fri, Mar 26, 2021 at 1:08 AM Gregory Nutt <spudan...@gmail.com> wrote:
> >> Just don't add it to the primary heap:  Add the region below and the
> >> region above the DMA buffers with two calls to mm_addregion().
> >>
> >> addregion (DMABUF_END, SRAM4_END - DMABUF_END, "SRAM4-A");
> >>
> >> addregion (SRAM4_START, DMABUF_START- SRAM4_START, "SRAM4-B");
> > Where should I call mm_addregion() from?
> up_addregion().  It is normally in the same file as up_allocateheap()
> > Right now, arm_addregion() in
> > arch/arm/src/stm32h7/stm32_allocateheap.c is unconditionally setting
> > up heap regions. I think that customizing it for my particular
> > situation is an ugly hack.
>
> It is customized in all MCUs that have complex RAM maps.  Using with a
> Kconfig setting that defines the start and end of RAM regions.  So only
> the configured part of the RAM region is added to the heap.
>
> I don't think the is ugly in the C code.  It is just a change in the
> naming so that the start and size start with CONFIG_

Ah, yes, I see what you mean.

Right now, arm_addregion() is defining those maps by directly using
defines from arch/arm/src/stm32h7/hardware/stm32h7x3xx_memorymap.h:

#define STM32_DTCRAM_BASE    0x20000000     /* 0x20000000-0x2001ffff:
DTCM-RAM on TCM interface */
#define STM32_AXISRAM_BASE   0x24000000     /* 0x24000000-0x247fffff:
System AXI SRAM */
#define STM32_SRAM1_BASE     0x30000000     /* 0x30000000-0x3001ffff:
System SRAM1 */
#define STM32_SRAM2_BASE     0x30020000     /* 0x30020000-0x3003ffff:
System SRAM2 */
#define STM32_SRAM3_BASE     0x3004c000     /* 0x30040000-0x30047fff:
System SRAM3 */
#define STM32_SRAM423_BASE   0x30000000     /* 0x30000000-0x30047fff:
System SRAM423 */
#define STM32_SRAM4_BASE     0x38000000     /* 0x38000000-0x3800ffff:
System SRAM4 */
#define STM32_BBSRAM_BASE    0x38800000     /* 0x38800000-0x38800fff:
System Backup SRAM */

So, if I understand you suggestion, we could have Kconfigs to
customize those ranges, and use the defaults unless customized.

But I think that could be error prone: if the user adds another
special buffer and forgets to update their customized Kconfig, then
the heap/buffer clobber problem returns.

I'd feel better with an automatic solution... What if C code contains
something like this, say, for adding SRAM4 to the heap:

#ifndef CONFIG_STM32H7_SRAM4EXCLUDE
#  define SRAM4_SIZE   (CONFIG_SRAM4_HEAP_END - CONFIG_SRAM4_HEAP_START)

static uint8_t g_sram4_reserve[SRAM4_SIZE]
__attribute__ ((section(".sram4")))
__attribute__ ((aligned (whatever)));
#endif

Then we add that buffer to the heap with:

addregion((uintptr_t) g_sram4_reserve, (uint32_t)
sizeof(g_sram4_reserve), "SRAM4");

Now if the user adds another buffer to .sram4 and forgets to update
the Kconfig to take it into account, the linker will throw an error,
preventing using firmware that contains the clobbering problem.

Speaking of the linker, is there a way to use a combination of the
linker script and __attribute__ incantations in the code to detect
automatically the size that g_sram4_reserve should be and entirely
eliminate the need for the user to specify the start and end of each
region in Kconfig?

Thanks,
Nathan

Reply via email to