[PATCH v4] firmware: remove FW_TEXT_START

[Jessica Clarke]

On 14 May 2024, at 17:50, Cheng Yang <yangcheng.work at foxmail.com> wrote:
> 
> >On Tue, May 14, 2024 at 9:58 PM Clément Léger <cleger at rivosinc.com> wrote:
> >>
> >>
> >>
> >> On 10/04/2024 07:18, Anup Patel wrote:
> >> > On Mon, Apr 8, 2024 at 9:01 PM Xiang W <wxjstz at 126.com> wrote:
> >> >>
> >> >> Now opensbi can run at any address via dynamic relocation. We can
> >> >> remove FW_TEXT_START.
> >> >>
> >> >> Signed-off-by: Xiang W <wxjstz at 126.com>
> >> >
> >> > LGTM.
> >> >
> >> > Reviewed-by: Anup Patel <anup at brainfault.org>
> >> > Tested-by: Anup Patel <anup at brainfault.org>
> >> >
> >> > Applied this patch to the riscv/opensbi repo.
> >> >
> >> > Thanks,
> >> > Anup
> >>
> >> Hi Anup,
> >>
> >> This patch seems to break spike support. The newly created ELF is not
> >> marked as EXEC anymore but DYNAMIC and it fails with the following error:
> >>
> >> spike: ../fesvr/elfloader.cc:45:
> >> std::map<std::__cxx11::basic_string<char>, long unsigned int>
> >> load_elf(const char*, memif_t*, reg_t*, unsigned int): Assertion
> >> `IS_ELF_EXEC(*eh64)' failed.
> >>
> >> Should we fixed OpenSBI or allow spike to load a DYNAMIC elf ?
> >
> >I think it is better to use FW_DYNAMIC in Spike because various
> >other projects (such as U-Boot, QEMU, etc) use it.
> >
> >Also, Spike should support loading M-mode firmware in BIN
> >(flat binary) format as well.
> >
> >Regards,
> >Anup
> >
> >>
> >> Thanks,
> >>
> >> Clément
> >>
> 
> Hi Anup,
> 
> I think this patch is not that necessary, and after applying 
> this patch, we lost an elf file that can disassemble or directly 
> provide symbol addresses to gdb, which caused some difficulties 
> in debugging. In the past, I could pass it in Specify the address 
> of FW_TEXT_START during compilation to generate an ELF file containing 
> accurate symbol addresses, which is useful for debugging OpenSBI.

That’s what symbol-file /path/to/elf -o offset is for.

Jess