[PATCH 1/5] ARM: Fix global_variable_offset() for aarch64

[PATCH 1/5] ARM: Fix global_variable_offset() for aarch64

[Sascha Hauer]

Not all toolchains use pc relative addresses for global variables.
Apparently the gcc 8.3.0 YOCTO toolchain uses absolute addresses.
This means can't simply return 0 for global_variable_offset() but
instead have to calculate the offset between the compile addresses
for global variables and their runtime address.
We do this by getting the address of a global variable pc relative
explicitely in assembly and substracting that address from the
location the C compiler thinks they are.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/include/asm/barebox-arm.h | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index 9840482b23..8b2ecd9ab2 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -31,6 +31,7 @@
 #include <linux/types.h>
 #include <linux/compiler.h>
 #include <asm/barebox-arm-head.h>
+#include <asm/sections.h>
 
 /*
  * We have a 4GiB address space split into 1MiB sections, with each
@@ -43,15 +44,22 @@ unsigned long get_runtime_offset(void);
 /* global_variable_offset() - Access global variables when not running at link address
  *
  * Get the offset of global variables when not running at the address we are
- * linked at. ARM uses absolute addresses, so we must add the runtime offset
- * whereas aarch64 uses PC relative addresses, so nothing must be done here.
+ * linked at.
  */
 static inline unsigned long global_variable_offset(void)
 {
-	if (IS_ENABLED(CONFIG_CPU_32))
-		return get_runtime_offset();
-	else
-		return 0;
+#ifdef CONFIG_CPU_V8
+	unsigned long text;
+
+	__asm__ __volatile__(
+		"adr    %0, _text\n"
+		: "=r" (text)
+		:
+		: "memory");
+	return text - (unsigned long)_text;
+#else
+	return get_runtime_offset();
+#endif
 }
 
 void setup_c(void);
-- 
2.20.1


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

[PATCH 2/5] ARM: nxp-imx8mq-evk: Do setup_c() before accessing global variables

[Sascha Hauer]

The lowlevel board code accesses global variables, so call
relocate_to_current_adr() / setup_c() to make sure we have a valid C
environment.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boards/nxp-imx8mq-evk/lowlevel.c | 54 +++++++++++++----------
 1 file changed, 31 insertions(+), 23 deletions(-)

diff --git a/arch/arm/boards/nxp-imx8mq-evk/lowlevel.c b/arch/arm/boards/nxp-imx8mq-evk/lowlevel.c
index 6451e5d414..e3fdc64edf 100644
--- a/arch/arm/boards/nxp-imx8mq-evk/lowlevel.c
+++ b/arch/arm/boards/nxp-imx8mq-evk/lowlevel.c
@@ -67,30 +67,8 @@ static void nxp_imx8mq_evk_sram_setup(void)
 	BUG_ON(ret);
 }
 
-/*
- * Power-on execution flow of start_nxp_imx8mq_evk() might not be
- * obvious for a very first read, so here's, hopefully helpful,
- * summary:
- *
- * 1. MaskROM uploads PBL into OCRAM and that's where this function is
- *    executed for the first time
- *
- * 2. DDR is initialized and full i.MX image is loaded to the
- *    beginning of RAM
- *
- * 3. start_nxp_imx8mq_evk, now in RAM, is executed again
- *
- * 4. BL31 blob is uploaded to OCRAM and the control is transfer to it
- *
- * 5. BL31 exits EL3 into EL2 at address MX8MQ_ATF_BL33_BASE_ADDR,
- *    executing start_nxp_imx8mq_evk() the third time
- *
- * 6. Standard barebox boot flow continues
- */
-ENTRY_FUNCTION(start_nxp_imx8mq_evk, r0, r1, r2)
+static __noreturn noinline void nxp_imx8mq_evk_start(void)
 {
-	imx8mq_cpu_lowlevel_init();
-
 	if (IS_ENABLED(CONFIG_DEBUG_LL))
 		setup_uart();
 
@@ -125,3 +103,33 @@ ENTRY_FUNCTION(start_nxp_imx8mq_evk, r0, r1, r2)
 	imx8mq_barebox_entry(__dtb_imx8mq_evk_start);
 }
 
+/*
+ * Power-on execution flow of start_nxp_imx8mq_evk() might not be
+ * obvious for a very first read, so here's, hopefully helpful,
+ * summary:
+ *
+ * 1. MaskROM uploads PBL into OCRAM and that's where this function is
+ *    executed for the first time
+ *
+ * 2. DDR is initialized and full i.MX image is loaded to the
+ *    beginning of RAM
+ *
+ * 3. start_nxp_imx8mq_evk, now in RAM, is executed again
+ *
+ * 4. BL31 blob is uploaded to OCRAM and the control is transfer to it
+ *
+ * 5. BL31 exits EL3 into EL2 at address MX8MQ_ATF_BL33_BASE_ADDR,
+ *    executing start_nxp_imx8mq_evk() the third time
+ *
+ * 6. Standard barebox boot flow continues
+ */
+ENTRY_FUNCTION(start_nxp_imx8mq_evk, r0, r1, r2)
+{
+	imx8mq_cpu_lowlevel_init();
+
+	relocate_to_current_adr();
+	setup_c();
+
+	nxp_imx8mq_evk_start();
+}
+
-- 
2.20.1


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

[PATCH 3/5] ARM: phyCORE-i.MX8M SOM: Do setup_c() before accessing global variables

[Sascha Hauer]

The lowlevel board code accesses global variables, so call
relocate_to_current_adr() / setup_c() to make sure we have a valid C
environment.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boards/phytec-som-imx8mq/lowlevel.c | 53 +++++++++++---------
 1 file changed, 30 insertions(+), 23 deletions(-)

diff --git a/arch/arm/boards/phytec-som-imx8mq/lowlevel.c b/arch/arm/boards/phytec-som-imx8mq/lowlevel.c
index e42e7a6fcc..4e52b92ad3 100644
--- a/arch/arm/boards/phytec-som-imx8mq/lowlevel.c
+++ b/arch/arm/boards/phytec-som-imx8mq/lowlevel.c
@@ -61,30 +61,8 @@ static void phytec_imx8mq_som_sram_setup(void)
 	BUG_ON(ret);
 }
 
-/*
- * Power-on execution flow of start_phytec_phycore_imx8mq() might not be
- * obvious for a very first read, so here's, hopefully helpful,
- * summary:
- *
- * 1. MaskROM uploads PBL into OCRAM and that's where this function is
- *    executed for the first time
- *
- * 2. DDR is initialized and full i.MX image is loaded to the
- *    beginning of RAM
- *
- * 3. start_phytec_phycore_imx8mq, now in RAM, is executed again
- *
- * 4. BL31 blob is uploaded to OCRAM and the control is transfer to it
- *
- * 5. BL31 exits EL3 into EL2 at address MX8MQ_ATF_BL33_BASE_ADDR,
- *    executing start_phytec_phycore_imx8mq() the third time
- *
- * 6. Standard barebox boot flow continues
- */
-ENTRY_FUNCTION(start_phytec_phycore_imx8mq, r0, r1, r2)
+static __noreturn noinline void phytec_phycore_imx8mq_start(void)
 {
-	imx8mq_cpu_lowlevel_init();
-
 	if (IS_ENABLED(CONFIG_DEBUG_LL))
 		setup_uart();
 
@@ -118,3 +96,32 @@ ENTRY_FUNCTION(start_phytec_phycore_imx8mq, r0, r1, r2)
 	 */
 	imx8mq_barebox_entry(__dtb_imx8mq_phytec_phycore_som_start);
 }
+
+/*
+ * Power-on execution flow of start_phytec_phycore_imx8mq() might not be
+ * obvious for a very first read, so here's, hopefully helpful,
+ * summary:
+ *
+ * 1. MaskROM uploads PBL into OCRAM and that's where this function is
+ *    executed for the first time
+ *
+ * 2. DDR is initialized and full i.MX image is loaded to the
+ *    beginning of RAM
+ *
+ * 3. start_phytec_phycore_imx8mq, now in RAM, is executed again
+ *
+ * 4. BL31 blob is uploaded to OCRAM and the control is transfer to it
+ *
+ * 5. BL31 exits EL3 into EL2 at address MX8MQ_ATF_BL33_BASE_ADDR,
+ *    executing start_phytec_phycore_imx8mq() the third time
+ *
+ * 6. Standard barebox boot flow continues
+ */
+ENTRY_FUNCTION(start_phytec_phycore_imx8mq, r0, r1, r2)
+{
+	imx8mq_cpu_lowlevel_init();
+	relocate_to_current_adr();
+	setup_c();
+
+	phytec_phycore_imx8mq_start();
+}
-- 
2.20.1


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

[PATCH 4/5] ARM: zii-imx8mq-dev: Do setup_c() before accessing global variables

[Sascha Hauer]

The lowlevel board code accesses global variables, so call
relocate_to_current_adr() / setup_c() to make sure we have a valid C
environment.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boards/zii-imx8mq-dev/lowlevel.c | 59 +++++++++++++----------
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/arch/arm/boards/zii-imx8mq-dev/lowlevel.c b/arch/arm/boards/zii-imx8mq-dev/lowlevel.c
index 33c007e05d..7eb8b684d2 100644
--- a/arch/arm/boards/zii-imx8mq-dev/lowlevel.c
+++ b/arch/arm/boards/zii-imx8mq-dev/lowlevel.c
@@ -120,36 +120,11 @@ static unsigned int get_system_type(void)
 extern char __dtb_imx8mq_zii_ultra_rmb3_start[];
 extern char __dtb_imx8mq_zii_ultra_zest_start[];
 
-/*
- * Power-on execution flow of start_zii_imx8mq_dev() might not be
- * obvious for a very frist read, so here's, hopefully helpful,
- * summary:
- *
- * 1. MaskROM uploads PBL into OCRAM and that's where this function is
- *    executed for the first time
- *
- * 2. DDR is initialized and full i.MX image is loaded to the
- *    beginning of RAM
- *
- * 3. start_nxp_imx8mq_evk, now in RAM, is executed again
- *
- * 4. BL31 blob is uploaded to OCRAM and the control is transfer to it
- *
- * 5. BL31 exits EL3 into EL2 at address MX8MQ_ATF_BL33_BASE_ADDR,
- *    executing start_nxp_imx8mq_evk() the third time
- *
- * 6. Standard barebox boot flow continues
- */
-ENTRY_FUNCTION(start_zii_imx8mq_dev, r0, r1, r2)
+static __noreturn noinline void zii_imx8mq_dev_start(void)
 {
 	unsigned int system_type;
 	void *fdt;
 
-	imx8mq_cpu_lowlevel_init();
-
-	if (IS_ENABLED(CONFIG_DEBUG_LL))
-		setup_uart();
-
 	if (get_pc() < MX8MQ_DDR_CSD1_BASE_ADDR) {
 		/*
 		 * We assume that we were just loaded by MaskROM into
@@ -202,3 +177,35 @@ ENTRY_FUNCTION(start_zii_imx8mq_dev, r0, r1, r2)
 	 */
 	imx8mq_barebox_entry(fdt);
 }
+
+/*
+ * Power-on execution flow of start_zii_imx8mq_dev() might not be
+ * obvious for a very frist read, so here's, hopefully helpful,
+ * summary:
+ *
+ * 1. MaskROM uploads PBL into OCRAM and that's where this function is
+ *    executed for the first time
+ *
+ * 2. DDR is initialized and full i.MX image is loaded to the
+ *    beginning of RAM
+ *
+ * 3. start_nxp_imx8mq_evk, now in RAM, is executed again
+ *
+ * 4. BL31 blob is uploaded to OCRAM and the control is transfer to it
+ *
+ * 5. BL31 exits EL3 into EL2 at address MX8MQ_ATF_BL33_BASE_ADDR,
+ *    executing start_nxp_imx8mq_evk() the third time
+ *
+ * 6. Standard barebox boot flow continues
+ */
+ENTRY_FUNCTION(start_zii_imx8mq_dev, r0, r1, r2)
+{
+	imx8mq_cpu_lowlevel_init();
+	relocate_to_current_adr();
+	setup_c();
+	
+	if (IS_ENABLED(CONFIG_DEBUG_LL))
+		setup_uart();
+
+	zii_imx8mq_dev_start();
+}
-- 
2.20.1


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

[PATCH 5/5] ARM: access __boot_cpu_mode with a function

[Sascha Hauer]

__boot_cpu_mode is accessed from barebox_multi_pbl_start() and
barebox_single_pbl_start(). These functions may be called at an
address different from the address they are linked at. Calculating
the address of global variables can yield wrong results when it
is done before setup_c() is called. We can't make sure when the
address is calculated, OSELAS.Toolchain-2018.12.0 indeed calculates
the address after setup_c() is called, but Debian
arm-linux-gnueabihf-gcc 8.3.0 does it before setup_c() is called
and thus doesn't work.
This is solved by accessing __boot_cpu_mode with a wrapper function
which we call explicitly after setup_c() is done.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/cpu/common.c         | 8 ++++++++
 arch/arm/cpu/sm.c             | 4 ++--
 arch/arm/cpu/uncompress.c     | 2 +-
 arch/arm/include/asm/secure.h | 3 ++-
 4 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/arm/cpu/common.c b/arch/arm/cpu/common.c
index 4a2b6734db..4d957da1dc 100644
--- a/arch/arm/cpu/common.c
+++ b/arch/arm/cpu/common.c
@@ -23,6 +23,7 @@
 #include <asm/barebox-arm.h>
 #include <asm/barebox-arm-head.h>
 #include <asm-generic/memory_layout.h>
+#include <asm/secure.h>
 #include <asm/sections.h>
 #include <asm/cache.h>
 #include <debug_ll.h>
@@ -147,3 +148,10 @@ int __pure cpu_architecture(void)
 	return __cpu_architecture;
 }
 #endif
+
+extern int __boot_cpu_mode;
+
+int boot_cpu_mode(void)
+{
+	return __boot_cpu_mode;
+}
diff --git a/arch/arm/cpu/sm.c b/arch/arm/cpu/sm.c
index b7a9eae89b..1f2c236d5a 100644
--- a/arch/arm/cpu/sm.c
+++ b/arch/arm/cpu/sm.c
@@ -157,7 +157,7 @@ int armv7_secure_monitor_install(void)
 		return -EINVAL;
 	}
 
-	if (__boot_cpu_mode == HYP_MODE)
+	if (boot_cpu_mode() == HYP_MODE)
 		return 0;
 
 	mmuon = get_cr() & CR_M;
@@ -235,7 +235,7 @@ static int sm_init(void)
 				  bootm_secure_state_names,
 				  ARRAY_SIZE(bootm_secure_state_names));
 
-	if (__boot_cpu_mode == HYP_MODE)
+	if (boot_cpu_mode() == HYP_MODE)
 		bootm_secure_state = ARM_STATE_HYP;
 
 	return 0;
diff --git a/arch/arm/cpu/uncompress.c b/arch/arm/cpu/uncompress.c
index c7851c5c75..4f16af22f8 100644
--- a/arch/arm/cpu/uncompress.c
+++ b/arch/arm/cpu/uncompress.c
@@ -103,7 +103,7 @@ void __noreturn barebox_multi_pbl_start(unsigned long membase,
 
 	pr_debug("jumping to uncompressed image at 0x%p\n", barebox);
 
-	if (IS_ENABLED(CONFIG_CPU_V7) && __boot_cpu_mode == HYP_MODE)
+	if (IS_ENABLED(CONFIG_CPU_V7) && boot_cpu_mode() == HYP_MODE)
 		armv7_switch_to_hyp();
 
 	barebox(membase, memsize, boarddata);
diff --git a/arch/arm/include/asm/secure.h b/arch/arm/include/asm/secure.h
index 663d81ea27..e0c2623723 100644
--- a/arch/arm/include/asm/secure.h
+++ b/arch/arm/include/asm/secure.h
@@ -10,7 +10,8 @@ void armv7_switch_to_hyp(void);
 void armv7_hyp_install(void);
 
 extern unsigned char secure_monitor_init_vectors[];
-extern int __boot_cpu_mode;
+
+int boot_cpu_mode(void);
 
 enum arm_security_state {
 	ARM_STATE_SECURE,
-- 
2.20.1


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

Re: [PATCH 1/5] ARM: Fix global_variable_offset() for aarch64

[Andrey Smirnov]

On Tue, Jul 16, 2019 at 6:50 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> Not all toolchains use pc relative addresses for global variables.
> Apparently the gcc 8.3.0 YOCTO toolchain uses absolute addresses.

Just out of curiosity, can this behavior be changed with -mcmodel=tiny
or maybe -mpc-relative-literal-loads ?

Thanks,
Andrey Smirnov

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

Re: [PATCH 1/5] ARM: Fix global_variable_offset() for aarch64

[Sascha Hauer]

On Tue, Jul 16, 2019 at 06:28:17PM -0700, Andrey Smirnov wrote:
> On Tue, Jul 16, 2019 at 6:50 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > Not all toolchains use pc relative addresses for global variables.
> > Apparently the gcc 8.3.0 YOCTO toolchain uses absolute addresses.
> 
> Just out of curiosity, can this behavior be changed with -mcmodel=tiny
> or maybe -mpc-relative-literal-loads ?

This doesn't seem to have any influence with one toolchain or the other.

BTW with this diff:

diff --git a/arch/arm/cpu/common.c b/arch/arm/cpu/common.c
index 4d957da1dc..65cf90f553 100644
--- a/arch/arm/cpu/common.c
+++ b/arch/arm/cpu/common.c
@@ -60,6 +60,14 @@ void pbl_barebox_break(void)
 	);
 }
 
+static noinline void _puthex_ll(unsigned long val)
+{
+	putc_ll('a');
+	putc_ll(':');
+	puthex_ll(val);
+	putc_ll('\n');
+}
+
 /*
  * relocate binary to the currently running address
  */
@@ -69,6 +77,8 @@ void relocate_to_current_adr(void)
 	unsigned long __maybe_unused *dynsym, *dynend;
 	void *dstart, *dend;
 
+	_puthex_ll((unsigned long)__rel_dyn_start);
+
 	/* Get offset between linked address and runtime address */
 	offset = get_runtime_offset();
 	offset_var = global_variable_offset();

This is what the two toolchains make from it (stripped some irrelevant instructions):

YOCTO:

0000000000003c38 <relocate_to_current_adr>:
    3c3c:       b0000100        adrp    x0, 24000 <__dtb_imx8mq_evk_start+0x5c40>
    3c48:       f943e013        ldr     x19, [x0, #1984]
    3c4c:       aa1303e0        mov     x0, x19
    3c50:       97ffffd1        bl      3b94 <_puthex_ll>

OSELAS:

0000000000003c20 <relocate_to_current_adr>:
    3c24:       b0000100        adrp    x0, 24000 <__dtb_imx8mq_evk_start+0x5c60>
    3c30:       911ee013        add     x19, x0, #0x7b8
    3c34:       aa1303e0        mov     x0, x19
    3c38:       97ffffd1        bl      3b7c <_puthex_ll>


Sascha

-- 
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