| Message ID | 20221221182300.307900-7-dbarboza@ventanamicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: opensbi boot test and cleanups | expand |
On 21/12/22 19:22, Daniel Henrique Barboza wrote: > This will make the code more in line with what the other boards are > doing. We'll also avoid an extra check to machine->kernel_filename since > we already checked that before executing riscv_load_kernel(). > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > hw/riscv/spike.c | 31 +++++++++++++++---------------- > 1 file changed, 15 insertions(+), 16 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On Thu, Dec 22, 2022 at 4:28 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > This will make the code more in line with what the other boards are > doing. We'll also avoid an extra check to machine->kernel_filename since > we already checked that before executing riscv_load_kernel(). > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > hw/riscv/spike.c | 31 +++++++++++++++---------------- > 1 file changed, 15 insertions(+), 16 deletions(-) > > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c > index 43341c20b6..f37a9bebbf 100644 > --- a/hw/riscv/spike.c > +++ b/hw/riscv/spike.c > @@ -257,6 +257,10 @@ static void spike_board_init(MachineState *machine) > memory_region_add_subregion(system_memory, memmap[SPIKE_MROM].base, > mask_rom); > > + /* Create device tree */ > + create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline, > + riscv_is_32bit(&s->soc[0])); > + > /* > * Not like other RISC-V machines that use plain binary bios images, > * keeping ELF files here was intentional because BIN files don't work > @@ -275,6 +279,17 @@ 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(machine->fdt, "/chosen", > + "linux,initrd-start", start); > + qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end", > + end); > + } > } else { > /* > * If dynamic firmware is used, it doesn't know where is the next mode > @@ -283,22 +298,6 @@ 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(machine->fdt, "/chosen", > - "linux,initrd-start", start); > - qemu_fdt_setprop_cell(machine->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, machine->fdt); > -- > 2.38.1 > >
On Thu, Dec 22, 2022 at 2:28 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > This will make the code more in line with what the other boards are > doing. We'll also avoid an extra check to machine->kernel_filename since > we already checked that before executing riscv_load_kernel(). > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > hw/riscv/spike.c | 31 +++++++++++++++---------------- > 1 file changed, 15 insertions(+), 16 deletions(-) > Reviewed-by: Bin Meng <bmeng@tinylab.org>
On Fri, Dec 23, 2022 at 6:04 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > On Thu, Dec 22, 2022 at 2:28 AM Daniel Henrique Barboza > <dbarboza@ventanamicro.com> wrote: > > > > This will make the code more in line with what the other boards are > > doing. We'll also avoid an extra check to machine->kernel_filename since > > we already checked that before executing riscv_load_kernel(). > > > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > > --- > > hw/riscv/spike.c | 31 +++++++++++++++---------------- > > 1 file changed, 15 insertions(+), 16 deletions(-) > > > > Reviewed-by: Bin Meng <bmeng@tinylab.org> This change unfortunately breaks the ELF boot on Spike. I will propose a patch to fix such unexpected dependency. Regards, Bin
On 12/26/22 10:49, Bin Meng wrote: > On Fri, Dec 23, 2022 at 6:04 PM Bin Meng <bmeng.cn@gmail.com> wrote: >> On Thu, Dec 22, 2022 at 2:28 AM Daniel Henrique Barboza >> <dbarboza@ventanamicro.com> wrote: >>> This will make the code more in line with what the other boards are >>> doing. We'll also avoid an extra check to machine->kernel_filename since >>> we already checked that before executing riscv_load_kernel(). >>> >>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >>> --- >>> hw/riscv/spike.c | 31 +++++++++++++++---------------- >>> 1 file changed, 15 insertions(+), 16 deletions(-) >>> >> Reviewed-by: Bin Meng <bmeng@tinylab.org> > This change unfortunately breaks the ELF boot on Spike. > > I will propose a patch to fix such unexpected dependency. Interesting. This is one of the most benign changes I did, or so I thought. I believe we should wait for you patch fixing it first. Daniel > > Regards, > Bin
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c index 43341c20b6..f37a9bebbf 100644 --- a/hw/riscv/spike.c +++ b/hw/riscv/spike.c @@ -257,6 +257,10 @@ static void spike_board_init(MachineState *machine) memory_region_add_subregion(system_memory, memmap[SPIKE_MROM].base, mask_rom); + /* Create device tree */ + create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline, + riscv_is_32bit(&s->soc[0])); + /* * Not like other RISC-V machines that use plain binary bios images, * keeping ELF files here was intentional because BIN files don't work @@ -275,6 +279,17 @@ 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(machine->fdt, "/chosen", + "linux,initrd-start", start); + qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end", + end); + } } else { /* * If dynamic firmware is used, it doesn't know where is the next mode @@ -283,22 +298,6 @@ 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(machine->fdt, "/chosen", - "linux,initrd-start", start); - qemu_fdt_setprop_cell(machine->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, machine->fdt);
This will make the code more in line with what the other boards are doing. We'll also avoid an extra check to machine->kernel_filename since we already checked that before executing riscv_load_kernel(). Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- hw/riscv/spike.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)