LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Daniel Kurtz <djkurtz@chromium.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Guenter Roeck <groeck@chromium.org>,
	adurbin@chromium.org, linux-kernel@vger.kernel.org,
	Frank Rowand <frowand.list@gmail.com>,
	Jiri Slaby <jslaby@suse.com>, Arnd Bergmann <arnd@arndb.de>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE"
	<devicetree@vger.kernel.org>,
	"open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>,
	"open list:GENERIC INCLUDE/ASM HEADER FILES"
	<linux-arch@vger.kernel.org>
Subject: Re: [PATCH v5] earlycon: Use a pointer table to fix __earlycon_table stride
Date: Mon, 26 Mar 2018 17:24:21 -0500	[thread overview]
Message-ID: <20180326222421.bwkdakaofoixp562@rob-hp-laptop> (raw)
In-Reply-To: <20180320220536.20245-1-djkurtz@chromium.org>

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@chromium.org>
> Suggested-by: Aaron Durbin <adurbin@chromium.org>
> ---
> 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@kernel.org>

      reply	other threads:[~2018-03-26 22:24 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-20 22:05 Daniel Kurtz
2018-03-26 22:24 ` Rob Herring [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180326222421.bwkdakaofoixp562@rob-hp-laptop \
    --to=robh@kernel.org \
    --cc=adurbin@chromium.org \
    --cc=arnd@arndb.de \
    --cc=devicetree@vger.kernel.org \
    --cc=djkurtz@chromium.org \
    --cc=frowand.list@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=groeck@chromium.org \
    --cc=jslaby@suse.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --subject='Re: [PATCH v5] earlycon: Use a pointer table to fix __earlycon_table stride' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).