[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/riscv: Use load address rather than entry point for fw_dy
From: |
Alistair Francis |
Subject: |
Re: [PATCH] hw/riscv: Use load address rather than entry point for fw_dynamic next_addr |
Date: |
Fri, 17 Dec 2021 15:46:32 +1000 |
On Tue, Dec 14, 2021 at 1:25 PM Jessica Clarke <jrtc27@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 <jrtc27@jrtc27.com>
Reviewed-by: Alistair Francis <alistair.francis@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
>
>