On Tue, Dec 14, 2021 at 1:25 PM Jessica Clarke <jrt...@jrtc27.com> wrote: > > The original BBL boot method had the kernel embedded as an opaque blob > that was blindly jumped to, which OpenSBI implemented as fw_payload. > OpenSBI then implemented fw_jump, which allows the payload to be loaded > elsewhere, but still blindly jumps to a fixed address at which the > kernel is to be loaded. Finally, OpenSBI introduced fw_dynamic, which > allows the previous stage to inform it where to jump to, rather than > having to blindly guess like fw_jump, or embed the payload as part of > the build like fw_payload. When used with an opaque binary (i.e. the > output of objcopy -O binary), it matches the behaviour of the previous > methods. However, when used with an ELF, QEMU currently passes on the > ELF's entry point address, which causes a discrepancy compared with all > the other boot methods if that entry point is not the first instruction > in the binary. > > This difference specific to fw_dynamic with an ELF is not apparent when > booting Linux, since its entry point is the first instruction in the > binary. However, FreeBSD has a separate ELF entry point, following the > calling convention used by its bootloader, that differs from the first > instruction in the binary, used for the legacy SBI entry point, and so > the specific combination of QEMU's default fw_dynamic firmware with > booting FreeBSD as an ELF rather than a raw binary does not work. > > Thus, align the behaviour when loading an ELF with the behaviour when > loading a raw binary; namely, use the base address of the loaded kernel > in place of the entry point. > > The uImage code is left as-is in using the U-Boot header's entry point, > since the calling convention for that entry point is the same as the SBI > one and it mirrors what U-Boot will do. > > Signed-off-by: Jessica Clarke <jrt...@jrtc27.com>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com> Alistair > --- > hw/riscv/boot.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > index 519fa455a1..f67264374e 100644 > --- a/hw/riscv/boot.c > +++ b/hw/riscv/boot.c > @@ -151,12 +151,19 @@ target_ulong riscv_load_kernel(const char > *kernel_filename, > target_ulong kernel_start_addr, > symbol_fn_t sym_cb) > { > - uint64_t kernel_entry; > + uint64_t kernel_load_base, kernel_entry; > > + /* > + * NB: Use low address not ELF entry point to ensure that the fw_dynamic > + * behaviour when loading an ELF matches the fw_payload, fw_jump and BBL > + * behaviour, as well as fw_dynamic with a raw binary, all of which jump > to > + * the (expected) load address load address. This allows kernels to have > + * separate SBI and ELF entry points (used by FreeBSD, for example). > + */ > if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, > - &kernel_entry, NULL, NULL, NULL, 0, > + NULL, &kernel_load_base, NULL, NULL, 0, > EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { > - return kernel_entry; > + return kernel_load_base; > } > > if (load_uimage_as(kernel_filename, &kernel_entry, NULL, NULL, > -- > 2.33.1 > >