J2 based devices expect to find a devicetree blob at the end of the bss
section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the
dtb, causing J2 devices to fail early in sh_fdt_init.
As J2 loader firmware calculates the dtb location based on the kernel
image .bss section size, rather than the __bss_stop symbol offset, the
required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE, 8).
Instead, inline modified version of the above macro, which grows .bss
by the required size.
While this change affects all existing SH boards, it should be benign on
platforms which don't need this alignment.
Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
---
arch/sh/kernel/vmlinux.lds.S | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/sh/kernel/vmlinux.lds.S b/arch/sh/kernel/vmlinux.lds.S
index 9644fe187a3f..008c30289eaa 100644
--- a/arch/sh/kernel/vmlinux.lds.S
+++ b/arch/sh/kernel/vmlinux.lds.S
@@ -71,7 +71,20 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
__init_end = .;
- BSS_SECTION(0, PAGE_SIZE, 4)
+ __bss_start = .;
+ SBSS(0)
+ . = ALIGN(PAGE_SIZE);
+ .bss : AT(ADDR(.bss) - LOAD_OFFSET) {
+ BSS_FIRST_SECTIONS
+ . = ALIGN(PAGE_SIZE);
+ *(.bss..page_aligned)
+ . = ALIGN(PAGE_SIZE);
+ *(.dynbss)
+ *(BSS_MAIN)
+ *(COMMON)
+ . = ALIGN(8);
+ }
+ __bss_stop = .;
_end = . ;
STABS_DEBUG
--
2.48.1Hi Artur,
On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
> J2 based devices expect to find a devicetree blob at the end of the bss
> section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the
> dtb, causing J2 devices to fail early in sh_fdt_init.
>
> As J2 loader firmware calculates the dtb location based on the kernel
> image .bss section size, rather than the __bss_stop symbol offset, the
> required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE, 8).
> Instead, inline modified version of the above macro, which grows .bss
> by the required size.
>
> While this change affects all existing SH boards, it should be benign on
> platforms which don't need this alignment.
>
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---
> arch/sh/kernel/vmlinux.lds.S | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/sh/kernel/vmlinux.lds.S b/arch/sh/kernel/vmlinux.lds.S
> index 9644fe187a3f..008c30289eaa 100644
> --- a/arch/sh/kernel/vmlinux.lds.S
> +++ b/arch/sh/kernel/vmlinux.lds.S
> @@ -71,7 +71,20 @@ SECTIONS
>
> . = ALIGN(PAGE_SIZE);
> __init_end = .;
> - BSS_SECTION(0, PAGE_SIZE, 4)
> + __bss_start = .;
> + SBSS(0)
> + . = ALIGN(PAGE_SIZE);
> + .bss : AT(ADDR(.bss) - LOAD_OFFSET) {
> + BSS_FIRST_SECTIONS
> + . = ALIGN(PAGE_SIZE);
> + *(.bss..page_aligned)
> + . = ALIGN(PAGE_SIZE);
> + *(.dynbss)
> + *(BSS_MAIN)
> + *(COMMON)
> + . = ALIGN(8);
> + }
> + __bss_stop = .;
> _end = . ;
>
> STABS_DEBUG
I'll pick this up for now since Uros has confirmed that the compiler
won't just use SBSS without breaking the ABI, so I think to use this
fix for now.
If it breaks in the future, we can change it again.
Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Hi Artur,
I'm currently trying to review this patch, but I'm not 100% sure how it
this change helps grows the .bss section, see below. Maybe you can help
me understand what's happening.
On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
> J2 based devices expect to find a devicetree blob at the end of the bss
> section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the
> dtb, causing J2 devices to fail early in sh_fdt_init.
>
> As J2 loader firmware calculates the dtb location based on the kernel
> image .bss section size, rather than the __bss_stop symbol offset, the
> required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE, 8).
> Instead, inline modified version of the above macro, which grows .bss
> by the required size.
>
> While this change affects all existing SH boards, it should be benign on
> platforms which don't need this alignment.
>
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---
> arch/sh/kernel/vmlinux.lds.S | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/sh/kernel/vmlinux.lds.S b/arch/sh/kernel/vmlinux.lds.S
> index 9644fe187a3f..008c30289eaa 100644
> --- a/arch/sh/kernel/vmlinux.lds.S
> +++ b/arch/sh/kernel/vmlinux.lds.S
> @@ -71,7 +71,20 @@ SECTIONS
>
> . = ALIGN(PAGE_SIZE);
> __init_end = .;
> - BSS_SECTION(0, PAGE_SIZE, 4)
> + __bss_start = .;
> + SBSS(0)
> + . = ALIGN(PAGE_SIZE);
What this effectively does is removing ". = ALIGN(sbss_align);" first from BSS_SECTION().
Then it inserts ". = ALIGN(PAGE_SIZE);" after the "SBSS(0)".
If I understand this correctly, SBSS() inserts a zero-padding and if I'm not mistaken,
inserting ". = ALIGN(PAGE_SIZE);" will cause this padding to grow to at least PAGE_SIZE
due the alignment.
Is this correct?
> + .bss : AT(ADDR(.bss) - LOAD_OFFSET) {
> + BSS_FIRST_SECTIONS
> + . = ALIGN(PAGE_SIZE);
> + *(.bss..page_aligned)
> + . = ALIGN(PAGE_SIZE);
> + *(.dynbss)
> + *(BSS_MAIN)
> + *(COMMON)
> + . = ALIGN(8);
If my understanding above is correct, why do we will need an additional ". = ALIGN(8)"
here?
> + }
> + __bss_stop = .;
> _end = . ;
>
> STABS_DEBUG
Thanks,
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
On 2025-03-11 18:28, John Paul Adrian Glaubitz wrote:
> Hi Artur,
Hey Adrian,
thanks for looking into this patch.
>
> I'm currently trying to review this patch, but I'm not 100% sure how it
> this change helps grows the .bss section, see below. Maybe you can help
> me understand what's happening.
>
> On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
>> J2 based devices expect to find a devicetree blob at the end of the
>> bss
>> section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the
>> dtb, causing J2 devices to fail early in sh_fdt_init.
>>
>> As J2 loader firmware calculates the dtb location based on the kernel
>> image .bss section size, rather than the __bss_stop symbol offset, the
>> required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE,
>> 8).
>> Instead, inline modified version of the above macro, which grows .bss
>> by the required size.
>>
>> While this change affects all existing SH boards, it should be benign
>> on
>> platforms which don't need this alignment.
>>
>> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
>> ---
>> arch/sh/kernel/vmlinux.lds.S | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/sh/kernel/vmlinux.lds.S
>> b/arch/sh/kernel/vmlinux.lds.S
>> index 9644fe187a3f..008c30289eaa 100644
>> --- a/arch/sh/kernel/vmlinux.lds.S
>> +++ b/arch/sh/kernel/vmlinux.lds.S
>> @@ -71,7 +71,20 @@ SECTIONS
>>
>> . = ALIGN(PAGE_SIZE);
>> __init_end = .;
>> - BSS_SECTION(0, PAGE_SIZE, 4)
>> + __bss_start = .;
>> + SBSS(0)
>> + . = ALIGN(PAGE_SIZE);
>
> What this effectively does is removing ". = ALIGN(sbss_align);" first
> from BSS_SECTION().
>
> Then it inserts ". = ALIGN(PAGE_SIZE);" after the "SBSS(0)".
>
> If I understand this correctly, SBSS() inserts a zero-padding and if
> I'm not mistaken,
> inserting ". = ALIGN(PAGE_SIZE);" will cause this padding to grow to at
> least PAGE_SIZE
> due the alignment.
>
> Is this correct?
>
>> + .bss : AT(ADDR(.bss) - LOAD_OFFSET) {
>> + BSS_FIRST_SECTIONS
>> + . = ALIGN(PAGE_SIZE);
>> + *(.bss..page_aligned)
>> + . = ALIGN(PAGE_SIZE);
>> + *(.dynbss)
>> + *(BSS_MAIN)
>> + *(COMMON)
>> + . = ALIGN(8);
>
> If my understanding above is correct, why do we will need an additional
> ". = ALIGN(8)"
> here?
I'll tackle both of the above questions at once.
I'm by no means an expert at GNU Linker syntax, but the intention of
this patch is to put . = ALIGN(8) inside the .bss : { ... } section
definition, so that the section itself grows by the requested padding.
In the original BSS_SECTION(0, PAGE_SIZE, 4), the last argument inserts
a 4 byte padding after the closing brace of .bss section definition,
causing the __bss_stop symbol offset to grow, but not the .bss section
itself:
#define BSS_SECTION(sbss_align, bss_align, stop_align) \
. = ALIGN(sbss_align); \
__bss_start = .; \
SBSS(sbss_align) \
BSS(bss_align) \
. = ALIGN(stop_align); \
__bss_stop = .;
TurtleBoard loader is only concerned with the .bss section size - it
doesn't care about any symbol offsets - and hence this seemingly cryptic
change (you can display the section size information with
readelf -t kernel_image).
The rest of the changes are simply to "inline" the BSS() macro (as I
needed to access that closing brace), and the former sbss_align,
bss_align (that's your PAGE_SIZE) and stop_align arguments are passed
accordingly, the same way they used to be passed before. The only
visible effect should be the move of ALIGN(stop_align) inside of .bss
section definition, and the change of stop_align value from 4 to 8.
Arguably the TurtleBoard loader should read the __bss_stop symbol offset
instead, but in this patch I'm trying to solve the issue from kernel's
point of view.
Cheers,
Artur
>
>> + }
>> + __bss_stop = .;
>> _end = . ;
>>
>> STABS_DEBUG
>
> Thanks,
> Adrian
Hi Artur,
On Wed, 2025-03-12 at 00:40 +0100, Artur Rojek wrote:
> On 2025-03-11 18:28, John Paul Adrian Glaubitz wrote:
> > Hi Artur,
>
> Hey Adrian,
> thanks for looking into this patch.
Sure. I just want to understand what's going on before signing it with "Reviewed-by",
I wouldn't dare that without fully understanding what the proposed change does ;-).
> > What this effectively does is removing ". = ALIGN(sbss_align);" first
> > from BSS_SECTION().
> >
> > Then it inserts ". = ALIGN(PAGE_SIZE);" after the "SBSS(0)".
> >
> > If I understand this correctly, SBSS() inserts a zero-padding and if
> > I'm not mistaken,
> > inserting ". = ALIGN(PAGE_SIZE);" will cause this padding to grow to at
> > least PAGE_SIZE
> > due the alignment.
> >
> > Is this correct?
> >
> > > + .bss : AT(ADDR(.bss) - LOAD_OFFSET) {
> > > + BSS_FIRST_SECTIONS
> > > + . = ALIGN(PAGE_SIZE);
> > > + *(.bss..page_aligned)
> > > + . = ALIGN(PAGE_SIZE);
> > > + *(.dynbss)
> > > + *(BSS_MAIN)
> > > + *(COMMON)
> > > + . = ALIGN(8);
> >
> > If my understanding above is correct, why do we will need an additional
> > ". = ALIGN(8)"
> > here?
>
> I'll tackle both of the above questions at once.
> I'm by no means an expert at GNU Linker syntax, but the intention of
> this patch is to put . = ALIGN(8) inside the .bss : { ... } section
> definition, so that the section itself grows by the requested padding.
Makes sense.
> In the original BSS_SECTION(0, PAGE_SIZE, 4), the last argument inserts
> a 4 byte padding after the closing brace of .bss section definition,
> causing the __bss_stop symbol offset to grow, but not the .bss section
> itself:
>
> #define BSS_SECTION(sbss_align, bss_align, stop_align) \
> . = ALIGN(sbss_align); \
> __bss_start = .; \
> SBSS(sbss_align) \
> BSS(bss_align) \
> . = ALIGN(stop_align); \
> __bss_stop = .;
OK, that's really odd. So, the __bss_stop would be moved to the desired
position but the section itself still remains small? What exactly does the
linker fill the region with? Sounds very strange.
> TurtleBoard loader is only concerned with the .bss section size - it
> doesn't care about any symbol offsets - and hence this seemingly cryptic
> change (you can display the section size information with
> readelf -t kernel_image).
Looking at the actual kernel image with readelf is a very good suggestion!
> The rest of the changes are simply to "inline" the BSS() macro (as I
> needed to access that closing brace), and the former sbss_align,
> bss_align (that's your PAGE_SIZE) and stop_align arguments are passed
> accordingly, the same way they used to be passed before. The only
> visible effect should be the move of ALIGN(stop_align) inside of .bss
> section definition, and the change of stop_align value from 4 to 8.
OK. FWIW, do you understand what SBSS is for? I couldn't find any explanation
for it.
> Arguably the TurtleBoard loader should read the __bss_stop symbol offset
> instead, but in this patch I'm trying to solve the issue from kernel's
> point of view.
That's absolutely sensible as this avoids having to update the firmware.
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
On Wed, Mar 12, 2025 at 9:22 AM John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> wrote: > > In the original BSS_SECTION(0, PAGE_SIZE, 4), the last argument inserts > > a 4 byte padding after the closing brace of .bss section definition, > > causing the __bss_stop symbol offset to grow, but not the .bss section > > itself: > > > > #define BSS_SECTION(sbss_align, bss_align, stop_align) \ > > . = ALIGN(sbss_align); \ > > __bss_start = .; \ > > SBSS(sbss_align) \ > > BSS(bss_align) \ > > . = ALIGN(stop_align); \ > > __bss_stop = .; > > OK, that's really odd. So, the __bss_stop would be moved to the desired > position but the section itself still remains small? What exactly does the > linker fill the region with? Sounds very strange. > > > TurtleBoard loader is only concerned with the .bss section size - it > > doesn't care about any symbol offsets - and hence this seemingly cryptic > > change (you can display the section size information with > > readelf -t kernel_image). > > Looking at the actual kernel image with readelf is a very good suggestion! > > > The rest of the changes are simply to "inline" the BSS() macro (as I > > needed to access that closing brace), and the former sbss_align, > > bss_align (that's your PAGE_SIZE) and stop_align arguments are passed > > accordingly, the same way they used to be passed before. The only > > visible effect should be the move of ALIGN(stop_align) inside of .bss > > section definition, and the change of stop_align value from 4 to 8. > > OK. FWIW, do you understand what SBSS is for? I couldn't find any explanation > for it. Small BSS section. The compiler can put data objects under a certain size threshold to the .sbss section. Looking at GCC sh config, sh does not use this section. Uros.
Hi Uros,
On Wed, 12 Mar 2025 at 09:32, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Mar 12, 2025 at 9:22 AM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
> > > In the original BSS_SECTION(0, PAGE_SIZE, 4), the last argument inserts
> > > a 4 byte padding after the closing brace of .bss section definition,
> > > causing the __bss_stop symbol offset to grow, but not the .bss section
> > > itself:
> > >
> > > #define BSS_SECTION(sbss_align, bss_align, stop_align) \
> > > . = ALIGN(sbss_align); \
> > > __bss_start = .; \
> > > SBSS(sbss_align) \
> > > BSS(bss_align) \
> > > . = ALIGN(stop_align); \
> > > __bss_stop = .;
> >
> > OK, that's really odd. So, the __bss_stop would be moved to the desired
> > position but the section itself still remains small? What exactly does the
> > linker fill the region with? Sounds very strange.
> >
> > > TurtleBoard loader is only concerned with the .bss section size - it
> > > doesn't care about any symbol offsets - and hence this seemingly cryptic
> > > change (you can display the section size information with
> > > readelf -t kernel_image).
> >
> > Looking at the actual kernel image with readelf is a very good suggestion!
> >
> > > The rest of the changes are simply to "inline" the BSS() macro (as I
> > > needed to access that closing brace), and the former sbss_align,
> > > bss_align (that's your PAGE_SIZE) and stop_align arguments are passed
> > > accordingly, the same way they used to be passed before. The only
> > > visible effect should be the move of ALIGN(stop_align) inside of .bss
> > > section definition, and the change of stop_align value from 4 to 8.
> >
> > OK. FWIW, do you understand what SBSS is for? I couldn't find any explanation
> > for it.
>
> Small BSS section. The compiler can put data objects under a certain
> size threshold to the .sbss section. Looking at GCC sh config, sh does
> not use this section.
Hence the moment gcc (or clang) starts using that section, the
TurtleBoard loader is broken again...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Wed, 12 Mar 2025 10:47:00 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Uros, > > On Wed, 12 Mar 2025 at 09:32, Uros Bizjak <ubizjak@gmail.com> wrote: > > On Wed, Mar 12, 2025 at 9:22 AM John Paul Adrian Glaubitz > > <glaubitz@physik.fu-berlin.de> wrote: .... > > > OK. FWIW, do you understand what SBSS is for? I couldn't find any explanation > > > for it. > > > > Small BSS section. The compiler can put data objects under a certain > > size threshold to the .sbss section. Looking at GCC sh config, sh does > > not use this section. The .sbss (and .sdata) sections are used by some architectures (eg Nios2) for data that can be accessed using a 16bit offset from a fixed register. (Although using the register as global register variable generates better code.) Historically they may have been used for data at the top/bottom of the addresses space (for the same reason). There is no reason to just use it for 'small' (eg 8 bytes or less) data, that is just some empirical default. I guess they could also be used on cpu like the strongarm for memory that used the 'small data cache' (useful for screen buffer memory!). David
On Wed, Mar 12, 2025 at 10:47 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Uros,
>
> On Wed, 12 Mar 2025 at 09:32, Uros Bizjak <ubizjak@gmail.com> wrote:
> > On Wed, Mar 12, 2025 at 9:22 AM John Paul Adrian Glaubitz
> > <glaubitz@physik.fu-berlin.de> wrote:
> > > > In the original BSS_SECTION(0, PAGE_SIZE, 4), the last argument inserts
> > > > a 4 byte padding after the closing brace of .bss section definition,
> > > > causing the __bss_stop symbol offset to grow, but not the .bss section
> > > > itself:
> > > >
> > > > #define BSS_SECTION(sbss_align, bss_align, stop_align) \
> > > > . = ALIGN(sbss_align); \
> > > > __bss_start = .; \
> > > > SBSS(sbss_align) \
> > > > BSS(bss_align) \
> > > > . = ALIGN(stop_align); \
> > > > __bss_stop = .;
> > >
> > > OK, that's really odd. So, the __bss_stop would be moved to the desired
> > > position but the section itself still remains small? What exactly does the
> > > linker fill the region with? Sounds very strange.
> > >
> > > > TurtleBoard loader is only concerned with the .bss section size - it
> > > > doesn't care about any symbol offsets - and hence this seemingly cryptic
> > > > change (you can display the section size information with
> > > > readelf -t kernel_image).
> > >
> > > Looking at the actual kernel image with readelf is a very good suggestion!
> > >
> > > > The rest of the changes are simply to "inline" the BSS() macro (as I
> > > > needed to access that closing brace), and the former sbss_align,
> > > > bss_align (that's your PAGE_SIZE) and stop_align arguments are passed
> > > > accordingly, the same way they used to be passed before. The only
> > > > visible effect should be the move of ALIGN(stop_align) inside of .bss
> > > > section definition, and the change of stop_align value from 4 to 8.
> > >
> > > OK. FWIW, do you understand what SBSS is for? I couldn't find any explanation
> > > for it.
> >
> > Small BSS section. The compiler can put data objects under a certain
> > size threshold to the .sbss section. Looking at GCC sh config, sh does
> > not use this section.
>
> Hence the moment gcc (or clang) starts using that section, the
> TurtleBoard loader is broken again...
Rest assured that the compiler won't just magically start using SBSS.
This is part of an ABI and in case ABI allows SBSS, the compiler needs
something like:
if (in_small_data)
switch_to_section (get_named_section (NULL, ".sbss", 0));
when emitting the declaration.
Uros.
Hi Uros, On Wed, 2025-03-12 at 09:32 +0100, Uros Bizjak wrote: > > OK. FWIW, do you understand what SBSS is for? I couldn't find any explanation > > for it. > > Small BSS section. The compiler can put data objects under a certain > size threshold to the .sbss section. Looking at GCC sh config, sh does > not use this section. Thank you, that helps me understand what's going a bit more! Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Hi Artur,
On Wed, 12 Mar 2025 at 00:40, Artur Rojek <contact@artur-rojek.eu> wrote:
> On 2025-03-11 18:28, John Paul Adrian Glaubitz wrote:
> > I'm currently trying to review this patch, but I'm not 100% sure how it
> > this change helps grows the .bss section, see below. Maybe you can help
> > me understand what's happening.
> >
> > On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
> >> J2 based devices expect to find a devicetree blob at the end of the
> >> bss
> >> section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the
> >> dtb, causing J2 devices to fail early in sh_fdt_init.
> >>
> >> As J2 loader firmware calculates the dtb location based on the kernel
> >> image .bss section size, rather than the __bss_stop symbol offset, the
> >> required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE,
> >> 8).
> >> Instead, inline modified version of the above macro, which grows .bss
> >> by the required size.
> >>
> >> While this change affects all existing SH boards, it should be benign
> >> on
> >> platforms which don't need this alignment.
> >>
> >> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> >> ---
> >> arch/sh/kernel/vmlinux.lds.S | 15 ++++++++++++++-
> >> 1 file changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/sh/kernel/vmlinux.lds.S
> >> b/arch/sh/kernel/vmlinux.lds.S
> >> index 9644fe187a3f..008c30289eaa 100644
> >> --- a/arch/sh/kernel/vmlinux.lds.S
> >> +++ b/arch/sh/kernel/vmlinux.lds.S
> >> @@ -71,7 +71,20 @@ SECTIONS
> >>
> >> . = ALIGN(PAGE_SIZE);
> >> __init_end = .;
> >> - BSS_SECTION(0, PAGE_SIZE, 4)
> >> + __bss_start = .;
> >> + SBSS(0)
> >> + . = ALIGN(PAGE_SIZE);
> >
> > What this effectively does is removing ". = ALIGN(sbss_align);" first
> > from BSS_SECTION().
> >
> > Then it inserts ". = ALIGN(PAGE_SIZE);" after the "SBSS(0)".
> >
> > If I understand this correctly, SBSS() inserts a zero-padding and if
> > I'm not mistaken,
> > inserting ". = ALIGN(PAGE_SIZE);" will cause this padding to grow to at
> > least PAGE_SIZE
> > due the alignment.
> >
> > Is this correct?
> >
> >> + .bss : AT(ADDR(.bss) - LOAD_OFFSET) {
> >> + BSS_FIRST_SECTIONS
> >> + . = ALIGN(PAGE_SIZE);
> >> + *(.bss..page_aligned)
> >> + . = ALIGN(PAGE_SIZE);
> >> + *(.dynbss)
> >> + *(BSS_MAIN)
> >> + *(COMMON)
> >> + . = ALIGN(8);
> >
> > If my understanding above is correct, why do we will need an additional
> > ". = ALIGN(8)"
> > here?
>
> I'll tackle both of the above questions at once.
> I'm by no means an expert at GNU Linker syntax, but the intention of
> this patch is to put . = ALIGN(8) inside the .bss : { ... } section
> definition, so that the section itself grows by the requested padding.
>
> In the original BSS_SECTION(0, PAGE_SIZE, 4), the last argument inserts
> a 4 byte padding after the closing brace of .bss section definition,
> causing the __bss_stop symbol offset to grow, but not the .bss section
> itself:
>
> #define BSS_SECTION(sbss_align, bss_align, stop_align) \
> . = ALIGN(sbss_align); \
> __bss_start = .; \
> SBSS(sbss_align) \
> BSS(bss_align) \
> . = ALIGN(stop_align); \
> __bss_stop = .;
>
> TurtleBoard loader is only concerned with the .bss section size - it
> doesn't care about any symbol offsets - and hence this seemingly cryptic
> change (you can display the section size information with
> readelf -t kernel_image).
> The rest of the changes are simply to "inline" the BSS() macro (as I
> needed to access that closing brace), and the former sbss_align,
> bss_align (that's your PAGE_SIZE) and stop_align arguments are passed
> accordingly, the same way they used to be passed before. The only
> visible effect should be the move of ALIGN(stop_align) inside of .bss
> section definition, and the change of stop_align value from 4 to 8.
>
> Arguably the TurtleBoard loader should read the __bss_stop symbol offset
> instead, but in this patch I'm trying to solve the issue from kernel's
> point of view.
What about moving (or duplicating, e.g. sbss_align alignment is
done before and after __bss_start) the stop_align alignment
from BSS_SECTION() into BSS() instead, i.e. just changing
include/asm-generic/vmlinux.lds.h for everyone? I don't think that
would hurt any platforms, while fixing the issue for good.
IMHO it is a bit strange that the size of the bss section can differ
from __bss_stop - __bss_start.
One last question though: what about sbss? How does the TurtleBoard
loader handle that? __bss_stop - __bss_start is not the size of bss,
but the sum of the sizes of sbss and bss, plus extra alignment in
between. The latter might cause trouble, too.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert, On Wed, 2025-03-12 at 09:06 +0100, Geert Uytterhoeven wrote: > What about moving (or duplicating, e.g. sbss_align alignment is > done before and after __bss_start) the stop_align alignment > from BSS_SECTION() into BSS() instead, i.e. just changing > include/asm-generic/vmlinux.lds.h for everyone? I don't think that > would hurt any platforms, while fixing the issue for good. > IMHO it is a bit strange that the size of the bss section can differ > from __bss_stop - __bss_start. This sounds reasonable. Could you send a patch? I assume that would go through a different tree as we're touching generic code. > One last question though: what about sbss? How does the TurtleBoard > loader handle that? __bss_stop - __bss_start is not the size of bss, > but the sum of the sizes of sbss and bss, plus extra alignment in > between. The latter might cause trouble, too. Does the compiler actually generate the SBSS section on SH? Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
On 2/16/25 11:55, Artur Rojek wrote: > J2 based devices expect to find a devicetree blob at the end of the bss > section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the > dtb, causing J2 devices to fail early in sh_fdt_init. > > As J2 loader firmware calculates the dtb location based on the kernel > image .bss section size, rather than the __bss_stop symbol offset, the > required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE, 8). > Instead, inline modified version of the above macro, which grows .bss > by the required size. > > While this change affects all existing SH boards, it should be benign on > platforms which don't need this alignment. > > Signed-off-by: Artur Rojek <contact@artur-rojek.eu> Tested-by: Rob Landley <rob@landley.net> Rob
© 2016 - 2025 Red Hat, Inc.