On Sat, 3 Apr 2021 09:03:32 +0200 Ahmad Fatoum <ahmad@xxxxxx> wrote: Hi Ahmad! I have just tested this patch on erizo FPGA. Barebox work fine. -- Best regards, Antony Pavlov > Like on ARM, the PBL common code does not relocate the piggy data and > instead keeps pointers into the old image's location. > > For relocate_to_current_adr, this doesn't matter, because both > destination and source address are the same, but when running from ROM > or flash relocate_to_adr will only copy up to __bss_start, so > determining the piggy data bounds _must_ happen before relocation. > > We did so, but input_data_len, which references the image end was > executed after relocation unearthing two bugs at once: > > - The compiler cached input_data_end from before relocation, so > the runtime offset wasn't being added > > - Even with runtime offset added, we were pointing at the new > address, but the old piggy data wasn't copied there (On erizo, > it's still in ROM) > > Fix these by evaluating the bounds before relocation and add a comment, > why it needs to be there. The same comment will also be added at the > code's origin in the ARM architecture support. > > Reported-by: Antony Pavlov <antonynpavlov@xxxxxxxxx> > Signed-off-by: Ahmad Fatoum <ahmad@xxxxxx> > --- > Please apply to master. > --- > arch/riscv/boot/uncompress.c | 6 +++--- > arch/riscv/include/asm/sections.h | 4 +++- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/riscv/boot/uncompress.c b/arch/riscv/boot/uncompress.c > index 411cefb0e31b..b4e010998a4a 100644 > --- a/arch/riscv/boot/uncompress.c > +++ b/arch/riscv/boot/uncompress.c > @@ -32,8 +32,11 @@ void __noreturn barebox_pbl_start(unsigned long membase, unsigned long memsize, > void *pg_start, *pg_end; > unsigned long pc = get_pc(); > > + /* piggy data is not relocated, so determine the bounds now */ > pg_start = input_data + get_runtime_offset(); > pg_end = input_data_end + get_runtime_offset(); > + pg_len = pg_end - pg_start; > + uncompressed_len = input_data_len(); > > /* > * If we run from inside the memory just relocate the binary > @@ -45,9 +48,6 @@ void __noreturn barebox_pbl_start(unsigned long membase, unsigned long memsize, > else > relocate_to_adr(membase); > > - pg_len = pg_end - pg_start; > - 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 725fd8db474e..6673648bcd58 100644 > --- a/arch/riscv/include/asm/sections.h > +++ b/arch/riscv/include/asm/sections.h > @@ -15,9 +15,11 @@ extern char __dynsym_end[]; > extern char input_data[]; > extern char input_data_end[]; > > +unsigned long get_runtime_offset(void); > + > static inline unsigned int input_data_len(void) > { > - return get_unaligned((const u32 *)(input_data_end - 4)); > + return get_unaligned((const u32 *)(input_data_end + get_runtime_offset() - 4)); > } > > #endif > -- > 2.30.0 > _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox