On Thu, Dec 22, 2022 at 4:28 AM Daniel Henrique Barboza
<dbarb...@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 <pal...@dabbelt.com>
> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.fran...@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
>
>
