[PATCH master 1/3] RISC-V: cpu: request stack memory region

[PATCH master 1/3] RISC-V: cpu: request stack memory region

[Ahmad Fatoum]

Now that the stack base region is determined dynamically,
mem_malloc_resource can no longer reserve the stack space.
Do as ARM does and add a RISC-V specific initcall to reserve
the main thread's stack space.

Reported-by: Antony Pavlov <antonynpavlov@gmail.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Fix for master as otherwise stack could be overwritten at runtime
---
 arch/riscv/cpu/core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/riscv/cpu/core.c b/arch/riscv/cpu/core.c
index bdcd500ed748..982d378eddec 100644
--- a/arch/riscv/cpu/core.c
+++ b/arch/riscv/cpu/core.c
@@ -2,6 +2,9 @@
 /*
  * Copyright (C) 2012 Regents of the University of California
  * Copyright (C) 2017 SiFive
+ * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
+ *
+ * Common RISC-V core initcalls.
  *
  * All RISC-V systems have a timer attached to every hart.  These timers can
  * either be read from the "time" and "timeh" CSRs, and can use the SBI to
@@ -14,8 +17,17 @@
 #include <of.h>
 #include <linux/clk.h>
 #include <linux/err.h>
+#include <memory.h>
+#include <asm-generic/memory_layout.h>
 #include <io.h>
 
+static int riscv_request_stack(void)
+{
+	extern unsigned long riscv_stack_top;
+	return PTR_ERR_OR_ZERO(request_sdram_region("stack", riscv_stack_top - STACK_SIZE, STACK_SIZE));
+}
+coredevice_initcall(riscv_request_stack);
+
 static struct device_d timer_dev;
 
 static int riscv_probe(struct device_d *parent)
-- 
2.29.2


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

[PATCH master 2/3] RISC-V: board-dt-2nd: ensure FDT doesn't overlap with early mem regions

[Ahmad Fatoum]

RISC-V PBL code currently reserves the last 2M of the memory for
firmware and places the stack before that. This serves virt, as qemu
places the FDT here, but negatively impacts normal targets with embedded
device tree as it increases fragmentation.

Add code to the generic DT entry point that cuts of a number of MiB from
the end of RAM, so the PBL arrives at a stack top that doesn't overlap
FDT.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Prequisite for fix in next patch
---
 arch/riscv/boot/board-dt-2nd.c    | 20 ++++++++++++++++++--
 arch/riscv/boot/uncompress.c      |  5 +----
 arch/riscv/include/asm/sections.h |  9 +++++++++
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/boot/board-dt-2nd.c b/arch/riscv/boot/board-dt-2nd.c
index 25fa7d4f2bef..be28ea23cd6d 100644
--- a/arch/riscv/boot/board-dt-2nd.c
+++ b/arch/riscv/boot/board-dt-2nd.c
@@ -1,8 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <common.h>
+#include <asm/sections.h>
+#include <linux/sizes.h>
 #include <debug_ll.h>
 #include <pbl.h>
+#include <fdt.h>
 
 #if __riscv_xlen == 64
 #define IMAGE_LOAD_OFFSET 0x200000 /* Image load offset(2MB) from start of RAM */
@@ -21,8 +24,8 @@
 
 ENTRY_FUNCTION(start_dt_2nd, a0, _fdt, a2)
 {
-	unsigned long membase, memsize;
-	void *fdt = (void *)_fdt;
+	unsigned long membase, memsize, endmem, endfdt, uncompressed_len;
+	struct fdt_header *fdt = (void *)_fdt;
 
 	if (!fdt)
 		hang();
@@ -31,6 +34,19 @@ ENTRY_FUNCTION(start_dt_2nd, a0, _fdt, a2)
 	setup_c();
 
 	fdt_find_mem(fdt, &membase, &memsize);
+	endmem = membase + memsize;
+	endfdt = _fdt + be32_to_cpu(fdt->totalsize);
+
+	/*
+	 * QEMU likes to place the FDT at the end of RAM, where barebox
+	 * would normally extract itself to. Accommodate this by moving
+	 * memory end, so it doesn't overlap FDT
+	 */
+	uncompressed_len = input_data_len() + MAX_BSS_SIZE;
+
+	if (riscv_mem_barebox_image(membase, endmem, uncompressed_len) < endfdt &&
+	    _fdt < riscv_mem_stack_top(membase, endmem))
+		memsize = ALIGN_DOWN(_fdt - membase, SZ_1M);
 
 	barebox_riscv_entry(membase, memsize, fdt);
 }
diff --git a/arch/riscv/boot/uncompress.c b/arch/riscv/boot/uncompress.c
index cf268bece1bf..411cefb0e31b 100644
--- a/arch/riscv/boot/uncompress.c
+++ b/arch/riscv/boot/uncompress.c
@@ -22,9 +22,6 @@
 unsigned long free_mem_ptr;
 unsigned long free_mem_end_ptr;
 
-extern unsigned char input_data[];
-extern unsigned char input_data_end[];
-
 void __noreturn barebox_pbl_start(unsigned long membase, unsigned long memsize,
 				  void *fdt)
 {
@@ -49,7 +46,7 @@ void __noreturn barebox_pbl_start(unsigned long membase, unsigned long memsize,
 		relocate_to_adr(membase);
 
 	pg_len = pg_end - pg_start;
-	uncompressed_len = get_unaligned((const u32 *)(pg_start + pg_len - 4));
+	uncompressed_len = input_data_len();
 
 	barebox_base = riscv_mem_barebox_image(membase, endmem,
 					       uncompressed_len + MAX_BSS_SIZE);
diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
index b5fbba8f165a..725fd8db474e 100644
--- a/arch/riscv/include/asm/sections.h
+++ b/arch/riscv/include/asm/sections.h
@@ -5,12 +5,21 @@
 #ifndef __ASSEMBLY__
 #include <asm-generic/sections.h>
 #include <linux/types.h>
+#include <asm/unaligned.h>
 
 extern char __rel_dyn_start[];
 extern char __rel_dyn_end[];
 extern char __dynsym_start[];
 extern char __dynsym_end[];
 
+extern char input_data[];
+extern char input_data_end[];
+
+static inline unsigned int input_data_len(void)
+{
+	return get_unaligned((const u32 *)(input_data_end - 4));
+}
+
 #endif
 
 #endif /* __ASM_SECTIONS_H */
-- 
2.29.2


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

[PATCH master 3/3] RISC-V: boot: move stack top to very end of memory

[Ahmad Fatoum]

Now that the generic DT entry point used for QEMU virt takes care to
prevent the PBL common code from overwriting the FDT, we can remove
the 2M wasteland after the stack top.
This reduces fragmentation on low memory platforms like erizo.

Reported-by: Antony Pavlov <antonynpavlov@gmail.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Fix for master to reduce memory fragmentation on erizo
---
 arch/riscv/include/asm/barebox-riscv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/barebox-riscv.h b/arch/riscv/include/asm/barebox-riscv.h
index 948a562c5c1f..bb1d15308b48 100644
--- a/arch/riscv/include/asm/barebox-riscv.h
+++ b/arch/riscv/include/asm/barebox-riscv.h
@@ -36,7 +36,7 @@ void *barebox_riscv_boot_dtb(void);
 static inline unsigned long riscv_mem_stack_top(unsigned long membase,
 						unsigned long endmem)
 {
-	return endmem - SZ_2M;
+	return endmem;
 }
 
 static inline unsigned long riscv_mem_stack(unsigned long membase,
-- 
2.29.2


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

Re: [PATCH master 1/3] RISC-V: cpu: request stack memory region

[Sascha Hauer]

On Wed, Mar 24, 2021 at 09:23:02AM +0100, Ahmad Fatoum wrote:
> Now that the stack base region is determined dynamically,
> mem_malloc_resource can no longer reserve the stack space.
> Do as ARM does and add a RISC-V specific initcall to reserve
> the main thread's stack space.
> 
> Reported-by: Antony Pavlov <antonynpavlov@gmail.com>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---

Applied, thanks

Sascha

> Fix for master as otherwise stack could be overwritten at runtime
> ---
>  arch/riscv/cpu/core.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/riscv/cpu/core.c b/arch/riscv/cpu/core.c
> index bdcd500ed748..982d378eddec 100644
> --- a/arch/riscv/cpu/core.c
> +++ b/arch/riscv/cpu/core.c
> @@ -2,6 +2,9 @@
>  /*
>   * Copyright (C) 2012 Regents of the University of California
>   * Copyright (C) 2017 SiFive
> + * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
> + *
> + * Common RISC-V core initcalls.
>   *
>   * All RISC-V systems have a timer attached to every hart.  These timers can
>   * either be read from the "time" and "timeh" CSRs, and can use the SBI to
> @@ -14,8 +17,17 @@
>  #include <of.h>
>  #include <linux/clk.h>
>  #include <linux/err.h>
> +#include <memory.h>
> +#include <asm-generic/memory_layout.h>
>  #include <io.h>
>  
> +static int riscv_request_stack(void)
> +{
> +	extern unsigned long riscv_stack_top;
> +	return PTR_ERR_OR_ZERO(request_sdram_region("stack", riscv_stack_top - STACK_SIZE, STACK_SIZE));
> +}
> +coredevice_initcall(riscv_request_stack);
> +
>  static struct device_d timer_dev;
>  
>  static int riscv_probe(struct device_d *parent)
> -- 
> 2.29.2
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

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

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

Re: [PATCH master 1/3] RISC-V: cpu: request stack memory region

[Antony Pavlov]

On Wed, 24 Mar 2021 09:23:02 +0100
Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

Current RISC-V erizo barebox master succesfully runs on QEMU
but hangs on FPGA.

git bisection shows this:

    barebox$ git bisect good 809e66d4ef
    ...
    barebox$ git bisect bad
    fef19e17f651a7f4b6063a76c506a67cabfe4a69 is the first bad commit
    commit fef19e17f651a7f4b6063a76c506a67cabfe4a69
    Author: Ahmad Fatoum <a.fatoum@pengutronix.de>
    Date:   Wed Mar 24 09:23:02 2021 +0100

        RISC-V: cpu: request stack memory region

I see that request_sdram_region("stack", ...) hangs in input_data_len().

On FPGA I load barebox image at 0x80000000 (erizo RAM start)
not at 0x0 (link address).
The problem is that input_data_end in input_data_len()
contains link-time address, not actual run-time address.

I have added this hack

--- a/arch/riscv/boot/uncompress.c
+++ b/arch/riscv/boot/uncompress.c
@@ -46,7 +46,7 @@ void __noreturn barebox_pbl_start(unsigned long membase, unsigned long memsize,
                relocate_to_adr(membase);
 
        pg_len = pg_end - pg_start;
-       uncompressed_len = input_data_len();
+       uncompressed_len = get_unaligned((const u32 *)(input_data_end - 4 + get_runtime_offset()));
 
        barebox_base = riscv_mem_barebox_image(membase, endmem,
                                               uncompressed_len + MAX_BSS_SIZE);

Alas, this hack does not fix the problem completely.


> Now that the stack base region is determined dynamically,
> mem_malloc_resource can no longer reserve the stack space.
> Do as ARM does and add a RISC-V specific initcall to reserve
> the main thread's stack space.
> 
> Reported-by: Antony Pavlov <antonynpavlov@gmail.com>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> Fix for master as otherwise stack could be overwritten at runtime
> ---
>  arch/riscv/cpu/core.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/riscv/cpu/core.c b/arch/riscv/cpu/core.c
> index bdcd500ed748..982d378eddec 100644
> --- a/arch/riscv/cpu/core.c
> +++ b/arch/riscv/cpu/core.c
> @@ -2,6 +2,9 @@
>  /*
>   * Copyright (C) 2012 Regents of the University of California
>   * Copyright (C) 2017 SiFive
> + * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
> + *
> + * Common RISC-V core initcalls.
>   *
>   * All RISC-V systems have a timer attached to every hart.  These timers can
>   * either be read from the "time" and "timeh" CSRs, and can use the SBI to
> @@ -14,8 +17,17 @@
>  #include <of.h>
>  #include <linux/clk.h>
>  #include <linux/err.h>
> +#include <memory.h>
> +#include <asm-generic/memory_layout.h>
>  #include <io.h>
>  
> +static int riscv_request_stack(void)
> +{
> +	extern unsigned long riscv_stack_top;
> +	return PTR_ERR_OR_ZERO(request_sdram_region("stack", riscv_stack_top - STACK_SIZE, STACK_SIZE));
> +}
> +coredevice_initcall(riscv_request_stack);
> +
>  static struct device_d timer_dev;
>  
>  static int riscv_probe(struct device_d *parent)
> -- 
> 2.29.2
> 


-- 
Best regards,
  Antony Pavlov

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

Re: [PATCH master 1/3] RISC-V: cpu: request stack memory region

[Ahmad Fatoum]

Hello Antony,

On 02.04.21 07:56, Antony Pavlov wrote:
> On Wed, 24 Mar 2021 09:23:02 +0100
> Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> 
> Current RISC-V erizo barebox master succesfully runs on QEMU
> but hangs on FPGA.
> 
> git bisection shows this:
> 
>     barebox$ git bisect good 809e66d4ef
>     ...
>     barebox$ git bisect bad
>     fef19e17f651a7f4b6063a76c506a67cabfe4a69 is the first bad commit
>     commit fef19e17f651a7f4b6063a76c506a67cabfe4a69
>     Author: Ahmad Fatoum <a.fatoum@pengutronix.de>
>     Date:   Wed Mar 24 09:23:02 2021 +0100
> 
>         RISC-V: cpu: request stack memory region
> 
> I see that request_sdram_region("stack", ...) hangs in input_data_len().
> 
> On FPGA I load barebox image at 0x80000000 (erizo RAM start)
> not at 0x0 (link address).
> The problem is that input_data_end in input_data_len()
> contains link-time address, not actual run-time address.

This is strange. Could you print get_runtime_offset() at this location?
It should be zero, because relocate_to_current_adr() had previously run,
but it seems it's not for you? I don't see how compiler reordering could have
occurred given that we don't do LTO and relocate_to_current_adr is thus
a compiler barrier.

Thanks for testing,
Ahmad

> 
> I have added this hack
> 
> --- a/arch/riscv/boot/uncompress.c
> +++ b/arch/riscv/boot/uncompress.c
> @@ -46,7 +46,7 @@ void __noreturn barebox_pbl_start(unsigned long membase, unsigned long memsize,
>                 relocate_to_adr(membase);
>  
>         pg_len = pg_end - pg_start;
> -       uncompressed_len = input_data_len();
> +       uncompressed_len = get_unaligned((const u32 *)(input_data_end - 4 + get_runtime_offset()));
>  
>         barebox_base = riscv_mem_barebox_image(membase, endmem,
>                                                uncompressed_len + MAX_BSS_SIZE);
> 
> Alas, this hack does not fix the problem completely.
> 
> 
>> Now that the stack base region is determined dynamically,
>> mem_malloc_resource can no longer reserve the stack space.
>> Do as ARM does and add a RISC-V specific initcall to reserve
>> the main thread's stack space.
>>
>> Reported-by: Antony Pavlov <antonynpavlov@gmail.com>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> Fix for master as otherwise stack could be overwritten at runtime
>> ---
>>  arch/riscv/cpu/core.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/arch/riscv/cpu/core.c b/arch/riscv/cpu/core.c
>> index bdcd500ed748..982d378eddec 100644
>> --- a/arch/riscv/cpu/core.c
>> +++ b/arch/riscv/cpu/core.c
>> @@ -2,6 +2,9 @@
>>  /*
>>   * Copyright (C) 2012 Regents of the University of California
>>   * Copyright (C) 2017 SiFive
>> + * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
>> + *
>> + * Common RISC-V core initcalls.
>>   *
>>   * All RISC-V systems have a timer attached to every hart.  These timers can
>>   * either be read from the "time" and "timeh" CSRs, and can use the SBI to
>> @@ -14,8 +17,17 @@
>>  #include <of.h>
>>  #include <linux/clk.h>
>>  #include <linux/err.h>
>> +#include <memory.h>
>> +#include <asm-generic/memory_layout.h>
>>  #include <io.h>
>>  
>> +static int riscv_request_stack(void)
>> +{
>> +	extern unsigned long riscv_stack_top;
>> +	return PTR_ERR_OR_ZERO(request_sdram_region("stack", riscv_stack_top - STACK_SIZE, STACK_SIZE));
>> +}
>> +coredevice_initcall(riscv_request_stack);
>> +
>>  static struct device_d timer_dev;
>>  
>>  static int riscv_probe(struct device_d *parent)
>> -- 
>> 2.29.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 |

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

Re: [PATCH master 1/3] RISC-V: cpu: request stack memory region

[Antony Pavlov]

On Fri, 2 Apr 2021 08:34:36 +0200
Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

Hi Ahmad!

> Hello Antony,
> 
> On 02.04.21 07:56, Antony Pavlov wrote:
> > On Wed, 24 Mar 2021 09:23:02 +0100
> > Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> > 
> > Current RISC-V erizo barebox master succesfully runs on QEMU
> > but hangs on FPGA.
> > 
> > git bisection shows this:
> > 
> >     barebox$ git bisect good 809e66d4ef
> >     ...
> >     barebox$ git bisect bad
> >     fef19e17f651a7f4b6063a76c506a67cabfe4a69 is the first bad commit
> >     commit fef19e17f651a7f4b6063a76c506a67cabfe4a69
> >     Author: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >     Date:   Wed Mar 24 09:23:02 2021 +0100
> > 
> >         RISC-V: cpu: request stack memory region
> > 
> > I see that request_sdram_region("stack", ...) hangs in input_data_len().
> > 
> > On FPGA I load barebox image at 0x80000000 (erizo RAM start)
> > not at 0x0 (link address).
> > The problem is that input_data_end in input_data_len()
> > contains link-time address, not actual run-time address.
> 
> This is strange. Could you print get_runtime_offset() at this location?
> It should be zero, because relocate_to_current_adr() had previously run,
> but it seems it's not for you? I don't see how compiler reordering could have
> occurred given that we don't do LTO and relocate_to_current_adr is thus
> a compiler barrier.

I have added debug output:

--- a/arch/riscv/boot/uncompress.c
+++ b/arch/riscv/boot/uncompress.c
@@ -46,6 +46,9 @@ void __noreturn barebox_pbl_start(unsigned long membase, unsigned long memsize,
                relocate_to_adr(membase);
 
        pg_len = pg_end - pg_start;
+       puts_ll("get_runtime_offset()=0x");
+       puthex_ll(get_runtime_offset());
+       puts_ll("\n");
        uncompressed_len = input_data_len();
 
        barebox_base = riscv_mem_barebox_image(membase, endmem,


I see 

  >get_runtime_offset()=0x00000000

on both QEMU and FPGA.


I have experimented with empty_slot on qemu. To reproduce my result please
apply this patch to erizo qemu, reconfigure and rebuild qemu.

diff --git a/default-configs/riscv32-softmmu.mak b/default-configs/riscv32-softmmu.mak
index f9e742120c..5aff9ed690 100644
--- a/default-configs/riscv32-softmmu.mak
+++ b/default-configs/riscv32-softmmu.mak
@@ -2,3 +2,4 @@
 
 CONFIG_SERIAL=y
 CONFIG_VIRTIO=y
+CONFIG_EMPTY_SLOT=y
diff --git a/default-configs/riscv64-softmmu.mak b/default-configs/riscv64-softmmu.mak
index f9e742120c..5aff9ed690 100644
--- a/default-configs/riscv64-softmmu.mak
+++ b/default-configs/riscv64-softmmu.mak
@@ -2,3 +2,4 @@
 
 CONFIG_SERIAL=y
 CONFIG_VIRTIO=y
+CONFIG_EMPTY_SLOT=y
diff --git a/hw/core/empty_slot.c b/hw/core/empty_slot.c
index c1b9c2b104..2358b849e8 100644
--- a/hw/core/empty_slot.c
+++ b/hw/core/empty_slot.c
@@ -14,7 +14,7 @@
 #include "hw/sysbus.h"
 #include "hw/empty_slot.h"
 
-//#define DEBUG_EMPTY_SLOT
+#define DEBUG_EMPTY_SLOT
 
 #ifdef DEBUG_EMPTY_SLOT
 #define DPRINTF(fmt, ...)                                       \
diff --git a/hw/riscv/erizo_board.c b/hw/riscv/erizo_board.c
index 9586c4df2f..c34f94cedc 100644
--- a/hw/riscv/erizo_board.c
+++ b/hw/riscv/erizo_board.c
@@ -17,6 +17,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/boards.h"
+#include "hw/empty_slot.h"
 #include "hw/riscv/riscv_hart.h"
 #include "hw/char/serial.h"
 #include "sysemu/arch_init.h"
@@ -66,6 +67,8 @@ static void erizo_init(MachineState *machine)
     DeviceState *dev = qdev_create(NULL, TYPE_ERIZO);
     ErizoState *s = ERIZO(dev);
 
+    empty_slot_init(0, 0xffffffff);
+
     qdev_init_nofail(dev);
     harts = &s->harts;
 
There are empty_slot read warnings on master:

    barebox$ qemu-system-riscv32 -nographic -M erizo -bios ./images/barebox-erizo-generic.img -serial stdio -monitor none -trace file=/dev/null 
    >get_runtime_offset()=0x00000000
    empty_slot: read from 0000000000022caf
    empty_slot: read from 0000000000022cae
    empty_slot: read from 0000000000022cb0
    empty_slot: read from 0000000000022cb1
    riscv-timer riscv-timer: running at 24000000 Hz
    Switch to console [cs0]

    barebox 2021.03.0-00134-gc4a1684da9-dirty #1 Fri Apr 2 10:33:45 MSK 2021

    Board: generic Erizo SoC board
    riscv-timer riscv-timer: running at 24000000 Hz
    malloc space: 0x804ffd80 -> 0x805ffd7f (size 1 MiB)

    Hit any to stop autoboot:    1
    boot: error 2
    barebox:/ 


There is no empty_slot warning with fef19e17f651^ barebox:

    barebox$ qemu-system-riscv32 -nographic -M erizo -bios ./images/barebox-erizo-generic.img -serial stdio -monitor none -trace file=/dev/null 
    >RISC-V system with no 'timebase-frequency' in DTS
    Switch to console [cs0]

    barebox 2021.03.0-00122-g6107208573 #1 Fri Apr 2 11:09:16 MSK 2021

    Board: generic Erizo SoC board
    RISC-V system with no 'timebase-frequency' in DTS
    malloc space: 0x802ffd80 -> 0x803ffd7f (size 1 MiB)

    Hit any to stop autoboot:    0
    boot: error 2
    barebox:/ 





> Thanks for testing,
> Ahmad
> 
> > 
> > I have added this hack
> > 
> > --- a/arch/riscv/boot/uncompress.c
> > +++ b/arch/riscv/boot/uncompress.c
> > @@ -46,7 +46,7 @@ void __noreturn barebox_pbl_start(unsigned long membase, unsigned long memsize,
> >                 relocate_to_adr(membase);
> >  
> >         pg_len = pg_end - pg_start;
> > -       uncompressed_len = input_data_len();
> > +       uncompressed_len = get_unaligned((const u32 *)(input_data_end - 4 + get_runtime_offset()));
> >  
> >         barebox_base = riscv_mem_barebox_image(membase, endmem,
> >                                                uncompressed_len + MAX_BSS_SIZE);
> > 
> > Alas, this hack does not fix the problem completely.
> > 
> > 
> >> Now that the stack base region is determined dynamically,
> >> mem_malloc_resource can no longer reserve the stack space.
> >> Do as ARM does and add a RISC-V specific initcall to reserve
> >> the main thread's stack space.
> >>
> >> Reported-by: Antony Pavlov <antonynpavlov@gmail.com>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >> Fix for master as otherwise stack could be overwritten at runtime
> >> ---
> >>  arch/riscv/cpu/core.c | 12 ++++++++++++
> >>  1 file changed, 12 insertions(+)
> >>
> >> diff --git a/arch/riscv/cpu/core.c b/arch/riscv/cpu/core.c
> >> index bdcd500ed748..982d378eddec 100644
> >> --- a/arch/riscv/cpu/core.c
> >> +++ b/arch/riscv/cpu/core.c
> >> @@ -2,6 +2,9 @@
> >>  /*
> >>   * Copyright (C) 2012 Regents of the University of California
> >>   * Copyright (C) 2017 SiFive
> >> + * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
> >> + *
> >> + * Common RISC-V core initcalls.
> >>   *
> >>   * All RISC-V systems have a timer attached to every hart.  These timers can
> >>   * either be read from the "time" and "timeh" CSRs, and can use the SBI to
> >> @@ -14,8 +17,17 @@
> >>  #include <of.h>
> >>  #include <linux/clk.h>
> >>  #include <linux/err.h>
> >> +#include <memory.h>
> >> +#include <asm-generic/memory_layout.h>
> >>  #include <io.h>
> >>  
> >> +static int riscv_request_stack(void)
> >> +{
> >> +	extern unsigned long riscv_stack_top;
> >> +	return PTR_ERR_OR_ZERO(request_sdram_region("stack", riscv_stack_top - STACK_SIZE, STACK_SIZE));
> >> +}
> >> +coredevice_initcall(riscv_request_stack);
> >> +
> >>  static struct device_d timer_dev;
> >>  
> >>  static int riscv_probe(struct device_d *parent)
> >> -- 
> >> 2.29.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 |


-- 
Best regards,
  Antony Pavlov

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

Re: [PATCH master 1/3] RISC-V: cpu: request stack memory region

[Ahmad Fatoum]

Hello Antony,

On 02.04.21 10:13, Antony Pavlov wrote:
>> This is strange. Could you print get_runtime_offset() at this location?
>> It should be zero, because relocate_to_current_adr() had previously run,
>> but it seems it's not for you? I don't see how compiler reordering could have
>> occurred given that we don't do LTO and relocate_to_current_adr is thus
>> a compiler barrier.

[ Naïve me thinking there are any guarantees from GCC for self-modifying code ]

> +    empty_slot_init(0, 0xffffffff);

That's tremendously useful. Thanks! I don't quite understand how it works
though. It registers a "fall-back" device for all of the 32-bit address
space?

> There are empty_slot read warnings on master:
> 
>     barebox$ qemu-system-riscv32 -nographic -M erizo -bios ./images/barebox-erizo-generic.img -serial stdio -monitor none -trace file=/dev/null 
>     >get_runtime_offset()=0x00000000
>     empty_slot: read from 0000000000022caf
>     empty_slot: read from 0000000000022cae
>     empty_slot: read from 0000000000022cb0
>     empty_slot: read from 0000000000022cb1
>     riscv-timer riscv-timer: running at 24000000 Hz
>     Switch to console [cs0]
> 
>     barebox 2021.03.0-00134-gc4a1684da9-dirty #1 Fri Apr 2 10:33:45 MSK 2021
> 
>     Board: generic Erizo SoC board
>     riscv-timer riscv-timer: running at 24000000 Hz
>     malloc space: 0x804ffd80 -> 0x805ffd7f (size 1 MiB)
> 
>     Hit any to stop autoboot:    1
>     boot: error 2
>     barebox:/ 
> 
> 
> There is no empty_slot warning with fef19e17f651^ barebox:
> 
>     barebox$ qemu-system-riscv32 -nographic -M erizo -bios ./images/barebox-erizo-generic.img -serial stdio -monitor none -trace file=/dev/null 
>     >RISC-V system with no 'timebase-frequency' in DTS
>     Switch to console [cs0]
> 
>     barebox 2021.03.0-00122-g6107208573 #1 Fri Apr 2 11:09:16 MSK 2021
> 
>     Board: generic Erizo SoC board
>     RISC-V system with no 'timebase-frequency' in DTS
>     malloc space: 0x802ffd80 -> 0x803ffd7f (size 1 MiB)
> 
>     Hit any to stop autoboot:    0
>     boot: error 2
>     barebox:/ 

Thanks. I identified two issues and CC'd you on a patch fixing them.
Breaking erizo has been a blessing to sort out this kind of bugs early :D

Cheers,
Ahmad

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

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

Re: [PATCH master 1/3] RISC-V: cpu: request stack memory region

[Antony Pavlov]

On Sat, 3 Apr 2021 09:09:15 +0200
Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

> Hello Antony,
> 
> On 02.04.21 10:13, Antony Pavlov wrote:
> >> This is strange. Could you print get_runtime_offset() at this location?
> >> It should be zero, because relocate_to_current_adr() had previously run,
> >> but it seems it's not for you? I don't see how compiler reordering could have
> >> occurred given that we don't do LTO and relocate_to_current_adr is thus
> >> a compiler barrier.
> 
> [ Naïve me thinking there are any guarantees from GCC for self-modifying code ]
> 
> > +    empty_slot_init(0, 0xffffffff);
> 
> That's tremendously useful. Thanks! I don't quite understand how it works
> though. It registers a "fall-back" device for all of the 32-bit address
> space?

Yes, it registers such device.

To my great surprise riscv-qemu (at least v2.12) generates no "bus error"
exception on access to unattended address.

I have just compared qemu-2.12 riscv behaviour with mips behaviour.
I have disabled empty_slot in mips_malta code:

--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1014,7 +1014,7 @@ void mips_malta_init(MachineState *machine)
     /* The whole address space decoded by the GT-64120A doesn't generate
        exception when accessing invalid memory. Create an empty slot to
        emulate this feature. */
-    empty_slot_init(0, 0x20000000);
+    //empty_slot_init(0, 0x20000000);
 
     qdev_init_nofail(dev);
 


I have reconfigured qemu malta barebox to use only 16 MB of RAM. 

diff --git a/arch/mips/boards/qemu-malta/lowlevel.S b/arch/mips/boards/qemu-malta/lowlevel.S
index 98821e0426..6cb77b6c3e 100644
--- a/arch/mips/boards/qemu-malta/lowlevel.S
+++ b/arch/mips/boards/qemu-malta/lowlevel.S
@@ -82,4 +82,4 @@ __start:
 	li	t0, GT_LD(0x1bdfffff)
 	sw	t0, GT_PCI0M1HD_OFS(t1)
 
-ENTRY_FUNCTION_END(BOARD_PBL_START, qemu_malta, SZ_256M)
+ENTRY_FUNCTION_END(BOARD_PBL_START, qemu_malta, SZ_16M)
diff --git a/arch/mips/configs/qemu-malta_defconfig b/arch/mips/configs/qemu-malta_defconfig
index b9994f392c..3d33ae4120 100644
--- a/arch/mips/configs/qemu-malta_defconfig
+++ b/arch/mips/configs/qemu-malta_defconfig
@@ -33,7 +33,6 @@ CONFIG_CMD_SHA256SUM=y
 CONFIG_CMD_UNCOMPRESS=y
 CONFIG_CMD_SLEEP=y
 CONFIG_CMD_DHCP=y
-CONFIG_CMD_MIITOOL=y
 CONFIG_CMD_PING=y
 CONFIG_CMD_TFTP=y
 CONFIG_CMD_ECHO_E=y
@@ -59,7 +58,6 @@ CONFIG_NET_NFS=y
 CONFIG_NET_NETCONSOLE=y
 CONFIG_OFDEVICE=y
 CONFIG_OF_BAREBOX_DRIVERS=y
-CONFIG_DRIVER_NET_RTL8139=y
 # CONFIG_SPI is not set
 CONFIG_I2C=y
 CONFIG_I2C_GPIO=y
@@ -71,10 +69,8 @@ CONFIG_DRIVER_CFI=y
 CONFIG_CFI_BUFFER_WRITE=y
 CONFIG_VIDEO=y
 CONFIG_FRAMEBUFFER_CONSOLE=y
-CONFIG_DRIVER_VIDEO_BOCHS_PCI=y
+CONFIG_DRIVER_VIDEO_EDID=y
 CONFIG_GPIO_MALTA_FPGA_I2C=y
-CONFIG_PCI=y
-CONFIG_PCI_DEBUG=y
 CONFIG_FS_CRAMFS=y
 CONFIG_FS_TFTP=y
 CONFIG_FS_FAT=y
diff --git a/arch/mips/dts/qemu-malta.dts b/arch/mips/dts/qemu-malta.dts
index 32e473cac0..69d78061f6 100644
--- a/arch/mips/dts/qemu-malta.dts
+++ b/arch/mips/dts/qemu-malta.dts
@@ -16,7 +16,7 @@
 
 	memory@0 {
 		device_type = "memory";
-		reg = <0x00000000 0x10000000>;
+		reg = <0x00000000 0x01000000>;
 	};
 
 	uart0: serial@180003f8 {


It is easy to see that emulated mips CPU generates bus error exception on access to unattended address
at 0xa2000000 (start RAM + 32M):

 
barebox$ qemu-system-mips-malta-no-empty-slot -nodefaults -M malta -m 16 -serial stdio -monitor null -bios barebox-flash-image


barebox 2021.03.0-00137-g8c6a139124-dirty #2 Sat Apr 3 11:38:55 MSK 2021


Board: qemu malta
malta-fpga-i2c-gpio 1f000b00.gpio@1f000b00.of: probed gpiochip-1 with base 0
cfi_flash 1e000000.flash@1e000000.of: found cfi flash at 0xbe000000, size 4 MiB
i2c-gpio i2c0.of: using pins 0 (SDA) and 1 (SCL)
netconsole: registered as netconsole-1
malloc space: 0xa0b50000 -> 0xa0f4ffff (size 4 MiB)
envfs: no envfs (magic mismatch) - envfs never written?

Hit any to stop autoboot:    2
barebox@qemu malta:/ iomem
0x00000000 - 0xffffffff (size 0x00000000) iomem
  0x180003f8 - 0x180003ff (size 0x00000008) 180003f8.serial@180003f8.of
  0x1e000000 - 0x1e3fffff (size 0x00400000) 1e000000.flash@1e000000.of
  0x1f000900 - 0x1f00093f (size 0x00000040) 1f000900.serial@1f000900.of
  0x1f000b00 - 0x1f000b1f (size 0x00000020) 1f000b00.gpio@1f000b00.of
  0xa0000000 - 0xa0ffffff (size 0x01000000) kseg1_ram0
    0xa0b49000 - 0xa0b4ffff (size 0x00007000) stack
    0xa0b50000 - 0xa0f4ffff (size 0x00400000) malloc space
    0xa0f50000 - 0xa0fa655f (size 0x00056560) barebox
    0xa0fa6560 - 0xa0ff2edf (size 0x0004c980) barebox data
    0xa0ffaee0 - 0xa0ffef33 (size 0x00004054) bss
barebox@qemu malta:/ md 0xa2000000

Ooops, bus error on load or store!

$ 0   : 00000000 00000000 00000000 00000001
$ 4   : 00000100 00000001 00000001 00000030
$ 8   : a0fa2198 00000000 ffffffff 00000010
$12   : 00000000 0000005c 0000002f 2f696e69
$16   : a2000000 00000000 a2000000 00000004
$20   : 00000000 a0b4fc30 a0b4fc80 a0b4fc39
$24   : 00000010 a0f60de4                  
$28   : 00000000 a0b4fc20 a0b4fc80 a0f50930
Hi    : 0000000a
Lo    : 00000000
epc   : a0f50930
ra    : a0f50930
Status: 00000002
Cause : 0000041c
Config: 80008482

### ERROR ### Please RESET the board ###








> > There are empty_slot read warnings on master:
> > 
> >     barebox$ qemu-system-riscv32 -nographic -M erizo -bios ./images/barebox-erizo-generic.img -serial stdio -monitor none -trace file=/dev/null 
> >     >get_runtime_offset()=0x00000000
> >     empty_slot: read from 0000000000022caf
> >     empty_slot: read from 0000000000022cae
> >     empty_slot: read from 0000000000022cb0
> >     empty_slot: read from 0000000000022cb1
> >     riscv-timer riscv-timer: running at 24000000 Hz
> >     Switch to console [cs0]
> > 
> >     barebox 2021.03.0-00134-gc4a1684da9-dirty #1 Fri Apr 2 10:33:45 MSK 2021
> > 
> >     Board: generic Erizo SoC board
> >     riscv-timer riscv-timer: running at 24000000 Hz
> >     malloc space: 0x804ffd80 -> 0x805ffd7f (size 1 MiB)
> > 
> >     Hit any to stop autoboot:    1
> >     boot: error 2
> >     barebox:/ 
> > 
> > 
> > There is no empty_slot warning with fef19e17f651^ barebox:
> > 
> >     barebox$ qemu-system-riscv32 -nographic -M erizo -bios ./images/barebox-erizo-generic.img -serial stdio -monitor none -trace file=/dev/null 
> >     >RISC-V system with no 'timebase-frequency' in DTS
> >     Switch to console [cs0]
> > 
> >     barebox 2021.03.0-00122-g6107208573 #1 Fri Apr 2 11:09:16 MSK 2021
> > 
> >     Board: generic Erizo SoC board
> >     RISC-V system with no 'timebase-frequency' in DTS
> >     malloc space: 0x802ffd80 -> 0x803ffd7f (size 1 MiB)
> > 
> >     Hit any to stop autoboot:    0
> >     boot: error 2
> >     barebox:/ 
> 
> Thanks. I identified two issues and CC'd you on a patch fixing them.
> Breaking erizo has been a blessing to sort out this kind of bugs early :D
> 
> Cheers,
> Ahmad
> 
> -- 
> 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 |


-- 
Best regards,
  Antony Pavlov

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

Re: [PATCH master 1/3] RISC-V: cpu: request stack memory region

[Ahmad Fatoum]

Hi,

On 03.04.21 11:00, Antony Pavlov wrote:
> To my great surprise riscv-qemu (at least v2.12) generates no "bus error"
> exception on access to unattended address.

I'd have expected a bus error as well if the memory area is unpopulated.
With MMU support, we should catch this.
@Rouven, any news on exception support? :^)

Cheers,
Ahmad

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

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