diff options
| -rw-r--r-- | arch/Kconfig | 11 | ||||
| -rw-r--r-- | doc/api/linker_lists.rst | 59 | ||||
| -rw-r--r-- | include/linker_lists.h | 3 |
3 files changed, 72 insertions, 1 deletions
diff --git a/arch/Kconfig b/arch/Kconfig index e8f9a9e..27843cd 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 + default 8 if ARM64 || X86 + default 4 + help + Force the each linker list to be aligned to this boundary. This + is required if ll_entry_get() is used, since otherwise the linker + may add padding into the table, thus breaking it. + See linker_lists.rst for full details. + choice prompt "Architecture select" default SANDBOX diff --git a/doc/api/linker_lists.rst b/doc/api/linker_lists.rst index 72f514e..7063fdc 100644 --- a/doc/api/linker_lists.rst +++ b/doc/api/linker_lists.rst @@ -96,5 +96,64 @@ defined for the whole list and each sub-list: %u_boot_list_2_drivers_2_pci_3 %u_boot_list_2_drivers_3 +Alignment issues +---------------- + +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. + +The simplest fix seems to be to force each separate linker_list to start +on the largest possible boundary that can be required by the compiler. This +is the purpose of CONFIG_LINKER_LIST_ALIGN + + .. kernel-doc:: include/linker_lists.h :internal: diff --git a/include/linker_lists.h b/include/linker_lists.h index d775d04..fd98ecd 100644 --- a/include/linker_lists.h +++ b/include/linker_lists.h @@ -124,7 +124,8 @@ */ #define ll_entry_start(_type, _list) \ ({ \ - static char start[0] __aligned(4) __attribute__((unused, \ + static char start[0] __aligned(CONFIG_LINKER_LIST_ALIGN) \ + __attribute__((unused, \ section(".u_boot_list_2_"#_list"_1"))); \ (_type *)&start; \ }) |