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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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