mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v2] ARM: aarch64: Avoid relocations in runtime-offset.S
@ 2019-01-30  1:30 Andrey Smirnov
  2019-02-01  6:40 ` Sascha Hauer
  0 siblings, 1 reply; 2+ messages in thread
From: Andrey Smirnov @ 2019-01-30  1:30 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Since get_runtime_offset() is executed as a part of reloaction logic,
it cannot have code dependend on any kind of
relocation. Unfortunately, current codebase violates this rule and

linkadr:
.quad get_runtime_offset

ends up producing R_AARCH64_RELATIVE relocation that has to be
resolved at runtime. From tiral and error experimentation it seems
that the simplest way to do this is to drop "a" (allocatable)
attribute fom the section directive in runtime-offset.S

With "a" (see first entry):

aarch64-linux-gnu-objdump -R images/start_zii_imx8mq_dev.pbl

images/start_zii_imx8mq_dev.pbl:     file format elf64-littleaarch64

DYNAMIC RELOCATION RECORDS
OFFSET           TYPE              VALUE
00000000000000b0 R_AARCH64_RELATIVE  *ABS*+0x00000000000000a0
0000000000004258 R_AARCH64_RELATIVE  *ABS*+0x0000000000028118
0000000000004260 R_AARCH64_RELATIVE  *ABS*+0x0000000000028128
00000000000042e0 R_AARCH64_RELATIVE  *ABS*
00000000000042e8 R_AARCH64_RELATIVE  *ABS*+0x0000000000028118
00000000000042f0 R_AARCH64_RELATIVE  *ABS*+0x00000000000042c8

Without "a":

 aarch64-linux-gnu-objdump -R images/start_zii_imx8mq_dev.pbl

images/start_zii_imx8mq_dev.pbl:     file format elf64-littleaarch64

DYNAMIC RELOCATION RECORDS
OFFSET           TYPE              VALUE
0000000000004258 R_AARCH64_RELATIVE  *ABS*+0x0000000000028100
0000000000004260 R_AARCH64_RELATIVE  *ABS*+0x0000000000028110
00000000000042e0 R_AARCH64_RELATIVE  *ABS*
00000000000042e8 R_AARCH64_RELATIVE  *ABS*+0x0000000000028100
00000000000042f0 R_AARCH64_RELATIVE  *ABS*+0x00000000000042c8

Note that on recent toolchains (tested on 8.1.1), this problem is
masked by the fact that

.quad get_runtime_offset

will be initialized with link-time value of "get_runtime_offset" in
addition to having a R_AARCH64_RELATIVE relocation.

00000000000000a0 <get_runtime_offset>:
      a0:	10000000 	adr	x0, a0 <get_runtime_offset>
      a4:	58000061 	ldr	x1, b0 <linkadr>
      a8:	eb010000 	subs	x0, x0, x1
      ac:	d65f03c0 	ret

00000000000000b0 <linkadr>:
      b0:	000000a0 	.word	0x000000a0
      b4:	00000000 	.word	0x00000000

_However_, older toolchains (tested on 5.5.0), will only issue a
R_AARCH64_RELATIVE, so memory location will contain only zeroes:

00000000000000a0 <get_runtime_offset>:
      a0:	10000000 	adr	x0, a0 <get_runtime_offset>
      a4:	58000061 	ldr	x1, b0 <linkadr>
      a8:	eb010000 	subs	x0, x0, x1
      ac:	d65f03c0 	ret

00000000000000b0 <linkadr>:
	...

This leads to an very early crash and complete boot failure in the
latter case.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Changes since [v1]:

    - Added a comment explaining the reasoning "a" specifier is
      missing

[v1] http://lists.infradead.org/pipermail/barebox/2019-January/036779.html

 arch/arm/lib64/runtime-offset.S | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/arm/lib64/runtime-offset.S b/arch/arm/lib64/runtime-offset.S
index 177ca6478..6624fdfa1 100644
--- a/arch/arm/lib64/runtime-offset.S
+++ b/arch/arm/lib64/runtime-offset.S
@@ -1,7 +1,19 @@
 #include <linux/linkage.h>
 #include <asm/assembler.h>
 
-.section ".text_bare_init","ax"
+/*
+ * The .section directive below intentionally omits "a", since that
+ * appears to be the simplest way to force assembler to not generate
+ * R_AARCH64_RELATIVE relocation for
+ *
+ * linkadr:
+ *	.quad get_runtime_offset
+ *
+ * statement below. While having that relocating was relatively
+ * harmless with GCC8, builging the code with GCC5 resulted in
+ * "linkaddr" being initialized to 0 causing complete boot breakdown
+ */
+.section ".text_bare_init","x"
 
 /*
  * Get the offset between the link address and the address
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] ARM: aarch64: Avoid relocations in runtime-offset.S
  2019-01-30  1:30 [PATCH v2] ARM: aarch64: Avoid relocations in runtime-offset.S Andrey Smirnov
@ 2019-02-01  6:40 ` Sascha Hauer
  0 siblings, 0 replies; 2+ messages in thread
From: Sascha Hauer @ 2019-02-01  6:40 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox

On Tue, Jan 29, 2019 at 05:30:50PM -0800, Andrey Smirnov wrote:
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---

Applied, thanks

Sascha

> 
> Changes since [v1]:
> 
>     - Added a comment explaining the reasoning "a" specifier is
>       missing
> 
> [v1] http://lists.infradead.org/pipermail/barebox/2019-January/036779.html
> 
>  arch/arm/lib64/runtime-offset.S | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/lib64/runtime-offset.S b/arch/arm/lib64/runtime-offset.S
> index 177ca6478..6624fdfa1 100644
> --- a/arch/arm/lib64/runtime-offset.S
> +++ b/arch/arm/lib64/runtime-offset.S
> @@ -1,7 +1,19 @@
>  #include <linux/linkage.h>
>  #include <asm/assembler.h>
>  
> -.section ".text_bare_init","ax"
> +/*
> + * The .section directive below intentionally omits "a", since that
> + * appears to be the simplest way to force assembler to not generate
> + * R_AARCH64_RELATIVE relocation for
> + *
> + * linkadr:
> + *	.quad get_runtime_offset
> + *
> + * statement below. While having that relocating was relatively
> + * harmless with GCC8, builging the code with GCC5 resulted in
> + * "linkaddr" being initialized to 0 causing complete boot breakdown
> + */
> +.section ".text_bare_init","x"
>  
>  /*
>   * Get the offset between the link address and the address
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-02-01  6:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-30  1:30 [PATCH v2] ARM: aarch64: Avoid relocations in runtime-offset.S Andrey Smirnov
2019-02-01  6:40 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox