[U-Boot] [PATCH 1/2] bouncebuf: Add static buffer allocation method for SPL.

[U-Boot] [PATCH 1/2] bouncebuf: Add static buffer allocation method for SPL.

[Christoph Muellner]

If we are using malloc-simple, we get into the problem, that
calls to free() won't free any memory. When using bouncebuf
in SPL with malloc-simple this means, that every allocated buffer
is lost. This can quickly consume the whole heap.

This patch addresses this memory wasting by introducing a static
allocated memory location, which is used instead of dynamically
allocated buffers.

Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---

 common/Kconfig     | 15 +++++++++++++++
 common/bouncebuf.c | 14 ++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/common/Kconfig b/common/Kconfig
index 1a1951f874..5fbca1e61a 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -702,6 +702,21 @@ config BOUNCE_BUFFER
 	  A second possible use of bounce buffers is their ability to
 	  provide aligned buffers for DMA operations.
 
+config SPL_BOUNCE_BUFFER_STATIC
+	bool "Static bounce buffer in SPL"
+	depends on SPL && BOUNCE_BUFFER
+	default n
+	help
+	  This option uses a static allocated memory area as
+	  bounce buffer (i.e. no dynamic allocation).
+
+config SPL_BOUNCE_BUFFER_STATIC_SIZE
+	hex "Size of static bounce buffer in SPL"
+	depends on SPL_BOUNCE_BUFFER_STATIC
+	default 0x2800
+	help
+	  Size of the static allocated bounce buffer.
+
 config BOARD_TYPES
 	bool "Call get_board_type() to get and display the board type"
 	help
diff --git a/common/bouncebuf.c b/common/bouncebuf.c
index a7098e2caf..92ee10fb93 100644
--- a/common/bouncebuf.c
+++ b/common/bouncebuf.c
@@ -10,6 +10,11 @@
 #include <errno.h>
 #include <bouncebuf.h>
 
+#if CONFIG_IS_ENABLED(BOUNCE_BUFFER_STATIC)
+static u8 static_bb[CONFIG_VAL(BOUNCE_BUFFER_STATIC_SIZE)];
+static const size_t static_bb_size = CONFIG_VAL(BOUNCE_BUFFER_STATIC_SIZE);
+#endif
+
 static int addr_aligned(struct bounce_buffer *state)
 {
 	const ulong align_mask = ARCH_DMA_MINALIGN - 1;
@@ -40,10 +45,19 @@ int bounce_buffer_start(struct bounce_buffer *state, void *data,
 	state->flags = flags;
 
 	if (!addr_aligned(state)) {
+#if CONFIG_IS_ENABLED(BOUNCE_BUFFER_STATIC)
+		if (state->len_aligned > static_bb_size) {
+			debug("Static allocated bounce buffer too small.\n");
+			return -ENOMEM;
+		}
+
+		state->bounce_buffer = static_bb;
+#else
 		state->bounce_buffer = memalign(ARCH_DMA_MINALIGN,
 						state->len_aligned);
 		if (!state->bounce_buffer)
 			return -ENOMEM;
+#endif
 
 		if (state->flags & GEN_BB_READ)
 			memcpy(state->bounce_buffer, state->user_buffer,
-- 
2.11.0

[U-Boot] [PATCH 2/2] rockchip: rk3399-puma: Enable SPL_BOUNCE_BUFFER_STATIC.

[Christoph Muellner]

Since the DMA-engine of the RK3399 cannot directly copy the
ATF SRAM/PMUSRAM memory sections to their destination, we need
to use bounce-buffers. This results in allocation of a huge amount
of heap memory, which can't be reused (because of simple-malloc).

Therefore we enable the static bounce buffer method.

Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---

 configs/puma-rk3399_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/puma-rk3399_defconfig b/configs/puma-rk3399_defconfig
index 964464ac0f..cb652922e3 100644
--- a/configs/puma-rk3399_defconfig
+++ b/configs/puma-rk3399_defconfig
@@ -23,6 +23,7 @@ CONFIG_MISC_INIT_R=y
 # CONFIG_DISPLAY_CPUINFO is not set
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_SPL_TEXT_BASE=0xff8c2000
+CONFIG_SPL_BOUNCE_BUFFER_STATIC=y
 CONFIG_SPL_BOARD_INIT=y
 # CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
 # CONFIG_SPL_LEGACY_IMAGE_SUPPORT is not set
-- 
2.11.0

[U-Boot] [PATCH 1/2] bouncebuf: Add static buffer allocation method for SPL.

[Marek Vasut]

On 5/7/19 11:09 AM, Christoph Muellner wrote:
> If we are using malloc-simple, we get into the problem, that
> calls to free() won't free any memory. When using bouncebuf
> in SPL with malloc-simple this means, that every allocated buffer
> is lost. This can quickly consume the whole heap.

When does such a scenario happen ?

> This patch addresses this memory wasting by introducing a static
> allocated memory location, which is used instead of dynamically
> allocated buffers.
> 
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> ---
> 
>  common/Kconfig     | 15 +++++++++++++++
>  common/bouncebuf.c | 14 ++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 1a1951f874..5fbca1e61a 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -702,6 +702,21 @@ config BOUNCE_BUFFER
>  	  A second possible use of bounce buffers is their ability to
>  	  provide aligned buffers for DMA operations.
>  
> +config SPL_BOUNCE_BUFFER_STATIC
> +	bool "Static bounce buffer in SPL"
> +	depends on SPL && BOUNCE_BUFFER
> +	default n
> +	help
> +	  This option uses a static allocated memory area as
> +	  bounce buffer (i.e. no dynamic allocation).
> +
> +config SPL_BOUNCE_BUFFER_STATIC_SIZE
> +	hex "Size of static bounce buffer in SPL"
> +	depends on SPL_BOUNCE_BUFFER_STATIC
> +	default 0x2800
> +	help
> +	  Size of the static allocated bounce buffer.
> +
>  config BOARD_TYPES
>  	bool "Call get_board_type() to get and display the board type"
>  	help
> diff --git a/common/bouncebuf.c b/common/bouncebuf.c
> index a7098e2caf..92ee10fb93 100644
> --- a/common/bouncebuf.c
> +++ b/common/bouncebuf.c
> @@ -10,6 +10,11 @@
>  #include <errno.h>
>  #include <bouncebuf.h>
>  
> +#if CONFIG_IS_ENABLED(BOUNCE_BUFFER_STATIC)
> +static u8 static_bb[CONFIG_VAL(BOUNCE_BUFFER_STATIC_SIZE)];
> +static const size_t static_bb_size = CONFIG_VAL(BOUNCE_BUFFER_STATIC_SIZE);
> +#endif
> +
>  static int addr_aligned(struct bounce_buffer *state)
>  {
>  	const ulong align_mask = ARCH_DMA_MINALIGN - 1;
> @@ -40,10 +45,19 @@ int bounce_buffer_start(struct bounce_buffer *state, void *data,
>  	state->flags = flags;
>  
>  	if (!addr_aligned(state)) {
> +#if CONFIG_IS_ENABLED(BOUNCE_BUFFER_STATIC)
> +		if (state->len_aligned > static_bb_size) {
> +			debug("Static allocated bounce buffer too small.\n");
> +			return -ENOMEM;
> +		}
> +
> +		state->bounce_buffer = static_bb;
> +#else
>  		state->bounce_buffer = memalign(ARCH_DMA_MINALIGN,
>  						state->len_aligned);
>  		if (!state->bounce_buffer)
>  			return -ENOMEM;
> +#endif
>  
>  		if (state->flags & GEN_BB_READ)
>  			memcpy(state->bounce_buffer, state->user_buffer,
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/2] bouncebuf: Add static buffer allocation method for SPL.

[Christoph Müllner]

On 07.05.19 15:06, Marek Vasut wrote:
> On 5/7/19 11:09 AM, Christoph Muellner wrote:
>> If we are using malloc-simple, we get into the problem, that
>> calls to free() won't free any memory. When using bouncebuf
>> in SPL with malloc-simple this means, that every allocated buffer
>> is lost. This can quickly consume the whole heap.
> 
> When does such a scenario happen ?

RK3399 with mainline U-Boot and mainline ATF.
We have 0x4000 bytes heap in SPL.
For the SRAM part of ATF we need to allocate 0x2000-0x2200 bytes.
For the PMUSRAM part about the same again.

I tried to increase the heap size to 0x10000, but that seems
to break some unchecked assumptions about the memory layout,
because then SPL resets very early.


> 
>> This patch addresses this memory wasting by introducing a static
>> allocated memory location, which is used instead of dynamically
>> allocated buffers.
>>
>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>> ---
>>
>>  common/Kconfig     | 15 +++++++++++++++
>>  common/bouncebuf.c | 14 ++++++++++++++
>>  2 files changed, 29 insertions(+)
>>
>> diff --git a/common/Kconfig b/common/Kconfig
>> index 1a1951f874..5fbca1e61a 100644
>> --- a/common/Kconfig
>> +++ b/common/Kconfig
>> @@ -702,6 +702,21 @@ config BOUNCE_BUFFER
>>  	  A second possible use of bounce buffers is their ability to
>>  	  provide aligned buffers for DMA operations.
>>  
>> +config SPL_BOUNCE_BUFFER_STATIC
>> +	bool "Static bounce buffer in SPL"
>> +	depends on SPL && BOUNCE_BUFFER
>> +	default n
>> +	help
>> +	  This option uses a static allocated memory area as
>> +	  bounce buffer (i.e. no dynamic allocation).
>> +
>> +config SPL_BOUNCE_BUFFER_STATIC_SIZE
>> +	hex "Size of static bounce buffer in SPL"
>> +	depends on SPL_BOUNCE_BUFFER_STATIC
>> +	default 0x2800
>> +	help
>> +	  Size of the static allocated bounce buffer.
>> +
>>  config BOARD_TYPES
>>  	bool "Call get_board_type() to get and display the board type"
>>  	help
>> diff --git a/common/bouncebuf.c b/common/bouncebuf.c
>> index a7098e2caf..92ee10fb93 100644
>> --- a/common/bouncebuf.c
>> +++ b/common/bouncebuf.c
>> @@ -10,6 +10,11 @@
>>  #include <errno.h>
>>  #include <bouncebuf.h>
>>  
>> +#if CONFIG_IS_ENABLED(BOUNCE_BUFFER_STATIC)
>> +static u8 static_bb[CONFIG_VAL(BOUNCE_BUFFER_STATIC_SIZE)];
>> +static const size_t static_bb_size = CONFIG_VAL(BOUNCE_BUFFER_STATIC_SIZE);
>> +#endif
>> +
>>  static int addr_aligned(struct bounce_buffer *state)
>>  {
>>  	const ulong align_mask = ARCH_DMA_MINALIGN - 1;
>> @@ -40,10 +45,19 @@ int bounce_buffer_start(struct bounce_buffer *state, void *data,
>>  	state->flags = flags;
>>  
>>  	if (!addr_aligned(state)) {
>> +#if CONFIG_IS_ENABLED(BOUNCE_BUFFER_STATIC)
>> +		if (state->len_aligned > static_bb_size) {
>> +			debug("Static allocated bounce buffer too small.\n");
>> +			return -ENOMEM;
>> +		}
>> +
>> +		state->bounce_buffer = static_bb;
>> +#else
>>  		state->bounce_buffer = memalign(ARCH_DMA_MINALIGN,
>>  						state->len_aligned);
>>  		if (!state->bounce_buffer)
>>  			return -ENOMEM;
>> +#endif
>>  
>>  		if (state->flags & GEN_BB_READ)
>>  			memcpy(state->bounce_buffer, state->user_buffer,
>>
> 
> 

[U-Boot] [PATCH 1/2] bouncebuf: Add static buffer allocation method for SPL.

[Marek Vasut]

On 5/7/19 3:45 PM, Christoph Müllner wrote:
> 
> 
> On 07.05.19 15:06, Marek Vasut wrote:
>> On 5/7/19 11:09 AM, Christoph Muellner wrote:
>>> If we are using malloc-simple, we get into the problem, that
>>> calls to free() won't free any memory. When using bouncebuf
>>> in SPL with malloc-simple this means, that every allocated buffer
>>> is lost. This can quickly consume the whole heap.
>>
>> When does such a scenario happen ?
> 
> RK3399 with mainline U-Boot and mainline ATF.
> We have 0x4000 bytes heap in SPL.
> For the SRAM part of ATF we need to allocate 0x2000-0x2200 bytes.
> For the PMUSRAM part about the same again.
> 
> I tried to increase the heap size to 0x10000, but that seems
> to break some unchecked assumptions about the memory layout,
> because then SPL resets very early.

So what calls this malloc-free pair so often to spend all the simple
malloc area ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/2] bouncebuf: Add static buffer allocation method for SPL.

[Christoph Müllner]

On 07.05.19 15:52, Marek Vasut wrote:
> On 5/7/19 3:45 PM, Christoph Müllner wrote:
>>
>>
>> On 07.05.19 15:06, Marek Vasut wrote:
>>> On 5/7/19 11:09 AM, Christoph Muellner wrote:
>>>> If we are using malloc-simple, we get into the problem, that
>>>> calls to free() won't free any memory. When using bouncebuf
>>>> in SPL with malloc-simple this means, that every allocated buffer
>>>> is lost. This can quickly consume the whole heap.
>>>
>>> When does such a scenario happen ?
>>
>> RK3399 with mainline U-Boot and mainline ATF.
>> We have 0x4000 bytes heap in SPL.
>> For the SRAM part of ATF we need to allocate 0x2000-0x2200 bytes.
>> For the PMUSRAM part about the same again.
>>
>> I tried to increase the heap size to 0x10000, but that seems
>> to break some unchecked assumptions about the memory layout,
>> because then SPL resets very early.
> 
> So what calls this malloc-free pair so often to spend all the simple
> malloc area ?

The bouncebuf code (bounce_buffer_start(), which does memalign().
It is called twice (once for SRAM and once for PMUSRAM) as mentioned above.

That's all part of loading the ATF parts (on RK3399 we have normal ATF code,
SRAM code and PMUSRAM code).

[U-Boot] [PATCH 1/2] bouncebuf: Add static buffer allocation method for SPL.

[Marek Vasut]

On 5/7/19 3:55 PM, Christoph Müllner wrote:
> 
> 
> On 07.05.19 15:52, Marek Vasut wrote:
>> On 5/7/19 3:45 PM, Christoph Müllner wrote:
>>>
>>>
>>> On 07.05.19 15:06, Marek Vasut wrote:
>>>> On 5/7/19 11:09 AM, Christoph Muellner wrote:
>>>>> If we are using malloc-simple, we get into the problem, that
>>>>> calls to free() won't free any memory. When using bouncebuf
>>>>> in SPL with malloc-simple this means, that every allocated buffer
>>>>> is lost. This can quickly consume the whole heap.
>>>>
>>>> When does such a scenario happen ?
>>>
>>> RK3399 with mainline U-Boot and mainline ATF.
>>> We have 0x4000 bytes heap in SPL.
>>> For the SRAM part of ATF we need to allocate 0x2000-0x2200 bytes.
>>> For the PMUSRAM part about the same again.
>>>
>>> I tried to increase the heap size to 0x10000, but that seems
>>> to break some unchecked assumptions about the memory layout,
>>> because then SPL resets very early.
>>
>> So what calls this malloc-free pair so often to spend all the simple
>> malloc area ?
> 
> The bouncebuf code (bounce_buffer_start(), which does memalign().
> It is called twice (once for SRAM and once for PMUSRAM) as mentioned above.
> 
> That's all part of loading the ATF parts (on RK3399 we have normal ATF code,
> SRAM code and PMUSRAM code).

Can you switch to full malloc ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/2] bouncebuf: Add static buffer allocation method for SPL.

[Christoph Müllner]

On 07.05.19 15:55, Marek Vasut wrote:
> On 5/7/19 3:55 PM, Christoph Müllner wrote:
>>
>>
>> On 07.05.19 15:52, Marek Vasut wrote:
>>> On 5/7/19 3:45 PM, Christoph Müllner wrote:
>>>>
>>>>
>>>> On 07.05.19 15:06, Marek Vasut wrote:
>>>>> On 5/7/19 11:09 AM, Christoph Muellner wrote:
>>>>>> If we are using malloc-simple, we get into the problem, that
>>>>>> calls to free() won't free any memory. When using bouncebuf
>>>>>> in SPL with malloc-simple this means, that every allocated buffer
>>>>>> is lost. This can quickly consume the whole heap.
>>>>>
>>>>> When does such a scenario happen ?
>>>>
>>>> RK3399 with mainline U-Boot and mainline ATF.
>>>> We have 0x4000 bytes heap in SPL.
>>>> For the SRAM part of ATF we need to allocate 0x2000-0x2200 bytes.
>>>> For the PMUSRAM part about the same again.
>>>>
>>>> I tried to increase the heap size to 0x10000, but that seems
>>>> to break some unchecked assumptions about the memory layout,
>>>> because then SPL resets very early.
>>>
>>> So what calls this malloc-free pair so often to spend all the simple
>>> malloc area ?
>>
>> The bouncebuf code (bounce_buffer_start(), which does memalign().
>> It is called twice (once for SRAM and once for PMUSRAM) as mentioned above.
>>
>> That's all part of loading the ATF parts (on RK3399 we have normal ATF code,
>> SRAM code and PMUSRAM code).
> 
> Can you switch to full malloc ?

Tested that as well, but that did not work either (again SPL crashes quite early).

I think the reason is the following:
Full malloc needs the data section to be working for the global variables
(besides gd fields). I think that's not possible in very early SPL stages,
where the first pages are going to be mapped (and malloc is used for allocating
the page table entries).

[U-Boot] [PATCH 1/2] bouncebuf: Add static buffer allocation method for SPL.

[Simon Goldschmidt]

On Tue, May 7, 2019 at 4:11 PM Christoph Müllner
<christoph.muellner@theobroma-systems.com> wrote:
>
>
>
> On 07.05.19 15:55, Marek Vasut wrote:
> > On 5/7/19 3:55 PM, Christoph Müllner wrote:
> >>
> >>
> >> On 07.05.19 15:52, Marek Vasut wrote:
> >>> On 5/7/19 3:45 PM, Christoph Müllner wrote:
> >>>>
> >>>>
> >>>> On 07.05.19 15:06, Marek Vasut wrote:
> >>>>> On 5/7/19 11:09 AM, Christoph Muellner wrote:
> >>>>>> If we are using malloc-simple, we get into the problem, that
> >>>>>> calls to free() won't free any memory. When using bouncebuf
> >>>>>> in SPL with malloc-simple this means, that every allocated buffer
> >>>>>> is lost. This can quickly consume the whole heap.
> >>>>>
> >>>>> When does such a scenario happen ?
> >>>>
> >>>> RK3399 with mainline U-Boot and mainline ATF.
> >>>> We have 0x4000 bytes heap in SPL.
> >>>> For the SRAM part of ATF we need to allocate 0x2000-0x2200 bytes.
> >>>> For the PMUSRAM part about the same again.
> >>>>
> >>>> I tried to increase the heap size to 0x10000, but that seems
> >>>> to break some unchecked assumptions about the memory layout,
> >>>> because then SPL resets very early.
> >>>
> >>> So what calls this malloc-free pair so often to spend all the simple
> >>> malloc area ?
> >>
> >> The bouncebuf code (bounce_buffer_start(), which does memalign().
> >> It is called twice (once for SRAM and once for PMUSRAM) as mentioned above.
> >>
> >> That's all part of loading the ATF parts (on RK3399 we have normal ATF code,
> >> SRAM code and PMUSRAM code).
> >
> > Can you switch to full malloc ?
>
> Tested that as well, but that did not work either (again SPL crashes quite early).
>
> I think the reason is the following:
> Full malloc needs the data section to be working for the global variables
> (besides gd fields). I think that's not possible in very early SPL stages,
> where the first pages are going to be mapped (and malloc is used for allocating
> the page table entries).

Exactly. I think you're at least the third person on this list stumbling
accross this in the last weeks :(

Regards,
Simon

[U-Boot] [PATCH 1/2] bouncebuf: Add static buffer allocation method for SPL.

[Christoph Müllner]

On 07.05.19 16:16, Simon Goldschmidt wrote:
> On Tue, May 7, 2019 at 4:11 PM Christoph Müllner
> <christoph.muellner@theobroma-systems.com> wrote:
>>
>>
>>
>> On 07.05.19 15:55, Marek Vasut wrote:
>>> On 5/7/19 3:55 PM, Christoph Müllner wrote:
>>>>
>>>>
>>>> On 07.05.19 15:52, Marek Vasut wrote:
>>>>> On 5/7/19 3:45 PM, Christoph Müllner wrote:
>>>>>>
>>>>>>
>>>>>> On 07.05.19 15:06, Marek Vasut wrote:
>>>>>>> On 5/7/19 11:09 AM, Christoph Muellner wrote:
>>>>>>>> If we are using malloc-simple, we get into the problem, that
>>>>>>>> calls to free() won't free any memory. When using bouncebuf
>>>>>>>> in SPL with malloc-simple this means, that every allocated buffer
>>>>>>>> is lost. This can quickly consume the whole heap.
>>>>>>>
>>>>>>> When does such a scenario happen ?
>>>>>>
>>>>>> RK3399 with mainline U-Boot and mainline ATF.
>>>>>> We have 0x4000 bytes heap in SPL.
>>>>>> For the SRAM part of ATF we need to allocate 0x2000-0x2200 bytes.
>>>>>> For the PMUSRAM part about the same again.
>>>>>>
>>>>>> I tried to increase the heap size to 0x10000, but that seems
>>>>>> to break some unchecked assumptions about the memory layout,
>>>>>> because then SPL resets very early.
>>>>>
>>>>> So what calls this malloc-free pair so often to spend all the simple
>>>>> malloc area ?
>>>>
>>>> The bouncebuf code (bounce_buffer_start(), which does memalign().
>>>> It is called twice (once for SRAM and once for PMUSRAM) as mentioned above.
>>>>
>>>> That's all part of loading the ATF parts (on RK3399 we have normal ATF code,
>>>> SRAM code and PMUSRAM code).
>>>
>>> Can you switch to full malloc ?
>>
>> Tested that as well, but that did not work either (again SPL crashes quite early).
>>
>> I think the reason is the following:
>> Full malloc needs the data section to be working for the global variables
>> (besides gd fields). I think that's not possible in very early SPL stages,
>> where the first pages are going to be mapped (and malloc is used for allocating
>> the page table entries).
> 
> Exactly. I think you're at least the third person on this list stumbling
> accross this in the last weeks :(

I was "lucky" enough to get bad weather this weekend, so I could debug
each and every ATF loader issue for the SD card boot case.

The tree I was testing with (on RK3399-Q7/Puma with Haikou) can be found here:

https://git.theobroma-systems.com/puma-u-boot.git/log/?h=puma-v2019.04-atf

[U-Boot] [PATCH 2/2] rockchip: rk3399-puma: Enable SPL_BOUNCE_BUFFER_STATIC.

[Simon Glass]

On Tue, 7 May 2019 at 03:09, Christoph Muellner
<christoph.muellner@theobroma-systems.com> wrote:
>
> Since the DMA-engine of the RK3399 cannot directly copy the
> ATF SRAM/PMUSRAM memory sections to their destination, we need
> to use bounce-buffers. This results in allocation of a huge amount
> of heap memory, which can't be reused (because of simple-malloc).
>
> Therefore we enable the static bounce buffer method.
>
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> ---
>
>  configs/puma-rk3399_defconfig | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Simon Glass <sjg@chromium.org>