On Sat, Jan 15, 2022 at 2:18 AM Anup Patel <apa...@ventanamicro.com> wrote:
>
> Currently, we have to use OpenSBI firmware ELF as bios for the spike
> machine because the HTIF console requires ELF for parsing "fromhost"
> and "tohost" symbols.
>
> The latest OpenSBI can now optionally pick-up HTIF register address
> from HTIF DT node so using this feature spike machine can now use
> OpenSBI firmware BIN as bios.
Cool!
I think that means we can remove the elfs from QEMU.
>
> Signed-off-by: Anup Patel <apa...@ventanamicro.com>
> ---
> hw/char/riscv_htif.c | 33 +++++++++++++++++++----------
> hw/riscv/spike.c | 41 ++++++++++++++++++++++--------------
> include/hw/char/riscv_htif.h | 5 ++++-
> include/hw/riscv/spike.h | 1 +
> 4 files changed, 52 insertions(+), 28 deletions(-)
>
> diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
> index ddae738d56..b59d321fb7 100644
> --- a/hw/char/riscv_htif.c
> +++ b/hw/char/riscv_htif.c
> @@ -228,13 +228,25 @@ static const MemoryRegionOps htif_mm_ops = {
> .write = htif_mm_write,
> };
>
> +bool htif_uses_elf_symbols(void)
> +{
> + return (address_symbol_set == 3) ? true : false;
> +}
> +
> HTIFState *htif_mm_init(MemoryRegion *address_space, MemoryRegion *main_mem,
> - CPURISCVState *env, Chardev *chr)
> + CPURISCVState *env, Chardev *chr, uint64_t nonelf_base)
> {
> - uint64_t base = MIN(tohost_addr, fromhost_addr);
> - uint64_t size = MAX(tohost_addr + 8, fromhost_addr + 8) - base;
> - uint64_t tohost_offset = tohost_addr - base;
> - uint64_t fromhost_offset = fromhost_addr - base;
> + uint64_t base, size, tohost_offset, fromhost_offset;
> +
> + if (address_symbol_set != 3) {
Why not use htif_uses_elf_symbols() ?
Alistair
> + fromhost_addr = nonelf_base;
> + tohost_addr = nonelf_base + 8;
> + }
> +
> + base = MIN(tohost_addr, fromhost_addr);
> + size = MAX(tohost_addr + 8, fromhost_addr + 8) - base;
> + tohost_offset = tohost_addr - base;
> + fromhost_offset = fromhost_addr - base;
>
> HTIFState *s = g_malloc0(sizeof(HTIFState));
> s->address_space = address_space;
> @@ -249,12 +261,11 @@ HTIFState *htif_mm_init(MemoryRegion *address_space,
> MemoryRegion *main_mem,
> qemu_chr_fe_init(&s->chr, chr, &error_abort);
> qemu_chr_fe_set_handlers(&s->chr, htif_can_recv, htif_recv, htif_event,
> htif_be_change, s, NULL, true);
> - if (address_symbol_set == 3) {
> - memory_region_init_io(&s->mmio, NULL, &htif_mm_ops, s,
> - TYPE_HTIF_UART, size);
> - memory_region_add_subregion_overlap(address_space, base,
> - &s->mmio, 1);
> - }
> +
> + memory_region_init_io(&s->mmio, NULL, &htif_mm_ops, s,
> + TYPE_HTIF_UART, size);
> + memory_region_add_subregion_overlap(address_space, base,
> + &s->mmio, 1);
>
> return s;
> }
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 288d69cd9f..597df4c288 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -42,6 +42,7 @@
>
> static const MemMapEntry spike_memmap[] = {
> [SPIKE_MROM] = { 0x1000, 0xf000 },
> + [SPIKE_HTIF] = { 0x1000000, 0x1000 },
> [SPIKE_CLINT] = { 0x2000000, 0x10000 },
> [SPIKE_DRAM] = { 0x80000000, 0x0 },
> };
> @@ -75,6 +76,10 @@ static void create_fdt(SpikeState *s, const MemMapEntry
> *memmap,
>
> qemu_fdt_add_subnode(fdt, "/htif");
> qemu_fdt_setprop_string(fdt, "/htif", "compatible", "ucb,htif0");
> + if (!htif_uses_elf_symbols()) {
> + qemu_fdt_setprop_cells(fdt, "/htif", "reg",
> + 0x0, memmap[SPIKE_HTIF].base, 0x0, memmap[SPIKE_HTIF].size);
> + }
>
> qemu_fdt_add_subnode(fdt, "/soc");
> qemu_fdt_setprop(fdt, "/soc", "ranges", NULL, 0);
> @@ -172,6 +177,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry
> *memmap,
> if (cmdline) {
> qemu_fdt_add_subnode(fdt, "/chosen");
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> + qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
> }
> }
>
> @@ -241,10 +247,6 @@ static void spike_board_init(MachineState *machine)
> memory_region_add_subregion(system_memory, memmap[SPIKE_DRAM].base,
> machine->ram);
>
> - /* create device tree */
> - create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> - riscv_is_32bit(&s->soc[0]));
> -
> /* boot rom */
> memory_region_init_rom(mask_rom, NULL, "riscv.spike.mrom",
> memmap[SPIKE_MROM].size, &error_fatal);
> @@ -266,6 +268,7 @@ static void spike_board_init(MachineState *machine)
> htif_symbol_callback);
> }
>
> + /* Load kernel */
> if (machine->kernel_filename) {
> kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0],
> firmware_end_addr);
> @@ -273,17 +276,6 @@ static void spike_board_init(MachineState *machine)
> kernel_entry = riscv_load_kernel(machine->kernel_filename,
> kernel_start_addr,
> htif_symbol_callback);
> -
> - if (machine->initrd_filename) {
> - hwaddr start;
> - hwaddr end = riscv_load_initrd(machine->initrd_filename,
> - machine->ram_size, kernel_entry,
> - &start);
> - qemu_fdt_setprop_cell(s->fdt, "/chosen",
> - "linux,initrd-start", start);
> - qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
> - end);
> - }
> } else {
> /*
> * If dynamic firmware is used, it doesn't know where is the next mode
> @@ -292,6 +284,22 @@ static void spike_board_init(MachineState *machine)
> kernel_entry = 0;
> }
>
> + /* Create device tree */
> + create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> + riscv_is_32bit(&s->soc[0]));
> +
> + /* Load initrd */
> + if (machine->kernel_filename && machine->initrd_filename) {
> + hwaddr start;
> + hwaddr end = riscv_load_initrd(machine->initrd_filename,
> + machine->ram_size, kernel_entry,
> + &start);
> + qemu_fdt_setprop_cell(s->fdt, "/chosen",
> + "linux,initrd-start", start);
> + qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
> + end);
> + }
> +
> /* Compute the fdt load address in dram */
> fdt_load_addr = riscv_load_fdt(memmap[SPIKE_DRAM].base,
> machine->ram_size, s->fdt);
> @@ -303,7 +311,8 @@ static void spike_board_init(MachineState *machine)
>
> /* initialize HTIF using symbols found in load_kernel */
> htif_mm_init(system_memory, mask_rom,
> - &s->soc[0].harts[0].env, serial_hd(0));
> + &s->soc[0].harts[0].env, serial_hd(0),
> + memmap[SPIKE_HTIF].base);
> }
>
> static void spike_machine_instance_init(Object *obj)
> diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h
> index fb9452cf51..f888ac1b30 100644
> --- a/include/hw/char/riscv_htif.h
> +++ b/include/hw/char/riscv_htif.h
> @@ -52,8 +52,11 @@ extern const MemoryRegionOps htif_io_ops;
> void htif_symbol_callback(const char *st_name, int st_info, uint64_t
> st_value,
> uint64_t st_size);
>
> +/* Check if HTIF uses ELF symbols */
> +bool htif_uses_elf_symbols(void);
> +
> /* legacy pre qom */
> HTIFState *htif_mm_init(MemoryRegion *address_space, MemoryRegion *main_mem,
> - CPURISCVState *env, Chardev *chr);
> + CPURISCVState *env, Chardev *chr, uint64_t nonelf_base);
>
> #endif
> diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
> index cdd1a13011..73d69234de 100644
> --- a/include/hw/riscv/spike.h
> +++ b/include/hw/riscv/spike.h
> @@ -43,6 +43,7 @@ struct SpikeState {
>
> enum {
> SPIKE_MROM,
> + SPIKE_HTIF,
> SPIKE_CLINT,
> SPIKE_DRAM
> };
> --
> 2.25.1
>
>
