Hi Richard,
On Thu, 14 Mar 2024 at 00:43, Richard Henderson <richard.hender...@linaro.org> wrote: > > On 3/13/24 11:43, Ilias Apalodimas wrote: > > Hi Richard, > > > > On Wed, 13 Mar 2024 at 22:19, Richard Henderson > > <richard.hender...@linaro.org> wrote: > >> > >> On 3/13/24 06:23, Ilias Apalodimas wrote: > >>> +++ b/arch/arm/cpu/armv8/u-boot-spl.lds > >>> @@ -63,18 +63,11 @@ SECTIONS > >>> > >>> _image_binary_end = .; > >>> > >>> - .bss_start (NOLOAD) : { > >>> - . = ALIGN(8); > >>> - KEEP(*(.__bss_start)); > >>> - } >.sdram > >>> - > >>> - .bss (NOLOAD) : { > >>> + .bss : { > >>> + __bss_start = .; > >>> *(.bss*) > >>> - . = ALIGN(8); > >>> - } >.sdram > >>> - > >>> - .bss_end (NOLOAD) : { > >>> - KEEP(*(.__bss_end)); > >>> + . = ALIGN(8); > >>> + __bss_end = .; > >> > >> Still missing the alignment on .bss, previously in .bss_start. > > > > Since this is emitted in .sdram memory I can't define it as > > .bss ALIGN(8) : {....} since the calculated memory will be outside the > > sdram-defined region > > > > I could define it as > > .bss : { > > . = ALIGN(8); > > __bss_start = .; > > ...... > > } > > > > But instead, I added an assert at the bottom which will break the > > linking if the __bss_start is not 8byte aligned. > > I think it would be clearer to assert on __bss_start address rather than > CONFIG_SPL_BSS_START_ADDR, if that's possible. If not, then the constant > will have to do. Fair enough and it is possible. I can convert that assertion to ASSERT(ADDR(.bss) % 8 == 0, ".bss must be 8-byte aligned"); Keep in mind that .bss, due to the linker aligning the section to the strictest input section requirement will end up aligning that to 8b regardless. IOW compiling with CONFIG_SPL_BSS_START_ADDR=0x1001, won't break the linking since .bss will end up at 0x1008. Testing the assertion with a section definition which looks like this will trigger the assert. .bss 0x1001 : { __bss_start = .; *(.bss*) . = ALIGN(8); __bss_end = .; } >.sdram I don't have a really strong opinion here. I think the assertion above is ok. What we could also do is .bss : { . = ALIGN(8); __bss_start = .; *(.bss*) . = ALIGN(8); __bss_end = .; } >.sdram which would also fix the __bss_start alignment regardless of the .bss output address. For example with the linker entry below __bss_start will end up at 0x1008 .bss 0x1001 : { . = ALIGN(8); __bss_start = .; *(.bss*) . = ALIGN(8); __bss_end = .; } >.sdram As you already mentioned, the bss_end - bss_start % 8 == 0 can be fixed by using memset. So since I plan to continue working on this to get RO, RW^x etc mappings I think I'll just go for the assertion for now -- but test the .bss address instead of the config option. Anyone objects? Thanks /Ilias > > > r~