arm: linker.ld: move bss section to ram end #21747

lgl88911 · 2020-01-07T15:14:15Z

fix bss section be copy to binary

Signed-off-by: Frank Li lgl88911@163.com

stephanosio

Please provide an explanation as to why this is necessary in the commit message.

include/arch/arm/aarch32/cortex_m/scripts/linker.ld

ioannisg · 2020-01-07T15:37:14Z

Please provide an explanation as to why this is necessary in the commit message.

agreed. What problem are you trying to address @lgl88911 ?

lgl88911 · 2020-01-07T16:05:08Z

@stephanosio @ioannisg Thanks for your review
When I hope load code to ram and run it, I compiling with non-xip(CONFIG_XIP=n), all sections will be placed in the ram. If NOBITS section is not placed at the end of the ram, these sections will be placed to binary file. eg: bss, noinit
You can reduce by using mimxrt1050_evk, follow below step:

Add CONFIG_XIP=n to mimxrt1050_evk_defconfig
build it with any sample
check zephyr.bin size
add large global variables to same sample and use these global variables to make sure it is not removed for optimization
build it again, you will find the size of zephyr.bin has grown

stephanosio · 2020-01-07T17:08:43Z

While I can confirm that behaviour, something is wrong here ...

/opt/zephyr-sdk/arm-zephyr-eabi/bin/arm-zephyr-eabi-objcopy -S --gap-fill 0xff --output-target=binary --remove-section=.comment --remove-section=COMMON --remove-section=.eh_frame zephyr.elf zephyr.bin

CONFIG_XIP=y

-rwxrwxr-x 1 stephanos stephanos 21464 Jan  8 01:24 zephyr/zephyr.bin

CONFIG_XIP=n

-rwxrwxr-x 1 stephanos stephanos 90676 Jan  8 01:25 zephyr/zephyr.bin

https://gist.github.com/stephanosio/a7f5cd34c7480bacb5254d4760b54805

The bss and noinit sections are marked NOBITS and the objcopy should not be putting them in the bin file (regardless of how the sections are ordered). Maybe I am missing something .. I will look into this tomorrow.

EDIT: This is the correct behaviour because the LMA for the non-XIP image is the same as the VMA.

stephanosio · 2020-01-09T08:01:59Z

If NOBITS section is not placed at the end of the ram, these sections will be placed to binary file. eg: bss, noinit

To be exact, it is not that the NOBITS sections are being "placed" into the output binary file; but rather, the gap between the last PROGBITS section before a NOBITS section and the next adjacent PROGBITS section must be filled before placing the data PROGBITS sections (note that LMA equals VMA in non-XIP images).

This in itself is not a bug (it does not break anything), but rather an inefficiency that comes with using the raw binary format.

stephanosio

I am not against this change because it does help reduce the output 'bin' file size, which can be useful if you must use the 'bin' image (e.g. booting from a 'bin' file in an SD card).

However, this seems to be breaking some existing platforms and they will need to be fixed before this PR can be approved.

lgl88911 · 2020-01-09T13:45:49Z

I am not against this change because it does help reduce the output 'bin' file size, which can be useful if you must use the 'bin' image (e.g. booting from a 'bin' file in an SD card).

However, this seems to be breaking some existing platforms and they will need to be fixed before this PR can be approved.

Thanks for your check.

Yes, I hope load bin from SD card to sdram, Decreasing bin size can help improve download and load speeds。

What kind of problems this change will affect those existing platforms？

lgl88911 · 2020-02-21T13:11:36Z

I am not against this change because it does help reduce the output 'bin' file size, which can be useful if you must use the 'bin' image (e.g. booting from a 'bin' file in an SD card).
However, this seems to be breaking some existing platforms and they will need to be fixed before this PR can be approved.

Thanks for your check.

Yes, I hope load bin from SD card to sdram, Decreasing bin size can help improve download and load speeds。

What kind of problems this change will affect those existing platforms？

I know "EXISTING PLATFORMS" problem is :
some object address in zephyr_prebuilt.elf diff to zephyr.elf. I'm trying to find the reason

lgl88911 · 2020-02-29T02:35:12Z

@stephanosio Shippable check pass, Could you help review again,Thanks

stephanosio

So, in summary, we cannot have the bss section as the last RAM section when USERSPACE is enabled; I wonder why that is.

IMO, we should figure out why the userspace tests fault when the bss section is placed as the last section in the RAM and provide a more fundamental fix, unless there is a technical reason as to why that is not possible and/or feasible.

I don't have time to look into it right now; if it does not get resolved in time, I will eventually come back to it.

include/arch/arm/aarch32/cortex_m/scripts/linker.ld

lgl88911 · 2020-03-06T13:31:52Z

So, in summary, we cannot have the bss section as the last RAM section when USERSPACE is enabled; I wonder why that is.

IMO, we should figure out why the userspace tests fault when the bss section is placed as the last section in the RAM and provide a more fundamental fix, unless there is a technical reason as to why that is not possible and/or feasible.

I don't have time to look into it right now; if it does not get resolved in time, I will eventually come back to it.

When bss moves to the end, the bss section will be after the kobject_data section.
Uninitialized kernel objects are placed in bss.
When userspace is enabled, there is no data in kobject_data during the first stage of linking(prelink), so the kernel object address is small.
Before the second stage of linking(finial), the kernel objects in prelink are scanned and their addresses are placed in kobject_data.
At this time, kobject_data becomes larger, bss be shifted backwards , cause the kernel object address in the final link result will become larger, which will be inconsistent with that in kobject_data.
If the position of bss is before kobject_data, there will not be this problem.
The appropriate method may be to reserve space for kobject_data during prelink, but the related process is relatively unfamiliar to me, so it is difficult to modify.

andrewboie · 2020-03-06T18:51:46Z

The appropriate method may be to reserve space for kobject_data during prelink, but the related process is relatively unfamiliar to me, so it is difficult to modify.

The relevant linker snippet is include/linker/kobject.ld.

I don't have a good suggestion to fix this at the moment. The problem is that the size of this area is a linear function of the number of kernel objects in the binary, there isn't a good way to predict how large it is without going through the full process of building.

lgl88911 · 2020-03-07T02:10:54Z

The appropriate method may be to reserve space for kobject_data during prelink, but the related process is relatively unfamiliar to me, so it is difficult to modify.

The relevant linker snippet is include/linker/kobject.ld.

I don't have a good suggestion to fix this at the moment. The problem is that the size of this area is a linear function of the number of kernel objects in the binary, there isn't a good way to predict how large it is without going through the full process of building.

Indeed, the inability to predict kernel object data is a problem. Maybe reserve enough space, or add a third link, use the final elf to generate kobject_hash.c again, and relink the new elf.
But both methods are "not pretty", so my idea is to make this scheme work on non-userspace first.

stephanosio · 2020-03-07T04:06:58Z

Indeed, the inability to predict kernel object data is a problem. Maybe reserve enough space, or add a third link, use the final elf to generate kobject_hash.c again, and relink the new elf.
But both methods are "not pretty", so my idea is to make this scheme work on non-userspace first.

Thanks for the explanation. It looks like that will require a major overhaul, which should probably be a separate PR.

stephanosio

Approving for now, noting that it is not feasible for userspace to have the bss as the last section in the RAM.

fix bss section be copy to binary. When compiling with non-xip. All sections will be placed in the ram. If NOBITS section isn't placed at the end of ram, objcopy will placed these section to binary file. The modify only for no-userspace. In userspace the section will be adjust, kobject_data size increase, the bss be shifted, the content of kobject_data will no mactch to kernel obj of bss. Signed-off-by: Frank Li <lgl88911@163.com>

lgl88911 · 2020-04-03T12:05:13Z

@galak @MaureenHelm @andrewboie @ioannisg This PR has been pending for a long time, can you help review if need to make other changes?

carlescufi · 2020-04-22T18:15:25Z

@galak @MaureenHelm @andrewboie @ioannisg This PR has been pending for a long time, can you help review if need to make other changes?

Yep, this should be followed-up. @lgl88911 if you get no response before tomorrow I can suggest you add it to the dev-review meeting and join if you are able to. But I think this mostly needs an approval from @ioannisg.

andrewboie

Fine with this for now, but I'm going to open a GH enhancement to investigate this more deeply, such that if user mode is enabled on a non-XIP system the binary isn't inflated by NOBITS sections.

lgl88911 · 2020-04-24T13:25:53Z

@carlescufi Is there any document explaining how to proceed with "add item to the dev-review meeting", I have never done this. Thanks!

…mode Userspace originally required bss to precede kobject_data, as kobject_data size was not known until the final linking stage (zephyrproject-rtos#21747). zephyrproject-rtos#33687 reserved space for kobject_data, allowing bss to now be after kobject_data. Placing bss/noinit at the end of the linker script reduces the size of a "zephyr.bin" file, as the NOLOAD sections can be excluded. It also keeps the code common between kernel/user mode Signed-off-by: Grant Ramsay <gramsay@enphaseenergy.com>

…mode Userspace originally required bss to precede kobject_data, as kobject_data size was not known until the final linking stage (#21747). #33687 reserved space for kobject_data, allowing bss to now be after kobject_data. Placing bss/noinit at the end of the linker script reduces the size of a "zephyr.bin" file, as the NOLOAD sections can be excluded. It also keeps the code common between kernel/user mode Signed-off-by: Grant Ramsay <gramsay@enphaseenergy.com>

…mode Userspace originally required bss to precede kobject_data, as kobject_data size was not known until the final linking stage (zephyrproject-rtos#21747). zephyrproject-rtos#33687 reserved space for kobject_data, allowing bss to now be after kobject_data. Placing bss/noinit at the end of the linker script reduces the size of a "zephyr.bin" file, as the NOLOAD sections can be excluded. It also keeps the code common between kernel/user mode (cherry picked from commit f9d69fc) Original-Signed-off-by: Grant Ramsay <gramsay@enphaseenergy.com> GitOrigin-RevId: f9d69fc Cr-Build-Id: 8723641050355720609 Cr-Build-Url: https://cr-buildbucket.appspot.com/build/8723641050355720609 Copybot-Job-Name: zephyr-main-copybot-downstream Change-Id: I96b5fbac36ff22a501f30c08c1b4ca42a0425fec Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/6241424 Commit-Queue: Jeremy Bettis <jbettis@chromium.org> Tested-by: Jeremy Bettis <jbettis@chromium.org> Tested-by: ChromeOS Prod (Robot) <chromeos-ci-prod@chromeos-bot.iam.gserviceaccount.com> Reviewed-by: Jonathon Murphy <jpmurphy@google.com> Reviewed-by: Jeremy Bettis <jbettis@chromium.org> Bot-Commit: ChromeOS Prod (Robot) <chromeos-ci-prod@chromeos-bot.iam.gserviceaccount.com>

lgl88911 requested review from galak, ioannisg and MaureenHelm as code owners January 7, 2020 15:14

zephyrbot added the area: API Changes to public APIs label Jan 7, 2020

stephanosio requested changes Jan 7, 2020

View reviewed changes

include/arch/arm/aarch32/cortex_m/scripts/linker.ld Outdated Show resolved Hide resolved

lgl88911 force-pushed the fix_ld_bss branch from 62c20de to d58b5fe Compare January 7, 2020 16:14

stephanosio added area: ARM ARM (32-bit) Architecture area: Linker Scripts and removed area: API Changes to public APIs labels Jan 7, 2020

stephanosio reviewed Jan 9, 2020

View reviewed changes

stephanosio self-requested a review January 9, 2020 08:29

galak assigned ioannisg Jan 28, 2020

lgl88911 force-pushed the fix_ld_bss branch from d58b5fe to fbcd431 Compare February 22, 2020 13:58

zephyrbot added the area: API Changes to public APIs label Feb 22, 2020

stephanosio reviewed Mar 6, 2020

View reviewed changes

include/arch/arm/aarch32/cortex_m/scripts/linker.ld Outdated Show resolved Hide resolved

include/arch/arm/aarch32/cortex_m/scripts/linker.ld Outdated Show resolved Hide resolved

include/arch/arm/aarch32/cortex_m/scripts/linker.ld Outdated Show resolved Hide resolved

stephanosio requested review from andrewboie and stephanosio March 6, 2020 01:56

lgl88911 force-pushed the fix_ld_bss branch 2 times, most recently from 5bd7a02 to 324f1bd Compare March 6, 2020 13:29

stephanosio approved these changes Mar 7, 2020

View reviewed changes

lgl88911 force-pushed the fix_ld_bss branch from 324f1bd to 5fd1f5f Compare March 7, 2020 12:27

andrewboie approved these changes Apr 22, 2020

View reviewed changes

andrewboie mentioned this pull request Apr 22, 2020

CONFIG_USERSPACE=y CONFIG_XIP=n causes .bin space to be wasted #24619

Closed

ioannisg added the dev-review To be discussed in dev-review meeting label Apr 24, 2020

galak removed the dev-review To be discussed in dev-review meeting label Apr 30, 2020

carlescufi merged commit a298c89 into zephyrproject-rtos:master May 4, 2020

yashi mentioned this pull request Jun 21, 2021

Move BSS section to the end of image #36297

Closed

tejlmand mentioned this pull request Dec 21, 2021

Linker: ARM64: the binary file always constains .bss section. #41354

Closed

gramsay0 mentioned this pull request Oct 17, 2024

arch: arm: cortex_m: Move bss/noinit sections to the end of linker script #74042

Merged

gramsay0 mentioned this pull request Jan 9, 2025

arch: arm: cortex_m: Consolidate bss/noinit sections for kernel/user mode #83721

Merged

arm: linker.ld: move bss section to ram end #21747

arm: linker.ld: move bss section to ram end #21747

Uh oh!

Conversation

lgl88911 commented Jan 7, 2020

Uh oh!

stephanosio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ioannisg commented Jan 7, 2020

Uh oh!

lgl88911 commented Jan 7, 2020

Uh oh!

stephanosio commented Jan 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stephanosio commented Jan 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stephanosio left a comment

Choose a reason for hiding this comment

Uh oh!

lgl88911 commented Jan 9, 2020

Uh oh!

lgl88911 commented Feb 21, 2020

Uh oh!

lgl88911 commented Feb 29, 2020

Uh oh!

stephanosio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

andrewboie commented Mar 6, 2020

Uh oh!

lgl88911 commented Mar 7, 2020

Uh oh!

stephanosio commented Mar 7, 2020

Uh oh!

stephanosio left a comment

Choose a reason for hiding this comment

Uh oh!

lgl88911 commented Apr 3, 2020

Uh oh!

carlescufi commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewboie left a comment

Choose a reason for hiding this comment

Uh oh!

lgl88911 commented Apr 24, 2020

Uh oh!

Uh oh!

stephanosio commented Jan 7, 2020 •

edited

Loading

stephanosio commented Jan 9, 2020 •

edited

Loading

lgl88911 commented Mar 6, 2020 •

edited

Loading

carlescufi commented Apr 22, 2020 •

edited

Loading