On Tue, Mar 20, 2018 at 04:05:36PM -0600, Daniel Kurtz wrote:
> Commit 99492c39f39f ("earlycon: Fix __earlycon_table stride") tried to fix
> __earlycon_table stride by forcing the earlycon_id struct alignment to 32
> and asking the linker to 32-byte align the __earlycon_table symbol. This
> fix was based on commit 07fca0e57fca92 ("tracing: Properly align linker
> defined symbols") which tried a similar fix for the tracing subsystem.
>
> However, this fix doesn't quite work because there is no guarantee that
> gcc will place structures packed into an array format. In fact, gcc 4.9
> chooses to 64-byte align these structs by inserting additional padding
> between the entries because it has no clue that they are supposed to be in
> an array. If we are unlucky, the linker will assign symbol
> "__earlycon_table" to a 32-byte aligned address which does not correspond
> to the 64-byte aligned contents of section "__earlycon_table".
>
> To address this same problem, the fix to the tracing system was
> subsequently re-implemented using a more robust table of pointers approach
> by commits:
> 3d56e331b653 ("tracing: Replace syscall_meta_data struct array with pointer array")
> 654986462939 ("tracepoints: Fix section alignment using pointer array")
> e4a9ea5ee7c8 ("tracing: Replace trace_event struct array with pointer array")
>
> Let's use this same "array of pointers to structs" approach for
> EARLYCON_TABLE.
Wouldn't every use of linker sections with structs have this problem? We
use them for clocks, irqs, timers unless those have been fixed.
>
> Fixes: 99492c39f39f ("earlycon: Fix __earlycon_table stride")
> Signed-off-by: Daniel Kurtz <djkurtz@xxxxxxxxxxxx>
> Suggested-by: Aaron Durbin <adurbin@xxxxxxxxxxxx>
> ---
> Changes since v2:
> * Use __initconst instead of __initdata to avoid h8300 and alpha kbuild errors
>
> Changes since v3:
> * Fixed typos in commit message
>
> Changes since v4:
> * removed Change-Id: from commit message
>
> drivers/of/fdt.c | 7 +++++--
> drivers/tty/serial/earlycon.c | 6 ++++--
> include/asm-generic/vmlinux.lds.h | 2 +-
> include/linux/serial_core.h | 21 ++++++++++++++-------
> 4 files changed, 24 insertions(+), 12 deletions(-)
Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
