[PATCH v4] firmware: remove FW_TEXT_START

[Jessica Clarke]

On 14 May 2024, at 18:25, Cheng Yang <yangcheng.work at foxmail.com> wrote:
> 
> 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
> >
> 
> Hi Jess,
> 
> Thanks for your tip, it can indeed solve the problem of gdb debugging. 
> But I use vscode to start gdb for opensbi debugging. It reads symbols 
> from the elf file in the specified "program" option by default, so I 
> still need some other settings. I still recommend keeping FW_TEXT_START 
> instead of removing it.

“My editor of choice is deficient” isn’t suitable justification for
imposing complexity like this on everyone IMO. But regardless, Visual
Studio Code has the ability to run custom commands on GDB startup[1].

Jess

[1] https://code.visualstudio.com/docs/cpp/launch-json-reference#_setupcommands