+Marek Vasut who originally wrote it Hi Heinrich,
On Sun, 29 Nov 2020 at 23:20, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > Am 30. November 2020 02:53:36 MEZ schrieb Simon Glass <s...@chromium.org>: > >The linker script uses alphabetic sorting to group the different linker > >lists together. Each group has its own struct and potentially its own > >alignment. But when the linker packs the structs together it cannot > >ensure that a linker list starts on the expected alignment boundary. > > > >For example, if the first list has a struct size of 8 and we place 3 of > >them in the image, that means that the next struct will start at offset > >0x18 from the start of the linker_list section. If the next struct has > >a size of 16 then it will start at an 8-byte aligned offset, but not a > >16-byte aligned offset. > > > >With sandbox on x86_64, a reference to a linker list item using > >ll_entry_get() can force alignment of that particular linker_list item, > >if it is in the same file as the linker_list item is declared. > > > >Consider this example, where struct driver is 0x80 bytes: > > > > ll_entry_declare(struct driver, fred, driver) > > > >... > > > > void *p = ll_entry_get(struct driver, fred, driver) > > > >If these two lines of code are in the same file, then the entry is > >forced > >to be aligned at the 'struct driver' alignment, which is 16 bytes. If > >the > >second line of code is in a different file, then no action is taken, > >since > >the compiler cannot update the alignment of the linker_list item. > > > >In the first case, an 8-byte 'fill' region is added: > > > > .u_boot_list_2_driver_2_testbus_drv > > 0x0000000000270018 0x80 test/built-in.o > > 0x0000000000270018 > > _u_boot_list_2_driver_2_testbus_drv > > .u_boot_list_2_driver_2_testfdt1_drv > > 0x0000000000270098 0x80 test/built-in.o > > 0x0000000000270098 > > _u_boot_list_2_driver_2_testfdt1_drv > > *fill* 0x0000000000270118 0x8 > > .u_boot_list_2_driver_2_testfdt_drv > > 0x0000000000270120 0x80 test/built-in.o > > 0x0000000000270120 > > _u_boot_list_2_driver_2_testfdt_drv > > .u_boot_list_2_driver_2_testprobe_drv > > 0x00000000002701a0 0x80 test/built-in.o > > 0x00000000002701a0 > > _u_boot_list_2_driver_2_testprobe_drv > > > >With this, the linker_list no-longer works since items after > >testfdt1_drv > >are not at the expected address. > > > >Ideally we would have a way to tell gcc not to align structs in this > >way. > >It is not clear how we could do this, and in any case it would require > >us > >to adjust every struct used by the linker_list feature. > > > >One possible fix is to force each separate linker_list to start on the > >largest possible boundary that can be required by the compiler. However > >that does not seem to work on x86_64, which uses 16-byte alignment in > >this > >case but needs 32-byte alignment. > > > >So add a Kconfig option to handle this. Set the default value to 4 so > >as to avoid changing platforms that don't need it. > > > >Update the ll_entry_start() accordingly. > > > >Signed-off-by: Simon Glass <s...@chromium.org> > >--- > > > > arch/Kconfig | 11 +++++++ > > doc/api/linker_lists.rst | 62 ++++++++++++++++++++++++++++++++++++++++ > > include/linker_lists.h | 3 +- > > 3 files changed, 75 insertions(+), 1 deletion(-) > > > >diff --git a/arch/Kconfig b/arch/Kconfig > >index 3aa99e08fce..aa8664212f1 100644 > >--- a/arch/Kconfig > >+++ b/arch/Kconfig > >@@ -7,6 +7,17 @@ config HAVE_ARCH_IOREMAP > > config NEEDS_MANUAL_RELOC > > bool > > > >+config LINKER_LIST_ALIGN > >+ int > >+ default 32 if SANDBOX > > What is so special about the sandbox? I'm not too sure, actually. Also, 32 seems to be larger than __BIGGEST_ALIGNMENT__ so it is confusing. > Just evaluate if the host is 64 bit and use 8 or 4 accordingly? > > >+ default 8 if ARM64 || X86 > > Shouldn't the default be 8 on all 64 bit platforms? And 4 on all 32 bit > platforms? Possibly, but who knows? One way to really get to the bottom of this is to have a test that checks that the alignment is what it should be. I spent half a day diagnosing this but not that much time thinking of the best solution. If you have time to dig into it please let me know. Regards, Simon
