qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 05/15] hw/riscv/boot.c: introduce riscv_default_firmware_name


From: Alistair Francis
Subject: Re: [PATCH 05/15] hw/riscv/boot.c: introduce riscv_default_firmware_name()
Date: Fri, 23 Dec 2022 13:17:37 +1000

On Thu, Dec 22, 2022 at 4:28 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Some boards are duplicating the 'riscv_find_and_load_firmware' call
> because the 32 and 64 bits images have different names. Create
> a function to handle this detail instead of hardcoding it in the boards.
>
> Ideally we would bake this logic inside riscv_find_and_load_firmware(),
> or even create a riscv_load_default_firmware(), but at this moment we
> cannot infer whether the machine is running 32 or 64 bits without
> accessing RISCVHartArrayState, which in turn can't be accessed via the
> common code from boot.c. In the end we would exchange 'firmware_name'
> for a flag with riscv_is_32bit(), which isn't much better than what we
> already have today.
>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/boot.c         |  9 +++++++++
>  hw/riscv/sifive_u.c     | 11 ++++-------
>  hw/riscv/spike.c        | 14 +++++---------
>  hw/riscv/virt.c         | 10 +++-------
>  include/hw/riscv/boot.h |  1 +
>  5 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 7361d5c0d8..e1a544b1d9 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -75,6 +75,15 @@ target_ulong 
> riscv_calc_kernel_start_addr(RISCVHartArrayState *harts,
>      }
>  }
>
> +const char *riscv_default_firmware_name(RISCVHartArrayState *harts)
> +{
> +    if (riscv_is_32bit(harts)) {
> +        return RISCV32_BIOS_BIN;
> +    }
> +
> +    return RISCV64_BIOS_BIN;
> +}
> +
>  static char *riscv_find_firmware(const char *firmware_filename)
>  {
>      char *filename;
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 9cf66957ab..ddceb750ea 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -533,6 +533,7 @@ static void sifive_u_machine_init(MachineState *machine)
>      MemoryRegion *flash0 = g_new(MemoryRegion, 1);
>      target_ulong start_addr = memmap[SIFIVE_U_DEV_DRAM].base;
>      target_ulong firmware_end_addr, kernel_start_addr;
> +    const char *firmware_name;
>      uint32_t start_addr_hi32 = 0x00000000;
>      int i;
>      uint32_t fdt_load_addr;
> @@ -595,13 +596,9 @@ static void sifive_u_machine_init(MachineState *machine)
>          break;
>      }
>
> -    if (riscv_is_32bit(&s->soc.u_cpus)) {
> -        firmware_end_addr = riscv_find_and_load_firmware(machine,
> -                                    RISCV32_BIOS_BIN, start_addr, NULL);
> -    } else {
> -        firmware_end_addr = riscv_find_and_load_firmware(machine,
> -                                    RISCV64_BIOS_BIN, start_addr, NULL);
> -    }
> +    firmware_name = riscv_default_firmware_name(&s->soc.u_cpus);
> +    firmware_end_addr = riscv_find_and_load_firmware(machine, firmware_name,
> +                                                     start_addr, NULL);
>
>      if (machine->kernel_filename) {
>          kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc.u_cpus,
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index d96f013e2e..43341c20b6 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -191,6 +191,7 @@ static void spike_board_init(MachineState *machine)
>      MemoryRegion *system_memory = get_system_memory();
>      MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>      target_ulong firmware_end_addr, kernel_start_addr;
> +    const char *firmware_name;
>      uint32_t fdt_load_addr;
>      uint64_t kernel_entry;
>      char *soc_name;
> @@ -261,15 +262,10 @@ static void spike_board_init(MachineState *machine)
>       * keeping ELF files here was intentional because BIN files don't work
>       * for the Spike machine as HTIF emulation depends on ELF parsing.
>       */
> -    if (riscv_is_32bit(&s->soc[0])) {
> -        firmware_end_addr = riscv_find_and_load_firmware(machine,
> -                                    RISCV32_BIOS_BIN, 
> memmap[SPIKE_DRAM].base,
> -                                    htif_symbol_callback);
> -    } else {
> -        firmware_end_addr = riscv_find_and_load_firmware(machine,
> -                                    RISCV64_BIOS_BIN, 
> memmap[SPIKE_DRAM].base,
> -                                    htif_symbol_callback);
> -    }
> +    firmware_name = riscv_default_firmware_name(&s->soc[0]);
> +    firmware_end_addr = riscv_find_and_load_firmware(machine, firmware_name,
> +                                                     memmap[SPIKE_DRAM].base,
> +                                                     htif_symbol_callback);
>
>      /* Load kernel */
>      if (machine->kernel_filename) {
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 94ff2a1584..408f7a2256 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1237,6 +1237,7 @@ static void virt_machine_done(Notifier *notifier, void 
> *data)
>      MachineState *machine = MACHINE(s);
>      target_ulong start_addr = memmap[VIRT_DRAM].base;
>      target_ulong firmware_end_addr, kernel_start_addr;
> +    const char *firmware_name = riscv_default_firmware_name(&s->soc[0]);
>      uint32_t fdt_load_addr;
>      uint64_t kernel_entry;
>
> @@ -1256,13 +1257,8 @@ static void virt_machine_done(Notifier *notifier, void 
> *data)
>          }
>      }
>
> -    if (riscv_is_32bit(&s->soc[0])) {
> -        firmware_end_addr = riscv_find_and_load_firmware(machine,
> -                                    RISCV32_BIOS_BIN, start_addr, NULL);
> -    } else {
> -        firmware_end_addr = riscv_find_and_load_firmware(machine,
> -                                    RISCV64_BIOS_BIN, start_addr, NULL);
> -    }
> +    firmware_end_addr = riscv_find_and_load_firmware(machine, firmware_name,
> +                                                     start_addr, NULL);
>
>      /*
>       * Init fw_cfg.  Must be done before riscv_load_fdt, otherwise the device
> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> index c03e4e74c5..60cf320c88 100644
> --- a/include/hw/riscv/boot.h
> +++ b/include/hw/riscv/boot.h
> @@ -37,6 +37,7 @@ target_ulong riscv_find_and_load_firmware(MachineState 
> *machine,
>                                            const char 
> *default_machine_firmware,
>                                            hwaddr firmware_load_addr,
>                                            symbol_fn_t sym_cb);
> +const char *riscv_default_firmware_name(RISCVHartArrayState *harts);
>  target_ulong riscv_load_firmware(const char *firmware_filename,
>                                   hwaddr firmware_load_addr,
>                                   symbol_fn_t sym_cb);
> --
> 2.38.1
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]