diff mbox series

[v3,01/28] linker_lists: Fix alignment issue

Message ID 20201216212001.v3.1.Ie3fa227cbe539fa6742fa1e647093557a2b9b8ee@changeid
State Accepted
Commit 0b2fa98aa5e5dbdac4f5e2b2f67a34cc34dcc6b8
Delegated to: Simon Glass
Headers show
Series dm: Change the way sequence numbers are implemented | expand

Commit Message

Simon Glass Dec. 17, 2020, 4:20 a.m. UTC
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 <sjg@chromium.org>
---

(no changes since v1)

 arch/Kconfig             | 11 ++++++++
 doc/api/linker_lists.rst | 59 ++++++++++++++++++++++++++++++++++++++++
 include/linker_lists.h   |  3 +-
 3 files changed, 72 insertions(+), 1 deletion(-)

Comments

Simon Glass Dec. 19, 2020, 4:40 p.m. UTC | #1
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 <sjg@chromium.org>
---

(no changes since v1)

 arch/Kconfig             | 11 ++++++++
 doc/api/linker_lists.rst | 59 ++++++++++++++++++++++++++++++++++++++++
 include/linker_lists.h   |  3 +-
 3 files changed, 72 insertions(+), 1 deletion(-)

Applied to u-boot-dm/next, thanks!
Heinrich Schuchardt April 15, 2021, 7:39 a.m. UTC | #2
On 17.12.20 05:20, Simon Glass wrote:
> 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 <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  arch/Kconfig             | 11 ++++++++
>  doc/api/linker_lists.rst | 59 ++++++++++++++++++++++++++++++++++++++++
>  include/linker_lists.h   |  3 +-
>  3 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index e8f9a9e1b77..27843cd79c4 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

On which host architecture would the sandbox actually require 32 bytes?

Please, use an alignment based on the bitness of the generated binary to
get relevant test results.

> +	default 8 if ARM64 || X86

Except for jmp_buf the maximum alignment requirement on i386 is 4 and on
amd64 it is 8.

> +	default 4

This defies the alignment requirements of 64 bit systems like RV64, mips64.

The code must work on *all* architectures.

Best regards

Heinrich

> +	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 72f514e0ac0..7063fdc8314 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 d775d041e04..fd98ecd297c 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;						\
>  })
>
Simon Glass April 21, 2021, 7:14 a.m. UTC | #3
Hi Heinrich,

On Thu, 15 Apr 2021 at 19:39, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 17.12.20 05:20, Simon Glass wrote:
> > 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 <sjg@chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >  arch/Kconfig             | 11 ++++++++
> >  doc/api/linker_lists.rst | 59 ++++++++++++++++++++++++++++++++++++++++
> >  include/linker_lists.h   |  3 +-
> >  3 files changed, 72 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index e8f9a9e1b77..27843cd79c4 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
>
> On which host architecture would the sandbox actually require 32 bytes?

x86_64, for example.

>
> Please, use an alignment based on the bitness of the generated binary to
> get relevant test results.
>
> > +     default 8 if ARM64 || X86
>
> Except for jmp_buf the maximum alignment requirement on i386 is 4 and on
> amd64 it is 8.
>
> > +     default 4

Please do check the above commit message. This is a real problem and
causes U-Boot to crash. If you have a better idea how to fix it, I am
all ears, as I don't like this either.

>
> This defies the alignment requirements of 64 bit systems like RV64, mips64.
>
> The code must work on *all* architectures.

The status quo is 4 as you can see, so I am only actually changing
sandbox. I have not seen this problem on other archs, although I
wouldn't rule it out.

Regards,
Simon

>
> > +     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 72f514e0ac0..7063fdc8314 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 d775d041e04..fd98ecd297c 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;                                                \
> >  })
> >
>
diff mbox series

Patch

diff --git a/arch/Kconfig b/arch/Kconfig
index e8f9a9e1b77..27843cd79c4 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 72f514e0ac0..7063fdc8314 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 d775d041e04..fd98ecd297c 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;						\
 })