stm32l475: lwip symbols in sram1 #9793

juhoeskeli · 2019-02-21T12:09:53Z

Description

Move LWIP symbols to SRAM1 region, as they do not fit in the SRAM2 (32kB) with all the other symbols. This is done for purpose of using PDMC on this target with PPP modems. PPP requires LWIP which takes up additional RAM.

Tested to work with and without PPP on GCC_ARM, ARM, IAR. The only caveat is that HEAP size for IAR is reduced from 0x17000 to 0x13C00 even when PPP is not used. This is because heap is fixed size in IAR.

Pull request type

[ ] Fix
[ ] Refactor
[x] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@linlingao

Release Notes

Fit LWIP into RAM regions
Heap size reduced for IAR from 0x17000 to 0x13C00

ciarmcom · 2019-02-21T14:00:28Z

@juhoeskeli, thank you for your changes.
@linlingao @ARMmbed/mbed-os-maintainers please review.

LMESTM

This change seems to fit a given precise Use case and I'm not against it if this doesn't break other existing use cases. Note to mbed-os maintainers, I think it would be best to find ways to over-write the linker files from an application level as different applications could have very different memory map constraints.

LMESTM · 2019-02-21T14:25:14Z

targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L475xG/device/TOOLCHAIN_GCC_ARM/STM32L475XX.ld

+        . = ALIGN(8);
+        __lwip_bss_start__ = .;
+        *mbed-os/features/lwipstack*.o(.bss*)
+        *mbed-os/features/lwipstack*.o(COMMON)


I don't think LWIP is a defaut feature on all L475 based boards - will that work fine anyway when there are no LWIP symbols at all (mbed2 application for instance)

Should work, I tested this precise use case by disabling lwip. I saw no ill effects. Can't comment for mbed2, though.

LMESTM · 2019-02-21T14:27:51Z

@ARMmbed/mbed-os-maintainers please comment on idea to overload the linker files from an application level ?

cmonr · 2019-02-21T16:35:45Z

#9793 (comment)

Pinging @ARMmbed/mbed-os-core for input.

linlingao

I am worried about the impact of this change to applications without LWIP. For IAR build, the heap size is reduced to only 79K. When I ran Pelion client on this target, the peak usage was close to 70K. This change will cause Pelion example to break very soon as heap demand continues to grow.
Is it possible to change the linker script, or add another one for applications that require LWIP so that the heap size does not decrease for applications without LWIP?

cmonr · 2019-02-22T01:30:29Z

@JanneKiiskila Mind chiming in?

juhoeskeli · 2019-02-22T08:55:55Z

I must say 70k heap usage sounds quite heavy. What was the setup that resulted in this high usage? I think in the past the usage has been closer to 50k, now maybe closer to 60k. It is true it has been creeping up, but we should put stop to this. I would also like to hear @JanneKiiskila comment on that.

If there is way to keep the IAR heap same without LWIP that would be perfect, but I don't know how to do that.

linlingao · 2019-02-22T21:39:35Z

@juhoeskeli Heavy heap usage was observed while running Pelion client example. max_size is the peak usage.
**** current_size: 63018
**** max_size : 69683
**** reserved_size : 94112
**** alloc_cnt: 1136
**** alloc_fail_cnt : 0
**** overhead_size: 16270
We may need to create two IAR linker scripts, one with LWIP, the other one without LWIP. For targets with really small memory footprint, heavy customization should be anticipated.

JanneKiiskila · 2019-02-25T08:31:46Z

TLS handshake is typically the peak point for our memory ussage.

cmonr · 2019-03-01T02:09:28Z

#9793 (comment)

I wonder if @ARMmbed/mbed-os-tools or @deepikabhavnani would be able to chime in on this one. Afaik, there's no generic way of overwriting linker definitions, and would likely require a fair amount of planning and work.

deepikabhavnani · 2019-03-01T14:49:37Z

The only caveat is that HEAP size for IAR is reduced from 0x17000 to 0x13C00 even when PPP is not used
I am worried about the impact of this change to applications without LWIP. For IAR build, the heap size is reduced to only 79K

Issue over here seems only with IAR - because we do not have support for dynamic heap. With update to IAR 8.x we can resolve this by using dynamic heap, so the heap size will be set as minimum to 0x13C00. In case LWIP is not present, unused memory will got to heap section.

But the catch is by changing linker file to use dynamic heap support, STM32L45 device will not compile for IAR 7.x. We are upgrading to IAR 8.x in 5.12 release, and that will make few targets with binaries to be non-compatible, but most will still work with IAR 7.x (no support though).

Change suggested here https://github.com/ARMmbed/mbed-os/pull/9793/files#r261633149
@linlingao @LMESTM - Please review

deepikabhavnani · 2019-03-01T14:57:07Z

targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L475xG/device/TOOLCHAIN_IAR/stm32l475xx.icf

@@ -33,7 +33,7 @@ if (!isdefinedsymbol(MBED_BOOT_STACK_SIZE)) {
 }

 define symbol __size_cstack__ = MBED_BOOT_STACK_SIZE;
-define symbol __size_heap__   = 0x17000;
+define symbol __size_heap__   = 0x13C00;
 define block CSTACK    with alignment = 8, size = __size_cstack__   { };
 define block HEAP      with alignment = 8, size = __size_heap__     { };


Suggested change

define block HEAP with alignment = 8, size = __size_heap__ { };

define block HEAP with expanding size, minimum size = __size_heap__, alignment = 8 { };

Dynamic HEAP for IAR @juhoeskeli - Please verify this with IAR 8.x

Deepika's suggestion is good. We don't waste RAM on a already constrained platform in builds without LWIP.

Thanks @deepikabhavnani . I will verify this and report back results.

Seems to work fine. Will do commit with the suggested change.

without PPP and with expanding change:

"P2", part 2 of 2: 0x17820
HEAP 0x2000'07e0 0x17820
HEAP uninit 0x2000'07e0 0x17820
- 0x2001'8000 0x17820

with PPP and with expanding change:
"P2", part 3 of 3: 0x144e8
HEAP 0x2000'3b18 0x144e8
HEAP uninit 0x2000'3b18 0x144e8
- 0x2001'8000 0x144e8

with&without PPP and without expanding change:
"P2", part 3 of 3: 0x13c00
HEAP 0x2000'3b18 0x13c00
HEAP uninit 0x2000'3b18 0x13c00
- 0x2001'7718 0x13c00

jeromecoutant · 2019-03-04T14:40:22Z

@ARMmbed/mbed-os-core same question as Laurent:
I don't like this linker script change only for 1 target and depending on 1 application...
Mbed OS should provide the availability to overload the linker files from an application level ?

deepikabhavnani · 2019-03-04T15:45:26Z

Mbed OS should provide the availability to overload the linker files from an application level ?

@jeromecoutant - I understand the concerns, but adding overload for linker files will be an enhancement from tools side and will need design and planning to start with, which will take long. It will be good to add it as separate issue (enhancement) in github.

For now, lets explore if we have any other option to resolve this issue, do you have any suggestions?

juhoeskeli · 2019-03-05T07:40:39Z

@jeromecoutant I understand your viewpoint and I agree it is not optimal solution. However, in the short term I don't see other options. This is particularly important use case for us as we intend to combine PPP mode modems with this MCU. In the long term we should get this sorted out in different way, e.g. what you suggested.

deepikabhavnani · 2019-03-06T22:36:21Z

@juhoeskeli - Does this PR #9944 solve your issue of lwip without adding special lwip section? Solution is added for GCC only, but I will be exploring other toolchains as well

deepikabhavnani · 2019-03-06T22:39:49Z

targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L475xG/device/TOOLCHAIN_ARM_STD/stm32l475xx.sct

@@ -61,6 +61,7 @@ LR_IROM1 MBED_APP_START MBED_APP_SIZE  {    ; load region size_region
  }
  RW_IRAM1 MBED_RAM0_START MBED_RAM0_SIZE-Stack_Size  { ; RW data 96k L4-SRAM1
   .ANY (+RW, +Last)
+   lwip* (+ZI)


ARM compiler supports expansion of RW and ZI in multiple banks, please try below solution

RW_IRAM1 MBED_RAM0_START MBED_RAM0_SIZE-Stack_Size { ; RW data 96k L4-SRAM1 .ANY (+RW +ZI) } ; Total: 98 vectors = 392 bytes (0x188) to be reserved in RAM RW_IRAM2 (0x10000000+0x188) (0x08000-0x188) { ; ZI data 32k L4-ECC-SRAM2 .ANY (+RW +ZI) }

deepikabhavnani · 2019-03-07T23:08:06Z

@juhoeskeli - I have added solution for IAR compiler as well #10003, please see if this works for your application

Solution for ARM is added as well - #10018

juhoeskeli · 2019-03-11T09:56:35Z

Thanks @deepikabhavnani. I think those PR are better general purpose solution. We tried already the GCC_ARM PR and it looks good so far. Let's wait for those to get merged and then I can close down this PR.

adbridge · 2019-03-27T16:15:51Z

@juhoeskeli What is the status on this PR ? No update for 16 days ?

mbed-ci · 2019-04-17T04:38:41Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

juhoeskeli · 2019-04-30T06:56:10Z

A more generic solution for GCC/ARM/IAR is already implemented so closing down this PR

stm32l475: lwip symbols in sram1

823db2d

ciarmcom requested review from linlingao and a team February 21, 2019 14:00

ciarmcom added the needs: review label Feb 21, 2019

LMESTM reviewed Feb 21, 2019

View reviewed changes

linlingao suggested changes Feb 21, 2019

View reviewed changes

deepikabhavnani reviewed Mar 1, 2019

View reviewed changes

Adjust HEAP dynamically with IAR 8.x

78837ef

linlingao approved these changes Mar 4, 2019

View reviewed changes

deepikabhavnani reviewed Mar 6, 2019

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Mar 8, 2019

juhoeskeli closed this Apr 30, 2019

	define block HEAP with alignment = 8, size = __size_heap__ { };
	define block HEAP with expanding size, minimum size = __size_heap__, alignment = 8 { };

stm32l475: lwip symbols in sram1 #9793

stm32l475: lwip symbols in sram1 #9793

Uh oh!

Conversation

juhoeskeli commented Feb 21, 2019

Description

Pull request type

Reviewers

Release Notes

Uh oh!

ciarmcom commented Feb 21, 2019

Uh oh!

LMESTM left a comment

Choose a reason for hiding this comment

Uh oh!

LMESTM Feb 21, 2019

Choose a reason for hiding this comment

Uh oh!

juhoeskeli Feb 22, 2019

Choose a reason for hiding this comment

Uh oh!

LMESTM commented Feb 21, 2019

Uh oh!

cmonr commented Feb 21, 2019

Uh oh!

linlingao left a comment

Choose a reason for hiding this comment

Uh oh!

cmonr commented Feb 22, 2019

Uh oh!

juhoeskeli commented Feb 22, 2019

Uh oh!

linlingao commented Feb 22, 2019

Uh oh!

JanneKiiskila commented Feb 25, 2019

Uh oh!

cmonr commented Mar 1, 2019

Uh oh!

deepikabhavnani commented Mar 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deepikabhavnani Mar 1, 2019

Choose a reason for hiding this comment

Uh oh!

linlingao Mar 1, 2019

Choose a reason for hiding this comment

Uh oh!

juhoeskeli Mar 4, 2019

Choose a reason for hiding this comment

Uh oh!

juhoeskeli Mar 4, 2019

Choose a reason for hiding this comment

Uh oh!

jeromecoutant commented Mar 4, 2019

Uh oh!

deepikabhavnani commented Mar 4, 2019

Uh oh!

juhoeskeli commented Mar 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deepikabhavnani commented Mar 6, 2019

Uh oh!

deepikabhavnani Mar 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deepikabhavnani commented Mar 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juhoeskeli commented Mar 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adbridge commented Mar 27, 2019

Uh oh!

mbed-ci commented Apr 17, 2019

Test run: SUCCESS

Uh oh!

juhoeskeli commented Apr 30, 2019

Uh oh!

Uh oh!

deepikabhavnani commented Mar 1, 2019 •

edited

Loading

juhoeskeli commented Mar 5, 2019 •

edited

Loading

deepikabhavnani Mar 6, 2019 •

edited

Loading

deepikabhavnani commented Mar 7, 2019 •

edited

Loading

juhoeskeli commented Mar 11, 2019 •

edited

Loading