aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--arch/Kconfig11
-rw-r--r--doc/api/linker_lists.rst59
-rw-r--r--include/linker_lists.h3
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; \
})