[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/riscv: spike: Allow using binary firmware as bios
|
From: |
Anup Patel |
|
Subject: |
Re: [PATCH] hw/riscv: spike: Allow using binary firmware as bios |
|
Date: |
Tue, 18 Jan 2022 10:57:22 +0530 |
On Tue, Jan 18, 2022 at 10:50 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Sat, Jan 15, 2022 at 2:18 AM Anup Patel <apatel@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.
Yes, we can definitely remove the ELFs from QEMU.
>
> >
> > Signed-off-by: Anup Patel <apatel@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() ?
Ahh, yes.
I should have used htif_uses_elf_symbols() here. I will update
it in the next revision.
Regards,
Anup
>
> 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
> >
> >