You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nuttx.apache.org by Nathan Hartman <ha...@gmail.com> on 2021/03/25 23:27:36 UTC

How to ensure HEAP will not overlap static DMA buffer?

tl;dr: How do I statically allocate DMA buffers and prevent the heap
from overlapping them?

Details: On STM32H7, BDMA can only reach SRAM4 (64 KB starting at
0x3800:0000). The armv7m DCache line size means the buffers must be
allocated on a cache line boundary. So a DMA buffer must meet 2
requirements: (1) be in SRAM4, and (2) be aligned and padded to 32
byte increments.

Ok, I can do this easily with:

static volatile uint8_t my_bdma_buf[128] __attribute__ ((section
(".sram4"))) __attribute__ ((aligned (32)));

or equivalent.

But then the heap will overlap this memory and my buffer will be
clobbered because arch/arm/src/stm32h7/stm32_allocateheap.c
arm_addregion() does this:

addregion (SRAM4_START, SRAM4_END - SRAM4_START, "SRAM4");

Looking into it, I see that in arch/arm/src/stm32h7/stm32_spi.c,
g_spi6_txbuf[] and g_spi6_rxbuf[] are declared like this:

static uint8_t g_spi6_txbuf[SPI6_DMABUFSIZE_ADJUSTED]
SPI6_DMABUFSIZE_ALGN locate_data(".sram4");
static uint8_t g_spi6_rxbuf[SPI6_DMABUFSIZE_ADJUSTED]
SPI6_DMABUFSIZE_ALGN locate_data(".sram4");

So they will suffer that same problem. (I'm pretty sure that's a bug,
unless I missed something.)

I discovered this the hard way, i.e., on my board, NSH suddenly
wouldn't start. In reality, it does start but it turns out that when
it tries to print the greeting message, it fails on the very first
character ('\n') and exits. Why? I single-stepped all the way into the
guts of the serial logic, and found that in fs/vfs/fs_write.c,
file_write(), it fetches the inode and tests if it's valid (!inode ||
!inode->u.i_ops || !inode->u.i_ops->write) or returns -EBADF. That's
where it fails, because it seems that the inode is allocated at
0x3800:0070, right on top of my BDMA buffers!

I don't know which question to ask, but I think it's one of these:

(1) Is there a standard(-ish) way to allocate a buffer that is both
aligned to a specified boundary AND in a specified region of memory?

(2) Is there a standard way to make a hole in the heap so that nothing
else in NuttX will overwrite the DMA buffer?

(Yes, I see that I can fiddle with CONFIG_MM_REGIONS, but then only
the 512 KB of AXI SRAM will be in the heap, and none of the other
regions, SRAM1,2,3, etc., and I don't think I want that.)

Thanks,
Nathan

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Nathan Hartman <ha...@gmail.com>.
On Fri, Mar 26, 2021 at 12:25 PM Fotis Panagiotopoulos
<f....@gmail.com> wrote:
>
> I have attached the linkerscripts that I was using in some older projects.

The attachment(s) did not come through. Could you resend them with a
.txt extension to get around the filter?

Thanks,
Nathan

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Fotis Panagiotopoulos <f....@gmail.com>.
I have attached the linkerscripts that I was using in some older projects.
My intention was to eventually reach NuttX to the same degree of
flexibility.
Maybe even use linkerscripts like this.

You can define absolutely any region that you may need, in any position and
size etc etc.
I have also included general-purpose regions, that helped me in cases like
the above.
It does not support multiple heap regions, but I guess this is a very
simple change.

Going further, maybe a file like rules.ld can belong to the arch itself,
and have the board to only provide the STM32F42xxI.ld like file.
This will greatly simplify the initial set-up of the memory map, will
reduce the number of (repeated) files along the various boards,
and generally make everything more maintainable, I think.

Even boards of different arches will be able to retain the same format,
provide the same definitions etc...

For the time I have to work on some changes/fixes in syslog.
I may soon be able to contribute this, but I would greatly appreciate any
help.


Στις Παρ, 26 Μαρ 2021 στις 6:17 μ.μ., ο/η Fotis Panagiotopoulos <
f.j.panag@gmail.com> έγραψε:

> I face similar problems (with a different use case) on an STM32F4.
> The thing is that although the linker script belongs to the board logic
> and thus it is freely customizable, the heap regions are hard-coded in the
> arch files.
>
> So, I started working on PR #2277 (
> https://github.com/apache/incubator-nuttx/pull/2277), but unfortunately I
> had to pause the development on this.
> The idea is similar to what you describe here. Everything can be defined
> in the linkerscript (addresses, order, sizeses etc).
>
> I was thinking a lot of any alternatives on this. I came to the conclusion
> that Kconfig is the wrong tool for this job.
> You lose all compile-time (and CI) checks and can easily be misconfigured.
> I am also afraid that we will end up with a few dozen "hacks" like above or
> like STM32_CCMEXCLUDE (I never liked this option....).
> And no matter what you do, you will never be able to satisfy any crazy
> memory mappings that any project may need.
>
> A similar issue to this is Issue #2001 (
> https://github.com/apache/incubator-nuttx/issues/2001).
> This was my first crash while trying out NuttX :)
> In short, there is the assumption that the main stack will always reside
> between BSS and Heap, again being very restrictive.
>
>
> Στις Παρ, 26 Μαρ 2021 στις 5:46 μ.μ., ο/η Nathan Hartman <
> hartman.nathan@gmail.com> έγραψε:
>
>> On Fri, Mar 26, 2021 at 11:41 AM Gregory Nutt <sp...@gmail.com>
>> wrote:
>>
>> > Missing bit of logic:
>> >
>> > >> 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?
>> > >
>> > > Are you thinking of something like this in the linker script:
>> > >
>> > >     .sram4_reserve :
>> > >     {
>> > >       _sram4_reserve_begin = ABSOLUTE(.);
>> > >       *(.sram4)
>> > >       _sram4_reserve_end = ABSOLUTE(.);
>> > >     }
>> > >
>> > > And in the C code:
>> > >
>> > We need to lie to C and tell it what to think those symbols are:
>> >
>> >     EXTERN const uint32_t _sram4_reserve_begin
>> >     EXTERN const uint32_t _sram4_reserve_begin
>>
>>
>>
>> Ah, yes, otherwise those symbols would be undefined. Later the linker will
>> resolve them to the correct addresses.
>>
>>
>> >     #define SRAM4_RESERVE_BEGIN &_sram4_reserve_begin
>> > >     #define SRAM4_RESERVE_END &_sram4_reserve_end
>> > >
>> > > The implied size depends on the size of all .sram4 sections.  I assume
>> > > this would be positioned at the beginning of SRAM4 and the size of the
>> > > region that could be added to the heap would be SRAM4_RESERVE_END
>> > > through SRAM_END.
>> > >
>> > You can see this same kind of thing in, for example,
>> > arch/arm/src/common/arm_internal.h
>>
>>
>>
>> Great! Thanks
>>
>> Nathan
>>
>

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Xiang Xiao <xi...@gmail.com>.
There is a smart way to fix this problem I learned from the CMSIS package,
and is used in our chip for several years.
The key idea is create the array directly inside link script, like this:
 .copy.table : {
_scopytable = ABSOLUTE(.);
LONG(LOADADDR(.cpram1.data))
LONG(ADDR(.cpram1.data))
LONG(ADDR(.cpram1.data) + SIZEOF(.cpram1.data))
_ecopytable = ABSOLUTE(.);
} > flash

.zero.table : {
_szerotable = ABSOLUTE(.);
LONG(_cpram0_sbss)
LONG(_cpram0_ebss)
LONG(ADDR(.cpram1.bss))
LONG(ADDR(.cpram1.bss) + SIZEOF(.cpram1.bss))
_ezerotable = ABSOLUTE(.);
} > flash
And then iterate through these tables in the startup:
https://github.com/FishsemiCode/nuttx/blob/song-u1/arch/arm/src/song/song_start.c#L56-L84
https://github.com/FishsemiCode/nuttx/blob/song-u1/arch/arm/src/song/song_start.c#L225-L237
The similar approach can be applied to the heap too.
This method can support:

   1. Any humber of memory region
   2. Only need provide the info in one place(link script)
   3. Adjust the boundary automatically by linker


On Fri, Mar 26, 2021 at 12:25 PM Fotis Panagiotopoulos <f....@gmail.com>
wrote:

> In my case I define in the linkerscript all regions that "I will ever
> need", so they are always there.
> The startup code can always find the predetermined symbols.
>
> If I don't use a region, I set it to DONT_LINK. Since it's size will be 0,
> the start up code will ignore it.
>
> I know, it's not the most elegant solution, but it's simple and working. I
> cannot image a system with 50 different heap regions, so it doesn't need to
> scale.
>
> Alternatively, the number (or names) of regions can be a Make.defs list.
> Every board has a Make.defs that defines the platform specifics, so it's a
> good candidate IMO.
> (I think I have done this in the past in a similar way, but I don't have
> access to this code anymore).
>
>
> On Fri, Mar 26, 2021, 20:56 Xiang Xiao <xi...@gmail.com> wrote:
>
> > Fotis, you define many symbols(e.g. __gp_ram?_bss_start__) in your linker
> > script. My question is how the common init code knows the number of these
> > symbols?
> >
> > On Fri, Mar 26, 2021 at 9:46 AM Fotis Panagiotopoulos <
> f.j.panag@gmail.com
> > >
> > wrote:
> >
> > > Oh, sorry.
> > > Attached again as .txt. Is it OK now?
> > >
> > > > A tool that takes the Kconfig + chip+ memorymap and make a linker
> > include
> > > > file and the config code for the heap may be the way to go.
> > >
> > > I am pretty sure that a "tool" will not be able to cover all use cases.
> > > Over the years I had to make custom scripts to account for:
> > > * Bootloaders
> > > * Binary blops
> > > * Firmware headers
> > > * ROM files
> > > * DMA buffers
> > > * External memories
> > >  etc etc..
> > >
> > > Do you believe that a tool can be made that can handle everything?
> > >
> > >
> > > Στις Παρ, 26 Μαρ 2021 στις 6:37 μ.μ., ο/η David Sidrane <
> > > David.Sidrane@nscdg.com> έγραψε:
> > >
> > >> I am just thinking out load...
> > >>
> > >> I agree this has to come from one place. But I do think it is just the
> > >> linker file.
> > >>
> > >> Currently we have
> > >> The arch memroymap h files have the base addresses, sizes  - This is
> > >> the
> > >> Reference manuals counterpart, it covers all the sub members of the
> > chips)
> > >> The chip.h files  that has sizes  - This is the Data Sheet
> counterpart,
> > it
> > >> covers one or more of the sub members of the chips)
> > >> The Kconfig - More flexible from a users stand point.
> > >> The heap c files - buried semantics - not good
> > >> linker file - the boards usage specific.
> > >>
> > >> I like using the linker file, but Kconfig is build time - no file
> > >> modification
> > >>
> > >> Just moving things to the linker file does not fix the ordering or
> > adding
> > >> issues. (it is link time not compile time)
> > >>
> > >> A tool that takes the Kconfig + chip+ memorymap and make a linker
> > include
> > >> file and the config code for the heap may be the way to go.
> > >>
> > >> David
> > >>
> > >>
> > >> -----Original Message-----
> > >> From: Fotis Panagiotopoulos [mailto:f.j.panag@gmail.com]
> > >> Sent: Friday, March 26, 2021 9:17 AM
> > >> To: dev@nuttx.apache.org
> > >> Subject: Re: How to ensure HEAP will not overlap static DMA buffer?
> > >>
> > >> I face similar problems (with a different use case) on an STM32F4.
> > >> The thing is that although the linker script belongs to the board
> logic
> > >> and
> > >> thus it is freely customizable, the heap regions are hard-coded in the
> > >> arch
> > >> files.
> > >>
> > >> So, I started working on PR #2277 (
> > >> https://github.com/apache/incubator-nuttx/pull/2277), but
> > unfortunately I
> > >> had to pause the development on this.
> > >> The idea is similar to what you describe here. Everything can be
> > >> defined
> > >> in
> > >> the linkerscript (addresses, order, sizeses etc).
> > >>
> > >> I was thinking a lot of any alternatives on this. I came to the
> > conclusion
> > >> that Kconfig is the wrong tool for this job.
> > >> You lose all compile-time (and CI) checks and can easily be
> > misconfigured.
> > >> I am also afraid that we will end up with a few dozen "hacks" like
> > >> above
> > >> or
> > >> like STM32_CCMEXCLUDE (I never liked this option....).
> > >> And no matter what you do, you will never be able to satisfy any crazy
> > >> memory mappings that any project may need.
> > >>
> > >> A similar issue to this is Issue #2001 (
> > >> https://github.com/apache/incubator-nuttx/issues/2001).
> > >> This was my first crash while trying out NuttX :)
> > >> In short, there is the assumption that the main stack will always
> > >> reside
> > >> between BSS and Heap, again being very restrictive.
> > >>
> > >>
> > >> Στις Παρ, 26 Μαρ 2021 στις 5:46 μ.μ., ο/η Nathan Hartman <
> > >> hartman.nathan@gmail.com> έγραψε:
> > >>
> > >> > On Fri, Mar 26, 2021 at 11:41 AM Gregory Nutt <sp...@gmail.com>
> > >> wrote:
> > >> >
> > >> > > Missing bit of logic:
> > >> > >
> > >> > > >> 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?
> > >> > > >
> > >> > > > Are you thinking of something like this in the linker script:
> > >> > > >
> > >> > > >     .sram4_reserve :
> > >> > > >     {
> > >> > > >       _sram4_reserve_begin = ABSOLUTE(.);
> > >> > > >       *(.sram4)
> > >> > > >       _sram4_reserve_end = ABSOLUTE(.);
> > >> > > >     }
> > >> > > >
> > >> > > > And in the C code:
> > >> > > >
> > >> > > We need to lie to C and tell it what to think those symbols are:
> > >> > >
> > >> > >     EXTERN const uint32_t _sram4_reserve_begin
> > >> > >     EXTERN const uint32_t _sram4_reserve_begin
> > >> >
> > >> >
> > >> >
> > >> > Ah, yes, otherwise those symbols would be undefined. Later the
> linker
> > >> will
> > >> > resolve them to the correct addresses.
> > >> >
> > >> >
> > >> > >     #define SRAM4_RESERVE_BEGIN &_sram4_reserve_begin
> > >> > > >     #define SRAM4_RESERVE_END &_sram4_reserve_end
> > >> > > >
> > >> > > > The implied size depends on the size of all .sram4 sections.  I
> > >> assume
> > >> > > > this would be positioned at the beginning of SRAM4 and the size
> > >> > > > of
> > >> the
> > >> > > > region that could be added to the heap would be
> SRAM4_RESERVE_END
> > >> > > > through SRAM_END.
> > >> > > >
> > >> > > You can see this same kind of thing in, for example,
> > >> > > arch/arm/src/common/arm_internal.h
> > >> >
> > >> >
> > >> >
> > >> > Great! Thanks
> > >> >
> > >> > Nathan
> > >> >
> > >>
> > >
> >
>

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Fotis Panagiotopoulos <f....@gmail.com>.
In my case I define in the linkerscript all regions that "I will ever
need", so they are always there.
The startup code can always find the predetermined symbols.

If I don't use a region, I set it to DONT_LINK. Since it's size will be 0,
the start up code will ignore it.

I know, it's not the most elegant solution, but it's simple and working. I
cannot image a system with 50 different heap regions, so it doesn't need to
scale.

Alternatively, the number (or names) of regions can be a Make.defs list.
Every board has a Make.defs that defines the platform specifics, so it's a
good candidate IMO.
(I think I have done this in the past in a similar way, but I don't have
access to this code anymore).


On Fri, Mar 26, 2021, 20:56 Xiang Xiao <xi...@gmail.com> wrote:

> Fotis, you define many symbols(e.g. __gp_ram?_bss_start__) in your linker
> script. My question is how the common init code knows the number of these
> symbols?
>
> On Fri, Mar 26, 2021 at 9:46 AM Fotis Panagiotopoulos <f.j.panag@gmail.com
> >
> wrote:
>
> > Oh, sorry.
> > Attached again as .txt. Is it OK now?
> >
> > > A tool that takes the Kconfig + chip+ memorymap and make a linker
> include
> > > file and the config code for the heap may be the way to go.
> >
> > I am pretty sure that a "tool" will not be able to cover all use cases.
> > Over the years I had to make custom scripts to account for:
> > * Bootloaders
> > * Binary blops
> > * Firmware headers
> > * ROM files
> > * DMA buffers
> > * External memories
> >  etc etc..
> >
> > Do you believe that a tool can be made that can handle everything?
> >
> >
> > Στις Παρ, 26 Μαρ 2021 στις 6:37 μ.μ., ο/η David Sidrane <
> > David.Sidrane@nscdg.com> έγραψε:
> >
> >> I am just thinking out load...
> >>
> >> I agree this has to come from one place. But I do think it is just the
> >> linker file.
> >>
> >> Currently we have
> >> The arch memroymap h files have the base addresses, sizes  - This is the
> >> Reference manuals counterpart, it covers all the sub members of the
> chips)
> >> The chip.h files  that has sizes  - This is the Data Sheet counterpart,
> it
> >> covers one or more of the sub members of the chips)
> >> The Kconfig - More flexible from a users stand point.
> >> The heap c files - buried semantics - not good
> >> linker file - the boards usage specific.
> >>
> >> I like using the linker file, but Kconfig is build time - no file
> >> modification
> >>
> >> Just moving things to the linker file does not fix the ordering or
> adding
> >> issues. (it is link time not compile time)
> >>
> >> A tool that takes the Kconfig + chip+ memorymap and make a linker
> include
> >> file and the config code for the heap may be the way to go.
> >>
> >> David
> >>
> >>
> >> -----Original Message-----
> >> From: Fotis Panagiotopoulos [mailto:f.j.panag@gmail.com]
> >> Sent: Friday, March 26, 2021 9:17 AM
> >> To: dev@nuttx.apache.org
> >> Subject: Re: How to ensure HEAP will not overlap static DMA buffer?
> >>
> >> I face similar problems (with a different use case) on an STM32F4.
> >> The thing is that although the linker script belongs to the board logic
> >> and
> >> thus it is freely customizable, the heap regions are hard-coded in the
> >> arch
> >> files.
> >>
> >> So, I started working on PR #2277 (
> >> https://github.com/apache/incubator-nuttx/pull/2277), but
> unfortunately I
> >> had to pause the development on this.
> >> The idea is similar to what you describe here. Everything can be defined
> >> in
> >> the linkerscript (addresses, order, sizeses etc).
> >>
> >> I was thinking a lot of any alternatives on this. I came to the
> conclusion
> >> that Kconfig is the wrong tool for this job.
> >> You lose all compile-time (and CI) checks and can easily be
> misconfigured.
> >> I am also afraid that we will end up with a few dozen "hacks" like above
> >> or
> >> like STM32_CCMEXCLUDE (I never liked this option....).
> >> And no matter what you do, you will never be able to satisfy any crazy
> >> memory mappings that any project may need.
> >>
> >> A similar issue to this is Issue #2001 (
> >> https://github.com/apache/incubator-nuttx/issues/2001).
> >> This was my first crash while trying out NuttX :)
> >> In short, there is the assumption that the main stack will always reside
> >> between BSS and Heap, again being very restrictive.
> >>
> >>
> >> Στις Παρ, 26 Μαρ 2021 στις 5:46 μ.μ., ο/η Nathan Hartman <
> >> hartman.nathan@gmail.com> έγραψε:
> >>
> >> > On Fri, Mar 26, 2021 at 11:41 AM Gregory Nutt <sp...@gmail.com>
> >> wrote:
> >> >
> >> > > Missing bit of logic:
> >> > >
> >> > > >> 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?
> >> > > >
> >> > > > Are you thinking of something like this in the linker script:
> >> > > >
> >> > > >     .sram4_reserve :
> >> > > >     {
> >> > > >       _sram4_reserve_begin = ABSOLUTE(.);
> >> > > >       *(.sram4)
> >> > > >       _sram4_reserve_end = ABSOLUTE(.);
> >> > > >     }
> >> > > >
> >> > > > And in the C code:
> >> > > >
> >> > > We need to lie to C and tell it what to think those symbols are:
> >> > >
> >> > >     EXTERN const uint32_t _sram4_reserve_begin
> >> > >     EXTERN const uint32_t _sram4_reserve_begin
> >> >
> >> >
> >> >
> >> > Ah, yes, otherwise those symbols would be undefined. Later the linker
> >> will
> >> > resolve them to the correct addresses.
> >> >
> >> >
> >> > >     #define SRAM4_RESERVE_BEGIN &_sram4_reserve_begin
> >> > > >     #define SRAM4_RESERVE_END &_sram4_reserve_end
> >> > > >
> >> > > > The implied size depends on the size of all .sram4 sections.  I
> >> assume
> >> > > > this would be positioned at the beginning of SRAM4 and the size of
> >> the
> >> > > > region that could be added to the heap would be SRAM4_RESERVE_END
> >> > > > through SRAM_END.
> >> > > >
> >> > > You can see this same kind of thing in, for example,
> >> > > arch/arm/src/common/arm_internal.h
> >> >
> >> >
> >> >
> >> > Great! Thanks
> >> >
> >> > Nathan
> >> >
> >>
> >
>

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Xiang Xiao <xi...@gmail.com>.
Fotis, you define many symbols(e.g. __gp_ram?_bss_start__) in your linker
script. My question is how the common init code knows the number of these
symbols?

On Fri, Mar 26, 2021 at 9:46 AM Fotis Panagiotopoulos <f....@gmail.com>
wrote:

> Oh, sorry.
> Attached again as .txt. Is it OK now?
>
> > A tool that takes the Kconfig + chip+ memorymap and make a linker include
> > file and the config code for the heap may be the way to go.
>
> I am pretty sure that a "tool" will not be able to cover all use cases.
> Over the years I had to make custom scripts to account for:
> * Bootloaders
> * Binary blops
> * Firmware headers
> * ROM files
> * DMA buffers
> * External memories
>  etc etc..
>
> Do you believe that a tool can be made that can handle everything?
>
>
> Στις Παρ, 26 Μαρ 2021 στις 6:37 μ.μ., ο/η David Sidrane <
> David.Sidrane@nscdg.com> έγραψε:
>
>> I am just thinking out load...
>>
>> I agree this has to come from one place. But I do think it is just the
>> linker file.
>>
>> Currently we have
>> The arch memroymap h files have the base addresses, sizes  - This is the
>> Reference manuals counterpart, it covers all the sub members of the chips)
>> The chip.h files  that has sizes  - This is the Data Sheet counterpart, it
>> covers one or more of the sub members of the chips)
>> The Kconfig - More flexible from a users stand point.
>> The heap c files - buried semantics - not good
>> linker file - the boards usage specific.
>>
>> I like using the linker file, but Kconfig is build time - no file
>> modification
>>
>> Just moving things to the linker file does not fix the ordering or adding
>> issues. (it is link time not compile time)
>>
>> A tool that takes the Kconfig + chip+ memorymap and make a linker include
>> file and the config code for the heap may be the way to go.
>>
>> David
>>
>>
>> -----Original Message-----
>> From: Fotis Panagiotopoulos [mailto:f.j.panag@gmail.com]
>> Sent: Friday, March 26, 2021 9:17 AM
>> To: dev@nuttx.apache.org
>> Subject: Re: How to ensure HEAP will not overlap static DMA buffer?
>>
>> I face similar problems (with a different use case) on an STM32F4.
>> The thing is that although the linker script belongs to the board logic
>> and
>> thus it is freely customizable, the heap regions are hard-coded in the
>> arch
>> files.
>>
>> So, I started working on PR #2277 (
>> https://github.com/apache/incubator-nuttx/pull/2277), but unfortunately I
>> had to pause the development on this.
>> The idea is similar to what you describe here. Everything can be defined
>> in
>> the linkerscript (addresses, order, sizeses etc).
>>
>> I was thinking a lot of any alternatives on this. I came to the conclusion
>> that Kconfig is the wrong tool for this job.
>> You lose all compile-time (and CI) checks and can easily be misconfigured.
>> I am also afraid that we will end up with a few dozen "hacks" like above
>> or
>> like STM32_CCMEXCLUDE (I never liked this option....).
>> And no matter what you do, you will never be able to satisfy any crazy
>> memory mappings that any project may need.
>>
>> A similar issue to this is Issue #2001 (
>> https://github.com/apache/incubator-nuttx/issues/2001).
>> This was my first crash while trying out NuttX :)
>> In short, there is the assumption that the main stack will always reside
>> between BSS and Heap, again being very restrictive.
>>
>>
>> Στις Παρ, 26 Μαρ 2021 στις 5:46 μ.μ., ο/η Nathan Hartman <
>> hartman.nathan@gmail.com> έγραψε:
>>
>> > On Fri, Mar 26, 2021 at 11:41 AM Gregory Nutt <sp...@gmail.com>
>> wrote:
>> >
>> > > Missing bit of logic:
>> > >
>> > > >> 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?
>> > > >
>> > > > Are you thinking of something like this in the linker script:
>> > > >
>> > > >     .sram4_reserve :
>> > > >     {
>> > > >       _sram4_reserve_begin = ABSOLUTE(.);
>> > > >       *(.sram4)
>> > > >       _sram4_reserve_end = ABSOLUTE(.);
>> > > >     }
>> > > >
>> > > > And in the C code:
>> > > >
>> > > We need to lie to C and tell it what to think those symbols are:
>> > >
>> > >     EXTERN const uint32_t _sram4_reserve_begin
>> > >     EXTERN const uint32_t _sram4_reserve_begin
>> >
>> >
>> >
>> > Ah, yes, otherwise those symbols would be undefined. Later the linker
>> will
>> > resolve them to the correct addresses.
>> >
>> >
>> > >     #define SRAM4_RESERVE_BEGIN &_sram4_reserve_begin
>> > > >     #define SRAM4_RESERVE_END &_sram4_reserve_end
>> > > >
>> > > > The implied size depends on the size of all .sram4 sections.  I
>> assume
>> > > > this would be positioned at the beginning of SRAM4 and the size of
>> the
>> > > > region that could be added to the heap would be SRAM4_RESERVE_END
>> > > > through SRAM_END.
>> > > >
>> > > You can see this same kind of thing in, for example,
>> > > arch/arm/src/common/arm_internal.h
>> >
>> >
>> >
>> > Great! Thanks
>> >
>> > Nathan
>> >
>>
>

RE: How to ensure HEAP will not overlap static DMA buffer?

Posted by David Sidrane <Da...@nscdg.com>.
That may be true, but I think that the dependencies and composition of a
board can and should be defined in the Kconfig. Since the board that are in
tree in NuttX are standard, it would need to be extensible to support
custom HW and the use cases you list. But again that is not compile time,
it is link time and there are dependencies that need to be matained.



*From:* Fotis Panagiotopoulos [mailto:f.j.panag@gmail.com]
*Sent:* Friday, March 26, 2021 9:46 AM
*To:* dev@nuttx.apache.org
*Subject:* Re: How to ensure HEAP will not overlap static DMA buffer?



Oh, sorry.

Attached again as .txt. Is it OK now?



> A tool that takes the Kconfig + chip+ memorymap and make a linker include
> file and the config code for the heap may be the way to go.



I am pretty sure that a "tool" will not be able to cover all use cases.

Over the years I had to make custom scripts to account for:

* Bootloaders

* Binary blops

* Firmware headers

* ROM files

* DMA buffers

* External memories

 etc etc..



Do you believe that a tool can be made that can handle everything?





Στις Παρ, 26 Μαρ 2021 στις 6:37 μ.μ., ο/η David Sidrane <
David.Sidrane@nscdg.com> έγραψε:

I am just thinking out load...

I agree this has to come from one place. But I do think it is just the
linker file.

Currently we have
The arch memroymap h files have the base addresses, sizes  - This is the
Reference manuals counterpart, it covers all the sub members of the chips)
The chip.h files  that has sizes  - This is the Data Sheet counterpart, it
covers one or more of the sub members of the chips)
The Kconfig - More flexible from a users stand point.
The heap c files - buried semantics - not good
linker file - the boards usage specific.

I like using the linker file, but Kconfig is build time - no file
modification

Just moving things to the linker file does not fix the ordering or adding
issues. (it is link time not compile time)

A tool that takes the Kconfig + chip+ memorymap and make a linker include
file and the config code for the heap may be the way to go.

David


-----Original Message-----
From: Fotis Panagiotopoulos [mailto:f.j.panag@gmail.com]
Sent: Friday, March 26, 2021 9:17 AM
To: dev@nuttx.apache.org
Subject: Re: How to ensure HEAP will not overlap static DMA buffer?

I face similar problems (with a different use case) on an STM32F4.
The thing is that although the linker script belongs to the board logic and
thus it is freely customizable, the heap regions are hard-coded in the arch
files.

So, I started working on PR #2277 (
https://github.com/apache/incubator-nuttx/pull/2277), but unfortunately I
had to pause the development on this.
The idea is similar to what you describe here. Everything can be defined in
the linkerscript (addresses, order, sizeses etc).

I was thinking a lot of any alternatives on this. I came to the conclusion
that Kconfig is the wrong tool for this job.
You lose all compile-time (and CI) checks and can easily be misconfigured.
I am also afraid that we will end up with a few dozen "hacks" like above or
like STM32_CCMEXCLUDE (I never liked this option....).
And no matter what you do, you will never be able to satisfy any crazy
memory mappings that any project may need.

A similar issue to this is Issue #2001 (
https://github.com/apache/incubator-nuttx/issues/2001).
This was my first crash while trying out NuttX :)
In short, there is the assumption that the main stack will always reside
between BSS and Heap, again being very restrictive.


Στις Παρ, 26 Μαρ 2021 στις 5:46 μ.μ., ο/η Nathan Hartman <
hartman.nathan@gmail.com> έγραψε:

> On Fri, Mar 26, 2021 at 11:41 AM Gregory Nutt <sp...@gmail.com> wrote:
>
> > Missing bit of logic:
> >
> > >> 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?
> > >
> > > Are you thinking of something like this in the linker script:
> > >
> > >     .sram4_reserve :
> > >     {
> > >       _sram4_reserve_begin = ABSOLUTE(.);
> > >       *(.sram4)
> > >       _sram4_reserve_end = ABSOLUTE(.);
> > >     }
> > >
> > > And in the C code:
> > >
> > We need to lie to C and tell it what to think those symbols are:
> >
> >     EXTERN const uint32_t _sram4_reserve_begin
> >     EXTERN const uint32_t _sram4_reserve_begin
>
>
>
> Ah, yes, otherwise those symbols would be undefined. Later the linker will
> resolve them to the correct addresses.
>
>
> >     #define SRAM4_RESERVE_BEGIN &_sram4_reserve_begin
> > >     #define SRAM4_RESERVE_END &_sram4_reserve_end
> > >
> > > The implied size depends on the size of all .sram4 sections.  I assume
> > > this would be positioned at the beginning of SRAM4 and the size of the
> > > region that could be added to the heap would be SRAM4_RESERVE_END
> > > through SRAM_END.
> > >
> > You can see this same kind of thing in, for example,
> > arch/arm/src/common/arm_internal.h
>
>
>
> Great! Thanks
>
> Nathan
>

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Fotis Panagiotopoulos <f....@gmail.com>.
I really don't see any simpler and more flexible way to define the memory
map than linker scripts.
This is the purpose of the script itself.

On the other hand though, I cannot argue against a tool that I haven't seen
yet...

In any case, I think that we agree that step no 1 is always the same.
The code must properly place everything to the correct region, so the
linker can produce the desired result.

That is, the heap definition shall change, and all "interesting" parts of
the memory shall be declared in an __attribute__((section(""))) way.



Στις Παρ, 26 Μαρ 2021 στις 7:32 μ.μ., ο/η Nathan Hartman <
hartman.nathan@gmail.com> έγραψε:

> On Fri, Mar 26, 2021 at 12:46 PM Fotis Panagiotopoulos
> <f....@gmail.com> wrote:
> > Oh, sorry.
> > Attached again as .txt. Is it OK now?
>
> Yes, the files went through this time. Thanks! More below...
>
> > > A tool that takes the Kconfig + chip+ memorymap and make a linker
> include
> > > file and the config code for the heap may be the way to go.
> >
> > I am pretty sure that a "tool" will not be able to cover all use cases.
> > Over the years I had to make custom scripts to account for:
> > * Bootloaders
> > * Binary blops
> > * Firmware headers
> > * ROM files
> > * DMA buffers
> > * External memories
> >  etc etc..
> >
> > Do you believe that a tool can be made that can handle everything?
>
> It is very annoying to have to do all of that, and error prone, and
> it's pretty awful when there's a potentially clobbering situation
> (like a heap allocation clobbering a DMA buffer) that might not
> manifest for hours or days in a running firmware. Could be very hard
> to debug.
>
> I like the idea of a tool that takes Kconfig settings and emits a
> correct linker script.
>
> Could a tool account for *all* of the above? And, you forgot to
> mention: Different toolchains with their own linker script dialect.
>
> I suppose that at first, it would not support all of that. It would
> probably start out supporting only a small subset of options, and then
> each user who has a different requirement would have to improve the
> tool, and hopefully open a PR so the rest of the community benefits
> from it.
>
> If we want to try this, I would recommend a Kconfig option
> CUSTOM_LINKER_SCRIPT. If set, the tool will not run, leaving your
> custom script in place.
>
> The good thing about such a tool: see for example at the nucleo-h743zi
> board. It has 5 different linker scripts and you have to choose the
> correct one(s), depending on flash, kernel, elf, etc. If a tool could
> generate the right one automatically, it would be harder work up front
> to create the tool, but then it would be much less work in maintenance
> later. Also it would be integrated with in-tree improvements so
> out-of-tree boards don't break every time we change something in the
> tree.
>
> Nathan
>

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Nathan Hartman <ha...@gmail.com>.
On Fri, Mar 26, 2021 at 12:46 PM Fotis Panagiotopoulos
<f....@gmail.com> wrote:
> Oh, sorry.
> Attached again as .txt. Is it OK now?

Yes, the files went through this time. Thanks! More below...

> > A tool that takes the Kconfig + chip+ memorymap and make a linker include
> > file and the config code for the heap may be the way to go.
>
> I am pretty sure that a "tool" will not be able to cover all use cases.
> Over the years I had to make custom scripts to account for:
> * Bootloaders
> * Binary blops
> * Firmware headers
> * ROM files
> * DMA buffers
> * External memories
>  etc etc..
>
> Do you believe that a tool can be made that can handle everything?

It is very annoying to have to do all of that, and error prone, and
it's pretty awful when there's a potentially clobbering situation
(like a heap allocation clobbering a DMA buffer) that might not
manifest for hours or days in a running firmware. Could be very hard
to debug.

I like the idea of a tool that takes Kconfig settings and emits a
correct linker script.

Could a tool account for *all* of the above? And, you forgot to
mention: Different toolchains with their own linker script dialect.

I suppose that at first, it would not support all of that. It would
probably start out supporting only a small subset of options, and then
each user who has a different requirement would have to improve the
tool, and hopefully open a PR so the rest of the community benefits
from it.

If we want to try this, I would recommend a Kconfig option
CUSTOM_LINKER_SCRIPT. If set, the tool will not run, leaving your
custom script in place.

The good thing about such a tool: see for example at the nucleo-h743zi
board. It has 5 different linker scripts and you have to choose the
correct one(s), depending on flash, kernel, elf, etc. If a tool could
generate the right one automatically, it would be harder work up front
to create the tool, but then it would be much less work in maintenance
later. Also it would be integrated with in-tree improvements so
out-of-tree boards don't break every time we change something in the
tree.

Nathan

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Fotis Panagiotopoulos <f....@gmail.com>.
Oh, sorry.
Attached again as .txt. Is it OK now?

> A tool that takes the Kconfig + chip+ memorymap and make a linker include
> file and the config code for the heap may be the way to go.

I am pretty sure that a "tool" will not be able to cover all use cases.
Over the years I had to make custom scripts to account for:
* Bootloaders
* Binary blops
* Firmware headers
* ROM files
* DMA buffers
* External memories
 etc etc..

Do you believe that a tool can be made that can handle everything?


Στις Παρ, 26 Μαρ 2021 στις 6:37 μ.μ., ο/η David Sidrane <
David.Sidrane@nscdg.com> έγραψε:

> I am just thinking out load...
>
> I agree this has to come from one place. But I do think it is just the
> linker file.
>
> Currently we have
> The arch memroymap h files have the base addresses, sizes  - This is the
> Reference manuals counterpart, it covers all the sub members of the chips)
> The chip.h files  that has sizes  - This is the Data Sheet counterpart, it
> covers one or more of the sub members of the chips)
> The Kconfig - More flexible from a users stand point.
> The heap c files - buried semantics - not good
> linker file - the boards usage specific.
>
> I like using the linker file, but Kconfig is build time - no file
> modification
>
> Just moving things to the linker file does not fix the ordering or adding
> issues. (it is link time not compile time)
>
> A tool that takes the Kconfig + chip+ memorymap and make a linker include
> file and the config code for the heap may be the way to go.
>
> David
>
>
> -----Original Message-----
> From: Fotis Panagiotopoulos [mailto:f.j.panag@gmail.com]
> Sent: Friday, March 26, 2021 9:17 AM
> To: dev@nuttx.apache.org
> Subject: Re: How to ensure HEAP will not overlap static DMA buffer?
>
> I face similar problems (with a different use case) on an STM32F4.
> The thing is that although the linker script belongs to the board logic and
> thus it is freely customizable, the heap regions are hard-coded in the arch
> files.
>
> So, I started working on PR #2277 (
> https://github.com/apache/incubator-nuttx/pull/2277), but unfortunately I
> had to pause the development on this.
> The idea is similar to what you describe here. Everything can be defined in
> the linkerscript (addresses, order, sizeses etc).
>
> I was thinking a lot of any alternatives on this. I came to the conclusion
> that Kconfig is the wrong tool for this job.
> You lose all compile-time (and CI) checks and can easily be misconfigured.
> I am also afraid that we will end up with a few dozen "hacks" like above or
> like STM32_CCMEXCLUDE (I never liked this option....).
> And no matter what you do, you will never be able to satisfy any crazy
> memory mappings that any project may need.
>
> A similar issue to this is Issue #2001 (
> https://github.com/apache/incubator-nuttx/issues/2001).
> This was my first crash while trying out NuttX :)
> In short, there is the assumption that the main stack will always reside
> between BSS and Heap, again being very restrictive.
>
>
> Στις Παρ, 26 Μαρ 2021 στις 5:46 μ.μ., ο/η Nathan Hartman <
> hartman.nathan@gmail.com> έγραψε:
>
> > On Fri, Mar 26, 2021 at 11:41 AM Gregory Nutt <sp...@gmail.com>
> wrote:
> >
> > > Missing bit of logic:
> > >
> > > >> 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?
> > > >
> > > > Are you thinking of something like this in the linker script:
> > > >
> > > >     .sram4_reserve :
> > > >     {
> > > >       _sram4_reserve_begin = ABSOLUTE(.);
> > > >       *(.sram4)
> > > >       _sram4_reserve_end = ABSOLUTE(.);
> > > >     }
> > > >
> > > > And in the C code:
> > > >
> > > We need to lie to C and tell it what to think those symbols are:
> > >
> > >     EXTERN const uint32_t _sram4_reserve_begin
> > >     EXTERN const uint32_t _sram4_reserve_begin
> >
> >
> >
> > Ah, yes, otherwise those symbols would be undefined. Later the linker
> will
> > resolve them to the correct addresses.
> >
> >
> > >     #define SRAM4_RESERVE_BEGIN &_sram4_reserve_begin
> > > >     #define SRAM4_RESERVE_END &_sram4_reserve_end
> > > >
> > > > The implied size depends on the size of all .sram4 sections.  I
> assume
> > > > this would be positioned at the beginning of SRAM4 and the size of
> the
> > > > region that could be added to the heap would be SRAM4_RESERVE_END
> > > > through SRAM_END.
> > > >
> > > You can see this same kind of thing in, for example,
> > > arch/arm/src/common/arm_internal.h
> >
> >
> >
> > Great! Thanks
> >
> > Nathan
> >
>

RE: How to ensure HEAP will not overlap static DMA buffer?

Posted by David Sidrane <Da...@nscdg.com>.
I am just thinking out load...

I agree this has to come from one place. But I do think it is just the
linker file.

Currently we have
The arch memroymap h files have the base addresses, sizes  - This is the
Reference manuals counterpart, it covers all the sub members of the chips)
The chip.h files  that has sizes  - This is the Data Sheet counterpart, it
covers one or more of the sub members of the chips)
The Kconfig - More flexible from a users stand point.
The heap c files - buried semantics - not good
linker file - the boards usage specific.

I like using the linker file, but Kconfig is build time - no file
modification

Just moving things to the linker file does not fix the ordering or adding
issues. (it is link time not compile time)

A tool that takes the Kconfig + chip+ memorymap and make a linker include
file and the config code for the heap may be the way to go.

David


-----Original Message-----
From: Fotis Panagiotopoulos [mailto:f.j.panag@gmail.com]
Sent: Friday, March 26, 2021 9:17 AM
To: dev@nuttx.apache.org
Subject: Re: How to ensure HEAP will not overlap static DMA buffer?

I face similar problems (with a different use case) on an STM32F4.
The thing is that although the linker script belongs to the board logic and
thus it is freely customizable, the heap regions are hard-coded in the arch
files.

So, I started working on PR #2277 (
https://github.com/apache/incubator-nuttx/pull/2277), but unfortunately I
had to pause the development on this.
The idea is similar to what you describe here. Everything can be defined in
the linkerscript (addresses, order, sizeses etc).

I was thinking a lot of any alternatives on this. I came to the conclusion
that Kconfig is the wrong tool for this job.
You lose all compile-time (and CI) checks and can easily be misconfigured.
I am also afraid that we will end up with a few dozen "hacks" like above or
like STM32_CCMEXCLUDE (I never liked this option....).
And no matter what you do, you will never be able to satisfy any crazy
memory mappings that any project may need.

A similar issue to this is Issue #2001 (
https://github.com/apache/incubator-nuttx/issues/2001).
This was my first crash while trying out NuttX :)
In short, there is the assumption that the main stack will always reside
between BSS and Heap, again being very restrictive.


Στις Παρ, 26 Μαρ 2021 στις 5:46 μ.μ., ο/η Nathan Hartman <
hartman.nathan@gmail.com> έγραψε:

> On Fri, Mar 26, 2021 at 11:41 AM Gregory Nutt <sp...@gmail.com> wrote:
>
> > Missing bit of logic:
> >
> > >> 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?
> > >
> > > Are you thinking of something like this in the linker script:
> > >
> > >     .sram4_reserve :
> > >     {
> > >       _sram4_reserve_begin = ABSOLUTE(.);
> > >       *(.sram4)
> > >       _sram4_reserve_end = ABSOLUTE(.);
> > >     }
> > >
> > > And in the C code:
> > >
> > We need to lie to C and tell it what to think those symbols are:
> >
> >     EXTERN const uint32_t _sram4_reserve_begin
> >     EXTERN const uint32_t _sram4_reserve_begin
>
>
>
> Ah, yes, otherwise those symbols would be undefined. Later the linker will
> resolve them to the correct addresses.
>
>
> >     #define SRAM4_RESERVE_BEGIN &_sram4_reserve_begin
> > >     #define SRAM4_RESERVE_END &_sram4_reserve_end
> > >
> > > The implied size depends on the size of all .sram4 sections.  I assume
> > > this would be positioned at the beginning of SRAM4 and the size of the
> > > region that could be added to the heap would be SRAM4_RESERVE_END
> > > through SRAM_END.
> > >
> > You can see this same kind of thing in, for example,
> > arch/arm/src/common/arm_internal.h
>
>
>
> Great! Thanks
>
> Nathan
>

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Fotis Panagiotopoulos <f....@gmail.com>.
I face similar problems (with a different use case) on an STM32F4.
The thing is that although the linker script belongs to the board logic and
thus it is freely customizable, the heap regions are hard-coded in the arch
files.

So, I started working on PR #2277 (
https://github.com/apache/incubator-nuttx/pull/2277), but unfortunately I
had to pause the development on this.
The idea is similar to what you describe here. Everything can be defined in
the linkerscript (addresses, order, sizeses etc).

I was thinking a lot of any alternatives on this. I came to the conclusion
that Kconfig is the wrong tool for this job.
You lose all compile-time (and CI) checks and can easily be misconfigured.
I am also afraid that we will end up with a few dozen "hacks" like above or
like STM32_CCMEXCLUDE (I never liked this option....).
And no matter what you do, you will never be able to satisfy any crazy
memory mappings that any project may need.

A similar issue to this is Issue #2001 (
https://github.com/apache/incubator-nuttx/issues/2001).
This was my first crash while trying out NuttX :)
In short, there is the assumption that the main stack will always reside
between BSS and Heap, again being very restrictive.


Στις Παρ, 26 Μαρ 2021 στις 5:46 μ.μ., ο/η Nathan Hartman <
hartman.nathan@gmail.com> έγραψε:

> On Fri, Mar 26, 2021 at 11:41 AM Gregory Nutt <sp...@gmail.com> wrote:
>
> > Missing bit of logic:
> >
> > >> 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?
> > >
> > > Are you thinking of something like this in the linker script:
> > >
> > >     .sram4_reserve :
> > >     {
> > >       _sram4_reserve_begin = ABSOLUTE(.);
> > >       *(.sram4)
> > >       _sram4_reserve_end = ABSOLUTE(.);
> > >     }
> > >
> > > And in the C code:
> > >
> > We need to lie to C and tell it what to think those symbols are:
> >
> >     EXTERN const uint32_t _sram4_reserve_begin
> >     EXTERN const uint32_t _sram4_reserve_begin
>
>
>
> Ah, yes, otherwise those symbols would be undefined. Later the linker will
> resolve them to the correct addresses.
>
>
> >     #define SRAM4_RESERVE_BEGIN &_sram4_reserve_begin
> > >     #define SRAM4_RESERVE_END &_sram4_reserve_end
> > >
> > > The implied size depends on the size of all .sram4 sections.  I assume
> > > this would be positioned at the beginning of SRAM4 and the size of the
> > > region that could be added to the heap would be SRAM4_RESERVE_END
> > > through SRAM_END.
> > >
> > You can see this same kind of thing in, for example,
> > arch/arm/src/common/arm_internal.h
>
>
>
> Great! Thanks
>
> Nathan
>

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Nathan Hartman <ha...@gmail.com>.
On Fri, Mar 26, 2021 at 11:41 AM Gregory Nutt <sp...@gmail.com> wrote:

> Missing bit of logic:
>
> >> 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?
> >
> > Are you thinking of something like this in the linker script:
> >
> >     .sram4_reserve :
> >     {
> >       _sram4_reserve_begin = ABSOLUTE(.);
> >       *(.sram4)
> >       _sram4_reserve_end = ABSOLUTE(.);
> >     }
> >
> > And in the C code:
> >
> We need to lie to C and tell it what to think those symbols are:
>
>     EXTERN const uint32_t _sram4_reserve_begin
>     EXTERN const uint32_t _sram4_reserve_begin



Ah, yes, otherwise those symbols would be undefined. Later the linker will
resolve them to the correct addresses.


>     #define SRAM4_RESERVE_BEGIN &_sram4_reserve_begin
> >     #define SRAM4_RESERVE_END &_sram4_reserve_end
> >
> > The implied size depends on the size of all .sram4 sections.  I assume
> > this would be positioned at the beginning of SRAM4 and the size of the
> > region that could be added to the heap would be SRAM4_RESERVE_END
> > through SRAM_END.
> >
> You can see this same kind of thing in, for example,
> arch/arm/src/common/arm_internal.h



Great! Thanks

Nathan

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Gregory Nutt <sp...@gmail.com>.
Missing bit of logic:

>> 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?
>
> Are you thinking of something like this in the linker script:
>
>     .sram4_reserve :
>     {
>       _sram4_reserve_begin = ABSOLUTE(.);
>       *(.sram4)
>       _sram4_reserve_end = ABSOLUTE(.);
>     }
>
> And in the C code:
>
We need to lie to C and tell it what to think those symbols are:

    EXTERN const uint32_t _sram4_reserve_begin
    EXTERN const uint32_t _sram4_reserve_begin

>     #define SRAM4_RESERVE_BEGIN &_sram4_reserve_begin
>     #define SRAM4_RESERVE_END &_sram4_reserve_end
>
> The implied size depends on the size of all .sram4 sections.  I assume 
> this would be positioned at the beginning of SRAM4 and the size of the 
> region that could be added to the heap would be SRAM4_RESERVE_END 
> through SRAM_END.
>
You can see this same kind of thing in, for example, 
arch/arm/src/common/arm_internal.h



Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Fotis Panagiotopoulos <f....@gmail.com>.
Forgot to mention that this applies to GP (general purpose) regions.

Others, like BSS (that you mentioned), DATA, the stacks etc, must always be
there and be unique.

On Fri, Mar 26, 2021, 21:18 Nathan Hartman <ha...@gmail.com> wrote:

> On Fri, Mar 26, 2021 at 2:34 PM Nathan Hartman <ha...@gmail.com>
> wrote:
> > FYI my work (in progress) on this is in the branch
> > "stm32h7-fix-heap-clobber" in my fork, in case anyone wants to look...
> >
> >
> https://github.com/hartmannathan/incubator-nuttx/tree/stm32h7-fix-heap-clobber
>
> Ok, created PR-3198
>
> https://github.com/apache/incubator-nuttx/pull/3198
>
> Nathan
>

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Nathan Hartman <ha...@gmail.com>.
On Fri, Mar 26, 2021 at 2:34 PM Nathan Hartman <ha...@gmail.com> wrote:
> FYI my work (in progress) on this is in the branch
> "stm32h7-fix-heap-clobber" in my fork, in case anyone wants to look...
>
> https://github.com/hartmannathan/incubator-nuttx/tree/stm32h7-fix-heap-clobber

Ok, created PR-3198

https://github.com/apache/incubator-nuttx/pull/3198

Nathan

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Nathan Hartman <ha...@gmail.com>.
On Fri, Mar 26, 2021 at 11:21 AM Gregory Nutt <sp...@gmail.com> wrote:
> > So, if I understand correctly, anyone can declare a static buffer like this:
> >
> > static uint8_t g_spi6_txbuf[SPI6_DMABUFSIZE_ADJUSTED]
> > SPI6_DMABUFSIZE_ALGN locate_data(".sram4");
> >
> > and automatically the heap will not overlap it?
>
> Yes provided that:
>
>  1. The .sram_resevere section is origined at SRAM4_BEGIN, and
>  2. The region that you add is in the range from SRAM4_RESERVE_END to
>     SRAM4_END

FYI my work (in progress) on this is in the branch
"stm32h7-fix-heap-clobber" in my fork, in case anyone wants to look...

https://github.com/hartmannathan/incubator-nuttx/tree/stm32h7-fix-heap-clobber

I've implemented this for my custom board and verified that it is
correctly skipping my SRAM4 DMA buffers and adding only the remainder
of SRAM4 to the heap. NSH starts up and the board is working.

So far the branch contains only the Kconfig and C code changes. I will
now fix all STM32H7 linker scripts to contain the required:

    .sram4_reserve :
    {
        _sram4_reserve_start = ABSOLUTE(.);
        *(.sram4)
        _sram4_reserve_end = ABSOLUTE(.);
    } > sram4

"I'll be back..." :-)
Nathan

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Gregory Nutt <sp...@gmail.com>.
> So, if I understand correctly, anyone can declare a static buffer like this:
>
> static uint8_t g_spi6_txbuf[SPI6_DMABUFSIZE_ADJUSTED]
> SPI6_DMABUFSIZE_ALGN locate_data(".sram4");
>
> and automatically the heap will not overlap it?

Yes provided that:

 1. The .sram_resevere section is origined at SRAM4_BEGIN, and
 2. The region that you add is in the range from SRAM4_RESERVE_END to
    SRAM4_END


Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Nathan Hartman <ha...@gmail.com>.
On Fri, Mar 26, 2021 at 11:14 AM Gregory Nutt <sp...@gmail.com> wrote:
> > 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?
>
> Are you thinking of something like this in the linker script:
>
>     .sram4_reserve :
>     {
>        _sram4_reserve_begin = ABSOLUTE(.);
>        *(.sram4)
>        _sram4_reserve_end = ABSOLUTE(.);
>     }
>
> And in the C code:
>
>     #define SRAM4_RESERVE_BEGIN &_sram4_reserve_begin
>     #define SRAM4_RESERVE_END &_sram4_reserve_end
>
> The implied size depends on the size of all .sram4 sections.  I assume
> this would be positioned at the beginning of SRAM4 and the size of the
> region that could be added to the heap would be SRAM4_RESERVE_END
> through SRAM_END.


Yes, that's what I had in mind (but I didn't know what incantations
would do it).

So, if I understand correctly, anyone can declare a static buffer like this:

static uint8_t g_spi6_txbuf[SPI6_DMABUFSIZE_ADJUSTED]
SPI6_DMABUFSIZE_ALGN locate_data(".sram4");

and automatically the heap will not overlap it?

Nathan

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Gregory Nutt <sp...@gmail.com>.
> 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?

Are you thinking of something like this in the linker script:

    .sram4_reserve :
    {
       _sram4_reserve_begin = ABSOLUTE(.);
       *(.sram4)
       _sram4_reserve_end = ABSOLUTE(.);
    }

And in the C code:

    #define SRAM4_RESERVE_BEGIN &_sram4_reserve_begin
    #define SRAM4_RESERVE_END &_sram4_reserve_end

The implied size depends on the size of all .sram4 sections.  I assume 
this would be positioned at the beginning of SRAM4 and the size of the 
region that could be added to the heap would be SRAM4_RESERVE_END 
through SRAM_END.




Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Nathan Hartman <ha...@gmail.com>.
On Fri, Mar 26, 2021 at 1:08 AM Gregory Nutt <sp...@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

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Gregory Nutt <sp...@gmail.com>.
>> 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_

> Or, I can leave the OS alone, and in my board config, set
> CONFIG_MM_REGIONS = 1 so arm_addregion() does nothing; then from
> somewhere (board_late_initialize()?) call mm_addregion() to customize
> other regions. This also feels like an ugly hack to me.
Seems a little uglier only because other boards would have to do the same.
> Option 3, I add a new Kconfig to exclude SRAM4 from the heap, as is
> already being done for DTCM RAM with CONFIG_STM32H7_DTCMEXCLUDE.
Yep.  I should have read futher before responding.
> Option 4, I like the Granule Allocator option; If I understand
> correctly, to use the granule allocator, I need to exclude SRAM4 from
> the heap (like option 3) + create a g_dmaheap in .sram4, and init the
> granule allocator from board init code. Is that correct?
Yes, there should be some good examples for STM32.  The granule 
allocator is nice because you can configure it so that each DMA region 
is automatically aligned to the cache line size and automatically forced 
to be multiples of the cache line size in size.
> I think SPI1 thru SPI5 are unaffected because (I think) their static
> allocations end up in .bss and automatically excluded from all heaps;
> but because g_spi6_txbuf and g_spi6_rxbuf have locate_data(".sram4"),
> they end up in the heap.
>
> We might have other __attribute__ ((section (".insert_name_here")))
> buffers in other files that suffer the same problem.
Using the granule allocate would solve that too.

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Nathan Hartman <ha...@gmail.com>.
On Thu, Mar 25, 2021 at 7:54 PM Gregory Nutt <sp...@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?

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.

Or, I can leave the OS alone, and in my board config, set
CONFIG_MM_REGIONS = 1 so arm_addregion() does nothing; then from
somewhere (board_late_initialize()?) call mm_addregion() to customize
other regions. This also feels like an ugly hack to me.

Option 3, I add a new Kconfig to exclude SRAM4 from the heap, as is
already being done for DTCM RAM with CONFIG_STM32H7_DTCMEXCLUDE. This
seems more sensible than above, because right now, any STM32H7 build
that uses SPI6 with BDMA will have its DMA buffers (g_spi6_txbuf and
g_spi6_rxbuf) clobbering heap-allocated objects. If I add a new
Kconfig to exclude SRAM4 from the heap, I could also add compile-time
logic in arch/arm/src/stm32h7/stm32_spi.c to trigger an #error
directive if CONFIG_STM32H7_SPI6 and CONFIG_STM32H7_SPI6_DMA_BUFFER
are defined while SRAM4 is included in the heap. That could save some
poor soul a big debugging session; but it will not help if someone
(like me) creates their own DMA buffers.

Option 4, I like the Granule Allocator option; If I understand
correctly, to use the granule allocator, I need to exclude SRAM4 from
the heap (like option 3) + create a g_dmaheap in .sram4, and init the
granule allocator from board init code. Is that correct?

Other notes:

I think SPI1 thru SPI5 are unaffected because (I think) their static
allocations end up in .bss and automatically excluded from all heaps;
but because g_spi6_txbuf and g_spi6_rxbuf have locate_data(".sram4"),
they end up in the heap.

We might have other __attribute__ ((section (".insert_name_here")))
buffers in other files that suffer the same problem.

I looked at SAMA5Dx as you suggested... I see in the README of
boards/arm/sama5/sama5d3x-ek/README.txt the following ominous warning:

      Care must be used applied these RAM locations; the graphics
      configurations use SDRAM in an incompatible way to set aside
      LCD framebuffers.

It seems that's the exact issue I've run into.

Thanks,
Nathan

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Gregory Nutt <sp...@gmail.com>.
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");

You can also create a different heap for the DMA region (or probably 
better, and granule allocator for the DMA region.  See mm/mm_gram.)  Or 
just use the DMA region directly if you don't need allocations from it.  
For big things like frame buffers, I usually put them at the beginning 
or end of the memory region to minimize the number of regions.

addregion (SRAM4_START, SRAM4_END - SRAM4_START - 128, "SRAM4");

You can see how that was done for most boards that need framebuffers 
like SAMA5Dx (the boards use Kconfig settings to determine the size of 
the memory regions).

This DMA buffer is rather tiny.

I suppose memalign() won't work because you can't guarantee that the 
memory lies in SRAM4.

On 3/25/2021 5:27 PM, Nathan Hartman wrote:
> tl;dr: How do I statically allocate DMA buffers and prevent the heap
> from overlapping them?
>
> Details: On STM32H7, BDMA can only reach SRAM4 (64 KB starting at
> 0x3800:0000). The armv7m DCache line size means the buffers must be
> allocated on a cache line boundary. So a DMA buffer must meet 2
> requirements: (1) be in SRAM4, and (2) be aligned and padded to 32
> byte increments.
>
> Ok, I can do this easily with:
>
> static volatile uint8_t my_bdma_buf[128] __attribute__ ((section
> (".sram4"))) __attribute__ ((aligned (32)));
>
> or equivalent.
>
> But then the heap will overlap this memory and my buffer will be
> clobbered because arch/arm/src/stm32h7/stm32_allocateheap.c
> arm_addregion() does this:
>
> addregion (SRAM4_START, SRAM4_END - SRAM4_START, "SRAM4");
>
> Looking into it, I see that in arch/arm/src/stm32h7/stm32_spi.c,
> g_spi6_txbuf[] and g_spi6_rxbuf[] are declared like this:
>
> static uint8_t g_spi6_txbuf[SPI6_DMABUFSIZE_ADJUSTED]
> SPI6_DMABUFSIZE_ALGN locate_data(".sram4");
> static uint8_t g_spi6_rxbuf[SPI6_DMABUFSIZE_ADJUSTED]
> SPI6_DMABUFSIZE_ALGN locate_data(".sram4");
>
> So they will suffer that same problem. (I'm pretty sure that's a bug,
> unless I missed something.)
>
> I discovered this the hard way, i.e., on my board, NSH suddenly
> wouldn't start. In reality, it does start but it turns out that when
> it tries to print the greeting message, it fails on the very first
> character ('\n') and exits. Why? I single-stepped all the way into the
> guts of the serial logic, and found that in fs/vfs/fs_write.c,
> file_write(), it fetches the inode and tests if it's valid (!inode ||
> !inode->u.i_ops || !inode->u.i_ops->write) or returns -EBADF. That's
> where it fails, because it seems that the inode is allocated at
> 0x3800:0070, right on top of my BDMA buffers!
>
> I don't know which question to ask, but I think it's one of these:
>
> (1) Is there a standard(-ish) way to allocate a buffer that is both
> aligned to a specified boundary AND in a specified region of memory?
>
> (2) Is there a standard way to make a hole in the heap so that nothing
> else in NuttX will overwrite the DMA buffer?
>
> (Yes, I see that I can fiddle with CONFIG_MM_REGIONS, but then only
> the 512 KB of AXI SRAM will be in the heap, and none of the other
> regions, SRAM1,2,3, etc., and I don't think I want that.)
>
> Thanks,
> Nathan

Re: How to ensure HEAP will not overlap static DMA buffer?

Posted by Nathan Hartman <ha...@gmail.com>.
On Fri, Mar 26, 2021 at 4:26 AM David Sidrane <Da...@nscdg.com> wrote:
> As you have noted checking at build time only works for in tree
> configurations. The quick fix to add an CONFIG_..._EXCLUDE_SRAM4... (naming
> TBD) and light it in things that select BDMA to SRAM4 would a good to
> addition, and help in some cases, but selecting it just on BDMA could be
> wasteful.
> Maybe that is just a warning if SRAM4 is added to the heap when BDMA is
> enabled.
>
> These more complex muti-power domain Soc, require more floor planning of
> resource allocation on the board designer and users part.
>
> I think a more flexible approach is would be to rework the heap addition
> logic to be selectable and orderable.
>
> Then a board designer can choose to add the RAM blocks to the heap and order
> them as desired.
>
> CONFIG_...ADD_ORDER_SRAM
> CONFIG_... ADD_ORDER _SRAM12..
> CONFIG_... ADD_ORDER _ SRAM3..
> CONFIG_... ADD_ORDER _SRAM4..
> CONFIG_... ADD_ORDER _DTCM..
>
> (naming TBD)
>
> 0 is not used. 1 first added, N last added.
>
> This gets defined into an array with the start addresses and lengths. The
> code then processes them in a loop until all are added.

In concept, I like this idea.

Before implementing it, I'd like to try the linker/sram_reserve idea
discussed with Greg, and create a PR for that, to solve the immediate
problem.

Afterwards, we should talk about reworking the heap logic to allow
better customization of the order and include/exclude options through
Kconfig.

> I spent quite a bit of time mulling over using the granual allocator and
> placing the buffers in the drivers. My conclusion was this:
> For the granual allocator you have to give it the static memory large enough
> to meet ALL demand. (this creates a runtime size dependency the user has to
> manage) After init, the memory will be given out to the drives and that is
> dependent on device messages sizes (i.e MAX SPI payload which is for the
> most part static, unless you have PNP SPI) So once the memory is allocated
> out, it is the same as the static allocation of buffers in the drivers. It
> is more code, more complexity, can fail at run time for out of memory or
> DMA/Dcache alignment reasons. The compiler telling you SRAM4 is full is a
> lot easier to debug :)

Agreed; failing as early as possible is best. I consider myself very
lucky that my overlapping heap/buffers crashed NSH right at the start
and made me aware of the problem right away. Having the compiler or
linker tell you at build time is even better. :-)

Cheers,
Nathan

RE: How to ensure HEAP will not overlap static DMA buffer?

Posted by David Sidrane <Da...@nscdg.com>.
Hi Nathan,

I just ran into this, this week on all the H7 boards in PX4.

This was broken by
https://github.com/apache/incubator-nuttx/pull/459/files#diff-8e09d7a85a1cc6cc65e0bb43d7aa751e3caaec9f9d5e824f9b741a1487ec9199L117-L138

I did not catch the implication of that change at the time.

The original idea we to leave sram4 out when the CONFIG_MM_REGIONS was 3

The side effect benefit of the ordering was that tasks allocated first used
DTCM :).

But that is not very deterministic. Adding the dtcm heap is a move in the
better direction. Then set CONFIG_MM_REGIONS to 1.

However, not having a "SPEED" parameter to allocate makes using the dtcm
very arch/chip specific.

As you have noted checking at build time only works for in tree
configurations. The quick fix to add an CONFIG_..._EXCLUDE_SRAM4... (naming
TBD) and light it in things that select BDMA to SRAM4 would a good to
addition, and help in some cases, but selecting it just on BDMA could be
wasteful.
Maybe that is just a warning if SRAM4 is added to the heap when BDMA is
enabled.

These more complex muti-power domain Soc, require more floor planning of
resource allocation on the board designer and users part.

I think a more flexible approach is would be to rework the heap addition
logic to be selectable and orderable.

Then a board designer can choose to add the RAM blocks to the heap and order
them as desired.

CONFIG_...ADD_ORDER_SRAM
CONFIG_... ADD_ORDER _SRAM12..
CONFIG_... ADD_ORDER _ SRAM3..
CONFIG_... ADD_ORDER _SRAM4..
CONFIG_... ADD_ORDER _DTCM..

(naming TBD)

0 is not used. 1 first added, N last added.

This gets defined into an array with the start addresses and lengths. The
code then processes them in a loop until all are added.

I think this would give us the all in one solution and not force the
solution back to calling add region in late init per board.

In the case of SRAM4 it is small and I do not think there is value in
carving it up with static and partial heap. (But when you are out of memory
times get desperate)

I spent quite a bit of time mulling over using the granual allocator and
placing the buffers in the drivers. My conclusion was this:
For the granual allocator you have to give it the static memory large enough
to meet ALL demand. (this creates a runtime size dependency the user has to
manage) After init, the memory will be given out to the drives and that is
dependent on device messages sizes (i.e MAX SPI payload which is for the
most part static, unless you have PNP SPI) So once the memory is allocated
out, it is the same as the static allocation of buffers in the drivers. It
is more code, more complexity, can fail at run time for out of memory or
DMA/Dcache alignment reasons. The compiler telling you SRAM4 is full is a
lot easier to debug :)
The other issues is there is no way to pass a buffer for 2 bytes to a driver
and DMA/Dcache validate it and fall back to non DMA. The in driver buffers
do add a mem copy but given the DMA/Dcache alignment issues, and after doing
the timing analysis is it very small addition compared to the actual IO
time.

David



-----Original Message-----
From: Nathan Hartman [mailto:hartman.nathan@gmail.com]
Sent: Thursday, March 25, 2021 4:28 PM
To: dev@nuttx.apache.org
Subject: How to ensure HEAP will not overlap static DMA buffer?

tl;dr: How do I statically allocate DMA buffers and prevent the heap
from overlapping them?

Details: On STM32H7, BDMA can only reach SRAM4 (64 KB starting at
0x3800:0000). The armv7m DCache line size means the buffers must be
allocated on a cache line boundary. So a DMA buffer must meet 2
requirements: (1) be in SRAM4, and (2) be aligned and padded to 32
byte increments.

Ok, I can do this easily with:

static volatile uint8_t my_bdma_buf[128] __attribute__ ((section
(".sram4"))) __attribute__ ((aligned (32)));

or equivalent.

But then the heap will overlap this memory and my buffer will be
clobbered because arch/arm/src/stm32h7/stm32_allocateheap.c
arm_addregion() does this:

addregion (SRAM4_START, SRAM4_END - SRAM4_START, "SRAM4");

Looking into it, I see that in arch/arm/src/stm32h7/stm32_spi.c,
g_spi6_txbuf[] and g_spi6_rxbuf[] are declared like this:

static uint8_t g_spi6_txbuf[SPI6_DMABUFSIZE_ADJUSTED]
SPI6_DMABUFSIZE_ALGN locate_data(".sram4");
static uint8_t g_spi6_rxbuf[SPI6_DMABUFSIZE_ADJUSTED]
SPI6_DMABUFSIZE_ALGN locate_data(".sram4");

So they will suffer that same problem. (I'm pretty sure that's a bug,
unless I missed something.)

I discovered this the hard way, i.e., on my board, NSH suddenly
wouldn't start. In reality, it does start but it turns out that when
it tries to print the greeting message, it fails on the very first
character ('\n') and exits. Why? I single-stepped all the way into the
guts of the serial logic, and found that in fs/vfs/fs_write.c,
file_write(), it fetches the inode and tests if it's valid (!inode ||
!inode->u.i_ops || !inode->u.i_ops->write) or returns -EBADF. That's
where it fails, because it seems that the inode is allocated at
0x3800:0070, right on top of my BDMA buffers!

I don't know which question to ask, but I think it's one of these:

(1) Is there a standard(-ish) way to allocate a buffer that is both
aligned to a specified boundary AND in a specified region of memory?

(2) Is there a standard way to make a hole in the heap so that nothing
else in NuttX will overwrite the DMA buffer?

(Yes, I see that I can fiddle with CONFIG_MM_REGIONS, but then only
the 512 KB of AXI SRAM will be in the heap, and none of the other
regions, SRAM1,2,3, etc., and I don't think I want that.)

Thanks,
Nathan