[PATCH 0/2] console: pbl: correctly handle relocate_to_adr after pbl_set_putc

[PATCH 0/2] console: pbl: correctly handle relocate_to_adr after pbl_set_putc

[Ahmad Fatoum]

pbl_set_putc may be called by a PBL entry point to store the absolute
address of a routine to be used for printing out a character.

If barebox happens to be located outside of the initially known RAM,
it will be relocated into it by means of relocate_to_adr(), but nothing
will take care to update the function pointer stored by pbl_set_putc.

This will usually continue to work until barebox sets up the MMU and
everything not known to be RAM is marked as eXecute Never. After that,
the next PBL console print will trigger an instruction abort.

Fix this by not storing the putc function pointer, but instead an offset
relative to _text.

This fixes the issue of barebox hanging on i.MX8M when located at an address
greater than 4G.


I thought about going further and make pbl_set_putc callable, even
before relocation. The problem here is that runtime_address() may
not be called on global variables defined in the same file for good
reasons described in its documentation. So that's left as future
exercise.

Ahmad Fatoum (2):
  ARM: mmu64: mark barebox text section executable during early init
  console: pbl: correctly handle relocate_to_adr after pbl_set_putc

 arch/arm/cpu/mmu_64.c |  1 +
 pbl/console.c         | 13 ++++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

-- 
2.39.2

[PATCH 1/2] ARM: mmu64: mark barebox text section executable during early init

[Ahmad Fatoum]

barebox on ARM64 is usually relocated to DRAM by the time mmu_early_enable()
is called, but in the future we may want to enable the MMU earlier and thus
we need to ensure that the location barebox is currently running from is not
marked eXecute Never, even if it's outside the initially known RAM bank.

This is the first part of fixing barebox hanging on i.MX8M when located
at an address greater than 4G.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/cpu/mmu_64.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index 63e70963224a..3124f8f3a987 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -289,6 +289,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize)
 	early_remap_range(0, 1UL << (BITS_PER_VA - 1), MAP_UNCACHED);
 	early_remap_range(membase, memsize - OPTEE_SIZE, MAP_CACHED);
 	early_remap_range(membase + memsize - OPTEE_SIZE, OPTEE_SIZE, MAP_FAULT);
+	early_remap_range(PAGE_ALIGN_DOWN((uintptr_t)_stext), PAGE_ALIGN(_etext - _stext), MAP_CACHED);
 
 	mmu_enable();
 }
-- 
2.39.2

[PATCH 2/2] console: pbl: correctly handle relocate_to_adr after pbl_set_putc

[Ahmad Fatoum]

pbl_set_putc may be called by a PBL entry point to store the absolute
address of a routine to be used for printing out a character.

If barebox happens to be located outside of the initially known RAM,
it will be relocated into it by means of relocate_to_adr(), but nothing
will take care to update the function pointer stored by pbl_set_putc.

This will usually continue to work until barebox sets up the MMU and
everything not known to be RAM is marked as eXecute Never. After that,
the next PBL console print will trigger an instruction abort.

Fix this by not storing the putc function pointer, but instead an offset
relative to _text.

This is the second part of fixing barebox hanging on i.MX8M when located
at an address greater than 4G.

This change has been tested on both i.MX8M (AArch64) and i.MX6 (Thumb2).

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 pbl/console.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/pbl/console.c b/pbl/console.c
index 1a6e839c1559..d81bf580d52f 100644
--- a/pbl/console.c
+++ b/pbl/console.c
@@ -2,13 +2,14 @@
 
 #include <common.h>
 #include <debug_ll.h>
+#include <asm/sections.h>
 #include <linux/err.h>
 
 /*
  * Put these in the data section so that they survive the clearing of the
  * BSS segment.
  */
-static __attribute__ ((section(".data"))) void (*__putc)(void *ctx, int c);
+static __attribute__ ((section(".data"))) ulong putc_offset;
 static __attribute__ ((section(".data"))) void *putc_ctx;
 
 /**
@@ -21,13 +22,19 @@ static __attribute__ ((section(".data"))) void *putc_ctx;
  */
 void pbl_set_putc(void (*putcf)(void *ctx, int c), void *ctx)
 {
-	__putc = putcf;
+	putc_offset = (ulong)putcf - (ulong)_text;
 	putc_ctx = ctx;
 }
 
+static void __putc(void *ctx, int c)
+{
+	void (*putc)(void *, int) = (void *)_text + putc_offset;
+	putc(ctx, c);
+}
+
 void console_putc(unsigned int ch, char c)
 {
-	if (__putc)
+	if (putc_offset)
 		__putc(putc_ctx, c);
 	else
 		putc_ll(c);
-- 
2.39.2

Re: [PATCH 0/2] console: pbl: correctly handle relocate_to_adr after pbl_set_putc

[Sascha Hauer]

On Thu, Sep 07, 2023 at 10:21:24AM +0200, Ahmad Fatoum wrote:
> pbl_set_putc may be called by a PBL entry point to store the absolute
> address of a routine to be used for printing out a character.
> 
> If barebox happens to be located outside of the initially known RAM,
> it will be relocated into it by means of relocate_to_adr(), but nothing
> will take care to update the function pointer stored by pbl_set_putc.
> 
> This will usually continue to work until barebox sets up the MMU and
> everything not known to be RAM is marked as eXecute Never. After that,
> the next PBL console print will trigger an instruction abort.
> 
> Fix this by not storing the putc function pointer, but instead an offset
> relative to _text.
> 
> This fixes the issue of barebox hanging on i.MX8M when located at an address
> greater than 4G.
> 
> 
> I thought about going further and make pbl_set_putc callable, even
> before relocation. The problem here is that runtime_address() may
> not be called on global variables defined in the same file for good
> reasons described in its documentation. So that's left as future
> exercise.
> 
> Ahmad Fatoum (2):
>   ARM: mmu64: mark barebox text section executable during early init
>   console: pbl: correctly handle relocate_to_adr after pbl_set_putc

Applied, thanks

Sascha

> 
>  arch/arm/cpu/mmu_64.c |  1 +
>  pbl/console.c         | 13 ++++++++++---
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> -- 
> 2.39.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |