Re: [PATCH] hw/riscv: Add optional symbol callback ptr to riscv_load_kernel()

[Alistair Francis]

On Tue, Nov 19, 2019 at 12:03 AM Zhuang, Siwei (Data61, Kensington
NSW) <address@hidden> wrote:
>
> This patch adds an optional function pointer, "sym_cb", to
> riscv_load_kernel() which provides the possibility to access the symbol
> table during kernel loading.
>
> The pointer is ignored, if supplied with Image or uImage file.
>
> The Spike board requires the access to locate the HTIF symbols.
>
> Fixes: 0ac24d56c5e7 ("hw/riscv: Split out the boot functions")
> Buglink: https://bugs.launchpad.net/qemu/+bug/1835827
> Signed-off-by: Siwei Zhuang <address@hidden>

Reviewed-by: Alistair Francis <address@hidden>

Alistair

> ---
>  hw/riscv/boot.c         | 7 ++++---
>  hw/riscv/sifive_e.c     | 2 +-
>  hw/riscv/sifive_u.c     | 3 ++-
>  hw/riscv/spike.c        | 6 +++---
>  hw/riscv/virt.c         | 3 ++-
>  include/hw/riscv/boot.h | 3 ++-
>  6 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 7fee98d2f8..027303d2a3 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -114,12 +114,13 @@ target_ulong riscv_load_firmware(const char 
> *firmware_filename,
>      exit(1);
>  }
>
> -target_ulong riscv_load_kernel(const char *kernel_filename)
> +target_ulong riscv_load_kernel(const char *kernel_filename, symbol_fn_t 
> sym_cb)
>  {
>      uint64_t kernel_entry, kernel_high;
>
> -    if (load_elf(kernel_filename, NULL, NULL, NULL,
> -                 &kernel_entry, NULL, &kernel_high, 0, EM_RISCV, 1, 0) > 0) {
> +    if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL,
> +                         &kernel_entry, NULL, &kernel_high, 0,
> +                         EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) {
>          return kernel_entry;
>      }
>
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index 0f9d641a0e..8a6b0348df 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -111,7 +111,7 @@ static void riscv_sifive_e_init(MachineState *machine)
>                            memmap[SIFIVE_E_MROM].base, &address_space_memory);
>
>      if (machine->kernel_filename) {
> -        riscv_load_kernel(machine->kernel_filename);
> +        riscv_load_kernel(machine->kernel_filename, NULL);
>      }
>  }
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 9552abf4dd..0140e95732 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -344,7 +344,8 @@ static void riscv_sifive_u_init(MachineState *machine)
>                                   memmap[SIFIVE_U_DRAM].base);
>
>      if (machine->kernel_filename) {
> -        uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename);
> +        uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename,
> +                                                  NULL);
>
>          if (machine->initrd_filename) {
>              hwaddr start;
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 8bbffbcd0f..8823681783 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -184,7 +184,7 @@ static void spike_board_init(MachineState *machine)
>                                  mask_rom);
>
>      if (machine->kernel_filename) {
> -        riscv_load_kernel(machine->kernel_filename);
> +        riscv_load_kernel(machine->kernel_filename, htif_symbol_callback);
>      }
>
>      /* reset vector */
> @@ -273,7 +273,7 @@ static void spike_v1_10_0_board_init(MachineState 
> *machine)
>                                  mask_rom);
>
>      if (machine->kernel_filename) {
> -        riscv_load_kernel(machine->kernel_filename);
> +        riscv_load_kernel(machine->kernel_filename, htif_symbol_callback);
>      }
>
>      /* reset vector */
> @@ -359,7 +359,7 @@ static void spike_v1_09_1_board_init(MachineState 
> *machine)
>                                  mask_rom);
>
>      if (machine->kernel_filename) {
> -        riscv_load_kernel(machine->kernel_filename);
> +        riscv_load_kernel(machine->kernel_filename, htif_symbol_callback);
>      }
>
>      /* reset vector */
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 23f340df19..65557c837e 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -476,7 +476,8 @@ static void riscv_virt_board_init(MachineState *machine)
>                                   memmap[VIRT_DRAM].base);
>
>      if (machine->kernel_filename) {
> -        uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename);
> +        uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename,
> +                                                  NULL);
>
>          if (machine->initrd_filename) {
>              hwaddr start;
> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> index 66075d0e57..df80051fbc 100644
> --- a/include/hw/riscv/boot.h
> +++ b/include/hw/riscv/boot.h
> @@ -28,7 +28,8 @@ void riscv_find_and_load_firmware(MachineState *machine,
>  char *riscv_find_firmware(const char *firmware_filename);
>  target_ulong riscv_load_firmware(const char *firmware_filename,
>                                   hwaddr firmware_load_addr);
> -target_ulong riscv_load_kernel(const char *kernel_filename);
> +target_ulong riscv_load_kernel(const char *kernel_filename,
> +                               symbol_fn_t sym_cb);
>  hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
>                           uint64_t kernel_entry, hwaddr *start);
>
> --
> 2.24.0
>